Re: [PATCH v3 1/4] arm64: dts: qcom: Add support for usb nodes on Shikra

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The 'usb_2' node incorrectly assigns the Configuration NOC clock to the 'iface' clock instead of the System NOC clock.
- [High] The newly added USB PHY nodes specify SoC-specific compatible strings without driver support, and the QMP PHY uses non-standard clock/reset names causing fatal probe failures.
--

--- Patch [1]: [PATCH v3 1/4] arm64: dts: qcom: Add support for usb nodes on Shikra ---
commit 9325329fd1f5c498b9188ae6a3b6adc14bd363e9
Author: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>

arm64: dts: qcom: Add support for usb nodes on Shikra

This commit adds support for both USB controllers and their respective phys
on the Shikra platform.

> diff --git a/arch/arm64/boot/dts/qcom/shikra.dtsi b/arch/arm64/boot/dts/qcom/shikra.dtsi
> --- a/arch/arm64/boot/dts/qcom/shikra.dtsi
> +++ b/arch/arm64/boot/dts/qcom/shikra.dtsi
> @@ -442,6 +443,85 @@ gcc: clock-controller@1400000 {
>  			#power-domain-cells = <1>;
>  		};
>  
> +		usb_1_hsphy: phy@1613000 {
> +			compatible = "qcom,shikra-qusb2-phy";

[Severity: High]
Does this compatible string have driver support merged, or should it include
a generic fallback? Using an SoC-specific compatible without a matching driver
implementation will prevent the PHY driver from probing.

[ ... ]

> +		usb_qmpphy: phy@1615000 {
> +			compatible = "qcom,shikra-qmp-usb3-dp-phy";
> +			reg = <0x0 0x01615000 0x0 0x2000>;
> +
> +			clocks = <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
> +				 <&gcc GCC_USB3_PRIM_CLKREF_EN>,
> +				 <&gcc GCC_AHB2PHY_USB_CLK>,
> +				 <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> +			clock-names = "aux",
> +				      "ref",
> +				      "cfg_ahb",
> +				      "pipe";

[Severity: High]
Should the clock name "pipe" be "usb3_pipe" instead?

The upstream QMP combo PHY driver strictly looks up "usb3_pipe" using
devm_clk_get(), and failing to find it will cause a probe failure with
-ENOENT.

> +
> +			resets = <&gcc GCC_USB3PHY_PHY_PRIM_SP0_BCR>,
> +				 <&gcc GCC_USB3_DP_PHY_PRIM_BCR>,
> +				 <&gcc GCC_USB3_PHY_PRIM_SP0_BCR>;
> +			reset-names = "phy_phy",
> +				      "dp_phy",
> +				      "phy";

[Severity: High]
Do these reset names deviate from what the QMP combo PHY driver expects?

The driver typically requires "phy" and "common". Deviating from this
list may prevent successful probing of the PHY and the dependent DWC3
controller, causing the USB subsystem to break.

[ ... ]

> +		usb_2: usb@4c00000 {
> +			compatible = "qcom,shikra-dwc3", "qcom,snps-dwc3";
> +			reg = <0x0 0x04c00000 0x0 0xfc100>;
> +
> +			clocks = <&gcc GCC_CFG_NOC_USB2_PRIM_AXI_CLK>,
> +				 <&gcc GCC_USB20_MASTER_CLK>,
> +				 <&gcc GCC_CFG_NOC_USB2_PRIM_AXI_CLK>,

[Severity: High]
Is the Configuration NOC clock intentionally duplicated here for the "iface"
slot?

The "iface" clock index usually expects the System NOC AXI interconnect clock.
Leaving the System NOC clock unvoted during USB2 operations could lead to
unclocked AXI transactions, potentially causing silent DMA failures or severe
interconnect bus hangs when the controller attempts to access system memory.

> +				 <&gcc GCC_USB20_SLEEP_CLK>,
> +				 <&gcc GCC_USB20_MOCK_UTMI_CLK>;
> +			clock-names = "cfg_noc",
> +				      "core",
> +				      "iface",

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716-shikra-usb-dt-v3-v3-0-13f73a0571bb@oss.qualcomm.com?part=1