Hi Michael,
Thank you for the patch.
On mer., mars 12, 2025 at 08:36, Michael Walle <mwa...@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 <mwa...@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
