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

[Frank Li <Frank.li@oss.nxp.com>]

On Mon, Oct 26, 2026 at 06:44:28PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] arm64: dts: imx95-navq: usb2 dr_mode = host breaks usb-role-switch
> --
>
> commit 7aa413a1299c4bee8b5a2613c0acf6256cd30c16
> 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 imx95-navq board.
>
> > diff --git a/arch/arm64/boot/dts/freescale/imx95-navq.dts b/arch/arm64/boot/dts/freescale/imx95-navq.dts
> > index 7860c113e9e02..5fe245b623068 100644
> > --- a/arch/arm64/boot/dts/freescale/imx95-navq.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx95-navq.dts
>
> [ ... ]
>
> > @@ -80,6 +82,72 @@ i2c3_gpio_expander_21: gpio-expander@21 {
>
> [ ... ]
>
> > +	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>;
> > +				};
> > +			};
> > +		};
> > +	};
> >  };
>
> [ ... ]
>
> > @@ -198,6 +270,55 @@ pinctrl_usdhc2: usdhc2grp {
> >  	};
> >  };
> >
> > +&usb2 {
> > +	disable-over-current;
> > +	dr_mode = "host";
> > +	usb-role-switch;
> > +
> > +	status = "okay";
> > +
> > +	port {
> > +		usb2_data_hs: endpoint {
> > +			remote-endpoint = <&typec2_con_hs>;
> > +		};
> > +	};
> > +};
>
> [Severity: Medium]
> Should usb2 use dr_mode = "otg" here instead of "host" to match the
> usb-role-switch property and the connector's data-role = "dual" in
> ptn5110_usb2, or should ptn5110_usb2 use data-role = "host" without
> usb-role-switch on usb2?

if use type C port, suppose it should be dual role device.

Frank

>
> When dr_mode is set to "host", ci_hdrc_probe() in drivers/usb/chipidea/core.c
> skips initializing the gadget role and OTG handling:
>
> 	if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) {
> 		ret = ci_hdrc_gadget_init(ci);
> 		...
> 	}
> 	...
> 	if (ci->is_otg && ci->roles[CI_ROLE_GADGET]) {
> 		ret = ci_hdrc_otg_init(ci);
> 		...
> 	}
>
> Because ci->roles[CI_ROLE_GADGET] remains NULL, ci_get_role() never enables
> OTGSC_IDIE:
>
> 	if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
> 		if (ci->is_otg) {
> 			role = ci_otg_role(ci);
> 			hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> 		} else {
>
> As a result, id_extcon.enabled and vbus_extcon.enabled remain false. Although
> the Type-C port manager configures ptn5110_usb2 for dual-role data and accepts
> USB PD data role swaps, calls along usb_role_switch_set_role() ->
> ci_usb_role_switch_set() -> ci_irq() -> ci_irq_handler() will have OTGSC_IDIE
> and OTGSC_BSVIE masked out by hw_read_otgsc(), leaving role switching
> non-functional and the controller permanently in host mode.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261026182150.11330-1-laurentiumihalcea111@gmail.com?part=1