There are in sum 952 dwords available for g-rx-fifo-size,
g-np-tx-fifo-size and the eight entries of g-tx-fifo-size. For high
speed endpoints the maximal packet size is 512 (for full speed it's 64)
bytes. So a tx-fifo-size of more than 128 (dwords) isn't sensible.
So instead of one (too) big and several small fifos, use two big fifos
and to better use the remaining available space increase one of the
small fifos.
This allows to work with CONFIG_USB_CDC_COMPOSITE (i.e. Ethernet and
ACM) which requires 4 endpoints with fifo sizes 512, 512, 16 and 10
respectively.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
---
Hello,
with CONFIG_USB_CDC_COMPOSITE enabled on the old device tree, the driver
dies in a rather bad way:
[ 2.472914] dwc2 49000000.usb-otg: dwc2_hsotg_ep_enable: No suitable fifo found
[ 2.478767] ------------[ cut here ]------------
[ 2.483369] WARNING: CPU: 0 PID: 0 at kernel/dma/mapping.c:532 dma_free_attrs+0xc8/0xcc
[ 2.491363] Modules linked in:
[ 2.494407] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.15.0-20221026-1 #1
[ 2.501267] Hardware name: STM32 (Device Tree Support)
[ 2.506409] [<c01110e8>] (unwind_backtrace) from [<c010c9c0>] (show_stack+0x18/0x1c)
[ 2.514129] [<c010c9c0>] (show_stack) from [<c0a83648>] (dump_stack_lvl+0x40/0x4c)
[ 2.521689] [<c0a83648>] (dump_stack_lvl) from [<c0136228>] (__warn+0xf4/0x150)
[ 2.528986] [<c0136228>] (__warn) from [<c0a7fe2c>] (warn_slowpath_fmt+0x6c/0xd0)
[ 2.536458] [<c0a7fe2c>] (warn_slowpath_fmt) from [<c01ad534>] (dma_free_attrs+0xc8/0xcc)
[ 2.544623] [<c01ad534>] (dma_free_attrs) from [<c01adbc0>] (dmam_free_coherent+0x40/0x9c)
[ 2.552876] [<c01adbc0>] (dmam_free_coherent) from [<c0754570>] (dwc2_hsotg_ep_enable+0x63c/0x6b0)
[ 2.561827] [<c0754570>] (dwc2_hsotg_ep_enable) from [<c0791a44>] (usb_ep_enable+0x40/0xf0)
[ 2.570167] [<c0791a44>] (usb_ep_enable) from [<c0798364>] (gether_connect+0x2c/0x1c0)
[ 2.578073] [<c0798364>] (gether_connect) from [<c0799f70>] (ecm_set_alt+0xcc/0x1f8)
[ 2.585805] [<c0799f70>] (ecm_set_alt) from [<c078d0ec>] (composite_setup+0x5bc/0x1d40)
[ 2.593799] [<c078d0ec>] (composite_setup) from [<c07568f0>] (dwc2_hsotg_complete_setup+0x16c/0x68c)
[ 2.602921] [<c07568f0>] (dwc2_hsotg_complete_setup) from [<c0755474>] (dwc2_hsotg_complete_request+0x9c/0x210)
[ 2.612999] [<c0755474>] (dwc2_hsotg_complete_request) from [<c0757d68>] (dwc2_hsotg_epint+0xe0c/0x1248)
[ 2.622470] [<c0757d68>] (dwc2_hsotg_epint) from [<c075a1a4>] (dwc2_hsotg_irq+0x9c4/0x10a4)
[ 2.630812] [<c075a1a4>] (dwc2_hsotg_irq) from [<c0194238>] (__handle_irq_event_percpu+0x64/0x234)
[ 2.639762] [<c0194238>] (__handle_irq_event_percpu) from [<c01944f0>] (handle_irq_event+0x64/0xc8)
[ 2.648798] [<c01944f0>] (handle_irq_event) from [<c0199028>] (handle_fasteoi_irq+0xbc/0x214)
[ 2.657312] [<c0199028>] (handle_fasteoi_irq) from [<c0193a9c>] (handle_domain_irq+0x84/0xb8)
[ 2.665827] [<c0193a9c>] (handle_domain_irq) from [<c05628b0>] (gic_handle_irq+0x84/0x98)
[ 2.673995] [<c05628b0>] (gic_handle_irq) from [<c0100afc>] (__irq_svc+0x5c/0x90)
[ 2.681466] Exception stack(0xc1001ef8 to 0xc1001f40)
[ 2.686509] 1ee0: 00000484 c0d6f994
[ 2.694680] 1f00: 00000000 c011afe0 c10f5ae0 00000000 ffffe000 c1004f54 00000000 00000000
[ 2.702848] 1f20: c1000000 c0e11af0 c1000000 c1001f48 c01091ec c01091f0 60000013 ffffffff
[ 2.711008] [<c0100afc>] (__irq_svc) from [<c01091f0>] (arch_cpu_idle+0x40/0x44)
[ 2.718393] [<c01091f0>] (arch_cpu_idle) from [<c0a91f40>] (default_idle_call+0x4c/0x168)
[ 2.726561] [<c0a91f40>] (default_idle_call) from [<c016f054>] (do_idle+0x23c/0x290)
[ 2.734294] [<c016f054>] (do_idle) from [<c016f3fc>] (cpu_startup_entry+0x20/0x24)
[ 2.741852] [<c016f3fc>] (cpu_startup_entry) from [<c0f01040>] (start_kernel+0x5e8/0x634)
[ 2.750020] [<c0f01040>] (start_kernel) from [<00000000>] (0x0)
[ 2.755929] ---[ end trace febb0e7bfc3d83c0 ]---
so there might be another change required to fail in a nicer way.
(That's the WARN_ON(irqs_disabled()) in dma_free_attrs() that triggers
here.)
Another thought I had while tuning the tx fifo sizes was: Why is the
size allocation not (more) done dynamically? At least only setting a
fixed amount of dwords aside for g-tx-fifo-size and allocate from that
shouldn't be too hard, should it?
Note I know very little about USB, so it might well be possible that I
missed a use case, but with this change my USB gadget works as expected.
Best regards
Uwe
arch/arm/boot/dts/stm32mp151.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/stm32mp151.dtsi b/arch/arm/boot/dts/stm32mp151.dtsi
index 5491b6c4dec2..af70ca1f9b57 100644
--- a/arch/arm/boot/dts/stm32mp151.dtsi
+++ b/arch/arm/boot/dts/stm32mp151.dtsi
@@ -1137,7 +1137,7 @@ usbotg_hs: usb-otg@49000000 {
interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
g-rx-fifo-size = <512>;
g-np-tx-fifo-size = <32>;
- g-tx-fifo-size = <256 16 16 16 16 16 16 16>;
+ g-tx-fifo-size = <128 128 64 16 16 16 16 16>;
dr_mode = "otg";
otg-rev = <0x200>;
usb33d-supply = <&usb33>;
--
2.39.0
