* [PATCH] uvcvideo: improve error handling in uvc_query_ctrl()
@ 2021-03-22 12:05 Hans Verkuil
2021-03-23 15:25 ` Ricardo Ribalda Delgado
0 siblings, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2021-03-22 12:05 UTC (permalink / raw)
To: Ricardo Ribalda; +Cc: Linux Media Mailing List
- If __uvc_query_ctrl() failed with a non-EPIPE error, then
report that with dev_err. If an error code is obtained, then
report that with dev_dbg.
- 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.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
Ricardo, this too can be added to the uvc series.
---
drivers/media/usb/uvc/uvc_video.c | 44 +++++++++++++++++--------------
1 file changed, 24 insertions(+), 20 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index b63c073ec30e..3f461bb4eeb9 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -68,7 +68,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
u8 intfnum, u8 cs, void *data, u16 size)
{
int ret;
- u8 error;
+ u8 error = 0;
u8 tmp;
ret = __uvc_query_ctrl(dev, query, unit, intfnum, cs, data, size,
@@ -76,35 +76,39 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
if (likely(ret == size))
return 0;
- dev_dbg(&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);
+ ret = ret < 0 ? ret : -EPIPE;
- if (ret != -EPIPE)
- return ret;
-
- tmp = *(u8 *)data;
+ if (ret == -EPIPE) {
+ tmp = *(u8 *)data;
- ret = __uvc_query_ctrl(dev, UVC_GET_CUR, 0, intfnum,
- UVC_VC_REQUEST_ERROR_CODE_CONTROL, data, 1,
- UVC_CTRL_CONTROL_TIMEOUT);
+ ret = __uvc_query_ctrl(dev, UVC_GET_CUR, 0, intfnum,
+ UVC_VC_REQUEST_ERROR_CODE_CONTROL, data, 1,
+ UVC_CTRL_CONTROL_TIMEOUT);
- error = *(u8 *)data;
- *(u8 *)data = tmp;
+ if (ret == 1)
+ error = *(u8 *)data;
+ *(u8 *)data = tmp;
+ if (ret != 1)
+ ret = ret < 0 ? ret : -EPIPE;
+ }
- if (ret != 1)
- return ret < 0 ? ret : -EPIPE;
+ if (error)
+ dev_dbg(&dev->udev->dev,
+ "Failed to query (%s) UVC control %u on unit %u: got error %u.\n",
+ uvc_query_name(query), cs, unit, error);
+ else
+ 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);
- uvc_dbg(dev, CONTROL, "Control error %u\n", error);
+ if (!error)
+ return ret;
switch (error) {
- case 0:
- /* Cannot happen - we received a STALL */
- return -EPIPE;
case 1: /* Not ready */
return -EBUSY;
case 2: /* Wrong state */
- return -EILSEQ;
+ return -EACCES;
case 3: /* Power */
return -EREMOTE;
case 4: /* Out of range */
--
2.30.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] uvcvideo: improve error handling in uvc_query_ctrl()
2021-03-22 12:05 [PATCH] uvcvideo: improve error handling in uvc_query_ctrl() Hans Verkuil
@ 2021-03-23 15:25 ` Ricardo Ribalda Delgado
2021-03-25 12:18 ` Hans Verkuil
0 siblings, 1 reply; 8+ messages in thread
From: Ricardo Ribalda Delgado @ 2021-03-23 15:25 UTC (permalink / raw)
To: Hans Verkuil; +Cc: Ricardo Ribalda, Linux Media Mailing List
Hi Hans
Thanks for the patch. I like how uvc is ending :)
On Mon, Mar 22, 2021 at 1:09 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> - If __uvc_query_ctrl() failed with a non-EPIPE error, then
> report that with dev_err. If an error code is obtained, then
> report that with dev_dbg.
>
> - 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.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
> Ricardo, this too can be added to the uvc series.
> ---
> drivers/media/usb/uvc/uvc_video.c | 44 +++++++++++++++++--------------
> 1 file changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index b63c073ec30e..3f461bb4eeb9 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -68,7 +68,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
> u8 intfnum, u8 cs, void *data, u16 size)
> {
> int ret;
> - u8 error;
> + u8 error = 0;
> u8 tmp;
>
> ret = __uvc_query_ctrl(dev, query, unit, intfnum, cs, data, size,
> @@ -76,35 +76,39 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
> if (likely(ret == size))
> return 0;
>
> - dev_dbg(&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);
> + ret = ret < 0 ? ret : -EPIPE;
>
> - if (ret != -EPIPE)
> - return ret;
> -
> - tmp = *(u8 *)data;
> + if (ret == -EPIPE) {
> + tmp = *(u8 *)data;
>
> - ret = __uvc_query_ctrl(dev, UVC_GET_CUR, 0, intfnum,
> - UVC_VC_REQUEST_ERROR_CODE_CONTROL, data, 1,
> - UVC_CTRL_CONTROL_TIMEOUT);
> + ret = __uvc_query_ctrl(dev, UVC_GET_CUR, 0, intfnum,
> + UVC_VC_REQUEST_ERROR_CODE_CONTROL, data, 1,
> + UVC_CTRL_CONTROL_TIMEOUT);
>
> - error = *(u8 *)data;
> - *(u8 *)data = tmp;
> + if (ret == 1)
> + error = *(u8 *)data;
> + *(u8 *)data = tmp;
> + if (ret != 1)
> + ret = ret < 0 ? ret : -EPIPE;
> + }
>
> - if (ret != 1)
> - return ret < 0 ? ret : -EPIPE;
> + if (error)
> + dev_dbg(&dev->udev->dev,
> + "Failed to query (%s) UVC control %u on unit %u: got error %u.\n",
> + uvc_query_name(query), cs, unit, error);
> + else
> + 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);
If __uvc_query_ctrl and UVC_VC_REQUEST_ERROR_CODE_CONTROL failed,
error is 0. And I think that you want to show a dev_err in that case.
Maybe we can initialize error to 7 ?
>
> - uvc_dbg(dev, CONTROL, "Control error %u\n", error);
> + if (!error)
> + return ret;
I think we do not want these two lines (read next comment)
>
> switch (error) {
> - case 0:
> - /* Cannot happen - we received a STALL */
> - return -EPIPE;
> case 1: /* Not ready */
> return -EBUSY;
> case 2: /* Wrong state */
> - return -EILSEQ;
> + return -EACCES;
> case 3: /* Power */
> return -EREMOTE;
> case 4: /* Out of range */
Maybe we want a dev_dbg if the error code is unknown and return ret?
> --
> 2.30.0
>
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] uvcvideo: improve error handling in uvc_query_ctrl()
2021-03-23 15:25 ` Ricardo Ribalda Delgado
@ 2021-03-25 12:18 ` Hans Verkuil
2021-03-25 13:54 ` Ricardo Ribalda
0 siblings, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2021-03-25 12:18 UTC (permalink / raw)
To: Ricardo Ribalda Delgado; +Cc: Ricardo Ribalda, Linux Media Mailing List
On 23/03/2021 16:25, Ricardo Ribalda Delgado wrote:
> Hi Hans
>
> Thanks for the patch. I like how uvc is ending :)
>
> On Mon, Mar 22, 2021 at 1:09 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>
>> - If __uvc_query_ctrl() failed with a non-EPIPE error, then
>> report that with dev_err. If an error code is obtained, then
>> report that with dev_dbg.
>>
>> - 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.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>> Ricardo, this too can be added to the uvc series.
>> ---
>> drivers/media/usb/uvc/uvc_video.c | 44 +++++++++++++++++--------------
>> 1 file changed, 24 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
>> index b63c073ec30e..3f461bb4eeb9 100644
>> --- a/drivers/media/usb/uvc/uvc_video.c
>> +++ b/drivers/media/usb/uvc/uvc_video.c
>> @@ -68,7 +68,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
>> u8 intfnum, u8 cs, void *data, u16 size)
>> {
>> int ret;
>> - u8 error;
>> + u8 error = 0;
>> u8 tmp;
>>
>> ret = __uvc_query_ctrl(dev, query, unit, intfnum, cs, data, size,
>> @@ -76,35 +76,39 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
>> if (likely(ret == size))
>> return 0;
>>
>> - dev_dbg(&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);
>> + ret = ret < 0 ? ret : -EPIPE;
>>
>> - if (ret != -EPIPE)
>> - return ret;
>> -
>> - tmp = *(u8 *)data;
>> + if (ret == -EPIPE) {
>> + tmp = *(u8 *)data;
>>
>> - ret = __uvc_query_ctrl(dev, UVC_GET_CUR, 0, intfnum,
>> - UVC_VC_REQUEST_ERROR_CODE_CONTROL, data, 1,
>> - UVC_CTRL_CONTROL_TIMEOUT);
>> + ret = __uvc_query_ctrl(dev, UVC_GET_CUR, 0, intfnum,
>> + UVC_VC_REQUEST_ERROR_CODE_CONTROL, data, 1,
>> + UVC_CTRL_CONTROL_TIMEOUT);
>>
>> - error = *(u8 *)data;
>> - *(u8 *)data = tmp;
>> + if (ret == 1)
>> + error = *(u8 *)data;
>> + *(u8 *)data = tmp;
>> + if (ret != 1)
>> + ret = ret < 0 ? ret : -EPIPE;
>> + }
>>
>> - if (ret != 1)
>> - return ret < 0 ? ret : -EPIPE;
>> + if (error)
>> + dev_dbg(&dev->udev->dev,
>> + "Failed to query (%s) UVC control %u on unit %u: got error %u.\n",
>> + uvc_query_name(query), cs, unit, error);
>> + else
>> + 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);
>
> If __uvc_query_ctrl and UVC_VC_REQUEST_ERROR_CODE_CONTROL failed,
> error is 0. And I think that you want to show a dev_err in that case.
> Maybe we can initialize error to 7 ?
I'm confused, if error == 0, then it does show dev_err.
>
>
>>
>> - uvc_dbg(dev, CONTROL, "Control error %u\n", error);
>> + if (!error)
>> + return ret;
> I think we do not want these two lines (read next comment)
>>
>> switch (error) {
>> - case 0:
>> - /* Cannot happen - we received a STALL */
>> - return -EPIPE;
>> case 1: /* Not ready */
>> return -EBUSY;
>> case 2: /* Wrong state */
>> - return -EILSEQ;
>> + return -EACCES;
>> case 3: /* Power */
>> return -EREMOTE;
>> case 4: /* Out of range */
>
> Maybe we want a dev_dbg if the error code is unknown and return ret?
Make sense.
Regards,
Hans
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] uvcvideo: improve error handling in uvc_query_ctrl()
2021-03-25 12:18 ` Hans Verkuil
@ 2021-03-25 13:54 ` Ricardo Ribalda
2021-03-25 13:57 ` Hans Verkuil
0 siblings, 1 reply; 8+ messages in thread
From: Ricardo Ribalda @ 2021-03-25 13:54 UTC (permalink / raw)
To: Hans Verkuil; +Cc: Ricardo Ribalda Delgado, Linux Media Mailing List
hi Hans
On Thu, Mar 25, 2021 at 1:18 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> On 23/03/2021 16:25, Ricardo Ribalda Delgado wrote:
> > Hi Hans
> >
> > Thanks for the patch. I like how uvc is ending :)
> >
> > On Mon, Mar 22, 2021 at 1:09 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> >>
> >> - If __uvc_query_ctrl() failed with a non-EPIPE error, then
> >> report that with dev_err. If an error code is obtained, then
> >> report that with dev_dbg.
> >>
> >> - 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.
> >>
> >> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> >> ---
> >> Ricardo, this too can be added to the uvc series.
> >> ---
> >> drivers/media/usb/uvc/uvc_video.c | 44 +++++++++++++++++--------------
> >> 1 file changed, 24 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> >> index b63c073ec30e..3f461bb4eeb9 100644
> >> --- a/drivers/media/usb/uvc/uvc_video.c
> >> +++ b/drivers/media/usb/uvc/uvc_video.c
> >> @@ -68,7 +68,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
> >> u8 intfnum, u8 cs, void *data, u16 size)
> >> {
> >> int ret;
> >> - u8 error;
> >> + u8 error = 0;
> >> u8 tmp;
> >>
> >> ret = __uvc_query_ctrl(dev, query, unit, intfnum, cs, data, size,
> >> @@ -76,35 +76,39 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
> >> if (likely(ret == size))
> >> return 0;
> >>
> >> - dev_dbg(&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);
> >> + ret = ret < 0 ? ret : -EPIPE;
> >>
> >> - if (ret != -EPIPE)
> >> - return ret;
> >> -
> >> - tmp = *(u8 *)data;
> >> + if (ret == -EPIPE) {
> >> + tmp = *(u8 *)data;
> >>
> >> - ret = __uvc_query_ctrl(dev, UVC_GET_CUR, 0, intfnum,
> >> - UVC_VC_REQUEST_ERROR_CODE_CONTROL, data, 1,
> >> - UVC_CTRL_CONTROL_TIMEOUT);
> >> + ret = __uvc_query_ctrl(dev, UVC_GET_CUR, 0, intfnum,
> >> + UVC_VC_REQUEST_ERROR_CODE_CONTROL, data, 1,
> >> + UVC_CTRL_CONTROL_TIMEOUT);
> >>
> >> - error = *(u8 *)data;
> >> - *(u8 *)data = tmp;
> >> + if (ret == 1)
> >> + error = *(u8 *)data;
> >> + *(u8 *)data = tmp;
> >> + if (ret != 1)
> >> + ret = ret < 0 ? ret : -EPIPE;
> >> + }
> >>
> >> - if (ret != 1)
> >> - return ret < 0 ? ret : -EPIPE;
> >> + if (error)
> >> + dev_dbg(&dev->udev->dev,
> >> + "Failed to query (%s) UVC control %u on unit %u: got error %u.\n",
> >> + uvc_query_name(query), cs, unit, error);
> >> + else
> >> + 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);
> >
> > If __uvc_query_ctrl and UVC_VC_REQUEST_ERROR_CODE_CONTROL failed,
> > error is 0. And I think that you want to show a dev_err in that case.
> > Maybe we can initialize error to 7 ?
>
> I'm confused, if error == 0, then it does show dev_err.
My bad.
Ignore the message.
Can we write it as ?:
if (!error) {
dev_dbg(&dev->udev->dev,
"Failed to query (%s) UVC control %u on unit %u: got error %u.\n",
uvc_query_name(query), cs, unit, error);
return ret;
}
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);
>
> >
> >
> >>
> >> - uvc_dbg(dev, CONTROL, "Control error %u\n", error);
> >> + if (!error)
> >> + return ret;
> > I think we do not want these two lines (read next comment)
> >>
> >> switch (error) {
> >> - case 0:
> >> - /* Cannot happen - we received a STALL */
> >> - return -EPIPE;
> >> case 1: /* Not ready */
> >> return -EBUSY;
> >> case 2: /* Wrong state */
> >> - return -EILSEQ;
> >> + return -EACCES;
> >> case 3: /* Power */
> >> return -EREMOTE;
> >> case 4: /* Out of range */
> >
> > Maybe we want a dev_dbg if the error code is unknown and return ret?
>
> Make sense.
>
> Regards,
>
> Hans
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] uvcvideo: improve error handling in uvc_query_ctrl()
2021-03-25 13:54 ` Ricardo Ribalda
@ 2021-03-25 13:57 ` Hans Verkuil
2021-03-25 14:02 ` Ricardo Ribalda
0 siblings, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2021-03-25 13:57 UTC (permalink / raw)
To: Ricardo Ribalda; +Cc: Ricardo Ribalda Delgado, Linux Media Mailing List
On 25/03/2021 14:54, Ricardo Ribalda wrote:
> hi Hans
>
> On Thu, Mar 25, 2021 at 1:18 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>
>> On 23/03/2021 16:25, Ricardo Ribalda Delgado wrote:
>>> Hi Hans
>>>
>>> Thanks for the patch. I like how uvc is ending :)
>>>
>>> On Mon, Mar 22, 2021 at 1:09 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>>>
>>>> - If __uvc_query_ctrl() failed with a non-EPIPE error, then
>>>> report that with dev_err. If an error code is obtained, then
>>>> report that with dev_dbg.
>>>>
>>>> - 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.
>>>>
>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>>> ---
>>>> Ricardo, this too can be added to the uvc series.
>>>> ---
>>>> drivers/media/usb/uvc/uvc_video.c | 44 +++++++++++++++++--------------
>>>> 1 file changed, 24 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
>>>> index b63c073ec30e..3f461bb4eeb9 100644
>>>> --- a/drivers/media/usb/uvc/uvc_video.c
>>>> +++ b/drivers/media/usb/uvc/uvc_video.c
>>>> @@ -68,7 +68,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
>>>> u8 intfnum, u8 cs, void *data, u16 size)
>>>> {
>>>> int ret;
>>>> - u8 error;
>>>> + u8 error = 0;
>>>> u8 tmp;
>>>>
>>>> ret = __uvc_query_ctrl(dev, query, unit, intfnum, cs, data, size,
>>>> @@ -76,35 +76,39 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
>>>> if (likely(ret == size))
>>>> return 0;
>>>>
>>>> - dev_dbg(&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);
>>>> + ret = ret < 0 ? ret : -EPIPE;
>>>>
>>>> - if (ret != -EPIPE)
>>>> - return ret;
>>>> -
>>>> - tmp = *(u8 *)data;
>>>> + if (ret == -EPIPE) {
>>>> + tmp = *(u8 *)data;
>>>>
>>>> - ret = __uvc_query_ctrl(dev, UVC_GET_CUR, 0, intfnum,
>>>> - UVC_VC_REQUEST_ERROR_CODE_CONTROL, data, 1,
>>>> - UVC_CTRL_CONTROL_TIMEOUT);
>>>> + ret = __uvc_query_ctrl(dev, UVC_GET_CUR, 0, intfnum,
>>>> + UVC_VC_REQUEST_ERROR_CODE_CONTROL, data, 1,
>>>> + UVC_CTRL_CONTROL_TIMEOUT);
>>>>
>>>> - error = *(u8 *)data;
>>>> - *(u8 *)data = tmp;
>>>> + if (ret == 1)
>>>> + error = *(u8 *)data;
>>>> + *(u8 *)data = tmp;
>>>> + if (ret != 1)
>>>> + ret = ret < 0 ? ret : -EPIPE;
>>>> + }
>>>>
>>>> - if (ret != 1)
>>>> - return ret < 0 ? ret : -EPIPE;
>>>> + if (error)
>>>> + dev_dbg(&dev->udev->dev,
>>>> + "Failed to query (%s) UVC control %u on unit %u: got error %u.\n",
>>>> + uvc_query_name(query), cs, unit, error);
>>>> + else
>>>> + 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);
>>>
>>> If __uvc_query_ctrl and UVC_VC_REQUEST_ERROR_CODE_CONTROL failed,
>>> error is 0. And I think that you want to show a dev_err in that case.
>>> Maybe we can initialize error to 7 ?
>>
>> I'm confused, if error == 0, then it does show dev_err.
>
> My bad.
>
> Ignore the message.
>
> Can we write it as ?:
>
> if (!error) {
> dev_dbg(&dev->udev->dev,
> "Failed to query (%s) UVC control %u on unit %u: got error %u.\n",
> uvc_query_name(query), cs, unit, error);
> return ret;
> }
>
> 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);
Sure! Just take this patch as inspiration :-)
Regards,
Hans
>
>
>
>>
>>>
>>>
>>>>
>>>> - uvc_dbg(dev, CONTROL, "Control error %u\n", error);
>>>> + if (!error)
>>>> + return ret;
>>> I think we do not want these two lines (read next comment)
>>>>
>>>> switch (error) {
>>>> - case 0:
>>>> - /* Cannot happen - we received a STALL */
>>>> - return -EPIPE;
>>>> case 1: /* Not ready */
>>>> return -EBUSY;
>>>> case 2: /* Wrong state */
>>>> - return -EILSEQ;
>>>> + return -EACCES;
>>>> case 3: /* Power */
>>>> return -EREMOTE;
>>>> case 4: /* Out of range */
>>>
>>> Maybe we want a dev_dbg if the error code is unknown and return ret?
>>
>> Make sense.
>>
>> Regards,
>>
>> Hans
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] uvcvideo: improve error handling in uvc_query_ctrl()
2021-03-25 13:57 ` Hans Verkuil
@ 2021-03-25 14:02 ` Ricardo Ribalda
2021-03-25 14:03 ` Hans Verkuil
0 siblings, 1 reply; 8+ messages in thread
From: Ricardo Ribalda @ 2021-03-25 14:02 UTC (permalink / raw)
To: Hans Verkuil; +Cc: Ricardo Ribalda Delgado, Linux Media Mailing List
Hi Hans
On Thu, Mar 25, 2021 at 2:57 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> On 25/03/2021 14:54, Ricardo Ribalda wrote:
> > hi Hans
> >
> > On Thu, Mar 25, 2021 at 1:18 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> >>
> >> On 23/03/2021 16:25, Ricardo Ribalda Delgado wrote:
> >>> Hi Hans
> >>>
> >>> Thanks for the patch. I like how uvc is ending :)
> >>>
> >>> On Mon, Mar 22, 2021 at 1:09 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> >>>>
> >>>> - If __uvc_query_ctrl() failed with a non-EPIPE error, then
> >>>> report that with dev_err. If an error code is obtained, then
> >>>> report that with dev_dbg.
> >>>>
> >>>> - 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.
> >>>>
> >>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> >>>> ---
> >>>> Ricardo, this too can be added to the uvc series.
> >>>> ---
> >>>> drivers/media/usb/uvc/uvc_video.c | 44 +++++++++++++++++--------------
> >>>> 1 file changed, 24 insertions(+), 20 deletions(-)
> >>>>
> >>>> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> >>>> index b63c073ec30e..3f461bb4eeb9 100644
> >>>> --- a/drivers/media/usb/uvc/uvc_video.c
> >>>> +++ b/drivers/media/usb/uvc/uvc_video.c
> >>>> @@ -68,7 +68,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
> >>>> u8 intfnum, u8 cs, void *data, u16 size)
> >>>> {
> >>>> int ret;
> >>>> - u8 error;
> >>>> + u8 error = 0;
> >>>> u8 tmp;
> >>>>
> >>>> ret = __uvc_query_ctrl(dev, query, unit, intfnum, cs, data, size,
> >>>> @@ -76,35 +76,39 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
> >>>> if (likely(ret == size))
> >>>> return 0;
> >>>>
> >>>> - dev_dbg(&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);
> >>>> + ret = ret < 0 ? ret : -EPIPE;
> >>>>
> >>>> - if (ret != -EPIPE)
> >>>> - return ret;
> >>>> -
> >>>> - tmp = *(u8 *)data;
> >>>> + if (ret == -EPIPE) {
> >>>> + tmp = *(u8 *)data;
> >>>>
> >>>> - ret = __uvc_query_ctrl(dev, UVC_GET_CUR, 0, intfnum,
> >>>> - UVC_VC_REQUEST_ERROR_CODE_CONTROL, data, 1,
> >>>> - UVC_CTRL_CONTROL_TIMEOUT);
> >>>> + ret = __uvc_query_ctrl(dev, UVC_GET_CUR, 0, intfnum,
> >>>> + UVC_VC_REQUEST_ERROR_CODE_CONTROL, data, 1,
> >>>> + UVC_CTRL_CONTROL_TIMEOUT);
> >>>>
> >>>> - error = *(u8 *)data;
> >>>> - *(u8 *)data = tmp;
> >>>> + if (ret == 1)
> >>>> + error = *(u8 *)data;
> >>>> + *(u8 *)data = tmp;
> >>>> + if (ret != 1)
> >>>> + ret = ret < 0 ? ret : -EPIPE;
> >>>> + }
> >>>>
> >>>> - if (ret != 1)
> >>>> - return ret < 0 ? ret : -EPIPE;
> >>>> + if (error)
> >>>> + dev_dbg(&dev->udev->dev,
> >>>> + "Failed to query (%s) UVC control %u on unit %u: got error %u.\n",
> >>>> + uvc_query_name(query), cs, unit, error);
> >>>> + else
> >>>> + 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);
> >>>
> >>> If __uvc_query_ctrl and UVC_VC_REQUEST_ERROR_CODE_CONTROL failed,
> >>> error is 0. And I think that you want to show a dev_err in that case.
> >>> Maybe we can initialize error to 7 ?
> >>
> >> I'm confused, if error == 0, then it does show dev_err.
> >
> > My bad.
> >
> > Ignore the message.
> >
> > Can we write it as ?:
> >
> > if (!error) {
> > dev_dbg(&dev->udev->dev,
> > "Failed to query (%s) UVC control %u on unit %u: got error %u.\n",
> > uvc_query_name(query), cs, unit, error);
> > return ret;
> > }
> >
> > 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);
>
> Sure! Just take this patch as inspiration :-)
Do you mind if I modify it before resend?
>
> Regards,
>
> Hans
>
> >
> >
> >
> >>
> >>>
> >>>
> >>>>
> >>>> - uvc_dbg(dev, CONTROL, "Control error %u\n", error);
> >>>> + if (!error)
> >>>> + return ret;
> >>> I think we do not want these two lines (read next comment)
> >>>>
> >>>> switch (error) {
> >>>> - case 0:
> >>>> - /* Cannot happen - we received a STALL */
> >>>> - return -EPIPE;
> >>>> case 1: /* Not ready */
> >>>> return -EBUSY;
> >>>> case 2: /* Wrong state */
> >>>> - return -EILSEQ;
> >>>> + return -EACCES;
> >>>> case 3: /* Power */
> >>>> return -EREMOTE;
> >>>> case 4: /* Out of range */
> >>>
> >>> Maybe we want a dev_dbg if the error code is unknown and return ret?
> >>
> >> Make sense.
> >>
> >> Regards,
> >>
> >> Hans
> >
> >
> >
>
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] uvcvideo: improve error handling in uvc_query_ctrl()
2021-03-25 14:02 ` Ricardo Ribalda
@ 2021-03-25 14:03 ` Hans Verkuil
2021-03-25 15:56 ` Ricardo Ribalda Delgado
0 siblings, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2021-03-25 14:03 UTC (permalink / raw)
To: Ricardo Ribalda; +Cc: Ricardo Ribalda Delgado, Linux Media Mailing List
On 25/03/2021 15:02, Ricardo Ribalda wrote:
> Hi Hans
>
> On Thu, Mar 25, 2021 at 2:57 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>
>> On 25/03/2021 14:54, Ricardo Ribalda wrote:
>>> hi Hans
>>>
>>> On Thu, Mar 25, 2021 at 1:18 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>>>
>>>> On 23/03/2021 16:25, Ricardo Ribalda Delgado wrote:
>>>>> Hi Hans
>>>>>
>>>>> Thanks for the patch. I like how uvc is ending :)
>>>>>
>>>>> On Mon, Mar 22, 2021 at 1:09 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>>>>>
>>>>>> - If __uvc_query_ctrl() failed with a non-EPIPE error, then
>>>>>> report that with dev_err. If an error code is obtained, then
>>>>>> report that with dev_dbg.
>>>>>>
>>>>>> - 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.
>>>>>>
>>>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>>>>> ---
>>>>>> Ricardo, this too can be added to the uvc series.
>>>>>> ---
>>>>>> drivers/media/usb/uvc/uvc_video.c | 44 +++++++++++++++++--------------
>>>>>> 1 file changed, 24 insertions(+), 20 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
>>>>>> index b63c073ec30e..3f461bb4eeb9 100644
>>>>>> --- a/drivers/media/usb/uvc/uvc_video.c
>>>>>> +++ b/drivers/media/usb/uvc/uvc_video.c
>>>>>> @@ -68,7 +68,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
>>>>>> u8 intfnum, u8 cs, void *data, u16 size)
>>>>>> {
>>>>>> int ret;
>>>>>> - u8 error;
>>>>>> + u8 error = 0;
>>>>>> u8 tmp;
>>>>>>
>>>>>> ret = __uvc_query_ctrl(dev, query, unit, intfnum, cs, data, size,
>>>>>> @@ -76,35 +76,39 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
>>>>>> if (likely(ret == size))
>>>>>> return 0;
>>>>>>
>>>>>> - dev_dbg(&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);
>>>>>> + ret = ret < 0 ? ret : -EPIPE;
>>>>>>
>>>>>> - if (ret != -EPIPE)
>>>>>> - return ret;
>>>>>> -
>>>>>> - tmp = *(u8 *)data;
>>>>>> + if (ret == -EPIPE) {
>>>>>> + tmp = *(u8 *)data;
>>>>>>
>>>>>> - ret = __uvc_query_ctrl(dev, UVC_GET_CUR, 0, intfnum,
>>>>>> - UVC_VC_REQUEST_ERROR_CODE_CONTROL, data, 1,
>>>>>> - UVC_CTRL_CONTROL_TIMEOUT);
>>>>>> + ret = __uvc_query_ctrl(dev, UVC_GET_CUR, 0, intfnum,
>>>>>> + UVC_VC_REQUEST_ERROR_CODE_CONTROL, data, 1,
>>>>>> + UVC_CTRL_CONTROL_TIMEOUT);
>>>>>>
>>>>>> - error = *(u8 *)data;
>>>>>> - *(u8 *)data = tmp;
>>>>>> + if (ret == 1)
>>>>>> + error = *(u8 *)data;
>>>>>> + *(u8 *)data = tmp;
>>>>>> + if (ret != 1)
>>>>>> + ret = ret < 0 ? ret : -EPIPE;
>>>>>> + }
>>>>>>
>>>>>> - if (ret != 1)
>>>>>> - return ret < 0 ? ret : -EPIPE;
>>>>>> + if (error)
>>>>>> + dev_dbg(&dev->udev->dev,
>>>>>> + "Failed to query (%s) UVC control %u on unit %u: got error %u.\n",
>>>>>> + uvc_query_name(query), cs, unit, error);
>>>>>> + else
>>>>>> + 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);
>>>>>
>>>>> If __uvc_query_ctrl and UVC_VC_REQUEST_ERROR_CODE_CONTROL failed,
>>>>> error is 0. And I think that you want to show a dev_err in that case.
>>>>> Maybe we can initialize error to 7 ?
>>>>
>>>> I'm confused, if error == 0, then it does show dev_err.
>>>
>>> My bad.
>>>
>>> Ignore the message.
>>>
>>> Can we write it as ?:
>>>
>>> if (!error) {
>>> dev_dbg(&dev->udev->dev,
>>> "Failed to query (%s) UVC control %u on unit %u: got error %u.\n",
>>> uvc_query_name(query), cs, unit, error);
>>> return ret;
>>> }
>>>
>>> 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);
>>
>> Sure! Just take this patch as inspiration :-)
>
> Do you mind if I modify it before resend?
No problem, feel free!
Hans
>>
>> Regards,
>>
>> Hans
>>
>>>
>>>
>>>
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> - uvc_dbg(dev, CONTROL, "Control error %u\n", error);
>>>>>> + if (!error)
>>>>>> + return ret;
>>>>> I think we do not want these two lines (read next comment)
>>>>>>
>>>>>> switch (error) {
>>>>>> - case 0:
>>>>>> - /* Cannot happen - we received a STALL */
>>>>>> - return -EPIPE;
>>>>>> case 1: /* Not ready */
>>>>>> return -EBUSY;
>>>>>> case 2: /* Wrong state */
>>>>>> - return -EILSEQ;
>>>>>> + return -EACCES;
>>>>>> case 3: /* Power */
>>>>>> return -EREMOTE;
>>>>>> case 4: /* Out of range */
>>>>>
>>>>> Maybe we want a dev_dbg if the error code is unknown and return ret?
>>>>
>>>> Make sense.
>>>>
>>>> Regards,
>>>>
>>>> Hans
>>>
>>>
>>>
>>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] uvcvideo: improve error handling in uvc_query_ctrl()
2021-03-25 14:03 ` Hans Verkuil
@ 2021-03-25 15:56 ` Ricardo Ribalda Delgado
0 siblings, 0 replies; 8+ messages in thread
From: Ricardo Ribalda Delgado @ 2021-03-25 15:56 UTC (permalink / raw)
To: Hans Verkuil; +Cc: Ricardo Ribalda, Linux Media Mailing List
Hi Hans
On Thu, Mar 25, 2021 at 3:03 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> On 25/03/2021 15:02, Ricardo Ribalda wrote:
> > Hi Hans
> >
> > On Thu, Mar 25, 2021 at 2:57 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> >>
> >> On 25/03/2021 14:54, Ricardo Ribalda wrote:
> >>> hi Hans
> >>>
> >>> On Thu, Mar 25, 2021 at 1:18 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> >>>>
> >>>> On 23/03/2021 16:25, Ricardo Ribalda Delgado wrote:
> >>>>> Hi Hans
> >>>>>
> >>>>> Thanks for the patch. I like how uvc is ending :)
> >>>>>
> >>>>> On Mon, Mar 22, 2021 at 1:09 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> >>>>>>
> >>>>>> - If __uvc_query_ctrl() failed with a non-EPIPE error, then
> >>>>>> report that with dev_err. If an error code is obtained, then
> >>>>>> report that with dev_dbg.
> >>>>>>
> >>>>>> - 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.
> >>>>>>
> >>>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> >>>>>> ---
> >>>>>> Ricardo, this too can be added to the uvc series.
> >>>>>> ---
> >>>>>> drivers/media/usb/uvc/uvc_video.c | 44 +++++++++++++++++--------------
> >>>>>> 1 file changed, 24 insertions(+), 20 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> >>>>>> index b63c073ec30e..3f461bb4eeb9 100644
> >>>>>> --- a/drivers/media/usb/uvc/uvc_video.c
> >>>>>> +++ b/drivers/media/usb/uvc/uvc_video.c
> >>>>>> @@ -68,7 +68,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
> >>>>>> u8 intfnum, u8 cs, void *data, u16 size)
> >>>>>> {
> >>>>>> int ret;
> >>>>>> - u8 error;
> >>>>>> + u8 error = 0;
> >>>>>> u8 tmp;
> >>>>>>
> >>>>>> ret = __uvc_query_ctrl(dev, query, unit, intfnum, cs, data, size,
> >>>>>> @@ -76,35 +76,39 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
> >>>>>> if (likely(ret == size))
> >>>>>> return 0;
> >>>>>>
> >>>>>> - dev_dbg(&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);
> >>>>>> + ret = ret < 0 ? ret : -EPIPE;
> >>>>>>
> >>>>>> - if (ret != -EPIPE)
> >>>>>> - return ret;
> >>>>>> -
> >>>>>> - tmp = *(u8 *)data;
> >>>>>> + if (ret == -EPIPE) {
> >>>>>> + tmp = *(u8 *)data;
> >>>>>>
> >>>>>> - ret = __uvc_query_ctrl(dev, UVC_GET_CUR, 0, intfnum,
> >>>>>> - UVC_VC_REQUEST_ERROR_CODE_CONTROL, data, 1,
> >>>>>> - UVC_CTRL_CONTROL_TIMEOUT);
> >>>>>> + ret = __uvc_query_ctrl(dev, UVC_GET_CUR, 0, intfnum,
> >>>>>> + UVC_VC_REQUEST_ERROR_CODE_CONTROL, data, 1,
> >>>>>> + UVC_CTRL_CONTROL_TIMEOUT);
> >>>>>>
> >>>>>> - error = *(u8 *)data;
> >>>>>> - *(u8 *)data = tmp;
> >>>>>> + if (ret == 1)
> >>>>>> + error = *(u8 *)data;
> >>>>>> + *(u8 *)data = tmp;
> >>>>>> + if (ret != 1)
> >>>>>> + ret = ret < 0 ? ret : -EPIPE;
> >>>>>> + }
> >>>>>>
> >>>>>> - if (ret != 1)
> >>>>>> - return ret < 0 ? ret : -EPIPE;
> >>>>>> + if (error)
> >>>>>> + dev_dbg(&dev->udev->dev,
> >>>>>> + "Failed to query (%s) UVC control %u on unit %u: got error %u.\n",
> >>>>>> + uvc_query_name(query), cs, unit, error);
> >>>>>> + else
> >>>>>> + 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);
> >>>>>
> >>>>> If __uvc_query_ctrl and UVC_VC_REQUEST_ERROR_CODE_CONTROL failed,
> >>>>> error is 0. And I think that you want to show a dev_err in that case.
> >>>>> Maybe we can initialize error to 7 ?
> >>>>
> >>>> I'm confused, if error == 0, then it does show dev_err.
> >>>
> >>> My bad.
> >>>
> >>> Ignore the message.
> >>>
> >>> Can we write it as ?:
> >>>
> >>> if (!error) {
> >>> dev_dbg(&dev->udev->dev,
> >>> "Failed to query (%s) UVC control %u on unit %u: got error %u.\n",
> >>> uvc_query_name(query), cs, unit, error);
> >>> return ret;
> >>> }
> >>>
> >>> 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);
> >>
> >> Sure! Just take this patch as inspiration :-)
> >
> > Do you mind if I modify it before resend?
>
> No problem, feel free!
>
Something like?
https://git.kernel.org/pub/scm/linux/kernel/git/ribalda/linux.git/commit/?h=uvc-compliance-v9&id=aaf96b6cab88059d4d70346669d26cde09276586
Will probably repost the series tomorrow.
Thanks!
> Hans
>
> >>
> >> Regards,
> >>
> >> Hans
> >>
> >>>
> >>>
> >>>
> >>>>
> >>>>>
> >>>>>
> >>>>>>
> >>>>>> - uvc_dbg(dev, CONTROL, "Control error %u\n", error);
> >>>>>> + if (!error)
> >>>>>> + return ret;
> >>>>> I think we do not want these two lines (read next comment)
> >>>>>>
> >>>>>> switch (error) {
> >>>>>> - case 0:
> >>>>>> - /* Cannot happen - we received a STALL */
> >>>>>> - return -EPIPE;
> >>>>>> case 1: /* Not ready */
> >>>>>> return -EBUSY;
> >>>>>> case 2: /* Wrong state */
> >>>>>> - return -EILSEQ;
> >>>>>> + return -EACCES;
> >>>>>> case 3: /* Power */
> >>>>>> return -EREMOTE;
> >>>>>> case 4: /* Out of range */
> >>>>>
> >>>>> Maybe we want a dev_dbg if the error code is unknown and return ret?
> >>>>
> >>>> Make sense.
> >>>>
> >>>> Regards,
> >>>>
> >>>> Hans
> >>>
> >>>
> >>>
> >>
> >
> >
>
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-03-25 15:57 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22 12:05 [PATCH] uvcvideo: improve error handling in uvc_query_ctrl() Hans Verkuil
2021-03-23 15:25 ` Ricardo Ribalda Delgado
2021-03-25 12:18 ` Hans Verkuil
2021-03-25 13:54 ` Ricardo Ribalda
2021-03-25 13:57 ` Hans Verkuil
2021-03-25 14:02 ` Ricardo Ribalda
2021-03-25 14:03 ` Hans Verkuil
2021-03-25 15:56 ` Ricardo Ribalda Delgado
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).