linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] clk: qcom: smd: Add support for MSM8998 rpm clocks
@ 2018-12-06 22:11 Jeffrey Hugo
  2018-12-07 10:30 ` Marc Gonzalez
  2018-12-07 14:23 ` Marc Gonzalez
  0 siblings, 2 replies; 7+ messages in thread
From: Jeffrey Hugo @ 2018-12-06 22:11 UTC (permalink / raw)
  To: mturquette, sboyd, robh+dt, mark.rutland
  Cc: bjorn.andersson, marc.w.gonzalez, linux-clk, devicetree,
	linux-arm-msm, linux-kernel, Jeffrey Hugo

Add rpm smd clocks, PMIC and bus clocks which are required on MSM8998
for clients to vote on.

Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org>
---
v2
-fix compatible ordering nits per Stephen

 .../devicetree/bindings/clock/qcom,rpmcc.txt       |  1 +
 drivers/clk/qcom/clk-smd-rpm.c                     | 62 ++++++++++++++++++++++
 include/dt-bindings/clock/qcom,rpmcc.h             |  6 +++
 3 files changed, 69 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt b/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
index 87b4949..944719b 100644
--- a/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
+++ b/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
@@ -16,6 +16,7 @@ Required properties :
 			"qcom,rpmcc-msm8974", "qcom,rpmcc"
 			"qcom,rpmcc-apq8064", "qcom,rpmcc"
 			"qcom,rpmcc-msm8996", "qcom,rpmcc"
+			"qcom,rpmcc-msm8998", "qcom,rpmcc"
 			"qcom,rpmcc-qcs404", "qcom,rpmcc"
 
 - #clock-cells : shall contain 1
diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index d3aadae..b1f95a9 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -655,10 +655,72 @@ static int clk_smd_rpm_enable_scaling(struct qcom_smd_rpm *rpm)
 	.num_clks = ARRAY_SIZE(qcs404_clks),
 };
 
+/* msm8998 */
+DEFINE_CLK_SMD_RPM(msm8998, snoc_clk, snoc_a_clk, QCOM_SMD_RPM_BUS_CLK, 1);
+DEFINE_CLK_SMD_RPM(msm8998, cnoc_clk, cnoc_a_clk, QCOM_SMD_RPM_BUS_CLK, 2);
+DEFINE_CLK_SMD_RPM(msm8998, ce1_clk, ce1_a_clk, QCOM_SMD_RPM_CE_CLK, 0);
+DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, div_clk1, div_clk1_a, 0xb);
+DEFINE_CLK_SMD_RPM(msm8998, ipa_clk, ipa_a_clk, QCOM_SMD_RPM_IPA_CLK, 0);
+DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, bb_clk1, bb_clk1_a, 1);
+DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, bb_clk2, bb_clk2_a, 2);
+DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(msm8998, bb_clk3_pin, bb_clk3_a_pin, 3);
+DEFINE_CLK_SMD_RPM(msm8998, mmssnoc_axi_rpm_clk, mmssnoc_axi_rpm_a_clk,
+		   QCOM_SMD_RPM_MMAXI_CLK, 0);
+DEFINE_CLK_SMD_RPM(msm8998, aggre1_noc_clk, aggre1_noc_a_clk,
+		   QCOM_SMD_RPM_AGGR_CLK, 1);
+DEFINE_CLK_SMD_RPM(msm8998, aggre2_noc_clk, aggre2_noc_a_clk,
+		   QCOM_SMD_RPM_AGGR_CLK, 2);
+DEFINE_CLK_SMD_RPM_QDSS(msm8998, qdss_clk, qdss_a_clk,
+			QCOM_SMD_RPM_MISC_CLK, 1);
+DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, rf_clk1, rf_clk1_a, 4);
+DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(msm8998, rf_clk2_pin, rf_clk2_a_pin, 5);
+DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, rf_clk3, rf_clk3_a, 6);
+DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(msm8998, rf_clk3_pin, rf_clk3_a_pin, 6);
+static struct clk_smd_rpm *msm8998_clks[] = {
+	[RPM_SMD_SNOC_CLK] = &msm8998_snoc_clk,
+	[RPM_SMD_SNOC_A_CLK] = &msm8998_snoc_a_clk,
+	[RPM_SMD_CNOC_CLK] = &msm8998_cnoc_clk,
+	[RPM_SMD_CNOC_A_CLK] = &msm8998_cnoc_a_clk,
+	[RPM_SMD_CE1_CLK] = &msm8998_ce1_clk,
+	[RPM_SMD_CE1_A_CLK] = &msm8998_ce1_a_clk,
+	[RPM_SMD_DIV_CLK1] = &msm8998_div_clk1,
+	[RPM_SMD_DIV_A_CLK1] = &msm8998_div_clk1_a,
+	[RPM_SMD_IPA_CLK] = &msm8998_ipa_clk,
+	[RPM_SMD_IPA_A_CLK] = &msm8998_ipa_a_clk,
+	[RPM_SMD_BB_CLK1] = &msm8998_bb_clk1,
+	[RPM_SMD_BB_CLK1_A] = &msm8998_bb_clk1_a,
+	[RPM_SMD_BB_CLK2] = &msm8998_bb_clk2,
+	[RPM_SMD_BB_CLK2_A] = &msm8998_bb_clk2_a,
+	[RPM_SMD_BB_CLK3_PIN] = &msm8998_bb_clk3_pin,
+	[RPM_SMD_BB_CLK3_A_PIN] = &msm8998_bb_clk3_a_pin,
+	[RPM_SMD_MMAXI_CLK] = &msm8998_mmssnoc_axi_rpm_clk,
+	[RPM_SMD_MMAXI_A_CLK] = &msm8998_mmssnoc_axi_rpm_a_clk,
+	[RPM_SMD_AGGR1_NOC_CLK] = &msm8998_aggre1_noc_clk,
+	[RPM_SMD_AGGR1_NOC_A_CLK] = &msm8998_aggre1_noc_a_clk,
+	[RPM_SMD_AGGR2_NOC_CLK] = &msm8998_aggre2_noc_clk,
+	[RPM_SMD_AGGR2_NOC_A_CLK] = &msm8998_aggre2_noc_a_clk,
+	[RPM_SMD_QDSS_CLK] = &msm8998_qdss_clk,
+	[RPM_SMD_QDSS_A_CLK] = &msm8998_qdss_a_clk,
+	[RPM_SMD_RF_CLK1] = &msm8998_rf_clk1,
+	[RPM_SMD_RF_CLK1_A] = &msm8998_rf_clk1_a,
+	[RPM_SMD_RF_CLK2_PIN] = &msm8998_rf_clk2_pin,
+	[RPM_SMD_RF_CLK2_A_PIN] = &msm8998_rf_clk2_a_pin,
+	[RPM_SMD_RF_CLK3] = &msm8998_rf_clk3,
+	[RPM_SMD_RF_CLK3_A] = &msm8998_rf_clk3_a,
+	[RPM_SMD_RF_CLK3_PIN] = &msm8998_rf_clk3_pin,
+	[RPM_SMD_RF_CLK3_A_PIN] = &msm8998_rf_clk3_a_pin,
+};
+
+static const struct rpm_smd_clk_desc rpm_clk_msm8998 = {
+	.clks = msm8998_clks,
+	.num_clks = ARRAY_SIZE(msm8998_clks),
+};
+
 static const struct of_device_id rpm_smd_clk_match_table[] = {
 	{ .compatible = "qcom,rpmcc-msm8916", .data = &rpm_clk_msm8916 },
 	{ .compatible = "qcom,rpmcc-msm8974", .data = &rpm_clk_msm8974 },
 	{ .compatible = "qcom,rpmcc-msm8996", .data = &rpm_clk_msm8996 },
+	{ .compatible = "qcom,rpmcc-msm8998", .data = &rpm_clk_msm8998 },
 	{ .compatible = "qcom,rpmcc-qcs404",  .data = &rpm_clk_qcs404  },
 	{ }
 };
diff --git a/include/dt-bindings/clock/qcom,rpmcc.h b/include/dt-bindings/clock/qcom,rpmcc.h
index 3658b0c..81dbd1f 100644
--- a/include/dt-bindings/clock/qcom,rpmcc.h
+++ b/include/dt-bindings/clock/qcom,rpmcc.h
@@ -127,5 +127,11 @@
 #define RPM_SMD_BIMC_GPU_A_CLK			77
 #define RPM_SMD_QPIC_CLK			78
 #define RPM_SMD_QPIC_CLK_A			79
+#define RPM_SMD_BB_CLK3_PIN			80
+#define RPM_SMD_BB_CLK3_A_PIN			81
+#define RPM_SMD_RF_CLK3				82
+#define RPM_SMD_RF_CLK3_A			83
+#define RPM_SMD_RF_CLK3_PIN			84
+#define RPM_SMD_RF_CLK3_A_PIN			85
 
 #endif
-- 
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] clk: qcom: smd: Add support for MSM8998 rpm clocks
  2018-12-06 22:11 [PATCH v2] clk: qcom: smd: Add support for MSM8998 rpm clocks Jeffrey Hugo
@ 2018-12-07 10:30 ` Marc Gonzalez
  2018-12-07 15:10   ` Jeffrey Hugo
  2018-12-07 14:23 ` Marc Gonzalez
  1 sibling, 1 reply; 7+ messages in thread
From: Marc Gonzalez @ 2018-12-07 10:30 UTC (permalink / raw)
  To: Jeffrey Hugo, Michael Turquette, Stephen Boyd
  Cc: Rob Herring, Mark Rutland, Bjorn Andersson, linux-clk, MSM, LKML

On 06/12/2018 23:11, Jeffrey Hugo wrote:

> Add rpm smd clocks, PMIC and bus clocks which are required on MSM8998
> for clients to vote on.
> 
> Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org>
> ---
> v2
> -fix compatible ordering nits per Stephen
> 
>  .../devicetree/bindings/clock/qcom,rpmcc.txt       |  1 +
>  drivers/clk/qcom/clk-smd-rpm.c                     | 62 ++++++++++++++++++++++
>  include/dt-bindings/clock/qcom,rpmcc.h             |  6 +++
>  3 files changed, 69 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt b/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
> index 87b4949..944719b 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
> +++ b/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
> @@ -16,6 +16,7 @@ Required properties :
>  			"qcom,rpmcc-msm8974", "qcom,rpmcc"
>  			"qcom,rpmcc-apq8064", "qcom,rpmcc"
>  			"qcom,rpmcc-msm8996", "qcom,rpmcc"
> +			"qcom,rpmcc-msm8998", "qcom,rpmcc"
>  			"qcom,rpmcc-qcs404", "qcom,rpmcc"
>  
>  - #clock-cells : shall contain 1
> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
> index d3aadae..b1f95a9 100644
> --- a/drivers/clk/qcom/clk-smd-rpm.c
> +++ b/drivers/clk/qcom/clk-smd-rpm.c
> @@ -655,10 +655,72 @@ static int clk_smd_rpm_enable_scaling(struct qcom_smd_rpm *rpm)
>  	.num_clks = ARRAY_SIZE(qcs404_clks),
>  };
>  
> +/* msm8998 */
> +DEFINE_CLK_SMD_RPM(msm8998, snoc_clk, snoc_a_clk, QCOM_SMD_RPM_BUS_CLK, 1);
> +DEFINE_CLK_SMD_RPM(msm8998, cnoc_clk, cnoc_a_clk, QCOM_SMD_RPM_BUS_CLK, 2);
> +DEFINE_CLK_SMD_RPM(msm8998, ce1_clk, ce1_a_clk, QCOM_SMD_RPM_CE_CLK, 0);
> +DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, div_clk1, div_clk1_a, 0xb);
> +DEFINE_CLK_SMD_RPM(msm8998, ipa_clk, ipa_a_clk, QCOM_SMD_RPM_IPA_CLK, 0);
> +DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, bb_clk1, bb_clk1_a, 1);
> +DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, bb_clk2, bb_clk2_a, 2);
> +DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(msm8998, bb_clk3_pin, bb_clk3_a_pin, 3);
> +DEFINE_CLK_SMD_RPM(msm8998, mmssnoc_axi_rpm_clk, mmssnoc_axi_rpm_a_clk,
> +		   QCOM_SMD_RPM_MMAXI_CLK, 0);
> +DEFINE_CLK_SMD_RPM(msm8998, aggre1_noc_clk, aggre1_noc_a_clk,
> +		   QCOM_SMD_RPM_AGGR_CLK, 1);
> +DEFINE_CLK_SMD_RPM(msm8998, aggre2_noc_clk, aggre2_noc_a_clk,
> +		   QCOM_SMD_RPM_AGGR_CLK, 2);
> +DEFINE_CLK_SMD_RPM_QDSS(msm8998, qdss_clk, qdss_a_clk,
> +			QCOM_SMD_RPM_MISC_CLK, 1);
> +DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, rf_clk1, rf_clk1_a, 4);
> +DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(msm8998, rf_clk2_pin, rf_clk2_a_pin, 5);
> +DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, rf_clk3, rf_clk3_a, 6);
> +DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(msm8998, rf_clk3_pin, rf_clk3_a_pin, 6);
> +static struct clk_smd_rpm *msm8998_clks[] = {
> +	[RPM_SMD_SNOC_CLK] = &msm8998_snoc_clk,
> +	[RPM_SMD_SNOC_A_CLK] = &msm8998_snoc_a_clk,
> +	[RPM_SMD_CNOC_CLK] = &msm8998_cnoc_clk,
> +	[RPM_SMD_CNOC_A_CLK] = &msm8998_cnoc_a_clk,
> +	[RPM_SMD_CE1_CLK] = &msm8998_ce1_clk,
> +	[RPM_SMD_CE1_A_CLK] = &msm8998_ce1_a_clk,
> +	[RPM_SMD_DIV_CLK1] = &msm8998_div_clk1,
> +	[RPM_SMD_DIV_A_CLK1] = &msm8998_div_clk1_a,
> +	[RPM_SMD_IPA_CLK] = &msm8998_ipa_clk,
> +	[RPM_SMD_IPA_A_CLK] = &msm8998_ipa_a_clk,
> +	[RPM_SMD_BB_CLK1] = &msm8998_bb_clk1,
> +	[RPM_SMD_BB_CLK1_A] = &msm8998_bb_clk1_a,
> +	[RPM_SMD_BB_CLK2] = &msm8998_bb_clk2,
> +	[RPM_SMD_BB_CLK2_A] = &msm8998_bb_clk2_a,
> +	[RPM_SMD_BB_CLK3_PIN] = &msm8998_bb_clk3_pin,
> +	[RPM_SMD_BB_CLK3_A_PIN] = &msm8998_bb_clk3_a_pin,
> +	[RPM_SMD_MMAXI_CLK] = &msm8998_mmssnoc_axi_rpm_clk,
> +	[RPM_SMD_MMAXI_A_CLK] = &msm8998_mmssnoc_axi_rpm_a_clk,
> +	[RPM_SMD_AGGR1_NOC_CLK] = &msm8998_aggre1_noc_clk,
> +	[RPM_SMD_AGGR1_NOC_A_CLK] = &msm8998_aggre1_noc_a_clk,
> +	[RPM_SMD_AGGR2_NOC_CLK] = &msm8998_aggre2_noc_clk,
> +	[RPM_SMD_AGGR2_NOC_A_CLK] = &msm8998_aggre2_noc_a_clk,
> +	[RPM_SMD_QDSS_CLK] = &msm8998_qdss_clk,
> +	[RPM_SMD_QDSS_A_CLK] = &msm8998_qdss_a_clk,
> +	[RPM_SMD_RF_CLK1] = &msm8998_rf_clk1,
> +	[RPM_SMD_RF_CLK1_A] = &msm8998_rf_clk1_a,
> +	[RPM_SMD_RF_CLK2_PIN] = &msm8998_rf_clk2_pin,
> +	[RPM_SMD_RF_CLK2_A_PIN] = &msm8998_rf_clk2_a_pin,
> +	[RPM_SMD_RF_CLK3] = &msm8998_rf_clk3,
> +	[RPM_SMD_RF_CLK3_A] = &msm8998_rf_clk3_a,
> +	[RPM_SMD_RF_CLK3_PIN] = &msm8998_rf_clk3_pin,
> +	[RPM_SMD_RF_CLK3_A_PIN] = &msm8998_rf_clk3_a_pin,
> +};
> +
> +static const struct rpm_smd_clk_desc rpm_clk_msm8998 = {
> +	.clks = msm8998_clks,
> +	.num_clks = ARRAY_SIZE(msm8998_clks),
> +};
> +
>  static const struct of_device_id rpm_smd_clk_match_table[] = {
>  	{ .compatible = "qcom,rpmcc-msm8916", .data = &rpm_clk_msm8916 },
>  	{ .compatible = "qcom,rpmcc-msm8974", .data = &rpm_clk_msm8974 },
>  	{ .compatible = "qcom,rpmcc-msm8996", .data = &rpm_clk_msm8996 },
> +	{ .compatible = "qcom,rpmcc-msm8998", .data = &rpm_clk_msm8998 },
>  	{ .compatible = "qcom,rpmcc-qcs404",  .data = &rpm_clk_qcs404  },
>  	{ }
>  };
> diff --git a/include/dt-bindings/clock/qcom,rpmcc.h b/include/dt-bindings/clock/qcom,rpmcc.h
> index 3658b0c..81dbd1f 100644
> --- a/include/dt-bindings/clock/qcom,rpmcc.h
> +++ b/include/dt-bindings/clock/qcom,rpmcc.h
> @@ -127,5 +127,11 @@
>  #define RPM_SMD_BIMC_GPU_A_CLK			77
>  #define RPM_SMD_QPIC_CLK			78
>  #define RPM_SMD_QPIC_CLK_A			79
> +#define RPM_SMD_BB_CLK3_PIN			80
> +#define RPM_SMD_BB_CLK3_A_PIN			81
> +#define RPM_SMD_RF_CLK3				82
> +#define RPM_SMD_RF_CLK3_A			83
> +#define RPM_SMD_RF_CLK3_PIN			84
> +#define RPM_SMD_RF_CLK3_A_PIN			85
>  
>  #endif
> 

What's the difference between RPM_SMD_LN_BB_CLK and RPM_SMD_BB_CLK1?

$ git grep RPM_SMD_LN_BB_CLK
arch/arm64/boot/dts/qcom/msm8996.dtsi:                  clocks = <&rpmcc RPM_SMD_LN_BB_CLK>,
arch/arm64/boot/dts/qcom/msm8996.dtsi:                          <&rpmcc RPM_SMD_LN_BB_CLK>,
drivers/clk/qcom/clk-smd-rpm.c: [RPM_SMD_LN_BB_CLK] = &msm8996_ln_bb_clk,
drivers/clk/qcom/clk-smd-rpm.c: [RPM_SMD_LN_BB_CLK] = &qcs404_ln_bb_clk,
include/dt-bindings/clock/qcom,rpmcc.h:#define RPM_SMD_LN_BB_CLK                        74


$ git grep 'RPM_SMD_BB_CLK1\>'
Documentation/devicetree/bindings/sound/qcom,wcd9335.txt:                <&rpmcc RPM_SMD_BB_CLK1>;
drivers/clk/qcom/clk-smd-rpm.c: [RPM_SMD_BB_CLK1]               = &msm8916_bb_clk1,
drivers/clk/qcom/clk-smd-rpm.c: [RPM_SMD_BB_CLK1] = &msm8996_bb_clk1,
drivers/clk/qcom/clk-smd-rpm.c: [RPM_SMD_BB_CLK1] = &msm8998_bb_clk1,
include/dt-bindings/clock/qcom,rpmcc.h:#define RPM_SMD_BB_CLK1                          10


Because the downstream kernel defines:

DEFINE_CLK_RPM_SMD_XO_BUFFER(ln_bb_clk1, ln_bb_clk1_ao, LN_BB_CLK1_ID);
DEFINE_CLK_RPM_SMD_XO_BUFFER_PINCTRL(ln_bb_clk1_pin, ln_bb_clk1_pin_ao,
				     LN_BB_CLK1_PIN_ID);
DEFINE_CLK_RPM_SMD_XO_BUFFER(ln_bb_clk2, ln_bb_clk2_ao, LN_BB_CLK2_ID);
DEFINE_CLK_RPM_SMD_XO_BUFFER_PINCTRL(ln_bb_clk2_pin, ln_bb_clk2_pin_ao,
				     LN_BB_CLK2_PIN_ID);
DEFINE_CLK_RPM_SMD_XO_BUFFER(ln_bb_clk3, ln_bb_clk3_ao, LN_BB_CLK3_ID);
DEFINE_CLK_RPM_SMD_XO_BUFFER_PINCTRL(ln_bb_clk3_pin, ln_bb_clk3_pin_ao,
				     LN_BB_CLK3_PIN_ID);

	CLK_LIST(ln_bb_clk1),
	CLK_LIST(ln_bb_clk1_ao),
	CLK_LIST(ln_bb_clk1_pin),
	CLK_LIST(ln_bb_clk1_pin_ao),
	CLK_LIST(ln_bb_clk2),
	CLK_LIST(ln_bb_clk2_ao),
	CLK_LIST(ln_bb_clk2_pin),
	CLK_LIST(ln_bb_clk2_pin_ao),
	CLK_LIST(ln_bb_clk3),
	CLK_LIST(ln_bb_clk3_ao),
	CLK_LIST(ln_bb_clk3_pin),
	CLK_LIST(ln_bb_clk3_pin_ao),


While your patch defines

DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, bb_clk1, bb_clk1_a, 1);
DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, bb_clk2, bb_clk2_a, 2);
DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(msm8998, bb_clk3_pin, bb_clk3_a_pin, 3);

	[RPM_SMD_BB_CLK1] = &msm8998_bb_clk1,
	[RPM_SMD_BB_CLK1_A] = &msm8998_bb_clk1_a,
	[RPM_SMD_BB_CLK2] = &msm8998_bb_clk2,
	[RPM_SMD_BB_CLK2_A] = &msm8998_bb_clk2_a,
	[RPM_SMD_BB_CLK3_PIN] = &msm8998_bb_clk3_pin,
	[RPM_SMD_BB_CLK3_A_PIN] = &msm8998_bb_clk3_a_pin,


Regards.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] clk: qcom: smd: Add support for MSM8998 rpm clocks
  2018-12-06 22:11 [PATCH v2] clk: qcom: smd: Add support for MSM8998 rpm clocks Jeffrey Hugo
  2018-12-07 10:30 ` Marc Gonzalez
@ 2018-12-07 14:23 ` Marc Gonzalez
  2018-12-07 15:13   ` Jeffrey Hugo
  1 sibling, 1 reply; 7+ messages in thread
From: Marc Gonzalez @ 2018-12-07 14:23 UTC (permalink / raw)
  To: Jeffrey Hugo, Michael Turquette, Stephen Boyd
  Cc: Bjorn Andersson, linux-clk, MSM, LKML

On 06/12/2018 23:11, Jeffrey Hugo wrote:

> Add rpm smd clocks, PMIC and bus clocks which are required on MSM8998
> for clients to vote on.
> 
> Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org>
> ---
> v2
> -fix compatible ordering nits per Stephen
> 
>  .../devicetree/bindings/clock/qcom,rpmcc.txt       |  1 +
>  drivers/clk/qcom/clk-smd-rpm.c                     | 62 ++++++++++++++++++++++
>  include/dt-bindings/clock/qcom,rpmcc.h             |  6 +++
>  3 files changed, 69 insertions(+)

Hmmm, my board seems to dislike this patch... it locks up for a while,
then reboots.

[    6.957289] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 1 failed, err = -11
[    6.961233] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry: failed with error -11, retries 2
[    6.970782] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry: query attribute, opcode 6, idn 1, failed with error -11 after 3 retires
[    6.979576] ufshcd-qcom 1da4000.ufshc: ufshcd_complete_dev_init setting fDeviceInit flag failed with error -11
[   23.432798] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
[   23.432884] rcu:     7-...0: (1 GPs behind) idle=6a2/1/0x4000000000000000 softirq=92/93 fqs=2626 
[   23.437927] rcu:     (detected by 0, t=5252 jiffies, g=-819, q=112)
[   23.446425] Task dump for CPU 7:
[   23.452567] swapper/0       R  running task        0     1      0 0x0000002a
[   23.455806] Call trace:
[   23.462872]  __switch_to+0x94/0xe0
[   23.465009]  _regmap_write+0x58/0xb0
[   23.468452]  _regmap_update_bits+0xf0/0x110
[   23.472186]  regmap_update_bits_base+0x60/0x90
[   23.476110]  clk_disable_regmap+0x34/0x40
[   23.480614]  clk_branch_toggle+0x108/0x1b0
[   23.484681]  clk_branch2_disable+0x18/0x20
[   23.488692]  clk_disable_unused_subtree+0xc4/0xe0
[   23.492761]  clk_disable_unused+0x3c/0x130
[   23.497535]  do_one_initcall+0x5c/0x180
[   23.501557]  kernel_init_freeable+0x198/0x244
[   23.505276]  kernel_init+0x10/0x110
[   23.509768]  ret_from_fork+0x10/0x20

/*** REBOOT ***/
Format: Log Type - Time(microsec) - Message - Optional Info
Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic

I need to check the ufs_reset pin and phy init seq before I can investigate
this issue, but I wanted to send a report ASAP.

Regards.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] clk: qcom: smd: Add support for MSM8998 rpm clocks
  2018-12-07 10:30 ` Marc Gonzalez
@ 2018-12-07 15:10   ` Jeffrey Hugo
  2018-12-07 16:08     ` Jeffrey Hugo
  0 siblings, 1 reply; 7+ messages in thread
From: Jeffrey Hugo @ 2018-12-07 15:10 UTC (permalink / raw)
  To: Marc Gonzalez, Michael Turquette, Stephen Boyd
  Cc: Rob Herring, Mark Rutland, Bjorn Andersson, linux-clk, MSM, LKML

On 12/7/2018 3:30 AM, Marc Gonzalez wrote:
> On 06/12/2018 23:11, Jeffrey Hugo wrote:
> 
>> Add rpm smd clocks, PMIC and bus clocks which are required on MSM8998
>> for clients to vote on.
>>
>> Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org>
>> ---
>> v2
>> -fix compatible ordering nits per Stephen
>>
>>   .../devicetree/bindings/clock/qcom,rpmcc.txt       |  1 +
>>   drivers/clk/qcom/clk-smd-rpm.c                     | 62 ++++++++++++++++++++++
>>   include/dt-bindings/clock/qcom,rpmcc.h             |  6 +++
>>   3 files changed, 69 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt b/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
>> index 87b4949..944719b 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
>> +++ b/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
>> @@ -16,6 +16,7 @@ Required properties :
>>   			"qcom,rpmcc-msm8974", "qcom,rpmcc"
>>   			"qcom,rpmcc-apq8064", "qcom,rpmcc"
>>   			"qcom,rpmcc-msm8996", "qcom,rpmcc"
>> +			"qcom,rpmcc-msm8998", "qcom,rpmcc"
>>   			"qcom,rpmcc-qcs404", "qcom,rpmcc"
>>   
>>   - #clock-cells : shall contain 1
>> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
>> index d3aadae..b1f95a9 100644
>> --- a/drivers/clk/qcom/clk-smd-rpm.c
>> +++ b/drivers/clk/qcom/clk-smd-rpm.c
>> @@ -655,10 +655,72 @@ static int clk_smd_rpm_enable_scaling(struct qcom_smd_rpm *rpm)
>>   	.num_clks = ARRAY_SIZE(qcs404_clks),
>>   };
>>   
>> +/* msm8998 */
>> +DEFINE_CLK_SMD_RPM(msm8998, snoc_clk, snoc_a_clk, QCOM_SMD_RPM_BUS_CLK, 1);
>> +DEFINE_CLK_SMD_RPM(msm8998, cnoc_clk, cnoc_a_clk, QCOM_SMD_RPM_BUS_CLK, 2);
>> +DEFINE_CLK_SMD_RPM(msm8998, ce1_clk, ce1_a_clk, QCOM_SMD_RPM_CE_CLK, 0);
>> +DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, div_clk1, div_clk1_a, 0xb);
>> +DEFINE_CLK_SMD_RPM(msm8998, ipa_clk, ipa_a_clk, QCOM_SMD_RPM_IPA_CLK, 0);
>> +DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, bb_clk1, bb_clk1_a, 1);
>> +DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, bb_clk2, bb_clk2_a, 2);
>> +DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(msm8998, bb_clk3_pin, bb_clk3_a_pin, 3);
>> +DEFINE_CLK_SMD_RPM(msm8998, mmssnoc_axi_rpm_clk, mmssnoc_axi_rpm_a_clk,
>> +		   QCOM_SMD_RPM_MMAXI_CLK, 0);
>> +DEFINE_CLK_SMD_RPM(msm8998, aggre1_noc_clk, aggre1_noc_a_clk,
>> +		   QCOM_SMD_RPM_AGGR_CLK, 1);
>> +DEFINE_CLK_SMD_RPM(msm8998, aggre2_noc_clk, aggre2_noc_a_clk,
>> +		   QCOM_SMD_RPM_AGGR_CLK, 2);
>> +DEFINE_CLK_SMD_RPM_QDSS(msm8998, qdss_clk, qdss_a_clk,
>> +			QCOM_SMD_RPM_MISC_CLK, 1);
>> +DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, rf_clk1, rf_clk1_a, 4);
>> +DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(msm8998, rf_clk2_pin, rf_clk2_a_pin, 5);
>> +DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, rf_clk3, rf_clk3_a, 6);
>> +DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(msm8998, rf_clk3_pin, rf_clk3_a_pin, 6);
>> +static struct clk_smd_rpm *msm8998_clks[] = {
>> +	[RPM_SMD_SNOC_CLK] = &msm8998_snoc_clk,
>> +	[RPM_SMD_SNOC_A_CLK] = &msm8998_snoc_a_clk,
>> +	[RPM_SMD_CNOC_CLK] = &msm8998_cnoc_clk,
>> +	[RPM_SMD_CNOC_A_CLK] = &msm8998_cnoc_a_clk,
>> +	[RPM_SMD_CE1_CLK] = &msm8998_ce1_clk,
>> +	[RPM_SMD_CE1_A_CLK] = &msm8998_ce1_a_clk,
>> +	[RPM_SMD_DIV_CLK1] = &msm8998_div_clk1,
>> +	[RPM_SMD_DIV_A_CLK1] = &msm8998_div_clk1_a,
>> +	[RPM_SMD_IPA_CLK] = &msm8998_ipa_clk,
>> +	[RPM_SMD_IPA_A_CLK] = &msm8998_ipa_a_clk,
>> +	[RPM_SMD_BB_CLK1] = &msm8998_bb_clk1,
>> +	[RPM_SMD_BB_CLK1_A] = &msm8998_bb_clk1_a,
>> +	[RPM_SMD_BB_CLK2] = &msm8998_bb_clk2,
>> +	[RPM_SMD_BB_CLK2_A] = &msm8998_bb_clk2_a,
>> +	[RPM_SMD_BB_CLK3_PIN] = &msm8998_bb_clk3_pin,
>> +	[RPM_SMD_BB_CLK3_A_PIN] = &msm8998_bb_clk3_a_pin,
>> +	[RPM_SMD_MMAXI_CLK] = &msm8998_mmssnoc_axi_rpm_clk,
>> +	[RPM_SMD_MMAXI_A_CLK] = &msm8998_mmssnoc_axi_rpm_a_clk,
>> +	[RPM_SMD_AGGR1_NOC_CLK] = &msm8998_aggre1_noc_clk,
>> +	[RPM_SMD_AGGR1_NOC_A_CLK] = &msm8998_aggre1_noc_a_clk,
>> +	[RPM_SMD_AGGR2_NOC_CLK] = &msm8998_aggre2_noc_clk,
>> +	[RPM_SMD_AGGR2_NOC_A_CLK] = &msm8998_aggre2_noc_a_clk,
>> +	[RPM_SMD_QDSS_CLK] = &msm8998_qdss_clk,
>> +	[RPM_SMD_QDSS_A_CLK] = &msm8998_qdss_a_clk,
>> +	[RPM_SMD_RF_CLK1] = &msm8998_rf_clk1,
>> +	[RPM_SMD_RF_CLK1_A] = &msm8998_rf_clk1_a,
>> +	[RPM_SMD_RF_CLK2_PIN] = &msm8998_rf_clk2_pin,
>> +	[RPM_SMD_RF_CLK2_A_PIN] = &msm8998_rf_clk2_a_pin,
>> +	[RPM_SMD_RF_CLK3] = &msm8998_rf_clk3,
>> +	[RPM_SMD_RF_CLK3_A] = &msm8998_rf_clk3_a,
>> +	[RPM_SMD_RF_CLK3_PIN] = &msm8998_rf_clk3_pin,
>> +	[RPM_SMD_RF_CLK3_A_PIN] = &msm8998_rf_clk3_a_pin,
>> +};
>> +
>> +static const struct rpm_smd_clk_desc rpm_clk_msm8998 = {
>> +	.clks = msm8998_clks,
>> +	.num_clks = ARRAY_SIZE(msm8998_clks),
>> +};
>> +
>>   static const struct of_device_id rpm_smd_clk_match_table[] = {
>>   	{ .compatible = "qcom,rpmcc-msm8916", .data = &rpm_clk_msm8916 },
>>   	{ .compatible = "qcom,rpmcc-msm8974", .data = &rpm_clk_msm8974 },
>>   	{ .compatible = "qcom,rpmcc-msm8996", .data = &rpm_clk_msm8996 },
>> +	{ .compatible = "qcom,rpmcc-msm8998", .data = &rpm_clk_msm8998 },
>>   	{ .compatible = "qcom,rpmcc-qcs404",  .data = &rpm_clk_qcs404  },
>>   	{ }
>>   };
>> diff --git a/include/dt-bindings/clock/qcom,rpmcc.h b/include/dt-bindings/clock/qcom,rpmcc.h
>> index 3658b0c..81dbd1f 100644
>> --- a/include/dt-bindings/clock/qcom,rpmcc.h
>> +++ b/include/dt-bindings/clock/qcom,rpmcc.h
>> @@ -127,5 +127,11 @@
>>   #define RPM_SMD_BIMC_GPU_A_CLK			77
>>   #define RPM_SMD_QPIC_CLK			78
>>   #define RPM_SMD_QPIC_CLK_A			79
>> +#define RPM_SMD_BB_CLK3_PIN			80
>> +#define RPM_SMD_BB_CLK3_A_PIN			81
>> +#define RPM_SMD_RF_CLK3				82
>> +#define RPM_SMD_RF_CLK3_A			83
>> +#define RPM_SMD_RF_CLK3_PIN			84
>> +#define RPM_SMD_RF_CLK3_A_PIN			85
>>   
>>   #endif
>>
> 
> What's the difference between RPM_SMD_LN_BB_CLK and RPM_SMD_BB_CLK1?
> 
> $ git grep RPM_SMD_LN_BB_CLK
> arch/arm64/boot/dts/qcom/msm8996.dtsi:                  clocks = <&rpmcc RPM_SMD_LN_BB_CLK>,
> arch/arm64/boot/dts/qcom/msm8996.dtsi:                          <&rpmcc RPM_SMD_LN_BB_CLK>,
> drivers/clk/qcom/clk-smd-rpm.c: [RPM_SMD_LN_BB_CLK] = &msm8996_ln_bb_clk,
> drivers/clk/qcom/clk-smd-rpm.c: [RPM_SMD_LN_BB_CLK] = &qcs404_ln_bb_clk,
> include/dt-bindings/clock/qcom,rpmcc.h:#define RPM_SMD_LN_BB_CLK                        74
> 
> 
> $ git grep 'RPM_SMD_BB_CLK1\>'
> Documentation/devicetree/bindings/sound/qcom,wcd9335.txt:                <&rpmcc RPM_SMD_BB_CLK1>;
> drivers/clk/qcom/clk-smd-rpm.c: [RPM_SMD_BB_CLK1]               = &msm8916_bb_clk1,
> drivers/clk/qcom/clk-smd-rpm.c: [RPM_SMD_BB_CLK1] = &msm8996_bb_clk1,
> drivers/clk/qcom/clk-smd-rpm.c: [RPM_SMD_BB_CLK1] = &msm8998_bb_clk1,
> include/dt-bindings/clock/qcom,rpmcc.h:#define RPM_SMD_BB_CLK1                          10
> 
> 
> Because the downstream kernel defines:
> 
> DEFINE_CLK_RPM_SMD_XO_BUFFER(ln_bb_clk1, ln_bb_clk1_ao, LN_BB_CLK1_ID);
> DEFINE_CLK_RPM_SMD_XO_BUFFER_PINCTRL(ln_bb_clk1_pin, ln_bb_clk1_pin_ao,
> 				     LN_BB_CLK1_PIN_ID);
> DEFINE_CLK_RPM_SMD_XO_BUFFER(ln_bb_clk2, ln_bb_clk2_ao, LN_BB_CLK2_ID);
> DEFINE_CLK_RPM_SMD_XO_BUFFER_PINCTRL(ln_bb_clk2_pin, ln_bb_clk2_pin_ao,
> 				     LN_BB_CLK2_PIN_ID);
> DEFINE_CLK_RPM_SMD_XO_BUFFER(ln_bb_clk3, ln_bb_clk3_ao, LN_BB_CLK3_ID);
> DEFINE_CLK_RPM_SMD_XO_BUFFER_PINCTRL(ln_bb_clk3_pin, ln_bb_clk3_pin_ao,
> 				     LN_BB_CLK3_PIN_ID);
> 
> 	CLK_LIST(ln_bb_clk1),
> 	CLK_LIST(ln_bb_clk1_ao),
> 	CLK_LIST(ln_bb_clk1_pin),
> 	CLK_LIST(ln_bb_clk1_pin_ao),
> 	CLK_LIST(ln_bb_clk2),
> 	CLK_LIST(ln_bb_clk2_ao),
> 	CLK_LIST(ln_bb_clk2_pin),
> 	CLK_LIST(ln_bb_clk2_pin_ao),
> 	CLK_LIST(ln_bb_clk3),
> 	CLK_LIST(ln_bb_clk3_ao),
> 	CLK_LIST(ln_bb_clk3_pin),
> 	CLK_LIST(ln_bb_clk3_pin_ao),
> 
> 
> While your patch defines
> 
> DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, bb_clk1, bb_clk1_a, 1);
> DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, bb_clk2, bb_clk2_a, 2);
> DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(msm8998, bb_clk3_pin, bb_clk3_a_pin, 3);
> 
> 	[RPM_SMD_BB_CLK1] = &msm8998_bb_clk1,
> 	[RPM_SMD_BB_CLK1_A] = &msm8998_bb_clk1_a,
> 	[RPM_SMD_BB_CLK2] = &msm8998_bb_clk2,
> 	[RPM_SMD_BB_CLK2_A] = &msm8998_bb_clk2_a,
> 	[RPM_SMD_BB_CLK3_PIN] = &msm8998_bb_clk3_pin,
> 	[RPM_SMD_BB_CLK3_A_PIN] = &msm8998_bb_clk3_a_pin,
> 

Good question.  I don't know.

The downstream ln_bb IDs correspond to the bb IDs used by upstream 8996, 
where as the upstream 8996 ln_bb IDs are different.  The IDs seem to be 
the important thing, where as the "name" seems to be fairly irrelevant 
to the actual handling of the clock.

I haven't yet found documentation other than the downstream code about 
these clocks, so I chose to be consistent with 8996.

Also, as a side note, I limited the list of clocks I enumerated to those 
which the downstream driver defined, and were used when grepping the 
downstream DT.

-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] clk: qcom: smd: Add support for MSM8998 rpm clocks
  2018-12-07 14:23 ` Marc Gonzalez
@ 2018-12-07 15:13   ` Jeffrey Hugo
  2018-12-10 23:10     ` Jeffrey Hugo
  0 siblings, 1 reply; 7+ messages in thread
From: Jeffrey Hugo @ 2018-12-07 15:13 UTC (permalink / raw)
  To: Marc Gonzalez, Michael Turquette, Stephen Boyd
  Cc: Bjorn Andersson, linux-clk, MSM, LKML

On 12/7/2018 7:23 AM, Marc Gonzalez wrote:
> On 06/12/2018 23:11, Jeffrey Hugo wrote:
> 
>> Add rpm smd clocks, PMIC and bus clocks which are required on MSM8998
>> for clients to vote on.
>>
>> Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org>
>> ---
>> v2
>> -fix compatible ordering nits per Stephen
>>
>>   .../devicetree/bindings/clock/qcom,rpmcc.txt       |  1 +
>>   drivers/clk/qcom/clk-smd-rpm.c                     | 62 ++++++++++++++++++++++
>>   include/dt-bindings/clock/qcom,rpmcc.h             |  6 +++
>>   3 files changed, 69 insertions(+)
> 
> Hmmm, my board seems to dislike this patch... it locks up for a while,
> then reboots.
> 
> [    6.957289] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 1 failed, err = -11
> [    6.961233] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry: failed with error -11, retries 2
> [    6.970782] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry: query attribute, opcode 6, idn 1, failed with error -11 after 3 retires
> [    6.979576] ufshcd-qcom 1da4000.ufshc: ufshcd_complete_dev_init setting fDeviceInit flag failed with error -11
> [   23.432798] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
> [   23.432884] rcu:     7-...0: (1 GPs behind) idle=6a2/1/0x4000000000000000 softirq=92/93 fqs=2626
> [   23.437927] rcu:     (detected by 0, t=5252 jiffies, g=-819, q=112)
> [   23.446425] Task dump for CPU 7:
> [   23.452567] swapper/0       R  running task        0     1      0 0x0000002a
> [   23.455806] Call trace:
> [   23.462872]  __switch_to+0x94/0xe0
> [   23.465009]  _regmap_write+0x58/0xb0
> [   23.468452]  _regmap_update_bits+0xf0/0x110
> [   23.472186]  regmap_update_bits_base+0x60/0x90
> [   23.476110]  clk_disable_regmap+0x34/0x40
> [   23.480614]  clk_branch_toggle+0x108/0x1b0
> [   23.484681]  clk_branch2_disable+0x18/0x20
> [   23.488692]  clk_disable_unused_subtree+0xc4/0xe0
> [   23.492761]  clk_disable_unused+0x3c/0x130
> [   23.497535]  do_one_initcall+0x5c/0x180
> [   23.501557]  kernel_init_freeable+0x198/0x244
> [   23.505276]  kernel_init+0x10/0x110
> [   23.509768]  ret_from_fork+0x10/0x20
> 
> /*** REBOOT ***/
> Format: Log Type - Time(microsec) - Message - Optional Info
> Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic
> 
> I need to check the ufs_reset pin and phy init seq before I can investigate
> this issue, but I wanted to send a report ASAP.
> 

Hmm.  I run with clk_ignore_unused.  I'm guessing that something was 
defined that should be used eventually, but isn't used now, and so its 
getting turned off when it probably shouldn't be.

I'll try to repro on my end.

-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] clk: qcom: smd: Add support for MSM8998 rpm clocks
  2018-12-07 15:10   ` Jeffrey Hugo
@ 2018-12-07 16:08     ` Jeffrey Hugo
  0 siblings, 0 replies; 7+ messages in thread
From: Jeffrey Hugo @ 2018-12-07 16:08 UTC (permalink / raw)
  To: Marc Gonzalez, Michael Turquette, Stephen Boyd
  Cc: Rob Herring, Mark Rutland, Bjorn Andersson, linux-clk, MSM, LKML

On 12/7/2018 8:10 AM, Jeffrey Hugo wrote:
> On 12/7/2018 3:30 AM, Marc Gonzalez wrote:
>> On 06/12/2018 23:11, Jeffrey Hugo wrote:
>>
>>> Add rpm smd clocks, PMIC and bus clocks which are required on MSM8998
>>> for clients to vote on.
>>>
>>> Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org>
>>> ---
>>> v2
>>> -fix compatible ordering nits per Stephen
>>>
>>>   .../devicetree/bindings/clock/qcom,rpmcc.txt       |  1 +
>>>   drivers/clk/qcom/clk-smd-rpm.c                     | 62 
>>> ++++++++++++++++++++++
>>>   include/dt-bindings/clock/qcom,rpmcc.h             |  6 +++
>>>   3 files changed, 69 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt 
>>> b/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
>>> index 87b4949..944719b 100644
>>> --- a/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
>>> +++ b/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
>>> @@ -16,6 +16,7 @@ Required properties :
>>>               "qcom,rpmcc-msm8974", "qcom,rpmcc"
>>>               "qcom,rpmcc-apq8064", "qcom,rpmcc"
>>>               "qcom,rpmcc-msm8996", "qcom,rpmcc"
>>> +            "qcom,rpmcc-msm8998", "qcom,rpmcc"
>>>               "qcom,rpmcc-qcs404", "qcom,rpmcc"
>>>   - #clock-cells : shall contain 1
>>> diff --git a/drivers/clk/qcom/clk-smd-rpm.c 
>>> b/drivers/clk/qcom/clk-smd-rpm.c
>>> index d3aadae..b1f95a9 100644
>>> --- a/drivers/clk/qcom/clk-smd-rpm.c
>>> +++ b/drivers/clk/qcom/clk-smd-rpm.c
>>> @@ -655,10 +655,72 @@ static int clk_smd_rpm_enable_scaling(struct 
>>> qcom_smd_rpm *rpm)
>>>       .num_clks = ARRAY_SIZE(qcs404_clks),
>>>   };
>>> +/* msm8998 */
>>> +DEFINE_CLK_SMD_RPM(msm8998, snoc_clk, snoc_a_clk, 
>>> QCOM_SMD_RPM_BUS_CLK, 1);
>>> +DEFINE_CLK_SMD_RPM(msm8998, cnoc_clk, cnoc_a_clk, 
>>> QCOM_SMD_RPM_BUS_CLK, 2);
>>> +DEFINE_CLK_SMD_RPM(msm8998, ce1_clk, ce1_a_clk, QCOM_SMD_RPM_CE_CLK, 
>>> 0);
>>> +DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, div_clk1, div_clk1_a, 0xb);
>>> +DEFINE_CLK_SMD_RPM(msm8998, ipa_clk, ipa_a_clk, 
>>> QCOM_SMD_RPM_IPA_CLK, 0);
>>> +DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, bb_clk1, bb_clk1_a, 1);
>>> +DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, bb_clk2, bb_clk2_a, 2);
>>> +DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(msm8998, bb_clk3_pin, 
>>> bb_clk3_a_pin, 3);
>>> +DEFINE_CLK_SMD_RPM(msm8998, mmssnoc_axi_rpm_clk, mmssnoc_axi_rpm_a_clk,
>>> +           QCOM_SMD_RPM_MMAXI_CLK, 0);
>>> +DEFINE_CLK_SMD_RPM(msm8998, aggre1_noc_clk, aggre1_noc_a_clk,
>>> +           QCOM_SMD_RPM_AGGR_CLK, 1);
>>> +DEFINE_CLK_SMD_RPM(msm8998, aggre2_noc_clk, aggre2_noc_a_clk,
>>> +           QCOM_SMD_RPM_AGGR_CLK, 2);
>>> +DEFINE_CLK_SMD_RPM_QDSS(msm8998, qdss_clk, qdss_a_clk,
>>> +            QCOM_SMD_RPM_MISC_CLK, 1);
>>> +DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, rf_clk1, rf_clk1_a, 4);
>>> +DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(msm8998, rf_clk2_pin, 
>>> rf_clk2_a_pin, 5);
>>> +DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, rf_clk3, rf_clk3_a, 6);
>>> +DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(msm8998, rf_clk3_pin, 
>>> rf_clk3_a_pin, 6);
>>> +static struct clk_smd_rpm *msm8998_clks[] = {
>>> +    [RPM_SMD_SNOC_CLK] = &msm8998_snoc_clk,
>>> +    [RPM_SMD_SNOC_A_CLK] = &msm8998_snoc_a_clk,
>>> +    [RPM_SMD_CNOC_CLK] = &msm8998_cnoc_clk,
>>> +    [RPM_SMD_CNOC_A_CLK] = &msm8998_cnoc_a_clk,
>>> +    [RPM_SMD_CE1_CLK] = &msm8998_ce1_clk,
>>> +    [RPM_SMD_CE1_A_CLK] = &msm8998_ce1_a_clk,
>>> +    [RPM_SMD_DIV_CLK1] = &msm8998_div_clk1,
>>> +    [RPM_SMD_DIV_A_CLK1] = &msm8998_div_clk1_a,
>>> +    [RPM_SMD_IPA_CLK] = &msm8998_ipa_clk,
>>> +    [RPM_SMD_IPA_A_CLK] = &msm8998_ipa_a_clk,
>>> +    [RPM_SMD_BB_CLK1] = &msm8998_bb_clk1,
>>> +    [RPM_SMD_BB_CLK1_A] = &msm8998_bb_clk1_a,
>>> +    [RPM_SMD_BB_CLK2] = &msm8998_bb_clk2,
>>> +    [RPM_SMD_BB_CLK2_A] = &msm8998_bb_clk2_a,
>>> +    [RPM_SMD_BB_CLK3_PIN] = &msm8998_bb_clk3_pin,
>>> +    [RPM_SMD_BB_CLK3_A_PIN] = &msm8998_bb_clk3_a_pin,
>>> +    [RPM_SMD_MMAXI_CLK] = &msm8998_mmssnoc_axi_rpm_clk,
>>> +    [RPM_SMD_MMAXI_A_CLK] = &msm8998_mmssnoc_axi_rpm_a_clk,
>>> +    [RPM_SMD_AGGR1_NOC_CLK] = &msm8998_aggre1_noc_clk,
>>> +    [RPM_SMD_AGGR1_NOC_A_CLK] = &msm8998_aggre1_noc_a_clk,
>>> +    [RPM_SMD_AGGR2_NOC_CLK] = &msm8998_aggre2_noc_clk,
>>> +    [RPM_SMD_AGGR2_NOC_A_CLK] = &msm8998_aggre2_noc_a_clk,
>>> +    [RPM_SMD_QDSS_CLK] = &msm8998_qdss_clk,
>>> +    [RPM_SMD_QDSS_A_CLK] = &msm8998_qdss_a_clk,
>>> +    [RPM_SMD_RF_CLK1] = &msm8998_rf_clk1,
>>> +    [RPM_SMD_RF_CLK1_A] = &msm8998_rf_clk1_a,
>>> +    [RPM_SMD_RF_CLK2_PIN] = &msm8998_rf_clk2_pin,
>>> +    [RPM_SMD_RF_CLK2_A_PIN] = &msm8998_rf_clk2_a_pin,
>>> +    [RPM_SMD_RF_CLK3] = &msm8998_rf_clk3,
>>> +    [RPM_SMD_RF_CLK3_A] = &msm8998_rf_clk3_a,
>>> +    [RPM_SMD_RF_CLK3_PIN] = &msm8998_rf_clk3_pin,
>>> +    [RPM_SMD_RF_CLK3_A_PIN] = &msm8998_rf_clk3_a_pin,
>>> +};
>>> +
>>> +static const struct rpm_smd_clk_desc rpm_clk_msm8998 = {
>>> +    .clks = msm8998_clks,
>>> +    .num_clks = ARRAY_SIZE(msm8998_clks),
>>> +};
>>> +
>>>   static const struct of_device_id rpm_smd_clk_match_table[] = {
>>>       { .compatible = "qcom,rpmcc-msm8916", .data = &rpm_clk_msm8916 },
>>>       { .compatible = "qcom,rpmcc-msm8974", .data = &rpm_clk_msm8974 },
>>>       { .compatible = "qcom,rpmcc-msm8996", .data = &rpm_clk_msm8996 },
>>> +    { .compatible = "qcom,rpmcc-msm8998", .data = &rpm_clk_msm8998 },
>>>       { .compatible = "qcom,rpmcc-qcs404",  .data = &rpm_clk_qcs404  },
>>>       { }
>>>   };
>>> diff --git a/include/dt-bindings/clock/qcom,rpmcc.h 
>>> b/include/dt-bindings/clock/qcom,rpmcc.h
>>> index 3658b0c..81dbd1f 100644
>>> --- a/include/dt-bindings/clock/qcom,rpmcc.h
>>> +++ b/include/dt-bindings/clock/qcom,rpmcc.h
>>> @@ -127,5 +127,11 @@
>>>   #define RPM_SMD_BIMC_GPU_A_CLK            77
>>>   #define RPM_SMD_QPIC_CLK            78
>>>   #define RPM_SMD_QPIC_CLK_A            79
>>> +#define RPM_SMD_BB_CLK3_PIN            80
>>> +#define RPM_SMD_BB_CLK3_A_PIN            81
>>> +#define RPM_SMD_RF_CLK3                82
>>> +#define RPM_SMD_RF_CLK3_A            83
>>> +#define RPM_SMD_RF_CLK3_PIN            84
>>> +#define RPM_SMD_RF_CLK3_A_PIN            85
>>>   #endif
>>>
>>
>> What's the difference between RPM_SMD_LN_BB_CLK and RPM_SMD_BB_CLK1?
>>
>> $ git grep RPM_SMD_LN_BB_CLK
>> arch/arm64/boot/dts/qcom/msm8996.dtsi:                  clocks = 
>> <&rpmcc RPM_SMD_LN_BB_CLK>,
>> arch/arm64/boot/dts/qcom/msm8996.dtsi:                          
>> <&rpmcc RPM_SMD_LN_BB_CLK>,
>> drivers/clk/qcom/clk-smd-rpm.c: [RPM_SMD_LN_BB_CLK] = &msm8996_ln_bb_clk,
>> drivers/clk/qcom/clk-smd-rpm.c: [RPM_SMD_LN_BB_CLK] = &qcs404_ln_bb_clk,
>> include/dt-bindings/clock/qcom,rpmcc.h:#define 
>> RPM_SMD_LN_BB_CLK                        74
>>
>>
>> $ git grep 'RPM_SMD_BB_CLK1\>'
>> Documentation/devicetree/bindings/sound/qcom,wcd9335.txt:                
>> <&rpmcc RPM_SMD_BB_CLK1>;
>> drivers/clk/qcom/clk-smd-rpm.c: [RPM_SMD_BB_CLK1]               = 
>> &msm8916_bb_clk1,
>> drivers/clk/qcom/clk-smd-rpm.c: [RPM_SMD_BB_CLK1] = &msm8996_bb_clk1,
>> drivers/clk/qcom/clk-smd-rpm.c: [RPM_SMD_BB_CLK1] = &msm8998_bb_clk1,
>> include/dt-bindings/clock/qcom,rpmcc.h:#define 
>> RPM_SMD_BB_CLK1                          10
>>
>>
>> Because the downstream kernel defines:
>>
>> DEFINE_CLK_RPM_SMD_XO_BUFFER(ln_bb_clk1, ln_bb_clk1_ao, LN_BB_CLK1_ID);
>> DEFINE_CLK_RPM_SMD_XO_BUFFER_PINCTRL(ln_bb_clk1_pin, ln_bb_clk1_pin_ao,
>>                      LN_BB_CLK1_PIN_ID);
>> DEFINE_CLK_RPM_SMD_XO_BUFFER(ln_bb_clk2, ln_bb_clk2_ao, LN_BB_CLK2_ID);
>> DEFINE_CLK_RPM_SMD_XO_BUFFER_PINCTRL(ln_bb_clk2_pin, ln_bb_clk2_pin_ao,
>>                      LN_BB_CLK2_PIN_ID);
>> DEFINE_CLK_RPM_SMD_XO_BUFFER(ln_bb_clk3, ln_bb_clk3_ao, LN_BB_CLK3_ID);
>> DEFINE_CLK_RPM_SMD_XO_BUFFER_PINCTRL(ln_bb_clk3_pin, ln_bb_clk3_pin_ao,
>>                      LN_BB_CLK3_PIN_ID);
>>
>>     CLK_LIST(ln_bb_clk1),
>>     CLK_LIST(ln_bb_clk1_ao),
>>     CLK_LIST(ln_bb_clk1_pin),
>>     CLK_LIST(ln_bb_clk1_pin_ao),
>>     CLK_LIST(ln_bb_clk2),
>>     CLK_LIST(ln_bb_clk2_ao),
>>     CLK_LIST(ln_bb_clk2_pin),
>>     CLK_LIST(ln_bb_clk2_pin_ao),
>>     CLK_LIST(ln_bb_clk3),
>>     CLK_LIST(ln_bb_clk3_ao),
>>     CLK_LIST(ln_bb_clk3_pin),
>>     CLK_LIST(ln_bb_clk3_pin_ao),
>>
>>
>> While your patch defines
>>
>> DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, bb_clk1, bb_clk1_a, 1);
>> DEFINE_CLK_SMD_RPM_XO_BUFFER(msm8998, bb_clk2, bb_clk2_a, 2);
>> DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(msm8998, bb_clk3_pin, 
>> bb_clk3_a_pin, 3);
>>
>>     [RPM_SMD_BB_CLK1] = &msm8998_bb_clk1,
>>     [RPM_SMD_BB_CLK1_A] = &msm8998_bb_clk1_a,
>>     [RPM_SMD_BB_CLK2] = &msm8998_bb_clk2,
>>     [RPM_SMD_BB_CLK2_A] = &msm8998_bb_clk2_a,
>>     [RPM_SMD_BB_CLK3_PIN] = &msm8998_bb_clk3_pin,
>>     [RPM_SMD_BB_CLK3_A_PIN] = &msm8998_bb_clk3_a_pin,
>>
> 
> Good question.  I don't know.
> 
> The downstream ln_bb IDs correspond to the bb IDs used by upstream 8996, 
> where as the upstream 8996 ln_bb IDs are different.  The IDs seem to be 
> the important thing, where as the "name" seems to be fairly irrelevant 
> to the actual handling of the clock.
> 
> I haven't yet found documentation other than the downstream code about 
> these clocks, so I chose to be consistent with 8996.

I found some schematics, and they have "ln_bb", so I'll respin with that 
nomenclature after I have a look at your crash.

> 
> Also, as a side note, I limited the list of clocks I enumerated to those 
> which the downstream driver defined, and were used when grepping the 
> downstream DT.
> 


-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] clk: qcom: smd: Add support for MSM8998 rpm clocks
  2018-12-07 15:13   ` Jeffrey Hugo
@ 2018-12-10 23:10     ` Jeffrey Hugo
  0 siblings, 0 replies; 7+ messages in thread
From: Jeffrey Hugo @ 2018-12-10 23:10 UTC (permalink / raw)
  To: Marc Gonzalez, Michael Turquette, Stephen Boyd
  Cc: Bjorn Andersson, linux-clk, MSM, LKML

On 12/7/2018 8:13 AM, Jeffrey Hugo wrote:
> On 12/7/2018 7:23 AM, Marc Gonzalez wrote:
>> On 06/12/2018 23:11, Jeffrey Hugo wrote:
>>
>>> Add rpm smd clocks, PMIC and bus clocks which are required on MSM8998
>>> for clients to vote on.
>>>
>>> Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org>
>>> ---
>>> v2
>>> -fix compatible ordering nits per Stephen
>>>
>>>   .../devicetree/bindings/clock/qcom,rpmcc.txt       |  1 +
>>>   drivers/clk/qcom/clk-smd-rpm.c                     | 62 
>>> ++++++++++++++++++++++
>>>   include/dt-bindings/clock/qcom,rpmcc.h             |  6 +++
>>>   3 files changed, 69 insertions(+)
>>
>> Hmmm, my board seems to dislike this patch... it locks up for a while,
>> then reboots.
>>
>> [    6.957289] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending 
>> flag query for idn 1 failed, err = -11
>> [    6.961233] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry: 
>> failed with error -11, retries 2
>> [    6.970782] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry: 
>> query attribute, opcode 6, idn 1, failed with error -11 after 3 retires
>> [    6.979576] ufshcd-qcom 1da4000.ufshc: ufshcd_complete_dev_init 
>> setting fDeviceInit flag failed with error -11
>> [   23.432798] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
>> [   23.432884] rcu:     7-...0: (1 GPs behind) 
>> idle=6a2/1/0x4000000000000000 softirq=92/93 fqs=2626
>> [   23.437927] rcu:     (detected by 0, t=5252 jiffies, g=-819, q=112)
>> [   23.446425] Task dump for CPU 7:
>> [   23.452567] swapper/0       R  running task        0     1      0 
>> 0x0000002a
>> [   23.455806] Call trace:
>> [   23.462872]  __switch_to+0x94/0xe0
>> [   23.465009]  _regmap_write+0x58/0xb0
>> [   23.468452]  _regmap_update_bits+0xf0/0x110
>> [   23.472186]  regmap_update_bits_base+0x60/0x90
>> [   23.476110]  clk_disable_regmap+0x34/0x40
>> [   23.480614]  clk_branch_toggle+0x108/0x1b0
>> [   23.484681]  clk_branch2_disable+0x18/0x20
>> [   23.488692]  clk_disable_unused_subtree+0xc4/0xe0
>> [   23.492761]  clk_disable_unused+0x3c/0x130
>> [   23.497535]  do_one_initcall+0x5c/0x180
>> [   23.501557]  kernel_init_freeable+0x198/0x244
>> [   23.505276]  kernel_init+0x10/0x110
>> [   23.509768]  ret_from_fork+0x10/0x20
>>
>> /*** REBOOT ***/
>> Format: Log Type - Time(microsec) - Message - Optional Info
>> Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic
>>
>> I need to check the ufs_reset pin and phy init seq before I can 
>> investigate
>> this issue, but I wanted to send a report ASAP.
>>
> 
> Hmm.  I run with clk_ignore_unused.  I'm guessing that something was 
> defined that should be used eventually, but isn't used now, and so its 
> getting turned off when it probably shouldn't be.
> 
> I'll try to repro on my end.
> 

I think I figured this out.

The issue appears to be with the mmssnoc_axi_rpm_clk that this patch 
defines.  Removing that, and I do not observe a crash.  This clock will 
eventually be needed for the multimedia subsystem (camera and the like).

Marc, it would be good if you could comment out this clock to verify my 
observations translate to your setup.

The problem occurs when the rpm driver, during probe, sends the active 
settings to the rpm.  This causes the rpm to try to access the mmss over 
the relevant config noc, and results in an unclocked bus error, which is 
a fatal error.

clk_ignore_unused avoids this issue because it leaves on the config noc 
clock defined in gcc - gcc_mmss_noc_cfg_ahb_clk

I can see 3 workarounds to this issue -

1. comment out mmssnoc_axi_rpm_clk with an explanation to why, and leave 
this problem to whomever needs the clock in the future

2. add the CLK_IGNORE_UNUSED flag to gcc_mmss_noc_cfg_ahb_clk so prevent 
it from being disabled (aka a selective version of clk_ignore_unused)

3. add a prepare_enable call to gcc_mmss_noc_cfg_ahb_clk in the gcc 
probe, so that there is always one active consumer of the clock (this is 
what downstream does)

Stephen, do you have a preference, or an alternative suggestion?

-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-12-10 23:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-06 22:11 [PATCH v2] clk: qcom: smd: Add support for MSM8998 rpm clocks Jeffrey Hugo
2018-12-07 10:30 ` Marc Gonzalez
2018-12-07 15:10   ` Jeffrey Hugo
2018-12-07 16:08     ` Jeffrey Hugo
2018-12-07 14:23 ` Marc Gonzalez
2018-12-07 15:13   ` Jeffrey Hugo
2018-12-10 23:10     ` Jeffrey Hugo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).