[PATCH v2] arm64: dts: imx95-navq: add USB1/USB2 nodes

[PATCH v2] arm64: dts: imx95-navq: add USB1/USB2 nodes

[Laurentiu Mihalcea]

From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

Add and enable the DT nodes for USB1 and USB2.

Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
---
v1 -> v2:

* change USB2 DR mode to OTG to match TypeC connector configuration. The
controller should actually support the software-based host <-> device
switch.

---
 arch/arm64/boot/dts/freescale/imx95-navq.dts | 121 +++++++++++++++++++
 1 file changed, 121 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx95-navq.dts b/arch/arm64/boot/dts/freescale/imx95-navq.dts
index 7860c113e9e0..b668c752905b 100644
--- a/arch/arm64/boot/dts/freescale/imx95-navq.dts
+++ b/arch/arm64/boot/dts/freescale/imx95-navq.dts
@@ -5,6 +5,8 @@
 
 /dts-v1/;
 
+#include <dt-bindings/usb/pd.h>
+
 #include "imx95.dtsi"
 
 / {
@@ -80,6 +82,72 @@ i2c3_gpio_expander_21: gpio-expander@21 {
 		pinctrl-0 = <&pinctrl_i2c3_gpio_expander_21>;
 		pinctrl-names = "default";
 	};
+
+	ptn5110_usb3: tcpc@50 {
+		compatible = "nxp,ptn5110", "tcpci";
+		reg = <0x50>;
+		interrupt-parent = <&gpio4>;
+		/* USB_PTN5110_ALERT_B - shared IRQ line with tcpc@51 */
+		interrupts = <25 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-0 = <&pinctrl_typec>;
+		pinctrl-names = "default";
+
+		/* no SINK circuit (EN_SINK1 tied to TP32) */
+		connector {
+			compatible = "usb-c-connector";
+			label = "USB1 3.0 Type C";
+			data-role = "dual";
+			power-role = "source";
+			self-powered;
+			source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+
+					typec1_con_hs: endpoint {
+						remote-endpoint = <&usb3_data_hs>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+
+					typec1_con_ss: endpoint {
+						remote-endpoint = <&usb3_data_ss>;
+					};
+				};
+			};
+		};
+	};
+
+	ptn5110_usb2: tcpc@51 {
+		compatible = "nxp,ptn5110", "tcpci";
+		reg = <0x51>;
+		interrupt-parent = <&gpio4>;
+		interrupts = <25 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-0 = <&pinctrl_typec>;
+		pinctrl-names = "default";
+
+		/* no SINK circuit (EN_SINK1 tied to TP30) */
+		connector {
+			compatible = "usb-c-connector";
+			label = "USB2 2.0 Type C";
+			data-role = "dual";
+			power-role = "source";
+			self-powered;
+			source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
+
+			port {
+				typec2_con_hs: endpoint {
+					remote-endpoint = <&usb2_data_hs>;
+				};
+			};
+		};
+	};
 };
 
 &lpuart1 {
@@ -145,6 +213,10 @@ pinctrl_reg_usdhc2_vmmc: regusdhc2vmmcgrp {
 		fsl,pins = <IMX95_PAD_SD2_RESET_B__GPIO3_IO_BIT7			0x31e>;
 	};
 
+	pinctrl_typec: typecgrp {
+		fsl,pins = <IMX95_PAD_ENET2_RD1__GPIO4_IO_BIT25				0x31e>;
+	};
+
 	pinctrl_uart1: uart1grp {
 		bootph-pre-ram;
 		fsl,pins = <IMX95_PAD_UART1_RXD__AONMIX_TOP_LPUART1_RX			0x31e>,
@@ -198,6 +270,55 @@ pinctrl_usdhc2: usdhc2grp {
 	};
 };
 
+&usb2 {
+	disable-over-current;
+	dr_mode = "otg";
+	usb-role-switch;
+
+	status = "okay";
+
+	port {
+		usb2_data_hs: endpoint {
+			remote-endpoint = <&typec2_con_hs>;
+		};
+	};
+};
+
+&usb3_dwc3 {
+	adp-disable;
+	dr_mode = "otg";
+	hnp-disable;
+	srp-disable;
+	usb-role-switch;
+	snps,dis-u1-entry-quirk;
+	snps,dis-u2-entry-quirk;
+
+	status = "okay";
+
+	port {
+		usb3_data_hs: endpoint {
+			remote-endpoint = <&typec1_con_hs>;
+		};
+	};
+};
+
+&usb3_phy {
+	orientation-switch;
+	fsl,phy-pcs-tx-deemph-3p5db-attenuation-db = <17>;
+	fsl,phy-pcs-tx-swing-full-percent = <100>;
+	fsl,phy-tx-preemp-amp-tune-microamp = <600>;
+	fsl,phy-tx-vboost-level-microvolt = <1156>;
+	fsl,phy-tx-vref-tune-percent = <100>;
+
+	status = "okay";
+
+	port {
+		usb3_data_ss: endpoint {
+			remote-endpoint = <&typec1_con_ss>;
+		};
+	};
+};
+
 &usdhc1 {
 	bootph-pre-ram;
 	bus-width = <8>;
-- 
2.53.0

Re: [PATCH v2] arm64: dts: imx95-navq: add USB1/USB2 nodes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] arm64: dts: imx95-navq: missing OTG capability disables on usb2 node
--

commit 1ff89e7196fb33692fe862b6274ad535b519c7f7
Author: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

arm64: dts: imx95-navq: add USB1/USB2 nodes

This commit adds and enables the device tree nodes for the USB1 and USB2
controllers and their Type-C port controllers on the i.MX95 NavQ board.

> diff --git a/arch/arm64/boot/dts/freescale/imx95-navq.dts b/arch/arm64/boot/dts/freescale/imx95-navq.dts
> index 7860c113e9e02..b668c752905bf 100644
> --- a/arch/arm64/boot/dts/freescale/imx95-navq.dts
> +++ b/arch/arm64/boot/dts/freescale/imx95-navq.dts

[ ... ]

> @@ -198,6 +270,55 @@ pinctrl_usdhc2: usdhc2grp {
>  	};
>  };
>  
> +&usb2 {
> +	disable-over-current;
> +	dr_mode = "otg";
> +	usb-role-switch;
> +
> +	status = "okay";

[Severity: Medium]
Should the &usb2 node also include adp-disable, hnp-disable, and
srp-disable, matching &usb3_dwc3 below?

In drivers/usb/chipidea/core.c:ci_get_platdata(), when dr_mode is set to
"otg", the ChipIdea driver enables hnp_support and srp_support by default
before calling of_usb_update_otg_caps():

	if (platdata->dr_mode == USB_DR_MODE_OTG) {
		/* We can support HNP and SRP of OTG 2.0 */
		platdata->ci_otg_caps.otg_rev = 0x0200;
		platdata->ci_otg_caps.hnp_support = true;
		platdata->ci_otg_caps.srp_support = true;

		/* Update otg capabilities by DT properties */
		ret = of_usb_update_otg_caps(dev->of_node,
					&platdata->ci_otg_caps);
		if (ret)
			return ret;
	}

Without those disable properties in the device tree, hnp_support and
srp_support remain true. In drivers/usb/chipidea/udc.c:udc_start(), this
sets ci->gadget.is_otg = 1 on a Type-C port, and when CONFIG_USB_OTG_FSM is
enabled, drivers/usb/chipidea/ci.h:ci_otg_is_fsm_mode() evaluates to true:

	return ci->is_otg && ci->roles[CI_ROLE_HOST] &&
		ci->roles[CI_ROLE_GADGET] && (otg_caps->srp_support ||
		otg_caps->hnp_support || otg_caps->adp_support);

Entering OTG FSM mode routes role changes to ci_otg_fsm_irq() instead of
ci_handle_id_switch() and ci_handle_vbus_change(). When TCPM calls
ci_usb_role_switch_set(), both id_extcon.changed (OTGSC_IDIS) and
vbus_extcon.changed (OTGSC_BSVIS) can be set together before invoking
ci_irq(), but drivers/usb/chipidea/otg_fsm.c:ci_otg_fsm_irq() handles them
in a mutually exclusive if / else if chain:

	if (otg_int_src) {
		if (otg_int_src & OTGSC_DPIS) {
			hw_write_otgsc(ci, OTGSC_DPIS, OTGSC_DPIS);
			fsm->a_srp_det = 1;
			fsm->a_bus_drop = 0;
		} else if (otg_int_src & OTGSC_IDIS) {
			hw_write_otgsc(ci, OTGSC_IDIS, OTGSC_IDIS);
			if (fsm->id == 0) {
				fsm->a_bus_drop = 0;
				fsm->a_bus_req = 1;
				ci->id_event = true;
			}
		} else if (otg_int_src & OTGSC_BSVIS) {

Could this cause OTGSC_BSVIS to be dropped on role transitions and prevent
software-driven host/device role switching from working properly on &usb2?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027131630.575-1-laurentiumihalcea111@gmail.com?part=1