On Wed, Jun 07, 2023 at 01:00:29PM +0200, Ahmad Fatoum wrote:
> Hello Roland,
> 
> On 07.06.23 10:33, Roland Hieber wrote:
> > On Mon, Jun 05, 2023 at 04:32:36PM +0200, Ahmad Fatoum wrote:
> >> Despite the generic name, of_find_device_by_uuid only handled
> >> ID_PART_TABLE_UUID (diskuuid) and ID_PART_ENTRY_UUID (partuuid),
> >> but not ID_PART_ENTRY_TYPE (GPT Partition Type UUID).
> >>
> >> In preparation for doing Type UUID lookups, adjust of_find_device_by_uuid
> >> to support all three types of GPT UUIDs.
> >>
> >> Signed-off-by: Ahmad Fatoum <a.fat...@pengutronix.de>
> >> ---
> >> This patch is the missing prerequisite for my GPT Type UUID series.
> >> ---
> >>  src/libdt.c | 30 +++++++++++++++++++-----------
> >>  1 file changed, 19 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/src/libdt.c b/src/libdt.c
> >> index e54d7fb5649d..2c994c647ac9 100644
> >> --- a/src/libdt.c
> >> +++ b/src/libdt.c
> >> @@ -2405,7 +2405,9 @@ out:
> >>    return dev;
> >>  }
> >>  
> >> -static struct udev_device *of_find_device_by_uuid(const char *uuid)
> >> +static struct udev_device *of_find_device_by_uuid(struct udev_device 
> >> *parent,
> >> +                                            const char *uuid,
> >> +                                            bool type_uuid)
> >>  {
> >>    struct udev *udev;
> >>    struct udev_enumerate *enumerate;
> >> @@ -2418,12 +2420,15 @@ static struct udev_device 
> >> *of_find_device_by_uuid(const char *uuid)
> >>    }
> >>  
> >>    enumerate = udev_enumerate_new(udev);
> >> +  if (parent)
> >> +          udev_enumerate_add_match_parent(enumerate, parent);
> >>    udev_enumerate_add_match_subsystem(enumerate, "block");
> >>    udev_enumerate_scan_devices(enumerate);
> >>    devices = udev_enumerate_get_list_entry(enumerate);
> >>    udev_list_entry_foreach(dev_list_entry, devices) {
> >>            const char *path, *devtype, *dev_uuid;
> >>            struct udev_device *device;
> >> +          const char *property;
> >>  
> >>            path = udev_list_entry_get_name(dev_list_entry);
> >>            device = udev_device_new_from_syspath(udev, path);
> >> @@ -2432,16 +2437,19 @@ static struct udev_device 
> >> *of_find_device_by_uuid(const char *uuid)
> >>            devtype = udev_device_get_devtype(device);
> >>            if (!devtype)
> >>                    continue;
> >> -          if (!strcmp(devtype, "disk")) {
> >> -                  dev_uuid = udev_device_get_property_value(device, 
> >> "ID_PART_TABLE_UUID");
> >> -                  if (dev_uuid && !strcasecmp(dev_uuid, uuid))
> >> -                          return device;
> >> -          } else if (!strcmp(devtype, "partition")) {
> >> -                  dev_uuid = udev_device_get_property_value(device, 
> >> "ID_PART_ENTRY_UUID");
> >> -                  if (dev_uuid && !strcasecmp(dev_uuid, uuid))
> >> -                          return device;
> >> -          }
> >>  
> >> +          if (type_uuid)
> >> +                  property = "ID_PART_ENTRY_TYPE";
> >> +          else if (!strcmp(devtype, "disk"))
> >> +                  property = "ID_PART_TABLE_UUID";
> >> +          else if (!strcmp(devtype, "partition"))
> >> +                  property = "ID_PART_ENTRY_UUID";
> >> +          else
> >> +                  continue;
> >> +
> >> +          dev_uuid = udev_device_get_property_value(device, property);
> >> +          if (dev_uuid && !strcasecmp(dev_uuid, uuid))
> >> +                  return device;
> > 
> > I wonder if we can't simply add a udev_enumerate_add_match_property
> > above to do the filtering for us instead of iterating over the match
> > entries ourselves.
> 
> I didn't know about udev_enumerate_add_match_property. Are multiple filters
> OR'd or AND'd? For the !type_uuid case we need to check one or the other
> property.

Sorry, I don't know either… it looks to me like they are ANDed, but the
documentation is a bit sparse on the udev side…

 - Roland

> 
> > But the patch is correct, so:
> > Reviewed-by: Roland Hieber <r...@pengutronix.de>
> 
> Thanks. I have applied this patch and will keep 
> udev_enumerate_add_match_property
> in mind for future rework.
> 
> Cheers,
> Ahmad
> 
> > 
> >>    }
> >>    return NULL;
> >>  }
> >> @@ -2540,7 +2548,7 @@ int of_get_devicepath(struct device_node 
> >> *partition_node, char **devpath, off_t
> >>                                    node->full_name);
> >>                    return -ENODEV;
> >>            }
> >> -          dev = of_find_device_by_uuid(uuid);
> >> +          dev = of_find_device_by_uuid(NULL, uuid, false);
> >>            if (!dev) {
> >>                    fprintf(stderr, "%s: cannot find device for uuid %s\n", 
> >> __func__,
> >>                            uuid);
> >> -- 
> >> 2.39.2
> >>
> >>
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> 

-- 
Roland Hieber, Pengutronix e.K.          | r.hie...@pengutronix.de     |
Steuerwalder Str. 21                     | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686         | Fax:   +49-5121-206917-5555 |

Reply via email to