Re: bug? dwc2: insufficient fifo memory

[John Youn]

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.

Regards,
John