[U-Boot] [PATCH] test: dm: regmap: Fix the long test delay

[U-Boot] [PATCH] test: dm: regmap: Fix the long test delay

[Simon Glass]

At present one of the regmap tests takes 5 seconds to run since it waits
for a timeout. This should be handled using sandbox_timer_add_offset()
which advances time for test purposes.

This requires a little change to make the regmap_read_poll_timeout()
testable.

Update the macro and the test.

Fixes: ebe3497c9c ("test: regmap: add regmap_read_poll_timeout test")

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/regmap.h | 14 +++++++++++++-
 test/dm/regmap.c |  7 ++++---
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/include/regmap.h b/include/regmap.h
index a3afb72df51..8359c511d25 100644
--- a/include/regmap.h
+++ b/include/regmap.h
@@ -248,6 +248,8 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
  * @cond:	Break condition (usually involving @val)
  * @sleep_us:	Maximum time to sleep between reads in us (0 tight-loops).
  * @timeout_ms:	Timeout in ms, 0 means never timeout
+ * @test_add_time: Used for sandbox testing - amount of time to add after
+ *		starting the loop (0 if not testing)
  *
  * Returns 0 on success and -ETIMEDOUT upon a timeout or the regmap_read
  * error return value in case of a error read. In the two former cases,
@@ -256,8 +258,12 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
  *
  * This is modelled after the regmap_read_poll_timeout macros in linux but
  * with millisecond timeout.
+ *
+ * The _test version is for sandbox testing only. Do not use this in normal
+ * code as it advances the timer.
  */
-#define regmap_read_poll_timeout(map, addr, val, cond, sleep_us, timeout_ms) \
+#define regmap_read_poll_timeout_test(map, addr, val, cond, sleep_us, \
+				      timeout_ms, test_add_time) \
 ({ \
 	unsigned long __start = get_timer(0); \
 	int __ret; \
@@ -267,6 +273,8 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
 			break; \
 		if (cond) \
 			break; \
+		if (IS_ENABLED(CONFIG_SANDBOX) && test_add_time) \
+			sandbox_timer_add_offset(test_add_time); \
 		if ((timeout_ms) && get_timer(__start) > (timeout_ms)) { \
 			__ret = regmap_read((map), (addr), &(val)); \
 			break; \
@@ -277,6 +285,10 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
 	__ret ?: ((cond) ? 0 : -ETIMEDOUT); \
 })
 
+#define regmap_read_poll_timeout(map, addr, val, cond, sleep_us, timeout_ms) \
+	regmap_read_poll_timeout_test(map, addr, val, cond, sleep_us, \
+				      timeout_ms, 0) \
+
 /**
  * regmap_update_bits() - Perform a read/modify/write using a mask
  *
diff --git a/test/dm/regmap.c b/test/dm/regmap.c
index 9a70c159ddb..82de295cb8f 100644
--- a/test/dm/regmap.c
+++ b/test/dm/regmap.c
@@ -160,9 +160,10 @@ static int dm_test_regmap_poll(struct unit_test_state *uts)
 	start = get_timer(0);
 
 	ut_asserteq(-ETIMEDOUT,
-		    regmap_read_poll_timeout(map, 0, reg,
-					     (reg == 0xcacafafa),
-					     1, 5 * CONFIG_SYS_HZ));
+		    regmap_read_poll_timeout_test(map, 0, reg,
+						  (reg == 0xcacafafa),
+						  1, 5 * CONFIG_SYS_HZ,
+						  5 * CONFIG_SYS_HZ));
 
 	ut_assert(get_timer(start) > (5 * CONFIG_SYS_HZ));
 
-- 
2.20.0.rc2.403.gdbc3b29805-goog

[U-Boot] [PATCH] test: dm: regmap: Fix the long test delay

[Neil Armstrong]

On 10/12/2018 01:11, Simon Glass wrote:
> At present one of the regmap tests takes 5 seconds to run since it waits
> for a timeout. This should be handled using sandbox_timer_add_offset()
> which advances time for test purposes.
> 
> This requires a little change to make the regmap_read_poll_timeout()
> testable.
> 
> Update the macro and the test.
> 
> Fixes: ebe3497c9c ("test: regmap: add regmap_read_poll_timeout test")
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  include/regmap.h | 14 +++++++++++++-
>  test/dm/regmap.c |  7 ++++---
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/include/regmap.h b/include/regmap.h
> index a3afb72df51..8359c511d25 100644
> --- a/include/regmap.h
> +++ b/include/regmap.h
> @@ -248,6 +248,8 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
>   * @cond:	Break condition (usually involving @val)
>   * @sleep_us:	Maximum time to sleep between reads in us (0 tight-loops).
>   * @timeout_ms:	Timeout in ms, 0 means never timeout
> + * @test_add_time: Used for sandbox testing - amount of time to add after
> + *		starting the loop (0 if not testing)
>   *
>   * Returns 0 on success and -ETIMEDOUT upon a timeout or the regmap_read
>   * error return value in case of a error read. In the two former cases,
> @@ -256,8 +258,12 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
>   *
>   * This is modelled after the regmap_read_poll_timeout macros in linux but
>   * with millisecond timeout.
> + *
> + * The _test version is for sandbox testing only. Do not use this in normal
> + * code as it advances the timer.
>   */
> -#define regmap_read_poll_timeout(map, addr, val, cond, sleep_us, timeout_ms) \
> +#define regmap_read_poll_timeout_test(map, addr, val, cond, sleep_us, \
> +				      timeout_ms, test_add_time) \

Can't we use timeout_ms directly here when in SANDBOX mode ?

>  ({ \
>  	unsigned long __start = get_timer(0); \
>  	int __ret; \
> @@ -267,6 +273,8 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
>  			break; \
>  		if (cond) \
>  			break; \
> +		if (IS_ENABLED(CONFIG_SANDBOX) && test_add_time) \
> +			sandbox_timer_add_offset(test_add_time); \
>  		if ((timeout_ms) && get_timer(__start) > (timeout_ms)) { \
>  			__ret = regmap_read((map), (addr), &(val)); \
>  			break; \
> @@ -277,6 +285,10 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
>  	__ret ?: ((cond) ? 0 : -ETIMEDOUT); \
>  })
>  
> +#define regmap_read_poll_timeout(map, addr, val, cond, sleep_us, timeout_ms) \
> +	regmap_read_poll_timeout_test(map, addr, val, cond, sleep_us, \
> +				      timeout_ms, 0) \
> +
>  /**
>   * regmap_update_bits() - Perform a read/modify/write using a mask
>   *
> diff --git a/test/dm/regmap.c b/test/dm/regmap.c
> index 9a70c159ddb..82de295cb8f 100644
> --- a/test/dm/regmap.c
> +++ b/test/dm/regmap.c
> @@ -160,9 +160,10 @@ static int dm_test_regmap_poll(struct unit_test_state *uts)
>  	start = get_timer(0);
>  
>  	ut_asserteq(-ETIMEDOUT,
> -		    regmap_read_poll_timeout(map, 0, reg,
> -					     (reg == 0xcacafafa),
> -					     1, 5 * CONFIG_SYS_HZ));
> +		    regmap_read_poll_timeout_test(map, 0, reg,
> +						  (reg == 0xcacafafa),
> +						  1, 5 * CONFIG_SYS_HZ,
> +						  5 * CONFIG_SYS_HZ));
>  
>  	ut_assert(get_timer(start) > (5 * CONFIG_SYS_HZ));
>  
> 

[U-Boot] [PATCH] test: dm: regmap: Fix the long test delay

[Simon Glass]

Hi Neil,

On Tue, 11 Dec 2018 at 01:27, Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> On 10/12/2018 01:11, Simon Glass wrote:
> > At present one of the regmap tests takes 5 seconds to run since it waits
> > for a timeout. This should be handled using sandbox_timer_add_offset()
> > which advances time for test purposes.
> >
> > This requires a little change to make the regmap_read_poll_timeout()
> > testable.
> >
> > Update the macro and the test.
> >
> > Fixes: ebe3497c9c ("test: regmap: add regmap_read_poll_timeout test")
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  include/regmap.h | 14 +++++++++++++-
> >  test/dm/regmap.c |  7 ++++---
> >  2 files changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/regmap.h b/include/regmap.h
> > index a3afb72df51..8359c511d25 100644
> > --- a/include/regmap.h
> > +++ b/include/regmap.h
> > @@ -248,6 +248,8 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
> >   * @cond:    Break condition (usually involving @val)
> >   * @sleep_us:        Maximum time to sleep between reads in us (0 tight-loops).
> >   * @timeout_ms:      Timeout in ms, 0 means never timeout
> > + * @test_add_time: Used for sandbox testing - amount of time to add after
> > + *           starting the loop (0 if not testing)
> >   *
> >   * Returns 0 on success and -ETIMEDOUT upon a timeout or the regmap_read
> >   * error return value in case of a error read. In the two former cases,
> > @@ -256,8 +258,12 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
> >   *
> >   * This is modelled after the regmap_read_poll_timeout macros in linux but
> >   * with millisecond timeout.
> > + *
> > + * The _test version is for sandbox testing only. Do not use this in normal
> > + * code as it advances the timer.
> >   */
> > -#define regmap_read_poll_timeout(map, addr, val, cond, sleep_us, timeout_ms) \
> > +#define regmap_read_poll_timeout_test(map, addr, val, cond, sleep_us, \
> > +                                   timeout_ms, test_add_time) \
>
> Can't we use timeout_ms directly here when in SANDBOX mode ?
>

The problem is we don't know if we are in a test, or not. The argument
tells us that. Some sandbox code might actually use this macro as part
of normal non-test code.

Regards,
Simon

[U-Boot] [PATCH] test: dm: regmap: Fix the long test delay

[sjg at google.com]

Hi Neil,

On Tue, 11 Dec 2018 at 01:27, Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> On 10/12/2018 01:11, Simon Glass wrote:
> > At present one of the regmap tests takes 5 seconds to run since it waits
> > for a timeout. This should be handled using sandbox_timer_add_offset()
> > which advances time for test purposes.
> >
> > This requires a little change to make the regmap_read_poll_timeout()
> > testable.
> >
> > Update the macro and the test.
> >
> > Fixes: ebe3497c9c ("test: regmap: add regmap_read_poll_timeout test")
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  include/regmap.h | 14 +++++++++++++-
> >  test/dm/regmap.c |  7 ++++---
> >  2 files changed, 17 insertions(+), 4 deletions(-)
> >
Applied to u-boot-dm/master, thanks!

[U-Boot] [PATCH] test: dm: regmap: Fix the long test delay

[Faiz Abbas]

Hi Simon,

On 10/12/18 5:41 AM, Simon Glass wrote:
> At present one of the regmap tests takes 5 seconds to run since it waits
> for a timeout. This should be handled using sandbox_timer_add_offset()
> which advances time for test purposes.
> 
> This requires a little change to make the regmap_read_poll_timeout()
> testable.
> 
> Update the macro and the test.
> 
> Fixes: ebe3497c9c ("test: regmap: add regmap_read_poll_timeout test")
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  include/regmap.h | 14 +++++++++++++-
>  test/dm/regmap.c |  7 ++++---
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/include/regmap.h b/include/regmap.h
> index a3afb72df51..8359c511d25 100644
> --- a/include/regmap.h
> +++ b/include/regmap.h
> @@ -248,6 +248,8 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
>   * @cond:	Break condition (usually involving @val)
>   * @sleep_us:	Maximum time to sleep between reads in us (0 tight-loops).
>   * @timeout_ms:	Timeout in ms, 0 means never timeout
> + * @test_add_time: Used for sandbox testing - amount of time to add after
> + *		starting the loop (0 if not testing)
>   *
>   * Returns 0 on success and -ETIMEDOUT upon a timeout or the regmap_read
>   * error return value in case of a error read. In the two former cases,
> @@ -256,8 +258,12 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
>   *
>   * This is modelled after the regmap_read_poll_timeout macros in linux but
>   * with millisecond timeout.
> + *
> + * The _test version is for sandbox testing only. Do not use this in normal
> + * code as it advances the timer.
>   */
> -#define regmap_read_poll_timeout(map, addr, val, cond, sleep_us, timeout_ms) \
> +#define regmap_read_poll_timeout_test(map, addr, val, cond, sleep_us, \
> +				      timeout_ms, test_add_time) \
>  ({ \
>  	unsigned long __start = get_timer(0); \
>  	int __ret; \
> @@ -267,6 +273,8 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
>  			break; \
>  		if (cond) \
>  			break; \
> +		if (IS_ENABLED(CONFIG_SANDBOX) && test_add_time) \
> +			sandbox_timer_add_offset(test_add_time); \

This causes a compilation warning here:
In file included from drivers/mmc/am654_sdhci.c:13:
drivers/mmc/am654_sdhci.c: In function ‘am654_sdhci_set_ios_post’:
include/regmap.h:277:4: warning: implicit declaration of function
‘sandbox_timer_add_offset’ [-Wimplicit-function-declaration]
    sandbox_timer_add_offset(test_add_time); \


Thanks,
Faiz