On 23.07.23 05:48, Simon Glass wrote:
Hi Heinrich,

On Fri, 21 Jul 2023 at 00:34, Heinrich Schuchardt
<[email protected]> wrote:

Move the recursive dp_fill(dev->parent) call to a single location.
Determine uclass_id only once.

Signed-off-by: Heinrich Schuchardt <[email protected]>
---
  lib/efi_loader/efi_device_path.c | 45 +++++++++++++-------------------
  1 file changed, 18 insertions(+), 27 deletions(-)

diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index 5b050fa17c..ed7214f3a3 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -548,14 +548,19 @@ __maybe_unused static unsigned int dp_size(struct udevice 
*dev)
   */
  __maybe_unused static void *dp_fill(void *buf, struct udevice *dev)
  {
+       enum uclass_id uclass_id;
+
         if (!dev || !dev->driver)
                 return buf;

-       switch (device_get_uclass_id(dev)) {
+       uclass_id = device_get_uclass_id(dev);
+       if (uclass_id != UCLASS_ROOT)

Can we fix this one now? We should use EFI_MEDIA here I think?

The function dp_fill() is used to create an EFI device path for any DM device.

Given a device we recursively create a device path with nodes for every device in the dm tree up to the root node. We must stop the recursion at the root node.


+               buf = dp_fill(buf, dev->parent);
+
+       switch (uclass_id) {
  #ifdef CONFIG_NETDEVICES
         case UCLASS_ETH: {
-               struct efi_device_path_mac_addr *dp =
-                       dp_fill(buf, dev->parent);

So how does the parent part get added? I am missing something
here...or it was it never needed??

dp_fill() is a recursive function as explained above.

Best regards

Heinrich


+               struct efi_device_path_mac_addr *dp = buf;
                 struct eth_pdata *pdata = dev_get_plat(dev);

[..]

Regards,
Simon

Reply via email to