[PATCH] cmd: bdinfo: fix incorrect Kconfig options check for print_eth()

[Tom Rini]

On Wed, Dec 17, 2025 at 04:35:47PM +0100, Quentin Schulz wrote:
> Hi Tom,
> 
> On 12/17/25 4:04 PM, Tom Rini wrote:
> > On Wed, Dec 17, 2025 at 03:51:03PM +0100, Quentin Schulz wrote:
> > > Hi Tom,
> > > 
> > > On 12/17/25 3:17 PM, Tom Rini wrote:
> > > > On Wed, Dec 17, 2025 at 03:01:10PM +0100, Quentin Schulz wrote:
> > > > 
> > > > > From: Quentin Schulz <quentin.schulz at cherry.de>
> > > > > 
> > > > > CMD_NET_LWIP has never existed so it cannot be right. I'm guessing the
> > > > > intent was to allow print_eth() to be called when NET_LWIP is defined
> > > > > (NET means "legacy networking stack" as opposed to NET_LWIP which is the
> > > > >    newest (and incompatible) stack). There probably was some mix-up
> > > > > between CMD_NET and NET options.
> > > > > 
> > > > > The dependency on CMD_NET seems unnecessary as it seems perfectly fine
> > > > > to run bdinfo without CMD_NET (build and run tested). So let's instead
> > > > > make the dependency on NET || NET_LWIP.
> > > > > 
> > > > > Fixes: 95744d2527cb ("cmd: bdinfo: enable -e when CONFIG_CMD_NET_LWIP=y")
> > > > > Signed-off-by: Quentin Schulz <quentin.schulz at cherry.de>
> > > > > ---
> > > > >    cmd/bdinfo.c | 6 +++---
> > > > >    1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c
> > > > > index 20c8c97f0cd..09fe8067642 100644
> > > > > --- a/cmd/bdinfo.c
> > > > > +++ b/cmd/bdinfo.c
> > > > > @@ -152,7 +152,7 @@ static int bdinfo_print_all(struct bd_info *bd)
> > > > >    	bdinfo_print_num_l("relocaddr", gd->relocaddr);
> > > > >    	bdinfo_print_num_l("reloc off", gd->reloc_off);
> > > > >    	printf("%-12s= %u-bit\n", "Build", (uint)sizeof(void *) * 8);
> > > > > -	if (IS_ENABLED(CONFIG_CMD_NET) || IS_ENABLED(CONFIG_CMD_NET_LWIP))
> > > > > +	if (IS_ENABLED(CONFIG_NET) || IS_ENABLED(CONFIG_NET_LWIP))
> > > > >    		print_eth();
> > > > >    	bdinfo_print_num_l("fdt_blob", (ulong)map_to_sysmem(gd->fdt_blob));
> > > > >    	if (IS_ENABLED(CONFIG_VIDEO))
> > > > > @@ -193,8 +193,8 @@ int do_bdinfo(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> > > > >    		case 'a':
> > > > >    			return bdinfo_print_all(bd);
> > > > >    		case 'e':
> > > > > -			if (!IS_ENABLED(CONFIG_CMD_NET) &&
> > > > > -			    !IS_ENABLED(CONFIG_CMD_NET_LWIP))
> > > > > +			if (!IS_ENABLED(CONFIG_NET) &&
> > > > > +			    !IS_ENABLED(CONFIG_NET_LWIP))
> > > > >    				return CMD_RET_USAGE;
> > > > >    			print_eth();
> > > > >    			return CMD_RET_SUCCESS;
> > > > 
> > > > It must also be the case that we never both build cmd/bdinfo.c and also
> > > > have no network stack, otherwise we'd get a warning around print_eth
> > > > being defined but unused. Can you guard that function too? It needs
> > > 
> > > This doesn't happen though for some reason.
> > > 
> > > make ringneck-px30_defconfig
> > > make menuconfig # CMD_NET=n, NO_NET=y
> > > make
> > > 
> > > No warning or error (CMD_BDI=y and I've checked by adding #error whatever in
> > > the file). eth-uclass.c which defines eth_get_dev_index() is not compiled in
> > > (no eth-uclass.o in build/drivers/).
> > 
> > Hunh, ok...
> > 
> 
> Compiler magic? /me shrugs
> 
> > > > eth_get_dev_index to exist which does require a network stack. Also,
> > > > maybe we should be checking on the "no networking enabled" symbol? Or
> > > 
> > > Yes we probably should but we do NET || NET_LWIP everywhere, so I believe
> > > it's better to stay consistent for now.
> > 
> > Makes sense.
> > 
> 
> 
> Mmmmm... So CMD_NET is only selectable when NET || NET_LWIP today[1], so
> keeping only CMD_NET check is enough (removing CMD_NET_LWIP which anyway
> doesn't exist). But at the same time, there's nothing CMD_NET does that is
> required for this part of bdinfo to work, so I think going for NET ||
> NET_LWIP still is better? What do you think?

I think it's a good candidate for making clearer, as part of how you're
reworking the symbols. There's lots of ways to configure U-Boot today so
that it doesn't build, so one more small spot here for a bit longer
isn't terrible.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20251217/35566c8d/attachment.sig>