[PATCH 0/3] arm64: dts: sm6115: Add USB SS qmp phy node and perform some cleanups

[PATCH 0/3] arm64: dts: sm6115: Add USB SS qmp phy node and perform some cleanups

[Bhupesh Sharma]

This series adds USB SS qmp phy node for Qualcomm SM6115 / SM4250 SoC
dtsi and also performs some related cleanups for USB nodes.

Note that this series is rebased on linux-next/master and is also
dependent on the corresponding dt-bindings and driver series sent via [1].

[1]. https://lore.kernel.org/linux-arm-msm/20221213122843.454845-1-bhupesh.sharma@linaro.org/

Bhupesh Sharma (3):
  arm64: dts: qcom: sm6115: Cleanup USB node names
  arm64: dts: qcom: sm6115: Move USB node's 'maximum-speed' and
    'dr_mode' properties to dts
  arm64: dts: qcom: sm6115: Add USB SS qmp phy node

 .../boot/dts/qcom/sm4250-oneplus-billie2.dts  |  9 +++-
 arch/arm64/boot/dts/qcom/sm6115.dtsi          | 46 ++++++++++++++++---
 2 files changed, 46 insertions(+), 9 deletions(-)

-- 
2.38.1

[PATCH 1/3] arm64: dts: qcom: sm6115: Cleanup USB node names

[Bhupesh Sharma]

There is only one USB controller present on SM6115 / SM4250
Qualcomm SoC, so drop the numbering used with USB nodes
in the dtsi and the related sm4250-oneplus-billie2.dts.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts | 4 ++--
 arch/arm64/boot/dts/qcom/sm6115.dtsi                | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
index a3f1c7c41fd73..fa57f4bf58256 100644
--- a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
+++ b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
@@ -225,11 +225,11 @@ &ufs_mem_phy {
 	status = "okay";
 };
 
-&usb_1 {
+&usb {
 	status = "okay";
 };
 
-&usb_1_hsphy {
+&usb_hsphy {
 	vdd-supply = <&vreg_l4a>;
 	vdda-pll-supply = <&vreg_l12a>;
 	vdda-phy-dpdm-supply = <&vreg_l15a>;
diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
index 572bf04adf906..b5f7480c2e713 100644
--- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
@@ -565,7 +565,7 @@ gcc: clock-controller@1400000 {
 			#power-domain-cells = <1>;
 		};
 
-		usb_1_hsphy: phy@1613000 {
+		usb_hsphy: phy@1613000 {
 			compatible = "qcom,sm6115-qusb2-phy";
 			reg = <0x01613000 0x180>;
 			#phy-cells = <0>;
@@ -991,7 +991,7 @@ spi5: spi@4a94000 {
 			};
 		};
 
-		usb_1: usb@4ef8800 {
+		usb: usb@4ef8800 {
 			compatible = "qcom,sm6115-dwc3", "qcom,dwc3";
 			reg = <0x04ef8800 0x400>;
 			#address-cells = <1>;
@@ -1019,11 +1019,11 @@ usb_1: usb@4ef8800 {
 			qcom,select-utmi-as-pipe-clk;
 			status = "disabled";
 
-			usb_1_dwc3: usb@4e00000 {
+			usb_dwc3: usb@4e00000 {
 				compatible = "snps,dwc3";
 				reg = <0x04e00000 0xcd00>;
 				interrupts = <GIC_SPI 255 IRQ_TYPE_LEVEL_HIGH>;
-				phys = <&usb_1_hsphy>;
+				phys = <&usb_hsphy>;
 				phy-names = "usb2-phy";
 				iommus = <&apps_smmu 0x120 0x0>;
 				snps,dis_u2_susphy_quirk;
-- 
2.38.1

Re: [PATCH 1/3] arm64: dts: qcom: sm6115: Cleanup USB node names

[Konrad Dybcio]

On 13.12.2022 13:38, Bhupesh Sharma wrote:
> There is only one USB controller present on SM6115 / SM4250
> Qualcomm SoC, so drop the numbering used with USB nodes
> in the dtsi and the related sm4250-oneplus-billie2.dts.
> 
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts | 4 ++--
>  arch/arm64/boot/dts/qcom/sm6115.dtsi                | 8 ++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> index a3f1c7c41fd73..fa57f4bf58256 100644
> --- a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> +++ b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> @@ -225,11 +225,11 @@ &ufs_mem_phy {
>  	status = "okay";
>  };
>  
> -&usb_1 {
> +&usb {
>  	status = "okay";
>  };
>  
> -&usb_1_hsphy {
> +&usb_hsphy {
>  	vdd-supply = <&vreg_l4a>;
>  	vdda-pll-supply = <&vreg_l12a>;
>  	vdda-phy-dpdm-supply = <&vreg_l15a>;
> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> index 572bf04adf906..b5f7480c2e713 100644
> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> @@ -565,7 +565,7 @@ gcc: clock-controller@1400000 {
>  			#power-domain-cells = <1>;
>  		};
>  
> -		usb_1_hsphy: phy@1613000 {
> +		usb_hsphy: phy@1613000 {
>  			compatible = "qcom,sm6115-qusb2-phy";
>  			reg = <0x01613000 0x180>;
>  			#phy-cells = <0>;
> @@ -991,7 +991,7 @@ spi5: spi@4a94000 {
>  			};
>  		};
>  
> -		usb_1: usb@4ef8800 {
> +		usb: usb@4ef8800 {
>  			compatible = "qcom,sm6115-dwc3", "qcom,dwc3";
>  			reg = <0x04ef8800 0x400>;
>  			#address-cells = <1>;
> @@ -1019,11 +1019,11 @@ usb_1: usb@4ef8800 {
>  			qcom,select-utmi-as-pipe-clk;
>  			status = "disabled";
>  
> -			usb_1_dwc3: usb@4e00000 {
> +			usb_dwc3: usb@4e00000 {
>  				compatible = "snps,dwc3";
>  				reg = <0x04e00000 0xcd00>;
>  				interrupts = <GIC_SPI 255 IRQ_TYPE_LEVEL_HIGH>;
> -				phys = <&usb_1_hsphy>;
> +				phys = <&usb_hsphy>;
>  				phy-names = "usb2-phy";
>  				iommus = <&apps_smmu 0x120 0x0>;
>  				snps,dis_u2_susphy_quirk;

Re: [PATCH 1/3] arm64: dts: qcom: sm6115: Cleanup USB node names

[Krzysztof Kozlowski]

On 13/12/2022 13:38, Bhupesh Sharma wrote:
> There is only one USB controller present on SM6115 / SM4250
> Qualcomm SoC, so drop the numbering used with USB nodes

The node names remain unaffected, so please mention you do it only for
the labels. Labels do not matter for the code, has no impact.

> in the dtsi and the related sm4250-oneplus-billie2.dts.
> 
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts | 4 ++--
>  arch/arm64/boot/dts/qcom/sm6115.dtsi                | 8 ++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> index a3f1c7c41fd73..fa57f4bf58256 100644
> --- a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> +++ b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> @@ -225,11 +225,11 @@ &ufs_mem_phy {
>  	status = "okay";
>  };
>  
> -&usb_1 {
> +&usb {
>  	status = "okay";
>  };

Best regards,
Krzysztof

Re: [PATCH 1/3] arm64: dts: qcom: sm6115: Cleanup USB node names

[Bhupesh Sharma]

Hi Krzysztof,

On Tue, 13 Dec 2022 at 18:21, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 13/12/2022 13:38, Bhupesh Sharma wrote:
> > There is only one USB controller present on SM6115 / SM4250
> > Qualcomm SoC, so drop the numbering used with USB nodes
>
> The node names remain unaffected, so please mention you do it only for
> the labels. Labels do not matter for the code, has no impact.

Fair point. Let me fix it in v2.

> > in the dtsi and the related sm4250-oneplus-billie2.dts.
> >
> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts | 4 ++--
> >  arch/arm64/boot/dts/qcom/sm6115.dtsi                | 8 ++++----
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> > index a3f1c7c41fd73..fa57f4bf58256 100644
> > --- a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> > +++ b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> > @@ -225,11 +225,11 @@ &ufs_mem_phy {
> >       status = "okay";
> >  };
> >
> > -&usb_1 {
> > +&usb {
> >       status = "okay";
> >  };

Regards,
Bhupesh

[PATCH 2/3] arm64: dts: qcom: sm6115: Move USB node's 'maximum-speed' and 'dr_mode' properties to dts

[Bhupesh Sharma]

Normally the 'maximum-speed' and 'dr_mode' properties
of a USB controller + port is dependent on the type of
the ports, regulators and mode change interrupt routing
available on the board(s).

So, move the same from the sm6115 dtsi file to respective
board file(s).

Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts | 5 +++++
 arch/arm64/boot/dts/qcom/sm6115.dtsi                | 2 --
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
index fa57f4bf58256..3f39f25e0721e 100644
--- a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
+++ b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
@@ -229,6 +229,11 @@ &usb {
 	status = "okay";
 };
 
+&usb_dwc3 {
+	maximum-speed = "high-speed";
+	dr_mode = "peripheral";
+};
+
 &usb_hsphy {
 	vdd-supply = <&vreg_l4a>;
 	vdda-pll-supply = <&vreg_l12a>;
diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
index b5f7480c2e713..e4ce135264f3d 100644
--- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
@@ -1031,8 +1031,6 @@ usb_dwc3: usb@4e00000 {
 				snps,has-lpm-erratum;
 				snps,hird-threshold = /bits/ 8 <0x10>;
 				snps,usb3_lpm_capable;
-				maximum-speed = "high-speed";
-				dr_mode = "peripheral";
 			};
 		};
 
-- 
2.38.1

Re: [PATCH 2/3] arm64: dts: qcom: sm6115: Move USB node's 'maximum-speed' and 'dr_mode' properties to dts

[Konrad Dybcio]

On 13.12.2022 13:38, Bhupesh Sharma wrote:
> Normally the 'maximum-speed' and 'dr_mode' properties
> of a USB controller + port is dependent on the type of
> the ports, regulators and mode change interrupt routing
> available on the board(s).
> 
> So, move the same from the sm6115 dtsi file to respective
> board file(s).
> 
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts | 5 +++++
>  arch/arm64/boot/dts/qcom/sm6115.dtsi                | 2 --
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> index fa57f4bf58256..3f39f25e0721e 100644
> --- a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> +++ b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> @@ -229,6 +229,11 @@ &usb {
>  	status = "okay";
>  };
>  
> +&usb_dwc3 {
> +	maximum-speed = "high-speed";
> +	dr_mode = "peripheral";
> +};
> +
>  &usb_hsphy {
>  	vdd-supply = <&vreg_l4a>;
>  	vdda-pll-supply = <&vreg_l12a>;
> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> index b5f7480c2e713..e4ce135264f3d 100644
> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> @@ -1031,8 +1031,6 @@ usb_dwc3: usb@4e00000 {
>  				snps,has-lpm-erratum;
>  				snps,hird-threshold = /bits/ 8 <0x10>;
>  				snps,usb3_lpm_capable;
> -				maximum-speed = "high-speed";
> -				dr_mode = "peripheral";
>  			};
>  		};
>  

[PATCH 3/3] arm64: dts: qcom: sm6115: Add USB SS qmp phy node

[Bhupesh Sharma]

Add USB superspeed qmp phy node to dtsi.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm6115.dtsi | 38 ++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
index e4ce135264f3d..9c5c024919f92 100644
--- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
@@ -579,6 +579,40 @@ usb_hsphy: phy@1613000 {
 			status = "disabled";
 		};
 
+		usb_qmpphy: phy@1615000 {
+			compatible = "qcom,sm6115-qmp-usb3-phy";
+			reg = <0x01615000 0x200>;
+			#clock-cells = <1>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+			clocks = <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
+				 <&gcc GCC_USB3_PRIM_CLKREF_CLK>,
+				 <&gcc GCC_AHB2PHY_USB_CLK>;
+			clock-names = "com_aux",
+				      "ref",
+				      "cfg_ahb";
+			resets = <&gcc GCC_USB3_PHY_PRIM_SP0_BCR>,
+				 <&gcc GCC_USB3PHY_PHY_PRIM_SP0_BCR>;
+			reset-names = "phy", "phy_phy";
+			status = "disabled";
+
+			usb_ssphy: phy@1615200 {
+				reg = <0x01615200 0x200>,
+				      <0x01615400 0x200>,
+				      <0x01615c00 0x400>,
+				      <0x01615600 0x200>,
+				      <0x01615800 0x200>,
+				      <0x01615a00 0x100>;
+				#phy-cells = <0>;
+				#clock-cells = <1>;
+				clocks = <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
+				clock-names = "pipe0";
+				clock-output-names = "usb3_phy_pipe_clk_src";
+			};
+		};
+
+
 		qfprom@1b40000 {
 			compatible = "qcom,sm6115-qfprom", "qcom,qfprom";
 			reg = <0x01b40000 0x7000>;
@@ -1023,8 +1057,8 @@ usb_dwc3: usb@4e00000 {
 				compatible = "snps,dwc3";
 				reg = <0x04e00000 0xcd00>;
 				interrupts = <GIC_SPI 255 IRQ_TYPE_LEVEL_HIGH>;
-				phys = <&usb_hsphy>;
-				phy-names = "usb2-phy";
+				phys = <&usb_hsphy>, <&usb_ssphy>;
+				phy-names = "usb2-phy", "usb3-phy";
 				iommus = <&apps_smmu 0x120 0x0>;
 				snps,dis_u2_susphy_quirk;
 				snps,dis_enblslpm_quirk;
-- 
2.38.1

Re: [PATCH 3/3] arm64: dts: qcom: sm6115: Add USB SS qmp phy node

[Konrad Dybcio]

On 13.12.2022 13:38, Bhupesh Sharma wrote:
> Add USB superspeed qmp phy node to dtsi.
> 
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
Please run make dtbs_check before sending dt patches, this one
introduces new errors.


>  arch/arm64/boot/dts/qcom/sm6115.dtsi | 38 ++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> index e4ce135264f3d..9c5c024919f92 100644
> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> @@ -579,6 +579,40 @@ usb_hsphy: phy@1613000 {
>  			status = "disabled";
>  		};
>  
> +		usb_qmpphy: phy@1615000 {
> +			compatible = "qcom,sm6115-qmp-usb3-phy";
> +			reg = <0x01615000 0x200>;
> +			#clock-cells = <1>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
These -cells and ranges properties could go after status=disabled

Konrad
> +			clocks = <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
> +				 <&gcc GCC_USB3_PRIM_CLKREF_CLK>,
> +				 <&gcc GCC_AHB2PHY_USB_CLK>;
> +			clock-names = "com_aux",
> +				      "ref",
> +				      "cfg_ahb";
> +			resets = <&gcc GCC_USB3_PHY_PRIM_SP0_BCR>,
> +				 <&gcc GCC_USB3PHY_PHY_PRIM_SP0_BCR>;
> +			reset-names = "phy", "phy_phy";
> +			status = "disabled";
> +
> +			usb_ssphy: phy@1615200 {
> +				reg = <0x01615200 0x200>,
> +				      <0x01615400 0x200>,
> +				      <0x01615c00 0x400>,
> +				      <0x01615600 0x200>,
> +				      <0x01615800 0x200>,
> +				      <0x01615a00 0x100>;
> +				#phy-cells = <0>;
> +				#clock-cells = <1>;
> +				clocks = <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> +				clock-names = "pipe0";
> +				clock-output-names = "usb3_phy_pipe_clk_src";
> +			};
> +		};
> +
> +
>  		qfprom@1b40000 {
>  			compatible = "qcom,sm6115-qfprom", "qcom,qfprom";
>  			reg = <0x01b40000 0x7000>;
> @@ -1023,8 +1057,8 @@ usb_dwc3: usb@4e00000 {
>  				compatible = "snps,dwc3";
>  				reg = <0x04e00000 0xcd00>;
>  				interrupts = <GIC_SPI 255 IRQ_TYPE_LEVEL_HIGH>;
> -				phys = <&usb_hsphy>;
> -				phy-names = "usb2-phy";
> +				phys = <&usb_hsphy>, <&usb_ssphy>;
> +				phy-names = "usb2-phy", "usb3-phy";
>  				iommus = <&apps_smmu 0x120 0x0>;
>  				snps,dis_u2_susphy_quirk;
>  				snps,dis_enblslpm_quirk;

Re: [PATCH 3/3] arm64: dts: qcom: sm6115: Add USB SS qmp phy node

[Dmitry Baryshkov]

On 13 December 2022 14:49:05 EET, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>On 13.12.2022 13:38, Bhupesh Sharma wrote:
>> Add USB superspeed qmp phy node to dtsi.
>> 
>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
>> ---
>Please run make dtbs_check before sending dt patches, this one
>introduces new errors.
>
>
>>  arch/arm64/boot/dts/qcom/sm6115.dtsi | 38 ++++++++++++++++++++++++++--
>>  1 file changed, 36 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
>> index e4ce135264f3d..9c5c024919f92 100644
>> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
>> @@ -579,6 +579,40 @@ usb_hsphy: phy@1613000 {
>>  			status = "disabled";
>>  		};
>>  
>> +		usb_qmpphy: phy@1615000 {
>> +			compatible = "qcom,sm6115-qmp-usb3-phy";
>> +			reg = <0x01615000 0x200>;
>> +			#clock-cells = <1>;
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +			ranges;
>These -cells and ranges properties could go after status=disabled
>
>Konrad
>> +			clocks = <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
>> +				 <&gcc GCC_USB3_PRIM_CLKREF_CLK>,
>> +				 <&gcc GCC_AHB2PHY_USB_CLK>;
>> +			clock-names = "com_aux",
>> +				      "ref",
>> +				      "cfg_ahb";
>> +			resets = <&gcc GCC_USB3_PHY_PRIM_SP0_BCR>,
>> +				 <&gcc GCC_USB3PHY_PHY_PRIM_SP0_BCR>;
>> +			reset-names = "phy", "phy_phy";
>> +			status = "disabled";
>> +
>> +			usb_ssphy: phy@1615200 {

We should not introduce additional qmp-with-child PHY nodes.


>> +				reg = <0x01615200 0x200>,
>> +				      <0x01615400 0x200>,
>> +				      <0x01615c00 0x400>,
>> +				      <0x01615600 0x200>,
>> +				      <0x01615800 0x200>,
>> +				      <0x01615a00 0x100>;
>> +				#phy-cells = <0>;
>> +				#clock-cells = <1>;
>> +				clocks = <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
>> +				clock-names = "pipe0";
>> +				clock-output-names = "usb3_phy_pipe_clk_src";
>> +			};
>> +		};
>> +
>> +
>>  		qfprom@1b40000 {
>>  			compatible = "qcom,sm6115-qfprom", "qcom,qfprom";
>>  			reg = <0x01b40000 0x7000>;
>> @@ -1023,8 +1057,8 @@ usb_dwc3: usb@4e00000 {
>>  				compatible = "snps,dwc3";
>>  				reg = <0x04e00000 0xcd00>;
>>  				interrupts = <GIC_SPI 255 IRQ_TYPE_LEVEL_HIGH>;
>> -				phys = <&usb_hsphy>;
>> -				phy-names = "usb2-phy";
>> +				phys = <&usb_hsphy>, <&usb_ssphy>;
>> +				phy-names = "usb2-phy", "usb3-phy";
>>  				iommus = <&apps_smmu 0x120 0x0>;
>>  				snps,dis_u2_susphy_quirk;
>>  				snps,dis_enblslpm_quirk;

-- 
With best wishes
Dmitry

Re: [PATCH 3/3] arm64: dts: qcom: sm6115: Add USB SS qmp phy node

[Bhupesh Sharma]

On Wed, 14 Dec 2022 at 03:48, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
>
>
> On 13 December 2022 14:49:05 EET, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> >
> >
> >On 13.12.2022 13:38, Bhupesh Sharma wrote:
> >> Add USB superspeed qmp phy node to dtsi.
> >>
> >> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> >> ---
> >Please run make dtbs_check before sending dt patches, this one
> >introduces new errors.
> >
> >
> >>  arch/arm64/boot/dts/qcom/sm6115.dtsi | 38 ++++++++++++++++++++++++++--
> >>  1 file changed, 36 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> >> index e4ce135264f3d..9c5c024919f92 100644
> >> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> >> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> >> @@ -579,6 +579,40 @@ usb_hsphy: phy@1613000 {
> >>                      status = "disabled";
> >>              };
> >>
> >> +            usb_qmpphy: phy@1615000 {
> >> +                    compatible = "qcom,sm6115-qmp-usb3-phy";
> >> +                    reg = <0x01615000 0x200>;
> >> +                    #clock-cells = <1>;
> >> +                    #address-cells = <1>;
> >> +                    #size-cells = <1>;
> >> +                    ranges;
> >These -cells and ranges properties could go after status=disabled
> >
> >Konrad
> >> +                    clocks = <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
> >> +                             <&gcc GCC_USB3_PRIM_CLKREF_CLK>,
> >> +                             <&gcc GCC_AHB2PHY_USB_CLK>;
> >> +                    clock-names = "com_aux",
> >> +                                  "ref",
> >> +                                  "cfg_ahb";
> >> +                    resets = <&gcc GCC_USB3_PHY_PRIM_SP0_BCR>,
> >> +                             <&gcc GCC_USB3PHY_PHY_PRIM_SP0_BCR>;
> >> +                    reset-names = "phy", "phy_phy";
> >> +                    status = "disabled";
> >> +
> >> +                    usb_ssphy: phy@1615200 {
>
> We should not introduce additional qmp-with-child PHY nodes.

Not sure I understand your point. Is there some recent change (being
discussed) regarding the same?

Thanks,
Bhupesh

>
> >> +                            reg = <0x01615200 0x200>,
> >> +                                  <0x01615400 0x200>,
> >> +                                  <0x01615c00 0x400>,
> >> +                                  <0x01615600 0x200>,
> >> +                                  <0x01615800 0x200>,
> >> +                                  <0x01615a00 0x100>;
> >> +                            #phy-cells = <0>;
> >> +                            #clock-cells = <1>;
> >> +                            clocks = <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> >> +                            clock-names = "pipe0";
> >> +                            clock-output-names = "usb3_phy_pipe_clk_src";
> >> +                    };
> >> +            };
> >> +
> >> +
> >>              qfprom@1b40000 {
> >>                      compatible = "qcom,sm6115-qfprom", "qcom,qfprom";
> >>                      reg = <0x01b40000 0x7000>;
> >> @@ -1023,8 +1057,8 @@ usb_dwc3: usb@4e00000 {
> >>                              compatible = "snps,dwc3";
> >>                              reg = <0x04e00000 0xcd00>;
> >>                              interrupts = <GIC_SPI 255 IRQ_TYPE_LEVEL_HIGH>;
> >> -                            phys = <&usb_hsphy>;
> >> -                            phy-names = "usb2-phy";
> >> +                            phys = <&usb_hsphy>, <&usb_ssphy>;
> >> +                            phy-names = "usb2-phy", "usb3-phy";
> >>                              iommus = <&apps_smmu 0x120 0x0>;
> >>                              snps,dis_u2_susphy_quirk;
> >>                              snps,dis_enblslpm_quirk;
>
> --
> With best wishes
> Dmitry

Re: [PATCH 3/3] arm64: dts: qcom: sm6115: Add USB SS qmp phy node

[Krzysztof Kozlowski]

On 13/12/2022 13:38, Bhupesh Sharma wrote:
> Add USB superspeed qmp phy node to dtsi.
> 
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm6115.dtsi | 38 ++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> index e4ce135264f3d..9c5c024919f92 100644
> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> @@ -579,6 +579,40 @@ usb_hsphy: phy@1613000 {
>  			status = "disabled";
>  		};
>  
> +		usb_qmpphy: phy@1615000 {
> +			compatible = "qcom,sm6115-qmp-usb3-phy";
> +			reg = <0x01615000 0x200>;
> +			#clock-cells = <1>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +			clocks = <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
> +				 <&gcc GCC_USB3_PRIM_CLKREF_CLK>,
> +				 <&gcc GCC_AHB2PHY_USB_CLK>;
> +			clock-names = "com_aux",
> +				      "ref",
> +				      "cfg_ahb";
> +			resets = <&gcc GCC_USB3_PHY_PRIM_SP0_BCR>,
> +				 <&gcc GCC_USB3PHY_PHY_PRIM_SP0_BCR>;
> +			reset-names = "phy", "phy_phy";
> +			status = "disabled";

Hm, you add a disabled PHY which is used by existing controller. The
controller is enabled in board DTS, but new PHY node isn't. Aren't you
now breaking it?

> +
> +			usb_ssphy: phy@1615200 {
> +				reg = <0x01615200 0x200>,
> +				      <0x01615400 0x200>,
> +				      <0x01615c00 0x400>,
> +				      <0x01615600 0x200>,
> +				      <0x01615800 0x200>,
> +				      <0x01615a00 0x100>;
> +				#phy-cells = <0>;
> +				#clock-cells = <1>;
> +				clocks = <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> +				clock-names = "pipe0";
> +				clock-output-names = "usb3_phy_pipe_clk_src";
> +			};
> +		};
> +
> +

Just one blank line.

>  		qfprom@1b40000 {
>  			compatible = "qcom,sm6115-qfprom", "qcom,qfprom";
>  			reg = <0x01b40000 0x7000>;
> @@ -1023,8 +1057,8 @@ usb_dwc3: usb@4e00000 {
>  				compatible = "snps,dwc3";
>  				reg = <0x04e00000 0xcd00>;
>  				interrupts = <GIC_SPI 255 IRQ_TYPE_LEVEL_HIGH>;
> -				phys = <&usb_hsphy>;
> -				phy-names = "usb2-phy";
> +				phys = <&usb_hsphy>, <&usb_ssphy>;
> +				phy-names = "usb2-phy", "usb3-phy";
>  				iommus = <&apps_smmu 0x120 0x0>;
>  				snps,dis_u2_susphy_quirk;
>  				snps,dis_enblslpm_quirk;

Best regards,
Krzysztof

Re: [PATCH 3/3] arm64: dts: qcom: sm6115: Add USB SS qmp phy node

[Bhupesh Sharma]

Hi Krzysztof,

On Tue, 13 Dec 2022 at 18:26, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 13/12/2022 13:38, Bhupesh Sharma wrote:
> > Add USB superspeed qmp phy node to dtsi.
> >
> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sm6115.dtsi | 38 ++++++++++++++++++++++++++--
> >  1 file changed, 36 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> > index e4ce135264f3d..9c5c024919f92 100644
> > --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> > @@ -579,6 +579,40 @@ usb_hsphy: phy@1613000 {
> >                       status = "disabled";
> >               };
> >
> > +             usb_qmpphy: phy@1615000 {
> > +                     compatible = "qcom,sm6115-qmp-usb3-phy";
> > +                     reg = <0x01615000 0x200>;
> > +                     #clock-cells = <1>;
> > +                     #address-cells = <1>;
> > +                     #size-cells = <1>;
> > +                     ranges;
> > +                     clocks = <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
> > +                              <&gcc GCC_USB3_PRIM_CLKREF_CLK>,
> > +                              <&gcc GCC_AHB2PHY_USB_CLK>;
> > +                     clock-names = "com_aux",
> > +                                   "ref",
> > +                                   "cfg_ahb";
> > +                     resets = <&gcc GCC_USB3_PHY_PRIM_SP0_BCR>,
> > +                              <&gcc GCC_USB3PHY_PHY_PRIM_SP0_BCR>;
> > +                     reset-names = "phy", "phy_phy";
> > +                     status = "disabled";
>
> Hm, you add a disabled PHY which is used by existing controller. The
> controller is enabled in board DTS, but new PHY node isn't. Aren't you
> now breaking it?

The USB controller is connected to two PHYs - one is HS PHY and the other is SS
QMP Phy. So while the exiting board dts describes and uses only the HS
PHY, newer
board dts files (which will soon be sent out as a separate patch),
will use both the HS and SS
USB PHYs.

So, this will not break the existing board dts files.

> > +
> > +                     usb_ssphy: phy@1615200 {
> > +                             reg = <0x01615200 0x200>,
> > +                                   <0x01615400 0x200>,
> > +                                   <0x01615c00 0x400>,
> > +                                   <0x01615600 0x200>,
> > +                                   <0x01615800 0x200>,
> > +                                   <0x01615a00 0x100>;
> > +                             #phy-cells = <0>;
> > +                             #clock-cells = <1>;
> > +                             clocks = <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> > +                             clock-names = "pipe0";
> > +                             clock-output-names = "usb3_phy_pipe_clk_src";
> > +                     };
> > +             };
> > +
> > +
>
> Just one blank line.

Ok, Will fix it in v2.

> >               qfprom@1b40000 {
> >                       compatible = "qcom,sm6115-qfprom", "qcom,qfprom";
> >                       reg = <0x01b40000 0x7000>;
> > @@ -1023,8 +1057,8 @@ usb_dwc3: usb@4e00000 {
> >                               compatible = "snps,dwc3";
> >                               reg = <0x04e00000 0xcd00>;
> >                               interrupts = <GIC_SPI 255 IRQ_TYPE_LEVEL_HIGH>;
> > -                             phys = <&usb_hsphy>;
> > -                             phy-names = "usb2-phy";
> > +                             phys = <&usb_hsphy>, <&usb_ssphy>;
> > +                             phy-names = "usb2-phy", "usb3-phy";
> >                               iommus = <&apps_smmu 0x120 0x0>;
> >                               snps,dis_u2_susphy_quirk;
> >                               snps,dis_enblslpm_quirk;
>

Thanks,
Bhupesh

Re: [PATCH 3/3] arm64: dts: qcom: sm6115: Add USB SS qmp phy node

[Konrad Dybcio]

On 13.12.2022 19:52, Bhupesh Sharma wrote:
> Hi Krzysztof,
> 
> On Tue, 13 Dec 2022 at 18:26, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 13/12/2022 13:38, Bhupesh Sharma wrote:
>>> Add USB superspeed qmp phy node to dtsi.
>>>
>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
>>> ---
>>>  arch/arm64/boot/dts/qcom/sm6115.dtsi | 38 ++++++++++++++++++++++++++--
>>>  1 file changed, 36 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
>>> index e4ce135264f3d..9c5c024919f92 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
>>> @@ -579,6 +579,40 @@ usb_hsphy: phy@1613000 {
>>>                       status = "disabled";
>>>               };
>>>
>>> +             usb_qmpphy: phy@1615000 {
>>> +                     compatible = "qcom,sm6115-qmp-usb3-phy";
>>> +                     reg = <0x01615000 0x200>;
>>> +                     #clock-cells = <1>;
>>> +                     #address-cells = <1>;
>>> +                     #size-cells = <1>;
>>> +                     ranges;
>>> +                     clocks = <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
>>> +                              <&gcc GCC_USB3_PRIM_CLKREF_CLK>,
>>> +                              <&gcc GCC_AHB2PHY_USB_CLK>;
>>> +                     clock-names = "com_aux",
>>> +                                   "ref",
>>> +                                   "cfg_ahb";
>>> +                     resets = <&gcc GCC_USB3_PHY_PRIM_SP0_BCR>,
>>> +                              <&gcc GCC_USB3PHY_PHY_PRIM_SP0_BCR>;
>>> +                     reset-names = "phy", "phy_phy";
>>> +                     status = "disabled";
>>
>> Hm, you add a disabled PHY which is used by existing controller. The
>> controller is enabled in board DTS, but new PHY node isn't. Aren't you
>> now breaking it?
> 
> The USB controller is connected to two PHYs - one is HS PHY and the other is SS
> QMP Phy. So while the exiting board dts describes and uses only the HS
> PHY, newer
> board dts files (which will soon be sent out as a separate patch),
> will use both the HS and SS
> USB PHYs.
It will break Oneplus billie2, you need to specify just the usb2
phy (and phy-names with just usb2-phy) there. Otherwise it's gonna
end up waiting infinitely for the usb3 one to probe (but it won't
because it's disabled)

Konrad
> 
> So, this will not break the existing board dts files.
> 
>>> +
>>> +                     usb_ssphy: phy@1615200 {
>>> +                             reg = <0x01615200 0x200>,
>>> +                                   <0x01615400 0x200>,
>>> +                                   <0x01615c00 0x400>,
>>> +                                   <0x01615600 0x200>,
>>> +                                   <0x01615800 0x200>,
>>> +                                   <0x01615a00 0x100>;
>>> +                             #phy-cells = <0>;
>>> +                             #clock-cells = <1>;
>>> +                             clocks = <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
>>> +                             clock-names = "pipe0";
>>> +                             clock-output-names = "usb3_phy_pipe_clk_src";
>>> +                     };
>>> +             };
>>> +
>>> +
>>
>> Just one blank line.
> 
> Ok, Will fix it in v2.
> 
>>>               qfprom@1b40000 {
>>>                       compatible = "qcom,sm6115-qfprom", "qcom,qfprom";
>>>                       reg = <0x01b40000 0x7000>;
>>> @@ -1023,8 +1057,8 @@ usb_dwc3: usb@4e00000 {
>>>                               compatible = "snps,dwc3";
>>>                               reg = <0x04e00000 0xcd00>;
>>>                               interrupts = <GIC_SPI 255 IRQ_TYPE_LEVEL_HIGH>;
>>> -                             phys = <&usb_hsphy>;
>>> -                             phy-names = "usb2-phy";
>>> +                             phys = <&usb_hsphy>, <&usb_ssphy>;
>>> +                             phy-names = "usb2-phy", "usb3-phy";
>>>                               iommus = <&apps_smmu 0x120 0x0>;
>>>                               snps,dis_u2_susphy_quirk;
>>>                               snps,dis_enblslpm_quirk;
>>
> 
> Thanks,
> Bhupesh

Re: [PATCH 3/3] arm64: dts: qcom: sm6115: Add USB SS qmp phy node

[Krzysztof Kozlowski]

On 13/12/2022 19:52, Bhupesh Sharma wrote:
> Hi Krzysztof,
> 
> On Tue, 13 Dec 2022 at 18:26, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 13/12/2022 13:38, Bhupesh Sharma wrote:
>>> Add USB superspeed qmp phy node to dtsi.
>>>
>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
>>> ---
>>>  arch/arm64/boot/dts/qcom/sm6115.dtsi | 38 ++++++++++++++++++++++++++--
>>>  1 file changed, 36 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
>>> index e4ce135264f3d..9c5c024919f92 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
>>> @@ -579,6 +579,40 @@ usb_hsphy: phy@1613000 {
>>>                       status = "disabled";
>>>               };
>>>
>>> +             usb_qmpphy: phy@1615000 {
>>> +                     compatible = "qcom,sm6115-qmp-usb3-phy";
>>> +                     reg = <0x01615000 0x200>;
>>> +                     #clock-cells = <1>;
>>> +                     #address-cells = <1>;
>>> +                     #size-cells = <1>;
>>> +                     ranges;
>>> +                     clocks = <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
>>> +                              <&gcc GCC_USB3_PRIM_CLKREF_CLK>,
>>> +                              <&gcc GCC_AHB2PHY_USB_CLK>;
>>> +                     clock-names = "com_aux",
>>> +                                   "ref",
>>> +                                   "cfg_ahb";
>>> +                     resets = <&gcc GCC_USB3_PHY_PRIM_SP0_BCR>,
>>> +                              <&gcc GCC_USB3PHY_PHY_PRIM_SP0_BCR>;
>>> +                     reset-names = "phy", "phy_phy";
>>> +                     status = "disabled";
>>
>> Hm, you add a disabled PHY which is used by existing controller. The
>> controller is enabled in board DTS, but new PHY node isn't. Aren't you
>> now breaking it?
> 
> The USB controller is connected to two PHYs - one is HS PHY and the other is SS
> QMP Phy. So while the exiting board dts describes and uses only the HS
> PHY, newer
> board dts files (which will soon be sent out as a separate patch),

Then I miss how do you narrow the existing DTS to use only one PHY. I
don't see anything in this patchset.

> will use both the HS and SS
> USB PHYs.
> 
> So, this will not break the existing board dts files.

I still think it will be. The board boots with USB with one phy enabled
and one disabled. The driver gets phys unconditionally and one of them
is disabled.

Even if Linux implementation will work (devm_usb_get_phy_by_phandle will
return -ENXIO or -ENODEV for disabled node), it is still a bit confusing
and I wonder how other users of such DTS should behave. Although it will
affect only one board, so maybe there are no other users?

Best regards,
Krzysztof

Re: [PATCH 3/3] arm64: dts: qcom: sm6115: Add USB SS qmp phy node

[Bhupesh Sharma]

On Wed, 14 Dec 2022 at 00:29, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 13/12/2022 19:52, Bhupesh Sharma wrote:
> > Hi Krzysztof,
> >
> > On Tue, 13 Dec 2022 at 18:26, Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 13/12/2022 13:38, Bhupesh Sharma wrote:
> >>> Add USB superspeed qmp phy node to dtsi.
> >>>
> >>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> >>> ---
> >>>  arch/arm64/boot/dts/qcom/sm6115.dtsi | 38 ++++++++++++++++++++++++++--
> >>>  1 file changed, 36 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> >>> index e4ce135264f3d..9c5c024919f92 100644
> >>> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> >>> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> >>> @@ -579,6 +579,40 @@ usb_hsphy: phy@1613000 {
> >>>                       status = "disabled";
> >>>               };
> >>>
> >>> +             usb_qmpphy: phy@1615000 {
> >>> +                     compatible = "qcom,sm6115-qmp-usb3-phy";
> >>> +                     reg = <0x01615000 0x200>;
> >>> +                     #clock-cells = <1>;
> >>> +                     #address-cells = <1>;
> >>> +                     #size-cells = <1>;
> >>> +                     ranges;
> >>> +                     clocks = <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
> >>> +                              <&gcc GCC_USB3_PRIM_CLKREF_CLK>,
> >>> +                              <&gcc GCC_AHB2PHY_USB_CLK>;
> >>> +                     clock-names = "com_aux",
> >>> +                                   "ref",
> >>> +                                   "cfg_ahb";
> >>> +                     resets = <&gcc GCC_USB3_PHY_PRIM_SP0_BCR>,
> >>> +                              <&gcc GCC_USB3PHY_PHY_PRIM_SP0_BCR>;
> >>> +                     reset-names = "phy", "phy_phy";
> >>> +                     status = "disabled";
> >>
> >> Hm, you add a disabled PHY which is used by existing controller. The
> >> controller is enabled in board DTS, but new PHY node isn't. Aren't you
> >> now breaking it?
> >
> > The USB controller is connected to two PHYs - one is HS PHY and the other is SS
> > QMP Phy. So while the exiting board dts describes and uses only the HS
> > PHY, newer
> > board dts files (which will soon be sent out as a separate patch),
>
> Then I miss how do you narrow the existing DTS to use only one PHY. I
> don't see anything in this patchset.
>
> > will use both the HS and SS
> > USB PHYs.
> >
> > So, this will not break the existing board dts files.
>
> I still think it will be. The board boots with USB with one phy enabled
> and one disabled. The driver gets phys unconditionally and one of them
> is disabled.
>
> Even if Linux implementation will work (devm_usb_get_phy_by_phandle will
> return -ENXIO or -ENODEV for disabled node), it is still a bit confusing
> and I wonder how other users of such DTS should behave. Although it will
> affect only one board, so maybe there are no other users?

Ah, now I get your point. So how does the following fix in
sm4250-oneplus-billie2.dts look like. It allows the base dtsi to carry
the usb nodes as exposed by the SoC and allows other board dts files
to use both the USB2 and UBS3 PHYs.

Please let me know.

--- a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
+++ b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
@@ -232,6 +232,9 @@ &usb {
 &usb_dwc3 {
        maximum-speed = "high-speed";
        dr_mode = "peripheral";
+
+       phys = <&usb_hsphy>;
+       phy-names = "usb2-phy";
 };

Thanks.

Re: [PATCH 3/3] arm64: dts: qcom: sm6115: Add USB SS qmp phy node

[Konrad Dybcio]

On 14.12.2022 06:20, Bhupesh Sharma wrote:
> On Wed, 14 Dec 2022 at 00:29, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 13/12/2022 19:52, Bhupesh Sharma wrote:
>>> Hi Krzysztof,
>>>
>>> On Tue, 13 Dec 2022 at 18:26, Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 13/12/2022 13:38, Bhupesh Sharma wrote:
>>>>> Add USB superspeed qmp phy node to dtsi.
>>>>>
>>>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
>>>>> ---
>>>>>  arch/arm64/boot/dts/qcom/sm6115.dtsi | 38 ++++++++++++++++++++++++++--
>>>>>  1 file changed, 36 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
>>>>> index e4ce135264f3d..9c5c024919f92 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
>>>>> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
>>>>> @@ -579,6 +579,40 @@ usb_hsphy: phy@1613000 {
>>>>>                       status = "disabled";
>>>>>               };
>>>>>
>>>>> +             usb_qmpphy: phy@1615000 {
>>>>> +                     compatible = "qcom,sm6115-qmp-usb3-phy";
>>>>> +                     reg = <0x01615000 0x200>;
>>>>> +                     #clock-cells = <1>;
>>>>> +                     #address-cells = <1>;
>>>>> +                     #size-cells = <1>;
>>>>> +                     ranges;
>>>>> +                     clocks = <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
>>>>> +                              <&gcc GCC_USB3_PRIM_CLKREF_CLK>,
>>>>> +                              <&gcc GCC_AHB2PHY_USB_CLK>;
>>>>> +                     clock-names = "com_aux",
>>>>> +                                   "ref",
>>>>> +                                   "cfg_ahb";
>>>>> +                     resets = <&gcc GCC_USB3_PHY_PRIM_SP0_BCR>,
>>>>> +                              <&gcc GCC_USB3PHY_PHY_PRIM_SP0_BCR>;
>>>>> +                     reset-names = "phy", "phy_phy";
>>>>> +                     status = "disabled";
>>>>
>>>> Hm, you add a disabled PHY which is used by existing controller. The
>>>> controller is enabled in board DTS, but new PHY node isn't. Aren't you
>>>> now breaking it?
>>>
>>> The USB controller is connected to two PHYs - one is HS PHY and the other is SS
>>> QMP Phy. So while the exiting board dts describes and uses only the HS
>>> PHY, newer
>>> board dts files (which will soon be sent out as a separate patch),
>>
>> Then I miss how do you narrow the existing DTS to use only one PHY. I
>> don't see anything in this patchset.
>>
>>> will use both the HS and SS
>>> USB PHYs.
>>>
>>> So, this will not break the existing board dts files.
>>
>> I still think it will be. The board boots with USB with one phy enabled
>> and one disabled. The driver gets phys unconditionally and one of them
>> is disabled.
>>
>> Even if Linux implementation will work (devm_usb_get_phy_by_phandle will
>> return -ENXIO or -ENODEV for disabled node), it is still a bit confusing
>> and I wonder how other users of such DTS should behave. Although it will
>> affect only one board, so maybe there are no other users?
> 
> Ah, now I get your point. So how does the following fix in
> sm4250-oneplus-billie2.dts look like. It allows the base dtsi to carry
> the usb nodes as exposed by the SoC and allows other board dts files
> to use both the USB2 and UBS3 PHYs.
> 
> Please let me know.
> 
> --- a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> +++ b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts
> @@ -232,6 +232,9 @@ &usb {
>  &usb_dwc3 {
>         maximum-speed = "high-speed";
>         dr_mode = "peripheral";
> +
> +       phys = <&usb_hsphy>;
> +       phy-names = "usb2-phy";
>  };
Looks good now!

Konrad
> 
> Thanks.