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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 14, 2015 at 10:40 PM, Felipe Balbi <balbi@xxxxxx> wrote:
>
> Hi,
>
> Jassi Brar <jassisinghbrar@xxxxxxxxx> writes:
>> On Wed, Oct 14, 2015 at 9:18 PM, Felipe Balbi <balbi@xxxxxx> wrote:
>>>
>>> Hi,
>>>
>>> Jassi Brar <jassisinghbrar@xxxxxxxxx> writes:
>>>> On Wed, Oct 14, 2015 at 8:42 PM, Felipe Balbi <balbi@xxxxxx> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> jaswinder.singh@xxxxxxxxxx writes:
>>>>>> From: Jassi Brar <jaswinder.singh@xxxxxxxxxx>
>>>>>>
>>>>>> 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.
>
I am sure your udc driver will (need to) track stages of a transfer.
If you put some print in usb_ep_ops.queue() you will notice the stack
queues 7byte transfer but the udc driver silently drops it and send a
zlp here.

  I can move the change into my driver as well, but the point is
gadget should never try to _send_ non-zlp as reply to control-OUT
command.


>>> 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 ?
>
I know I can't stress test enough, but this bug is 100% hit and
staring in the eye at protocol level.

>> 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 ?
>
I said "_reply_ to setup request" so I meant status phase.

UDC driver receives the SETUP command as '21 20 00 00 01 00 07 00',
passes it onto composite, which hands it over to acm and which
pretends we need to return a 7byte packet to host (value = w_length =
7).
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]