Re: [PATCH] usb: gadget: function: acm: return zlp for OUT setup

[Felipe Balbi <balbi@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 3220 bytes --]


Hi,

Jassi Brar <jassisinghbrar@gmail.com> writes:
> On Wed, Oct 14, 2015 at 9:18 PM, Felipe Balbi <balbi@ti.com> wrote:
>>
>> Hi,
>>
>> Jassi Brar <jassisinghbrar@gmail.com> writes:
>>> On Wed, Oct 14, 2015 at 8:42 PM, Felipe Balbi <balbi@ti.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> jaswinder.singh@linaro.org writes:
>>>>> From: Jassi Brar <jaswinder.singh@linaro.org>
>>>>>
>>>>> We must return 0 for any OUT setup request, otherwise
>>>>> protocol error may occur.
>>>>
>>>> have you seen any such errors ?
>>>>
>>> Yes. While testing my new udc driver.
>>>
>>>
>>>> Care to describe what problems you have seen and which UDC you were using,
>>>n> what's the exact scenario. How have you tested this ?
>>>>
>>> The udc I am writing the driver for is not yet public. To test my
>>> driver at lowest level possible, I use 'usbmon' to capture and analyze
>>> traffic since I don't have any hardware probe.
>>>
>>> I see the following on gadget side
>>> -------
>>> configfs-gadget gadget: non-core control req21.20 v0000 i0001 l7
>>> configfs-gadget gadget: acm ttyGS0 req21.20 v0000 i0001 l7
>>> configfs-gadget gadget: acm ttyGS0 completion, err -71
>>> -------
>>>
>>> and the following on host side usbmon capture
>>> -------
>>> ffff88041ed01f00 540296433 S Co:3:023:0 s 21 20 0000 0001 0007 7 =
>>> 80250000 000008
>>> ffff88041ed01f00 540296790 C Co:3:023:0 -71 0
>>> -------
>>
>> and you did you even consider this could be a bug in your new UDC driver
>> at all ? This works fine with other UDCs.
>>
> I was happy too until I decided to look closely at the annoying print
> on gadget side. This is a non-fatal error and visible only when
> debugging is enabled, so every udc seems 'fine'

I tried with my sniffer and saw no stalls, nothing. My setup request to
set line coding to 9600 baud worked just fine.

>> How far are you in developing this new UDC driver ?
>>
> Its done and tested for fsg+acm composite and each alone.

stress tests ? Meaning, did you leave it running for a couple of weeks
at least ?

>> Did you run USBCV at all ?
>>
> No USBCV not yet. I borrowed Windows machine to test FSG but forgot
> USBCV since it's been years I wrote last udc driver. Will give it a
> try tomorrow but I don't think it could emulate the sequence I hit
> with f_acm.

USBCV might already have some ACM test cases and it should exercise all
mandatory setup requests.

>> Are you sure you're implementing EP0 handling correctly ? What
>> sort of tests have you done with this UDC ? Is it working with testusb
>> against a known good host (EHCI and XHCI should be good for that) ?
>>
> Well as I said I have been looking at low level transfers and
> everything is good.

still, run testusb with the acompanying shell script and keep it running
for at least 2 weeks.

> BTW, should the gadget stack ever queue a Non-ZLP as reply to some
> setup request that has USB_DIR_IN not set?

What phase of the setup are we talking about ? If it has a DATA phase,
then data can have non-ZLP transfers and it can also require a ZLP to
end the data phase itself (if wLength % wMaxPacketSize == 0). Status
phase is always zlp.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]