[PATCH 10/11] fastboot: DM_REBOOT_MODE reset support

[Simon Glass]

Hi Sam,

On 2026-06-06T08:47:32, Sam Day via B4 Relay
<devnull+me.samcday.com at kernel.org> wrote:
> fastboot: DM_REBOOT_MODE reset support
>
> The weak symbol currently implements `fastboot reboot` support using the
> BCB. Nowadays, there's robust reboot-mode driver support which is ideal
> for this use-case.
>
> Signed-off-by: Sam Day <me at samcday.com>
>
> drivers/fastboot/Kconfig          |  7 ++++
>  drivers/fastboot/Makefile         |  1 +
>  drivers/fastboot/fb_reboot_mode.c | 51 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 59 insertions(+)

Two things on the commit message: please use single quotes rather than
backticks for 'fastboot reboot', and spell out that this provides a
non-weak override of fastboot_set_reboot_flag() - i.e. a board which
currently supplies its own override will hit a duplicate-symbol link
error if it also enables FASTBOOT_REBOOT_MODE. Either call that out,
or make the new definition __weak so existing boards keep working.

> diff --git a/drivers/fastboot/fb_reboot_mode.c b/drivers/fastboot/fb_reboot_mode.c
> @@ -0,0 +1,51 @@
> +static int fastboot_set_reboot_mode(const char *mode)
> +{
> +     struct udevice *dev;
> +     int ret = -ENOENT;
> +
> +     uclass_foreach_dev_probe(UCLASS_REBOOT_MODE, dev)
> +     {
> +             ret = dm_reboot_mode_activate(dev, mode);
> +             if (!ret || (ret != -ENOENT && ret != -ENOSYS))
> +                     return ret;
> +     }

The opening brace for the foreach loop should be on the same line as
the macro invocation - kernel style treats this as a for statement.
Please reformat.

> diff --git a/drivers/fastboot/fb_reboot_mode.c b/drivers/fastboot/fb_reboot_mode.c
> @@ -0,0 +1,51 @@
> +int fastboot_set_reboot_flag(enum fastboot_reboot_reason reason)
> +{
> +     static const char *const
> +             reboot_modes[FASTBOOT_REBOOT_REASONS_COUNT][2] = {
> +                     [FASTBOOT_REBOOT_REASON_BOOTLOADER] = { 'bootloader',
> +                                                             NULL },
> +                     [FASTBOOT_REBOOT_REASON_FASTBOOTD] = { 'fastboot',
> +                                                            'bootloader' },
> +                     [FASTBOOT_REBOOT_REASON_RECOVERY] = { 'recovery',
> +                                                           NULL },
> +             };

The hard-coded second dimension of [2] is brittle - if a future reason
needs a third fallback the table silently truncates. A NULL-terminated
list per reason (or a struct with a count) would scale better, lets
you drop the explicit NULL entries plus the continue check, and moving
the table to file scope would avoid the awkward type-on-its-own-line
split. What do you think?

> diff --git a/drivers/fastboot/fb_reboot_mode.c b/drivers/fastboot/fb_reboot_mode.c
> @@ -0,0 +1,51 @@
> +int fastboot_set_reboot_flag(enum fastboot_reboot_reason reason)

A short kerneldoc here (and on fastboot_set_reboot_mode() above) would
help - in particular, documenting the FASTBOOTD fallback to bootloader
is non-obvious behaviour that a reader of include/fastboot.h would not
expect.

Regards,
Simon