Hi Mattijs, > > 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?
I've seen these. More below.
>
> > ---
> > Alternatively, we could add the defines and stub functions to the lwip
> > header.
This is relevant :)
> > ---
> > 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.
No, because net_loop(), net_set_state() and NETLOOP_SUCCESS is only
defined in net-legacy.h. Thus we need this ifdeffery.. Unless of
course, we add the enums and function stubs to the new lwip net
inlcude. I don't know if that's a good idea though.
-michael
>
> > 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
signature.asc
Description: PGP signature
