Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 29 Dec 2022 05:25:42 +0200
From:   Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To:     Ricardo Ribalda <ribalda@chromium.org>
Cc:     Mauro Carvalho Chehab <mchehab@kernel.org>,
        Hans Verkuil <hverkuil@xs4all.nl>,
        Hans Verkuil <hverkuil-cisco@xs4all.nl>,
        linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
        Hans Verkuil <hans.verkuil@cisco.com>
Subject: Re: [PATCH RESEND v2 3/7] media: uvcvideo: Return -EACCES for Wrong
 state error
Message-ID: <Y60Itu7apCFpCKzW@pendragon.ideasonboard.com>
References: <20220920-resend-v4l2-compliance-v2-0-b0ceb15353ac@chromium.org>
 <20220920-resend-v4l2-compliance-v2-3-b0ceb15353ac@chromium.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20220920-resend-v4l2-compliance-v2-3-b0ceb15353ac@chromium.org>
Precedence: bulk

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 */
> 

-- 
Regards,

Laurent Pinchart