[U-Boot] [PATCH] regmap: fix regmap_read_poll_timeout warning about sandbox_timer_add_offset
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 *
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
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
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
participants (2)
-
Neil Armstrong -
Simon Glass