Re: [PATCH] usb: dwc2: gadget: fix ISOC frame overflow handling

[Minas Harutyunyan]

Hi John,

On 12/21/2018 8:05 PM, John Keeping wrote:
> Hi Minas,
> 
> On Wed, 19 Dec 2018 14:09:01 +0000
> Minas Harutyunyan <minas.harutyunyan@xxxxxxxxxxxx> wrote:
> 
>> On 12/18/2018 6:35 PM, John Keeping wrote:
>>> Hi Minas,
>>>
>>> On Fri, 14 Dec 2018 09:00:08 +0000
>>> Minas Harutyunyan <minas.harutyunyan@xxxxxxxxxxxx> wrote:
>>>> First of all, sorry for delayed answer.
>>>> Looks like similar issue seen by Andrzej Pietrasiewicz
>>>> <andrzej.p@xxxxxxxxxxx>: "dwc2 isochronous transfers issues". Same
>>>> feedback provided to Andrzej.
>>>>
>>>> I run tests on 4.20.0-rc4 in DDMA. By default IN ISOC traffic
>>>> failed because of BNA interrupts. It's happen because UAC2
>>>> requests count set by default to 2. Our core and driver can't work
>>>> in DDMA with descriptor list length equal to 2. It's not possible
>>>> on time prepare next descriptors to avoid BNA interrupt.
>>>>
>>>> By changing UAC2_DEF_REQ_NUM to 4 all audio gadget tests passed
>>>> smoothly. Could you please apply this patch and run tests in DDMA
>>>> mode:
>>>>
>>>> diff --git a/drivers/usb/gadget/function/u_uac2.h
>>>> b/drivers/usb/gadget/function/u_uac2.h
>>>> index 8362ee572e1e..5e649259ab76 100644
>>>> --- a/drivers/usb/gadget/function/u_uac2.h
>>>> +++ b/drivers/usb/gadget/function/u_uac2.h
>>>> @@ -21,7 +21,7 @@
>>>>     #define UAC2_DEF_CCHMASK 0x3
>>>>     #define UAC2_DEF_CSRATE 64000
>>>>     #define UAC2_DEF_CSSIZE 2
>>>> -#define UAC2_DEF_REQ_NUM 2
>>>> +#define UAC2_DEF_REQ_NUM 4
>>>>
>>>>     struct f_uac2_opts {
>>>>            struct usb_function_instance    func_inst;
>>>>
>>>>
>>>> If it will OK on your side also then will switch to BDMA mode and
>>>> debug.
>>>
>>> With DDMA enabled, I see the following error after the stream has
>>> been running for a while (anything from a few seconds to a few
>>> minutes):
>>> -- >8 --
>>> [ 1798.836322] dwc2 ff580000.usb: dwc2_hsotg_ep_disable: called for
>>> ep0 [ 1798.836329] dwc2 ff580000.usb: dwc2_hsotg_ep_disable: called
>>> for ep0 [ 1798.851092] dwc2 ff580000.usb: new device is high-speed
>>> [ 1798.924461] dwc2 ff580000.usb: new device is high-speed
>>> [ 1798.982887] dwc2 ff580000.usb: new address 25
>>> [ 1799.002463] configfs-gadget gadget: high-speed config #1: config
>>> -- 8< --
>>>
>>> On the host side (Linux 4.18.16 Intel xHCI), I see this logged at
>>> the same time:
>>>    
>>> -- >8 --
>>> [1735740.716242] retire_capture_urb: usb 1-2.2.7: frame 0 active:
>>> -71 [1735740.716990] retire_capture_urb: usb 1-2.2.7: frame 0
>>> active: -71 [1735740.717906] retire_capture_urb: usb 1-2.2.7: frame
>>> 0 active: -71 [1735740.718905] retire_capture_urb: usb 1-2.2.7:
>>> frame 0 active: -71 [1735740.719916] retire_capture_urb: usb
>>> 1-2.2.7: frame 0 active: -71 [1735740.720032] usb 1-2.2-port7:
>>> disabled by hub (EMI?), re-enabling... [1735740.720036] usb
>>> 1-2.2.7: USB disconnect, device number 28 [1735740.937500] usb
>>> 1-2.2.7: new high-speed USB device number 29 using xhci_hcd -- 8< --
>>>
>>> The device does then enumerate and works for a period of time
>>> before the same error happens again.
>>>    
>>   From logs not clear who initiate disconnect: host or device.  More
>> probably host, after some errors in retire_capture_urb initiate
>> disconnect. Do you have any idea?
>> Can't you connect device to host on same platform?
>> If root cause of disconnect by host is device issue, i.e. not able to
>> send to host data packets in time then I need device side dmesg log
>> with debug enabled. USB trace around the disconnect will help to
>> debug.
> 
> I remember running a test with a hardware USB analyzer and which showed
> an isochronous packet with an incorrect length (much larger than it
> should have been) when the failure occurred.
> 
> I don't have any logs from that test and I'm out of the office at the
> moment, but I will capture logs when I'm back in January.

I think, if you enable debug prints, you will see BNA interrupts. So, my 
recommendation is to increase more UAC2_DEF_REQ_NUM until BNA will 
disappear. Per me value of UAC2_DEF_REQ_NUM is platform's latency dependent.

Thanks,
Minas

> 
> 
> Thanks,
> John
>