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.
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); > -- > 2.52.0 > -- Best Regards, Krzysztof
