Re: bug? dwc2: insufficient fifo memory

[John Stultz]

On Mon, Jun 5, 2017 at 10:50 AM, John Youn <John.Youn@xxxxxxxxxxxx> wrote:
> On 6/5/2017 5:32 AM, Minas Harutyunyan wrote:
>> On 6/2/2017 10:20 PM, John Stultz wrote:
>>> On Mon, Apr 17, 2017 at 3:36 PM, John Stultz <john.stultz@xxxxxxxxxx> wrote:
>>>> On Fri, Feb 24, 2017 at 2:46 PM, John Stultz <john.stultz@xxxxxxxxxx> wrote:
>>>>> Hey John,
>>>>>   So after the USB tree landed in 4.11-rc, I've been seeing the
>>>>> following warning at bootup.
>>>>>
>>>>> It seems the fifo_mem/total_fifo_size value on hikey is 1920, but I'm
>>>>> seeing the addresses zip upward quickly as the txfsz values are all
>>>>> 2048.  Not exactly sure whats wrong here.  Things still seem to work
>>>>> properly.
>>>>>
>>>>> thanks
>>>>> -john
>>>>>
>>>>>
>>>>> [    8.944987] dwc2 f72c0000.usb: bound driver configfs-gadget
>>>>> [    8.956651] insufficient fifo memory
>>>>> [    8.956703] ------------[ cut here ]------------
>>>>> [    8.964906] WARNING: CPU: 7 PID: 1 at drivers/usb/dwc2/gadget.c:330
>>>>> dwc2_hsotg_init_fifo+0x1a8/0x1c8
>>>>
>>>>
>>>> Hey John,
>>>>    So I finally got a bit of time to look deeper into this, and it
>>>> seems like this issue was introduced by commit 3c6aea7344c3 ("usb:
>>>> dwc2: gadget: Add checking for g-tx-fifo-size parameter"), as that
>>>> change added the following snippit:
>>>>
>>>>                if (hsotg->params.g_tx_fifo_size[fifo] < min ||
>>>>                    hsotg->params.g_tx_fifo_size[fifo] >  dptxfszn) {
>>>>                        dev_warn(hsotg->dev, "%s: Invalid parameter
>>>> g_tx_fifo_size[%d]=%d\n",
>>>>                                 __func__, fifo,
>>>>                                 hsotg->params.g_tx_fifo_size[fifo]);
>>>>                        hsotg->params.g_tx_fifo_size[fifo] = dptxfszn;
>>>>                }
>>>>
>>>> On HiKey, we have g-tx-fifo-size = <128 128 128 128 128 128> in the
>>>> dtsi, and the fifo_mem value ends up initialized at 1920.
>>>>
>>>> Unfortunately, in the above, it sees other entries in the
>>>> g_tx_fifo_size[] array are initialized to zero, which then causes them
>>>> to be set to the "default" value of dptxfszn which is 2048.  So then
>>>> later in dwc2_hsotg_init_fifo() the addr value (which adds the
>>>> fifo_size array value each step) quickly grows beyond the fifo_mem
>>>> value, causing the warning.
>>>>
>>>> Not sure what the right fix is here? Should the min value be used
>>>> instead of the "default" dptxfszn value?  Again, I'm not sure I see
>>>> any side-effects from this warning, but wanted to try to figure out
>>>> how to fix it properly.
>>>
>>> Hey John, Minas,
>>>   Wanted to follow up on this again.  I'm using a patch that looks as
>>> follows (sorry for the copy-paste whitespace damage) in the meantime
>>> to work around this issue:
>>>
>>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
>>> index 9cd8722..13a911b 100644
>>> --- a/drivers/usb/dwc2/params.c
>>> +++ b/drivers/usb/dwc2/params.c
>>> @@ -475,7 +475,7 @@ static void dwc2_check_param_tx_fifo_sizes(struct
>>> dwc2_hsotg *hsotg)
>>>                         dev_warn(hsotg->dev, "%s: Invalid parameter
>>> g_tx_fifo_size[%d]=%d\n",
>>>                                  __func__, fifo,
>>>                                  hsotg->params.g_tx_fifo_size[fifo]);
>>> -                       hsotg->params.g_tx_fifo_size[fifo] = dptxfszn;
>>> +                       hsotg->params.g_tx_fifo_size[fifo] = min;
>>>                 }
>>>         }
>>>  }
>>>
>>> Any suggestions on what a proper fix would look like?
>>>
>>> thanks
>>> -john
>>>
>>
>> Hi John,
>>
>> The patch series: "[PATCH 0/4] usb: dwc2: gadget: Fix dynamic FIFO
>> initialization flow" from Sevak Arakelyan submitted to LKML at 4/26/2017
>> should resolve this issue.
>>
>> Thanks,
>> Minas
>>
>
> Hi John,
>
> See if this patch helps and let us know.
>
> Unfortunately, the author of this patch (and expert on this part of
> the code) has left Synopsys. So it might take us a bit to look into
> this.


Hey Folks,
   I realized I didn't get back to you, as I got pulled out to other
work. I was testing with 4.13-rc, and noticed the same issue, and
figured I should get back to you.

I've applied the 4 patches from the patch series you've pointed me to.
Though with this patchset I still hit the same warning, but this time
from dwc2_hsotg_core_init_disconnected() rather then
dwc2_hsotg_init_fifo().


[    8.016067] init: processing action (sys.usb.config=adb &&
sys.usb.configfs=1 && sys.usb.ffs.ready=1) from
(/init.usb.configfs.rc:21)
[    8.029367] dwc2 f72c0000.usb: bound driver configfs-gadget
[    8.035140] ------------[ cut here ]------------
[    8.043357] WARNING: CPU: 7 PID: 1 at drivers/usb/dwc2/gadget.c:317
dwc2_hsotg_core_init_disconnected+0x52c/0x560
[    8.053643] CPU: 7 PID: 1 Comm: init Not tainted
4.13.0-rc1-00038-ga257c7e #3331
[    8.061042] Hardware name: HiKey Development Board (DT)
[    8.066285] task: ffffffc005f68000 task.stack: ffffffc005f70000
[    8.072230] PC is at dwc2_hsotg_core_init_disconnected+0x52c/0x560
[    8.078415] LR is at dwc2_hsotg_core_init_disconnected+0x52c/0x560
[    8.084593] pc : [<ffffff80086a65ac>] lr : [<ffffff80086a65ac>]
pstate: 600001c5
[    8.091987] sp : ffffffc005f73be0
[    8.095298] x29: ffffffc005f73be0 x28: ffffff8008dc9a18
[    8.100629] x27: ffffffc074d1907c x26: 000000000000000f
[    8.105949] x25: 0000000000000007 x24: 000000000000011c
[    8.111263] x23: 0000000000000580 x22: 0000000000000000
[    8.116577] x21: 0000000000000007 x20: 0000000008000580
[    8.121902] x19: ffffffc074d19018 x18: 0000000000000010
[    8.127229] x17: aaaaaaaaaaaaaaab x16: ffffff80081da680
[    8.132548] x15: ffffff8088e5b3a7 x14: 0000000000000006
[    8.137863] x13: ffffff8008e5b3b5 x12: ffffff8008d89118
[    8.143176] x11: 0000000000000000 x10: ffffffc005f73be0
[    8.148493] x9 : ffffffc005f73be0 x8 : 79726f6d656d206f
[    8.153807] x7 : 66696620746e6569 x6 : 0000000000000340
[    8.159121] x5 : 0000000000000015 x4 : 0000000000000000
[    8.164439] x3 : 0000000000000002 x2 : 0000000000000002
[    8.169769] x1 : 0000000000000001 x0 : 0000000000000018
[    8.175099] Call trace:
[    8.177552] Exception stack(0xffffffc005f73a10 to 0xffffffc005f73b40)
[    8.183990] 3a00:
ffffffc074d19018 0000008000000000
[    8.191829] 3a20: ffffffc005f73be0 ffffff80086a65ac
0000000000000007 000000000000000f
[    8.199664] 3a40: ffffffc074d1907c ffffff8008dc9a18
ffffffc005f73a90 0000000000000000
[    8.207512] 3a60: ffffffc005f73be0 ffffffc005f73be0
ffffffc005f73ba0 00000000ffffffc8
[    8.215362] 3a80: ffffffc005f73ab0 ffffff80081057a4
ffffffc005f73be0 ffffffc005f73be0
[    8.223210] 3aa0: ffffffc005f73ba0 00000000ffffffc8
0000000000000018 0000000000000001
[    8.231065] 3ac0: 0000000000000002 0000000000000002
0000000000000000 0000000000000015
[    8.238919] 3ae0: 0000000000000340 66696620746e6569
79726f6d656d206f ffffffc005f73be0
[    8.246774] 3b00: ffffffc005f73be0 0000000000000000
ffffff8008d89118 ffffff8008e5b3b5
[    8.254627] 3b20: 0000000000000006 ffffff8088e5b3a7
ffffff80081da680 aaaaaaaaaaaaaaab
[    8.262487] [<ffffff80086a65ac>]
dwc2_hsotg_core_init_disconnected+0x52c/0x560
[    8.269740] [<ffffff80086a7038>] dwc2_hsotg_pullup+0x90/0x100
[    8.275495] [<ffffff80086cb2a0>] usb_udc_connect_control.isra.0+0x78/0x90
[    8.282308] [<ffffff80086cb3a4>] udc_bind_to_driver+0xec/0x110
[    8.288166] [<ffffff80086cc308>] usb_gadget_probe_driver+0xa0/0x150
[    8.294459] [<ffffff80086ca73c>] gadget_dev_desc_UDC_store+0xe4/0x100
[    8.300931] [<ffffff8008258af8>] configfs_write_file+0xc0/0x198
[    8.306881] [<ffffff80081da234>] __vfs_write+0x1c/0x118
[    8.312133] [<ffffff80081da4d0>] vfs_write+0xa0/0x1b0
[    8.317201] [<ffffff80081da6c4>] SyS_write+0x44/0xa0
[    8.322193] [<ffffff8008082f30>] el0_svc_naked+0x24/0x28
[    8.327508] ---[ end trace c6da4c029bccb75e ]---


The same hack I was using earlier
-                       hsotg->params.g_tx_fifo_size[fifo] = dptxfszn;
+                       hsotg->params.g_tx_fifo_size[fifo] = min;

still avoids the issue.

thanks
-john