On 7 January 2017 at 04:01, Hauke Mehrtens <ha...@hauke-m.de> wrote: > On 01/06/2017 11:31 PM, Antti Seppälä wrote: >> On 6 January 2017 at 21:32, Hauke Mehrtens <ha...@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 <ha...@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 _______________________________________________ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
