Re: [PATCH v4 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] Missing `interconnects` property in USB DWC3 nodes
- [Low] Device tree nodes are not sorted by unit address
--

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

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

Add support for both USB controllers and their respective phys on Shikra.

> diff --git a/arch/arm64/boot/dts/qcom/shikra.dtsi b/arch/arm64/boot/dts/qcom/shikra.dtsi
> index 4e5bc9e17c8ed..7f5d584fc17ba 100644
> --- 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";
> +			reg = <0x0 0x01613000 0x0 0x180>;

[ ... ]

> +		usb_2_hsphy: phy@1617000 {
> +			compatible = "qcom,shikra-qusb2-phy";
> +			reg = <0x0 0x01617000 0x0 0x180>;
> +
> +			clocks = <&gcc GCC_AHB2PHY_USB_CLK>,
> +				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
> +			clock-names = "cfg_ahb", "ref";
> +
> +			resets = <&gcc GCC_QUSB2PHY_SEC_BCR>;
> +			nvmem-cells = <&qusb2_hstx_trim_2>;
> +			#phy-cells = <0>;
> +
> +			status = "disabled";
> +		};
> +
> +		usb_qmpphy: phy@1615000 {
> +			compatible = "qcom,shikra-qmp-usb3-dp-phy";
> +			reg = <0x0 0x01615000 0x0 0x2000>;

[Severity: Low]
This isn't a bug, but are these nodes sorted correctly by unit address?

The usb_2_hsphy node at 1617000 is placed before the usb_qmpphy node at
1615000, which doesn't follow the convention to sort sibling nodes
alphanumerically by their unit address.

[ ... ]

> @@ -493,6 +573,11 @@ qusb2_hstx_trim_1: hstx-trim@25b {
>  				bits = <1 4>;
>  			};
>  
> +			qusb2_hstx_trim_2: hstx-trim@25a {
> +				reg = <0x25a 0x1>;
> +				bits = <4 4>;
> +			};

[Severity: Low]
This isn't a bug, but should hstx-trim@25a be placed before hstx-trim@25b
to maintain alphanumeric sorting by unit address?

[ ... ]

> @@ -640,6 +725,142 @@ &clk_virt SLAVE_QUP_CORE_0 RPM_ALWAYS_TAG>,
>  			};
>  		};
>  
> +		usb_2: usb@4c00000 {
> +			compatible = "qcom,shikra-dwc3", "qcom,snps-dwc3";
> +			reg = <0x0 0x04c00000 0x0 0xfc100>;

[Severity: High]
Does this node need an interconnects property?

Without interconnects specified, the DWC3 core driver won't vote for
bandwidth. If other consumers drop their votes, the system NoC might throttle
or gate the path, potentially causing USB DMA transfer stalls, device
disconnects, or system freezes.

Since of_icc_get returns NULL when the property is missing, the driver
will silently skip voting without showing probe errors.

[ ... ]

> +		usb_1: usb@4e00000 {
> +			compatible = "qcom,shikra-dwc3", "qcom,snps-dwc3";
> +			reg = <0x0 0x04e00000 0x0 0xfc100>;

[Severity: High]
Is the interconnects property missing here as well?

Similar to the usb_2 node, omitting interconnects might expose the hardware
to starvation if the system NoC throttles or gates the path when other
devices become idle.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-usb-shikra-dt-v4-v4-0-2e7a092d7f27@oss.qualcomm.com?part=1