overlays: dwc2: Increase RX FIFO size

Commit Message

Pavel Hofman Feb. 7, 2020, 4:30 p.m. UTC

The previous version of the dwc2 overlay set the RX FIFO size to
256 4-byte words. This sounds large enough for a 1024 byte packet (the
largest isochronous high speed packet allowed), but it doesn't take
into account some extra space needed by the hardware.

Minas Harutyunyan at Synopsys (the source of the DWC OTG design)
came up with a more correct value, 301, but since there is spare packet
RAM this can be increased to 558 to allow two packets per frame.

See: https://github.com/raspberrypi/linux/issues/3447

Signed-off-by: Phil Elwell <[e-mail]>
Signed-off-by: Pavel Hofman <[e-mail]>
---
 arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi        | 2 +-
 arch/arm/boot/dts/bcm283x-rpi-usb-peripheral.dtsi | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Nicolas Saenz Julienne Feb. 7, 2020, 5:25 p.m. UTC | #1

Hi Pavel,
Could you change the patch title to something like this (see other patches
changing the device-trees, you'll see it's the common pattern):

	ARM: dts: bcm283x: increase dwc2's RX FIFO size

On Fri, 2020-02-07 at 17:30 +0100, Pavel Hofman wrote:
> The previous version of the dwc2 overlay set the RX FIFO size to
> 256 4-byte words. This sounds large enough for a 1024 byte packet (the
> largest isochronous high speed packet allowed), but it doesn't take
> into account some extra space needed by the hardware.
> 
> Minas Harutyunyan at Synopsys (the source of the DWC OTG design)
> came up with a more correct value, 301, but since there is spare packet
> RAM this can be increased to 558 to allow two packets per frame.

I suggest making a short summary of your github explanation[1] instead of just
citing Minas. On one hand Minas' memory might get fuzzy after some time (or not
be able to access the documentation) and...

> See: https://github.com/raspberrypi/linux/issues/3447

...you're assuming this link will be always available. Which is unlikely. After
properly explaining the issue, you won't need the link anyway. Ultimately a
patch description should be as self-contained/self-explanatory as possible.

Other than that the patch looks good.
Thanks!
Nicolas

[1] https://github.com/raspberrypi/linux/issues/3447#issuecomment-581815066

Pavel Hofman Feb. 7, 2020, 7:24 p.m. UTC | #2

Hi Nicolas,

Dne 07. 02. 20 v 18:25 Nicolas Saenz Julienne napsal(a):
> 
> I suggest making a short summary of your github explanation[1] instead of just
> citing Minas. On one hand Minas' memory might get fuzzy after some time (or not
> be able to access the documentation) and...
> 
>> See: https://github.com/raspberrypi/linux/issues/3447
> 
> ...you're assuming this link will be always available. Which is unlikely. After
> properly explaining the issue, you won't need the link anyway. Ultimately a
> patch description should be as self-contained/self-explanatory as possible.

No problem. I tend to copy the whole explanation from that post as it 
explains in detail every step of the calculation which is quite complex. 
Would that be OK?

Thanks,

Pavel.

Patch

diff --git a/arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi
b/arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi
index e2fd961..20322de 100644
--- a/arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi
+++ b/arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi
@@ -1,7 +1,7 @@ 
 // SPDX-License-Identifier: GPL-2.0
 &usb {
 	dr_mode = "otg";
-	g-rx-fifo-size = <256>;
+	g-rx-fifo-size = <558>;
 	g-np-tx-fifo-size = <32>;
 	/*
 	 * According to dwc2 the sum of all device EP
diff --git a/arch/arm/boot/dts/bcm283x-rpi-usb-peripheral.dtsi
b/arch/arm/boot/dts/bcm283x-rpi-usb-peripheral.dtsi
index 0ff0e9e..1409d1b 100644
--- a/arch/arm/boot/dts/bcm283x-rpi-usb-peripheral.dtsi
+++ b/arch/arm/boot/dts/bcm283x-rpi-usb-peripheral.dtsi
@@ -1,7 +1,7 @@ 
 // SPDX-License-Identifier: GPL-2.0
 &usb {
 	dr_mode = "peripheral";
-	g-rx-fifo-size = <256>;
+	g-rx-fifo-size = <558>;
 	g-np-tx-fifo-size = <32>;
 	g-tx-fifo-size = <256 256 512 512 512 768 768>;
 };