diff mbox series

[3/8] arm64: dts: qcom: ipq5332: Add USB Super-Speed PHY node

Message ID 20230929084209.3033093-4-quic_ipkumar@quicinc.com (mailing list archive)
State Changes Requested
Headers show
Series Enable USB3 for Qualcomm IPQ5332 | expand

Commit Message

Praveenkumar I Sept. 29, 2023, 8:42 a.m. UTC 
Add USB Super-Speed UNIPHY node and populate the phandle on
gcc node for the parent clock map.

Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
---
 arch/arm64/boot/dts/qcom/ipq5332.dtsi | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Comments

Konrad Dybcio Sept. 29, 2023, 12:56 p.m. UTC | #1 
On 29.09.2023 10:42, Praveenkumar I wrote:
> Add USB Super-Speed UNIPHY node and populate the phandle on
> gcc node for the parent clock map.
> 
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/ipq5332.dtsi | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> index d3fef2f80a81..b08ffd8c094e 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> @@ -158,6 +158,29 @@ usbphy0: phy@7b000 {
>  			status = "disabled";
>  		};
>  
> +		usbphy1: phy@4b0000 {
> +			compatible = "qcom,ipq5332-usb-uniphy";
> +			reg = <0x4b0000 0x800>;
Please pad the address part to 8 hex digits with leading zeroes.

> +
> +			clocks = <&gcc GCC_PCIE3X1_PHY_AHB_CLK>,
> +				 <&gcc GCC_USB0_PHY_CFG_AHB_CLK>,
> +				 <&gcc GCC_USB0_PIPE_CLK>;
> +			clock-names = "ahb",
> +				      "cfg_ahb",
> +				      "pipe";
> +
> +			resets =  <&gcc GCC_USB0_PHY_BCR>;
Looks like there's a double space after '='

Konrad
Praveenkumar I Sept. 29, 2023, 1:30 p.m. UTC | #2 
On 9/29/2023 6:26 PM, Konrad Dybcio wrote:
> On 29.09.2023 10:42, Praveenkumar I wrote:
>> Add USB Super-Speed UNIPHY node and populate the phandle on
>> gcc node for the parent clock map.
>>
>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/ipq5332.dtsi | 25 ++++++++++++++++++++++++-
>>   1 file changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>> index d3fef2f80a81..b08ffd8c094e 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>> @@ -158,6 +158,29 @@ usbphy0: phy@7b000 {
>>   			status = "disabled";
>>   		};
>>   
>> +		usbphy1: phy@4b0000 {
>> +			compatible = "qcom,ipq5332-usb-uniphy";
>> +			reg = <0x4b0000 0x800>;
> Please pad the address part to 8 hex digits with leading zeroes.
Sure, will add.
>
>> +
>> +			clocks = <&gcc GCC_PCIE3X1_PHY_AHB_CLK>,
>> +				 <&gcc GCC_USB0_PHY_CFG_AHB_CLK>,
>> +				 <&gcc GCC_USB0_PIPE_CLK>;
>> +			clock-names = "ahb",
>> +				      "cfg_ahb",
>> +				      "pipe";
>> +
>> +			resets =  <&gcc GCC_USB0_PHY_BCR>;
> Looks like there's a double space after '='
Sure, will remove the extra space.

--
Thanks,
Praveenkumar
>
> Konrad
Dmitry Baryshkov Sept. 30, 2023, 5:22 p.m. UTC | #3 
On 29/09/2023 11:42, Praveenkumar I wrote:
> Add USB Super-Speed UNIPHY node and populate the phandle on
> gcc node for the parent clock map.
> 
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
>   arch/arm64/boot/dts/qcom/ipq5332.dtsi | 25 ++++++++++++++++++++++++-
>   1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> index d3fef2f80a81..b08ffd8c094e 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> @@ -158,6 +158,29 @@ usbphy0: phy@7b000 {
>   			status = "disabled";
>   		};
>   
> +		usbphy1: phy@4b0000 {

Are there other USB PHYs on this platform?

> +			compatible = "qcom,ipq5332-usb-uniphy";
> +			reg = <0x4b0000 0x800>;
> +
> +			clocks = <&gcc GCC_PCIE3X1_PHY_AHB_CLK>,
> +				 <&gcc GCC_USB0_PHY_CFG_AHB_CLK>,
> +				 <&gcc GCC_USB0_PIPE_CLK>;
> +			clock-names = "ahb",
> +				      "cfg_ahb",
> +				      "pipe";
> +
> +			resets =  <&gcc GCC_USB0_PHY_BCR>;
> +
> +			#clock-cells = <0>;
> +			clock-output-names = "usb0_pipe_clk_src";

I'm not sure, what is the best approach her. For QMP USB and PCIe PHYs 
we had to use fixed names historically. On the other hand for QMP DP 
clocks we are fine with the generated names. I'd prefer the latter case.

> +
> +			qcom,phy-usb-mux-sel = <&tcsr 0x10540>;
> +
> +			#phy-cells = <0>;
> +
> +			status = "disabled";
> +		};
> +
>   		qfprom: efuse@a4000 {
>   			compatible = "qcom,ipq5332-qfprom", "qcom,qfprom";
>   			reg = <0x000a4000 0x721>;
> @@ -200,7 +223,7 @@ gcc: clock-controller@1800000 {
>   				 <&sleep_clk>,
>   				 <0>,
>   				 <0>,
> -				 <0>;
> +				 <&usbphy1>;
>   		};
>   
>   		tcsr_mutex: hwlock@1905000 {
Praveenkumar I Oct. 3, 2023, 2:28 p.m. UTC | #4 
On 9/30/2023 10:52 PM, Dmitry Baryshkov wrote:
> On 29/09/2023 11:42, Praveenkumar I wrote:
>> Add USB Super-Speed UNIPHY node and populate the phandle on
>> gcc node for the parent clock map.
>>
>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/ipq5332.dtsi | 25 ++++++++++++++++++++++++-
>>   1 file changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi 
>> b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>> index d3fef2f80a81..b08ffd8c094e 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
>> @@ -158,6 +158,29 @@ usbphy0: phy@7b000 {
>>               status = "disabled";
>>           };
>>   +        usbphy1: phy@4b0000 {
>
> Are there other USB PHYs on this platform?
No. Only two USB PHYs.
>
>> +            compatible = "qcom,ipq5332-usb-uniphy";
>> +            reg = <0x4b0000 0x800>;
>> +
>> +            clocks = <&gcc GCC_PCIE3X1_PHY_AHB_CLK>,
>> +                 <&gcc GCC_USB0_PHY_CFG_AHB_CLK>,
>> +                 <&gcc GCC_USB0_PIPE_CLK>;
>> +            clock-names = "ahb",
>> +                      "cfg_ahb",
>> +                      "pipe";
>> +
>> +            resets =  <&gcc GCC_USB0_PHY_BCR>;
>> +
>> +            #clock-cells = <0>;
>> +            clock-output-names = "usb0_pipe_clk_src";
>
> I'm not sure, what is the best approach her. For QMP USB and PCIe PHYs 
> we had to use fixed names historically. On the other hand for QMP DP 
> clocks we are fine with the generated names. I'd prefer the latter case.
Sure, will use the generated names.
>
>> +
>> +            qcom,phy-usb-mux-sel = <&tcsr 0x10540>;
>> +
>> +            #phy-cells = <0>;
>> +
>> +            status = "disabled";
>> +        };
>> +
>>           qfprom: efuse@a4000 {
>>               compatible = "qcom,ipq5332-qfprom", "qcom,qfprom";
>>               reg = <0x000a4000 0x721>;
>> @@ -200,7 +223,7 @@ gcc: clock-controller@1800000 {
>>                    <&sleep_clk>,
>>                    <0>,
>>                    <0>,
>> -                 <0>;
>> +                 <&usbphy1>;
>>           };
>>             tcsr_mutex: hwlock@1905000 {
>
--
Thanks,
Praveenkumar
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
index d3fef2f80a81..b08ffd8c094e 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
@@ -158,6 +158,29 @@  usbphy0: phy@7b000 {
 			status = "disabled";
 		};
 
+		usbphy1: phy@4b0000 {
+			compatible = "qcom,ipq5332-usb-uniphy";
+			reg = <0x4b0000 0x800>;
+
+			clocks = <&gcc GCC_PCIE3X1_PHY_AHB_CLK>,
+				 <&gcc GCC_USB0_PHY_CFG_AHB_CLK>,
+				 <&gcc GCC_USB0_PIPE_CLK>;
+			clock-names = "ahb",
+				      "cfg_ahb",
+				      "pipe";
+
+			resets =  <&gcc GCC_USB0_PHY_BCR>;
+
+			#clock-cells = <0>;
+			clock-output-names = "usb0_pipe_clk_src";
+
+			qcom,phy-usb-mux-sel = <&tcsr 0x10540>;
+
+			#phy-cells = <0>;
+
+			status = "disabled";
+		};
+
 		qfprom: efuse@a4000 {
 			compatible = "qcom,ipq5332-qfprom", "qcom,qfprom";
 			reg = <0x000a4000 0x721>;
@@ -200,7 +223,7 @@  gcc: clock-controller@1800000 {
 				 <&sleep_clk>,
 				 <0>,
 				 <0>,
-				 <0>;
+				 <&usbphy1>;
 		};
 
 		tcsr_mutex: hwlock@1905000 {