[PATCH 0/4] RK3576 USB Enablement

[PATCH 0/4] RK3576 USB Enablement

[Nicolas Frattaroli]

This series is the result of what I thought would be a quick 10 minute
job, but turned out to be more like 3 days of pain, suffering, and
confusion. This should be expected with USB Type C though.

The first patch in the series extends the inno usb2 PHY binding in order
for us to be able to do what the second patch in the series does, which
is fiddle with some GRF flags in that driver when the PHY is connected
to a USB Type C port. Without this change, devices on USB-C simply don't
enumerate at all, as the state machine gets stuck waiting for vbus to go
low or something along those lines.

An alternate way to implement this would've been a vendor property in
the PHY binding which is then checked for in the driver and needs to be
present in all rockchip inno u2phy instances that happen to be connected
to a USB Type C connector. This is what downstream does, for example.

Patch 3 was really lovely to run into and needed a good amount of
sleuthing. I'm still certain this is "the wrong" fix, as it simply keeps
the PHY on, but it's a fix nonetheless, and one which has precedent,
namely in the RK3399. If someone is on a hunt to save milliamps in the
future, they might want to invest the time to come up with a less
sledgehammer sized solution for this problem, e.g. by disabling power
management until the controller is fully initialised.

Patch 4 adds the USB related nodes, including associated regulators and
Type C controllers, to the Sige5 tree.

As it stands, superspeed on the Sige 5's Type C port is still finnicky;
after a full day of debugging I still can't figure out why superspeed
only works in one cable orientation. I did a thorough pick through the
usbdp PHY driver, comparing it with what downstream has, etc. etc.
Because everything to do with testing Type-C ultimately comes down to
narrowing down the search space of non-compliant devices and
non-compliant cables to where you can sort of guess at the bug, it's a
huge time investment to debug this, and I'd rather submit the series
as-is right now for review and maybe some additional insight and testing
from someone with a wider variety of USB Type C cables and devices.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Nicolas Frattaroli (4):
      dt-bindings: phy: rockchip,inno-usb2phy: add port property
      phy: rockchip: inno-usb2: add soft vbusvalid control
      arm64: dts: rockchip: add phy suspend quirk to usb on rk3576
      arm64: dts: rockchip: enable USB on Sige5

 .../bindings/phy/rockchip,inno-usb2phy.yaml        |   5 +
 .../boot/dts/rockchip/rk3576-armsom-sige5.dts      | 153 +++++++++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3576.dtsi           |   2 +
 drivers/phy/rockchip/phy-rockchip-inno-usb2.c      |  78 ++++++++++-
 4 files changed, 234 insertions(+), 4 deletions(-)
---
base-commit: 64e9fdfc89a76fed38d8ddeed72d42ec71957ed9
change-id: 20250328-rk3576-sige5-usb-230102aeeaca

Best regards,
-- 
Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

[PATCH 1/4] dt-bindings: phy: rockchip,inno-usb2phy: add port property

[Nicolas Frattaroli]

USB connectors like to have OF graph connections to high-speed related
nodes to do various things. In the case of the RK3576, we can make use
of a port in the usb2 PHY to detect whether the OTG controller is
connected to a type C port and apply some special behaviour accordingly.

The usefulness of having different bits of a fully functioning USB stack
point to each other is more general though, and not constrained to
RK3576 at all, even for this use-case.

Add a port property to the binding.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
index 6a7ef556414cebad63c10de754778f84fd4486ee..3a662bfc353250a8ad9386ebb5575d1e84c1b5ba 100644
--- a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
+++ b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
@@ -78,6 +78,11 @@ properties:
       When set the driver will request its phandle as one companion-grf
       for some special SoCs (e.g rv1108).
 
+  port:
+    $ref: /schemas/graph.yaml#/properties/port
+    description:
+      A port node to link the PHY to a USB connector's "high-speed" port.
+
   host-port:
     type: object
     additionalProperties: false

-- 
2.49.0

[PATCH 2/4] phy: rockchip: inno-usb2: add soft vbusvalid control

[Nicolas Frattaroli]

With USB type C connectors, the vbus detect pin of the OTG controller
attached to it is pulled high by a USB Type C controller chip such as
the fusb302. This means USB enumeration on Type-C ports never works, as
the vbus is always seen as high.

Rockchip added some GRF register flags to deal with this situation. The
RK3576 TRM calls these "soft_vbusvalid_bvalid" (con0 bit index 15) and
"soft_vbusvalid_bvalid_sel" (con0 bit index 14).

Downstream introduces a new vendor property which tells the USB 2 PHY
that it's connected to a type C port, but we can do better. Since in
such an arrangement, we'll have an OF graph connection to the USB
connector anyway, we can walk said OF graph and check the connector's
compatible to determine this without adding any further vendor
properties.

Do keep in mind that the usbdp PHY driver seemingly fiddles with these
register fields as well, but what it does doesn't appear to be enough
for us to get working USB enumeration, presumably because the whole
vbus_attach logic needs to be adjusted as well either way.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 78 +++++++++++++++++++++++++--
 1 file changed, 74 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
index b5e6a864deebbcb33375001fc6ed67b2dfee6954..7dc4f53e18fcf0c9d010dd78e5fcd089b6bae43f 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
+#include <linux/of_graph.h>
 #include <linux/of_irq.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
@@ -114,6 +115,8 @@ struct rockchip_chg_det_reg {
 /**
  * struct rockchip_usb2phy_port_cfg - usb-phy port configuration.
  * @phy_sus: phy suspend register.
+ * @svbus_en: soft vbus bvalid enable register.
+ * @svbus_sel: soft vbus bvalid selection register.
  * @bvalid_det_en: vbus valid rise detection enable register.
  * @bvalid_det_st: vbus valid rise detection status register.
  * @bvalid_det_clr: vbus valid rise detection clear register.
@@ -140,6 +143,8 @@ struct rockchip_chg_det_reg {
  */
 struct rockchip_usb2phy_port_cfg {
 	struct usb2phy_reg	phy_sus;
+	struct usb2phy_reg	svbus_en;
+	struct usb2phy_reg	svbus_sel;
 	struct usb2phy_reg	bvalid_det_en;
 	struct usb2phy_reg	bvalid_det_st;
 	struct usb2phy_reg	bvalid_det_clr;
@@ -203,6 +208,7 @@ struct rockchip_usb2phy_cfg {
  * @event_nb: hold event notification callback.
  * @state: define OTG enumeration states before device reset.
  * @mode: the dr_mode of the controller.
+ * @typec_vbus_det: whether to apply Type C logic to OTG vbus detection.
  */
 struct rockchip_usb2phy_port {
 	struct phy	*phy;
@@ -222,6 +228,7 @@ struct rockchip_usb2phy_port {
 	struct notifier_block	event_nb;
 	enum usb_otg_state	state;
 	enum usb_dr_mode	mode;
+	bool typec_vbus_det;
 };
 
 /**
@@ -495,6 +502,13 @@ static int rockchip_usb2phy_init(struct phy *phy)
 	mutex_lock(&rport->mutex);
 
 	if (rport->port_id == USB2PHY_PORT_OTG) {
+		if (rport->typec_vbus_det) {
+			if (rport->port_cfg->svbus_en.enable &&
+					rport->port_cfg->svbus_sel.enable) {
+				property_enable(rphy->grf, &rport->port_cfg->svbus_en, true);
+				property_enable(rphy->grf, &rport->port_cfg->svbus_sel, true);
+			}
+		}
 		if (rport->mode != USB_DR_MODE_HOST &&
 		    rport->mode != USB_DR_MODE_UNKNOWN) {
 			/* clear bvalid status and enable bvalid detect irq */
@@ -535,8 +549,7 @@ static int rockchip_usb2phy_init(struct phy *phy)
 			if (ret)
 				goto out;
 
-			schedule_delayed_work(&rport->otg_sm_work,
-					      OTG_SCHEDULE_DELAY * 3);
+			schedule_delayed_work(&rport->otg_sm_work, 0);
 		} else {
 			/* If OTG works in host only mode, do nothing. */
 			dev_dbg(&rport->phy->dev, "mode %d\n", rport->mode);
@@ -666,8 +679,17 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
 	unsigned long delay;
 	bool vbus_attach, sch_work, notify_charger;
 
-	vbus_attach = property_enabled(rphy->grf,
-				       &rport->port_cfg->utmi_bvalid);
+	if (rport->port_cfg->svbus_en.enable && rport->typec_vbus_det) {
+		if (property_enabled(rphy->grf, &rport->port_cfg->svbus_en) &&
+		    property_enabled(rphy->grf, &rport->port_cfg->svbus_sel)) {
+			vbus_attach = true;
+		} else {
+			vbus_attach = false;
+		}
+	} else {
+		vbus_attach = property_enabled(rphy->grf,
+					       &rport->port_cfg->utmi_bvalid);
+	}
 
 	sch_work = false;
 	notify_charger = false;
@@ -1276,6 +1298,48 @@ static int rockchip_otg_event(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }
 
+static const char *const rockchip_usb2phy_typec_cons[] = {
+	"usb-c-connector",
+	NULL,
+};
+
+static bool rockchip_usb2phy_otg_is_type_c(struct rockchip_usb2phy *rphy)
+{
+	struct device_node *node = rphy->dev->of_node;
+	struct device_node *ports;
+	struct device_node *ep = NULL;
+	struct device_node *parent;
+
+	ports = of_get_child_by_name(node, "ports");
+	if (ports)
+		node = ports;
+
+	for_each_of_graph_port(node, port) {
+		ep = of_get_child_by_name(port, "endpoint");
+		if (!ep)
+			continue;
+
+		parent = of_graph_get_remote_port_parent(ep);
+		of_node_put(ep);
+		if (!parent)
+			continue;
+
+		if (of_device_compatible_match(parent, rockchip_usb2phy_typec_cons)) {
+			of_node_put(parent);
+			if (ports)
+				of_node_put(ports);
+			return true;
+		}
+
+		of_node_put(parent);
+	}
+
+	if (ports)
+		of_node_put(ports);
+
+	return false;
+}
+
 static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
 					  struct rockchip_usb2phy_port *rport,
 					  struct device_node *child_np)
@@ -1297,6 +1361,8 @@ static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
 
 	mutex_init(&rport->mutex);
 
+	rport->typec_vbus_det = rockchip_usb2phy_otg_is_type_c(rphy);
+
 	rport->mode = of_usb_get_dr_mode_by_phy(child_np, -1);
 	if (rport->mode == USB_DR_MODE_HOST ||
 	    rport->mode == USB_DR_MODE_UNKNOWN) {
@@ -1971,6 +2037,8 @@ static const struct rockchip_usb2phy_cfg rk3576_phy_cfgs[] = {
 		.port_cfgs	= {
 			[USB2PHY_PORT_OTG] = {
 				.phy_sus	= { 0x0000, 8, 0, 0, 0x1d1 },
+				.svbus_en	= { 0x0000, 15, 15, 0, 1 },
+				.svbus_sel	= { 0x0000, 14, 14, 0, 1 },
 				.bvalid_det_en	= { 0x00c0, 1, 1, 0, 1 },
 				.bvalid_det_st	= { 0x00c4, 1, 1, 0, 1 },
 				.bvalid_det_clr = { 0x00c8, 1, 1, 0, 1 },
@@ -2008,6 +2076,8 @@ static const struct rockchip_usb2phy_cfg rk3576_phy_cfgs[] = {
 		.port_cfgs	= {
 			[USB2PHY_PORT_OTG] = {
 				.phy_sus	= { 0x2000, 8, 0, 0, 0x1d1 },
+				.svbus_en	= { 0x2000, 15, 15, 0, 1 },
+				.svbus_sel	= { 0x2000, 14, 14, 0, 1 },
 				.bvalid_det_en	= { 0x20c0, 1, 1, 0, 1 },
 				.bvalid_det_st	= { 0x20c4, 1, 1, 0, 1 },
 				.bvalid_det_clr = { 0x20c8, 1, 1, 0, 1 },

-- 
2.49.0

[PATCH 3/4] arm64: dts: rockchip: add phy suspend quirk to usb on rk3576

[Nicolas Frattaroli]

The dwc3 controllers on RK3576 appear to have some quirky behaviour
relating to runtime power suspension. Specifically, they will cause
SError aborts on register reads in the xhci code when setting the device
role.

Downstream appears to work around this by manually checking for the
rk3576 compatible in the dwc3 core driver, and then setting the
autosuspend delay to 200. This appears to be smaller than what mainline
uses, and I'm not quite sure how that fixes it, but I could also be
reading the code wrong. The important takeaway is that the RK3399 uses
the same codepath.

Looking at RK3399 in mainline then, we see that it has one quirk that
RK3576 doesn't have in its dwc3 controllers: snps,dis_u2_susphy_quirk.

Long story short, adding the snps,dis_u2_susphy_quirk property to
rk3576.dtsi's dwc3 controllers fixes the issue. Quite suspicious that
this is not needed among usb controllers based on dwc3 in other Rockchip
designs, especially other RK35xx ones.

Fixes: 23ec57a32da4 ("arm64: dts: rockchip: add usb related nodes for rk3576")
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 arch/arm64/boot/dts/rockchip/rk3576.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
index ebb5fc8bb8b1363127b9d3782801c4a79b678a92..71c71f5a2b77c52b251bea16e4deaa4b0109715a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3576.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
@@ -470,6 +470,7 @@ usb_drd0_dwc3: usb@23000000 {
 			snps,dis-u2-freeclk-exists-quirk;
 			snps,dis-del-phy-power-chg-quirk;
 			snps,dis-tx-ipgap-linecheck-quirk;
+			snps,dis_u2_susphy_quirk;
 			snps,parkmode-disable-hs-quirk;
 			snps,parkmode-disable-ss-quirk;
 			status = "disabled";
@@ -496,6 +497,7 @@ usb_drd1_dwc3: usb@23400000 {
 			snps,dis-del-phy-power-chg-quirk;
 			snps,dis-tx-ipgap-linecheck-quirk;
 			snps,dis_rxdet_inp3_quirk;
+			snps,dis_u2_susphy_quirk;
 			snps,parkmode-disable-hs-quirk;
 			snps,parkmode-disable-ss-quirk;
 			dma-coherent;

-- 
2.49.0

[PATCH 4/4] arm64: dts: rockchip: enable USB on Sige5

[Nicolas Frattaroli]

The ArmSoM Sige5 has several USB ports: a Type-A USB 3 port (USB2 lines
going through a hub), a Type-A USB 2.0 port (also going through a hub),
a Type-C DC input port that has absolutely no USB data connection and a
Type-C port with USB3.2 Gen1x1 that's also the maskrom programming port.

Enable these ports, and set the device role to be host for the host
ports.

The data capable Type-C USB port uses a fusb302 for data role switching.
It currently does not have functioning SuperSpeed with certain Type-C
cables in one of the two possible orientations.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 .../boot/dts/rockchip/rk3576-armsom-sige5.dts      | 153 +++++++++++++++++++++
 1 file changed, 153 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dts b/arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dts
index 828bde7fab68dc6bcbd13d75c8a72540b3666071..cdd8f8b2319105b1dae34395be20e50d45cd4431 100644
--- a/arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dts
@@ -175,6 +175,33 @@ vcc_3v3_ufs_s0: regulator-vcc-ufs-s0 {
 		regulator-max-microvolt = <3300000>;
 		vin-supply = <&vcc_5v0_sys>;
 	};
+
+	vcc_5v0_typec0: regulator-vcc-5v0-typec0 {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpios = <&gpio4 RK_PA6 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&usb_otg0_pwren>;
+		regulator-name = "vcc_5v0_typec0";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		vin-supply = <&vcc_5v0_device>;
+	};
+	vcc_5v0_usbhost: regulator-vcc-5v0-usbhost {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpios = <&gpio4 RK_PA4 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&usb_host_pwren>;
+		regulator-name = "vcc_5v0_usbhost";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		vin-supply = <&vcc_5v0_device>;
+	};
+};
+
+&combphy1_psu {
+	status = "okay";
 };
 
 &cpu_l0 {
@@ -605,6 +632,58 @@ regulator-state-mem {
 &i2c2 {
 	status = "okay";
 
+	usbc0: typec-portc@22 {
+		compatible = "fcs,fusb302";
+		reg = <0x22>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <RK_PA5 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&usbc0_interrupt>;
+		vbus-supply = <&vcc_5v0_typec0>;
+
+		connector {
+			compatible = "usb-c-connector";
+			label = "USB-C";
+			data-role = "dual";
+			/* fusb302 supports PD Rev 2.0 Ver 1.2 */
+			pd-revision = /bits/ 8 <0x2 0x0 0x1 0x2>;
+			power-role = "source";
+			source-pdos = <PDO_FIXED(5000, 2000,
+						 PDO_FIXED_USB_COMM | PDO_FIXED_DATA_SWAP)>;
+
+			altmodes {
+				displayport {
+					svid = /bits/ 16 <0xff01>;
+					vdo = <0xffffffff>;
+				};
+			};
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					usbc0_hs_ep: endpoint {
+						remote-endpoint = <&u2phy0_ep>;
+					};
+				};
+				port@1 {
+					reg = <1>;
+					usbc0_ss_ep: endpoint {
+						remote-endpoint = <&usb_drd0_ep>;
+					};
+				};
+				port@2 {
+					reg = <2>;
+					usbc0_dp_ep: endpoint {
+						remote-endpoint = <&usbdp_phy_ep>;
+					};
+				};
+			};
+		};
+	};
+
 	hym8563: rtc@51 {
 		compatible = "haoyu,hym8563";
 		reg = <0x51>;
@@ -655,6 +734,24 @@ led_rgb_g: led-green-en {
 			rockchip,pins = <4 RK_PB2 RK_FUNC_GPIO &pcfg_pull_none>;
 		};
 	};
+
+	usb {
+		usb_host_pwren: usb-host-pwren {
+			rockchip,pins = <4 RK_PA4 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+		usb_otg0_pwren: usb-otg0-pwren {
+			rockchip,pins = <4 RK_PA6 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+		usbc0_interrupt: usbc0-interrupt {
+			rockchip,pins = <0 RK_PA5 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+		usbc0_sbu1: usbc0-sbu1 {
+			rockchip,pins = <2 RK_PA6 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+		usbc0_sbu2: usbc0-sbu2 {
+			rockchip,pins = <2 RK_PA7 RK_FUNC_GPIO &pcfg_pull_down>;
+		};
+	};
 };
 
 &sdhci {
@@ -683,11 +780,67 @@ &sdmmc {
 	status = "okay";
 };
 
+&u2phy0 {
+	status = "okay";
+
+	port {
+		u2phy0_ep: endpoint {
+			remote-endpoint = <&usbc0_hs_ep>;
+		};
+	};
+};
+
+&u2phy0_otg {
+	status = "okay";
+};
+
+&u2phy1 {
+	status = "okay";
+};
+
+&u2phy1_otg {
+	phy-supply = <&vcc_5v0_usbhost>;
+	status = "okay";
+};
+
 &uart0 {
 	pinctrl-0 = <&uart0m0_xfer>;
 	status = "okay";
 };
 
+&usb_drd0_dwc3 {
+	usb-role-switch;
+	dr_mode = "otg";
+	status = "okay";
+
+	port {
+		usb_drd0_ep: endpoint {
+			remote-endpoint = <&usbc0_ss_ep>;
+		};
+	};
+};
+
+&usb_drd1_dwc3 {
+	dr_mode = "host";
+	status = "okay";
+};
+
+&usbdp_phy {
+	mode-switch;
+	orientation-switch;
+	pinctrl-names = "default";
+	pinctrl-0 = <&usbc0_sbu1 &usbc0_sbu2>;
+	sbu1-dc-gpios = <&gpio2 RK_PA6 GPIO_ACTIVE_HIGH>;
+	sbu2-dc-gpios = <&gpio2 RK_PA7 GPIO_ACTIVE_HIGH>;
+	status = "okay";
+
+	port {
+		usbdp_phy_ep: endpoint {
+			remote-endpoint = <&usbc0_dp_ep>;
+		};
+	};
+};
+
 &vop {
 	status = "okay";
 };

-- 
2.49.0

Re: [PATCH 1/4] dt-bindings: phy: rockchip,inno-usb2phy: add port property

[Rob Herring]

On Mon, Apr 07, 2025 at 08:09:14PM +0200, Nicolas Frattaroli wrote:
> USB connectors like to have OF graph connections to high-speed related
> nodes to do various things. In the case of the RK3576, we can make use
> of a port in the usb2 PHY to detect whether the OTG controller is
> connected to a type C port and apply some special behaviour accordingly.
> 
> The usefulness of having different bits of a fully functioning USB stack
> point to each other is more general though, and not constrained to
> RK3576 at all, even for this use-case.
> 
> Add a port property to the binding.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
> index 6a7ef556414cebad63c10de754778f84fd4486ee..3a662bfc353250a8ad9386ebb5575d1e84c1b5ba 100644
> --- a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
> @@ -78,6 +78,11 @@ properties:
>        When set the driver will request its phandle as one companion-grf
>        for some special SoCs (e.g rv1108).
>  
> +  port:
> +    $ref: /schemas/graph.yaml#/properties/port
> +    description:
> +      A port node to link the PHY to a USB connector's "high-speed" port.

I don't think this is correct. The HS port of the connector goes to the 
controller. The controller has the link to the phy.

If the PHY is also what handles USB-C muxing or orientation switching, 
then it might have ports, but then it needs input and output ports.

Rob

Re: [PATCH 1/4] dt-bindings: phy: rockchip,inno-usb2phy: add port property

[Nicolas Frattaroli]

On Thursday, 10 April 2025 23:11:23 Central European Summer Time Rob Herring wrote:
> On Mon, Apr 07, 2025 at 08:09:14PM +0200, Nicolas Frattaroli wrote:
> > USB connectors like to have OF graph connections to high-speed related
> > nodes to do various things. In the case of the RK3576, we can make use
> > of a port in the usb2 PHY to detect whether the OTG controller is
> > connected to a type C port and apply some special behaviour accordingly.
> > 
> > The usefulness of having different bits of a fully functioning USB stack
> > point to each other is more general though, and not constrained to
> > RK3576 at all, even for this use-case.
> > 
> > Add a port property to the binding.
> > 
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> >  Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
> > index 6a7ef556414cebad63c10de754778f84fd4486ee..3a662bfc353250a8ad9386ebb5575d1e84c1b5ba 100644
> > --- a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
> > @@ -78,6 +78,11 @@ properties:
> >        When set the driver will request its phandle as one companion-grf
> >        for some special SoCs (e.g rv1108).
> >  
> > +  port:
> > +    $ref: /schemas/graph.yaml#/properties/port
> > +    description:
> > +      A port node to link the PHY to a USB connector's "high-speed" port.
> 
> I don't think this is correct. The HS port of the connector goes to the 
> controller. The controller has the link to the phy.
> 
> If the PHY is also what handles USB-C muxing or orientation switching, 
> then it might have ports, but then it needs input and output ports.
> 
> Rob
> 

Hi Rob,

thank you for the quick response.

I've feared this would be the case, but chose to go ahead with this solution
anyway because I'm not super stoked about the alternatives I can think of. The
problem is that I need to go from the USB PHY node to the USB connector somehow,
but there's no way I can see to get from the PHY node to the USB2 controller
it's connected to, unless I'm missing something obvious.

So I see two alternatives:
1. Extend the usb connector binding to add additional ports for PHYs that handle
   vbus detection or something, then extend either the inno2 binding or a more
   general usb PHY binding to add that port definition.
2. Revert to what the downstream vendor kernel does and simply add a boolean
   flag property to the inno usb2phy binding that tells it whether it's
   connected to a USB-C port and should therefore expect vbusdet to remain high.

Let me know if there's any better alternatives I missed. If there's some OF
driver function to enumerate all controllers a PHY is referenced by, then that
would probably work as well, allowing me to just point the HS port to the
controller instead as intended.

If no better solutions exist then I'm partial to 2. While it makes writing
device trees a little more error prone and I don't like vendor properties, it
means we don't set the "all USB-C ports will always have vbusdet pulled high" in
stone, a claim that I am not 100% confident in.

Kind regards,
Nicolas Frattaroli

Re: [PATCH 1/4] dt-bindings: phy: rockchip,inno-usb2phy: add port property

[Rob Herring]

On Fri, Apr 11, 2025 at 04:31:38PM +0200, Nicolas Frattaroli wrote:
> On Thursday, 10 April 2025 23:11:23 Central European Summer Time Rob Herring wrote:
> > On Mon, Apr 07, 2025 at 08:09:14PM +0200, Nicolas Frattaroli wrote:
> > > USB connectors like to have OF graph connections to high-speed related
> > > nodes to do various things. In the case of the RK3576, we can make use
> > > of a port in the usb2 PHY to detect whether the OTG controller is
> > > connected to a type C port and apply some special behaviour accordingly.
> > > 
> > > The usefulness of having different bits of a fully functioning USB stack
> > > point to each other is more general though, and not constrained to
> > > RK3576 at all, even for this use-case.
> > > 
> > > Add a port property to the binding.
> > > 
> > > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > > ---
> > >  Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
> > > index 6a7ef556414cebad63c10de754778f84fd4486ee..3a662bfc353250a8ad9386ebb5575d1e84c1b5ba 100644
> > > --- a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
> > > +++ b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
> > > @@ -78,6 +78,11 @@ properties:
> > >        When set the driver will request its phandle as one companion-grf
> > >        for some special SoCs (e.g rv1108).
> > >  
> > > +  port:
> > > +    $ref: /schemas/graph.yaml#/properties/port
> > > +    description:
> > > +      A port node to link the PHY to a USB connector's "high-speed" port.
> > 
> > I don't think this is correct. The HS port of the connector goes to the 
> > controller. The controller has the link to the phy.
> > 
> > If the PHY is also what handles USB-C muxing or orientation switching, 
> > then it might have ports, but then it needs input and output ports.
> > 
> > Rob
> > 
> 
> Hi Rob,
> 
> thank you for the quick response.
> 
> I've feared this would be the case, but chose to go ahead with this solution
> anyway because I'm not super stoked about the alternatives I can think of. The
> problem is that I need to go from the USB PHY node to the USB connector somehow,
> but there's no way I can see to get from the PHY node to the USB2 controller
> it's connected to, unless I'm missing something obvious.
> 
> So I see two alternatives:
> 1. Extend the usb connector binding to add additional ports for PHYs that handle
>    vbus detection or something, then extend either the inno2 binding or a more
>    general usb PHY binding to add that port definition.
> 2. Revert to what the downstream vendor kernel does and simply add a boolean
>    flag property to the inno usb2phy binding that tells it whether it's
>    connected to a USB-C port and should therefore expect vbusdet to remain high.
> 
> Let me know if there's any better alternatives I missed. If there's some OF
> driver function to enumerate all controllers a PHY is referenced by, then that
> would probably work as well, allowing me to just point the HS port to the
> controller instead as intended.

The building blocks are there. You can iterate over nodes with 'phys' 
with for_each_node_with_property(), then on each entry in 'phys' check 
if it matches your node. Then you need to iterate over the ports to 
check for connection to usb-c-connector. 
of_graph_get_remote_port_parent() will help you there. Not terribly 
efficient, but you're only doing it once.

Another option is extend phy modes to distinguish USB-C or not. Then you 
can set the mode either with the 'phy-mode' property or in phy cells. 
Though if you have to add a cell, that's an ABI change (not sure if the 
existing kernel would accept another cell).

I rather see the kernel use the information that's already there rather 
than have 2 sources of information.

Rob