[PATCH 1/1] efi_loader: simplify dp_fill()

[Simon Glass]

On Wed, 26 Jul 2023 at 07:02, Heinrich Schuchardt
<heinrich.schuchardt at 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.schuchardt at 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.schuchardt at 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 <sjg at chromium.org>

Regards,
SImon