[LEDE-DEV] [PATCH] lantiq: update USB controller handling

[Antti Seppälä]

On 7 January 2017 at 04:01, Hauke Mehrtens <hauke at hauke-m.de> wrote:
> On 01/06/2017 11:31 PM, Antti Seppälä wrote:
>> On 6 January 2017 at 21:32, Hauke Mehrtens <hauke at hauke-m.de> wrote:
>>> On 01/06/2017 08:26 PM, Martin Blumenstingl wrote:
>>>> Hi Hauke,
>>>>
>>>> (CC'ing Mathias as he was looking into some of the USB issues as well)
>>>>
>>>> On Fri, Jan 6, 2017 at 8:06 PM, Hauke Mehrtens <hauke at hauke-m.de> wrote:
>>>>> This makes the code use the same settings aas the vendor sdrivers.
>>>> s/aas/as/
>>>> s/sdrivers/driver/
>>>> you should probably also mention that this:
>>>> 1. backports USB initialization fixes for XRX300, Danube and Amazon-SE
>>>> 2. sets the clock source for all SoCs which fixes USB (initialization)
>>>> issues on Danube when using the dwc2 driver (thus fixes FS#351 and
>>>> supersedes Mathias' "lantiq: set the usb clock source" RFC patch)
>>>
>>> Yes after I send the patch I noticed that I haven't updated the commit
>>> message. I will fix that.
>>>
>>> Could someone please test this patch and report back if it fixes the
>>> problems on Danube and works on Amazon SE or improves something in VR9?
>>> Positive and negative reports would be nice. I have only tested it on
>>> VR9 till now, I have to look what other boards I have here.
>>>
>>
>> Hi Hauke.
>
> Hi Antti,
>
> I am also not an USB or DWC2 expert looked into this more or less the
> first time some days ago.
>
>> The patch looks good except for the choices of fifo sizes from vendor
>> driver which from the point-of-view of usb packet sizes are quite odd
>> will break at least certain audio devices.
>
> Probably the vendor driver is optimized for storage or mobile data
> devices which is the most common use case for USB on this SoC.
>
>> If I recall correctly the dwc2 specification (Method 1) proposes
>> values for fifo size at minimum of:
>>
>> rx fifo size: 258 + number of host channels
>> non-periodic tx fifo size: 128
>> periodic tx fifo size: 768
>>
>> The spec Method 2 which is used by the upstream driver autodetection
>> logic[1] doubles the rx and nptx sizes to try to fit two packets at
>> once for greater performance.
>>
>> Especially the periodic fifo size choice of 32 for vr9 is way too
>> small to accommodate a complete usb packet.
>> We used to have the vendor driver values in use for vr9 but there were
>> some complaints on openwrt forum that usb audio devices did not work.
>>
>> Changing the values to closer resemble the dwc2 specification actually
>> allows audio devices to work for some people [2].
>
> Does a Fifo size of 96 work for most audio applications? If that is true
> we should probably use that.
>

Apparently it does work, at least there have not been further
bug-reports that I know of. We've been using the new fifo values for
over a year now.

>> Unfortunately the default total fifo size is 512 which can never
>> fulfil the dwc2 specification completely.
>
> The reset value is the biggest possible value which was configured in
> the coreConsultant configuration which is probably the step when the
> hardware core gets configured by the SoC designer.
>
> So my understanding is that the controller first copies the data
> received from USB into this RAM and then does a DMA transfer into main
> memory, for transmit this is going in the other direction.
>
>> Is there a way to programmatically re-allocate a bigger fifo for the
>> lantiq platform during initialization? If there is we could let the
>> dwc2 driver autoconfigure the fifos as it sees fit.
>
> I think the RAM is internal in the USB controller and not an external
> RAM or an area of main memory. The dwc2 driver says on xrx200 we have
> total_fifo_size=552. My documentation says that we have 2.5 KBytes in
> xrx200 and 8 KBytes on Danube. There are no lantiq specific registers
> for this FIFOs only the dwc2 common ones.
>

Too bad but it makes sense. It's strange that the size was actually
reduced in newer SoCs but I guess there's nothing we can do but try to
cope with it.

> On xrx200 we have 552 fifo positions to use, I think we should go back
> to your values and add the extra 40 fifo positions somewhere.
>

I think on ar9 the driver reports a total fifo size of 512 so I did
the parameters with that upper limit in mind. If we want to create a
separate xrx200 configuration then I'd probably use the extra 40
positions to further increase the periodic tx fifo.

Upstream driver is working to deprecate the built-in parameters in
favor of devicetree properties which I guess would be the best way to
add new configurations.

>> Also the dwc2 driver apparently does a pretty good job at
>> autoconfiguring various other parameters for optimum performance so I
>> wonder whether we should rely on that instead of hard-coding them?
>
> Is there any autoconfig code for the fifo sizes? I only saw the code in
> dwc2_calculate_dynamic_fifo() which will not work with our restricted
> memory.
>

No that's the only one but I was actually referring to other
parameters in dwc2_core_params such as dma_desc_enable, speed,
phy_type and phy_utmi_width.

>> A disclaimer though: I'm not an usb expert. I only did the best I
>> could with the fifo values based on my testing and reading the dwc2
>> spec :)
>
> Do you have a dwc2 spec, I only have only the description of some, not
> all registers.
>

Sadly not anymore but I think you can request it from Synopsys...

>> 1. https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/dwc2/core.c?h=v4.4#n892
>> 2. https://forum.openwrt.org/viewtopic.php?pid=299862#p299862
>>
>

-- 
Antti