Skip to contentSkip to conversation

arm64: dts: qcom: sc8280xp: fix UFS reference clocks and PHY nodes

PATCH Applied Johan Hovold 5 participants 11 messages Nov 4, 2022 – Dec 6, 2022

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover arm64: dts: qcom: sc8280xp: fix UFS reference clocks and PHY nodes

After some initial confusion, we've finally settled how the UFS ref clocks are used.

The first patch fixes the UFS controller and PHY nodes so that they reflect the hardware. This one should go in 6.1-rc where the two previous attempts to address this are heading.

Note this patch depends on first updating the clock driver to reflect that the device ref clock is fed from CXO. This is needed as the UFS controller driver requires a valid frequency for the ref clock. A patch that addresses the this has been submitted, but a v2 is in the works.
[1]

The second patch updates the UFS PHY nodes so that they reflect the fixed UFS PHY binding that is now in linux-next. This one is 6.2 material but depends on first fixing the clocks.

So to summarise the dependency order:

1. clock driver should be fixed in 6.1-rc
2. patch 1/2 should go in 6.1-rc (after 1)
3. patch 2/2 should go in 6.2 (after 1 and 2)

Johan

[1] https://lore.kernel.org/r/[email protected]

Johan Hovold (2):
arm64: dts: qcom: sc8280xp: fix UFS reference clocks
arm64: dts: qcom: sc8280xp: update UFS PHY nodes

 arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 57 ++++++++++----------------
 1 file changed, 21 insertions(+), 36 deletions(-)
Signature · 1 line
2.37.3
#1 lore raw

PATCH 1/2 arm64: dts: qcom: sc8280xp: fix UFS reference clocks

There are three UFS reference clocks on SC8280XP which are used as follows:

- The GCC_UFS_REF_CLKREF_CLK clock is fed to any UFS device connected to either controller.

- The GCC_UFS_1_CARD_CLKREF_CLK and GCC_UFS_CARD_CLKREF_CLK clocks provide reference clocks to the two PHYs.

Note that this depends on first updating the clock driver to reflect that all three clocks are sourced from CXO. Specifically, the UFS controller driver expects the device reference clock to have a valid frequency:

ufshcd-qcom 1d84000.ufs: invalid ref_clk setting = 0

Fixes 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform")
Fixes 8d6b458ce6e9 ("arm64: dts: qcom: sc8280xp: fix ufs_card_phy ref clock")
Fixes f3aa975e230e ("arm64: dts: qcom: sc8280xp: correct ref clock for ufs_mem_phy")
---
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/arm64/boot/dts/qcom/sc8280xp.dtsi +4 -4
index 21ac119e0382..e0d0fb6994b5 100644
@@ -912,7 +912,7 @@ ufs_mem_hc: ufs@1d84000 {
 				 <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
 				 <&gcc GCC_UFS_PHY_AHB_CLK>,
 				 <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>,
-				 <&rpmhcc RPMH_CXO_CLK>,
+				 <&gcc GCC_UFS_REF_CLKREF_CLK>,
 				 <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>,
 				 <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>,
 				 <&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>;
@@ -943,7 +943,7 @@ ufs_mem_phy: phy@1d87000 {
 			ranges;
 			clock-names = "ref",
 				      "ref_aux";
-			clocks = <&gcc GCC_UFS_REF_CLKREF_CLK>,
+			clocks = <&gcc GCC_UFS_CARD_CLKREF_CLK>,
 				 <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
 
 			resets = <&ufs_mem_hc 0>;
@@ -980,7 +980,7 @@ ufs_card_hc: ufs@1da4000 {
 				 <&gcc GCC_AGGRE_UFS_CARD_AXI_CLK>,
 				 <&gcc GCC_UFS_CARD_AHB_CLK>,
 				 <&gcc GCC_UFS_CARD_UNIPRO_CORE_CLK>,
-				 <&rpmhcc RPMH_CXO_CLK>,
+				 <&gcc GCC_UFS_REF_CLKREF_CLK>,
 				 <&gcc GCC_UFS_CARD_TX_SYMBOL_0_CLK>,
 				 <&gcc GCC_UFS_CARD_RX_SYMBOL_0_CLK>,
 				 <&gcc GCC_UFS_CARD_RX_SYMBOL_1_CLK>;
@@ -1011,7 +1011,7 @@ ufs_card_phy: phy@1da7000 {
 			ranges;
 			clock-names = "ref",
 				      "ref_aux";
-			clocks = <&gcc GCC_UFS_REF_CLKREF_CLK>,
+			clocks = <&gcc GCC_UFS_1_CARD_CLKREF_CLK>,
 				 <&gcc GCC_UFS_CARD_PHY_AUX_CLK>;
 
 			resets = <&ufs_card_hc 0>;
Signature · 1 line
2.37.3
Reviewed-by on v1 1/2
#2 lore raw

Re: [PATCH 1/2] arm64: dts: qcom: sc8280xp: fix UFS reference clocks

Quoted 16 lines · Johan Hovold

There are three UFS reference clocks on SC8280XP which are used as follows:

- The GCC_UFS_REF_CLKREF_CLK clock is fed to any UFS device connected to either controller.

- The GCC_UFS_1_CARD_CLKREF_CLK and GCC_UFS_CARD_CLKREF_CLK clocks provide reference clocks to the two PHYs.

Note that this depends on first updating the clock driver to reflect that all three clocks are sourced from CXO. Specifically, the UFS controller driver expects the device reference clock to have a valid frequency:

ufshcd-qcom 1d84000.ufs: invalid ref_clk setting = 0

Quoted 7 lines
Fixes 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform")
Fixes 8d6b458ce6e9 ("arm64: dts: qcom: sc8280xp: fix ufs_card_phy ref clock")
Fixes f3aa975e230e ("arm64: dts: qcom: sc8280xp: correct ref clock for ufs_mem_phy")

---

Konrad

Quoted 43 lines
  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)
arch/arm64/boot/dts/qcom/sc8280xp.dtsi +4 -4
index 21ac119e0382..e0d0fb6994b5 100644
@@ -912,7 +912,7 @@ ufs_mem_hc: ufs@1d84000 {
  				 <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
  				 <&gcc GCC_UFS_PHY_AHB_CLK>,
  				 <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>,
-				 <&rpmhcc RPMH_CXO_CLK>,
+				 <&gcc GCC_UFS_REF_CLKREF_CLK>,
  				 <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>,
  				 <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>,
  				 <&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>;
@@ -943,7 +943,7 @@ ufs_mem_phy: phy@1d87000 {
  			ranges;
  			clock-names = "ref",
  				      "ref_aux";
-			clocks = <&gcc GCC_UFS_REF_CLKREF_CLK>,
+			clocks = <&gcc GCC_UFS_CARD_CLKREF_CLK>,
  				 <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
 
  			resets = <&ufs_mem_hc 0>;
@@ -980,7 +980,7 @@ ufs_card_hc: ufs@1da4000 {
  				 <&gcc GCC_AGGRE_UFS_CARD_AXI_CLK>,
  				 <&gcc GCC_UFS_CARD_AHB_CLK>,
  				 <&gcc GCC_UFS_CARD_UNIPRO_CORE_CLK>,
-				 <&rpmhcc RPMH_CXO_CLK>,
+				 <&gcc GCC_UFS_REF_CLKREF_CLK>,
  				 <&gcc GCC_UFS_CARD_TX_SYMBOL_0_CLK>,
  				 <&gcc GCC_UFS_CARD_RX_SYMBOL_0_CLK>,
  				 <&gcc GCC_UFS_CARD_RX_SYMBOL_1_CLK>;
@@ -1011,7 +1011,7 @@ ufs_card_phy: phy@1da7000 {
  			ranges;
  			clock-names = "ref",
  				      "ref_aux";
-			clocks = <&gcc GCC_UFS_REF_CLKREF_CLK>,
+			clocks = <&gcc GCC_UFS_1_CARD_CLKREF_CLK>,
  				 <&gcc GCC_UFS_CARD_PHY_AUX_CLK>;
 
  			resets = <&ufs_card_hc 0>;
Reviewed-by on v1 1/2
#3 lore raw

Re: [PATCH 1/2] arm64: dts: qcom: sc8280xp: fix UFS reference clocks

In reply to Johan Hovold
Quoted 14 lines · Johan Hovold

There are three UFS reference clocks on SC8280XP which are used as follows:

- The GCC_UFS_REF_CLKREF_CLK clock is fed to any UFS device connected to either controller.

- The GCC_UFS_1_CARD_CLKREF_CLK and GCC_UFS_CARD_CLKREF_CLK clocks provide reference clocks to the two PHYs.

Note that this depends on first updating the clock driver to reflect that all three clocks are sourced from CXO. Specifically, the UFS controller driver expects the device reference clock to have a valid frequency:

Quoted 8 lines

ufshcd-qcom 1d84000.ufs: invalid ref_clk setting = 0

Fixes 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform")
Fixes 8d6b458ce6e9 ("arm64: dts: qcom: sc8280xp: fix ufs_card_phy ref clock")
Fixes f3aa975e230e ("arm64: dts: qcom: sc8280xp: correct ref clock for ufs_mem_phy")

Note that there was no 5.20 kernel; that should be 6.0. Bjorn should be able to fix this up during merge.

https://en.wikipedia.org/wiki/Linux_kernel_version_history

#4 lore raw

Re: [PATCH 1/2] arm64: dts: qcom: sc8280xp: fix UFS reference clocks

Quoted 24 lines · Brian Masney
Quoted 14 lines · Johan Hovold

There are three UFS reference clocks on SC8280XP which are used as follows:

- The GCC_UFS_REF_CLKREF_CLK clock is fed to any UFS device connected to either controller.

- The GCC_UFS_1_CARD_CLKREF_CLK and GCC_UFS_CARD_CLKREF_CLK clocks provide reference clocks to the two PHYs.

Note that this depends on first updating the clock driver to reflect that all three clocks are sourced from CXO. Specifically, the UFS controller driver expects the device reference clock to have a valid frequency:

Quoted 8 lines

ufshcd-qcom 1d84000.ufs: invalid ref_clk setting = 0

Fixes 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform")
Fixes 8d6b458ce6e9 ("arm64: dts: qcom: sc8280xp: fix ufs_card_phy ref clock")
Fixes f3aa975e230e ("arm64: dts: qcom: sc8280xp: correct ref clock for ufs_mem_phy")
Quoted 4 lines

Note that there was no 5.20 kernel; that should be 6.0. Bjorn should be able to fix this up during merge.

Good catch. I based this on a tag created before 6.0 was released and failed to notice.

Johan

#5 lore raw

PATCH 2/2 arm64: dts: qcom: sc8280xp: update UFS PHY nodes

In reply to Johan Hovold

Update the UFS PHY nodes to match the new binding.

---
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 49 +++++++++-----------------
 1 file changed, 17 insertions(+), 32 deletions(-)
arch/arm64/boot/dts/qcom/sc8280xp.dtsi +17 -32
index e0d0fb6994b5..1b309fa93484 100644
@@ -896,7 +896,7 @@ ufs_mem_hc: ufs@1d84000 {
 				     "jedec,ufs-2.0";
 			reg = <0 0x01d84000 0 0x3000>;
 			interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>;
-			phys = <&ufs_mem_phy_lanes>;
+			phys = <&ufs_mem_phy>;
 			phy-names = "ufsphy";
 			lanes-per-direction = <2>;
 			#reset-cells = <1>;
@@ -937,27 +937,20 @@ ufs_mem_hc: ufs@1d84000 {
 
 		ufs_mem_phy: phy@1d87000 {
 			compatible = "qcom,sc8280xp-qmp-ufs-phy";
-			reg = <0 0x01d87000 0 0x1c8>;
-			#address-cells = <2>;
-			#size-cells = <2>;
-			ranges;
-			clock-names = "ref",
-				      "ref_aux";
+			reg = <0 0x01d87000 0 0x1000>;
+
 			clocks = <&gcc GCC_UFS_CARD_CLKREF_CLK>,
 				 <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
+			clock-names = "ref", "ref_aux";
+
+			power-domains = <&gcc UFS_PHY_GDSC>;
 
 			resets = <&ufs_mem_hc 0>;
 			reset-names = "ufsphy";
-			status = "disabled";
 
-			ufs_mem_phy_lanes: phy@1d87400 {
-				reg = <0 0x01d87400 0 0x108>,
-				      <0 0x01d87600 0 0x1e0>,
-				      <0 0x01d87c00 0 0x1dc>,
-				      <0 0x01d87800 0 0x108>,
-				      <0 0x01d87a00 0 0x1e0>;
-				#phy-cells = <0>;
-			};
+			#phy-cells = <0>;
+
+			status = "disabled";
 		};
 
 		ufs_card_hc: ufs@1da4000 {
@@ -965,7 +958,7 @@ ufs_card_hc: ufs@1da4000 {
 				     "jedec,ufs-2.0";
 			reg = <0 0x01da4000 0 0x3000>;
 			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
-			phys = <&ufs_card_phy_lanes>;
+			phys = <&ufs_card_phy>;
 			phy-names = "ufsphy";
 			lanes-per-direction = <2>;
 			#reset-cells = <1>;
@@ -1005,28 +998,20 @@ ufs_card_hc: ufs@1da4000 {
 
 		ufs_card_phy: phy@1da7000 {
 			compatible = "qcom,sc8280xp-qmp-ufs-phy";
-			reg = <0 0x01da7000 0 0x1c8>;
-			#address-cells = <2>;
-			#size-cells = <2>;
-			ranges;
-			clock-names = "ref",
-				      "ref_aux";
+			reg = <0 0x01da7000 0 0x1000>;
+
 			clocks = <&gcc GCC_UFS_1_CARD_CLKREF_CLK>,
 				 <&gcc GCC_UFS_CARD_PHY_AUX_CLK>;
+			clock-names = "ref", "ref_aux";
+
+			power-domains = <&gcc UFS_CARD_GDSC>;
 
 			resets = <&ufs_card_hc 0>;
 			reset-names = "ufsphy";
 
-			status = "disabled";
+			#phy-cells = <0>;
 
-			ufs_card_phy_lanes: phy@1da7400 {
-				reg = <0 0x01da7400 0 0x108>,
-				      <0 0x01da7600 0 0x1e0>,
-				      <0 0x01da7c00 0 0x1dc>,
-				      <0 0x01da7800 0 0x108>,
-				      <0 0x01da7a00 0 0x1e0>;
-				#phy-cells = <0>;
-			};
+			status = "disabled";
 		};
 
 		tcsr_mutex: hwlock@1f40000 {
Signature · 1 line
2.37.3
Reviewed-by on v1 2/2
#6 lore raw

Re: [PATCH 2/2] arm64: dts: qcom: sc8280xp: update UFS PHY nodes

Quoted 4 lines · Johan Hovold

Update the UFS PHY nodes to match the new binding.

---

Konrad

Quoted 97 lines
  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 49 +++++++++-----------------
  1 file changed, 17 insertions(+), 32 deletions(-)
arch/arm64/boot/dts/qcom/sc8280xp.dtsi +17 -32
index e0d0fb6994b5..1b309fa93484 100644
@@ -896,7 +896,7 @@ ufs_mem_hc: ufs@1d84000 {
  				     "jedec,ufs-2.0";
  			reg = <0 0x01d84000 0 0x3000>;
  			interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>;
-			phys = <&ufs_mem_phy_lanes>;
+			phys = <&ufs_mem_phy>;
  			phy-names = "ufsphy";
  			lanes-per-direction = <2>;
  			#reset-cells = <1>;
@@ -937,27 +937,20 @@ ufs_mem_hc: ufs@1d84000 {
 
  		ufs_mem_phy: phy@1d87000 {
  			compatible = "qcom,sc8280xp-qmp-ufs-phy";
-			reg = <0 0x01d87000 0 0x1c8>;
-			#address-cells = <2>;
-			#size-cells = <2>;
-			ranges;
-			clock-names = "ref",
-				      "ref_aux";
+			reg = <0 0x01d87000 0 0x1000>;
+
  			clocks = <&gcc GCC_UFS_CARD_CLKREF_CLK>,
  				 <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
+			clock-names = "ref", "ref_aux";
+
+			power-domains = <&gcc UFS_PHY_GDSC>;
 
  			resets = <&ufs_mem_hc 0>;
  			reset-names = "ufsphy";
-			status = "disabled";
 
-			ufs_mem_phy_lanes: phy@1d87400 {
-				reg = <0 0x01d87400 0 0x108>,
-				      <0 0x01d87600 0 0x1e0>,
-				      <0 0x01d87c00 0 0x1dc>,
-				      <0 0x01d87800 0 0x108>,
-				      <0 0x01d87a00 0 0x1e0>;
-				#phy-cells = <0>;
-			};
+			#phy-cells = <0>;
+
+			status = "disabled";
  		};
 
  		ufs_card_hc: ufs@1da4000 {
@@ -965,7 +958,7 @@ ufs_card_hc: ufs@1da4000 {
  				     "jedec,ufs-2.0";
  			reg = <0 0x01da4000 0 0x3000>;
  			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
-			phys = <&ufs_card_phy_lanes>;
+			phys = <&ufs_card_phy>;
  			phy-names = "ufsphy";
  			lanes-per-direction = <2>;
  			#reset-cells = <1>;
@@ -1005,28 +998,20 @@ ufs_card_hc: ufs@1da4000 {
 
  		ufs_card_phy: phy@1da7000 {
  			compatible = "qcom,sc8280xp-qmp-ufs-phy";
-			reg = <0 0x01da7000 0 0x1c8>;
-			#address-cells = <2>;
-			#size-cells = <2>;
-			ranges;
-			clock-names = "ref",
-				      "ref_aux";
+			reg = <0 0x01da7000 0 0x1000>;
+
  			clocks = <&gcc GCC_UFS_1_CARD_CLKREF_CLK>,
  				 <&gcc GCC_UFS_CARD_PHY_AUX_CLK>;
+			clock-names = "ref", "ref_aux";
+
+			power-domains = <&gcc UFS_CARD_GDSC>;
 
  			resets = <&ufs_card_hc 0>;
  			reset-names = "ufsphy";
 
-			status = "disabled";
+			#phy-cells = <0>;
 
-			ufs_card_phy_lanes: phy@1da7400 {
-				reg = <0 0x01da7400 0 0x108>,
-				      <0 0x01da7600 0 0x1e0>,
-				      <0 0x01da7c00 0 0x1dc>,
-				      <0 0x01da7800 0 0x108>,
-				      <0 0x01da7a00 0 0x1e0>;
-				#phy-cells = <0>;
-			};
+			status = "disabled";
  		};
 
  		tcsr_mutex: hwlock@1f40000 {
#8 lore raw

Re: [PATCH 2/2] arm64: dts: qcom: sc8280xp: update UFS PHY nodes

Quoted 6 lines · Brian Masney
Quoted 3 lines · Johan Hovold

Update the UFS PHY nodes to match the new binding.

This is needed in order to boot the sa8540p on linux-next-20221110.

Actually, I take that back... This is not needed to boot on -next. There was a separate problem on the latest -next that I describe at:

https://lore.kernel.org/linux-arm-msm/[email protected]/T/#u

My Reviewed-by still applies for the patch.

Brian

maintainer Applied
#9 lore raw

Re: (subset) [PATCH 0/2] arm64: dts: qcom: sc8280xp: fix UFS reference clocks and PHY nodes

In reply to Johan Hovold
Quoted 8 lines · Johan Hovold

After some initial confusion, we've finally settled how the UFS ref clocks are used.

The first patch fixes the UFS controller and PHY nodes so that they reflect the hardware. This one should go in 6.1-rc where the two previous attempts to address this are heading.

[...]

Applied, thanks!

[1/2] arm64: dts: qcom: sc8280xp: fix UFS reference clocks

      commit: f446022b932aff1d6a308ca5d537ec2b512debdc

Best regards,

Signature · 1 line
Bjorn Andersson <[email protected]>
maintainer Applied
#10 lore raw

Re: (subset) [PATCH 0/2] arm64: dts: qcom: sc8280xp: fix UFS reference clocks and PHY nodes

In reply to Johan Hovold
Quoted 8 lines · Johan Hovold

After some initial confusion, we've finally settled how the UFS ref clocks are used.

The first patch fixes the UFS controller and PHY nodes so that they reflect the hardware. This one should go in 6.1-rc where the two previous attempts to address this are heading.

[...]

Applied, thanks!

[2/2] arm64: dts: qcom: sc8280xp: update UFS PHY nodes

      commit: 33c4e6588e4f018abc43381ee21fe2bed37e34a5

Best regards,

Signature · 1 line
Bjorn Andersson <[email protected]>