Re: [PATCH v3 4/6] arm64: dts: qcom: ipq5332: Add USB related nodes

On Tue, Jul 11, 2023 at 11:01:03AM +0200, Krzysztof Kozlowski wrote:

On 11/07/2023 10:51, Varadarajan Narayanan wrote:

quoted

Add USB phy and controller nodes.

Signed-off-by: Varadarajan Narayanan <redacted>
---
v1:
	Rename phy node

I don't see any improvements.

Will fix and post a new patch

quoted

	Change compatible from m31,ipq5332-usb-hsphy -> qcom,ipq5332-usb-hsphy
	Remove 'qscratch' from phy node
	Fix alignment and upper-case hex no.s
	Add clock definition for the phy
	Remove snps,ref-clock-period-ns as it is not used. dwc3_ref_clk_period()
	in dwc3/core.c takes the frequency from ref clock and calculates fladj
	as appropriate.
---
 arch/arm64/boot/dts/qcom/ipq5332.dtsi | 54 +++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
index 8bfc2db..c945ff6 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi

@@ -405,6 +405,60 @@
 				status = "disabled";
 			};
 		};
+
+		usbphy0: ipq5332-hsphy@7b000 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

"phy"

Will fix and post a new patch

quoted

+			compatible = "qcom,ipq5332-usb-hsphy";
+			reg = <0x0007b000 0x12c>;
+
+			clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>;
+			clock-names = "cfg_ahb";
+
+			resets = <&gcc GCC_QUSB2_0_PHY_BCR>;
+
+			status = "disabled";
+		};
+
+		usb2: usb2@8a00000 {

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

'make dtbs_check' passed. The '2' in 'usb2' is to indicate USB v2.
There is one more USB v3 controller in this SoC. Hence, to
differentiate between the two used 'usb2'.

Hope that is ok.

quoted

+			compatible = "qcom,ipq5332-dwc3", "qcom,dwc3";
+

No need for blank line.

Will remove.

Thanks
Varada

quoted

+			reg = <0x08af8800 0x400>;
+
+			interrupts = <GIC_SPI 62 IRQ_

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel