diff mbox series

[v4,4/6] arm64: dts: qcom: ipq5332: Add USB related nodes

Message ID 1f99805b6437aa8d6eaa4663e8d27b98ee595f00.1689160067.git.quic_varada@quicinc.com
State New
Headers
Series Enable IPQ5332 USB2 |

Commit Message

Varadarajan Narayanan July 12, 2023, 11:38 a.m. UTC 
  Add USB phy and controller nodes.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v4:
	Change node name
	Remove blank line
	'make CHECK_DTBS=y DT_SCHEMA_FILES=qcom qcom/ipq5332-rdp441.dtb' passed
v1:
	Rename phy node
	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 | 53 +++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

Comments

Krzysztof Kozlowski July 12, 2023, 12:04 p.m. UTC | #1 
On 12/07/2023 13:38, Varadarajan Narayanan wrote:
> Add USB phy and controller nodes.
> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v4:
> 	Change node name
> 	Remove blank line
> 	'make CHECK_DTBS=y DT_SCHEMA_FILES=qcom qcom/ipq5332-rdp441.dtb' passed
> v1:
> 	Rename phy node
> 	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 | 53 +++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> index 8bfc2db..8118356 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> @@ -405,6 +405,59 @@
>  				status = "disabled";
>  			};
>  		};
> +
> +		usbphy0: usb-phy@7b000 {
> +			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 {

So you responded to my comments, wait ten minutes and send v2? No need
to wait for my feedback, right?

No, it's not ok. This is "usb", not "usb2". Are you saying you have
second device with the same address?

Best regards,
Krzysztof
Dmitry Baryshkov July 12, 2023, 12:28 p.m. UTC | #2 
On 12/07/2023 15:04, Krzysztof Kozlowski wrote:
> On 12/07/2023 13:38, Varadarajan Narayanan wrote:
>> Add USB phy and controller nodes.
>>
>> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
>> ---
>> v4:
>> 	Change node name
>> 	Remove blank line
>> 	'make CHECK_DTBS=y DT_SCHEMA_FILES=qcom qcom/ipq5332-rdp441.dtb' passed
>> v1:
>> 	Rename phy node
>> 	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 | 53 +++++++++++++++++++++++++++++++++++
>>   1 file changed, 53 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>> index 8bfc2db..8118356 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>> @@ -405,6 +405,59 @@
>>   				status = "disabled";
>>   			};
>>   		};
>> +
>> +		usbphy0: usb-phy@7b000 {
>> +			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 {
> 
> So you responded to my comments, wait ten minutes and send v2? No need
> to wait for my feedback, right?
> 
> No, it's not ok. This is "usb", not "usb2". Are you saying you have
> second device with the same address?

Just to emphasise, it's the node name `usb2', which is not fine. DT 
label `usb2' is (hopefully) fine.
Varadarajan Narayanan July 13, 2023, 4:14 a.m. UTC | #3 
On Wed, Jul 12, 2023 at 03:28:43PM +0300, Dmitry Baryshkov wrote:
> On 12/07/2023 15:04, Krzysztof Kozlowski wrote:
> >On 12/07/2023 13:38, Varadarajan Narayanan wrote:
> >>Add USB phy and controller nodes.
> >>
> >>Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> >>---
> >>v4:
> >>	Change node name
> >>	Remove blank line
> >>	'make CHECK_DTBS=y DT_SCHEMA_FILES=qcom qcom/ipq5332-rdp441.dtb' passed
> >>v1:
> >>	Rename phy node
> >>	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 | 53 +++++++++++++++++++++++++++++++++++
> >>  1 file changed, 53 insertions(+)
> >>
> >>diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> >>index 8bfc2db..8118356 100644
> >>--- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> >>+++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> >>@@ -405,6 +405,59 @@
> >>  				status = "disabled";
> >>  			};
> >>  		};
> >>+
> >>+		usbphy0: usb-phy@7b000 {
> >>+			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 {
> >
> >So you responded to my comments, wait ten minutes and send v2? No need
> >to wait for my feedback, right?
> >
> >No, it's not ok. This is "usb", not "usb2". Are you saying you have
> >second device with the same address?
>
> Just to emphasise, it's the node name `usb2', which is not fine. DT label
> `usb2' is (hopefully) fine.

Thanks for the clarification. Will post a new patch.

-Varada
Konrad Dybcio July 15, 2023, 12:36 p.m. UTC | #4 
On 12.07.2023 13:38, Varadarajan Narayanan wrote:
> Add USB phy and controller nodes.
> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v4:
> 	Change node name
> 	Remove blank line
> 	'make CHECK_DTBS=y DT_SCHEMA_FILES=qcom qcom/ipq5332-rdp441.dtb' passed
DT_SCHEMA_FILES accepts yaml files

Konrad
> v1:
> 	Rename phy node
> 	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 | 53 +++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> index 8bfc2db..8118356 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> @@ -405,6 +405,59 @@
>  				status = "disabled";
>  			};
>  		};
> +
> +		usbphy0: usb-phy@7b000 {
> +			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 {
> +			compatible = "qcom,ipq5332-dwc3", "qcom,dwc3";
> +			reg = <0x08af8800 0x400>;
> +
> +			interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "hs_phy_irq";
> +
> +			clocks = <&gcc GCC_USB0_MASTER_CLK>,
> +				 <&gcc GCC_SNOC_USB_CLK>,
> +				 <&gcc GCC_USB0_SLEEP_CLK>,
> +				 <&gcc GCC_USB0_MOCK_UTMI_CLK>;
> +			clock-names = "core",
> +				      "iface",
> +				      "sleep",
> +				      "mock_utmi";
> +
> +			resets = <&gcc GCC_USB_BCR>;
> +
> +			qcom,select-utmi-as-pipe-clk;
> +
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			status = "disabled";
> +
> +			usb2_0_dwc: usb@8a00000 {
> +				compatible = "snps,dwc3";
> +				reg = <0x08a00000 0xe000>;
> +				clocks = <&gcc GCC_USB0_MOCK_UTMI_CLK>;
> +				clock-names = "ref";
> +				interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
> +				usb-phy = <&usbphy0>;
> +				tx-fifo-resize;
> +				snps,is-utmi-l1-suspend;
> +				snps,hird-threshold = /bits/ 8 <0x0>;
> +				snps,dis_u2_susphy_quirk;
> +				snps,dis_u3_susphy_quirk;
> +			};
> +		};
>  	};
>  
>  	timer {
Varadarajan Narayanan July 18, 2023, 8:14 a.m. UTC | #5 
On Sat, Jul 15, 2023 at 02:36:18PM +0200, Konrad Dybcio wrote:
> On 12.07.2023 13:38, Varadarajan Narayanan wrote:
> > Add USB phy and controller nodes.
> >
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> > v4:
> > 	Change node name
> > 	Remove blank line
> > 	'make CHECK_DTBS=y DT_SCHEMA_FILES=qcom qcom/ipq5332-rdp441.dtb' passed
> DT_SCHEMA_FILES accepts yaml files

I followed the example given in https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/

		make CHECK_DTBS=y DT_SCHEMA_FILES=trivial-devices.yaml qcom/sm8450-hdk.dtb
		make CHECK_DTBS=y DT_SCHEMA_FILES=/gpio/ qcom/sm8450-hdk.dtb
	---->	make CHECK_DTBS=y DT_SCHEMA_FILES=qcom qcom/sm8450-hdk.dtb

Will include the yaml from next time.

Thanks
Varada

> Konrad
> > v1:
> > 	Rename phy node
> > 	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 | 53 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 53 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> > index 8bfc2db..8118356 100644
> > --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> > @@ -405,6 +405,59 @@
> >  				status = "disabled";
> >  			};
> >  		};
> > +
> > +		usbphy0: usb-phy@7b000 {
> > +			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 {
> > +			compatible = "qcom,ipq5332-dwc3", "qcom,dwc3";
> > +			reg = <0x08af8800 0x400>;
> > +
> > +			interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
> > +			interrupt-names = "hs_phy_irq";
> > +
> > +			clocks = <&gcc GCC_USB0_MASTER_CLK>,
> > +				 <&gcc GCC_SNOC_USB_CLK>,
> > +				 <&gcc GCC_USB0_SLEEP_CLK>,
> > +				 <&gcc GCC_USB0_MOCK_UTMI_CLK>;
> > +			clock-names = "core",
> > +				      "iface",
> > +				      "sleep",
> > +				      "mock_utmi";
> > +
> > +			resets = <&gcc GCC_USB_BCR>;
> > +
> > +			qcom,select-utmi-as-pipe-clk;
> > +
> > +			#address-cells = <1>;
> > +			#size-cells = <1>;
> > +			ranges;
> > +
> > +			status = "disabled";
> > +
> > +			usb2_0_dwc: usb@8a00000 {
> > +				compatible = "snps,dwc3";
> > +				reg = <0x08a00000 0xe000>;
> > +				clocks = <&gcc GCC_USB0_MOCK_UTMI_CLK>;
> > +				clock-names = "ref";
> > +				interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
> > +				usb-phy = <&usbphy0>;
> > +				tx-fifo-resize;
> > +				snps,is-utmi-l1-suspend;
> > +				snps,hird-threshold = /bits/ 8 <0x0>;
> > +				snps,dis_u2_susphy_quirk;
> > +				snps,dis_u3_susphy_quirk;
> > +			};
> > +		};
> >  	};
> >
> >  	timer {
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
index 8bfc2db..8118356 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
@@ -405,6 +405,59 @@ 
 				status = "disabled";
 			};
 		};
+
+		usbphy0: usb-phy@7b000 {
+			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 {
+			compatible = "qcom,ipq5332-dwc3", "qcom,dwc3";
+			reg = <0x08af8800 0x400>;
+
+			interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "hs_phy_irq";
+
+			clocks = <&gcc GCC_USB0_MASTER_CLK>,
+				 <&gcc GCC_SNOC_USB_CLK>,
+				 <&gcc GCC_USB0_SLEEP_CLK>,
+				 <&gcc GCC_USB0_MOCK_UTMI_CLK>;
+			clock-names = "core",
+				      "iface",
+				      "sleep",
+				      "mock_utmi";
+
+			resets = <&gcc GCC_USB_BCR>;
+
+			qcom,select-utmi-as-pipe-clk;
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+
+			status = "disabled";
+
+			usb2_0_dwc: usb@8a00000 {
+				compatible = "snps,dwc3";
+				reg = <0x08a00000 0xe000>;
+				clocks = <&gcc GCC_USB0_MOCK_UTMI_CLK>;
+				clock-names = "ref";
+				interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
+				usb-phy = <&usbphy0>;
+				tx-fifo-resize;
+				snps,is-utmi-l1-suspend;
+				snps,hird-threshold = /bits/ 8 <0x0>;
+				snps,dis_u2_susphy_quirk;
+				snps,dis_u3_susphy_quirk;
+			};
+		};
 	};
 
 	timer {