[PATCH V2 0/2] ARM: dts: bcm2711: Add generic xHCI

[PATCH V2 0/2] ARM: dts: bcm2711: Add generic xHCI

[Stefan Wahren]

In contrast to the Raspberry Pi 4, the Compute Module 4 or the IO board
does not have a VL805 USB 3.0 host controller, which is connected via
PCIe. Instead, the BCM2711 on the Compute Module provides the built-in
xHCI.

Changes in V2:
- adjust xHCI compatible as suggested by Justin & Florian
- keep xHCI disabled in order to let the bootloader decide which
  USB block should be enabled, which result in a drop of patch 3

Stefan Wahren (2):
  dt-bindings: usb: xhci: Add optional power-domains
  ARM: dts: bcm2711: Add generic xHCI

 .../devicetree/bindings/usb/generic-xhci.yaml      |  3 +++
 arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi        |  5 +++++
 arch/arm/boot/dts/broadcom/bcm2711.dtsi            | 14 ++++++++++++++
 3 files changed, 22 insertions(+)

--
2.34.1

[PATCH V2 1/2] dt-bindings: usb: xhci: Add optional power-domains

[Stefan Wahren]

BCM2711 SoC use a power domain for the xHCI. So allow supplying this
domain in bindings.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 Documentation/devicetree/bindings/usb/generic-xhci.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/generic-xhci.yaml b/Documentation/devicetree/bindings/usb/generic-xhci.yaml
index 594ebb3ee432..e48beaff7d0d 100644
--- a/Documentation/devicetree/bindings/usb/generic-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/generic-xhci.yaml
@@ -49,6 +49,9 @@ properties:
       - const: core
       - const: reg

+  power-domains:
+    maxItems: 1
+
 unevaluatedProperties: false

 required:
--
2.34.1

Re: [PATCH V2 1/2] dt-bindings: usb: xhci: Add optional power-domains

[Florian Fainelli]

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

On 11/30/23 07:42, Stefan Wahren wrote:
> BCM2711 SoC use a power domain for the xHCI. So allow supplying this
> domain in bindings.
> 
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>   Documentation/devicetree/bindings/usb/generic-xhci.yaml | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/generic-xhci.yaml b/Documentation/devicetree/bindings/usb/generic-xhci.yaml
> index 594ebb3ee432..e48beaff7d0d 100644
> --- a/Documentation/devicetree/bindings/usb/generic-xhci.yaml
> +++ b/Documentation/devicetree/bindings/usb/generic-xhci.yaml
> @@ -49,6 +49,9 @@ properties:
>         - const: core
>         - const: reg
> 
> +  power-domains:
> +    maxItems: 1

That works, I am not sure whether we want to key off based upon the 
compatible string, but either way:

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH V2 1/2] dt-bindings: usb: xhci: Add optional power-domains

[Conor Dooley]

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

On Thu, Nov 30, 2023 at 09:50:17AM -0800, Florian Fainelli wrote:
> On 11/30/23 07:42, Stefan Wahren wrote:
> > BCM2711 SoC use a power domain for the xHCI. So allow supplying this
> > domain in bindings.
> > 
> > Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> >   Documentation/devicetree/bindings/usb/generic-xhci.yaml | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/generic-xhci.yaml b/Documentation/devicetree/bindings/usb/generic-xhci.yaml
> > index 594ebb3ee432..e48beaff7d0d 100644
> > --- a/Documentation/devicetree/bindings/usb/generic-xhci.yaml
> > +++ b/Documentation/devicetree/bindings/usb/generic-xhci.yaml
> > @@ -49,6 +49,9 @@ properties:
> >         - const: core
> >         - const: reg
> > 
> > +  power-domains:
> > +    maxItems: 1
> 
> That works, I am not sure whether we want to key off based upon the
> compatible string, but either way:

If by "key it off" you mean selectively permit "power-domains" based on
the compatible, I think that would be a good idea. There is no bcm2711
specific compatible though at present and I figured there could well be
god-knows-how-many other users of generic-xhci out there that also make
use of power domains, which is why I did not request it of Stefan.

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

[PATCH V2 2/2] ARM: dts: bcm2711: Add generic xHCI

[Stefan Wahren]

The BCM2711 SoC also has a generic xHCI. The USB port is currently
only usable on the Compute Module 4 (e.g. via IO board). Because
DWC2 and xHCI share the same PHY, we let the bootloader enable it
on demand.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Tested-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
 arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi |  5 +++++
 arch/arm/boot/dts/broadcom/bcm2711.dtsi     | 14 ++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi b/arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi
index 98817a6675b9..d233a191c139 100644
--- a/arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include "bcm2835-rpi.dtsi"

+#include <dt-bindings/power/raspberrypi-power.h>
 #include <dt-bindings/reset/raspberrypi,firmware-reset.h>

 / {
@@ -76,3 +77,7 @@ &v3d {
 &vchiq {
 	interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
 };
+
+&xhci {
+	power-domains = <&power RPI_POWER_DOMAIN_USB>;
+};
diff --git a/arch/arm/boot/dts/broadcom/bcm2711.dtsi b/arch/arm/boot/dts/broadcom/bcm2711.dtsi
index 4a379a14966d..8e68884c2f0d 100644
--- a/arch/arm/boot/dts/broadcom/bcm2711.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm2711.dtsi
@@ -604,6 +604,20 @@ genet_mdio: mdio@e14 {
 			};
 		};

+		xhci: usb@7e9c0000 {
+			compatible = "brcm,xhci-brcm-v2";
+			reg = <0x0 0x7e9c0000 0x100000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
+			/* DWC2 and this IP block share the same USB PHY,
+			 * enabling both at the same time results in lockups.
+			 * So keep this node disabled and let the bootloader
+			 * decide which interface should be enabled.
+			 */
+			status = "disabled";
+		};
+
 		v3d: gpu@7ec00000 {
 			compatible = "brcm,2711-v3d";
 			reg = <0x0 0x7ec00000 0x4000>,
--
2.34.1

Re: [PATCH V2 2/2] ARM: dts: bcm2711: Add generic xHCI

[Florian Fainelli]

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

On 11/30/23 07:42, Stefan Wahren wrote:
> The BCM2711 SoC also has a generic xHCI. The USB port is currently
> only usable on the Compute Module 4 (e.g. via IO board). Because
> DWC2 and xHCI share the same PHY, we let the bootloader enable it
> on demand.
> 
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> Tested-by: Florian Fainelli <florian.fainelli@broadcom.com>

And this gives a consistent + 4MB/sec throughput compared to dwc2, so 
this looks great!
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH V2 2/2] ARM: dts: bcm2711: Add generic xHCI

[Justin Chen]

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



On 11/30/23 7:42 AM, Stefan Wahren wrote:
> The BCM2711 SoC also has a generic xHCI. The USB port is currently
> only usable on the Compute Module 4 (e.g. via IO board). Because
> DWC2 and xHCI share the same PHY, we let the bootloader enable it
> on demand.
> 
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> Tested-by: Florian Fainelli <florian.fainelli@broadcom.com>

Reviewed-by: Justin Chen <justin.chen@broadcom.com>

> ---
>   arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi |  5 +++++
>   arch/arm/boot/dts/broadcom/bcm2711.dtsi     | 14 ++++++++++++++
>   2 files changed, 19 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi b/arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi
> index 98817a6675b9..d233a191c139 100644
> --- a/arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi
> +++ b/arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi
> @@ -1,6 +1,7 @@
>   // SPDX-License-Identifier: GPL-2.0
>   #include "bcm2835-rpi.dtsi"
> 
> +#include <dt-bindings/power/raspberrypi-power.h>
>   #include <dt-bindings/reset/raspberrypi,firmware-reset.h>
> 
>   / {
> @@ -76,3 +77,7 @@ &v3d {
>   &vchiq {
>   	interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
>   };
> +
> +&xhci {
> +	power-domains = <&power RPI_POWER_DOMAIN_USB>;
> +};
> diff --git a/arch/arm/boot/dts/broadcom/bcm2711.dtsi b/arch/arm/boot/dts/broadcom/bcm2711.dtsi
> index 4a379a14966d..8e68884c2f0d 100644
> --- a/arch/arm/boot/dts/broadcom/bcm2711.dtsi
> +++ b/arch/arm/boot/dts/broadcom/bcm2711.dtsi
> @@ -604,6 +604,20 @@ genet_mdio: mdio@e14 {
>   			};
>   		};
> 
> +		xhci: usb@7e9c0000 {
> +			compatible = "brcm,xhci-brcm-v2";
> +			reg = <0x0 0x7e9c0000 0x100000>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
> +			/* DWC2 and this IP block share the same USB PHY,
> +			 * enabling both at the same time results in lockups.
> +			 * So keep this node disabled and let the bootloader
> +			 * decide which interface should be enabled.
> +			 */
> +			status = "disabled";
> +		};
> +
>   		v3d: gpu@7ec00000 {
>   			compatible = "brcm,2711-v3d";
>   			reg = <0x0 0x7ec00000 0x4000>,
> --
> 2.34.1
> 

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4206 bytes --]

Re: [PATCH V2 0/2] ARM: dts: bcm2711: Add generic xHCI

[Cyril Brulebois]

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

Hi Stefan,

Stefan Wahren <wahrenst@gmx.net> (2023-11-30):
> In contrast to the Raspberry Pi 4, the Compute Module 4 or the IO board
> does not have a VL805 USB 3.0 host controller, which is connected via
> PCIe. Instead, the BCM2711 on the Compute Module provides the built-in
> xHCI.
> 
> Changes in V2:
> - adjust xHCI compatible as suggested by Justin & Florian
> - keep xHCI disabled in order to let the bootloader decide which
>   USB block should be enabled, which result in a drop of patch 3
> 
> Stefan Wahren (2):
>   dt-bindings: usb: xhci: Add optional power-domains
>   ARM: dts: bcm2711: Add generic xHCI
> 
>  .../devicetree/bindings/usb/generic-xhci.yaml      |  3 +++
>  arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi        |  5 +++++
>  arch/arm/boot/dts/broadcom/bcm2711.dtsi            | 14 ++++++++++++++
>  3 files changed, 22 insertions(+)

Thanks, tests look much better this time!

Tested-by: Cyril Brulebois <cyril@debamax.com>


With CM4 Lite on CM4 IO Board, with a Samsung flash drive and a USB
keyboard connected to onboard USB ports, I'm getting the following
results (still with a Debian 12 arm64 userspace):

1. With unpatched kernel and unmodified config.txt:
     Both USB devices are working fine.

2. With unpatched kernel and otg_mode=1 in config.txt:
     Both USB devices disappear. lsmod reports dwc2 is no longer loaded,
     along with all USB and SCSI related modules.

3. With patched kernel and unmodified config.txt:
     Both USB devices are still working fine. lsmod confirms dwc2 is
     still used.

4. With patched kernel and otg_mode=1 in config.txt:
     Both USB devices are still working fine. lsmod reports dwc2 is
     going away, and other USB modules come up: usbhid, xhci_hcd,
     xhci_plat_hcd, along with others like hid, hid_generic, joydev. 

     Reading from the Samsung flash drive gives a little boost, from
     37.5 MB/s to 38.7 MB/s. Writing to it gives a little boost, from
     16.5 MB/s to 17.4 MB/s. Not as spectacular as Florian's results but
     still not a regression! :)

I tested that initially with a CM4 Lite Rev 1.0 (which was breaking case
number 3 with the v1 of this patch series), then extended testing to CM4
8/32 Rev 1.0 and CM4 4/32 Rev 1.1, which confirmed those results.


Adding a PCIe-to-USB expansion board to see if this has side effects on
other USB things, that still works fine in cases 3 and 4 (so without or
with otg_mode=1), having a Samsung flash drive on the PCIe-to-USB board
and another one the onboard USB port. At this point, I only verified the
block devices were reported by lsblk though (no actual transfer tests).

Of course that relies on also applying Jim Quinlan's PCIe patch series
v8 to make sure PCIe isn't an issue:
  https://lore.kernel.org/all/20231126201946.ffm3bhg5du2xgztv@mraw.org/

I've confirmed the presence of both Samsung flash drives with three
different cards (adding CONFIG_USB_XHCI_PCI_RENESAS=m to the config
shared in the v1 thread, and adding /lib/firmware/renesas_usb_fw.mem):
 - SupaHub PCE6U1C-R02, VER 006
 - SupaHub PCE6U1C-R02, VER 006S
 - Waveshare PCIe TO USB 3.2 Gen1 (B)
   https://www.waveshare.com/wiki/PCIe_TO_USB_3.2_Gen1_(B)


Finally, I've deployed the patched kernel (still this v2 plus Jim's v8)
in a CM4-based product that uses both onboard USB ports and PCIe-to-USB
ports, and all USB components still work fine (3 RF adapters, 1 modem).
That's the case with an unmodified config.txt, but also when adding
otg_mode=1:
 - xhci_pci and xhci_pci_renesas were already loaded;
 - xhci_plat_hcd appears in OTG mode, while dwc2 goes away.


Cheers,
-- 
Cyril Brulebois (kibi@debian.org)            <https://debamax.com/>
D-I release manager -- Release team member -- Freelance Consultant

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