Skip to contentSkip to conversation

arm64: dts: qcom: sc7180: Added xo clock for eMMC and Sd card

PATCH In review Shaik Sajida Bhanu 3 participants 9 messages Jun 10, 2021 – Jun 15, 2021

Subsystems (5)
Device tree bindings Arm Qualcomm mailing list MMC, SD and SDIO Arm Qualcomm support +1 more subsystems
lore
Share & tools
lore

Shaik Sajida Bhanu

author
#0 lore raw

PATCH 1/1 arm64: dts: qcom: sc7180: Added xo clock for eMMC and Sd card

Added xo clock for eMMC and Sd card.

Signed-off-by Shaik Sajida Bhanu <[email protected]>
---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
arch/arm64/boot/dts/qcom/sc7180.dtsi +6 -4
index 295844e..5bb6bd4 100644
@@ -701,8 +701,9 @@
 			interrupt-names = "hc_irq", "pwr_irq";
 
 			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
-					<&gcc GCC_SDCC1_AHB_CLK>;
-			clock-names = "core", "iface";
+					<&gcc GCC_SDCC1_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "core", "iface","xo";
 			interconnects = <&aggre1_noc MASTER_EMMC 0 &mc_virt SLAVE_EBI1 0>,
 					<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_EMMC_CFG 0>;
 			interconnect-names = "sdhc-ddr","cpu-sdhc";

@@ -2666,8 +2667,9 @@
 			interrupt-names = "hc_irq", "pwr_irq";
 
 			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
-					<&gcc GCC_SDCC2_AHB_CLK>;
-			clock-names = "core", "iface";
+					<&gcc GCC_SDCC2_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "core", "iface", "xo";
 
 			interconnects = <&aggre1_noc MASTER_SDCC_2 0 &mc_virt SLAVE_EBI1 0>,
 					<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_SDCC_2 0>;
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member 
of Code Aurora Forum, hosted by The Linux Foundation
#1 lore raw

Hi,

arch/arm64/boot/dts/qcom/sc7180.dtsi line 705

Quoted 10 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +2 -2
@@ -701,8 +701,9 @@
 			interrupt-names = "hc_irq", "pwr_irq";
 
 			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
-					<&gcc GCC_SDCC1_AHB_CLK>;
-			clock-names = "core", "iface";
+					<&gcc GCC_SDCC1_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;

Don't these clocks fit in 100 chars?

arch/arm64/boot/dts/qcom/sc7180.dtsi line 706

Quoted 1 line
arch/arm64/boot/dts/qcom/sc7180.dtsi +1 -0
+			clock-names = "core", "iface","xo";

A space is missing before "xo".

arch/arm64/boot/dts/qcom/sc7180.dtsi line 2671

Quoted 11 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +2 -2
 			interconnects = <&aggre1_noc MASTER_EMMC 0 &mc_virt SLAVE_EBI1 0>,
 					<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_EMMC_CFG 0>;
 			interconnect-names = "sdhc-ddr","cpu-sdhc";

@@ -2666,8 +2667,9 @@
 			interrupt-names = "hc_irq", "pwr_irq";
 
 			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
-					<&gcc GCC_SDCC2_AHB_CLK>;
-			clock-names = "core", "iface";
+					<&gcc GCC_SDCC2_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;

Ditto

Konrad

#2 lore raw
Quoted 8 lines · Konrad Dybcio

Hi,

Quoted 5 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +0 -0
@@ -701,8 +701,9 @@
 			interrupt-names = "hc_irq", "pwr_irq";
Quoted 8 lines

arch/arm64/boot/dts/qcom/sc7180.dtsi line 705

Quoted 5 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +2 -2
 			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
-					<&gcc GCC_SDCC1_AHB_CLK>;
-			clock-names = "core", "iface";
+					<&gcc GCC_SDCC1_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;

Don't these clocks fit in 100 chars?

We typically list them on one line each, so I'm fine with that.

But all three clocks should be aligned.

Quoted 6 lines

arch/arm64/boot/dts/qcom/sc7180.dtsi line 706

Quoted 3 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +1 -0
+			clock-names = "core", "iface","xo";

A space is missing before "xo".

Thanks for pointing this out.

I fixed up the indentation and applied the patch.

Regards,
Bjorn

Quoted 20 lines

arch/arm64/boot/dts/qcom/sc7180.dtsi line 2671

Quoted 13 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +2 -2
 			interconnects = <&aggre1_noc MASTER_EMMC 0 &mc_virt SLAVE_EBI1 0>,
 					<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_EMMC_CFG 0>;
 			interconnect-names = "sdhc-ddr","cpu-sdhc";

@@ -2666,8 +2667,9 @@
 			interrupt-names = "hc_irq", "pwr_irq";
 
 			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
-					<&gcc GCC_SDCC2_AHB_CLK>;
-			clock-names = "core", "iface";
+					<&gcc GCC_SDCC2_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;

Ditto

Konrad

author
#3 lore raw
In reply to Konrad Dybcio
Quoted 8 lines · Konrad Dybcio

Hi,

Quoted 5 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +0 -0
@@ -701,8 +701,9 @@
 			interrupt-names = "hc_irq", "pwr_irq";
Quoted 7 lines

arch/arm64/boot/dts/qcom/sc7180.dtsi line 705

Quoted 5 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +2 -2
 			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
-					<&gcc GCC_SDCC1_AHB_CLK>;
-			clock-names = "core", "iface";
+					<&gcc GCC_SDCC1_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;

Don't these clocks fit in 100 chars?

These two clocks can fit in 100 chars but we have 3 clocks they don't fit in 100 chars.

Quoted 6 lines

arch/arm64/boot/dts/qcom/sc7180.dtsi line 706

Quoted 4 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +1 -0
+			clock-names = "core", "iface","xo";

A space is missing before "xo".

Sure

Quoted 9 lines
Quoted 9 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +0 -0
 			interconnects = <&aggre1_noc MASTER_EMMC 0 &mc_virt SLAVE_EBI1 0>,
 					<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_EMMC_CFG 0>;
 			interconnect-names = "sdhc-ddr","cpu-sdhc";

@@ -2666,8 +2667,9 @@
 			interrupt-names = "hc_irq", "pwr_irq";
Quoted 7 lines

arch/arm64/boot/dts/qcom/sc7180.dtsi line 2671

Quoted 5 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +2 -2
 			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
-					<&gcc GCC_SDCC2_AHB_CLK>;
-			clock-names = "core", "iface";
+					<&gcc GCC_SDCC2_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;

Ditto

Same as above

Quoted 4 lines

Konrad

Reviewed-by
#4 lore raw
Quoted 9 lines · On 14.06.2021 13:44, [email protected] wrote:
Quoted 8 lines · Konrad Dybcio

Hi,

Quoted 5 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +0 -0
@@ -701,8 +701,9 @@
              interrupt-names = "hc_irq", "pwr_irq";
Quoted 8 lines
Quoted 7 lines

arch/arm64/boot/dts/qcom/sc7180.dtsi line 705

Quoted 5 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +2 -2
              clocks = <&gcc GCC_SDCC1_APPS_CLK>,
-                    <&gcc GCC_SDCC1_AHB_CLK>;
-            clock-names = "core", "iface";
+                    <&gcc GCC_SDCC1_AHB_CLK>,
+                    <&rpmhcc RPMH_CXO_CLK>;

Don't these clocks fit in 100 chars?

These two clocks can fit in 100 chars but we have 3 clocks they don't fit in 100 chars.

You're right.

With the space before xo,

Reviewed-by Konrad Dybcio <[email protected]>

Konrad

#5 lore raw
In reply to Shaik Sajida Bhanu
Quoted 1 line · Shaik Sajida Bhanu

Added xo clock for eMMC and Sd card.

Was about to push out my branch of patches, but before I do. Can you please describe WHY this is needed?

Regards,
Bjorn

Quoted 38 lines
Signed-off-by Shaik Sajida Bhanu <[email protected]>
---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
arch/arm64/boot/dts/qcom/sc7180.dtsi +6 -4
index 295844e..5bb6bd4 100644
@@ -701,8 +701,9 @@
 			interrupt-names = "hc_irq", "pwr_irq";
 
 			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
-					<&gcc GCC_SDCC1_AHB_CLK>;
-			clock-names = "core", "iface";
+					<&gcc GCC_SDCC1_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "core", "iface","xo";
 			interconnects = <&aggre1_noc MASTER_EMMC 0 &mc_virt SLAVE_EBI1 0>,
 					<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_EMMC_CFG 0>;
 			interconnect-names = "sdhc-ddr","cpu-sdhc";

@@ -2666,8 +2667,9 @@
 			interrupt-names = "hc_irq", "pwr_irq";
 
 			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
-					<&gcc GCC_SDCC2_AHB_CLK>;
-			clock-names = "core", "iface";
+					<&gcc GCC_SDCC2_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "core", "iface", "xo";
 
 			interconnects = <&aggre1_noc MASTER_SDCC_2 0 &mc_virt SLAVE_EBI1 0>,
 					<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_SDCC_2 0>;
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member 
of Code Aurora Forum, hosted by The Linux Foundation
author
#6 lore raw
Quoted 9 lines · Bjorn Andersson
Quoted 1 line · Shaik Sajida Bhanu

Added xo clock for eMMC and Sd card.

Was about to push out my branch of patches, but before I do. Can you please describe WHY this is needed?

Regards,
Bjorn

We are making use of this clock in dll register value calculation, The default PoR value is also same as calculated value for HS200/HS400/SDR104 modes.
But just not to rely on default register values we need this entry.

Quoted 40 lines
Quoted 40 lines
Signed-off-by Shaik Sajida Bhanu <[email protected]>
---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
arch/arm64/boot/dts/qcom/sc7180.dtsi +6 -4
index 295844e..5bb6bd4 100644
@@ -701,8 +701,9 @@
 			interrupt-names = "hc_irq", "pwr_irq";
 
 			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
-					<&gcc GCC_SDCC1_AHB_CLK>;
-			clock-names = "core", "iface";
+					<&gcc GCC_SDCC1_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "core", "iface","xo";
 			interconnects = <&aggre1_noc MASTER_EMMC 0 &mc_virt SLAVE_EBI1 0>,
 					<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_EMMC_CFG 0>;
 			interconnect-names = "sdhc-ddr","cpu-sdhc";

@@ -2666,8 +2667,9 @@
 			interrupt-names = "hc_irq", "pwr_irq";
 
 			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
-					<&gcc GCC_SDCC2_AHB_CLK>;
-			clock-names = "core", "iface";
+					<&gcc GCC_SDCC2_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "core", "iface", "xo";
 
 			interconnects = <&aggre1_noc MASTER_SDCC_2 0 &mc_virt SLAVE_EBI1 
 0>,

<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_SDCC_2 0>;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

#7 lore raw
Quoted 8 lines · On Mon 14 Jun 06:55 CDT 2021, [email protected] wrote:
Quoted 7 lines · Bjorn Andersson
Quoted 1 line · Shaik Sajida Bhanu

Added xo clock for eMMC and Sd card.

Was about to push out my branch of patches, but before I do. Can you please describe WHY this is needed?

Quoted 8 lines
Quoted 2 lines

Regards,
Bjorn

We are making use of this clock in dll register value calculation, The default PoR value is also same as calculated value for HS200/HS400/SDR104 modes.
But just not to rely on default register values we need this entry.

That is the perfect thing to include in a commit message!

I rewrote yours and applied the change, but please next time do describe the "why" of your change.

Regards,
Bjorn

Quoted 42 lines
Quoted 42 lines
Quoted 42 lines
Signed-off-by Shaik Sajida Bhanu <[email protected]>
---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
arch/arm64/boot/dts/qcom/sc7180.dtsi +6 -4
index 295844e..5bb6bd4 100644
@@ -701,8 +701,9 @@
 			interrupt-names = "hc_irq", "pwr_irq";
 
 			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
-					<&gcc GCC_SDCC1_AHB_CLK>;
-			clock-names = "core", "iface";
+					<&gcc GCC_SDCC1_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "core", "iface","xo";
 			interconnects = <&aggre1_noc MASTER_EMMC 0 &mc_virt SLAVE_EBI1 0>,
 					<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_EMMC_CFG 0>;
 			interconnect-names = "sdhc-ddr","cpu-sdhc";

@@ -2666,8 +2667,9 @@
 			interrupt-names = "hc_irq", "pwr_irq";
 
 			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
-					<&gcc GCC_SDCC2_AHB_CLK>;
-			clock-names = "core", "iface";
+					<&gcc GCC_SDCC2_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "core", "iface", "xo";
 
 			interconnects = <&aggre1_noc MASTER_SDCC_2 0 &mc_virt SLAVE_EBI1
 0>,

<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_SDCC_2 0>;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation

author
#8 lore raw
Quoted 19 lines · Bjorn Andersson
Quoted 8 lines · On Mon 14 Jun 06:55 CDT 2021, [email protected] wrote:
Quoted 7 lines · Bjorn Andersson
Quoted 1 line · Shaik Sajida Bhanu

Added xo clock for eMMC and Sd card.

Was about to push out my branch of patches, but before I do. Can you please describe WHY this is needed?

Quoted 8 lines
Quoted 2 lines

Regards,
Bjorn

We are making use of this clock in dll register value calculation, The default PoR value is also same as calculated value for HS200/HS400/SDR104 modes.
But just not to rely on default register values we need this entry.

Quoted 7 lines

That is the perfect thing to include in a commit message!

I rewrote yours and applied the change, but please next time do describe the "why" of your change.

Regards,
Bjorn

Sure

Quoted 43 lines
Quoted 43 lines
Quoted 43 lines
Quoted 43 lines
Signed-off-by Shaik Sajida Bhanu <[email protected]>
---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
arch/arm64/boot/dts/qcom/sc7180.dtsi +6 -4
index 295844e..5bb6bd4 100644
@@ -701,8 +701,9 @@
 			interrupt-names = "hc_irq", "pwr_irq";
 
 			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
-					<&gcc GCC_SDCC1_AHB_CLK>;
-			clock-names = "core", "iface";
+					<&gcc GCC_SDCC1_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "core", "iface","xo";
 			interconnects = <&aggre1_noc MASTER_EMMC 0 &mc_virt SLAVE_EBI1 0>,
 					<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_EMMC_CFG 0>;
 			interconnect-names = "sdhc-ddr","cpu-sdhc";

@@ -2666,8 +2667,9 @@
 			interrupt-names = "hc_irq", "pwr_irq";
 
 			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
-					<&gcc GCC_SDCC2_AHB_CLK>;
-			clock-names = "core", "iface";
+					<&gcc GCC_SDCC2_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "core", "iface", "xo";
 
 			interconnects = <&aggre1_noc MASTER_SDCC_2 0 &mc_virt SLAVE_EBI1
 0>,

<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_SDCC_2 0>;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation