Hi Pavel, Am 06.08.21 um 16:46 schrieb Pavel Hofman: > Hi Stefan, > > Dne 06. 08. 21 v 16:08 Stefan Wahren napsal(a): >> Hi Pavel, >> >> Am 06.08.21 um 15:03 schrieb Pavel Hofman: >>> Dne 28. 05. 21 v 10:59 Pavel Hofman napsal(a): >>>> Dne 27. 05. 21 v 15:47 Stefan Wahren napsal(a): >>>> >>>>>> I think I see the problem. >>>>>> >>>>>> IIUC the calculations and checks, all g-tx-fifo-size values + >>>>>> g-rx-fifo-size + g-np-tx-fifo-size must not exceed >>>>>> total_fifo_size. My >>>>>> RPi4 reports the total_fifo_size as 4080 (in >>>>>> /sys/kernel/debug/usb/fe980000.usb/hw_params). >>>>>> >>>>>> Linux mainline >>>>>> https://github.com/torvalds/linux/search?p=3&q=g-tx-fifo-size : >>>>>> >>>>>> The increase in value of g-rx-fifo-size exceeds the limit for the >>>>>> DTSI >>>>>> files we patched: >>>>>> >>>>>> Both bcm283x-rpi-usb-peripheral.dtsi and bcm283x-rpi-usb-otg.dtsi: >>>>>> 558 + 32 + 256 + 256 + 512 + 512 + 512 + 768 + 768 = 4174 > 4080 >>>>>> >>>>>> while the sum with the previous value of 256 reached just 3872 < >>>>>> 4080. >>>>>> >>>>>> >>>>>> The raspberrypi repo >>>>>> https://github.com/raspberrypi/linux/search?q=g-tx-fifo-size : >>>>>> >>>>>> It has a different mix of the DTSI files >>>>>> dwc2-overlay.dts >>>>>> upstream-overlay.dts >>>>>> upstream-pi4-overlay.dts >>>>> yes these overlay files are vendor specific and doesn't exist in >>>>> mainline. The upstream*dts were intended to "simulate" mainline >>>>> behavior, but unfortunately differ in this case. >>>>>> >>>>>> all of which define >>>>>> g-tx-fifo-size = <512 512 512 512 512 256 256>; >>>>>> >>>>>> Here the calculation holds: >>>>>> 558 + 32 + 512 + 512 + 512 + 512 + 512 + 256 + 256 = 3662 < 4080 >>>>>> >>>>>> My RPi4 uses one of these DTSIs, because my >>>>>> /sys/kernel/debug/usb/fe980000.usb/params says: >>>>>> >>>>>> g_rx_fifo_size : 558 >>>>>> g_np_tx_fifo_size : 32 >>>>>> g_tx_fifo_size[0] : 0 >>>>>> g_tx_fifo_size[1] : 512 >>>>>> g_tx_fifo_size[2] : 512 >>>>>> g_tx_fifo_size[3] : 512 >>>>>> g_tx_fifo_size[4] : 512 >>>>>> g_tx_fifo_size[5] : 512 >>>>>> g_tx_fifo_size[6] : 256 >>>>>> g_tx_fifo_size[7] : 256 >>>>>> >>>>>> >>>>>> IIUC the tx_fifo values in bcm283x-rpi-usb-peripheral.dtsi and >>>>>> bcm283x-rpi-usb-otg.dtsi files can be lowered to the values used and >>>>>> tested (at least by me) in the RPi repo. But this is outside of my >>>>>> knowledge, honestly I do not know what is the most appropriate >>>>>> distribution of the remaining fifo space among the g_tx_fifo >>>>>> buffers. >>>>>> Please can the RPi developers (Phil?) suggest a fix? >>>>> >>>>> As author of the mainline bcm283x-rpi-usb-otg.dtsi i was trying to >>>>> optimize the fifo sizes for EP 6 and 7. But i don't remember why. >>>>> So my >>>>> suggestion for a fix would be: >>>>> >>>>> g-tx-fifo-size = <256 256 256 512 512 768 768>; >>>>> >>>>> But i'm also unsure about the values. >>>>> >>>> >>>> IIUC this code >>>> https://github.com/torvalds/linux/blob/master/drivers/usb/dwc2/gadget.c#L4091 >>>> >>>> >>>> optimizes the FIFO assignment to endpoints. From that I would conclude >>>> that correct values are specific for each use-case configuration of >>>> endpoints. Maybe a varied selection (256, 512, 768) is more convenient >>>> than just 256 and 512. I really do not know what use cases need >>>> what TX >>>> fifo values. >>>> >>> >>> My patch raising g-rx-fifo-size = 558 has been reverted back to >>> g-rx-fifo-size = 256 in upstream. 256 is clearly a wrong value. 558 is >>> enough for 2 packets per microframe. How about raising the value in >>> the mainline DTS files to 301 instead which will correctly work with 1 >>> packet per microframe (the most common scenario) and comply with the >>> 4080 limit of the RX + all TXs sum of the TX configs in the mainline? >> >> thank for your feedback. It has been reverted because the last patch >> broke USB completely on Raspberry Pi Zero and the only explanation for >> me is it has never been tested. The workflow is that the submitter is >> responsibly for testing. In case this is not possible the patch must be >> tagged with RFT or at least it must be mentioned in the commit message. >> >> In case you want to have a different value, please feel free to send a >> patch, but please test it against a mainline kernel before. In case you >> have problems with it, feel free to ask. >> >> Sorry, in case this sounds grumpy but it's very annoying to hunt down >> especially avoidable regressions with every kernel release. This wastes >> other developers time to get their patches upstream. >> > > I understand your points. I really did not test the patch with > mainline combination of the TX values, sorry for that. I have no > problem with the revert at all, just pointing out that the value of > 256 is incorrect. It took a number of hours with patient help of Minas > to find the culprit of the dwc2 gadget dropping audio samples with > packet sizes over 980 bytes. believe me, i understand this absolutely as the author of the mainline Raspberry Pi Zero DTS (back in 2017). In the old days there were a lot of issues in the DWC2. It took until ~ 4.14 to get a proper working USB host mode. > > However, even if I did test and changed the TX values on my RPi4 > accordingly, I would not have been able to test on RPi Zero and the > other RPi models. This doesn't matter. The USB IP is always the same. The mentioned issue was also on the Raspberry Pi 4, but nobody notices this (using Raspberry Pi 4 as USB gadget is very special). But for the RPI Zero this issue was very critical. > The questions are: > > * Why did your TX values, changed to comply with the 4080 limit, break > RPi Zero, what are the rules apart of the max sum of 4080? Unfortunately i don't have access to the DWC2 reference manual and the time to figure out all these constrains. > > * Why does mainline config have different RX and TX sizes than the > RPi-vendor specific config (which I happen/ed to use)? For my initial version of the DTS i took some working values, i don't remember exactly. During time the values in the vendor tree changed. Nobody upstreamed the later changes. I'm fine with changing all to RPi-vendor specific settings, as long as it works with OTG, Gadget mode, with and without USB hub, ... I don't have a strong opinion for these magic numbers. Currently i'm busy in my spare time with CM4 upstreaming, so not much time for this topic. I hope this helps. Best regards Stefan > > Maybe these questions should be resolved, allowing for safer > developing the gadget on the RPi hardware. > > Best regards, > > Pavel.
