Hi Carlo,

Thank you for the patch.

compared with v3, v5 introduced some new changes which I don't
understand.

Please see below.

On Sun, Sep 06, 2026 at 22:01, Carlo Caione <[email protected]> wrote:

> The fastboot command currently owns USB gadget setup, protocol
> initialization, the service loop and teardown. This prevents callers
> which do not use the command line from starting USB fastboot without
> duplicating the same session lifecycle.
>
> Move that lifecycle into fastboot_usb_run() and leave cmd/fastboot.c
> responsible only for argument parsing and transport selection.
> Initialize network sessions in their transport path so their existing
> behavior is preserved.
>
> Keep the command-line diagnostics for a missing USB cable. The helper
> returns -ENODEV in that case, which the command maps to its existing
> failure result. Unregister the gadget before releasing the UDC so no
> registered function remains attached to a released controller.
>
> Signed-off-by: Julien Masson <[email protected]>
> Signed-off-by: Vitor Sato Eschholz <[email protected]>
> Reviewed-by: Mattijs Korpershoek <[email protected]>
> Signed-off-by: Carlo Caione <[email protected]>
> ---
>  cmd/fastboot.c            | 53 ++++---------------------------------
>  drivers/fastboot/Makefile |  1 +
>  drivers/fastboot/fb_usb.c | 66 
> +++++++++++++++++++++++++++++++++++++++++++++++
>  include/fastboot.h        | 10 +++++++
>  4 files changed, 82 insertions(+), 48 deletions(-)
>

[...]

>               return do_fastboot_udp(argc, argv, buf_addr, buf_size);
> diff --git a/drivers/fastboot/Makefile b/drivers/fastboot/Makefile
> index a341af076d1..32e8e072c88 100644
> --- a/drivers/fastboot/Makefile
> +++ b/drivers/fastboot/Makefile
> @@ -3,6 +3,7 @@
>  obj-y += fb_common.o
>  obj-y += fb_getvar.o
>  obj-y += fb_command.o
> +obj-$(CONFIG_USB_FUNCTION_FASTBOOT) += fb_usb.o
>  obj-$(CONFIG_FASTBOOT_FLASH_BLOCK) += fb_block.o
>  # MMC reuses block implementation
>  obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fb_block.o fb_mmc.o
> diff --git a/drivers/fastboot/fb_usb.c b/drivers/fastboot/fb_usb.c
> new file mode 100644
> index 00000000000..5c62bf42e72
> --- /dev/null
> +++ b/drivers/fastboot/fb_usb.c
> @@ -0,0 +1,66 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later

Why is this GPL-2.0-or-later? In v3 it was GPL-2.0+ which is what's
present in cmd/fastboot.c

> +/*
> + * Copyright 2026 BayLibre SAS

This was also not present in v3 and does not seem an appropriate
addition to me since we are just copying code around. Why should it have
a new copyright?

> + *
> + * Copyright 2008 - 2009 Windriver, <www.windriver.com>
> + * Author: Tom Rix <[email protected]>
> + *
> + * (C) Copyright 2014 Linaro, Ltd.
> + * Rob Herring <[email protected]>
> + */
> +
> +#include <console.h>
> +#include <fastboot.h>
> +#include <g_dnl.h>
> +#include <usb.h>
> +#include <u-boot/schedule.h>
> +#include <linux/errno.h>
> +#include <linux/printk.h>
> +
> +int fastboot_usb_run(int controller_index, void *buf_addr, u32 buf_size)
> +{
> +     struct udevice *udc;
> +     int ret;
> +
> +     ret = udc_device_get_by_index(controller_index, &udc);
> +     if (ret) {
> +             pr_err("USB init failed: %d\n", ret);
> +             return ret;
> +     }
> +
> +     fastboot_init(buf_addr, buf_size);
> +     g_dnl_clear_detach();
> +
> +     ret = g_dnl_register("usb_dnl_fastboot");
> +     if (ret)
> +             goto err_put;
> +
> +     if (!g_dnl_board_usb_cable_connected()) {
> +             puts("\rUSB cable not detected.\n"
> +                  "Command exit.\n");

This is a function that's not necessarily only called from a command.
Please drop the Command exit. part (that was also not present in v3).

> +             ret = -ENODEV;
> +             goto err_unregister;
> +     }
> +
> +     while (!g_dnl_detach()) {
> +             if (IS_ENABLED(CONFIG_CMD_FASTBOOT_ABORT_KEYED)) {
> +                     if (tstc()) {
> +                             getchar();
> +                             puts("\rOperation aborted.\n");
> +                             break;
> +                     }
> +             } else if (ctrlc()) {
> +                     break;
> +             }
> +             schedule();
> +             dm_usb_gadget_handle_interrupts(udc);
> +     }
> +
> +err_unregister:
> +     g_dnl_unregister();
> +     g_dnl_clear_detach();
> +err_put:
> +     udc_device_put(udc);
> +
> +     return ret;
> +}
> diff --git a/include/fastboot.h b/include/fastboot.h
> index b106d617749..f02d2559f2b 100644
> --- a/include/fastboot.h
> +++ b/include/fastboot.h
> @@ -125,6 +125,16 @@ void fastboot_set_progress_callback(void 
> (*progress)(const char *msg));
>   */
>  void fastboot_init(void *buf_addr, u32 buf_size);
>  
> +/**
> + * fastboot_usb_run() - run a USB fastboot session
> + *
> + * @controller_index: USB gadget controller index
> + * @buf_addr: Pointer to download buffer, or NULL for default
> + * @buf_size: Size of download buffer, or zero for default
> + * Return: 0 on success, or a negative error code
> + */
> +int fastboot_usb_run(int controller_index, void *buf_addr, u32 buf_size);
> +
>  /**
>   * fastboot_boot() - Execute fastboot boot command
>   *
>
> -- 
> 2.55.0

Reply via email to