2017-06-05 12:32:26

by Minas Harutyunyan

[permalink] [raw]
Subject: Re: bug? dwc2: insufficient fifo memory

On 6/2/2017 10:20 PM, John Stultz wrote:
> On Mon, Apr 17, 2017 at 3:36 PM, John Stultz <[email protected]> wrote:
>> On Fri, Feb 24, 2017 at 2:46 PM, John Stultz <[email protected]> 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