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.

Best regards

Heinrich



-       return 0;
+       return ret;
  }

  struct bootdev_ops eth_bootdev_ops = {
--
2.45.2


Regards,
Simon

Reply via email to