[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZwUfD5Kfzv93-46f@valkosipuli.retiisi.eu>
Date: Tue, 8 Oct 2024 12:01:19 +0000
From: Sakari Ailus <sakari.ailus@....fi>
To: Ricardo Ribalda <ribalda@...omium.org>
Cc: Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] media: uvcvideo: Refactor uvc_query_ctrl
Hi Ricardo,
On Tue, Oct 08, 2024 at 07:06:15AM +0000, Ricardo Ribalda wrote:
> Move the query control error logic to its own function.
> There is no functional change introduced by this patch.
>
> Signed-off-by: Ricardo Ribalda <ribalda@...omium.org>
> ---
> drivers/media/usb/uvc/uvc_video.c | 45 ++++++++++++++++++++++-----------------
> 1 file changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index 853dfb7b5f7b..a57272a2c9e1 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -67,30 +67,12 @@ static const char *uvc_query_name(u8 query)
> }
> }
>
> -int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
> - u8 intfnum, u8 cs, void *data, u16 size)
> +static int uvc_query_ctrl_error(struct uvc_device *dev, u8 intfnum, void *data)
> {
> int ret;
> u8 error;
> u8 tmp;
>
> - ret = __uvc_query_ctrl(dev, query, unit, intfnum, cs, data, size,
> - UVC_CTRL_CONTROL_TIMEOUT);
> - if (likely(ret == size))
> - return 0;
> -
> - if (ret > 0 && ret < size) {
> - memset(data + ret, 0, size - ret);
> - return 0;
> - }
> -
> - if (ret != -EPIPE) {
> - dev_err(&dev->udev->dev,
> - "Failed to query (%s) UVC control %u on unit %u: %d (exp. %u).\n",
> - uvc_query_name(query), cs, unit, ret, size);
> - return ret ? ret : -EPIPE;
> - }
> -
> /* Reuse data[0] to request the error code. */
> tmp = *(u8 *)data;
>
> @@ -135,6 +117,31 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
> return -EPIPE;
> }
>
> +int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
> + u8 intfnum, u8 cs, void *data, u16 size)
> +{
> + int ret;
> +
> + ret = __uvc_query_ctrl(dev, query, unit, intfnum, cs, data, size,
> + UVC_CTRL_CONTROL_TIMEOUT);
> + if (likely(ret == size))
> + return 0;
> +
> + if (ret == -EPIPE)
> + return uvc_query_ctrl_error(dev, intfnum, data);
> +
> + dev_err(&dev->udev->dev,
> + "Failed to query (%s) UVC control %u on unit %u: %d (exp. %u).\n",
> + uvc_query_name(query), cs, unit, ret, size);
This message should probably be printed after the check below.
I'd actually move the below check before the ret == -EPIPE check as it's a
successful case (and changing the condition to <= would make the ret ==
size check redundant).
> +
> + if (ret > 0 && ret < size) {
> + memset(data + ret, 0, size - ret);
> + return 0;
> + }
> +
> + return ret ? ret : -EPIPE;
> +}
> +
> static const struct usb_device_id elgato_cam_link_4k = {
> USB_DEVICE(0x0fd9, 0x0066)
> };
>
--
Kind regards,
Sakari Ailus
Powered by blists - more mailing lists