On Fri, Mar 28, 2025 at 10:06:12AM +0100, Michael Walle wrote: > 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.
That is also my least favorite checkpatch warning, and it's a warning not an error. Best judgement is needed about making things readable rather than just silencing checkpatch. > > > > > > --- > > > 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. Yeah, I'm not sure that's worth it either, or at least as maybe only a future clean-up or investigate more. Perhaps filing an issue on source.denx.de on the -net tree so it's not forgotten? -- Tom
signature.asc
Description: PGP signature
