diff mbox series

Discussing some limitations of USB onboard devices

Message ID 939f75e9-7d17-4627-b19f-71e286d11256@cherry.de
State New
Headers show
Series Discussing some limitations of USB onboard devices | expand

Commit Message

Quentin Schulz March 20, 2025, 1:56 p.m. UTC 
Hi all,

We're working on fixing the Device Tree for our RK3399 Puma board to use 
onboard USB hubs instead of some hack we currently have for handling the 
reset line that is unreliable.

This is the diff I believe should represent what needs to be done for 
supporting that:

"""
"""

However, this simply doesn't work for a few reasons.

1) pinctrl request is broken, I get the following error messages:

"""
[    2.979479] rockchip-pinctrl pinctrl: pin gpio4-3 already requested 
by fe900000.usb:hub@1; cannot claim for 1-1
[    2.981799] rockchip-usb2phy ff770000.syscon:usb2phy@e450: Requested 
PHY is disabled
[    2.981846] xhci-hcd xhci-hcd.4.auto: xHCI Host Controller
[    2.993129] rockchip-pinctrl pinctrl: error -EINVAL: pin-131 (1-1)
[    3.002141] xhci-hcd xhci-hcd.4.auto: new USB bus registered, 
assigned bus number 3
[    3.007936] rockchip-pinctrl pinctrl: error -EINVAL: could not 
request pin 131 (gpio4-3) from group cy3304-reset on device rockchip-pinctrl
[    3.023446] onboard-usb-dev 1-1: Error applying setting, reverse 
things back
[...]
[    3.131038] rockchip-pinctrl pinctrl: pin gpio4-3 already requested 
by fe900000.usb:hub@1; cannot claim for 2-1
[    3.132598] dwmmc_rockchip fe320000.mmc: Version ID is 270a
[    3.136997] rockchip-pinctrl pinctrl: error -EINVAL: pin-131 (2-1)
[    3.142490] dwmmc_rockchip fe320000.mmc: DW MMC controller at irq 
81,32 bit host data width,256 deep fifo
[    3.149860] rockchip-pinctrl pinctrl: error -EINVAL: could not 
request pin 131 (gpio4-3) from group cy3304-reset on device rockchip-pinctrl
[    3.199221] onboard-usb-dev 2-1: Error applying setting, reverse 
things back
"""

This can be worked around by using the HCD node to store the pinctrl, 
but it feels wrong to me.

Another way could be to have a pinctrl-names differ from "default" or 
"init" (e.g. "hub") and handle that directly in the probe of the 
platform device to avoid triggering the automagic pinctrl 
"default"/"init" behavior pre-binding/probing (which I assume is the 
reason for this error?).

2) I get error messages wrt vdd/vdd2 supply for the second hub:

"""
[    2.564690] dwc3 fe900000.usb: Failed to create device link (0x180) 
with supplier regulator-vcc1v2-phy for /usb@fe900000/usb@fe900000/hub@2
[    2.578928] dwc3 fe900000.usb: Failed to create device link (0x180) 
with supplier regulator-vcc3v3-sys for /usb@fe900000/usb@fe900000/hub@2
"""

I assume this may be because there's no device created with the OF node 
of the second hub (the second device uses the platform device and its OF 
node from the peer-hub, the first device to probe; if I'm not mistaken?).

For that SoM it isn't a big issue because both power rails are always on 
and at boot (like, enforced by hardware), so it'd be fine for us to use 
dummy regulators (e.g. not having the properties in the first place).

3) reset-gpios (and probably other properties like clocks, i2c-bus, 
etc...) need to be identical on both hubs.

Otherwise the first hub to probe will be whose OF node will be used, so 
if properties are missing or different from the other hub, we'll just 
handle properties from that first-to-probe hub. I assume this is fine as 
we expect peer hubs to use the same reset GPIO(s), clocks and everything 
else as they are seen as individual physical entities and that's the 
expectation of how the HW is functioning? Should we (can we?) enforce 
that on the dt-binding level to make sure that is the case?

What are the recommendations here? Did I misunderstand how to declare 
our onboard hub in the DT? Did I misunderstand some internals of this 
onboard hub driver maybe? Is there anything we can do to improve that?

Cheers,
Quentin
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
index e00fbaa8acc16..7aa260fc9dda7 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
@@ -60,16 +60,6 @@  vcc3v3_sys: regulator-vcc3v3-sys {
  		vin-supply = <&vcc5v0_sys>;
  	};

-	vcc5v0_host: regulator-vcc5v0-host {
-		compatible = "regulator-fixed";
-		gpio = <&gpio4 RK_PA3 GPIO_ACTIVE_LOW>;
-		pinctrl-names = "default";
-		pinctrl-0 = <&vcc5v0_host_en>;
-		regulator-name = "vcc5v0_host";
-		regulator-always-on;
-		vin-supply = <&vcc5v0_sys>;
-	};
-
  	vcc5v0_sys: regulator-vcc5v0-sys {
  		compatible = "regulator-fixed";
  		regulator-name = "vcc5v0_sys";
@@ -527,8 +517,8 @@  pmic_int_l: pmic-int-l {
  		};
  	};

-	usb2 {
-		vcc5v0_host_en: vcc5v0-host-en {
+	usb {
+		cy3304_reset: cy3304-reset {
  			rockchip,pins =
  			  <4 RK_PA3 RK_FUNC_GPIO &pcfg_pull_none>;
  		};
@@ -597,7 +587,6 @@  u2phy1_otg: otg-port {
  	};

  	u2phy1_host: host-port {
-		phy-supply = <&vcc5v0_host>;
  		status = "okay";
  	};
  };
@@ -609,6 +598,30 @@  &usbdrd3_1 {
  &usbdrd_dwc3_1 {
  	status = "okay";
  	dr_mode = "host";
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	hub_2_0: hub@1 {
+		compatible = "usb4b4,6502";
+		reg = <1>;
+		peer-hub = <&hub_3_0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&cy3304_reset>;
+		reset-gpios = <&gpio4 RK_PA3 GPIO_ACTIVE_HIGH>;
+		vdd-supply = <&vcc1v2_phy>;
+		vdd2-supply = <&vcc3v3_sys>;
+	};
+
+	hub_3_0: hub@2 {
+		compatible = "usb4b4,6500";
+		reg = <2>;
+		peer-hub = <&hub_2_0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&cy3304_reset>;
+		reset-gpios = <&gpio4 RK_PA3 GPIO_ACTIVE_HIGH>;
+		vdd-supply = <&vcc1v2_phy>;
+		vdd2-supply = <&vcc3v3_sys>;
+	};
  };

  &usb_host1_ehci {