Re: [PATCH RFC v2 1/5] fastboot: factor out the USB session runner

[David Lechner via U-Boot <u-boot@lists.u-boot-project.org>]

On 7/22/26 3:43 PM, Carlo Caione 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_start() 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.
> 

...

>  static int do_fastboot(struct cmd_tbl *cmdtp, int flag, int argc,
> @@ -167,13 +124,14 @@ NXTARG:
>  		return CMD_RET_USAGE;
>  	}
>  
> -	fastboot_init((void *)buf_addr, buf_size);
> -
>  #if CONFIG_IS_ENABLED(NET_LEGACY)
> -	if (!strcmp(argv[1], "udp"))
> -		return do_fastboot_udp(argc, argv, buf_addr, buf_size);
> -	if (!strcmp(argv[1], "tcp"))
> +	if (!strcmp(argv[1], "udp") || !strcmp(argv[1], "tcp")) {
> +		fastboot_init((void *)buf_addr, buf_size);
> +		if (!strcmp(argv[1], "udp"))
> +			return do_fastboot_udp(argc, argv, buf_addr, buf_size);
> +
>  		return do_fastboot_tcp(argc, argv, buf_addr, buf_size);
> +	}

Would it be simpler just to move the fastboot_init() call inside of
do_fastboot_udp() and do_fastboot_tcp()?

Duplicating one line of code seems simpler that making nested if statements.

>  #endif
>  	if (!strcmp(argv[1], "usb")) {
>  		argv++;
> 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..08aba7b5c12
> --- /dev/null
> +++ b/drivers/fastboot/fb_usb.c
> @@ -0,0 +1,65 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2008 - 2009 Windriver, <www.windriver.com>
> + * Author: Tom Rix <Tom.Rix@windriver.com>
> + *
> + * (C) Copyright 2014 Linaro, Ltd.
> + * Rob Herring <robh@kernel.org>
> + */
> +
> +#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_start(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");
> +		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);
> +	}
> +
> +	ret = 0;

This ret = 0; looks like dead code. It is already 0 at this point.

> +
> +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..b7b661b9591 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_start() - run a USB fastboot session

If this runs a full session and there is no corresponding fastboot_usb_stop()
perhaps we should call this fastboot_usb_run()?

> + *
> + * @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_start(int controller_index, void *buf_addr, u32 buf_size);
> +
>  /**
>   * fastboot_boot() - Execute fastboot boot command
>   *
>