Hi Heinrich, On Wed, 27 Nov 2024 at 07:00, Heinrich Schuchardt <[email protected]> wrote: > > On 27.11.24 14:07, Simon Glass wrote: > > Hi Heinrich, > > > > On Wed, 27 Nov 2024 at 00:07, Heinrich Schuchardt > > <[email protected]> wrote: > >> > >> Currently when booting dhcp_run() may be executed multiple times: > >> once in eth_bootdev_hunt() and once in the network booting bootmeth. > >> > >> We need to call eth_bootdev_hunt() when setting up the EFI sub-system to > >> supply the simple network protocol. We don't need an IP address set up. > >> > >> We can reduce the bootime by not executing dhcp_run() in > >> eth_bootdev_hunt(). > >> > >> Furthermore eth_bootdev_hunt() with autostart=yes leads on the legacy > >> network statck leads to downloading a file via TFTP and to booting the > > > > spelling > > > >> downloaded file. > > > > I have now found the feature you're referring to - the calls to > > env_get_autostart(). Perhaps we can remove this feature? It seems > > pretty old and I don't see boards using it. > > > > That feature stops bootstd working properly. > > > > But if we don't remove it, we need to disable autostart in dhcp_run(), > > since in that case it does not behave correctly. I can do a patch for > > that if needed. > > > >> > >> Instead of running dchp_run() just check that there is a network device > >> in eth_bootdev_hunt(). > >> > >> Reviewed-by: Ilias Apalodimas <[email protected]> > >> Signed-off-by: Heinrich Schuchardt <[email protected]> > >> --- > >> v2: > >> reword the commit message > >> --- > >> net/eth_bootdev.c | 30 ++++++++++++++++++------------ > >> 1 file changed, 18 insertions(+), 12 deletions(-) > >> > >> diff --git a/net/eth_bootdev.c b/net/eth_bootdev.c > >> index 6ee54e3c790..b0fca6e8313 100644 > >> --- a/net/eth_bootdev.c > >> +++ b/net/eth_bootdev.c > >> @@ -64,9 +64,23 @@ static int eth_bootdev_bind(struct udevice *dev) > >> return 0; > >> } > >> > >> +/** > >> + * eth_bootdev_hunt() - probe all network devices > >> + * > >> + * Network devices can also come from USB, but that is a higher > >> + * priority (BOOTDEVP_5_SCAN_SLOW) than network, so it should have been > >> + * enumerated already. If something like 'bootflow scan dhcp' is used, > >> + * then the user will need to run 'usb start' first. > >> + * > >> + * @info: info structure describing this hunter > >> + * @show: true to show information from the hunter > >> + * > >> + * Return: 0 if device found, -EINVAL otherwise > >> + */ > >> static int eth_bootdev_hunt(struct bootdev_hunter *info, bool show) > >> { > >> int ret; > >> + struct udevice *dev = NULL; > >> > >> if (!test_eth_enabled()) > >> return 0; > >> @@ -78,19 +92,11 @@ static int eth_bootdev_hunt(struct bootdev_hunter > >> *info, bool show) > >> log_warning("Failed to init PCI (%dE)\n", ret); > >> } > >> > >> - /* > >> - * Ethernet devices can also come from USB, but that is a higher > >> - * priority (BOOTDEVP_5_SCAN_SLOW) than ethernet, so it should > >> have been > >> - * enumerated already. If something like 'bootflow scan dhcp' is > >> used > >> - * then the user will need to run 'usb start' first. > >> - */ > > > > Please keep this comment > > I have moved the comment to the function description. See above. I think > that is the adequate place. > > > > >> - if (IS_ENABLED(CONFIG_CMD_DHCP)) { > >> - ret = dhcp_run(0, NULL, false); > >> - if (ret) > >> - return -EINVAL; > >> - } > >> + ret = -EINVAL; > >> + uclass_foreach_dev_probe(UCLASS_ETH, dev) > >> + ret = 0; > > > > There is a uclass_probe_all() function. > > > > My suggestion from the original series was to just do the dhcp once. > > > > How does probing the ethernet devices actually help EFI? It is not > > needed for bootstd, since it probes any devices it uses. > > bootstd is not the only way to start an EFI application, you could use > bootefi or bootm.
That's fine, but I'm just trying to understand what probing does, in this particular case. Doesn't EFI probe a device before it uses it? I think I am just missing some context. Regards, Simon
