On Wed, 23 Apr 2025 at 02:06, Laurent Pinchart laurent.pinchart@ideasonboard.com wrote:
Hi Ricardo,
Thank you for the patch.
On Thu, Mar 13, 2025 at 12:20:39PM +0000, Ricardo Ribalda wrote:
uvc_gpio_parse() can return -EPROBE_DEFER when the GPIOs it depends on have not yet been probed. This return code should be propagated to the caller of uvc_probe() to ensure that probing is retried when the required GPIOs become available.
Currently, this error code is incorrectly converted to -ENODEV, causing some internal cameras to be ignored.
This commit fixes this issue by propagating the -EPROBE_DEFER error.
Cc: stable@vger.kernel.org Fixes: 2886477ff987 ("media: uvcvideo: Implement UVC_EXT_GPIO_UNIT") Reviewed-by: Douglas Anderson dianders@chromium.org Signed-off-by: Ricardo Ribalda ribalda@chromium.org
drivers/media/usb/uvc/uvc_driver.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index deadbcea5e227c832976fd176c7cdbfd7809c608..e966bdb9239f345fd157588ebdad2b3ebe45168d 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -2231,13 +2231,16 @@ static int uvc_probe(struct usb_interface *intf, #endif
/* Parse the Video Class control descriptor. */
if (uvc_parse_control(dev) < 0) {
ret = uvc_parse_control(dev);if (ret < 0) {ret = -ENODEV;Why do you set ret to -ENODEV here...
uvc_dbg(dev, PROBE, "Unable to parse UVC descriptors\n"); goto error; } /* Parse the associated GPIOs. */
if (uvc_gpio_parse(dev) < 0) {
ret = uvc_gpio_parse(dev);if (ret < 0) { uvc_dbg(dev, PROBE, "Unable to parse UVC GPIOs\n"); goto error; }@@ -2263,24 +2266,32 @@ static int uvc_probe(struct usb_interface *intf, }
/* Register the V4L2 device. */
if (v4l2_device_register(&intf->dev, &dev->vdev) < 0)
ret = v4l2_device_register(&intf->dev, &dev->vdev);if (ret < 0)... but not here ? The code below is also not very consistant.
For all the "external" functions I was looking into populating their error code to probe(). Other drivers (check vivid for example) do exactly this.
There is more value in returning the real cause of the error (ENOMEM, EINVAL) that the plain ENODEV.
goto error; /* Scan the device for video chains. */
if (uvc_scan_device(dev) < 0)
if (uvc_scan_device(dev) < 0) {ret = -ENODEV; goto error;} /* Initialize controls. */
if (uvc_ctrl_init_device(dev) < 0)
if (uvc_ctrl_init_device(dev) < 0) {ret = -ENODEV; goto error;} /* Register video device nodes. */
if (uvc_register_chains(dev) < 0)
if (uvc_register_chains(dev) < 0) {ret = -ENODEV; goto error;}#ifdef CONFIG_MEDIA_CONTROLLER /* Register the media device node */
if (media_device_register(&dev->mdev) < 0)
ret = media_device_register(&dev->mdev);if (ret < 0) goto error;#endif /* Save our data pointer in the interface data. */ @@ -2314,7 +2325,7 @@ static int uvc_probe(struct usb_interface *intf, error: uvc_unregister_video(dev); kref_put(&dev->ref, uvc_delete);
return -ENODEV;
return ret;}
static void uvc_disconnect(struct usb_interface *intf)
-- Regards,
Laurent Pinchart