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

[Heinrich Schuchardt]

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.

Best regards

Heinrich

> 
>> +               struct efi_device_path_mac_addr *dp = buf;
>>                  struct eth_pdata *pdata = dev_get_plat(dev);
>>
> [..]
> 
> Regards,
> Simon