Re: [PATCH 1/3] fastboot: lift restrictions on !NET_LWIP for USB

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

Hi Michael,

Thank you for the patch.

On mer., mars 12, 2025 at 08:36, Michael Walle <mwalle@kernel.org> wrote:

> Fastboot works either over TCP, UDP or USB. The latter doesn't have
> anything to do with networking, thus should work just fine with
> regardless which network stack is selected. In practice, header symbols
> are used inside common code paths. Add some ifdeffery to guard against
> that.
>
> This will make fastboot over USB work with the new LWIP stack.
>
> Signed-off-by: Michael Walle <mwalle@kernel.org>

checkpatch.pl reports some issues with this:

$ ./scripts/checkpatch.pl --strict --u-boot --git HEAD^..HEAD

<snip>
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
<snip>

Some occurences below could be fixed. Could you please have a look?

> ---
> Alternatively, we could add the defines and stub functions to the lwip
> header.
> ---
>  cmd/fastboot.c               | 4 ++++
>  drivers/fastboot/Kconfig     | 1 -
>  drivers/fastboot/fb_common.c | 4 ++++
>  3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/fastboot.c b/cmd/fastboot.c
> index d4cfc0c7a28..be84a482b81 100644
> --- a/cmd/fastboot.c
> +++ b/cmd/fastboot.c
> @@ -16,6 +16,7 @@
>  #include <linux/printk.h>
>  #include <linux/stringify.h>
>  
> +#if CONFIG_IS_ENABLED(NET)

I think this can be dropped. I hope that since it's a static function,
-if there are no users in the file- the compiler will optimize it out.
Note: I have not verified this, so I might be wrong.

If you measure and see size changes between keeping the #if and not
keeping it, please ignore this comment.

>  static int do_fastboot_udp(int argc, char *const argv[],
>  			   uintptr_t buf_addr, size_t buf_size)
>  {
> @@ -55,6 +56,7 @@ static int do_fastboot_tcp(int argc, char *const argv[],
>  
>  	return CMD_RET_SUCCESS;
>  }
> +#endif
>  
>  static int do_fastboot_usb(int argc, char *const argv[],
>  			   uintptr_t buf_addr, size_t buf_size)
> @@ -160,10 +162,12 @@ NXTARG:
>  
>  	fastboot_init((void *)buf_addr, buf_size);
>  
> +#if CONFIG_IS_ENABLED(NET)
>  	if (!strcmp(argv[1], "udp"))
>  		return do_fastboot_udp(argc, argv, buf_addr, buf_size);
>  	if (!strcmp(argv[1], "tcp"))
>  		return do_fastboot_tcp(argc, argv, buf_addr, buf_size);
> +#endif

This can be replaced with:

	if (IS_ENABLED(CONFIG_NET)) {
		if (!strcmp(argv[1], "udp"))
			return do_fastboot_udp(argc, argv, buf_addr, buf_size);
		if (!strcmp(argv[1], "tcp"))
			return do_fastboot_tcp(argc, argv, buf_addr, buf_size);
	}

>  	if (!strcmp(argv[1], "usb")) {
>  		argv++;
>  		argc--;
> diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
> index 1eb460f5a02..70207573de2 100644
> --- a/drivers/fastboot/Kconfig
> +++ b/drivers/fastboot/Kconfig
> @@ -1,6 +1,5 @@
>  menu "Fastboot support"
>  	depends on CMDLINE
> -	depends on !NET_LWIP
>  
>  config FASTBOOT
>  	bool
> diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c
> index 12ffb463deb..68f92c4b887 100644
> --- a/drivers/fastboot/fb_common.c
> +++ b/drivers/fastboot/fb_common.c
> @@ -183,11 +183,15 @@ void fastboot_handle_boot(int command, bool success)
>  	switch (command) {
>  	case FASTBOOT_COMMAND_BOOT:
>  		fastboot_boot();
> +#if CONFIG_IS_ENABLED(NET)
>  		net_set_state(NETLOOP_SUCCESS);
> +#endif

This can be replaced with:

		if (IS_ENABLED((CONFIG_NET))
		    net_set_state(NETLOOP_SUCCESS);

>  		break;
>  
>  	case FASTBOOT_COMMAND_CONTINUE:
> +#if CONFIG_IS_ENABLED(NET)
>  		net_set_state(NETLOOP_SUCCESS);
> +#endif

Same here.

>  		break;
>  
>  	case FASTBOOT_COMMAND_REBOOT:
> -- 
> 2.39.5