[PATCH v2 1/2] ARM: dts: stm32mp15x: adjust USB OTG gadget tx fifo sizes

[PATCH v2 1/2] ARM: dts: stm32mp15x: adjust USB OTG gadget tx fifo sizes

[Michael Grzeschik]

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 patch is already discussed for the mainline kernel.

https://lore.kernel.org/linux-arm-kernel/20230112112013.1086787-1-u.kleine-koenig@pengutronix.de/

Until this is fix has reached the synced kernel dts tree in barebox we
fix this locally.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
v1 -> v2: - added this patch to solve the fifo issue on stm32 dwc2 when loading fastboot

 arch/arm/dts/stm32mp151.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/dts/stm32mp151.dtsi b/arch/arm/dts/stm32mp151.dtsi
index d3e924dc00..602bcd60b2 100644
--- a/arch/arm/dts/stm32mp151.dtsi
+++ b/arch/arm/dts/stm32mp151.dtsi
@@ -52,3 +52,7 @@
 		barebox,mode-serial = <0xFF>;
 	};
 };
+
+&usbotg_hs {
+	g-tx-fifo-size = <128 128 16 16 16 16 16 16>;
+};
-- 
2.30.2

[PATCH v2 2/2] usb: dwc2: fix multiplier handling in endpoint setup

[Michael Grzeschik]

If the multiplier is 0 in the descriptor bitfield the multiplier count
is one. For calculating the multiplier count the extra function
usb_endpoint_maxp_mult should be used. Rework the dwc2 driver to use it
and make multi packages work.

While at it, we also remove the USB_EP_MAXP_MULT and
USB_ENDPOINT_MAXP_MASK macros that would stay left unused and are
defined in include/usb/ch9.ch anyway.

Tested-by: Jules Maselbas <jmaselbas@kalray.eu>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
v1 -> v2: - added tested-by

 drivers/usb/dwc2/gadget.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 7070485410..5a72ba795b 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -14,13 +14,6 @@
 #define spin_lock_irqsave(lock, flags) (void)(flags)
 #define spin_unlock_irqrestore(lock, flags) (void)(flags)
 
-#ifndef USB_ENDPOINT_MAXP_MASK
-#define USB_ENDPOINT_MAXP_MASK	0x07ff
-#endif
-#ifndef USB_EP_MAXP_MULT
-#define USB_EP_MAXP_MULT(m)	(((m) & 0x1800) >> 11)
-#endif
-
 static void kill_all_requests(struct dwc2 *, struct dwc2_ep *, int);
 
 static inline struct dwc2_ep *index_to_ep(struct dwc2 *dwc2,
@@ -484,7 +477,7 @@ static int dwc2_ep_enable(struct usb_ep *ep,
 
 	ep_type = desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK;
 	mps = usb_endpoint_maxp(desc) & USB_ENDPOINT_MAXP_MASK;
-	mc =  USB_EP_MAXP_MULT(usb_endpoint_maxp(desc));
+	mc = usb_endpoint_maxp_mult(desc);
 
 	/* note, we handle this here instead of dwc2_set_ep_maxpacket */
 	epctrl_reg = dir_in ? DIEPCTL(index) : DOEPCTL(index);
-- 
2.30.2

Re: [PATCH v2 1/2] ARM: dts: stm32mp15x: adjust USB OTG gadget tx fifo sizes

[Sascha Hauer]

On Tue, Jan 24, 2023 at 09:01:13PM +0100, Michael Grzeschik 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.
> 
> 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 patch is already discussed for the mainline kernel.
> 
> https://lore.kernel.org/linux-arm-kernel/20230112112013.1086787-1-u.kleine-koenig@pengutronix.de/
> 
> Until this is fix has reached the synced kernel dts tree in barebox we
> fix this locally.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
> v1 -> v2: - added this patch to solve the fifo issue on stm32 dwc2 when loading fastboot
> 
>  arch/arm/dts/stm32mp151.dtsi | 4 ++++
>  1 file changed, 4 insertions(+)

Applied, thanks

Sascha

> 
> diff --git a/arch/arm/dts/stm32mp151.dtsi b/arch/arm/dts/stm32mp151.dtsi
> index d3e924dc00..602bcd60b2 100644
> --- a/arch/arm/dts/stm32mp151.dtsi
> +++ b/arch/arm/dts/stm32mp151.dtsi
> @@ -52,3 +52,7 @@
>  		barebox,mode-serial = <0xFF>;
>  	};
>  };
> +
> +&usbotg_hs {
> +	g-tx-fifo-size = <128 128 16 16 16 16 16 16>;
> +};
> -- 
> 2.30.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v2 1/2] ARM: dts: stm32mp15x: adjust USB OTG gadget tx fifo sizes

[Ahmad Fatoum]

On 24.01.23 21:01, Michael Grzeschik 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.

There is no driver support for this binding, so this patch achieves nothing.

> 
> 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 patch is already discussed for the mainline kernel.
> 
> https://lore.kernel.org/linux-arm-kernel/20230112112013.1086787-1-u.kleine-koenig@pengutronix.de/
> 
> Until this is fix has reached the synced kernel dts tree in barebox we
> fix this locally.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
> v1 -> v2: - added this patch to solve the fifo issue on stm32 dwc2 when loading fastboot
> 
>  arch/arm/dts/stm32mp151.dtsi | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/dts/stm32mp151.dtsi b/arch/arm/dts/stm32mp151.dtsi
> index d3e924dc00..602bcd60b2 100644
> --- a/arch/arm/dts/stm32mp151.dtsi
> +++ b/arch/arm/dts/stm32mp151.dtsi
> @@ -52,3 +52,7 @@
>  		barebox,mode-serial = <0xFF>;
>  	};
>  };
> +
> +&usbotg_hs {
> +	g-tx-fifo-size = <128 128 16 16 16 16 16 16>;
> +};

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v2 2/2] usb: dwc2: fix multiplier handling in endpoint setup

[Ahmad Fatoum]

On 24.01.23 21:01, Michael Grzeschik wrote:
> If the multiplier is 0 in the descriptor bitfield the multiplier count
> is one. For calculating the multiplier count the extra function
> usb_endpoint_maxp_mult should be used. Rework the dwc2 driver to use it
> and make multi packages work.
> 
> While at it, we also remove the USB_EP_MAXP_MULT and
> USB_ENDPOINT_MAXP_MASK macros that would stay left unused and are
> defined in include/usb/ch9.ch anyway.
> 
> Tested-by: Jules Maselbas <jmaselbas@kalray.eu>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

This induces the same breakage as in v1, even a simple ACM gadget
fails:

  barebox@Linux Automation Test Automation Controller (TAC):/ usbgadget -a
  udc0: registering UDC driver [g_multi]
  multi_bind: creating ACM function
  g_multi usbgadget: Multifunction Composite Gadget
  g_multi usbgadget: g_multi ready
  dwc2 49000000.usb-otg@49000000.of: bound driver g_multi
  dwc2 49000000.usb-otg@49000000.of: new address 3
  g_multi usbgadget: high-speed config #1: Multifunction Composite Gadget
  ERROR: dwc2 49000000.usb-otg@49000000.of: dwc2_ep_enable: No suitable fifo found


The other patch while maybe applicable to the kernel, doesn't do anything
for barebox. Please test barebox patches in barebox for v3.

@Sascha, can you drop this series from next?

Cheers,
Ahmad

> ---
> v1 -> v2: - added tested-by
> 
>  drivers/usb/dwc2/gadget.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 7070485410..5a72ba795b 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -14,13 +14,6 @@
>  #define spin_lock_irqsave(lock, flags) (void)(flags)
>  #define spin_unlock_irqrestore(lock, flags) (void)(flags)
>  
> -#ifndef USB_ENDPOINT_MAXP_MASK
> -#define USB_ENDPOINT_MAXP_MASK	0x07ff
> -#endif
> -#ifndef USB_EP_MAXP_MULT
> -#define USB_EP_MAXP_MULT(m)	(((m) & 0x1800) >> 11)
> -#endif
> -
>  static void kill_all_requests(struct dwc2 *, struct dwc2_ep *, int);
>  
>  static inline struct dwc2_ep *index_to_ep(struct dwc2 *dwc2,
> @@ -484,7 +477,7 @@ static int dwc2_ep_enable(struct usb_ep *ep,
>  
>  	ep_type = desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK;
>  	mps = usb_endpoint_maxp(desc) & USB_ENDPOINT_MAXP_MASK;
> -	mc =  USB_EP_MAXP_MULT(usb_endpoint_maxp(desc));
> +	mc = usb_endpoint_maxp_mult(desc);
>  
>  	/* note, we handle this here instead of dwc2_set_ep_maxpacket */
>  	epctrl_reg = dir_in ? DIEPCTL(index) : DOEPCTL(index);

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v2 2/2] usb: dwc2: fix multiplier handling in endpoint setup

[Sascha Hauer]

On Wed, Feb 01, 2023 at 07:13:24PM +0100, Ahmad Fatoum wrote:
> On 24.01.23 21:01, Michael Grzeschik wrote:
> > If the multiplier is 0 in the descriptor bitfield the multiplier count
> > is one. For calculating the multiplier count the extra function
> > usb_endpoint_maxp_mult should be used. Rework the dwc2 driver to use it
> > and make multi packages work.
> > 
> > While at it, we also remove the USB_EP_MAXP_MULT and
> > USB_ENDPOINT_MAXP_MASK macros that would stay left unused and are
> > defined in include/usb/ch9.ch anyway.
> > 
> > Tested-by: Jules Maselbas <jmaselbas@kalray.eu>
> > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> 
> This induces the same breakage as in v1, even a simple ACM gadget
> fails:
> 
>   barebox@Linux Automation Test Automation Controller (TAC):/ usbgadget -a
>   udc0: registering UDC driver [g_multi]
>   multi_bind: creating ACM function
>   g_multi usbgadget: Multifunction Composite Gadget
>   g_multi usbgadget: g_multi ready
>   dwc2 49000000.usb-otg@49000000.of: bound driver g_multi
>   dwc2 49000000.usb-otg@49000000.of: new address 3
>   g_multi usbgadget: high-speed config #1: Multifunction Composite Gadget
>   ERROR: dwc2 49000000.usb-otg@49000000.of: dwc2_ep_enable: No suitable fifo found
> 
> 
> The other patch while maybe applicable to the kernel, doesn't do anything
> for barebox. Please test barebox patches in barebox for v3.
> 
> @Sascha, can you drop this series from next?

Did that.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |