| Message ID | 20250509-uvc-followup-v1-4-73bcde30d2b5@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | media: uvcvideo: Follow-up patches for next-media-uvc-20250509 | expand |
Hi Ricardo, On Fri, May 09, 2025 at 06:24:16PM +0000, Ricardo Ribalda wrote: > Now we are replacing most of the error codes with -ENODEV. > Instead, Populate the error code from the functions called by > uvc_probe(). > > Take this opportunity to replace a generic error code from > uvc_scan_device() into something more meaningful. > > Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > --- > drivers/media/usb/uvc/uvc_driver.c | 15 ++++----------- > 1 file changed, 4 insertions(+), 11 deletions(-) > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index da24a655ab68cc0957762f2b67387677c22224d1..cdf4bbe582272277a6a95267e9752010adc51b6b 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -1866,7 +1866,7 @@ static int uvc_scan_device(struct uvc_device *dev) > > if (list_empty(&dev->chains)) { > dev_info(&dev->udev->dev, "No valid video chain found.\n"); > - return -1; > + return -ENODEV; > } > > /* Add GPIO entity to the first chain. */ > @@ -2239,7 +2239,6 @@ static int uvc_probe(struct usb_interface *intf, > /* Parse the Video Class control descriptor. */ > ret = uvc_parse_control(dev); > if (ret < 0) { > - ret = -ENODEV; > uvc_dbg(dev, PROBE, "Unable to parse UVC descriptors\n"); > goto error; > } > @@ -2275,22 +2274,16 @@ static int uvc_probe(struct usb_interface *intf, > goto error; > > /* Scan the device for video chains. */ > - if (uvc_scan_device(dev) < 0) { > - ret = -ENODEV; > + if (uvc_scan_device(dev) < 0) That's not going to work. You probably meant ret = uvc_scan_device(dev); if (ret < 0) Same elsewhere where applicable. > goto error; > - } > > /* Initialize controls. */ > - if (uvc_ctrl_init_device(dev) < 0) { > - ret = -ENODEV; > + if (uvc_ctrl_init_device(dev) < 0) > goto error; > - } > > /* Register video device nodes. */ > - if (uvc_register_chains(dev) < 0) { > - ret = -ENODEV; > + if (uvc_register_chains(dev) < 0) > goto error; > - } > > #ifdef CONFIG_MEDIA_CONTROLLER > /* Register the media device node */
Hi Laurent On Fri, 23 May 2025 at 13:58, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Ricardo, > > On Fri, May 09, 2025 at 06:24:16PM +0000, Ricardo Ribalda wrote: > > Now we are replacing most of the error codes with -ENODEV. > > Instead, Populate the error code from the functions called by > > uvc_probe(). > > > > Take this opportunity to replace a generic error code from > > uvc_scan_device() into something more meaningful. > > > > Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > --- > > drivers/media/usb/uvc/uvc_driver.c | 15 ++++----------- > > 1 file changed, 4 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > > index da24a655ab68cc0957762f2b67387677c22224d1..cdf4bbe582272277a6a95267e9752010adc51b6b 100644 > > --- a/drivers/media/usb/uvc/uvc_driver.c > > +++ b/drivers/media/usb/uvc/uvc_driver.c > > @@ -1866,7 +1866,7 @@ static int uvc_scan_device(struct uvc_device *dev) > > > > if (list_empty(&dev->chains)) { > > dev_info(&dev->udev->dev, "No valid video chain found.\n"); > > - return -1; > > + return -ENODEV; > > } > > > > /* Add GPIO entity to the first chain. */ > > @@ -2239,7 +2239,6 @@ static int uvc_probe(struct usb_interface *intf, > > /* Parse the Video Class control descriptor. */ > > ret = uvc_parse_control(dev); > > if (ret < 0) { > > - ret = -ENODEV; > > uvc_dbg(dev, PROBE, "Unable to parse UVC descriptors\n"); > > goto error; > > } > > @@ -2275,22 +2274,16 @@ static int uvc_probe(struct usb_interface *intf, > > goto error; > > > > /* Scan the device for video chains. */ > > - if (uvc_scan_device(dev) < 0) { > > - ret = -ENODEV; > > + if (uvc_scan_device(dev) < 0) > > That's not going to work. You probably meant > > ret = uvc_scan_device(dev); > if (ret < 0) Ups, seems like I sent a partial path :S Sorry about that. Shall I resend 4/4 or you want to take it as well? > > Same elsewhere where applicable. > > > goto error; > > - } > > > > /* Initialize controls. */ > > - if (uvc_ctrl_init_device(dev) < 0) { > > - ret = -ENODEV; > > + if (uvc_ctrl_init_device(dev) < 0) > > goto error; > > - } > > > > /* Register video device nodes. */ > > - if (uvc_register_chains(dev) < 0) { > > - ret = -ENODEV; > > + if (uvc_register_chains(dev) < 0) > > goto error; > > - } > > > > #ifdef CONFIG_MEDIA_CONTROLLER > > /* Register the media device node */ > > -- > Regards, > > Laurent Pinchart
On Fri, May 23, 2025 at 02:24:00PM +0200, Ricardo Ribalda wrote: > On Fri, 23 May 2025 at 13:58, Laurent Pinchart wrote: > > On Fri, May 09, 2025 at 06:24:16PM +0000, Ricardo Ribalda wrote: > > > Now we are replacing most of the error codes with -ENODEV. > > > Instead, Populate the error code from the functions called by > > > uvc_probe(). > > > > > > Take this opportunity to replace a generic error code from > > > uvc_scan_device() into something more meaningful. > > > > > > Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > > --- > > > drivers/media/usb/uvc/uvc_driver.c | 15 ++++----------- > > > 1 file changed, 4 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > > > index da24a655ab68cc0957762f2b67387677c22224d1..cdf4bbe582272277a6a95267e9752010adc51b6b 100644 > > > --- a/drivers/media/usb/uvc/uvc_driver.c > > > +++ b/drivers/media/usb/uvc/uvc_driver.c > > > @@ -1866,7 +1866,7 @@ static int uvc_scan_device(struct uvc_device *dev) > > > > > > if (list_empty(&dev->chains)) { > > > dev_info(&dev->udev->dev, "No valid video chain found.\n"); > > > - return -1; > > > + return -ENODEV; > > > } > > > > > > /* Add GPIO entity to the first chain. */ > > > @@ -2239,7 +2239,6 @@ static int uvc_probe(struct usb_interface *intf, > > > /* Parse the Video Class control descriptor. */ > > > ret = uvc_parse_control(dev); > > > if (ret < 0) { > > > - ret = -ENODEV; > > > uvc_dbg(dev, PROBE, "Unable to parse UVC descriptors\n"); > > > goto error; > > > } > > > @@ -2275,22 +2274,16 @@ static int uvc_probe(struct usb_interface *intf, > > > goto error; > > > > > > /* Scan the device for video chains. */ > > > - if (uvc_scan_device(dev) < 0) { > > > - ret = -ENODEV; > > > + if (uvc_scan_device(dev) < 0) > > > > That's not going to work. You probably meant > > > > ret = uvc_scan_device(dev); > > if (ret < 0) > > Ups, seems like I sent a partial path :S > > Sorry about that. Shall I resend 4/4 or you want to take it as well? Could you resend just 4/4 ? I'll handle the rest. > > Same elsewhere where applicable. > > > > > goto error; > > > - } > > > > > > /* Initialize controls. */ > > > - if (uvc_ctrl_init_device(dev) < 0) { > > > - ret = -ENODEV; > > > + if (uvc_ctrl_init_device(dev) < 0) > > > goto error; > > > - } > > > > > > /* Register video device nodes. */ > > > - if (uvc_register_chains(dev) < 0) { > > > - ret = -ENODEV; > > > + if (uvc_register_chains(dev) < 0) > > > goto error; > > > - } > > > > > > #ifdef CONFIG_MEDIA_CONTROLLER > > > /* Register the media device node */
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index da24a655ab68cc0957762f2b67387677c22224d1..cdf4bbe582272277a6a95267e9752010adc51b6b 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -1866,7 +1866,7 @@ static int uvc_scan_device(struct uvc_device *dev) if (list_empty(&dev->chains)) { dev_info(&dev->udev->dev, "No valid video chain found.\n"); - return -1; + return -ENODEV; } /* Add GPIO entity to the first chain. */ @@ -2239,7 +2239,6 @@ static int uvc_probe(struct usb_interface *intf, /* Parse the Video Class control descriptor. */ ret = uvc_parse_control(dev); if (ret < 0) { - ret = -ENODEV; uvc_dbg(dev, PROBE, "Unable to parse UVC descriptors\n"); goto error; } @@ -2275,22 +2274,16 @@ static int uvc_probe(struct usb_interface *intf, goto error; /* Scan the device for video chains. */ - if (uvc_scan_device(dev) < 0) { - ret = -ENODEV; + if (uvc_scan_device(dev) < 0) goto error; - } /* Initialize controls. */ - if (uvc_ctrl_init_device(dev) < 0) { - ret = -ENODEV; + if (uvc_ctrl_init_device(dev) < 0) goto error; - } /* Register video device nodes. */ - if (uvc_register_chains(dev) < 0) { - ret = -ENODEV; + if (uvc_register_chains(dev) < 0) goto error; - } #ifdef CONFIG_MEDIA_CONTROLLER /* Register the media device node */
Now we are replacing most of the error codes with -ENODEV. Instead, Populate the error code from the functions called by uvc_probe(). Take this opportunity to replace a generic error code from uvc_scan_device() into something more meaningful. Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/usb/uvc/uvc_driver.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-)