Hi Krzysztof,

On Friday, 30 January 2026 14:36:46 CET Krzysztof Karas wrote:
> Hi Janusz,
> 
> >  #define RETRIES_GET_PARENT 5
> Now that you excluded the "parent" word from names in function
> below, so it's only implicitly operating on parent devices, you
> could change this name to RETRIES_GET_DEVICE.

OK.

Thanks,
Janusz

> 
> Rest of the code looks good to me:
> Reviewed-by: Krzysztof Karas <[email protected]>
> 
> > -/* For each drm igt_device add or update its parent igt_device to the 
> > array.
> > - * As card/render drm devices mostly have same parent (vkms is an 
> > exception)
> > - * link to it and update corresponding drm_card / drm_render fields.
> > - */
> > -static void update_or_add_parent(struct udev *udev,
> > -                            struct udev_device *dev,
> > -                            struct igt_device *idev,
> > -                            bool limit_attrs)
> > +
> > +static struct igt_device *find_or_add_igt_device(struct udev *udev,
> > +                                            struct udev_device *dev,
> > +                                            bool limit_attrs)
> >  {
> > -   struct udev_device *parent_dev;
> > -   struct igt_device *parent_idev;
> > -   const char *subsystem, *syspath, *devname;
> >     int retries = RETRIES_GET_PARENT;
> > +   const char *subsystem, *syspath;
> > +   struct igt_device *idev;
> >  
> > -   /*
> > -    * Get parent for drm node. It caches parent in udev device
> > -    * and will be destroyed along with the node.
> > -    */
> > -   parent_dev = udev_device_get_parent(dev);
> > -   igt_assert(parent_dev);
> > -
> > -   subsystem = udev_device_get_subsystem(parent_dev);
> > -   syspath = udev_device_get_syspath(parent_dev);
> > +   subsystem = udev_device_get_subsystem(dev);
> > +   syspath = udev_device_get_syspath(dev);
> >  
> > -   parent_idev = igt_device_find(subsystem, syspath);
> > -   while (!parent_idev && retries--) {
> > +   idev = igt_device_find(subsystem, syspath);
> > +   while (!idev && retries--) {
> >             /*
> >              * Don't care about previous parent_dev, it is tracked
> >              * by the child node. There's very rare race when driver module
> > @@ -951,15 +939,41 @@ static void update_or_add_parent(struct udev *udev,
> >              * only udev_device_new*() will scan sys directory and
> >              * return fresh udev device.
> >              */
> > -           parent_dev = udev_device_new_from_syspath(udev, syspath);
> > -           parent_idev = igt_device_new_from_udev(parent_dev, limit_attrs);
> > -           udev_device_unref(parent_dev);
> > +           dev = udev_device_new_from_syspath(udev, syspath);
> > +           idev = igt_device_new_from_udev(dev, limit_attrs);
> > +           udev_device_unref(dev);
> >  
> > -           if (parent_idev)
> > -                   igt_list_add_tail(&parent_idev->link, &igt_devs.all);
> > +           if (idev)
> > +                   igt_list_add_tail(&idev->link, &igt_devs.all);
> >             else
> >                     usleep(100000); /* arbitrary, 100ms should be enough */
> >     }
> > +
> > +   return idev;
> > +}
> > +
> > +/*
> > + * For each drm igt_device add or update its parent igt_device to the 
> > array.
> > + * As card/render drm devices mostly have same parent (vkms is an 
> > exception)
> > + * link to it and update corresponding drm_card / drm_render fields.
> > + */
> > +static void update_or_add_parent(struct udev *udev,
> > +                            struct udev_device *dev,
> > +                            struct igt_device *idev,
> > +                            bool limit_attrs)
> > +{
> > +   struct udev_device *parent_dev;
> > +   struct igt_device *parent_idev;
> > +   const char *devname;
> > +
> > +   /*
> > +    * Get parent for drm node. It caches parent in udev device
> > +    * and will be destroyed along with the node.
> > +    */
> > +   parent_dev = udev_device_get_parent(dev);
> > +   igt_assert(parent_dev);
> > +
> > +   parent_idev = find_or_add_igt_device(udev, parent_dev, limit_attrs);
> >     igt_assert(parent_idev);
> >  
> >     devname = udev_device_get_devnode(dev);
> 
> 




Reply via email to