Re: [PATCH RESEND v2 5/5] test: wdt: add a test for max_timeout_ms

Hi Juuso,

On 2026-08-12T09:48:39, Juuso Rinta <[email protected]> wrote:
> test: wdt: add a test for max_timeout_ms
>
> Add a DM test for the case where wdt_start() is called with
> a timeout_ms value greater than the (emulated) hardware-supported
> limit.
>
> Add console-print asserts for both the clamped and unclamped cases,
> as well as for the per-device uclass-plat data max_timeout_ms field.
>
> Add matching tests for the GPIO driver, which does not set the
> max_timeout_ms value.
>
> Signed-off-by: Juuso Rinta <[email protected]>
>
> test/dm/wdt.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 47 insertions(+), 2 deletions(-)

> diff --git a/test/dm/wdt.c b/test/dm/wdt.c
> @@ -16,34 +17,68 @@
> +static void format_wdt_servicing_str(char *str, size_t len, struct udevice *dev)
> +{
> +     if (IS_ENABLED(CONFIG_WATCHDOG)) {
> +             u32 reset_period = dev_read_u32_default(dev, "hw_margin_ms",
> +                                                     4000) / 4;
> +
> +             snprintf(str, len, " with servicing every %ums ",
> +                      reset_period);
> +     } else {
> +             snprintf(str, len, " without servicing ");
> +     }
> +}

This helper reimplements internal wdt-uclass details (the format
string, the "every %ldms" text, and the hw_margin_ms / 4 calculation
from wdt_pre_probe()). If any of that drifts, the tests silently rot
rather than catching a real change. Please drop the string
reconstruction and either match against a fixed literal (choose
test-side values that make the expected text deterministic) or split
the assertion into a handful of ut_assert_nextlinen() / substring
checks. Reproducing driver-private logic in the test isn't the right
layer.

Also, wdt-uclass prints "every %ldms" (long), not %ums, and when
CONFIG_WATCHDOG is not enabled the driver's format string produces two
spaces before the '(' (because svc_str is empty inside " %s ("), while
this helper emits only one trailing space. Sandbox does not select
CONFIG_WATCHDOG, so I suspect the ut_assert_nextline() calls below
will not match today.

> diff --git a/test/dm/wdt.c b/test/dm/wdt.c
> @@ -16,34 +17,68 @@
> +     /* Clamped timeout path */
> +     console_record_reset();
> +     ut_assertok(wdt_start(dev, SANDBOX_WDT_MAX_TIMEOUT_MS + 1000, 0));
> +     ut_asserteq(SANDBOX_WDT_MAX_TIMEOUT_MS, state->wdt.counter);
> +     ut_assert_nextline("WDT:   Started %s%s(%ds timeout, requested %ds)",
> +                        dev->name, svc, SANDBOX_WDT_MAX_TIMEOUT_MS / 1000,
> +                        (SANDBOX_WDT_MAX_TIMEOUT_MS + 1000) / 1000);
> +     ut_assert_console_end();

The clamped case only exercises timeout_ms > max_timeout_ms. The
uclass code has a second branch, the sub-second difference guard, that
suppresses req_str when the whole-second value would be unchanged. How
about a case where timeout_ms is greater than max but rounds to the
same whole second (e.g. max + 100ms), to lock down that behaviour?

Regards,
Simon