drivers/media/usb/uvc/uvc_driver.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
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")
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
drivers/media/usb/uvc/uvc_driver.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index a10d4f4d9f95..73a7f23b616c 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2253,9 +2253,10 @@ static int uvc_probe(struct usb_interface *intf,
}
/* 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;
+ goto error_retcode;
}
dev_info(&dev->udev->dev, "Found UVC %u.%02x device %s (%04x:%04x)\n",
@@ -2328,9 +2329,11 @@ static int uvc_probe(struct usb_interface *intf,
return 0;
error:
+ ret = -ENODEV;
+error_retcode:
uvc_unregister_video(dev);
kref_put(&dev->ref, uvc_delete);
- return -ENODEV;
+ return ret;
}
static void uvc_disconnect(struct usb_interface *intf)
---
base-commit: c4b7779abc6633677e6edb79e2809f4f61fde157
change-id: 20250129-uvc-eprobedefer-b5ebb4db63cc
Best regards,
--
Ricardo Ribalda <ribalda@chromium.org>Hi Ricardo,
Thank you for the patch.
On Wed, Jan 29, 2025 at 12:39:46PM +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")
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> drivers/media/usb/uvc/uvc_driver.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index a10d4f4d9f95..73a7f23b616c 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -2253,9 +2253,10 @@ static int uvc_probe(struct usb_interface *intf,
> }
>
> /* 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;
> + goto error_retcode;
> }
>
> dev_info(&dev->udev->dev, "Found UVC %u.%02x device %s (%04x:%04x)\n",
> @@ -2328,9 +2329,11 @@ static int uvc_probe(struct usb_interface *intf,
> return 0;
>
> error:
> + ret = -ENODEV;
> +error_retcode:
This isn't very nice. Could we instead also propagate error codes from
other locations in the uvc_probe() function ? If you want to minimize
changes, you can initialize ret to -ENODEV, and turn the (ret < 0) check
for uvc_gpio_parse() to a (ret) check.
> uvc_unregister_video(dev);
> kref_put(&dev->ref, uvc_delete);
> - return -ENODEV;
> + return ret;
> }
>
> static void uvc_disconnect(struct usb_interface *intf)
>
> ---
> base-commit: c4b7779abc6633677e6edb79e2809f4f61fde157
> change-id: 20250129-uvc-eprobedefer-b5ebb4db63cc
--
Regards,
Laurent Pinchart
On Sun, 23 Feb 2025 at 15:36, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Ricardo,
>
> Thank you for the patch.
>
> On Wed, Jan 29, 2025 at 12:39:46PM +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")
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> > drivers/media/usb/uvc/uvc_driver.c | 9 ++++++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > index a10d4f4d9f95..73a7f23b616c 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -2253,9 +2253,10 @@ static int uvc_probe(struct usb_interface *intf,
> > }
> >
> > /* 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;
> > + goto error_retcode;
> > }
> >
> > dev_info(&dev->udev->dev, "Found UVC %u.%02x device %s (%04x:%04x)\n",
> > @@ -2328,9 +2329,11 @@ static int uvc_probe(struct usb_interface *intf,
> > return 0;
> >
> > error:
> > + ret = -ENODEV;
> > +error_retcode:
>
> This isn't very nice. Could we instead also propagate error codes from
> other locations in the uvc_probe() function ? If you want to minimize
> changes, you can initialize ret to -ENODEV, and turn the (ret < 0) check
> for uvc_gpio_parse() to a (ret) check.
Not very nice, but easy to backport to stables. What about a follow-up
change like this:
index c93abe2367aa..8c67feca1688 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2261,7 +2261,7 @@ static int uvc_probe(struct usb_interface *intf,
ret = uvc_gpio_parse(dev);
if (ret < 0) {
uvc_dbg(dev, PROBE, "Unable to parse UVC GPIOs\n");
- goto error_retcode;
+ goto error;
}
dev_info(&dev->udev->dev, "Found UVC %u.%02x device %s (%04x:%04x)\n",
@@ -2285,24 +2285,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)
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. */
@@ -2334,8 +2342,6 @@ static int uvc_probe(struct usb_interface *intf,
return 0;
error:
- ret = -ENODEV;
-error_retcode:
uvc_unregister_video(dev);
kref_put(&dev->ref, uvc_delete);
return ret;
>
> > uvc_unregister_video(dev);
> > kref_put(&dev->ref, uvc_delete);
> > - return -ENODEV;
> > + return ret;
> > }
> >
> > static void uvc_disconnect(struct usb_interface *intf)
> >
> > ---
> > base-commit: c4b7779abc6633677e6edb79e2809f4f61fde157
> > change-id: 20250129-uvc-eprobedefer-b5ebb4db63cc
>
> --
> Regards,
>
> Laurent Pinchart
Let me know what do you think so I can send a v2 with the change
proposed by Doug.
Regards!
--
Ricardo Ribalda
On Sun, Feb 23, 2025 at 08:32:24PM +0100, Ricardo Ribalda wrote:
> On Sun, 23 Feb 2025 at 15:36, Laurent Pinchart wrote:
> > On Wed, Jan 29, 2025 at 12:39:46PM +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")
> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > ---
> > > drivers/media/usb/uvc/uvc_driver.c | 9 ++++++---
> > > 1 file changed, 6 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > > index a10d4f4d9f95..73a7f23b616c 100644
> > > --- a/drivers/media/usb/uvc/uvc_driver.c
> > > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > > @@ -2253,9 +2253,10 @@ static int uvc_probe(struct usb_interface *intf,
> > > }
> > >
> > > /* 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;
> > > + goto error_retcode;
> > > }
> > >
> > > dev_info(&dev->udev->dev, "Found UVC %u.%02x device %s (%04x:%04x)\n",
> > > @@ -2328,9 +2329,11 @@ static int uvc_probe(struct usb_interface *intf,
> > > return 0;
> > >
> > > error:
> > > + ret = -ENODEV;
> > > +error_retcode:
> >
> > This isn't very nice. Could we instead also propagate error codes from
> > other locations in the uvc_probe() function ? If you want to minimize
> > changes, you can initialize ret to -ENODEV, and turn the (ret < 0) check
> > for uvc_gpio_parse() to a (ret) check.
>
> Not very nice, but easy to backport to stables. What about a follow-up
> change like this:
How about the nicer fix for mainline, and a simpler to backport one for
stable ? The main focus should be on mainline, not backports.
> index c93abe2367aa..8c67feca1688 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -2261,7 +2261,7 @@ static int uvc_probe(struct usb_interface *intf,
> ret = uvc_gpio_parse(dev);
> if (ret < 0) {
> uvc_dbg(dev, PROBE, "Unable to parse UVC GPIOs\n");
> - goto error_retcode;
> + goto error;
> }
>
> dev_info(&dev->udev->dev, "Found UVC %u.%02x device %s (%04x:%04x)\n",
> @@ -2285,24 +2285,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)
> 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. */
> @@ -2334,8 +2342,6 @@ static int uvc_probe(struct usb_interface *intf,
> return 0;
>
> error:
> - ret = -ENODEV;
> -error_retcode:
> uvc_unregister_video(dev);
> kref_put(&dev->ref, uvc_delete);
> return ret;
>
> > > uvc_unregister_video(dev);
> > > kref_put(&dev->ref, uvc_delete);
> > > - return -ENODEV;
> > > + return ret;
> > > }
> > >
> > > static void uvc_disconnect(struct usb_interface *intf)
> > >
> > > ---
> > > base-commit: c4b7779abc6633677e6edb79e2809f4f61fde157
> > > change-id: 20250129-uvc-eprobedefer-b5ebb4db63cc
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
>
> Let me know what do you think so I can send a v2 with the change
> proposed by Doug.
>
> Regards!
--
Regards,
Laurent Pinchart
Hi,
On Wed, Jan 29, 2025 at 4:40 AM Ricardo Ribalda <ribalda@chromium.org> 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")
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> drivers/media/usb/uvc/uvc_driver.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index a10d4f4d9f95..73a7f23b616c 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -2253,9 +2253,10 @@ static int uvc_probe(struct usb_interface *intf,
> }
>
> /* 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;
> + goto error_retcode;
FWIW, since you're specifically considering the -EPROBE_DEFER case,
it's probably worthwhile to make sure that dev_err_probe() is called.
That eventually calls device_set_deferred_probe_reason() which can be
helpful for tracking down problems.
It looks like uvc_gpio_parse() already calls this if gpiod_to_irq()
returns an error code probably you also want to make sure that
dev_err_probe() also gets called in the case where
devm_gpiod_get_optional() returns -EPROBE_DEFER. In that case,
potentially one could also get rid of the uvc_dbg() above.
In any case, IMO even without those changes your patch is still worth
landing. ...and maybe my suggestion should be in a separate follow-on
patch anyway. Thus:
Reviewed-by: Douglas Anderson <dianders@chromium.org>
© 2016 - 2026 Red Hat, Inc.