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 <[email protected]> > > > > > > > > > > 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 <[email protected]> > > > > > --- > > > > > 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
signature.asc
Description: PGP signature
