Re: [PATCH 2/2] arm64: dts: imx8qm-ss-conn: Add HSIC usb@5b0e0000 node

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The `usbphynop2` node requests the wrong power domain (`IMX_SC_R_USB_0_PHY` instead of `IMX_SC_R_USB_1_PHY`).
- [Medium] The `usbh1` node is enabled by default (missing `status = "disabled";`), violating SoC `.dtsi` practices and causing unconditional probing.
- [Low] Missing required `#phy-cells` property in the `usbphynop2` node.
--

--- Patch [2]: [PATCH 2/2] arm64: dts: imx8qm-ss-conn: Add HSIC usb@5b0e0000 node ---
commit 9497d73ee79f69a11e0a9347cd596ca4c9842914
Author: Frank Li <Frank.Li@nxp.com>

arm64: dts: imx8qm-ss-conn: Add HSIC usb@5b0e0000 node

Add usb@5b0e0000 and related node for i.MX8QM.

> diff --git a/arch/arm64/boot/dts/freescale/imx8qm-ss-conn.dtsi b/arch/arm64/boot/dts/freescale/imx8qm-ss-conn.dtsi
> index 1370a571153a5..c7063aaac4a34 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qm-ss-conn.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qm-ss-conn.dtsi
> @@ -4,6 +4,40 @@
>   *	Dong Aisheng <aisheng.dong@nxp.com>
>   */
>  
> +&{/} {
> +	usbphynop2: usbphynop2 {
> +		compatible = "usb-nop-xceiv";

[Severity: Low]
Does this node need a #phy-cells = <0>; property?  The usb-nop-xceiv.yaml
binding schema typically requires this property to be present, and omitting
it might cause dtbs_check validation failures during the build.

> +		clocks = <&usb2_lpcg IMX_LPCG_CLK_7>;
> +		clock-names = "main_clk";
> +		power-domains = <&pd IMX_SC_R_USB_0_PHY>;

[Severity: High]
Is IMX_SC_R_USB_0_PHY the correct power domain for usbphynop2?  Since this is
for the USB_1 controller (usbh1), should this be IMX_SC_R_USB_1_PHY instead?

If it requests the USB_0 PHY domain, the USB_1 PHY hardware might remain
unpowered, causing the HSIC port to fail.

> +		status = "disabled";
> +	};
> +};
> +
> +&conn_subsys {
> +	usbh1: usb@5b0e0000 {
> +		compatible = "fsl,imx7ulp-usb", "fsl,imx6ul-usb", "fsl,imx27-usb";
> +		reg = <0x5b0e0000 0x200>;

[Severity: Medium]
Should this node include a status = "disabled"; property?  Without it, the
usbh1 node is enabled by default and will probe unconditionally on all i.MX8QM
boards, even if the hardware interface isn't wired up.

Since the required usbphynop2 PHY is disabled, the controller might fail to
initialize, leading to boot delays and unnecessary resource allocation.

> +		interrupt-parent = <&gic>;
> +		interrupts = <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-qm_smmu-v1-0-3689fbd8b516@nxp.com?part=2