Hi Balaji,
On 2026-08-11T05:18:35, Balaji Selvanathan
<[email protected]> wrote:
> test: dm: reboot-mode: test trigger dispatch and mode enumeration
>
> Add a trigger-only reboot-mode backend and two device tree modes to the
> sandbox test tree, then cover the new trigger path:
>
> - reboot_mode_request() fires the owning device's trigger op with the
> correct magic cells for both a 2-cell and a 1-cell mode;
> - an unknown mode, and a backing-store-only mode (gpio/rtc), both return
> -ENOENT because they are not triggerable;
> - reboot_mode_list() enumerates only the triggerable modes and hides the
> backing-store modes.
>
> Signed-off-by: Balaji Selvanathan <[email protected]>
>
> arch/sandbox/dts/test.dts | 6 ++++
> test/dm/reboot-mode.c | 87
> +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 93 insertions(+)
> diff --git a/test/dm/reboot-mode.c b/test/dm/reboot-mode.c
> @@ -5,6 +5,7 @@
>
> #include <dm.h>
> #include <reboot-mode/reboot-mode.h>
> +#include <console.h>
> #include <env.h>
> #include <log.h>
Please sort alphabetically - console.h belongs above dm.h (or at least
above env.h), not tucked in after the subdirectory header (which
should be at the end).
> diff --git a/test/dm/reboot-mode.c b/test/dm/reboot-mode.c
> @@ -16,6 +17,42 @@
> +static int test_trigger(struct udevice *dev, const u32 *magic, int count)
> +{
> + int i;
> +
> + test_trigger_count = count;
> + for (i = 0; i < count && i < REBOOT_MODE_MAX_MAGIC; i++)
> + test_trigger_magic[i] = magic[i];
> +
> + /* A real backend does not return here; the test one does. */
> + return -EINPROGRESS;
> +}
Since the framework's contract is that trigger() does not return on
success, returning -EINPROGRESS here is a bit of a smell - a future
change that treats any negative return as failure would break this
test silently. Consider returning 0 and asserting 0, or add a comment
near the ops declaration reminding readers this is a deliberate
test-only convention.
> diff --git a/test/dm/reboot-mode.c b/test/dm/reboot-mode.c
> @@ -66,3 +103,53 @@ static int dm_test_reboot_mode_rtc(struct unit_test_state
> *uts)
> + test_trigger_count = 0;
> + test_trigger_magic[0] = 0;
> + test_trigger_magic[1] = 0;
memset(test_trigger_magic, '\0', sizeof(test_trigger_magic)) is safer
- with REBOOT_MODE_MAX_MAGIC == 3, magic[2] is left uninitialised, and
if a future mode grows to 3 cells this partial clear will hide state
leaked across ut runs.
> diff --git a/test/dm/reboot-mode.c b/test/dm/reboot-mode.c
> @@ -66,3 +103,53 @@ static int dm_test_reboot_mode_rtc(struct unit_test_state
> *uts)
> + ut_assert_nextline("Available reset modes:");
> + ut_assert_nextline(" obelisk");
> + ut_assert_nextline(" sarcophagus");
> + /* Backing-store modes (test/download) have no trigger and are hidden */
> + ut_assert_console_end();
This bakes in the property iteration order of dev_for_each_property()
and the uclass probe order. That happens to be deterministic on
sandbox today, but please add a brief comment explaining why the order
is guaranteed, otherwise a future dts reshuffle will look like a test
regression.
Regards,
Simon
