[RESEND,v2,3/7] media: uvcvideo: Return -EACCES for Wrong state error

Message ID 20220920-resend-v4l2-compliance-v2-3-b0ceb15353ac@chromium.org
State New
Headers
Series Follow-up patches for uvc v4l2-compliance |

Commit Message

Ricardo Ribalda Dec. 2, 2022, 5:21 p.m. UTC
  For error 2 (Wrong state) return -EACCES instead of -EILSEQ.
EACCES is a much more appropriate error code. EILSEQ will return
"Invalid or incomplete multibyte or wide character." in strerror(),
which is a *very* confusing message.

Suggested-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_video.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Laurent Pinchart Dec. 29, 2022, 3:25 a.m. UTC | #1
Hi Ricardo,

Thank you for the patch.

On Fri, Dec 02, 2022 at 06:21:37PM +0100, Ricardo Ribalda wrote:
> For error 2 (Wrong state) return -EACCES instead of -EILSEQ.
> EACCES is a much more appropriate error code. EILSEQ will return
> "Invalid or incomplete multibyte or wide character." in strerror(),
> which is a *very* confusing message.

Unless there's an objection, I'd like to use the following text to
replace the commit message to provide more information:

Error 2 is defined by UVC as

  Wrong State: The device is in a state that disallows the specific
  request. The device will remain in this state until a specific action
  from the host or the user is completed.

This is documented as happening happen when attempting to set the value
of a manual control when the device is in auto mode. While V4L2 allows
this, the closest error code defined by VIDIOC_S_CTRL is indeed EACCES:

EACCES

    Attempt to set a read-only control or to get a write-only control.

    Or if there is an attempt to set an inactive control and the driver
    is not capable of caching the new value until the control is active
    again.

Replace EILSEQ with EACCESS.

> Suggested-by: Hans Verkuil <hans.verkuil@cisco.com>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/usb/uvc/uvc_video.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index 2cf7f692c0bb..497073a50194 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -108,7 +108,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
>  	case 1: /* Not ready */
>  		return -EBUSY;
>  	case 2: /* Wrong state */
> -		return -EILSEQ;
> +		return -EACCES;
>  	case 3: /* Power */
>  		return -EREMOTE;
>  	case 4: /* Out of range */
>
  

Patch

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 2cf7f692c0bb..497073a50194 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -108,7 +108,7 @@  int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
 	case 1: /* Not ready */
 		return -EBUSY;
 	case 2: /* Wrong state */
-		return -EILSEQ;
+		return -EACCES;
 	case 3: /* Power */
 		return -EREMOTE;
 	case 4: /* Out of range */