* [U-Boot] [PATCH] regmap: fix regmap_read_poll_timeout warning about sandbox_timer_add_offset
@ 2019-04-11 15:12 ` Neil Armstrong
0 siblings, 0 replies; 8+ messages in thread
From: Neil Armstrong @ 2019-04-11 15:12 UTC (permalink / raw)
To: u-boot
When fixing sandbox test for regmap_read_poll_timeout(), the
sandbox_timer_add_offset was introduced but only defined in sandbox code
thus generating warnings when used out of sandbox :
include/regmap.h:289:2: note: in expansion of macro 'regmap_read_poll_timeout_test'
regmap_read_poll_timeout_test(map, addr, val, cond, sleep_us, \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/spi/meson_spifc.c:169:8: note: in expansion of macro 'regmap_read_poll_timeout'
ret = regmap_read_poll_timeout(spifc->regmap, REG_SLAVE, data,
^~~~~~~~~~~~~~~~~~~~~~~~
drivers/spi/meson_spifc.c: In function 'meson_spifc_txrx':
include/regmap.h:277:4: warning: implicit declaration of function 'sandbox_timer_add_offset' [-Wimplicit-function-declaration]
This simply adds an empty static inline sandbox_timer_add_offset()
implementation that will be dropped by the compiler optimization routines.
Cc: Simon Glass <sjg@chromium.org>
Reported-by: Tom Rini <trini@konsulko.com>
Fixes: df9cf1cc08 ("test: dm: regmap: Fix the long test delay")
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
include/regmap.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/regmap.h b/include/regmap.h
index 8359c511d2..144cffbcc0 100644
--- a/include/regmap.h
+++ b/include/regmap.h
@@ -239,6 +239,10 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
#define regmap_get(map, type, member, valp) \
regmap_range_get(map, 0, type, member, valp)
+#ifndef CONFIG_SANDBOX
+static inline void sandbox_timer_add_offset(unsigned long offset) { }
+#endif
+
/**
* regmap_read_poll_timeout - Poll until a condition is met or a timeout occurs
*
--
2.21.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] regmap: fix regmap_read_poll_timeout warning about sandbox_timer_add_offset
@ 2019-04-11 15:12 ` Neil Armstrong
0 siblings, 0 replies; 8+ messages in thread
From: Neil Armstrong @ 2019-04-11 15:12 UTC (permalink / raw)
To: Simon Glass, Tom Rini; +Cc: Neil Armstrong, u-boot, u-boot-amlogic
When fixing sandbox test for regmap_read_poll_timeout(), the
sandbox_timer_add_offset was introduced but only defined in sandbox code
thus generating warnings when used out of sandbox :
include/regmap.h:289:2: note: in expansion of macro 'regmap_read_poll_timeout_test'
regmap_read_poll_timeout_test(map, addr, val, cond, sleep_us, \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/spi/meson_spifc.c:169:8: note: in expansion of macro 'regmap_read_poll_timeout'
ret = regmap_read_poll_timeout(spifc->regmap, REG_SLAVE, data,
^~~~~~~~~~~~~~~~~~~~~~~~
drivers/spi/meson_spifc.c: In function 'meson_spifc_txrx':
include/regmap.h:277:4: warning: implicit declaration of function 'sandbox_timer_add_offset' [-Wimplicit-function-declaration]
This simply adds an empty static inline sandbox_timer_add_offset()
implementation that will be dropped by the compiler optimization routines.
Cc: Simon Glass <sjg@chromium.org>
Reported-by: Tom Rini <trini@konsulko.com>
Fixes: df9cf1cc08 ("test: dm: regmap: Fix the long test delay")
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
include/regmap.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/regmap.h b/include/regmap.h
index 8359c511d2..144cffbcc0 100644
--- a/include/regmap.h
+++ b/include/regmap.h
@@ -239,6 +239,10 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
#define regmap_get(map, type, member, valp) \
regmap_range_get(map, 0, type, member, valp)
+#ifndef CONFIG_SANDBOX
+static inline void sandbox_timer_add_offset(unsigned long offset) { }
+#endif
+
/**
* regmap_read_poll_timeout - Poll until a condition is met or a timeout occurs
*
--
2.21.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] regmap: fix regmap_read_poll_timeout warning about sandbox_timer_add_offset
2019-04-11 15:12 ` Neil Armstrong
@ 2019-04-12 13:09 ` Simon Glass
-1 siblings, 0 replies; 8+ messages in thread
From: Simon Glass @ 2019-04-12 13:09 UTC (permalink / raw)
To: u-boot
Hi,
On Thu, 11 Apr 2019 at 09:12, Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> When fixing sandbox test for regmap_read_poll_timeout(), the
> sandbox_timer_add_offset was introduced but only defined in sandbox code
> thus generating warnings when used out of sandbox :
>
> include/regmap.h:289:2: note: in expansion of macro 'regmap_read_poll_timeout_test'
> regmap_read_poll_timeout_test(map, addr, val, cond, sleep_us, \
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/spi/meson_spifc.c:169:8: note: in expansion of macro 'regmap_read_poll_timeout'
> ret = regmap_read_poll_timeout(spifc->regmap, REG_SLAVE, data,
> ^~~~~~~~~~~~~~~~~~~~~~~~
> drivers/spi/meson_spifc.c: In function 'meson_spifc_txrx':
> include/regmap.h:277:4: warning: implicit declaration of function 'sandbox_timer_add_offset' [-Wimplicit-function-declaration]
>
> This simply adds an empty static inline sandbox_timer_add_offset()
> implementation that will be dropped by the compiler optimization routines.
>
> Cc: Simon Glass <sjg@chromium.org>
> Reported-by: Tom Rini <trini@konsulko.com>
> Fixes: df9cf1cc08 ("test: dm: regmap: Fix the long test delay")
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> include/regmap.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
This is OK with me, but I wonder if it would be better to change the code:
if (IS_ENABLED(CONFIG_SANDBOX) && test_add_time) \
sandbox_timer_add_offset(test_add_time); \
to something like
do_test_add_time(test_add_time);
where do_test_add_time() is defined above as either a sandbox call, or empty?
The reason I say this is it seems a bit weird to define a function
called sandbox_... in non-sandbox code.
> diff --git a/include/regmap.h b/include/regmap.h
> index 8359c511d2..144cffbcc0 100644
> --- a/include/regmap.h
> +++ b/include/regmap.h
> @@ -239,6 +239,10 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
> #define regmap_get(map, type, member, valp) \
> regmap_range_get(map, 0, type, member, valp)
>
> +#ifndef CONFIG_SANDBOX
> +static inline void sandbox_timer_add_offset(unsigned long offset) { }
> +#endif
> +
> /**
> * regmap_read_poll_timeout - Poll until a condition is met or a timeout occurs
> *
> --
> 2.21.0
>
Regards,
Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] regmap: fix regmap_read_poll_timeout warning about sandbox_timer_add_offset
@ 2019-04-12 13:09 ` Simon Glass
0 siblings, 0 replies; 8+ messages in thread
From: Simon Glass @ 2019-04-12 13:09 UTC (permalink / raw)
To: Neil Armstrong; +Cc: Tom Rini, U-Boot Mailing List, u-boot-amlogic
Hi,
On Thu, 11 Apr 2019 at 09:12, Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> When fixing sandbox test for regmap_read_poll_timeout(), the
> sandbox_timer_add_offset was introduced but only defined in sandbox code
> thus generating warnings when used out of sandbox :
>
> include/regmap.h:289:2: note: in expansion of macro 'regmap_read_poll_timeout_test'
> regmap_read_poll_timeout_test(map, addr, val, cond, sleep_us, \
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/spi/meson_spifc.c:169:8: note: in expansion of macro 'regmap_read_poll_timeout'
> ret = regmap_read_poll_timeout(spifc->regmap, REG_SLAVE, data,
> ^~~~~~~~~~~~~~~~~~~~~~~~
> drivers/spi/meson_spifc.c: In function 'meson_spifc_txrx':
> include/regmap.h:277:4: warning: implicit declaration of function 'sandbox_timer_add_offset' [-Wimplicit-function-declaration]
>
> This simply adds an empty static inline sandbox_timer_add_offset()
> implementation that will be dropped by the compiler optimization routines.
>
> Cc: Simon Glass <sjg@chromium.org>
> Reported-by: Tom Rini <trini@konsulko.com>
> Fixes: df9cf1cc08 ("test: dm: regmap: Fix the long test delay")
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> include/regmap.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
This is OK with me, but I wonder if it would be better to change the code:
if (IS_ENABLED(CONFIG_SANDBOX) && test_add_time) \
sandbox_timer_add_offset(test_add_time); \
to something like
do_test_add_time(test_add_time);
where do_test_add_time() is defined above as either a sandbox call, or empty?
The reason I say this is it seems a bit weird to define a function
called sandbox_... in non-sandbox code.
> diff --git a/include/regmap.h b/include/regmap.h
> index 8359c511d2..144cffbcc0 100644
> --- a/include/regmap.h
> +++ b/include/regmap.h
> @@ -239,6 +239,10 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
> #define regmap_get(map, type, member, valp) \
> regmap_range_get(map, 0, type, member, valp)
>
> +#ifndef CONFIG_SANDBOX
> +static inline void sandbox_timer_add_offset(unsigned long offset) { }
> +#endif
> +
> /**
> * regmap_read_poll_timeout - Poll until a condition is met or a timeout occurs
> *
> --
> 2.21.0
>
Regards,
Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] regmap: fix regmap_read_poll_timeout warning about sandbox_timer_add_offset
2019-04-12 13:09 ` Simon Glass
@ 2019-04-12 14:56 ` Neil Armstrong
-1 siblings, 0 replies; 8+ messages in thread
From: Neil Armstrong @ 2019-04-12 14:56 UTC (permalink / raw)
To: u-boot
Hi,
On 12/04/2019 15:09, Simon Glass wrote:
> Hi,
>
> On Thu, 11 Apr 2019 at 09:12, Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> When fixing sandbox test for regmap_read_poll_timeout(), the
>> sandbox_timer_add_offset was introduced but only defined in sandbox code
>> thus generating warnings when used out of sandbox :
>>
>> include/regmap.h:289:2: note: in expansion of macro 'regmap_read_poll_timeout_test'
>> regmap_read_poll_timeout_test(map, addr, val, cond, sleep_us, \
>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/spi/meson_spifc.c:169:8: note: in expansion of macro 'regmap_read_poll_timeout'
>> ret = regmap_read_poll_timeout(spifc->regmap, REG_SLAVE, data,
>> ^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/spi/meson_spifc.c: In function 'meson_spifc_txrx':
>> include/regmap.h:277:4: warning: implicit declaration of function 'sandbox_timer_add_offset' [-Wimplicit-function-declaration]
>>
>> This simply adds an empty static inline sandbox_timer_add_offset()
>> implementation that will be dropped by the compiler optimization routines.
>>
>> Cc: Simon Glass <sjg@chromium.org>
>> Reported-by: Tom Rini <trini@konsulko.com>
>> Fixes: df9cf1cc08 ("test: dm: regmap: Fix the long test delay")
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> include/regmap.h | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>
> This is OK with me, but I wonder if it would be better to change the code:
>
> if (IS_ENABLED(CONFIG_SANDBOX) && test_add_time) \
> sandbox_timer_add_offset(test_add_time); \
>
> to something like
>
> do_test_add_time(test_add_time);
>
> where do_test_add_time() is defined above as either a sandbox call, or empty?
>
> The reason I say this is it seems a bit weird to define a function
> called sandbox_... in non-sandbox code.
Yes, but it still seems weird to have empty test code defined in this header.
Can't we use sandbox_state->skip_delays here instead ?
Neil
>
>> diff --git a/include/regmap.h b/include/regmap.h
>> index 8359c511d2..144cffbcc0 100644
>> --- a/include/regmap.h
>> +++ b/include/regmap.h
>> @@ -239,6 +239,10 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
>> #define regmap_get(map, type, member, valp) \
>> regmap_range_get(map, 0, type, member, valp)
>>
>> +#ifndef CONFIG_SANDBOX
>> +static inline void sandbox_timer_add_offset(unsigned long offset) { }
>> +#endif
>> +
>> /**
>> * regmap_read_poll_timeout - Poll until a condition is met or a timeout occurs
>> *
>> --
>> 2.21.0
>>
>
> Regards,
> Simon
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] regmap: fix regmap_read_poll_timeout warning about sandbox_timer_add_offset
@ 2019-04-12 14:56 ` Neil Armstrong
0 siblings, 0 replies; 8+ messages in thread
From: Neil Armstrong @ 2019-04-12 14:56 UTC (permalink / raw)
To: Simon Glass; +Cc: Tom Rini, U-Boot Mailing List, u-boot-amlogic
Hi,
On 12/04/2019 15:09, Simon Glass wrote:
> Hi,
>
> On Thu, 11 Apr 2019 at 09:12, Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> When fixing sandbox test for regmap_read_poll_timeout(), the
>> sandbox_timer_add_offset was introduced but only defined in sandbox code
>> thus generating warnings when used out of sandbox :
>>
>> include/regmap.h:289:2: note: in expansion of macro 'regmap_read_poll_timeout_test'
>> regmap_read_poll_timeout_test(map, addr, val, cond, sleep_us, \
>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/spi/meson_spifc.c:169:8: note: in expansion of macro 'regmap_read_poll_timeout'
>> ret = regmap_read_poll_timeout(spifc->regmap, REG_SLAVE, data,
>> ^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/spi/meson_spifc.c: In function 'meson_spifc_txrx':
>> include/regmap.h:277:4: warning: implicit declaration of function 'sandbox_timer_add_offset' [-Wimplicit-function-declaration]
>>
>> This simply adds an empty static inline sandbox_timer_add_offset()
>> implementation that will be dropped by the compiler optimization routines.
>>
>> Cc: Simon Glass <sjg@chromium.org>
>> Reported-by: Tom Rini <trini@konsulko.com>
>> Fixes: df9cf1cc08 ("test: dm: regmap: Fix the long test delay")
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> include/regmap.h | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>
> This is OK with me, but I wonder if it would be better to change the code:
>
> if (IS_ENABLED(CONFIG_SANDBOX) && test_add_time) \
> sandbox_timer_add_offset(test_add_time); \
>
> to something like
>
> do_test_add_time(test_add_time);
>
> where do_test_add_time() is defined above as either a sandbox call, or empty?
>
> The reason I say this is it seems a bit weird to define a function
> called sandbox_... in non-sandbox code.
Yes, but it still seems weird to have empty test code defined in this header.
Can't we use sandbox_state->skip_delays here instead ?
Neil
>
>> diff --git a/include/regmap.h b/include/regmap.h
>> index 8359c511d2..144cffbcc0 100644
>> --- a/include/regmap.h
>> +++ b/include/regmap.h
>> @@ -239,6 +239,10 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
>> #define regmap_get(map, type, member, valp) \
>> regmap_range_get(map, 0, type, member, valp)
>>
>> +#ifndef CONFIG_SANDBOX
>> +static inline void sandbox_timer_add_offset(unsigned long offset) { }
>> +#endif
>> +
>> /**
>> * regmap_read_poll_timeout - Poll until a condition is met or a timeout occurs
>> *
>> --
>> 2.21.0
>>
>
> Regards,
> Simon
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] regmap: fix regmap_read_poll_timeout warning about sandbox_timer_add_offset
2019-04-12 14:56 ` Neil Armstrong
@ 2019-04-12 15:52 ` Neil Armstrong
-1 siblings, 0 replies; 8+ messages in thread
From: Neil Armstrong @ 2019-04-12 15:52 UTC (permalink / raw)
To: u-boot
On 12/04/2019 16:56, Neil Armstrong wrote:
> Hi,
>
>
> On 12/04/2019 15:09, Simon Glass wrote:
>> Hi,
>>
>> On Thu, 11 Apr 2019 at 09:12, Neil Armstrong <narmstrong@baylibre.com> wrote:
>>>
>>> When fixing sandbox test for regmap_read_poll_timeout(), the
>>> sandbox_timer_add_offset was introduced but only defined in sandbox code
>>> thus generating warnings when used out of sandbox :
>>>
>>> include/regmap.h:289:2: note: in expansion of macro 'regmap_read_poll_timeout_test'
>>> regmap_read_poll_timeout_test(map, addr, val, cond, sleep_us, \
>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/spi/meson_spifc.c:169:8: note: in expansion of macro 'regmap_read_poll_timeout'
>>> ret = regmap_read_poll_timeout(spifc->regmap, REG_SLAVE, data,
>>> ^~~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/spi/meson_spifc.c: In function 'meson_spifc_txrx':
>>> include/regmap.h:277:4: warning: implicit declaration of function 'sandbox_timer_add_offset' [-Wimplicit-function-declaration]
>>>
>>> This simply adds an empty static inline sandbox_timer_add_offset()
>>> implementation that will be dropped by the compiler optimization routines.
>>>
>>> Cc: Simon Glass <sjg@chromium.org>
>>> Reported-by: Tom Rini <trini@konsulko.com>
>>> Fixes: df9cf1cc08 ("test: dm: regmap: Fix the long test delay")
>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>> ---
>>> include/regmap.h | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>
>> This is OK with me, but I wonder if it would be better to change the code:
>>
>> if (IS_ENABLED(CONFIG_SANDBOX) && test_add_time) \
>> sandbox_timer_add_offset(test_add_time); \
>>
>> to something like
>>
>> do_test_add_time(test_add_time);
>>
>> where do_test_add_time() is defined above as either a sandbox call, or empty?
>>
>> The reason I say this is it seems a bit weird to define a function
>> called sandbox_... in non-sandbox code.
>
> Yes, but it still seems weird to have empty test code defined in this header.
>
> Can't we use sandbox_state->skip_delays here instead ?
What about the following ?
-----><--------------------------------------------
diff --git a/arch/sandbox/include/asm/test.h b/arch/sandbox/include/asm/test.h
index fc52f47f82..e956a05262 100644
--- a/arch/sandbox/include/asm/test.h
+++ b/arch/sandbox/include/asm/test.h
@@ -59,14 +59,6 @@ void sandbox_i2c_eeprom_set_test_mode(struct udevice *dev,
void sandbox_i2c_eeprom_set_offset_len(struct udevice *dev, int offset_len);
-/*
- * sandbox_timer_add_offset()
- *
- * Allow tests to add to the time reported through lib/time.c functions
- * offset: number of milliseconds to advance the system time
- */
-void sandbox_timer_add_offset(unsigned long offset);
-
/**
* sandbox_i2c_rtc_set_offset() - set the time offset from system/base time
*
diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
index 397e7561d4..9ca1eca027 100644
--- a/board/sandbox/sandbox.c
+++ b/board/sandbox/sandbox.c
@@ -31,7 +31,7 @@ void flush_cache(unsigned long start, unsigned long size)
/* system timer offset in ms */
static unsigned long sandbox_timer_offset;
-void sandbox_timer_add_offset(unsigned long offset)
+void timer_test_add_offset(unsigned long offset)
{
sandbox_timer_offset += offset;
}
diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c
index decce2fa59..c136392350 100644
--- a/drivers/net/sandbox.c
+++ b/drivers/net/sandbox.c
@@ -350,7 +350,7 @@ static int sb_eth_recv(struct udevice *dev, int flags, uchar **packetp)
struct eth_sandbox_priv *priv = dev_get_priv(dev);
if (skip_timeout) {
- sandbox_timer_add_offset(11000UL);
+ timer_test_add_offset(11000UL);
skip_timeout = false;
}
diff --git a/drivers/timer/sandbox_timer.c b/drivers/timer/sandbox_timer.c
index 6d2b045fe8..5228486082 100644
--- a/drivers/timer/sandbox_timer.c
+++ b/drivers/timer/sandbox_timer.c
@@ -14,7 +14,7 @@
/* system timer offset in ms */
static unsigned long sandbox_timer_offset;
-void sandbox_timer_add_offset(unsigned long offset)
+void timer_test_add_offset(unsigned long offset)
{
sandbox_timer_offset += offset;
}
diff --git a/include/regmap.h b/include/regmap.h
index 8359c511d2..3cd7a66cea 100644
--- a/include/regmap.h
+++ b/include/regmap.h
@@ -274,7 +274,7 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
if (cond) \
break; \
if (IS_ENABLED(CONFIG_SANDBOX) && test_add_time) \
- sandbox_timer_add_offset(test_add_time); \
+ timer_test_add_offset(test_add_time); \
if ((timeout_ms) && get_timer(__start) > (timeout_ms)) { \
__ret = regmap_read((map), (addr), &(val)); \
break; \
diff --git a/include/time.h b/include/time.h
index 825991e222..9fd0d73fb4 100644
--- a/include/time.h
+++ b/include/time.h
@@ -13,6 +13,14 @@ unsigned long get_timer(unsigned long base);
*/
unsigned long timer_get_us(void);
+/*
+ * timer_test_add_offset()
+ *
+ * Allow tests to add to the time reported through lib/time.c functions
+ * offset: number of milliseconds to advance the system time
+ */
+void timer_test_add_offset(unsigned long offset);
+
/*
* These inlines deal with timer wrapping correctly. You are
* strongly encouraged to use them
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] regmap: fix regmap_read_poll_timeout warning about sandbox_timer_add_offset
@ 2019-04-12 15:52 ` Neil Armstrong
0 siblings, 0 replies; 8+ messages in thread
From: Neil Armstrong @ 2019-04-12 15:52 UTC (permalink / raw)
To: Simon Glass; +Cc: Tom Rini, U-Boot Mailing List, u-boot-amlogic
On 12/04/2019 16:56, Neil Armstrong wrote:
> Hi,
>
>
> On 12/04/2019 15:09, Simon Glass wrote:
>> Hi,
>>
>> On Thu, 11 Apr 2019 at 09:12, Neil Armstrong <narmstrong@baylibre.com> wrote:
>>>
>>> When fixing sandbox test for regmap_read_poll_timeout(), the
>>> sandbox_timer_add_offset was introduced but only defined in sandbox code
>>> thus generating warnings when used out of sandbox :
>>>
>>> include/regmap.h:289:2: note: in expansion of macro 'regmap_read_poll_timeout_test'
>>> regmap_read_poll_timeout_test(map, addr, val, cond, sleep_us, \
>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/spi/meson_spifc.c:169:8: note: in expansion of macro 'regmap_read_poll_timeout'
>>> ret = regmap_read_poll_timeout(spifc->regmap, REG_SLAVE, data,
>>> ^~~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/spi/meson_spifc.c: In function 'meson_spifc_txrx':
>>> include/regmap.h:277:4: warning: implicit declaration of function 'sandbox_timer_add_offset' [-Wimplicit-function-declaration]
>>>
>>> This simply adds an empty static inline sandbox_timer_add_offset()
>>> implementation that will be dropped by the compiler optimization routines.
>>>
>>> Cc: Simon Glass <sjg@chromium.org>
>>> Reported-by: Tom Rini <trini@konsulko.com>
>>> Fixes: df9cf1cc08 ("test: dm: regmap: Fix the long test delay")
>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>> ---
>>> include/regmap.h | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>
>> This is OK with me, but I wonder if it would be better to change the code:
>>
>> if (IS_ENABLED(CONFIG_SANDBOX) && test_add_time) \
>> sandbox_timer_add_offset(test_add_time); \
>>
>> to something like
>>
>> do_test_add_time(test_add_time);
>>
>> where do_test_add_time() is defined above as either a sandbox call, or empty?
>>
>> The reason I say this is it seems a bit weird to define a function
>> called sandbox_... in non-sandbox code.
>
> Yes, but it still seems weird to have empty test code defined in this header.
>
> Can't we use sandbox_state->skip_delays here instead ?
What about the following ?
-----><--------------------------------------------
diff --git a/arch/sandbox/include/asm/test.h b/arch/sandbox/include/asm/test.h
index fc52f47f82..e956a05262 100644
--- a/arch/sandbox/include/asm/test.h
+++ b/arch/sandbox/include/asm/test.h
@@ -59,14 +59,6 @@ void sandbox_i2c_eeprom_set_test_mode(struct udevice *dev,
void sandbox_i2c_eeprom_set_offset_len(struct udevice *dev, int offset_len);
-/*
- * sandbox_timer_add_offset()
- *
- * Allow tests to add to the time reported through lib/time.c functions
- * offset: number of milliseconds to advance the system time
- */
-void sandbox_timer_add_offset(unsigned long offset);
-
/**
* sandbox_i2c_rtc_set_offset() - set the time offset from system/base time
*
diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
index 397e7561d4..9ca1eca027 100644
--- a/board/sandbox/sandbox.c
+++ b/board/sandbox/sandbox.c
@@ -31,7 +31,7 @@ void flush_cache(unsigned long start, unsigned long size)
/* system timer offset in ms */
static unsigned long sandbox_timer_offset;
-void sandbox_timer_add_offset(unsigned long offset)
+void timer_test_add_offset(unsigned long offset)
{
sandbox_timer_offset += offset;
}
diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c
index decce2fa59..c136392350 100644
--- a/drivers/net/sandbox.c
+++ b/drivers/net/sandbox.c
@@ -350,7 +350,7 @@ static int sb_eth_recv(struct udevice *dev, int flags, uchar **packetp)
struct eth_sandbox_priv *priv = dev_get_priv(dev);
if (skip_timeout) {
- sandbox_timer_add_offset(11000UL);
+ timer_test_add_offset(11000UL);
skip_timeout = false;
}
diff --git a/drivers/timer/sandbox_timer.c b/drivers/timer/sandbox_timer.c
index 6d2b045fe8..5228486082 100644
--- a/drivers/timer/sandbox_timer.c
+++ b/drivers/timer/sandbox_timer.c
@@ -14,7 +14,7 @@
/* system timer offset in ms */
static unsigned long sandbox_timer_offset;
-void sandbox_timer_add_offset(unsigned long offset)
+void timer_test_add_offset(unsigned long offset)
{
sandbox_timer_offset += offset;
}
diff --git a/include/regmap.h b/include/regmap.h
index 8359c511d2..3cd7a66cea 100644
--- a/include/regmap.h
+++ b/include/regmap.h
@@ -274,7 +274,7 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
if (cond) \
break; \
if (IS_ENABLED(CONFIG_SANDBOX) && test_add_time) \
- sandbox_timer_add_offset(test_add_time); \
+ timer_test_add_offset(test_add_time); \
if ((timeout_ms) && get_timer(__start) > (timeout_ms)) { \
__ret = regmap_read((map), (addr), &(val)); \
break; \
diff --git a/include/time.h b/include/time.h
index 825991e222..9fd0d73fb4 100644
--- a/include/time.h
+++ b/include/time.h
@@ -13,6 +13,14 @@ unsigned long get_timer(unsigned long base);
*/
unsigned long timer_get_us(void);
+/*
+ * timer_test_add_offset()
+ *
+ * Allow tests to add to the time reported through lib/time.c functions
+ * offset: number of milliseconds to advance the system time
+ */
+void timer_test_add_offset(unsigned long offset);
+
/*
* These inlines deal with timer wrapping correctly. You are
* strongly encouraged to use them
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-04-12 15:52 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-11 15:12 [U-Boot] [PATCH] regmap: fix regmap_read_poll_timeout warning about sandbox_timer_add_offset Neil Armstrong
2019-04-11 15:12 ` Neil Armstrong
2019-04-12 13:09 ` [U-Boot] " Simon Glass
2019-04-12 13:09 ` Simon Glass
2019-04-12 14:56 ` [U-Boot] " Neil Armstrong
2019-04-12 14:56 ` Neil Armstrong
2019-04-12 15:52 ` [U-Boot] " Neil Armstrong
2019-04-12 15:52 ` Neil Armstrong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.