Re: usb: gadget: webcam broken?

[Roger Quadros]

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: usb: gadget: webcam broken?

[Petr Cvek]

Dne 2.3.2017 v 09:19 Roger Quadros napsal(a):
> Petr,
> 
> On 02/03/17 06:57, Petr Cvek wrote:
>> Dne 2.3.2017 v 00:22 Laurent Pinchart napsal(a):
>>> Hi Roger,
>>>
>>> On Wednesday 01 Mar 2017 17:09:51 Roger Quadros wrote:
 Hi,

 I'm no longer able to use g_webcam with uvc-gadget [1] since v4.9. Logs at
 the end. It looks like we're goofing up on the control endpoint.

 If I revert the following commit everything works fine.
 commit 4fbac5206afd01b717d4bdc58793d471f3391b4b
 Author: Petr Cvek 
 Date:   Wed Aug 17 12:36:57 2016 +0200

 usb: gadget: uvc: Add missing call for additional setup data

 Am I missing something on uvc-gadget side or is the commit really bad?
 From what I understand, uvc-gadget is responsible for sending response to
 UVC class specific requests on control endpoint in uvc_send_response()
 in uvc_v4l2.c.

 So the reported commit is sending a duplicate response with probably
 improper data.
>>>
>>> Yes, this looks very dubious to me. I think it should be reverted. My 
>>> apologies for not having caught the patch during review.
>>
>> Hi,
>>
>> Now I've watched all codepaths again and yeah it is probably wrong patch, 
>> sorry.
>>
>> But if the code path is really:
>>
>> uvc_function_setup() -> userspace setup -> ioctl UVCIOC_SEND_RESPONSE -> 
>> uvc_send_response() -> usb_ep_queue() -> uvc_function_ep0_complete() -> 
>> userspace data
>>
>> it seems the USB timeouts with my hardware (PXA27x UDC) but with my patch it 
>> gets response immediately.
>>
> 
> I hope you were running uvc-gadget application on the PXA27x.
> 
> Just sending a response is not sufficient. It must send a response with 
> proper data.
> f_uvc itself doesn't know how to handle UVC class specific requests and has to
> depend on the user space application to populate the data in the response.
> 

Hi,

Yeah it was on the PXA27x, but that callback is not working there, so I was 
mistaken the bug is in UVC and not in PXA27x UDC itself.

Anyway I was using a newer (unofficial [1]) version of uvc-gadget, which 
already implements the most of the UVC requests.

BTW Did you try to use a configfs method for your usb webcam application? There 
was a discussion [2] about removing legacy UDC stuff (g_webcam and others g_*) 
in the future. If you used it, does it create these files for you?
streaming_interval
streaming_maxpacket
streaming_maxburst

P.S. Are you planning to use some generic linux friendly USB VID/PID? ;-)

[1] 
https://github.com/madscientist42/uvc-gadget/commit/e127ec3a3022e1090b3b741b48f661670f5dade2
[2] http://www.spinics.net/lists/linux-usb/msg152678.html

--
cheers,
Petr

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: usb: gadget: webcam broken?

[Roger Quadros]

Petr,

On 02/03/17 06:57, Petr Cvek wrote:
> Dne 2.3.2017 v 00:22 Laurent Pinchart napsal(a):
>> Hi Roger,
>>
>> On Wednesday 01 Mar 2017 17:09:51 Roger Quadros wrote:
>>> Hi,
>>>
>>> I'm no longer able to use g_webcam with uvc-gadget [1] since v4.9. Logs at
>>> the end. It looks like we're goofing up on the control endpoint.
>>>
>>> If I revert the following commit everything works fine.
>>> commit 4fbac5206afd01b717d4bdc58793d471f3391b4b
>>> Author: Petr Cvek 
>>> Date:   Wed Aug 17 12:36:57 2016 +0200
>>>
>>> usb: gadget: uvc: Add missing call for additional setup data
>>>
>>> Am I missing something on uvc-gadget side or is the commit really bad?
>>> From what I understand, uvc-gadget is responsible for sending response to
>>> UVC class specific requests on control endpoint in uvc_send_response()
>>> in uvc_v4l2.c.
>>>
>>> So the reported commit is sending a duplicate response with probably
>>> improper data.
>>
>> Yes, this looks very dubious to me. I think it should be reverted. My 
>> apologies for not having caught the patch during review.
> 
> Hi,
> 
> Now I've watched all codepaths again and yeah it is probably wrong patch, 
> sorry.
> 
> But if the code path is really:
> 
> uvc_function_setup() -> userspace setup -> ioctl UVCIOC_SEND_RESPONSE -> 
> uvc_send_response() -> usb_ep_queue() -> uvc_function_ep0_complete() -> 
> userspace data
> 
> it seems the USB timeouts with my hardware (PXA27x UDC) but with my patch it 
> gets response immediately.
> 

I hope you were running uvc-gadget application on the PXA27x.

Just sending a response is not sufficient. It must send a response with proper 
data.
f_uvc itself doesn't know how to handle UVC class specific requests and has to
depend on the user space application to populate the data in the response.

-- 
cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: usb: gadget: webcam broken?

[Petr Cvek]

Dne 2.3.2017 v 00:22 Laurent Pinchart napsal(a):
> Hi Roger,
> 
> On Wednesday 01 Mar 2017 17:09:51 Roger Quadros wrote:
>> Hi,
>>
>> I'm no longer able to use g_webcam with uvc-gadget [1] since v4.9. Logs at
>> the end. It looks like we're goofing up on the control endpoint.
>>
>> If I revert the following commit everything works fine.
>> commit 4fbac5206afd01b717d4bdc58793d471f3391b4b
>> Author: Petr Cvek 
>> Date:   Wed Aug 17 12:36:57 2016 +0200
>>
>> usb: gadget: uvc: Add missing call for additional setup data
>>
>> Am I missing something on uvc-gadget side or is the commit really bad?
>> From what I understand, uvc-gadget is responsible for sending response to
>> UVC class specific requests on control endpoint in uvc_send_response()
>> in uvc_v4l2.c.
>>
>> So the reported commit is sending a duplicate response with probably
>> improper data.
> 
> Yes, this looks very dubious to me. I think it should be reverted. My 
> apologies for not having caught the patch during review.

Hi,

Now I've watched all codepaths again and yeah it is probably wrong patch, sorry.

But if the code path is really:

uvc_function_setup() -> userspace setup -> ioctl UVCIOC_SEND_RESPONSE -> 
uvc_send_response() -> usb_ep_queue() -> uvc_function_ep0_complete() -> 
userspace data

it seems the USB timeouts with my hardware (PXA27x UDC) but with my patch it 
gets response immediately.

Petr

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: usb: gadget: webcam broken?

[Laurent Pinchart]

Hi Roger,

On Wednesday 01 Mar 2017 17:09:51 Roger Quadros wrote:
> Hi,
> 
> I'm no longer able to use g_webcam with uvc-gadget [1] since v4.9. Logs at
> the end. It looks like we're goofing up on the control endpoint.
> 
> If I revert the following commit everything works fine.
> commit 4fbac5206afd01b717d4bdc58793d471f3391b4b
> Author: Petr Cvek 
> Date:   Wed Aug 17 12:36:57 2016 +0200
> 
> usb: gadget: uvc: Add missing call for additional setup data
> 
> Am I missing something on uvc-gadget side or is the commit really bad?
> From what I understand, uvc-gadget is responsible for sending response to
> UVC class specific requests on control endpoint in uvc_send_response()
> in uvc_v4l2.c.
> 
> So the reported commit is sending a duplicate response with probably
> improper data.

Yes, this looks very dubious to me. I think it should be reverted. My 
apologies for not having caught the patch during review.

Would you like to send a revert patch ?

> [1] - git://git.ideasonboard.org/uvc-gadget.git

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html