[PATCH] ARM: dts: stm32mp15x: adjust USB OTG gadget tx fifo sizes

[PATCH] ARM: dts: stm32mp15x: adjust USB OTG gadget tx fifo sizes

[Uwe Kleine-König]

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@pengutronix.de>
---
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

Re: [Linux-stm32] [PATCH] ARM: dts: stm32mp15x: adjust USB OTG gadget tx fifo sizes

[Fabrice Gasnier]

On 1/12/23 12:20, Uwe Kleine-König wrote:
> 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.

Hi Uwe,

The current FIFO tuning in the device tree allows to maximize throughput
regarding single function device. Having twice the packet size for the
endpoint allows the controller to simultaneously transfer data to the
USB, while gathering next data from the system memory.

> 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.

So, I wouldn't mention "too" big here. That's a performance tuning for
single function device use case.

Drawback is this doesn't allow multi-function device, as you're trying
to achieve (with 2 x 512 endpoints).
Hence, the "No suitable fifo found" message you've noticed.

So just on the wording, I'd rather talk about allowing multi function
(composite) device with 512 bytes endpoints. Doing this has an impact on
the performance for all current users in terms of performance.

This change is probably fine, as it enables one additional use case, and
it is in the SOC dtsi file.
I'm just wondering a bit if we could/should keep current tuning in some
board dts files ? (Or let each board vendor do their own tuning ?)

Perhaps you could CC people that pushed various boards here ?

> 
> 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.

Just a note, this one looks like a legacy driver. I think the preferred
method is to use configfs gadget to compose a device.

> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> 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.)

Indeed, probably all dwc2 users could be affected by this (not only
stm32). IMHO, This could be reported to the USB mailing list.

> 
> 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?

Same, better place could be to suggest this directly on the USB ML (with
Minas in CC).

> 
> 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.

See my earlier comment on the use case. It's probably a good idea to
update the gadget FIFO size to enable composite device with two
functions (w/512 bytes EP).
Could you update the commit message with these updates ?

Not sure thought, if perf penalty should be handled (and how) for single
function device use case, possibly affecting all current users.

Best Regard,
Fabrice

> 
> 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>;

Re: [Linux-stm32] [PATCH] ARM: dts: stm32mp15x: adjust USB OTG gadget tx fifo sizes

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 8810 bytes --]

[Cc += linux-usb, so I keep a bit more context than I'd normally do]

Hello Fabrice,

I stumbled about this old thread while cleaning up my mailbox. I think
this topic is still open, at least stm32mp151.dtsi still uses the
settings I changed with my patch.

On Fri, Jan 20, 2023 at 10:18:32AM +0100, Fabrice Gasnier wrote:
> On 1/12/23 12:20, Uwe Kleine-König wrote:
> > 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.
> 
> The current FIFO tuning in the device tree allows to maximize throughput
> regarding single function device.

Are you sure here? I don't claim to be an expert for usb and/or
stm32mp1, but in my understanding an allocation of 256 dwords (= 1024
bytes) will only be used in the first half when a driver requests 512
bytes. If so, it would complicate to implement the idea to dynamically
allocate the fifo chunks (sketched below).

> Having twice the packet size for the endpoint allows the controller to
> simultaneously transfer data to the USB, while gathering next data
> from the system memory.
> 
> > 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.
> 
> So, I wouldn't mention "too" big here. That's a performance tuning for
> single function device use case.
> 
> Drawback is this doesn't allow multi-function device, as you're trying
> to achieve (with 2 x 512 endpoints).
> Hence, the "No suitable fifo found" message you've noticed.
> 
> So just on the wording, I'd rather talk about allowing multi function
> (composite) device with 512 bytes endpoints. Doing this has an impact on
> the performance for all current users in terms of performance.
> 
> This change is probably fine, as it enables one additional use case, and
> it is in the SOC dtsi file.
> I'm just wondering a bit if we could/should keep current tuning in some
> board dts files ? (Or let each board vendor do their own tuning ?)
> 
> Perhaps you could CC people that pushed various boards here ?

Maybe it would make sense to overwrite the setting for the existing
boards such that the boards are not affected by the change. I will
consider this if and when I prepare a v2.
 
> > 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.
> 
> Just a note, this one looks like a legacy driver. I think the preferred
> method is to use configfs gadget to compose a device.

The nice thing about CONFIG_USB_CDC_COMPOSITE is that it works without
userspace, e.g. for nfsroot.
 
> > 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.)
> 
> Indeed, probably all dwc2 users could be affected by this (not only
> stm32). IMHO, This could be reported to the USB mailing list.

Indeed. I hope someone picks up this topic.
 
> > 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?
> 
> Same, better place could be to suggest this directly on the USB ML (with
> Minas in CC).

Minas was already on Cc for the initial mail. I wonder if other host
cores also need this explicit fifo size allocation. Then maybe there is
some place to copy a dynamic allocation from?

> > 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.
> 
> See my earlier comment on the use case. It's probably a good idea to
> update the gadget FIFO size to enable composite device with two
> functions (w/512 bytes EP).
> Could you update the commit message with these updates ?
> 
> Not sure thought, if perf penalty should be handled (and how) for single
> function device use case, possibly affecting all current users.
> 
> >  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>;

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]