[PATCH v6 0/5] clk: qcom: Add support for SM8550

[PATCH v6 0/5] clk: qcom: Add support for SM8550

[Abel Vesa]

This patchset adds more clocks support for the Qualcomm SM8550 SoC,
It adds the TCSR clock controller driver and the rpmh clocks.

Changes since v5:
 * moved the 3rd patch as 1st, since otherwise check for the example
   from the TCSR binding schema fails
 * changed the description of the clocks property in the TCSR schema
   file to "TCXO pad clock"
 * added syscon to compatible property for TCSR schema and in its
   example

To: Andy Gross <agross@kernel.org>
To: Bjorn Andersson <andersson@kernel.org>
To: Konrad Dybcio <konrad.dybcio@linaro.org>
To: Michael Turquette <mturquette@baylibre.com>
To: Stephen Boyd <sboyd@kernel.org>
To: Rob Herring <robh+dt@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-clk@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Abel Vesa (5):
  dt-bindings: clock: qcom,rpmh: Add CXO PAD clock IDs
  dt-bindings: clock: Add SM8550 TCSR CC clocks
  dt-bindings: clock: Add RPMHCC for SM8550
  clk: qcom: rpmh: Add support for SM8550 rpmh clocks
  clk: qcom: Add TCSR clock driver for SM8550

 .../bindings/clock/qcom,rpmhcc.yaml           |   1 +
 .../bindings/clock/qcom,sm8550-tcsr.yaml      |  55 +++++
 drivers/clk/qcom/Kconfig                      |   7 +
 drivers/clk/qcom/Makefile                     |   1 +
 drivers/clk/qcom/clk-rpmh.c                   | 110 ++++++++--
 drivers/clk/qcom/tcsrcc-sm8550.c              | 192 ++++++++++++++++++
 include/dt-bindings/clock/qcom,rpmh.h         |   2 +
 include/dt-bindings/clock/qcom,sm8550-tcsr.h  |  18 ++
 8 files changed, 366 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,sm8550-tcsr.yaml
 create mode 100644 drivers/clk/qcom/tcsrcc-sm8550.c
 create mode 100644 include/dt-bindings/clock/qcom,sm8550-tcsr.h

-- 
2.34.1

[PATCH v6 1/5] dt-bindings: clock: qcom,rpmh: Add CXO PAD clock IDs

[Abel Vesa]

The SM8550 has a new fixed divider as child clock of CXO
called CXO_PAD, so add IDs for it.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 include/dt-bindings/clock/qcom,rpmh.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/dt-bindings/clock/qcom,rpmh.h b/include/dt-bindings/clock/qcom,rpmh.h
index 0a7d1be0d124..f3e0288420ce 100644
--- a/include/dt-bindings/clock/qcom,rpmh.h
+++ b/include/dt-bindings/clock/qcom,rpmh.h
@@ -33,5 +33,7 @@
 #define RPMH_HWKM_CLK				24
 #define RPMH_QLINK_CLK				25
 #define RPMH_QLINK_CLK_A			26
+#define RPMH_CXO_PAD_CLK			27
+#define RPMH_CXO_PAD_CLK_A			28
 
 #endif
-- 
2.34.1

[PATCH v6 2/5] dt-bindings: clock: Add SM8550 TCSR CC clocks

[Abel Vesa]

Add bindings documentation for clock TCSR driver on SM8550.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/clock/qcom,sm8550-tcsr.yaml      | 55 +++++++++++++++++++
 include/dt-bindings/clock/qcom,sm8550-tcsr.h  | 18 ++++++
 2 files changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,sm8550-tcsr.yaml
 create mode 100644 include/dt-bindings/clock/qcom,sm8550-tcsr.h

diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8550-tcsr.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8550-tcsr.yaml
new file mode 100644
index 000000000000..2b72ab82041a
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,sm8550-tcsr.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/qcom,sm8550-tcsr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm TCSR Clock Controller on SM8550
+
+maintainers:
+  - Bjorn Andersson <andersson@kernel.org>
+
+description: |
+  Qualcomm TCSR clock control module provides the clocks, resets and
+  power domains on SM8550
+
+  See also:: include/dt-bindings/clock/qcom,sm8550-tcsr.h
+
+properties:
+  compatible:
+    items:
+      - const: qcom,sm8550-tcsr
+      - const: syscon
+
+  clocks:
+    items:
+      - description: TCXO pad clock
+
+  reg:
+    maxItems: 1
+
+  '#clock-cells':
+    const: 1
+
+  '#reset-cells':
+    const: 1
+
+required:
+  - compatible
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,rpmh.h>
+
+    clock-controller@1fc0000 {
+      compatible = "qcom,sm8550-tcsr", "syscon";
+      reg = <0x1fc0000 0x30000>;
+      clocks = <&rpmhcc RPMH_CXO_PAD_CLK>;
+      #clock-cells = <1>;
+      #reset-cells = <1>;
+    };
+
+...
diff --git a/include/dt-bindings/clock/qcom,sm8550-tcsr.h b/include/dt-bindings/clock/qcom,sm8550-tcsr.h
new file mode 100644
index 000000000000..091cb76f953a
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,sm8550-tcsr.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2022, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2022, Linaro Limited
+ */
+
+#ifndef _DT_BINDINGS_CLK_QCOM_TCSR_CC_SM8550_H
+#define _DT_BINDINGS_CLK_QCOM_TCSR_CC_SM8550_H
+
+/* TCSR CC clocks */
+#define TCSR_PCIE_0_CLKREF_EN					0
+#define TCSR_PCIE_1_CLKREF_EN					1
+#define TCSR_UFS_CLKREF_EN					2
+#define TCSR_UFS_PAD_CLKREF_EN					3
+#define TCSR_USB2_CLKREF_EN					4
+#define TCSR_USB3_CLKREF_EN					5
+
+#endif
-- 
2.34.1

[PATCH v6 3/5] dt-bindings: clock: Add RPMHCC for SM8550

[Abel Vesa]

Add bindings and update documentation for clock rpmh driver on SM8550.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 Documentation/devicetree/bindings/clock/qcom,rpmhcc.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,rpmhcc.yaml b/Documentation/devicetree/bindings/clock/qcom,rpmhcc.yaml
index fccb91e78e49..b405da7700be 100644
--- a/Documentation/devicetree/bindings/clock/qcom,rpmhcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,rpmhcc.yaml
@@ -31,6 +31,7 @@ properties:
       - qcom,sm8250-rpmh-clk
       - qcom,sm8350-rpmh-clk
       - qcom,sm8450-rpmh-clk
+      - qcom,sm8550-rpmh-clk
 
   clocks:
     maxItems: 1
-- 
2.34.1

[PATCH v6 4/5] clk: qcom: rpmh: Add support for SM8550 rpmh clocks

[Abel Vesa]

Adds the RPMH clocks present in SM8550 SoC.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-rpmh.c | 110 +++++++++++++++++++++++++++++-------
 1 file changed, 90 insertions(+), 20 deletions(-)

diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c
index 2c2ef4b6d130..ce81c76ed0fd 100644
--- a/drivers/clk/qcom/clk-rpmh.c
+++ b/drivers/clk/qcom/clk-rpmh.c
@@ -130,6 +130,34 @@ static DEFINE_MUTEX(rpmh_clk_lock);
 		},							\
 	}
 
+#define DEFINE_CLK_FIXED_FACTOR(_name, _parent_name, _div)		\
+	static struct clk_fixed_factor clk_fixed_factor##_##_name = {	\
+		.mult = 1,						\
+		.div = _div,						\
+		.hw.init = &(struct clk_init_data){			\
+			.ops = &clk_fixed_factor_ops,			\
+			.name = #_name,					\
+			.parent_data =  &(const struct clk_parent_data){ \
+				.fw_name = #_parent_name,		\
+				.name = #_parent_name,			\
+			},						\
+			.num_parents = 1,				\
+		},							\
+	};								\
+	static struct clk_fixed_factor clk_fixed_factor##_##_name##_ao = { \
+		.mult = 1,						\
+		.div = _div,						\
+		.hw.init = &(struct clk_init_data){			\
+			.ops = &clk_fixed_factor_ops,			\
+			.name = #_name "_ao",				\
+			.parent_data =  &(const struct clk_parent_data){ \
+				.fw_name = #_parent_name "_ao",		\
+				.name = #_parent_name "_ao",		\
+			},						\
+			.num_parents = 1,				\
+		},							\
+	}
+
 static inline struct clk_rpmh *to_clk_rpmh(struct clk_hw *_hw)
 {
 	return container_of(_hw, struct clk_rpmh, hw);
@@ -345,6 +373,8 @@ DEFINE_CLK_RPMH_ARC(bi_tcxo, "xo.lvl", 0x3, 2);
 DEFINE_CLK_RPMH_ARC(bi_tcxo, "xo.lvl", 0x3, 4);
 DEFINE_CLK_RPMH_ARC(qlink, "qphy.lvl", 0x1, 4);
 
+DEFINE_CLK_FIXED_FACTOR(bi_tcxo_div2, bi_tcxo, 2);
+
 DEFINE_CLK_RPMH_VRM(ln_bb_clk1, _a2, "lnbclka1", 2);
 DEFINE_CLK_RPMH_VRM(ln_bb_clk2, _a2, "lnbclka2", 2);
 DEFINE_CLK_RPMH_VRM(ln_bb_clk3, _a2, "lnbclka3", 2);
@@ -366,6 +396,16 @@ DEFINE_CLK_RPMH_VRM(rf_clk2, _d, "rfclkd2", 1);
 DEFINE_CLK_RPMH_VRM(rf_clk3, _d, "rfclkd3", 1);
 DEFINE_CLK_RPMH_VRM(rf_clk4, _d, "rfclkd4", 1);
 
+DEFINE_CLK_RPMH_VRM(clk1, _a1, "clka1", 1);
+DEFINE_CLK_RPMH_VRM(clk2, _a1, "clka2", 1);
+DEFINE_CLK_RPMH_VRM(clk3, _a1, "clka3", 1);
+DEFINE_CLK_RPMH_VRM(clk4, _a1, "clka4", 1);
+DEFINE_CLK_RPMH_VRM(clk5, _a1, "clka5", 1);
+
+DEFINE_CLK_RPMH_VRM(clk6, _a2, "clka6", 2);
+DEFINE_CLK_RPMH_VRM(clk7, _a2, "clka7", 2);
+DEFINE_CLK_RPMH_VRM(clk8, _a2, "clka8", 2);
+
 DEFINE_CLK_RPMH_VRM(div_clk1, _div2, "divclka1", 2);
 
 DEFINE_CLK_RPMH_BCM(ce, "CE0");
@@ -576,6 +616,33 @@ static const struct clk_rpmh_desc clk_rpmh_sm8450 = {
 	.num_clks = ARRAY_SIZE(sm8450_rpmh_clocks),
 };
 
+static struct clk_hw *sm8550_rpmh_clocks[] = {
+	[RPMH_CXO_PAD_CLK]      = &clk_rpmh_bi_tcxo_div2.hw,
+	[RPMH_CXO_PAD_CLK_A]    = &clk_rpmh_bi_tcxo_div2_ao.hw,
+	[RPMH_CXO_CLK]		= &clk_fixed_factor_bi_tcxo_div2.hw,
+	[RPMH_CXO_CLK_A]	= &clk_fixed_factor_bi_tcxo_div2_ao.hw,
+	[RPMH_LN_BB_CLK1]	= &clk_rpmh_clk6_a2.hw,
+	[RPMH_LN_BB_CLK1_A]	= &clk_rpmh_clk6_a2_ao.hw,
+	[RPMH_LN_BB_CLK2]	= &clk_rpmh_clk7_a2.hw,
+	[RPMH_LN_BB_CLK2_A]	= &clk_rpmh_clk7_a2_ao.hw,
+	[RPMH_LN_BB_CLK3]	= &clk_rpmh_clk8_a2.hw,
+	[RPMH_LN_BB_CLK3_A]	= &clk_rpmh_clk8_a2_ao.hw,
+	[RPMH_RF_CLK1]		= &clk_rpmh_clk1_a1.hw,
+	[RPMH_RF_CLK1_A]	= &clk_rpmh_clk1_a1_ao.hw,
+	[RPMH_RF_CLK2]		= &clk_rpmh_clk2_a1.hw,
+	[RPMH_RF_CLK2_A]	= &clk_rpmh_clk2_a1_ao.hw,
+	[RPMH_RF_CLK3]		= &clk_rpmh_clk3_a1.hw,
+	[RPMH_RF_CLK3_A]	= &clk_rpmh_clk3_a1_ao.hw,
+	[RPMH_RF_CLK4]		= &clk_rpmh_clk4_a1.hw,
+	[RPMH_RF_CLK4_A]	= &clk_rpmh_clk4_a1_ao.hw,
+	[RPMH_IPA_CLK]		= &clk_rpmh_ipa.hw,
+};
+
+static const struct clk_rpmh_desc clk_rpmh_sm8550 = {
+	.clks = sm8550_rpmh_clocks,
+	.num_clks = ARRAY_SIZE(sm8550_rpmh_clocks),
+};
+
 static struct clk_hw *sc7280_rpmh_clocks[] = {
 	[RPMH_CXO_CLK]      = &clk_rpmh_bi_tcxo_div4.hw,
 	[RPMH_CXO_CLK_A]    = &clk_rpmh_bi_tcxo_div4_ao.hw,
@@ -683,29 +750,31 @@ static int clk_rpmh_probe(struct platform_device *pdev)
 
 		name = hw_clks[i]->init->name;
 
-		rpmh_clk = to_clk_rpmh(hw_clks[i]);
-		res_addr = cmd_db_read_addr(rpmh_clk->res_name);
-		if (!res_addr) {
-			dev_err(&pdev->dev, "missing RPMh resource address for %s\n",
-				rpmh_clk->res_name);
-			return -ENODEV;
-		}
+		if (hw_clks[i]->init->ops != &clk_fixed_factor_ops) {
+			rpmh_clk = to_clk_rpmh(hw_clks[i]);
+			res_addr = cmd_db_read_addr(rpmh_clk->res_name);
+			if (!res_addr) {
+				dev_err(&pdev->dev, "missing RPMh resource address for %s\n",
+					rpmh_clk->res_name);
+				return -ENODEV;
+			}
 
-		data = cmd_db_read_aux_data(rpmh_clk->res_name, &aux_data_len);
-		if (IS_ERR(data)) {
-			ret = PTR_ERR(data);
-			dev_err(&pdev->dev,
-				"error reading RPMh aux data for %s (%d)\n",
-				rpmh_clk->res_name, ret);
-			return ret;
-		}
+			data = cmd_db_read_aux_data(rpmh_clk->res_name, &aux_data_len);
+			if (IS_ERR(data)) {
+				ret = PTR_ERR(data);
+				dev_err(&pdev->dev,
+					"error reading RPMh aux data for %s (%d)\n",
+					rpmh_clk->res_name, ret);
+				return ret;
+			}
 
-		/* Convert unit from Khz to Hz */
-		if (aux_data_len == sizeof(*data))
-			rpmh_clk->unit = le32_to_cpu(data->unit) * 1000ULL;
+			/* Convert unit from Khz to Hz */
+			if (aux_data_len == sizeof(*data))
+				rpmh_clk->unit = le32_to_cpu(data->unit) * 1000ULL;
 
-		rpmh_clk->res_addr += res_addr;
-		rpmh_clk->dev = &pdev->dev;
+			rpmh_clk->res_addr += res_addr;
+			rpmh_clk->dev = &pdev->dev;
+		}
 
 		ret = devm_clk_hw_register(&pdev->dev, hw_clks[i]);
 		if (ret) {
@@ -741,6 +810,7 @@ static const struct of_device_id clk_rpmh_match_table[] = {
 	{ .compatible = "qcom,sm8250-rpmh-clk", .data = &clk_rpmh_sm8250},
 	{ .compatible = "qcom,sm8350-rpmh-clk", .data = &clk_rpmh_sm8350},
 	{ .compatible = "qcom,sm8450-rpmh-clk", .data = &clk_rpmh_sm8450},
+	{ .compatible = "qcom,sm8550-rpmh-clk", .data = &clk_rpmh_sm8550},
 	{ .compatible = "qcom,sc7280-rpmh-clk", .data = &clk_rpmh_sc7280},
 	{ }
 };
-- 
2.34.1

Re: [PATCH v6 4/5] clk: qcom: rpmh: Add support for SM8550 rpmh clocks

[Dmitry Baryshkov]

On 07/12/2022 00:45, Abel Vesa wrote:
> Adds the RPMH clocks present in SM8550 SoC.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>   drivers/clk/qcom/clk-rpmh.c | 110 +++++++++++++++++++++++++++++-------
>   1 file changed, 90 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c
> index 2c2ef4b6d130..ce81c76ed0fd 100644
> --- a/drivers/clk/qcom/clk-rpmh.c
> +++ b/drivers/clk/qcom/clk-rpmh.c
> @@ -130,6 +130,34 @@ static DEFINE_MUTEX(rpmh_clk_lock);
>   		},							\
>   	}
>   
> +#define DEFINE_CLK_FIXED_FACTOR(_name, _parent_name, _div)		\
> +	static struct clk_fixed_factor clk_fixed_factor##_##_name = {	\
> +		.mult = 1,						\
> +		.div = _div,						\
> +		.hw.init = &(struct clk_init_data){			\
> +			.ops = &clk_fixed_factor_ops,			\
> +			.name = #_name,					\
> +			.parent_data =  &(const struct clk_parent_data){ \
> +				.fw_name = #_parent_name,		\
> +				.name = #_parent_name,			\
> +			},						\
> +			.num_parents = 1,				\
> +		},							\
> +	};								\
> +	static struct clk_fixed_factor clk_fixed_factor##_##_name##_ao = { \
> +		.mult = 1,						\
> +		.div = _div,						\
> +		.hw.init = &(struct clk_init_data){			\
> +			.ops = &clk_fixed_factor_ops,			\
> +			.name = #_name "_ao",				\
> +			.parent_data =  &(const struct clk_parent_data){ \
> +				.fw_name = #_parent_name "_ao",		\
> +				.name = #_parent_name "_ao",		\
> +			},						\
> +			.num_parents = 1,				\
> +		},							\
> +	}
> +
>   static inline struct clk_rpmh *to_clk_rpmh(struct clk_hw *_hw)
>   {
>   	return container_of(_hw, struct clk_rpmh, hw);
> @@ -345,6 +373,8 @@ DEFINE_CLK_RPMH_ARC(bi_tcxo, "xo.lvl", 0x3, 2);
>   DEFINE_CLK_RPMH_ARC(bi_tcxo, "xo.lvl", 0x3, 4);
>   DEFINE_CLK_RPMH_ARC(qlink, "qphy.lvl", 0x1, 4);
>   
> +DEFINE_CLK_FIXED_FACTOR(bi_tcxo_div2, bi_tcxo, 2);
> +
>   DEFINE_CLK_RPMH_VRM(ln_bb_clk1, _a2, "lnbclka1", 2);
>   DEFINE_CLK_RPMH_VRM(ln_bb_clk2, _a2, "lnbclka2", 2);
>   DEFINE_CLK_RPMH_VRM(ln_bb_clk3, _a2, "lnbclka3", 2);
> @@ -366,6 +396,16 @@ DEFINE_CLK_RPMH_VRM(rf_clk2, _d, "rfclkd2", 1);
>   DEFINE_CLK_RPMH_VRM(rf_clk3, _d, "rfclkd3", 1);
>   DEFINE_CLK_RPMH_VRM(rf_clk4, _d, "rfclkd4", 1);
>   
> +DEFINE_CLK_RPMH_VRM(clk1, _a1, "clka1", 1);
> +DEFINE_CLK_RPMH_VRM(clk2, _a1, "clka2", 1);
> +DEFINE_CLK_RPMH_VRM(clk3, _a1, "clka3", 1);
> +DEFINE_CLK_RPMH_VRM(clk4, _a1, "clka4", 1);
> +DEFINE_CLK_RPMH_VRM(clk5, _a1, "clka5", 1);
> +
> +DEFINE_CLK_RPMH_VRM(clk6, _a2, "clka6", 2);
> +DEFINE_CLK_RPMH_VRM(clk7, _a2, "clka7", 2);
> +DEFINE_CLK_RPMH_VRM(clk8, _a2, "clka8", 2);
> +
>   DEFINE_CLK_RPMH_VRM(div_clk1, _div2, "divclka1", 2);
>   
>   DEFINE_CLK_RPMH_BCM(ce, "CE0");
> @@ -576,6 +616,33 @@ static const struct clk_rpmh_desc clk_rpmh_sm8450 = {
>   	.num_clks = ARRAY_SIZE(sm8450_rpmh_clocks),
>   };
>   
> +static struct clk_hw *sm8550_rpmh_clocks[] = {
> +	[RPMH_CXO_PAD_CLK]      = &clk_rpmh_bi_tcxo_div2.hw,
> +	[RPMH_CXO_PAD_CLK_A]    = &clk_rpmh_bi_tcxo_div2_ao.hw,
> +	[RPMH_CXO_CLK]		= &clk_fixed_factor_bi_tcxo_div2.hw,
> +	[RPMH_CXO_CLK_A]	= &clk_fixed_factor_bi_tcxo_div2_ao.hw,
> +	[RPMH_LN_BB_CLK1]	= &clk_rpmh_clk6_a2.hw,
> +	[RPMH_LN_BB_CLK1_A]	= &clk_rpmh_clk6_a2_ao.hw,
> +	[RPMH_LN_BB_CLK2]	= &clk_rpmh_clk7_a2.hw,
> +	[RPMH_LN_BB_CLK2_A]	= &clk_rpmh_clk7_a2_ao.hw,
> +	[RPMH_LN_BB_CLK3]	= &clk_rpmh_clk8_a2.hw,
> +	[RPMH_LN_BB_CLK3_A]	= &clk_rpmh_clk8_a2_ao.hw,
> +	[RPMH_RF_CLK1]		= &clk_rpmh_clk1_a1.hw,
> +	[RPMH_RF_CLK1_A]	= &clk_rpmh_clk1_a1_ao.hw,
> +	[RPMH_RF_CLK2]		= &clk_rpmh_clk2_a1.hw,
> +	[RPMH_RF_CLK2_A]	= &clk_rpmh_clk2_a1_ao.hw,
> +	[RPMH_RF_CLK3]		= &clk_rpmh_clk3_a1.hw,
> +	[RPMH_RF_CLK3_A]	= &clk_rpmh_clk3_a1_ao.hw,
> +	[RPMH_RF_CLK4]		= &clk_rpmh_clk4_a1.hw,
> +	[RPMH_RF_CLK4_A]	= &clk_rpmh_clk4_a1_ao.hw,
> +	[RPMH_IPA_CLK]		= &clk_rpmh_ipa.hw,
> +};
> +
> +static const struct clk_rpmh_desc clk_rpmh_sm8550 = {
> +	.clks = sm8550_rpmh_clocks,
> +	.num_clks = ARRAY_SIZE(sm8550_rpmh_clocks),
> +};
> +
>   static struct clk_hw *sc7280_rpmh_clocks[] = {
>   	[RPMH_CXO_CLK]      = &clk_rpmh_bi_tcxo_div4.hw,
>   	[RPMH_CXO_CLK_A]    = &clk_rpmh_bi_tcxo_div4_ao.hw,
> @@ -683,29 +750,31 @@ static int clk_rpmh_probe(struct platform_device *pdev)
>   
>   		name = hw_clks[i]->init->name;
>   
> -		rpmh_clk = to_clk_rpmh(hw_clks[i]);
> -		res_addr = cmd_db_read_addr(rpmh_clk->res_name);
> -		if (!res_addr) {
> -			dev_err(&pdev->dev, "missing RPMh resource address for %s\n",
> -				rpmh_clk->res_name);
> -			return -ENODEV;
> -		}
> +		if (hw_clks[i]->init->ops != &clk_fixed_factor_ops) {

We discussed this separately, the fixed factor clocks will be moved to 
the child nodes, leaving rpmhcc with only cmd-db based clocks.

> +			rpmh_clk = to_clk_rpmh(hw_clks[i]);
> +			res_addr = cmd_db_read_addr(rpmh_clk->res_name);
> +			if (!res_addr) {
> +				dev_err(&pdev->dev, "missing RPMh resource address for %s\n",
> +					rpmh_clk->res_name);
> +				return -ENODEV;
> +			}
>   
> -		data = cmd_db_read_aux_data(rpmh_clk->res_name, &aux_data_len);
> -		if (IS_ERR(data)) {
> -			ret = PTR_ERR(data);
> -			dev_err(&pdev->dev,
> -				"error reading RPMh aux data for %s (%d)\n",
> -				rpmh_clk->res_name, ret);
> -			return ret;
> -		}
> +			data = cmd_db_read_aux_data(rpmh_clk->res_name, &aux_data_len);
> +			if (IS_ERR(data)) {
> +				ret = PTR_ERR(data);
> +				dev_err(&pdev->dev,
> +					"error reading RPMh aux data for %s (%d)\n",
> +					rpmh_clk->res_name, ret);
> +				return ret;
> +			}
>   
> -		/* Convert unit from Khz to Hz */
> -		if (aux_data_len == sizeof(*data))
> -			rpmh_clk->unit = le32_to_cpu(data->unit) * 1000ULL;
> +			/* Convert unit from Khz to Hz */
> +			if (aux_data_len == sizeof(*data))
> +				rpmh_clk->unit = le32_to_cpu(data->unit) * 1000ULL;
>   
> -		rpmh_clk->res_addr += res_addr;
> -		rpmh_clk->dev = &pdev->dev;
> +			rpmh_clk->res_addr += res_addr;
> +			rpmh_clk->dev = &pdev->dev;
> +		}
>   
>   		ret = devm_clk_hw_register(&pdev->dev, hw_clks[i]);
>   		if (ret) {
> @@ -741,6 +810,7 @@ static const struct of_device_id clk_rpmh_match_table[] = {
>   	{ .compatible = "qcom,sm8250-rpmh-clk", .data = &clk_rpmh_sm8250},
>   	{ .compatible = "qcom,sm8350-rpmh-clk", .data = &clk_rpmh_sm8350},
>   	{ .compatible = "qcom,sm8450-rpmh-clk", .data = &clk_rpmh_sm8450},
> +	{ .compatible = "qcom,sm8550-rpmh-clk", .data = &clk_rpmh_sm8550},
>   	{ .compatible = "qcom,sc7280-rpmh-clk", .data = &clk_rpmh_sc7280},
>   	{ }
>   };

-- 
With best wishes
Dmitry

Re: [PATCH v6 4/5] clk: qcom: rpmh: Add support for SM8550 rpmh clocks

[Bjorn Andersson]

On Wed, Dec 14, 2022 at 06:25:01PM +0200, Dmitry Baryshkov wrote:
> On 07/12/2022 00:45, Abel Vesa wrote:
> > Adds the RPMH clocks present in SM8550 SoC.
> > 
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> > ---
> >   drivers/clk/qcom/clk-rpmh.c | 110 +++++++++++++++++++++++++++++-------
> >   1 file changed, 90 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c
> > index 2c2ef4b6d130..ce81c76ed0fd 100644
> > --- a/drivers/clk/qcom/clk-rpmh.c
> > +++ b/drivers/clk/qcom/clk-rpmh.c
> > @@ -130,6 +130,34 @@ static DEFINE_MUTEX(rpmh_clk_lock);
> >   		},							\
> >   	}
> > +#define DEFINE_CLK_FIXED_FACTOR(_name, _parent_name, _div)		\
> > +	static struct clk_fixed_factor clk_fixed_factor##_##_name = {	\
> > +		.mult = 1,						\
> > +		.div = _div,						\
> > +		.hw.init = &(struct clk_init_data){			\
> > +			.ops = &clk_fixed_factor_ops,			\
> > +			.name = #_name,					\
> > +			.parent_data =  &(const struct clk_parent_data){ \
> > +				.fw_name = #_parent_name,		\
> > +				.name = #_parent_name,			\
> > +			},						\
> > +			.num_parents = 1,				\
> > +		},							\
> > +	};								\
> > +	static struct clk_fixed_factor clk_fixed_factor##_##_name##_ao = { \
> > +		.mult = 1,						\
> > +		.div = _div,						\
> > +		.hw.init = &(struct clk_init_data){			\
> > +			.ops = &clk_fixed_factor_ops,			\
> > +			.name = #_name "_ao",				\
> > +			.parent_data =  &(const struct clk_parent_data){ \
> > +				.fw_name = #_parent_name "_ao",		\
> > +				.name = #_parent_name "_ao",		\
> > +			},						\
> > +			.num_parents = 1,				\
> > +		},							\
> > +	}
> > +
> >   static inline struct clk_rpmh *to_clk_rpmh(struct clk_hw *_hw)
> >   {
> >   	return container_of(_hw, struct clk_rpmh, hw);
> > @@ -345,6 +373,8 @@ DEFINE_CLK_RPMH_ARC(bi_tcxo, "xo.lvl", 0x3, 2);
> >   DEFINE_CLK_RPMH_ARC(bi_tcxo, "xo.lvl", 0x3, 4);
> >   DEFINE_CLK_RPMH_ARC(qlink, "qphy.lvl", 0x1, 4);
> > +DEFINE_CLK_FIXED_FACTOR(bi_tcxo_div2, bi_tcxo, 2);
> > +
> >   DEFINE_CLK_RPMH_VRM(ln_bb_clk1, _a2, "lnbclka1", 2);
> >   DEFINE_CLK_RPMH_VRM(ln_bb_clk2, _a2, "lnbclka2", 2);
> >   DEFINE_CLK_RPMH_VRM(ln_bb_clk3, _a2, "lnbclka3", 2);
> > @@ -366,6 +396,16 @@ DEFINE_CLK_RPMH_VRM(rf_clk2, _d, "rfclkd2", 1);
> >   DEFINE_CLK_RPMH_VRM(rf_clk3, _d, "rfclkd3", 1);
> >   DEFINE_CLK_RPMH_VRM(rf_clk4, _d, "rfclkd4", 1);
> > +DEFINE_CLK_RPMH_VRM(clk1, _a1, "clka1", 1);
> > +DEFINE_CLK_RPMH_VRM(clk2, _a1, "clka2", 1);
> > +DEFINE_CLK_RPMH_VRM(clk3, _a1, "clka3", 1);
> > +DEFINE_CLK_RPMH_VRM(clk4, _a1, "clka4", 1);
> > +DEFINE_CLK_RPMH_VRM(clk5, _a1, "clka5", 1);
> > +
> > +DEFINE_CLK_RPMH_VRM(clk6, _a2, "clka6", 2);
> > +DEFINE_CLK_RPMH_VRM(clk7, _a2, "clka7", 2);
> > +DEFINE_CLK_RPMH_VRM(clk8, _a2, "clka8", 2);
> > +
> >   DEFINE_CLK_RPMH_VRM(div_clk1, _div2, "divclka1", 2);
> >   DEFINE_CLK_RPMH_BCM(ce, "CE0");
> > @@ -576,6 +616,33 @@ static const struct clk_rpmh_desc clk_rpmh_sm8450 = {
> >   	.num_clks = ARRAY_SIZE(sm8450_rpmh_clocks),
> >   };
> > +static struct clk_hw *sm8550_rpmh_clocks[] = {
> > +	[RPMH_CXO_PAD_CLK]      = &clk_rpmh_bi_tcxo_div2.hw,
> > +	[RPMH_CXO_PAD_CLK_A]    = &clk_rpmh_bi_tcxo_div2_ao.hw,
> > +	[RPMH_CXO_CLK]		= &clk_fixed_factor_bi_tcxo_div2.hw,
> > +	[RPMH_CXO_CLK_A]	= &clk_fixed_factor_bi_tcxo_div2_ao.hw,
> > +	[RPMH_LN_BB_CLK1]	= &clk_rpmh_clk6_a2.hw,
> > +	[RPMH_LN_BB_CLK1_A]	= &clk_rpmh_clk6_a2_ao.hw,
> > +	[RPMH_LN_BB_CLK2]	= &clk_rpmh_clk7_a2.hw,
> > +	[RPMH_LN_BB_CLK2_A]	= &clk_rpmh_clk7_a2_ao.hw,
> > +	[RPMH_LN_BB_CLK3]	= &clk_rpmh_clk8_a2.hw,
> > +	[RPMH_LN_BB_CLK3_A]	= &clk_rpmh_clk8_a2_ao.hw,
> > +	[RPMH_RF_CLK1]		= &clk_rpmh_clk1_a1.hw,
> > +	[RPMH_RF_CLK1_A]	= &clk_rpmh_clk1_a1_ao.hw,
> > +	[RPMH_RF_CLK2]		= &clk_rpmh_clk2_a1.hw,
> > +	[RPMH_RF_CLK2_A]	= &clk_rpmh_clk2_a1_ao.hw,
> > +	[RPMH_RF_CLK3]		= &clk_rpmh_clk3_a1.hw,
> > +	[RPMH_RF_CLK3_A]	= &clk_rpmh_clk3_a1_ao.hw,
> > +	[RPMH_RF_CLK4]		= &clk_rpmh_clk4_a1.hw,
> > +	[RPMH_RF_CLK4_A]	= &clk_rpmh_clk4_a1_ao.hw,
> > +	[RPMH_IPA_CLK]		= &clk_rpmh_ipa.hw,
> > +};
> > +
> > +static const struct clk_rpmh_desc clk_rpmh_sm8550 = {
> > +	.clks = sm8550_rpmh_clocks,
> > +	.num_clks = ARRAY_SIZE(sm8550_rpmh_clocks),
> > +};
> > +
> >   static struct clk_hw *sc7280_rpmh_clocks[] = {
> >   	[RPMH_CXO_CLK]      = &clk_rpmh_bi_tcxo_div4.hw,
> >   	[RPMH_CXO_CLK_A]    = &clk_rpmh_bi_tcxo_div4_ao.hw,
> > @@ -683,29 +750,31 @@ static int clk_rpmh_probe(struct platform_device *pdev)
> >   		name = hw_clks[i]->init->name;
> > -		rpmh_clk = to_clk_rpmh(hw_clks[i]);
> > -		res_addr = cmd_db_read_addr(rpmh_clk->res_name);
> > -		if (!res_addr) {
> > -			dev_err(&pdev->dev, "missing RPMh resource address for %s\n",
> > -				rpmh_clk->res_name);
> > -			return -ENODEV;
> > -		}
> > +		if (hw_clks[i]->init->ops != &clk_fixed_factor_ops) {
> 
> We discussed this separately, the fixed factor clocks will be moved to the
> child nodes, leaving rpmhcc with only cmd-db based clocks.
> 

Are you saying that you will represent bi_tcxo as a fixed-factor-clock
under /clocks with RPMH_CXO_PAD_CLK as parent and a clock-div = <2>; ?

If so that sounds reasonable to me, but adding Mike for his
input/information.

Regards,
Bjorn

> > +			rpmh_clk = to_clk_rpmh(hw_clks[i]);
> > +			res_addr = cmd_db_read_addr(rpmh_clk->res_name);
> > +			if (!res_addr) {
> > +				dev_err(&pdev->dev, "missing RPMh resource address for %s\n",
> > +					rpmh_clk->res_name);
> > +				return -ENODEV;
> > +			}
> > -		data = cmd_db_read_aux_data(rpmh_clk->res_name, &aux_data_len);
> > -		if (IS_ERR(data)) {
> > -			ret = PTR_ERR(data);
> > -			dev_err(&pdev->dev,
> > -				"error reading RPMh aux data for %s (%d)\n",
> > -				rpmh_clk->res_name, ret);
> > -			return ret;
> > -		}
> > +			data = cmd_db_read_aux_data(rpmh_clk->res_name, &aux_data_len);
> > +			if (IS_ERR(data)) {
> > +				ret = PTR_ERR(data);
> > +				dev_err(&pdev->dev,
> > +					"error reading RPMh aux data for %s (%d)\n",
> > +					rpmh_clk->res_name, ret);
> > +				return ret;
> > +			}
> > -		/* Convert unit from Khz to Hz */
> > -		if (aux_data_len == sizeof(*data))
> > -			rpmh_clk->unit = le32_to_cpu(data->unit) * 1000ULL;
> > +			/* Convert unit from Khz to Hz */
> > +			if (aux_data_len == sizeof(*data))
> > +				rpmh_clk->unit = le32_to_cpu(data->unit) * 1000ULL;
> > -		rpmh_clk->res_addr += res_addr;
> > -		rpmh_clk->dev = &pdev->dev;
> > +			rpmh_clk->res_addr += res_addr;
> > +			rpmh_clk->dev = &pdev->dev;
> > +		}
> >   		ret = devm_clk_hw_register(&pdev->dev, hw_clks[i]);
> >   		if (ret) {
> > @@ -741,6 +810,7 @@ static const struct of_device_id clk_rpmh_match_table[] = {
> >   	{ .compatible = "qcom,sm8250-rpmh-clk", .data = &clk_rpmh_sm8250},
> >   	{ .compatible = "qcom,sm8350-rpmh-clk", .data = &clk_rpmh_sm8350},
> >   	{ .compatible = "qcom,sm8450-rpmh-clk", .data = &clk_rpmh_sm8450},
> > +	{ .compatible = "qcom,sm8550-rpmh-clk", .data = &clk_rpmh_sm8550},
> >   	{ .compatible = "qcom,sc7280-rpmh-clk", .data = &clk_rpmh_sc7280},
> >   	{ }
> >   };
> 
> -- 
> With best wishes
> Dmitry
> 

Re: [PATCH v6 4/5] clk: qcom: rpmh: Add support for SM8550 rpmh clocks

[Dmitry Baryshkov]

On 28/12/2022 20:52, Bjorn Andersson wrote:
> On Wed, Dec 14, 2022 at 06:25:01PM +0200, Dmitry Baryshkov wrote:
>> On 07/12/2022 00:45, Abel Vesa wrote:
>>> Adds the RPMH clocks present in SM8550 SoC.
>>>
>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>> ---
>>>    drivers/clk/qcom/clk-rpmh.c | 110 +++++++++++++++++++++++++++++-------
>>>    1 file changed, 90 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c
>>> index 2c2ef4b6d130..ce81c76ed0fd 100644
>>> --- a/drivers/clk/qcom/clk-rpmh.c
>>> +++ b/drivers/clk/qcom/clk-rpmh.c
>>> @@ -130,6 +130,34 @@ static DEFINE_MUTEX(rpmh_clk_lock);
>>>    		},							\
>>>    	}
>>> +#define DEFINE_CLK_FIXED_FACTOR(_name, _parent_name, _div)		\
>>> +	static struct clk_fixed_factor clk_fixed_factor##_##_name = {	\
>>> +		.mult = 1,						\
>>> +		.div = _div,						\
>>> +		.hw.init = &(struct clk_init_data){			\
>>> +			.ops = &clk_fixed_factor_ops,			\
>>> +			.name = #_name,					\
>>> +			.parent_data =  &(const struct clk_parent_data){ \
>>> +				.fw_name = #_parent_name,		\
>>> +				.name = #_parent_name,			\
>>> +			},						\
>>> +			.num_parents = 1,				\
>>> +		},							\
>>> +	};								\
>>> +	static struct clk_fixed_factor clk_fixed_factor##_##_name##_ao = { \
>>> +		.mult = 1,						\
>>> +		.div = _div,						\
>>> +		.hw.init = &(struct clk_init_data){			\
>>> +			.ops = &clk_fixed_factor_ops,			\
>>> +			.name = #_name "_ao",				\
>>> +			.parent_data =  &(const struct clk_parent_data){ \
>>> +				.fw_name = #_parent_name "_ao",		\
>>> +				.name = #_parent_name "_ao",		\
>>> +			},						\
>>> +			.num_parents = 1,				\
>>> +		},							\
>>> +	}
>>> +
>>>    static inline struct clk_rpmh *to_clk_rpmh(struct clk_hw *_hw)
>>>    {
>>>    	return container_of(_hw, struct clk_rpmh, hw);
>>> @@ -345,6 +373,8 @@ DEFINE_CLK_RPMH_ARC(bi_tcxo, "xo.lvl", 0x3, 2);
>>>    DEFINE_CLK_RPMH_ARC(bi_tcxo, "xo.lvl", 0x3, 4);
>>>    DEFINE_CLK_RPMH_ARC(qlink, "qphy.lvl", 0x1, 4);
>>> +DEFINE_CLK_FIXED_FACTOR(bi_tcxo_div2, bi_tcxo, 2);
>>> +
>>>    DEFINE_CLK_RPMH_VRM(ln_bb_clk1, _a2, "lnbclka1", 2);
>>>    DEFINE_CLK_RPMH_VRM(ln_bb_clk2, _a2, "lnbclka2", 2);
>>>    DEFINE_CLK_RPMH_VRM(ln_bb_clk3, _a2, "lnbclka3", 2);
>>> @@ -366,6 +396,16 @@ DEFINE_CLK_RPMH_VRM(rf_clk2, _d, "rfclkd2", 1);
>>>    DEFINE_CLK_RPMH_VRM(rf_clk3, _d, "rfclkd3", 1);
>>>    DEFINE_CLK_RPMH_VRM(rf_clk4, _d, "rfclkd4", 1);
>>> +DEFINE_CLK_RPMH_VRM(clk1, _a1, "clka1", 1);
>>> +DEFINE_CLK_RPMH_VRM(clk2, _a1, "clka2", 1);
>>> +DEFINE_CLK_RPMH_VRM(clk3, _a1, "clka3", 1);
>>> +DEFINE_CLK_RPMH_VRM(clk4, _a1, "clka4", 1);
>>> +DEFINE_CLK_RPMH_VRM(clk5, _a1, "clka5", 1);
>>> +
>>> +DEFINE_CLK_RPMH_VRM(clk6, _a2, "clka6", 2);
>>> +DEFINE_CLK_RPMH_VRM(clk7, _a2, "clka7", 2);
>>> +DEFINE_CLK_RPMH_VRM(clk8, _a2, "clka8", 2);
>>> +
>>>    DEFINE_CLK_RPMH_VRM(div_clk1, _div2, "divclka1", 2);
>>>    DEFINE_CLK_RPMH_BCM(ce, "CE0");
>>> @@ -576,6 +616,33 @@ static const struct clk_rpmh_desc clk_rpmh_sm8450 = {
>>>    	.num_clks = ARRAY_SIZE(sm8450_rpmh_clocks),
>>>    };
>>> +static struct clk_hw *sm8550_rpmh_clocks[] = {
>>> +	[RPMH_CXO_PAD_CLK]      = &clk_rpmh_bi_tcxo_div2.hw,
>>> +	[RPMH_CXO_PAD_CLK_A]    = &clk_rpmh_bi_tcxo_div2_ao.hw,
>>> +	[RPMH_CXO_CLK]		= &clk_fixed_factor_bi_tcxo_div2.hw,
>>> +	[RPMH_CXO_CLK_A]	= &clk_fixed_factor_bi_tcxo_div2_ao.hw,
>>> +	[RPMH_LN_BB_CLK1]	= &clk_rpmh_clk6_a2.hw,
>>> +	[RPMH_LN_BB_CLK1_A]	= &clk_rpmh_clk6_a2_ao.hw,
>>> +	[RPMH_LN_BB_CLK2]	= &clk_rpmh_clk7_a2.hw,
>>> +	[RPMH_LN_BB_CLK2_A]	= &clk_rpmh_clk7_a2_ao.hw,
>>> +	[RPMH_LN_BB_CLK3]	= &clk_rpmh_clk8_a2.hw,
>>> +	[RPMH_LN_BB_CLK3_A]	= &clk_rpmh_clk8_a2_ao.hw,
>>> +	[RPMH_RF_CLK1]		= &clk_rpmh_clk1_a1.hw,
>>> +	[RPMH_RF_CLK1_A]	= &clk_rpmh_clk1_a1_ao.hw,
>>> +	[RPMH_RF_CLK2]		= &clk_rpmh_clk2_a1.hw,
>>> +	[RPMH_RF_CLK2_A]	= &clk_rpmh_clk2_a1_ao.hw,
>>> +	[RPMH_RF_CLK3]		= &clk_rpmh_clk3_a1.hw,
>>> +	[RPMH_RF_CLK3_A]	= &clk_rpmh_clk3_a1_ao.hw,
>>> +	[RPMH_RF_CLK4]		= &clk_rpmh_clk4_a1.hw,
>>> +	[RPMH_RF_CLK4_A]	= &clk_rpmh_clk4_a1_ao.hw,
>>> +	[RPMH_IPA_CLK]		= &clk_rpmh_ipa.hw,
>>> +};
>>> +
>>> +static const struct clk_rpmh_desc clk_rpmh_sm8550 = {
>>> +	.clks = sm8550_rpmh_clocks,
>>> +	.num_clks = ARRAY_SIZE(sm8550_rpmh_clocks),
>>> +};
>>> +
>>>    static struct clk_hw *sc7280_rpmh_clocks[] = {
>>>    	[RPMH_CXO_CLK]      = &clk_rpmh_bi_tcxo_div4.hw,
>>>    	[RPMH_CXO_CLK_A]    = &clk_rpmh_bi_tcxo_div4_ao.hw,
>>> @@ -683,29 +750,31 @@ static int clk_rpmh_probe(struct platform_device *pdev)
>>>    		name = hw_clks[i]->init->name;
>>> -		rpmh_clk = to_clk_rpmh(hw_clks[i]);
>>> -		res_addr = cmd_db_read_addr(rpmh_clk->res_name);
>>> -		if (!res_addr) {
>>> -			dev_err(&pdev->dev, "missing RPMh resource address for %s\n",
>>> -				rpmh_clk->res_name);
>>> -			return -ENODEV;
>>> -		}
>>> +		if (hw_clks[i]->init->ops != &clk_fixed_factor_ops) {
>>
>> We discussed this separately, the fixed factor clocks will be moved to the
>> child nodes, leaving rpmhcc with only cmd-db based clocks.
>>
> 
> Are you saying that you will represent bi_tcxo as a fixed-factor-clock
> under /clocks with RPMH_CXO_PAD_CLK as parent and a clock-div = <2>; ?

Yes, this was the idea. The rpmhcc driver is pretty much centric around 
the cmd-db clocks. Adding a fixed-factor clock results either in a 
horrible hacks or in a significant code refactoring. However we already 
have an existing way to fixed-factor clocks: DT nodes. Adding support 
for such nodes to rpmhcc driver requires just a single additional API 
call: devm_of_platform_populate().

> If so that sounds reasonable to me, but adding Mike for his
> input/information.

-- 
With best wishes
Dmitry

Re: [PATCH v6 4/5] clk: qcom: rpmh: Add support for SM8550 rpmh clocks

[Krzysztof Kozlowski]

On 28/12/2022 19:59, Dmitry Baryshkov wrote:
> On 28/12/2022 20:52, Bjorn Andersson wrote:
>> On Wed, Dec 14, 2022 at 06:25:01PM +0200, Dmitry Baryshkov wrote:
>>> On 07/12/2022 00:45, Abel Vesa wrote:
>>>> Adds the RPMH clocks present in SM8550 SoC.
>>>>
>>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>>> ---
>>>>    drivers/clk/qcom/clk-rpmh.c | 110 +++++++++++++++++++++++++++++-------
>>>>    1 file changed, 90 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c
>>>> index 2c2ef4b6d130..ce81c76ed0fd 100644
>>>> --- a/drivers/clk/qcom/clk-rpmh.c
>>>> +++ b/drivers/clk/qcom/clk-rpmh.c
>>>> @@ -130,6 +130,34 @@ static DEFINE_MUTEX(rpmh_clk_lock);
>>>>    		},							\
>>>>    	}
>>>> +#define DEFINE_CLK_FIXED_FACTOR(_name, _parent_name, _div)		\
>>>> +	static struct clk_fixed_factor clk_fixed_factor##_##_name = {	\
>>>> +		.mult = 1,						\
>>>> +		.div = _div,						\
>>>> +		.hw.init = &(struct clk_init_data){			\
>>>> +			.ops = &clk_fixed_factor_ops,			\
>>>> +			.name = #_name,					\
>>>> +			.parent_data =  &(const struct clk_parent_data){ \
>>>> +				.fw_name = #_parent_name,		\
>>>> +				.name = #_parent_name,			\
>>>> +			},						\
>>>> +			.num_parents = 1,				\
>>>> +		},							\
>>>> +	};								\
>>>> +	static struct clk_fixed_factor clk_fixed_factor##_##_name##_ao = { \
>>>> +		.mult = 1,						\
>>>> +		.div = _div,						\
>>>> +		.hw.init = &(struct clk_init_data){			\
>>>> +			.ops = &clk_fixed_factor_ops,			\
>>>> +			.name = #_name "_ao",				\
>>>> +			.parent_data =  &(const struct clk_parent_data){ \
>>>> +				.fw_name = #_parent_name "_ao",		\
>>>> +				.name = #_parent_name "_ao",		\
>>>> +			},						\
>>>> +			.num_parents = 1,				\
>>>> +		},							\
>>>> +	}
>>>> +
>>>>    static inline struct clk_rpmh *to_clk_rpmh(struct clk_hw *_hw)
>>>>    {
>>>>    	return container_of(_hw, struct clk_rpmh, hw);
>>>> @@ -345,6 +373,8 @@ DEFINE_CLK_RPMH_ARC(bi_tcxo, "xo.lvl", 0x3, 2);
>>>>    DEFINE_CLK_RPMH_ARC(bi_tcxo, "xo.lvl", 0x3, 4);
>>>>    DEFINE_CLK_RPMH_ARC(qlink, "qphy.lvl", 0x1, 4);
>>>> +DEFINE_CLK_FIXED_FACTOR(bi_tcxo_div2, bi_tcxo, 2);
>>>> +
>>>>    DEFINE_CLK_RPMH_VRM(ln_bb_clk1, _a2, "lnbclka1", 2);
>>>>    DEFINE_CLK_RPMH_VRM(ln_bb_clk2, _a2, "lnbclka2", 2);
>>>>    DEFINE_CLK_RPMH_VRM(ln_bb_clk3, _a2, "lnbclka3", 2);
>>>> @@ -366,6 +396,16 @@ DEFINE_CLK_RPMH_VRM(rf_clk2, _d, "rfclkd2", 1);
>>>>    DEFINE_CLK_RPMH_VRM(rf_clk3, _d, "rfclkd3", 1);
>>>>    DEFINE_CLK_RPMH_VRM(rf_clk4, _d, "rfclkd4", 1);
>>>> +DEFINE_CLK_RPMH_VRM(clk1, _a1, "clka1", 1);
>>>> +DEFINE_CLK_RPMH_VRM(clk2, _a1, "clka2", 1);
>>>> +DEFINE_CLK_RPMH_VRM(clk3, _a1, "clka3", 1);
>>>> +DEFINE_CLK_RPMH_VRM(clk4, _a1, "clka4", 1);
>>>> +DEFINE_CLK_RPMH_VRM(clk5, _a1, "clka5", 1);
>>>> +
>>>> +DEFINE_CLK_RPMH_VRM(clk6, _a2, "clka6", 2);
>>>> +DEFINE_CLK_RPMH_VRM(clk7, _a2, "clka7", 2);
>>>> +DEFINE_CLK_RPMH_VRM(clk8, _a2, "clka8", 2);
>>>> +
>>>>    DEFINE_CLK_RPMH_VRM(div_clk1, _div2, "divclka1", 2);
>>>>    DEFINE_CLK_RPMH_BCM(ce, "CE0");
>>>> @@ -576,6 +616,33 @@ static const struct clk_rpmh_desc clk_rpmh_sm8450 = {
>>>>    	.num_clks = ARRAY_SIZE(sm8450_rpmh_clocks),
>>>>    };
>>>> +static struct clk_hw *sm8550_rpmh_clocks[] = {
>>>> +	[RPMH_CXO_PAD_CLK]      = &clk_rpmh_bi_tcxo_div2.hw,
>>>> +	[RPMH_CXO_PAD_CLK_A]    = &clk_rpmh_bi_tcxo_div2_ao.hw,
>>>> +	[RPMH_CXO_CLK]		= &clk_fixed_factor_bi_tcxo_div2.hw,
>>>> +	[RPMH_CXO_CLK_A]	= &clk_fixed_factor_bi_tcxo_div2_ao.hw,
>>>> +	[RPMH_LN_BB_CLK1]	= &clk_rpmh_clk6_a2.hw,
>>>> +	[RPMH_LN_BB_CLK1_A]	= &clk_rpmh_clk6_a2_ao.hw,
>>>> +	[RPMH_LN_BB_CLK2]	= &clk_rpmh_clk7_a2.hw,
>>>> +	[RPMH_LN_BB_CLK2_A]	= &clk_rpmh_clk7_a2_ao.hw,
>>>> +	[RPMH_LN_BB_CLK3]	= &clk_rpmh_clk8_a2.hw,
>>>> +	[RPMH_LN_BB_CLK3_A]	= &clk_rpmh_clk8_a2_ao.hw,
>>>> +	[RPMH_RF_CLK1]		= &clk_rpmh_clk1_a1.hw,
>>>> +	[RPMH_RF_CLK1_A]	= &clk_rpmh_clk1_a1_ao.hw,
>>>> +	[RPMH_RF_CLK2]		= &clk_rpmh_clk2_a1.hw,
>>>> +	[RPMH_RF_CLK2_A]	= &clk_rpmh_clk2_a1_ao.hw,
>>>> +	[RPMH_RF_CLK3]		= &clk_rpmh_clk3_a1.hw,
>>>> +	[RPMH_RF_CLK3_A]	= &clk_rpmh_clk3_a1_ao.hw,
>>>> +	[RPMH_RF_CLK4]		= &clk_rpmh_clk4_a1.hw,
>>>> +	[RPMH_RF_CLK4_A]	= &clk_rpmh_clk4_a1_ao.hw,
>>>> +	[RPMH_IPA_CLK]		= &clk_rpmh_ipa.hw,
>>>> +};
>>>> +
>>>> +static const struct clk_rpmh_desc clk_rpmh_sm8550 = {
>>>> +	.clks = sm8550_rpmh_clocks,
>>>> +	.num_clks = ARRAY_SIZE(sm8550_rpmh_clocks),
>>>> +};
>>>> +
>>>>    static struct clk_hw *sc7280_rpmh_clocks[] = {
>>>>    	[RPMH_CXO_CLK]      = &clk_rpmh_bi_tcxo_div4.hw,
>>>>    	[RPMH_CXO_CLK_A]    = &clk_rpmh_bi_tcxo_div4_ao.hw,
>>>> @@ -683,29 +750,31 @@ static int clk_rpmh_probe(struct platform_device *pdev)
>>>>    		name = hw_clks[i]->init->name;
>>>> -		rpmh_clk = to_clk_rpmh(hw_clks[i]);
>>>> -		res_addr = cmd_db_read_addr(rpmh_clk->res_name);
>>>> -		if (!res_addr) {
>>>> -			dev_err(&pdev->dev, "missing RPMh resource address for %s\n",
>>>> -				rpmh_clk->res_name);
>>>> -			return -ENODEV;
>>>> -		}
>>>> +		if (hw_clks[i]->init->ops != &clk_fixed_factor_ops) {
>>>
>>> We discussed this separately, the fixed factor clocks will be moved to the
>>> child nodes, leaving rpmhcc with only cmd-db based clocks.
>>>
>>
>> Are you saying that you will represent bi_tcxo as a fixed-factor-clock
>> under /clocks with RPMH_CXO_PAD_CLK as parent and a clock-div = <2>; ?
> 
> Yes, this was the idea. The rpmhcc driver is pretty much centric around 
> the cmd-db clocks. Adding a fixed-factor clock results either in a 
> horrible hacks or in a significant code refactoring. However we already 
> have an existing way to fixed-factor clocks: DT nodes. Adding support 
> for such nodes to rpmhcc driver requires just a single additional API 
> call: devm_of_platform_populate().

Please no. DT is not to solve driver issues, skip some code or make
things simpler for driver developers. If everyone - U-boot, *BSD,
firmwares - pushes to DT stuff like this, because this makes their
driver development easier, you would have total mess. Linux does not
have any priorities here in this approach.

Best regards,
Krzysztof

Re: [PATCH v6 4/5] clk: qcom: rpmh: Add support for SM8550 rpmh clocks

[Krzysztof Kozlowski]

On 06/01/2023 08:45, Krzysztof Kozlowski wrote:
> On 28/12/2022 19:59, Dmitry Baryshkov wrote:
>> On 28/12/2022 20:52, Bjorn Andersson wrote:
>>> On Wed, Dec 14, 2022 at 06:25:01PM +0200, Dmitry Baryshkov wrote:
>>>> On 07/12/2022 00:45, Abel Vesa wrote:
>>>>> Adds the RPMH clocks present in SM8550 SoC.
>>>>>
>>>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>>>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>>>> ---
>>>>>    drivers/clk/qcom/clk-rpmh.c | 110 +++++++++++++++++++++++++++++-------
>>>>>    1 file changed, 90 insertions(+), 20 deletions(-)
>>>>>
>>>>> diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c
>>>>> index 2c2ef4b6d130..ce81c76ed0fd 100644
>>>>> --- a/drivers/clk/qcom/clk-rpmh.c
>>>>> +++ b/drivers/clk/qcom/clk-rpmh.c
>>>>> @@ -130,6 +130,34 @@ static DEFINE_MUTEX(rpmh_clk_lock);
>>>>>    		},							\
>>>>>    	}
>>>>> +#define DEFINE_CLK_FIXED_FACTOR(_name, _parent_name, _div)		\
>>>>> +	static struct clk_fixed_factor clk_fixed_factor##_##_name = {	\
>>>>> +		.mult = 1,						\
>>>>> +		.div = _div,						\
>>>>> +		.hw.init = &(struct clk_init_data){			\
>>>>> +			.ops = &clk_fixed_factor_ops,			\
>>>>> +			.name = #_name,					\
>>>>> +			.parent_data =  &(const struct clk_parent_data){ \
>>>>> +				.fw_name = #_parent_name,		\
>>>>> +				.name = #_parent_name,			\
>>>>> +			},						\
>>>>> +			.num_parents = 1,				\
>>>>> +		},							\
>>>>> +	};								\
>>>>> +	static struct clk_fixed_factor clk_fixed_factor##_##_name##_ao = { \
>>>>> +		.mult = 1,						\
>>>>> +		.div = _div,						\
>>>>> +		.hw.init = &(struct clk_init_data){			\
>>>>> +			.ops = &clk_fixed_factor_ops,			\
>>>>> +			.name = #_name "_ao",				\
>>>>> +			.parent_data =  &(const struct clk_parent_data){ \
>>>>> +				.fw_name = #_parent_name "_ao",		\
>>>>> +				.name = #_parent_name "_ao",		\
>>>>> +			},						\
>>>>> +			.num_parents = 1,				\
>>>>> +		},							\
>>>>> +	}
>>>>> +
>>>>>    static inline struct clk_rpmh *to_clk_rpmh(struct clk_hw *_hw)
>>>>>    {
>>>>>    	return container_of(_hw, struct clk_rpmh, hw);
>>>>> @@ -345,6 +373,8 @@ DEFINE_CLK_RPMH_ARC(bi_tcxo, "xo.lvl", 0x3, 2);
>>>>>    DEFINE_CLK_RPMH_ARC(bi_tcxo, "xo.lvl", 0x3, 4);
>>>>>    DEFINE_CLK_RPMH_ARC(qlink, "qphy.lvl", 0x1, 4);
>>>>> +DEFINE_CLK_FIXED_FACTOR(bi_tcxo_div2, bi_tcxo, 2);
>>>>> +
>>>>>    DEFINE_CLK_RPMH_VRM(ln_bb_clk1, _a2, "lnbclka1", 2);
>>>>>    DEFINE_CLK_RPMH_VRM(ln_bb_clk2, _a2, "lnbclka2", 2);
>>>>>    DEFINE_CLK_RPMH_VRM(ln_bb_clk3, _a2, "lnbclka3", 2);
>>>>> @@ -366,6 +396,16 @@ DEFINE_CLK_RPMH_VRM(rf_clk2, _d, "rfclkd2", 1);
>>>>>    DEFINE_CLK_RPMH_VRM(rf_clk3, _d, "rfclkd3", 1);
>>>>>    DEFINE_CLK_RPMH_VRM(rf_clk4, _d, "rfclkd4", 1);
>>>>> +DEFINE_CLK_RPMH_VRM(clk1, _a1, "clka1", 1);
>>>>> +DEFINE_CLK_RPMH_VRM(clk2, _a1, "clka2", 1);
>>>>> +DEFINE_CLK_RPMH_VRM(clk3, _a1, "clka3", 1);
>>>>> +DEFINE_CLK_RPMH_VRM(clk4, _a1, "clka4", 1);
>>>>> +DEFINE_CLK_RPMH_VRM(clk5, _a1, "clka5", 1);
>>>>> +
>>>>> +DEFINE_CLK_RPMH_VRM(clk6, _a2, "clka6", 2);
>>>>> +DEFINE_CLK_RPMH_VRM(clk7, _a2, "clka7", 2);
>>>>> +DEFINE_CLK_RPMH_VRM(clk8, _a2, "clka8", 2);
>>>>> +
>>>>>    DEFINE_CLK_RPMH_VRM(div_clk1, _div2, "divclka1", 2);
>>>>>    DEFINE_CLK_RPMH_BCM(ce, "CE0");
>>>>> @@ -576,6 +616,33 @@ static const struct clk_rpmh_desc clk_rpmh_sm8450 = {
>>>>>    	.num_clks = ARRAY_SIZE(sm8450_rpmh_clocks),
>>>>>    };
>>>>> +static struct clk_hw *sm8550_rpmh_clocks[] = {
>>>>> +	[RPMH_CXO_PAD_CLK]      = &clk_rpmh_bi_tcxo_div2.hw,
>>>>> +	[RPMH_CXO_PAD_CLK_A]    = &clk_rpmh_bi_tcxo_div2_ao.hw,
>>>>> +	[RPMH_CXO_CLK]		= &clk_fixed_factor_bi_tcxo_div2.hw,
>>>>> +	[RPMH_CXO_CLK_A]	= &clk_fixed_factor_bi_tcxo_div2_ao.hw,
>>>>> +	[RPMH_LN_BB_CLK1]	= &clk_rpmh_clk6_a2.hw,
>>>>> +	[RPMH_LN_BB_CLK1_A]	= &clk_rpmh_clk6_a2_ao.hw,
>>>>> +	[RPMH_LN_BB_CLK2]	= &clk_rpmh_clk7_a2.hw,
>>>>> +	[RPMH_LN_BB_CLK2_A]	= &clk_rpmh_clk7_a2_ao.hw,
>>>>> +	[RPMH_LN_BB_CLK3]	= &clk_rpmh_clk8_a2.hw,
>>>>> +	[RPMH_LN_BB_CLK3_A]	= &clk_rpmh_clk8_a2_ao.hw,
>>>>> +	[RPMH_RF_CLK1]		= &clk_rpmh_clk1_a1.hw,
>>>>> +	[RPMH_RF_CLK1_A]	= &clk_rpmh_clk1_a1_ao.hw,
>>>>> +	[RPMH_RF_CLK2]		= &clk_rpmh_clk2_a1.hw,
>>>>> +	[RPMH_RF_CLK2_A]	= &clk_rpmh_clk2_a1_ao.hw,
>>>>> +	[RPMH_RF_CLK3]		= &clk_rpmh_clk3_a1.hw,
>>>>> +	[RPMH_RF_CLK3_A]	= &clk_rpmh_clk3_a1_ao.hw,
>>>>> +	[RPMH_RF_CLK4]		= &clk_rpmh_clk4_a1.hw,
>>>>> +	[RPMH_RF_CLK4_A]	= &clk_rpmh_clk4_a1_ao.hw,
>>>>> +	[RPMH_IPA_CLK]		= &clk_rpmh_ipa.hw,
>>>>> +};
>>>>> +
>>>>> +static const struct clk_rpmh_desc clk_rpmh_sm8550 = {
>>>>> +	.clks = sm8550_rpmh_clocks,
>>>>> +	.num_clks = ARRAY_SIZE(sm8550_rpmh_clocks),
>>>>> +};
>>>>> +
>>>>>    static struct clk_hw *sc7280_rpmh_clocks[] = {
>>>>>    	[RPMH_CXO_CLK]      = &clk_rpmh_bi_tcxo_div4.hw,
>>>>>    	[RPMH_CXO_CLK_A]    = &clk_rpmh_bi_tcxo_div4_ao.hw,
>>>>> @@ -683,29 +750,31 @@ static int clk_rpmh_probe(struct platform_device *pdev)
>>>>>    		name = hw_clks[i]->init->name;
>>>>> -		rpmh_clk = to_clk_rpmh(hw_clks[i]);
>>>>> -		res_addr = cmd_db_read_addr(rpmh_clk->res_name);
>>>>> -		if (!res_addr) {
>>>>> -			dev_err(&pdev->dev, "missing RPMh resource address for %s\n",
>>>>> -				rpmh_clk->res_name);
>>>>> -			return -ENODEV;
>>>>> -		}
>>>>> +		if (hw_clks[i]->init->ops != &clk_fixed_factor_ops) {
>>>>
>>>> We discussed this separately, the fixed factor clocks will be moved to the
>>>> child nodes, leaving rpmhcc with only cmd-db based clocks.
>>>>
>>>
>>> Are you saying that you will represent bi_tcxo as a fixed-factor-clock
>>> under /clocks with RPMH_CXO_PAD_CLK as parent and a clock-div = <2>; ?
>>
>> Yes, this was the idea. The rpmhcc driver is pretty much centric around 
>> the cmd-db clocks. Adding a fixed-factor clock results either in a 
>> horrible hacks or in a significant code refactoring. However we already 
>> have an existing way to fixed-factor clocks: DT nodes. Adding support 
>> for such nodes to rpmhcc driver requires just a single additional API 
>> call: devm_of_platform_populate().
> 
> Please no. DT is not to solve driver issues, skip some code or make
> things simpler for driver developers. If everyone - U-boot, *BSD,
> firmwares - pushes to DT stuff like this, because this makes their
> driver development easier, you would have total mess. Linux does not
> have any priorities here in this approach.

Assuming we talk about Abel's implementation of putting these nodes in
rpmhcc, because you wrote here devm_of_platform_populate()...

Best regards,
Krzysztof

Re: [PATCH v6 4/5] clk: qcom: rpmh: Add support for SM8550 rpmh clocks

[Bjorn Andersson]

On Fri, Jan 06, 2023 at 08:45:42AM +0100, Krzysztof Kozlowski wrote:
> On 28/12/2022 19:59, Dmitry Baryshkov wrote:
> > On 28/12/2022 20:52, Bjorn Andersson wrote:
> >> On Wed, Dec 14, 2022 at 06:25:01PM +0200, Dmitry Baryshkov wrote:
> >>> On 07/12/2022 00:45, Abel Vesa wrote:
> >>>> Adds the RPMH clocks present in SM8550 SoC.
> >>>>
> >>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> >>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> >>>> ---
> >>>>    drivers/clk/qcom/clk-rpmh.c | 110 +++++++++++++++++++++++++++++-------
> >>>>    1 file changed, 90 insertions(+), 20 deletions(-)
> >>>>
> >>>> diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c
> >>>> index 2c2ef4b6d130..ce81c76ed0fd 100644
> >>>> --- a/drivers/clk/qcom/clk-rpmh.c
> >>>> +++ b/drivers/clk/qcom/clk-rpmh.c
> >>>> @@ -130,6 +130,34 @@ static DEFINE_MUTEX(rpmh_clk_lock);
> >>>>    		},							\
> >>>>    	}
> >>>> +#define DEFINE_CLK_FIXED_FACTOR(_name, _parent_name, _div)		\
> >>>> +	static struct clk_fixed_factor clk_fixed_factor##_##_name = {	\
> >>>> +		.mult = 1,						\
> >>>> +		.div = _div,						\
> >>>> +		.hw.init = &(struct clk_init_data){			\
> >>>> +			.ops = &clk_fixed_factor_ops,			\
> >>>> +			.name = #_name,					\
> >>>> +			.parent_data =  &(const struct clk_parent_data){ \
> >>>> +				.fw_name = #_parent_name,		\
> >>>> +				.name = #_parent_name,			\
> >>>> +			},						\
> >>>> +			.num_parents = 1,				\
> >>>> +		},							\
> >>>> +	};								\
> >>>> +	static struct clk_fixed_factor clk_fixed_factor##_##_name##_ao = { \
> >>>> +		.mult = 1,						\
> >>>> +		.div = _div,						\
> >>>> +		.hw.init = &(struct clk_init_data){			\
> >>>> +			.ops = &clk_fixed_factor_ops,			\
> >>>> +			.name = #_name "_ao",				\
> >>>> +			.parent_data =  &(const struct clk_parent_data){ \
> >>>> +				.fw_name = #_parent_name "_ao",		\
> >>>> +				.name = #_parent_name "_ao",		\
> >>>> +			},						\
> >>>> +			.num_parents = 1,				\
> >>>> +		},							\
> >>>> +	}
> >>>> +
> >>>>    static inline struct clk_rpmh *to_clk_rpmh(struct clk_hw *_hw)
> >>>>    {
> >>>>    	return container_of(_hw, struct clk_rpmh, hw);
> >>>> @@ -345,6 +373,8 @@ DEFINE_CLK_RPMH_ARC(bi_tcxo, "xo.lvl", 0x3, 2);
> >>>>    DEFINE_CLK_RPMH_ARC(bi_tcxo, "xo.lvl", 0x3, 4);
> >>>>    DEFINE_CLK_RPMH_ARC(qlink, "qphy.lvl", 0x1, 4);
> >>>> +DEFINE_CLK_FIXED_FACTOR(bi_tcxo_div2, bi_tcxo, 2);
> >>>> +
> >>>>    DEFINE_CLK_RPMH_VRM(ln_bb_clk1, _a2, "lnbclka1", 2);
> >>>>    DEFINE_CLK_RPMH_VRM(ln_bb_clk2, _a2, "lnbclka2", 2);
> >>>>    DEFINE_CLK_RPMH_VRM(ln_bb_clk3, _a2, "lnbclka3", 2);
> >>>> @@ -366,6 +396,16 @@ DEFINE_CLK_RPMH_VRM(rf_clk2, _d, "rfclkd2", 1);
> >>>>    DEFINE_CLK_RPMH_VRM(rf_clk3, _d, "rfclkd3", 1);
> >>>>    DEFINE_CLK_RPMH_VRM(rf_clk4, _d, "rfclkd4", 1);
> >>>> +DEFINE_CLK_RPMH_VRM(clk1, _a1, "clka1", 1);
> >>>> +DEFINE_CLK_RPMH_VRM(clk2, _a1, "clka2", 1);
> >>>> +DEFINE_CLK_RPMH_VRM(clk3, _a1, "clka3", 1);
> >>>> +DEFINE_CLK_RPMH_VRM(clk4, _a1, "clka4", 1);
> >>>> +DEFINE_CLK_RPMH_VRM(clk5, _a1, "clka5", 1);
> >>>> +
> >>>> +DEFINE_CLK_RPMH_VRM(clk6, _a2, "clka6", 2);
> >>>> +DEFINE_CLK_RPMH_VRM(clk7, _a2, "clka7", 2);
> >>>> +DEFINE_CLK_RPMH_VRM(clk8, _a2, "clka8", 2);
> >>>> +
> >>>>    DEFINE_CLK_RPMH_VRM(div_clk1, _div2, "divclka1", 2);
> >>>>    DEFINE_CLK_RPMH_BCM(ce, "CE0");
> >>>> @@ -576,6 +616,33 @@ static const struct clk_rpmh_desc clk_rpmh_sm8450 = {
> >>>>    	.num_clks = ARRAY_SIZE(sm8450_rpmh_clocks),
> >>>>    };
> >>>> +static struct clk_hw *sm8550_rpmh_clocks[] = {
> >>>> +	[RPMH_CXO_PAD_CLK]      = &clk_rpmh_bi_tcxo_div2.hw,
> >>>> +	[RPMH_CXO_PAD_CLK_A]    = &clk_rpmh_bi_tcxo_div2_ao.hw,
> >>>> +	[RPMH_CXO_CLK]		= &clk_fixed_factor_bi_tcxo_div2.hw,
> >>>> +	[RPMH_CXO_CLK_A]	= &clk_fixed_factor_bi_tcxo_div2_ao.hw,
> >>>> +	[RPMH_LN_BB_CLK1]	= &clk_rpmh_clk6_a2.hw,
> >>>> +	[RPMH_LN_BB_CLK1_A]	= &clk_rpmh_clk6_a2_ao.hw,
> >>>> +	[RPMH_LN_BB_CLK2]	= &clk_rpmh_clk7_a2.hw,
> >>>> +	[RPMH_LN_BB_CLK2_A]	= &clk_rpmh_clk7_a2_ao.hw,
> >>>> +	[RPMH_LN_BB_CLK3]	= &clk_rpmh_clk8_a2.hw,
> >>>> +	[RPMH_LN_BB_CLK3_A]	= &clk_rpmh_clk8_a2_ao.hw,
> >>>> +	[RPMH_RF_CLK1]		= &clk_rpmh_clk1_a1.hw,
> >>>> +	[RPMH_RF_CLK1_A]	= &clk_rpmh_clk1_a1_ao.hw,
> >>>> +	[RPMH_RF_CLK2]		= &clk_rpmh_clk2_a1.hw,
> >>>> +	[RPMH_RF_CLK2_A]	= &clk_rpmh_clk2_a1_ao.hw,
> >>>> +	[RPMH_RF_CLK3]		= &clk_rpmh_clk3_a1.hw,
> >>>> +	[RPMH_RF_CLK3_A]	= &clk_rpmh_clk3_a1_ao.hw,
> >>>> +	[RPMH_RF_CLK4]		= &clk_rpmh_clk4_a1.hw,
> >>>> +	[RPMH_RF_CLK4_A]	= &clk_rpmh_clk4_a1_ao.hw,
> >>>> +	[RPMH_IPA_CLK]		= &clk_rpmh_ipa.hw,
> >>>> +};
> >>>> +
> >>>> +static const struct clk_rpmh_desc clk_rpmh_sm8550 = {
> >>>> +	.clks = sm8550_rpmh_clocks,
> >>>> +	.num_clks = ARRAY_SIZE(sm8550_rpmh_clocks),
> >>>> +};
> >>>> +
> >>>>    static struct clk_hw *sc7280_rpmh_clocks[] = {
> >>>>    	[RPMH_CXO_CLK]      = &clk_rpmh_bi_tcxo_div4.hw,
> >>>>    	[RPMH_CXO_CLK_A]    = &clk_rpmh_bi_tcxo_div4_ao.hw,
> >>>> @@ -683,29 +750,31 @@ static int clk_rpmh_probe(struct platform_device *pdev)
> >>>>    		name = hw_clks[i]->init->name;
> >>>> -		rpmh_clk = to_clk_rpmh(hw_clks[i]);
> >>>> -		res_addr = cmd_db_read_addr(rpmh_clk->res_name);
> >>>> -		if (!res_addr) {
> >>>> -			dev_err(&pdev->dev, "missing RPMh resource address for %s\n",
> >>>> -				rpmh_clk->res_name);
> >>>> -			return -ENODEV;
> >>>> -		}
> >>>> +		if (hw_clks[i]->init->ops != &clk_fixed_factor_ops) {
> >>>
> >>> We discussed this separately, the fixed factor clocks will be moved to the
> >>> child nodes, leaving rpmhcc with only cmd-db based clocks.
> >>>
> >>
> >> Are you saying that you will represent bi_tcxo as a fixed-factor-clock
> >> under /clocks with RPMH_CXO_PAD_CLK as parent and a clock-div = <2>; ?
> > 
> > Yes, this was the idea. The rpmhcc driver is pretty much centric around 
> > the cmd-db clocks. Adding a fixed-factor clock results either in a 
> > horrible hacks or in a significant code refactoring. However we already 
> > have an existing way to fixed-factor clocks: DT nodes. Adding support 
> > for such nodes to rpmhcc driver requires just a single additional API 
> > call: devm_of_platform_populate().
> 
> Please no. DT is not to solve driver issues, skip some code or make
> things simpler for driver developers. If everyone - U-boot, *BSD,
> firmwares - pushes to DT stuff like this, because this makes their
> driver development easier, you would have total mess. Linux does not
> have any priorities here in this approach.

This is not solving the driver issue, rather the opposite.

Moving it out of the driver accurately represents that there's an
additional divider between the clock that is controlled by this driver
and the next clock provider(s).

Regards,
Bjorn

[PATCH v6 5/5] clk: qcom: Add TCSR clock driver for SM8550

[Abel Vesa]

The TCSR clock controller found on SM8550 provides refclks
for PCIE, USB and UFS. Add clock driver for it.

This patch is based on initial code downstream.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/Kconfig         |   7 ++
 drivers/clk/qcom/Makefile        |   1 +
 drivers/clk/qcom/tcsrcc-sm8550.c | 192 +++++++++++++++++++++++++++++++
 3 files changed, 200 insertions(+)
 create mode 100644 drivers/clk/qcom/tcsrcc-sm8550.c

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 70d43f0a8919..b9f5505d68f0 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -797,6 +797,13 @@ config SM_GPUCC_8350
 	  Say Y if you want to support graphics controller devices and
 	  functionality such as 3D graphics.
 
+config SM_TCSRCC_8550
+	tristate "SM8550 TCSR Clock Controller"
+	select QCOM_GDSC
+	help
+	  Support for the TCSR clock controller on SM8550 devices.
+	  Say Y if you want to use peripheral devices such as SD/UFS.
+
 config SM_VIDEOCC_8150
 	tristate "SM8150 Video Clock Controller"
 	select SM_GCC_8150
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index f18c446a97ea..f5ce429c724c 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -112,6 +112,7 @@ obj-$(CONFIG_SM_GPUCC_6350) += gpucc-sm6350.o
 obj-$(CONFIG_SM_GPUCC_8150) += gpucc-sm8150.o
 obj-$(CONFIG_SM_GPUCC_8250) += gpucc-sm8250.o
 obj-$(CONFIG_SM_GPUCC_8350) += gpucc-sm8350.o
+obj-$(CONFIG_SM_TCSRCC_8550) += tcsrcc-sm8550.o
 obj-$(CONFIG_SM_VIDEOCC_8150) += videocc-sm8150.o
 obj-$(CONFIG_SM_VIDEOCC_8250) += videocc-sm8250.o
 obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
diff --git a/drivers/clk/qcom/tcsrcc-sm8550.c b/drivers/clk/qcom/tcsrcc-sm8550.c
new file mode 100644
index 000000000000..2c67ee71c196
--- /dev/null
+++ b/drivers/clk/qcom/tcsrcc-sm8550.c
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2021, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2022, Linaro Limited
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/clock/qcom,sm8550-tcsr.h>
+
+#include "clk-alpha-pll.h"
+#include "clk-branch.h"
+#include "clk-pll.h"
+#include "clk-rcg.h"
+#include "clk-regmap.h"
+#include "clk-regmap-divider.h"
+#include "clk-regmap-mux.h"
+#include "common.h"
+#include "reset.h"
+
+enum {
+	DT_BI_TCXO_PAD,
+};
+
+static struct clk_branch tcsr_pcie_0_clkref_en = {
+	.halt_reg = 0x15100,
+	.halt_check = BRANCH_HALT_SKIP,
+	.clkr = {
+		.enable_reg = 0x15100,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "tcsr_pcie_0_clkref_en",
+			.parent_data = &(const struct clk_parent_data){
+				.index = DT_BI_TCXO_PAD,
+			},
+			.num_parents = 1,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch tcsr_pcie_1_clkref_en = {
+	.halt_reg = 0x15114,
+	.halt_check = BRANCH_HALT_SKIP,
+	.clkr = {
+		.enable_reg = 0x15114,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "tcsr_pcie_1_clkref_en",
+			.parent_data = &(const struct clk_parent_data){
+				.index = DT_BI_TCXO_PAD,
+			},
+			.num_parents = 1,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch tcsr_ufs_clkref_en = {
+	.halt_reg = 0x15110,
+	.halt_check = BRANCH_HALT_SKIP,
+	.clkr = {
+		.enable_reg = 0x15110,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "tcsr_ufs_clkref_en",
+			.parent_data = &(const struct clk_parent_data){
+				.index = DT_BI_TCXO_PAD,
+			},
+			.num_parents = 1,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch tcsr_ufs_pad_clkref_en = {
+	.halt_reg = 0x15104,
+	.halt_check = BRANCH_HALT_SKIP,
+	.clkr = {
+		.enable_reg = 0x15104,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "tcsr_ufs_pad_clkref_en",
+			.parent_data = &(const struct clk_parent_data){
+				.index = DT_BI_TCXO_PAD,
+			},
+			.num_parents = 1,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch tcsr_usb2_clkref_en = {
+	.halt_reg = 0x15118,
+	.halt_check = BRANCH_HALT_SKIP,
+	.clkr = {
+		.enable_reg = 0x15118,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "tcsr_usb2_clkref_en",
+			.parent_data = &(const struct clk_parent_data){
+				.index = DT_BI_TCXO_PAD,
+			},
+			.num_parents = 1,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch tcsr_usb3_clkref_en = {
+	.halt_reg = 0x15108,
+	.halt_check = BRANCH_HALT_SKIP,
+	.clkr = {
+		.enable_reg = 0x15108,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "tcsr_usb3_clkref_en",
+			.parent_data = &(const struct clk_parent_data){
+				.index = DT_BI_TCXO_PAD,
+			},
+			.num_parents = 1,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_regmap *tcsr_cc_sm8550_clocks[] = {
+	[TCSR_PCIE_0_CLKREF_EN] = &tcsr_pcie_0_clkref_en.clkr,
+	[TCSR_PCIE_1_CLKREF_EN] = &tcsr_pcie_1_clkref_en.clkr,
+	[TCSR_UFS_CLKREF_EN] = &tcsr_ufs_clkref_en.clkr,
+	[TCSR_UFS_PAD_CLKREF_EN] = &tcsr_ufs_pad_clkref_en.clkr,
+	[TCSR_USB2_CLKREF_EN] = &tcsr_usb2_clkref_en.clkr,
+	[TCSR_USB3_CLKREF_EN] = &tcsr_usb3_clkref_en.clkr,
+};
+
+static const struct regmap_config tcsr_cc_sm8550_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = 0x2f000,
+	.fast_io = true,
+};
+
+static const struct qcom_cc_desc tcsr_cc_sm8550_desc = {
+	.config = &tcsr_cc_sm8550_regmap_config,
+	.clks = tcsr_cc_sm8550_clocks,
+	.num_clks = ARRAY_SIZE(tcsr_cc_sm8550_clocks),
+};
+
+static const struct of_device_id tcsr_cc_sm8550_match_table[] = {
+	{ .compatible = "qcom,sm8550-tcsr" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tcsr_cc_sm8550_match_table);
+
+static int tcsr_cc_sm8550_probe(struct platform_device *pdev)
+{
+	struct regmap *regmap;
+
+	regmap = qcom_cc_map(pdev, &tcsr_cc_sm8550_desc);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	return qcom_cc_really_probe(pdev, &tcsr_cc_sm8550_desc, regmap);
+}
+
+static struct platform_driver tcsr_cc_sm8550_driver = {
+	.probe = tcsr_cc_sm8550_probe,
+	.driver = {
+		.name = "tcsr_cc-sm8550",
+		.of_match_table = tcsr_cc_sm8550_match_table,
+	},
+};
+
+static int __init tcsr_cc_sm8550_init(void)
+{
+	return platform_driver_register(&tcsr_cc_sm8550_driver);
+}
+subsys_initcall(tcsr_cc_sm8550_init);
+
+static void __exit tcsr_cc_sm8550_exit(void)
+{
+	platform_driver_unregister(&tcsr_cc_sm8550_driver);
+}
+module_exit(tcsr_cc_sm8550_exit);
+
+MODULE_DESCRIPTION("QTI TCSRCC SM8550 Driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1