On Wed, 26 Jul 2023 at 07:02, Heinrich Schuchardt <heinrich.schucha...@canonical.com> wrote: > > On 23.07.23 05:48, Simon Glass wrote: > > Hi Heinrich, > > > > On Fri, 21 Jul 2023 at 00:34, Heinrich Schuchardt > > <heinrich.schucha...@canonical.com> wrote: > >> > >> Move the recursive dp_fill(dev->parent) call to a single location. > >> Determine uclass_id only once. > >> > >> Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com> > >> --- > >> 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.
OK I see Reviewed-by: Simon Glass <s...@chromium.org> Regards, SImon