[PATCH v4 00/10] Enable CPR for IPQ9574

[PATCH v4 00/10] Enable CPR for IPQ9574

[Varadarajan Narayanan]

This series tries to enable CPR on IPQ9574, that implements
CPRv4. Since [1] is older, faced few minor issues. Those are
addressed in [2].

dt_binding_check and dtbs_check passed.

Depends:
	[1] https://lore.kernel.org/lkml/20230217-topic-cpr3h-v14-0-9fd23241493d@linaro.org/T/
	[2] https://github.com/quic-varada/cpr/tree/4de50be55a89eb29ab0d40d3fcfe9aa7a9ccf910

v4: s/cprh/cpr4/
    Create new match data for ipq9574 that includes genpd_names
    Update cloop-vadj & oloop-vadj with minItems

v3: Fix patch authorship for 2 patches
    Include CPR3 file changes done to Konrad's patches in https://github.com/quic-varada/cpr/commits/konrad/
    Change url for [2] to skip the cpr3 file changes

v2: Fix Signed-off-by order in 2 patches
    Update constraints in qcom,cpr3.yaml
    Add rbcpr_clk_src registration
    Add Reviewed-by to one of the patches
    Not adding Acked-by as the file has changed

Praveenkumar I (2):
  pmdomain: qcom: rpmpd: Add IPQ9574 power domains
  soc: qcom: cpr3: Add IPQ9574 definitions

Varadarajan Narayanan (8):
  soc: qcom: cpr3: Fix 'acc_desc' usage
  cpufreq: qcom-nvmem: Add support for IPQ9574
  dt-bindings: power: rpmpd: Add IPQ9574 power domains
  dt-bindings: soc: qcom: cpr3: Add bindings for IPQ9574
  dt-bindings: clock: Add CPR clock defines for IPQ9574
  clk: qcom: gcc-ipq9574: Add CPR clock definition
  dt-bindings: opp: v2-qcom-level: Update minItems for oloop-vadj &
    cloop-vadj
  dts: arm64: qcom: ipq9574: Enable CPR

 .../bindings/opp/opp-v2-qcom-level.yaml       |   2 +
 .../devicetree/bindings/power/qcom,rpmpd.yaml |   1 +
 .../bindings/soc/qcom/qcom,cpr3.yaml          |  35 +++
 arch/arm64/boot/dts/qcom/ipq9574.dtsi         | 269 ++++++++++++++++--
 drivers/clk/qcom/gcc-ipq9574.c                |  39 +++
 drivers/cpufreq/qcom-cpufreq-nvmem.c          |   7 +-
 drivers/pmdomain/qcom/cpr3.c                  | 147 +++++++++-
 drivers/pmdomain/qcom/rpmpd.c                 |  19 ++
 include/dt-bindings/clock/qcom,ipq9574-gcc.h  |   2 +
 include/dt-bindings/power/qcom-rpmpd.h        |   3 +
 10 files changed, 502 insertions(+), 22 deletions(-)

-- 
2.34.1

[PATCH v4 01/10] soc: qcom: cpr3: Fix 'acc_desc' usage

[Varadarajan Narayanan]

cpr3 code assumes that 'acc_desc' is available for SoCs
implementing CPR version 4 or less. However, IPQ9574 SoC
implements CPRv4 without ACC. This causes NULL pointer accesses
resulting in crashes. Hence, check if 'acc_desc' is populated
before using it.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v4: Undo the acc_desc validation in probe function as that could
    affect other SoC.
---
 drivers/pmdomain/qcom/cpr3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pmdomain/qcom/cpr3.c b/drivers/pmdomain/qcom/cpr3.c
index c7790a71e74f..6ceb7605f84d 100644
--- a/drivers/pmdomain/qcom/cpr3.c
+++ b/drivers/pmdomain/qcom/cpr3.c
@@ -2399,12 +2399,12 @@ static int cpr_pd_attach_dev(struct generic_pm_domain *domain,
 		if (ret)
 			goto exit;
 
-		if (acc_desc->config)
+		if (acc_desc && acc_desc->config)
 			regmap_multi_reg_write(drv->tcsr, acc_desc->config,
 					       acc_desc->num_regs_per_fuse);
 
 		/* Enable ACC if required */
-		if (acc_desc->enable_mask)
+		if (acc_desc && acc_desc->enable_mask)
 			regmap_update_bits(drv->tcsr, acc_desc->enable_reg,
 					   acc_desc->enable_mask,
 					   acc_desc->enable_mask);
-- 
2.34.1

[PATCH v4 02/10] cpufreq: qcom-nvmem: Add support for IPQ9574

[Varadarajan Narayanan]

Add qcom_cpufreq_match_data for IPQ9574. This is used for tying
up the cpu@N nodes with the power domains. match_data_kryo is not
used since it doesn't set genpd_names. If genpd_names is not set,
'cat /sys/kernel/debug/qcom_cpr3/thread0' causes cpr3_debug_info_show()
to crash while trying to read thread->corner->last_uV as
thread->corner is NULL.

	Call trace:
		cpr3_debug_info_show
		seq_read_iter
		seq_read

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v4: Update commit log to include stack trace
    Introduce qcom_cpufreq_match_data for IPQ9574
---
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
index 939702dfa73f..95558586c2e6 100644
--- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
+++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
@@ -428,6 +428,11 @@ static const struct qcom_cpufreq_match_data match_data_ipq8074 = {
 	.get_version = qcom_cpufreq_ipq8074_name_version,
 };
 
+static const struct qcom_cpufreq_match_data match_data_ipq9574 = {
+	.get_version = qcom_cpufreq_kryo_name_version,
+	.genpd_names = generic_genpd_names,
+};
+
 static void qcom_cpufreq_suspend_virt_devs(struct qcom_cpufreq_drv *drv, unsigned int cpu)
 {
 	const char * const *name = drv->data->genpd_names;
@@ -621,7 +626,7 @@ static const struct of_device_id qcom_cpufreq_match_list[] __initconst = {
 	{ .compatible = "qcom,ipq8064", .data = &match_data_ipq8064 },
 	{ .compatible = "qcom,ipq8074", .data = &match_data_ipq8074 },
 	{ .compatible = "qcom,apq8064", .data = &match_data_krait },
-	{ .compatible = "qcom,ipq9574", .data = &match_data_kryo },
+	{ .compatible = "qcom,ipq9574", .data = &match_data_ipq9574 },
 	{ .compatible = "qcom,msm8974", .data = &match_data_krait },
 	{ .compatible = "qcom,msm8960", .data = &match_data_krait },
 	{},
-- 
2.34.1

[PATCH v4 03/10] dt-bindings: power: rpmpd: Add IPQ9574 power domains

[Varadarajan Narayanan]

Add the compatibles and indexes for the rpmpd in IPQ9574.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v3 & v4: no changes
v2: Add Reviewed-by
---
 Documentation/devicetree/bindings/power/qcom,rpmpd.yaml | 1 +
 include/dt-bindings/power/qcom-rpmpd.h                  | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml b/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml
index 929b7ef9c1bc..e20ba25fa094 100644
--- a/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml
+++ b/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml
@@ -17,6 +17,7 @@ properties:
   compatible:
     oneOf:
       - enum:
+          - qcom,ipq9574-rpmpd
           - qcom,mdm9607-rpmpd
           - qcom,msm8226-rpmpd
           - qcom,msm8909-rpmpd
diff --git a/include/dt-bindings/power/qcom-rpmpd.h b/include/dt-bindings/power/qcom-rpmpd.h
index 608087fb9a3d..0538370bfbb4 100644
--- a/include/dt-bindings/power/qcom-rpmpd.h
+++ b/include/dt-bindings/power/qcom-rpmpd.h
@@ -402,6 +402,9 @@
 #define QCM2290_VDD_LPI_CX	6
 #define QCM2290_VDD_LPI_MX	7
 
+/* IPQ9574 Power Domains */
+#define IPQ9574_VDDAPC		0
+
 /* RPM SMD Power Domain performance levels */
 #define RPM_SMD_LEVEL_RETENTION       16
 #define RPM_SMD_LEVEL_RETENTION_PLUS  32
-- 
2.34.1

[PATCH v4 04/10] dt-bindings: soc: qcom: cpr3: Add bindings for IPQ9574

[Varadarajan Narayanan]

Add the bindings for the IPQ9574 CPR3 driver to the documentation.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v4: Change compatible string to cpr4 instead of cprh
    Not adding Reviewed-By as compatible string changed

v2: Constrained nvmem-cells and the other variant.
    Removed unnecessary blank line.
---
 .../bindings/soc/qcom/qcom,cpr3.yaml          | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,cpr3.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,cpr3.yaml
index acf2e294866b..b875a7633f31 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,cpr3.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,cpr3.yaml
@@ -24,6 +24,7 @@ properties:
       - const: qcom,cpr4
       - items:
           - enum:
+              - qcom,ipq9574-cpr4
               - qcom,msm8998-cprh
               - qcom,sdm630-cprh
           - const: qcom,cprh
@@ -52,9 +53,11 @@ properties:
 
   nvmem-cells:
     description: Cells containing the fuse corners and revision data
+    minItems: 17
     maxItems: 32
 
   nvmem-cell-names:
+    minItems: 17
     maxItems: 32
 
   operating-points-v2: true
@@ -74,6 +77,36 @@ required:
 additionalProperties: false
 
 allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,ipq9574-cpr4
+    then:
+      properties:
+        nvmem-cells:
+          maxItems: 17
+        nvmem-cell-names:
+          items:
+            - const: cpr_speed_bin
+            - const: cpr_fuse_revision
+            - const: cpr0_quotient1
+            - const: cpr0_quotient2
+            - const: cpr0_quotient3
+            - const: cpr0_quotient4
+            - const: cpr0_quotient_offset2
+            - const: cpr0_quotient_offset3
+            - const: cpr0_quotient_offset4
+            - const: cpr0_init_voltage1
+            - const: cpr0_init_voltage2
+            - const: cpr0_init_voltage3
+            - const: cpr0_init_voltage4
+            - const: cpr0_ring_osc1
+            - const: cpr0_ring_osc2
+            - const: cpr0_ring_osc3
+            - const: cpr0_ring_osc4
+
   - if:
       properties:
         compatible:
@@ -82,6 +115,8 @@ allOf:
               - qcom,msm8998-cprh
     then:
       properties:
+        nvmem-cells:
+          minItems: 32
         nvmem-cell-names:
           items:
             - const: cpr_speed_bin
-- 
2.34.1

[PATCH v4 05/10] pmdomain: qcom: rpmpd: Add IPQ9574 power domains

[Varadarajan Narayanan]

From: Praveenkumar I <quic_ipkumar@quicinc.com>

Add the APC power domain definitions used in IPQ9574.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v4: Add Reviewed-by: Dmitry Baryshkov
v3: Fix patch author
v2: Fix Signed-off-by order
---
 drivers/pmdomain/qcom/rpmpd.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/pmdomain/qcom/rpmpd.c b/drivers/pmdomain/qcom/rpmpd.c
index 5e6280b4cf70..947d6a9c3897 100644
--- a/drivers/pmdomain/qcom/rpmpd.c
+++ b/drivers/pmdomain/qcom/rpmpd.c
@@ -38,6 +38,7 @@ static struct qcom_smd_rpm *rpmpd_smd_rpm;
 #define KEY_FLOOR_CORNER	0x636676   /* vfc */
 #define KEY_FLOOR_LEVEL		0x6c6676   /* vfl */
 #define KEY_LEVEL		0x6c766c76 /* vlvl */
+#define RPM_KEY_UV		0x00007675 /* "uv" */
 
 #define MAX_CORNER_RPMPD_STATE	6
 
@@ -644,6 +645,23 @@ static const struct rpmpd_desc mdm9607_desc = {
 	.max_state = RPM_SMD_LEVEL_TURBO,
 };
 
+static struct rpmpd apc_s1_lvl = {
+	.pd = { .name = "apc", },
+	.res_type = RPMPD_SMPA,
+	.res_id = 1,
+	.key = RPM_KEY_UV,
+};
+
+static struct rpmpd *ipq9574_rpmpds[] = {
+	[IPQ9574_VDDAPC] =	&apc_s1_lvl,
+};
+
+static const struct rpmpd_desc ipq9574_desc = {
+	.rpmpds = ipq9574_rpmpds,
+	.num_pds = ARRAY_SIZE(ipq9574_rpmpds),
+	.max_state = RPM_SMD_LEVEL_TURBO_HIGH,
+};
+
 static struct rpmpd *msm8226_rpmpds[] = {
 	[MSM8226_VDDCX] =	&cx_s1a_corner,
 	[MSM8226_VDDCX_AO] =	&cx_s1a_corner_ao,
@@ -931,6 +949,7 @@ static const struct rpmpd_desc qcm2290_desc = {
 };
 
 static const struct of_device_id rpmpd_match_table[] = {
+	{ .compatible = "qcom,ipq9574-rpmpd", .data = &ipq9574_desc },
 	{ .compatible = "qcom,mdm9607-rpmpd", .data = &mdm9607_desc },
 	{ .compatible = "qcom,msm8226-rpmpd", .data = &msm8226_desc },
 	{ .compatible = "qcom,msm8909-rpmpd", .data = &msm8916_desc },
-- 
2.34.1

[PATCH v4 06/10] dt-bindings: clock: Add CPR clock defines for IPQ9574

[Varadarajan Narayanan]

Add defines for the CPR block present in IPQ9574.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v4: No change
v3: Add Acked-By
v2: Add GCC_RBCPR_CLK_SRC define.
    Not adding 'Acked-by: Krzysztof Kozlowski' as the file changed.
---
 include/dt-bindings/clock/qcom,ipq9574-gcc.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/dt-bindings/clock/qcom,ipq9574-gcc.h b/include/dt-bindings/clock/qcom,ipq9574-gcc.h
index 52123c5a09fa..4c65de04cb7b 100644
--- a/include/dt-bindings/clock/qcom,ipq9574-gcc.h
+++ b/include/dt-bindings/clock/qcom,ipq9574-gcc.h
@@ -220,4 +220,6 @@
 #define GCC_PCIE1_PIPE_CLK				211
 #define GCC_PCIE2_PIPE_CLK				212
 #define GCC_PCIE3_PIPE_CLK				213
+#define GCC_RBCPR_CLK_SRC				214
+#define GCC_RBCPR_CLK					215
 #endif
-- 
2.34.1

[PATCH v4 07/10] clk: qcom: gcc-ipq9574: Add CPR clock definition

[Varadarajan Narayanan]

Add the CPR clock definition needed for enabling access to
CPR register space.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
 drivers/clk/qcom/gcc-ipq9574.c | 39 ++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/clk/qcom/gcc-ipq9574.c b/drivers/clk/qcom/gcc-ipq9574.c
index e1dc74d04ed1..eac557937fd3 100644
--- a/drivers/clk/qcom/gcc-ipq9574.c
+++ b/drivers/clk/qcom/gcc-ipq9574.c
@@ -3994,6 +3994,43 @@ static struct clk_branch gcc_xo_div4_clk = {
 	},
 };
 
+static const struct freq_tbl ftbl_hmss_rbcpr_clk_src[] = {
+	F(24000000, P_XO, 1, 0, 0),
+	{ }
+};
+
+static struct clk_rcg2 rbcpr_clk_src = {
+	.cmd_rcgr = 0x48044,
+	.mnd_width = 0,
+	.hid_width = 5,
+	.parent_map = gcc_xo_map,
+	.freq_tbl = ftbl_gp1_clk_src,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "rbcpr_clk_src",
+		.parent_data = gcc_xo_gpll0_gpll4,
+		.num_parents = ARRAY_SIZE(gcc_xo_map),
+		.ops = &clk_rcg2_ops,
+	},
+};
+
+static struct clk_branch gcc_rbcpr_clk = {
+	.halt_reg = 0x48008,
+	.halt_check = BRANCH_HALT,
+	.clkr = {
+		.enable_reg = 0x48008,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "gcc_rbcpr_clk",
+			.parent_hws = (const struct clk_hw *[]) {
+				&rbcpr_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
 static struct clk_hw *gcc_ipq9574_hws[] = {
 	&gpll0_out_main_div2.hw,
 	&gcc_xo_div4_clk_src.hw,
@@ -4219,6 +4256,8 @@ static struct clk_regmap *gcc_ipq9574_clks[] = {
 	[GCC_PCIE1_PIPE_CLK] = &gcc_pcie1_pipe_clk.clkr,
 	[GCC_PCIE2_PIPE_CLK] = &gcc_pcie2_pipe_clk.clkr,
 	[GCC_PCIE3_PIPE_CLK] = &gcc_pcie3_pipe_clk.clkr,
+	[GCC_RBCPR_CLK_SRC] = &rbcpr_clk_src.clkr,
+	[GCC_RBCPR_CLK] = &gcc_rbcpr_clk.clkr,
 };
 
 static const struct qcom_reset_map gcc_ipq9574_resets[] = {
-- 
2.34.1

[PATCH v4 08/10] soc: qcom: cpr3: Add IPQ9574 definitions

[Varadarajan Narayanan]

From: Praveenkumar I <quic_ipkumar@quicinc.com>

* Add thread, scaling factor, CPR descriptor defines to enable
  CPR on IPQ9574.

* Skip 'acc' usage since IPQ9574 does not have acc

Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v4: s/silver//, s/cprh/cpr4/
    Skip 'acc' related code as IPQ9574 does not have acc

v3: Fix patch author
    Included below information in cover letter
v2: Fix Signed-off-by order
Depends:
	[1] https://lore.kernel.org/lkml/20230217-topic-cpr3h-v14-0-9fd23241493d@linaro.org/T/
	[2] https://github.com/quic-varada/cpr/commits/konrad/
---
 drivers/pmdomain/qcom/cpr3.c | 143 ++++++++++++++++++++++++++++++++++-
 1 file changed, 141 insertions(+), 2 deletions(-)

diff --git a/drivers/pmdomain/qcom/cpr3.c b/drivers/pmdomain/qcom/cpr3.c
index 6ceb7605f84d..abd890cc4d9e 100644
--- a/drivers/pmdomain/qcom/cpr3.c
+++ b/drivers/pmdomain/qcom/cpr3.c
@@ -2056,6 +2056,142 @@ static const struct cpr_acc_desc msm8998_cpr_acc_desc = {
 	.cpr_desc = &msm8998_cpr_desc,
 };
 
+static const int ipq9574_scaling_factor[][CPR3_RO_COUNT] = {
+	/* Fuse Corner 0 */
+	{
+		2383, 2112, 2250, 1502, 2269, 2055, 2046, 1949,
+		2128, 1945, 2282, 2061, 2010, 2216, 2054, 2332
+	},
+	/* Fuse Corner 1 */
+	{
+		2383, 2112, 2250, 1502, 2269, 2055, 2046, 1949,
+		2128, 1945, 2282, 2061, 2010, 2216, 2054, 2332
+	},
+	/* Fuse Corner 2 */
+	{
+		2383, 2112, 2250, 1502, 2269, 2055, 2046, 1949,
+		2128, 1945, 2282, 2061, 2010, 2216, 2054, 2332
+	},
+	/* Fuse Corner 3 */
+	{
+		2383, 2112, 2250, 1502, 2269, 2055, 2046, 1949,
+		2128, 1945, 2282, 2061, 2010, 2216, 2054, 2332
+	},
+};
+
+static const struct cpr_thread_desc ipq9574_thread = {
+	.controller_id = 0,
+	.hw_tid = 0,
+	.ro_scaling_factor = ipq9574_scaling_factor,
+	.sensor_range_start = 0,
+	.sensor_range_end = 6,
+	.init_voltage_step = 10000,
+	.init_voltage_width = 6,
+	.step_quot_init_min = 0,
+	.step_quot_init_max = 15,
+	.num_fuse_corners = 4,
+	.fuse_corner_data = (struct fuse_corner_data[]){
+		/* fuse corner 0 */
+		{
+			.ref_uV = 725000,
+			.max_uV = 725000,
+			.min_uV = 725000,
+			.range_uV = 0,
+			.volt_cloop_adjust = 0,
+			.volt_oloop_adjust = 0,
+			.max_volt_scale = 4,
+			.max_quot_scale = 10,
+			.quot_offset = 0,
+			.quot_scale = 1,
+			.quot_adjust = 0,
+			.quot_offset_scale = 5,
+			.quot_offset_adjust = 0,
+		},
+		/* fuse corner 1 */
+		{
+			.ref_uV = 862500,
+			.max_uV = 862500,
+			.min_uV = 725000,
+			.range_uV = 0,
+			.volt_cloop_adjust = 0,
+			.volt_oloop_adjust = 0,
+			.max_volt_scale = 500,
+			.max_quot_scale = 800,
+			.quot_offset = 0,
+			.quot_scale = 1,
+			.quot_adjust = 0,
+			.quot_offset_scale = 5,
+			.quot_offset_adjust = 0,
+		},
+		/* fuse corner 2 */
+		{
+			.ref_uV = 987500,
+			.max_uV = 987500,
+			.min_uV = 787500,
+			.range_uV = 0,
+			.volt_cloop_adjust = 0,
+			.volt_oloop_adjust = 0,
+			.max_volt_scale = 280,
+			.max_quot_scale = 650,
+			.quot_offset = 0,
+			.quot_scale = 1,
+			.quot_adjust = 0,
+			.quot_offset_scale = 5,
+			.quot_offset_adjust = 0,
+
+		},
+		/* fuse corner 3 */
+		{
+			.ref_uV = 1062500,
+			.max_uV = 1062500,
+			.min_uV = 850000,
+			.range_uV = 0,
+			.volt_cloop_adjust = 0,
+			.volt_oloop_adjust = 0,
+			.max_volt_scale = 430,
+			.max_quot_scale = 800,
+			.quot_offset = 0,
+			.quot_scale = 1,
+			.quot_adjust = 0,
+			.quot_offset_scale = 5,
+			.quot_offset_adjust = 0,
+		},
+	},
+};
+
+static const struct cpr_desc ipq9574_cpr_desc = {
+	.cpr_type = CTRL_TYPE_CPR4,
+	.num_threads = 1,
+	.apm_threshold = 850000,
+	.apm_crossover = 880000,
+	.apm_hysteresis = 0,
+	.cpr_base_voltage = 700000,
+	.cpr_max_voltage = 1100000,
+	.timer_delay_us = 5000,
+	.timer_cons_up = 0,
+	.timer_cons_down = 0,
+	.up_threshold = 2,
+	.down_threshold = 2,
+	.idle_clocks = 15,
+	.count_mode = CPR3_CPR_CTL_COUNT_MODE_ALL_AT_ONCE_MIN,
+	.count_repeat = 1,
+	.gcnt_us = 1,
+	.vreg_step_fixed = 12500,
+	.vreg_step_up_limit = 1,
+	.vreg_step_down_limit = 1,
+	.vdd_settle_time_us = 34,
+	.corner_settle_time_us = 6,
+	.reduce_to_corner_uV = true,
+	.hw_closed_loop_en = false,
+	.threads = (const struct cpr_thread_desc *[]) {
+		&ipq9574_thread,
+	},
+};
+
+static const struct cpr_acc_desc ipq9574_cpr_acc_desc = {
+	.cpr_desc = &ipq9574_cpr_desc,
+};
+
 static const int sdm630_gold_scaling_factor[][CPR3_RO_COUNT] = {
 	/* Same RO factors for all fuse corners */
 	{
@@ -2676,7 +2812,8 @@ static int cpr_probe(struct platform_device *pdev)
 	desc = data->cpr_desc;
 
 	/* CPRh disallows MEM-ACC access from the HLOS */
-	if (!data->acc_desc && desc->cpr_type < CTRL_TYPE_CPRH)
+	if (!data->acc_desc && desc->cpr_type < CTRL_TYPE_CPRH &&
+	    !of_device_is_compatible(dev->of_node, "qcom,ipq9574-cpr4"))
 		return -EINVAL;
 
 	drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL);
@@ -2703,7 +2840,8 @@ static int cpr_probe(struct platform_device *pdev)
 
 	mutex_init(&drv->lock);
 
-	if (desc->cpr_type < CTRL_TYPE_CPRH) {
+	if (desc->cpr_type < CTRL_TYPE_CPRH &&
+	    !of_device_is_compatible(dev->of_node, "qcom,ipq9574-cpr4")) {
 		np = of_parse_phandle(dev->of_node, "qcom,acc", 0);
 		if (!np)
 			return -ENODEV;
@@ -2828,6 +2966,7 @@ static void cpr_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id cpr3_match_table[] = {
+	{ .compatible = "qcom,ipq9574-cpr4", .data = &ipq9574_cpr_acc_desc },
 	{ .compatible = "qcom,msm8998-cprh", .data = &msm8998_cpr_acc_desc },
 	{ .compatible = "qcom,sdm630-cprh", .data = &sdm630_cpr_acc_desc },
 	{ }
-- 
2.34.1

[PATCH v4 09/10] dt-bindings: opp: v2-qcom-level: Update minItems for oloop-vadj & cloop-vadj

[Varadarajan Narayanan]

Since IPQ9574 has only one CPR thread it will specify
only one voltage adjustment value. Hence update min items
accordingly for oloop-vadj and cloop-vadj. Without
constraining min items, dt_binding_check gives errors

	opp-table-cpr4:opp-0:qcom,opp-cloop-vadj:0: [0] is too short
	opp-table-cpr4:opp-0:qcom,opp-oloop-vadj:0: [0] is too short

	Failed validating 'minItems' in schema . . .
		{'maxItems': 2, 'minItems': 2}

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v4: Fix dt_bindings_check error
---
 Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml b/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml
index b203ea01b17a..1c1a9e12d57a 100644
--- a/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml
+++ b/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml
@@ -39,6 +39,7 @@ patternProperties:
           An array of per-thread values representing the closed-loop
           voltage adjustment value associated with this OPP node.
         $ref: /schemas/types.yaml#/definitions/int32-array
+        minItems: 1
         maxItems: 2
 
       qcom,opp-oloop-vadj:
@@ -46,6 +47,7 @@ patternProperties:
           An array of per-thread values representing the open-loop
           voltage adjustment value associated with this OPP node.
         $ref: /schemas/types.yaml#/definitions/int32-array
+        minItems: 1
         maxItems: 2
 
     required:
-- 
2.34.1

[PATCH v4 10/10] dts: arm64: qcom: ipq9574: Enable CPR

[Varadarajan Narayanan]

* Add CPR, RPMPD, OPP table nodes as applicable to IPQ9574 to
  enable CPR functionality on IPQ9574.

* Bootloader set frequency 792MHz is added to the OPP table to
  the avoid 'need at least 2 OPPs to use CPR' error

* Remove 1.2GHz as it is not supported in any of the IPQ9574 SKUs.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v4: s/cprh/cpr4/
v2: Update commit log. No code change.
---
 arch/arm64/boot/dts/qcom/ipq9574.dtsi | 269 ++++++++++++++++++++++++--
 1 file changed, 252 insertions(+), 17 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
index 04ba09a9156c..cda30a877877 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
@@ -11,6 +11,7 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/reset/qcom,ipq9574-gcc.h>
 #include <dt-bindings/thermal/thermal.h>
+#include <dt-bindings/power/qcom-rpmpd.h>
 
 / {
 	interrupt-parent = <&intc>;
@@ -42,8 +43,9 @@ CPU0: cpu@0 {
 			clocks = <&apcs_glb APCS_ALIAS0_CORE_CLK>;
 			clock-names = "cpu";
 			operating-points-v2 = <&cpu_opp_table>;
-			cpu-supply = <&ipq9574_s1>;
 			#cooling-cells = <2>;
+			power-domains = <&apc_cpr4 0>;
+			power-domain-names = "perf";
 		};
 
 		CPU1: cpu@1 {
@@ -55,8 +57,9 @@ CPU1: cpu@1 {
 			clocks = <&apcs_glb APCS_ALIAS0_CORE_CLK>;
 			clock-names = "cpu";
 			operating-points-v2 = <&cpu_opp_table>;
-			cpu-supply = <&ipq9574_s1>;
 			#cooling-cells = <2>;
+			power-domains = <&apc_cpr4 0>;
+			power-domain-names = "perf";
 		};
 
 		CPU2: cpu@2 {
@@ -68,8 +71,9 @@ CPU2: cpu@2 {
 			clocks = <&apcs_glb APCS_ALIAS0_CORE_CLK>;
 			clock-names = "cpu";
 			operating-points-v2 = <&cpu_opp_table>;
-			cpu-supply = <&ipq9574_s1>;
 			#cooling-cells = <2>;
+			power-domains = <&apc_cpr4 0>;
+			power-domain-names = "perf";
 		};
 
 		CPU3: cpu@3 {
@@ -81,8 +85,9 @@ CPU3: cpu@3 {
 			clocks = <&apcs_glb APCS_ALIAS0_CORE_CLK>;
 			clock-names = "cpu";
 			operating-points-v2 = <&cpu_opp_table>;
-			cpu-supply = <&ipq9574_s1>;
 			#cooling-cells = <2>;
+			power-domains = <&apc_cpr4 0>;
+			power-domain-names = "perf";
 		};
 
 		L2_0: l2-cache {
@@ -105,58 +110,111 @@ memory@40000000 {
 		reg = <0x0 0x40000000 0x0 0x0>;
 	};
 
+	cpr4_opp_table: opp-table-cpr4 {
+		compatible = "operating-points-v2-qcom-level";
+
+		cpr4_opp0: opp-0 {
+			opp-level = <1>;
+			qcom,opp-fuse-level = <1>;
+			qcom,opp-cloop-vadj = <0>;
+			qcom,opp-oloop-vadj = <0>;
+		};
+
+		cpr4_opp1: opp-1 {
+			opp-level = <2>;
+			qcom,opp-fuse-level = <1>;
+			qcom,opp-cloop-vadj = <0>;
+			qcom,opp-oloop-vadj = <0>;
+		};
+
+		cpr4_opp2: opp-2 {
+			opp-level = <3>;
+			qcom,opp-fuse-level = <1>;
+			qcom,opp-cloop-vadj = <0>;
+			qcom,opp-oloop-vadj = <0>;
+		};
+
+		cpr4_opp3: opp-3 {
+			opp-level = <4>;
+			qcom,opp-fuse-level = <2>;
+			qcom,opp-cloop-vadj = <0>;
+			qcom,opp-oloop-vadj = <0>;
+		};
+
+		cpr4_opp4: opp-4 {
+			opp-level = <5>;
+			qcom,opp-fuse-level = <2>;
+			qcom,opp-cloop-vadj = <0>;
+			qcom,opp-oloop-vadj = <0>;
+		};
+
+		cpr4_opp5: opp-5 {
+			opp-level = <6>;
+			qcom,opp-fuse-level = <3>;
+			qcom,opp-cloop-vadj = <0>;
+			qcom,opp-oloop-vadj = <0>;
+		};
+
+		cpr4_opp6: opp-6 {
+			opp-level = <7>;
+			qcom,opp-fuse-level = <4>;
+			qcom,opp-cloop-vadj = <0>;
+			qcom,opp-oloop-vadj = <0>;
+		};
+	};
+
 	cpu_opp_table: opp-table-cpu {
 		compatible = "operating-points-v2-kryo-cpu";
 		opp-shared;
 		nvmem-cells = <&cpu_speed_bin>;
 
+		opp-792000000 {
+			opp-hz = /bits/ 64 <792000000>;
+			opp-supported-hw = <0x0>;
+			clock-latency-ns = <200000>;
+			required-opps = <&cpr4_opp0>;
+		};
+
 		opp-936000000 {
 			opp-hz = /bits/ 64 <936000000>;
-			opp-microvolt = <725000>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			required-opps = <&cpr4_opp1>;
 		};
 
 		opp-1104000000 {
 			opp-hz = /bits/ 64 <1104000000>;
-			opp-microvolt = <787500>;
-			opp-supported-hw = <0xf>;
-			clock-latency-ns = <200000>;
-		};
-
-		opp-1200000000 {
-			opp-hz = /bits/ 64 <1200000000>;
-			opp-microvolt = <862500>;
 			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
+			required-opps = <&cpr4_opp2>;
 		};
 
 		opp-1416000000 {
 			opp-hz = /bits/ 64 <1416000000>;
-			opp-microvolt = <862500>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
+			required-opps = <&cpr4_opp3>;
 		};
 
 		opp-1488000000 {
 			opp-hz = /bits/ 64 <1488000000>;
-			opp-microvolt = <925000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
+			required-opps = <&cpr4_opp4>;
 		};
 
 		opp-1800000000 {
 			opp-hz = /bits/ 64 <1800000000>;
-			opp-microvolt = <987500>;
 			opp-supported-hw = <0x5>;
 			clock-latency-ns = <200000>;
+			required-opps = <&cpr4_opp5>;
 		};
 
 		opp-2208000000 {
 			opp-hz = /bits/ 64 <2208000000>;
-			opp-microvolt = <1062500>;
 			opp-supported-hw = <0x1>;
 			clock-latency-ns = <200000>;
+			required-opps = <&cpr4_opp6>;
 		};
 	};
 
@@ -182,6 +240,40 @@ glink-edge {
 			rpm_requests: rpm-requests {
 				compatible = "qcom,rpm-ipq9574";
 				qcom,glink-channels = "rpm_requests";
+
+				rpmpd: power-controller {
+					compatible = "qcom,ipq9574-rpmpd";
+					#power-domain-cells = <1>;
+					operating-points-v2 = <&rpmpd_opp_table>;
+
+					rpmpd_opp_table: opp-table {
+						compatible = "operating-points-v2";
+
+						rpmpd_opp_svs: opp1 {
+							opp-level = <RPM_SMD_LEVEL_SVS>;
+						};
+
+						rpmpd_opp_svs_plus: opp2 {
+							opp-level = <RPM_SMD_LEVEL_SVS_PLUS>;
+						};
+
+						rpmpd_opp_nom: opp3 {
+							opp-level = <RPM_SMD_LEVEL_NOM>;
+						};
+
+						rpmpd_opp_nom_plus: opp4 {
+							opp-level = <RPM_SMD_LEVEL_NOM_PLUS>;
+						};
+
+						rpmpd_opp_turbo: opp5 {
+							opp-level = <RPM_SMD_LEVEL_TURBO>;
+						};
+
+						rpmpd_opp_turbo_high: opp6 {
+							opp-level = <RPM_SMD_LEVEL_TURBO_HIGH>;
+						};
+					};
+				};
 			};
 		};
 	};
@@ -252,6 +344,95 @@ cpu_speed_bin: cpu-speed-bin@15 {
 				reg = <0x15 0x2>;
 				bits = <7 2>;
 			};
+
+			cpr_efuse_speedbin: speedbin@5 {
+				reg = <0x5 0x8>;
+				bits = <0 3>;
+			};
+
+			cpr_fuse_revision: cpr-fusing-rev@7 {
+				reg = <0x7 0x8>;
+				bits = <1 5>;
+			};
+
+			/* CPR Ring Oscillator: Power Cluster */
+			cpr_ro_sel0_pwrcl: rosel0-pwrcl@358 {	/* ROSEL_SVS */
+				reg = <0x358 0x1>;
+				bits = <4 4>;
+			};
+
+			cpr_ro_sel1_pwrcl: rosel1-pwrcl@358 {	/* ROSEL_NOM */
+				reg = <0x358 0x1>;
+				bits = <0 4>;
+			};
+
+			cpr_ro_sel2_pwrcl: rosel2-pwrcl@350 {	/* ROSEL_TUR */
+				reg = <0x350 0x1>;
+				bits = <4 4>;
+			};
+
+			cpr_ro_sel3_pwrcl: rosel3-pwrcl@350 {	/* ROSEL_STUR */
+				reg = <0x350 0x1>;
+				bits = <0 4>;
+			};
+
+			/* CPR Init Voltage: Power Cluster */
+			cpr_init_voltage0_pwrcl: ivolt0-pwrcl@343 {	/* VOLT_SVS */
+				reg = <0x343 0x1>;
+				bits = <0 6>;
+			};
+
+			cpr_init_voltage1_pwrcl: ivolt1-pwrcl@342 {	/* VOLT_NOM */
+				reg = <0x342 0x1>;
+				bits = <2 6>;
+			};
+
+			cpr_init_voltage2_pwrcl: ivolt2-pwrcl@341 {	/* VOLT_TUR */
+				reg = <0x341 0x2>;
+				bits = <4 6>;
+			};
+
+			cpr_init_voltage3_pwrcl: ivolt3-pwrcl@340 {	/* VOLT_STUR */
+				reg = <0x340 0x2>;
+				bits = <6 6>;
+			};
+
+			/* CPR Target Quotients: Power Cluster */
+			cpr_quot0_pwrcl: quot0-pwrcl@354 {	/* QUOT_VMIN_SVS */
+				reg = <0x354 0x2>;
+				bits = <0 12>;
+			};
+
+			cpr_quot1_pwrcl: quot1-pwrcl@352 {	/* QUOT_VMIN_NOM */
+				reg = <0x352 0x2>;
+				bits = <4 12>;
+			};
+
+			cpr_quot2_pwrcl: quot2-pwrcl@351 {	/* QUOT_VMIN_TUR */
+				reg = <0x351 0x2>;
+				bits = <0 12>;
+			};
+
+			cpr_quot3_pwrcl: quot3-pwrcl@355 {	/* QUOT_VMIN_STUR */
+				reg = <0x355 0x2>;
+				bits = <4 12>;
+			};
+
+			/* CPR Quotient Offsets: Power Cluster */
+			cpr_quot_offset1_pwrcl: qoff1-pwrcl@34e {	/* QUOT_OFFSET_NOM_SVS */
+				reg = <0x34e 0x1>;
+				bits = <0 8>;
+			};
+
+			cpr_quot_offset2_pwrcl: qoff2-pwrcl@34d {	/* QUOT_OFFSET_TUR_NOM */
+				reg = <0x34d 0x1>;
+				bits = <0 8>;
+			};
+
+			cpr_quot_offset3_pwrcl: qoff0-pwrcl@34c {	/* QUOT_OFFSET_STUR_TUR */
+				reg = <0x34c 0x1>;
+				bits = <0 8>;
+			};
 		};
 
 		cryptobam: dma-controller@704000 {
@@ -639,6 +820,60 @@ usb_0_dwc3: usb@8a00000 {
 			};
 		};
 
+		apc_cpr4: power-controller@b018000 {
+			compatible = "qcom,ipq9574-cpr4", "qcom,cprh";
+			reg = <0x0b018000 0x4000>,
+			      <0x00048000 0x4000>;
+
+			clocks = <&gcc GCC_RBCPR_CLK>;
+
+			interrupts = <GIC_SPI 15 IRQ_TYPE_EDGE_RISING>;
+			vdd-supply = <&ipq9574_s1>;
+
+			/* Set the CPR clock here, it needs to match XO */
+			assigned-clocks = <&gcc GCC_RBCPR_CLK>;
+			assigned-clock-rates = <24000000>;
+
+			operating-points-v2 = <&cpr4_opp_table>;
+			power-domains = <&rpmpd IPQ9574_VDDAPC>;
+			#power-domain-cells = <1>;
+
+			nvmem-cells = <&cpr_efuse_speedbin>,
+				      <&cpr_fuse_revision>,
+				      <&cpr_quot0_pwrcl>,
+				      <&cpr_quot1_pwrcl>,
+				      <&cpr_quot2_pwrcl>,
+				      <&cpr_quot3_pwrcl>,
+				      <&cpr_quot_offset1_pwrcl>,
+				      <&cpr_quot_offset2_pwrcl>,
+				      <&cpr_quot_offset3_pwrcl>,
+				      <&cpr_init_voltage0_pwrcl>,
+				      <&cpr_init_voltage1_pwrcl>,
+				      <&cpr_init_voltage2_pwrcl>,
+				      <&cpr_init_voltage3_pwrcl>,
+				      <&cpr_ro_sel0_pwrcl>,
+				      <&cpr_ro_sel1_pwrcl>,
+				      <&cpr_ro_sel2_pwrcl>,
+				      <&cpr_ro_sel3_pwrcl>;
+			nvmem-cell-names = "cpr_speed_bin",
+					   "cpr_fuse_revision",
+					   "cpr0_quotient1",
+					   "cpr0_quotient2",
+					   "cpr0_quotient3",
+					   "cpr0_quotient4",
+					   "cpr0_quotient_offset2",
+					   "cpr0_quotient_offset3",
+					   "cpr0_quotient_offset4",
+					   "cpr0_init_voltage1",
+					   "cpr0_init_voltage2",
+					   "cpr0_init_voltage3",
+					   "cpr0_init_voltage4",
+					   "cpr0_ring_osc1",
+					   "cpr0_ring_osc2",
+					   "cpr0_ring_osc3",
+					   "cpr0_ring_osc4";
+		};
+
 		intc: interrupt-controller@b000000 {
 			compatible = "qcom,msm-qgic2";
 			reg = <0x0b000000 0x1000>,  /* GICD */
-- 
2.34.1

Re: [PATCH v4 02/10] cpufreq: qcom-nvmem: Add support for IPQ9574

[Viresh Kumar]

On 03-07-24, 14:46, Varadarajan Narayanan wrote:
> Add qcom_cpufreq_match_data for IPQ9574. This is used for tying
> up the cpu@N nodes with the power domains. match_data_kryo is not
> used since it doesn't set genpd_names. If genpd_names is not set,
> 'cat /sys/kernel/debug/qcom_cpr3/thread0' causes cpr3_debug_info_show()
> to crash while trying to read thread->corner->last_uV as
> thread->corner is NULL.
> 
> 	Call trace:
> 		cpr3_debug_info_show
> 		seq_read_iter
> 		seq_read
> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v4: Update commit log to include stack trace
>     Introduce qcom_cpufreq_match_data for IPQ9574

Can I apply this without other changes ?

-- 
viresh

Re: [PATCH v4 01/10] soc: qcom: cpr3: Fix 'acc_desc' usage

[Dmitry Baryshkov]

On Wed, Jul 03, 2024 at 02:46:42PM GMT, Varadarajan Narayanan wrote:
> cpr3 code assumes that 'acc_desc' is available for SoCs
> implementing CPR version 4 or less. However, IPQ9574 SoC
> implements CPRv4 without ACC. This causes NULL pointer accesses
> resulting in crashes. Hence, check if 'acc_desc' is populated
> before using it.
> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v4: Undo the acc_desc validation in probe function as that could
>     affect other SoC.
> ---
>  drivers/pmdomain/qcom/cpr3.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pmdomain/qcom/cpr3.c b/drivers/pmdomain/qcom/cpr3.c
> index c7790a71e74f..6ceb7605f84d 100644
> --- a/drivers/pmdomain/qcom/cpr3.c
> +++ b/drivers/pmdomain/qcom/cpr3.c
> @@ -2399,12 +2399,12 @@ static int cpr_pd_attach_dev(struct generic_pm_domain *domain,
>  		if (ret)
>  			goto exit;
>  
> -		if (acc_desc->config)
> +		if (acc_desc && acc_desc->config)
>  			regmap_multi_reg_write(drv->tcsr, acc_desc->config,
>  					       acc_desc->num_regs_per_fuse);
>  
>  		/* Enable ACC if required */
> -		if (acc_desc->enable_mask)
> +		if (acc_desc && acc_desc->enable_mask)
>  			regmap_update_bits(drv->tcsr, acc_desc->enable_reg,
>  					   acc_desc->enable_mask,
>  					   acc_desc->enable_mask);

Should the same fix be applied to other places which access acc_desc?
For example cpr_pre_voltage() and cpr_post_voltage() which call
cpr_set_acc()?

-- 
With best wishes
Dmitry

Re: [PATCH v4 02/10] cpufreq: qcom-nvmem: Add support for IPQ9574

[Dmitry Baryshkov]

On Wed, Jul 03, 2024 at 03:49:44PM GMT, Viresh Kumar wrote:
> On 03-07-24, 14:46, Varadarajan Narayanan wrote:
> > Add qcom_cpufreq_match_data for IPQ9574. This is used for tying
> > up the cpu@N nodes with the power domains. match_data_kryo is not
> > used since it doesn't set genpd_names. If genpd_names is not set,
> > 'cat /sys/kernel/debug/qcom_cpr3/thread0' causes cpr3_debug_info_show()
> > to crash while trying to read thread->corner->last_uV as
> > thread->corner is NULL.
> > 
> > 	Call trace:
> > 		cpr3_debug_info_show
> > 		seq_read_iter
> > 		seq_read
> > 
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> > v4: Update commit log to include stack trace
> >     Introduce qcom_cpufreq_match_data for IPQ9574
> 
> Can I apply this without other changes ?

It will break CPUfreq support on IPQ9574. So this patch should go last.

-- 
With best wishes
Dmitry

Re: [PATCH v4 08/10] soc: qcom: cpr3: Add IPQ9574 definitions

[Dmitry Baryshkov]

On Wed, Jul 03, 2024 at 02:46:49PM GMT, Varadarajan Narayanan wrote:
> From: Praveenkumar I <quic_ipkumar@quicinc.com>
> 
> * Add thread, scaling factor, CPR descriptor defines to enable
>   CPR on IPQ9574.
> 
> * Skip 'acc' usage since IPQ9574 does not have acc
> 
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v4: s/silver//, s/cprh/cpr4/
>     Skip 'acc' related code as IPQ9574 does not have acc
> 
> v3: Fix patch author
>     Included below information in cover letter
> v2: Fix Signed-off-by order
> Depends:
> 	[1] https://lore.kernel.org/lkml/20230217-topic-cpr3h-v14-0-9fd23241493d@linaro.org/T/
> 	[2] https://github.com/quic-varada/cpr/commits/konrad/
> ---
>  drivers/pmdomain/qcom/cpr3.c | 143 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 141 insertions(+), 2 deletions(-)
> 



> @@ -2703,7 +2840,8 @@ static int cpr_probe(struct platform_device *pdev)
>  
>  	mutex_init(&drv->lock);
>  
> -	if (desc->cpr_type < CTRL_TYPE_CPRH) {
> +	if (desc->cpr_type < CTRL_TYPE_CPRH &&
> +	    !of_device_is_compatible(dev->of_node, "qcom,ipq9574-cpr4")) {

No. Check for ->acc_desc instead.

>  		np = of_parse_phandle(dev->of_node, "qcom,acc", 0);
>  		if (!np)
>  			return -ENODEV;
> @@ -2828,6 +2966,7 @@ static void cpr_remove(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id cpr3_match_table[] = {
> +	{ .compatible = "qcom,ipq9574-cpr4", .data = &ipq9574_cpr_acc_desc },
>  	{ .compatible = "qcom,msm8998-cprh", .data = &msm8998_cpr_acc_desc },
>  	{ .compatible = "qcom,sdm630-cprh", .data = &sdm630_cpr_acc_desc },
>  	{ }
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v4 02/10] cpufreq: qcom-nvmem: Add support for IPQ9574

[Dmitry Baryshkov]

On Wed, Jul 03, 2024 at 02:46:43PM GMT, Varadarajan Narayanan wrote:
> Add qcom_cpufreq_match_data for IPQ9574. This is used for tying
> up the cpu@N nodes with the power domains. match_data_kryo is not
> used since it doesn't set genpd_names. If genpd_names is not set,
> 'cat /sys/kernel/debug/qcom_cpr3/thread0' causes cpr3_debug_info_show()
> to crash while trying to read thread->corner->last_uV as
> thread->corner is NULL.
> 
> 	Call trace:
> 		cpr3_debug_info_show
> 		seq_read_iter
> 		seq_read

I'd say, the commit message might be simpler:

IPQ9574 uses CPR4 power domain to manage core supplies. Use
device-specific match data for this platform that includes genpd_names
configuration.

Anyway:

Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v4: Update commit log to include stack trace
>     Introduce qcom_cpufreq_match_data for IPQ9574
> ---
>  drivers/cpufreq/qcom-cpufreq-nvmem.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> index 939702dfa73f..95558586c2e6 100644
> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> @@ -428,6 +428,11 @@ static const struct qcom_cpufreq_match_data match_data_ipq8074 = {
>  	.get_version = qcom_cpufreq_ipq8074_name_version,
>  };
>  
> +static const struct qcom_cpufreq_match_data match_data_ipq9574 = {
> +	.get_version = qcom_cpufreq_kryo_name_version,
> +	.genpd_names = generic_genpd_names,
> +};
> +
>  static void qcom_cpufreq_suspend_virt_devs(struct qcom_cpufreq_drv *drv, unsigned int cpu)
>  {
>  	const char * const *name = drv->data->genpd_names;
> @@ -621,7 +626,7 @@ static const struct of_device_id qcom_cpufreq_match_list[] __initconst = {
>  	{ .compatible = "qcom,ipq8064", .data = &match_data_ipq8064 },
>  	{ .compatible = "qcom,ipq8074", .data = &match_data_ipq8074 },
>  	{ .compatible = "qcom,apq8064", .data = &match_data_krait },
> -	{ .compatible = "qcom,ipq9574", .data = &match_data_kryo },
> +	{ .compatible = "qcom,ipq9574", .data = &match_data_ipq9574 },
>  	{ .compatible = "qcom,msm8974", .data = &match_data_krait },
>  	{ .compatible = "qcom,msm8960", .data = &match_data_krait },
>  	{},
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v4 01/10] soc: qcom: cpr3: Fix 'acc_desc' usage

[Varadarajan Narayanan]

On Wed, Jul 03, 2024 at 01:46:54PM +0300, Dmitry Baryshkov wrote:
> On Wed, Jul 03, 2024 at 02:46:42PM GMT, Varadarajan Narayanan wrote:
> > cpr3 code assumes that 'acc_desc' is available for SoCs
> > implementing CPR version 4 or less. However, IPQ9574 SoC
> > implements CPRv4 without ACC. This causes NULL pointer accesses
> > resulting in crashes. Hence, check if 'acc_desc' is populated
> > before using it.
> >
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> > v4: Undo the acc_desc validation in probe function as that could
> >     affect other SoC.
> > ---
> >  drivers/pmdomain/qcom/cpr3.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pmdomain/qcom/cpr3.c b/drivers/pmdomain/qcom/cpr3.c
> > index c7790a71e74f..6ceb7605f84d 100644
> > --- a/drivers/pmdomain/qcom/cpr3.c
> > +++ b/drivers/pmdomain/qcom/cpr3.c
> > @@ -2399,12 +2399,12 @@ static int cpr_pd_attach_dev(struct generic_pm_domain *domain,
> >  		if (ret)
> >  			goto exit;
> >
> > -		if (acc_desc->config)
> > +		if (acc_desc && acc_desc->config)
> >  			regmap_multi_reg_write(drv->tcsr, acc_desc->config,
> >  					       acc_desc->num_regs_per_fuse);
> >
> >  		/* Enable ACC if required */
> > -		if (acc_desc->enable_mask)
> > +		if (acc_desc && acc_desc->enable_mask)
> >  			regmap_update_bits(drv->tcsr, acc_desc->enable_reg,
> >  					   acc_desc->enable_mask,
> >  					   acc_desc->enable_mask);
>
> Should the same fix be applied to other places which access acc_desc?
> For example cpr_pre_voltage() and cpr_post_voltage() which call
> cpr_set_acc()?

With this patch alone, if acc_desc is NULL, cpr_probe() will fail
at the start itself because of this check

	if (!data->acc_desc && desc->cpr_type < CTRL_TYPE_CPRH)
		return -EINVAL;

After applying this patch series, cpr_probe will cross the above
check to accomodate IPQ9574. However, the check below will ensure
drv->tcsr is not initialized.

	if (desc->cpr_type < CTRL_TYPE_CPRH &&
	    !of_device_is_compatible(dev->of_node, "qcom,ipq9574-cpr4"))

cpr_pre_voltage() and cpr_post_voltage() call cpr_set_acc() only
if drv->tcsr is not NULL. Hence acc_desc need not be checked.

Will add the check to cpr_pre_voltage() and cpr_post_voltage() if
you feel it will make it more robust regardless of the changes to
cpr_probe in future. Please let me know.

Thanks
Varada

Re: [PATCH v4 08/10] soc: qcom: cpr3: Add IPQ9574 definitions

[Varadarajan Narayanan]

On Wed, Jul 03, 2024 at 01:49:15PM +0300, Dmitry Baryshkov wrote:
> On Wed, Jul 03, 2024 at 02:46:49PM GMT, Varadarajan Narayanan wrote:
> > From: Praveenkumar I <quic_ipkumar@quicinc.com>
> >
> > * Add thread, scaling factor, CPR descriptor defines to enable
> >   CPR on IPQ9574.
> >
> > * Skip 'acc' usage since IPQ9574 does not have acc
> >
> > Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> > v4: s/silver//, s/cprh/cpr4/
> >     Skip 'acc' related code as IPQ9574 does not have acc
> >
> > v3: Fix patch author
> >     Included below information in cover letter
> > v2: Fix Signed-off-by order
> > Depends:
> > 	[1] https://lore.kernel.org/lkml/20230217-topic-cpr3h-v14-0-9fd23241493d@linaro.org/T/
> > 	[2] https://github.com/quic-varada/cpr/commits/konrad/
> > ---
> >  drivers/pmdomain/qcom/cpr3.c | 143 ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 141 insertions(+), 2 deletions(-)
> >
>
>
>
> > @@ -2703,7 +2840,8 @@ static int cpr_probe(struct platform_device *pdev)
> >
> >  	mutex_init(&drv->lock);
> >
> > -	if (desc->cpr_type < CTRL_TYPE_CPRH) {
> > +	if (desc->cpr_type < CTRL_TYPE_CPRH &&
> > +	    !of_device_is_compatible(dev->of_node, "qcom,ipq9574-cpr4")) {
>
> No. Check for ->acc_desc instead.

Ok.

Thanks
Varada

> >  		np = of_parse_phandle(dev->of_node, "qcom,acc", 0);
> >  		if (!np)
> >  			return -ENODEV;
> > @@ -2828,6 +2966,7 @@ static void cpr_remove(struct platform_device *pdev)
> >  }
> >
> >  static const struct of_device_id cpr3_match_table[] = {
> > +	{ .compatible = "qcom,ipq9574-cpr4", .data = &ipq9574_cpr_acc_desc },
> >  	{ .compatible = "qcom,msm8998-cprh", .data = &msm8998_cpr_acc_desc },
> >  	{ .compatible = "qcom,sdm630-cprh", .data = &sdm630_cpr_acc_desc },
> >  	{ }
> > --
> > 2.34.1
> >
>
> --
> With best wishes
> Dmitry

Re: [PATCH v4 04/10] dt-bindings: soc: qcom: cpr3: Add bindings for IPQ9574

[Krzysztof Kozlowski]

On 03/07/2024 11:16, Varadarajan Narayanan wrote:
> Add the bindings for the IPQ9574 CPR3 driver to the documentation.
> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v4 01/10] soc: qcom: cpr3: Fix 'acc_desc' usage

[Dmitry Baryshkov]

On Thu, Jul 04, 2024 at 10:30:50AM GMT, Varadarajan Narayanan wrote:
> On Wed, Jul 03, 2024 at 01:46:54PM +0300, Dmitry Baryshkov wrote:
> > On Wed, Jul 03, 2024 at 02:46:42PM GMT, Varadarajan Narayanan wrote:
> > > cpr3 code assumes that 'acc_desc' is available for SoCs
> > > implementing CPR version 4 or less. However, IPQ9574 SoC
> > > implements CPRv4 without ACC. This causes NULL pointer accesses
> > > resulting in crashes. Hence, check if 'acc_desc' is populated
> > > before using it.
> > >
> > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > ---
> > > v4: Undo the acc_desc validation in probe function as that could
> > >     affect other SoC.
> > > ---
> > >  drivers/pmdomain/qcom/cpr3.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/pmdomain/qcom/cpr3.c b/drivers/pmdomain/qcom/cpr3.c
> > > index c7790a71e74f..6ceb7605f84d 100644
> > > --- a/drivers/pmdomain/qcom/cpr3.c
> > > +++ b/drivers/pmdomain/qcom/cpr3.c
> > > @@ -2399,12 +2399,12 @@ static int cpr_pd_attach_dev(struct generic_pm_domain *domain,
> > >  		if (ret)
> > >  			goto exit;
> > >
> > > -		if (acc_desc->config)
> > > +		if (acc_desc && acc_desc->config)
> > >  			regmap_multi_reg_write(drv->tcsr, acc_desc->config,
> > >  					       acc_desc->num_regs_per_fuse);
> > >
> > >  		/* Enable ACC if required */
> > > -		if (acc_desc->enable_mask)
> > > +		if (acc_desc && acc_desc->enable_mask)
> > >  			regmap_update_bits(drv->tcsr, acc_desc->enable_reg,
> > >  					   acc_desc->enable_mask,
> > >  					   acc_desc->enable_mask);
> >
> > Should the same fix be applied to other places which access acc_desc?
> > For example cpr_pre_voltage() and cpr_post_voltage() which call
> > cpr_set_acc()?
> 
> With this patch alone, if acc_desc is NULL, cpr_probe() will fail
> at the start itself because of this check
> 
> 	if (!data->acc_desc && desc->cpr_type < CTRL_TYPE_CPRH)
> 		return -EINVAL;
> 
> After applying this patch series, cpr_probe will cross the above
> check to accomodate IPQ9574. However, the check below will ensure
> drv->tcsr is not initialized.
> 
> 	if (desc->cpr_type < CTRL_TYPE_CPRH &&
> 	    !of_device_is_compatible(dev->of_node, "qcom,ipq9574-cpr4"))
> 
> cpr_pre_voltage() and cpr_post_voltage() call cpr_set_acc() only
> if drv->tcsr is not NULL. Hence acc_desc need not be checked.
> 
> Will add the check to cpr_pre_voltage() and cpr_post_voltage() if
> you feel it will make it more robust regardless of the changes to
> cpr_probe in future. Please let me know.

Having !acc_desc check instead of the of_device_is_compatible would
solve the issue.

-- 
With best wishes
Dmitry

Re: [PATCH v4 09/10] dt-bindings: opp: v2-qcom-level: Update minItems for oloop-vadj & cloop-vadj

[Rob Herring]

On Wed, Jul 03, 2024 at 02:46:50PM +0530, Varadarajan Narayanan wrote:
> Since IPQ9574 has only one CPR thread it will specify
> only one voltage adjustment value. Hence update min items
> accordingly for oloop-vadj and cloop-vadj. Without
> constraining min items, dt_binding_check gives errors
> 
> 	opp-table-cpr4:opp-0:qcom,opp-cloop-vadj:0: [0] is too short
> 	opp-table-cpr4:opp-0:qcom,opp-oloop-vadj:0: [0] is too short
> 
> 	Failed validating 'minItems' in schema . . .
> 		{'maxItems': 2, 'minItems': 2}
> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v4: Fix dt_bindings_check error
> ---
>  Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml | 2 ++
>  1 file changed, 2 insertions(+)

This is going to need to be rolled into your dependency because it needs 
the same fix.

> 
> diff --git a/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml b/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml
> index b203ea01b17a..1c1a9e12d57a 100644
> --- a/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml
> +++ b/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml
> @@ -39,6 +39,7 @@ patternProperties:
>            An array of per-thread values representing the closed-loop
>            voltage adjustment value associated with this OPP node.
>          $ref: /schemas/types.yaml#/definitions/int32-array
> +        minItems: 1
>          maxItems: 2
>  
>        qcom,opp-oloop-vadj:
> @@ -46,6 +47,7 @@ patternProperties:
>            An array of per-thread values representing the open-loop
>            voltage adjustment value associated with this OPP node.
>          $ref: /schemas/types.yaml#/definitions/int32-array
> +        minItems: 1
>          maxItems: 2
>  
>      required:
> -- 
> 2.34.1
> 

Re: [PATCH v4 09/10] dt-bindings: opp: v2-qcom-level: Update minItems for oloop-vadj & cloop-vadj

[Varadarajan Narayanan]

On Mon, Jul 08, 2024 at 09:55:29AM -0600, Rob Herring wrote:
> On Wed, Jul 03, 2024 at 02:46:50PM +0530, Varadarajan Narayanan wrote:
> > Since IPQ9574 has only one CPR thread it will specify
> > only one voltage adjustment value. Hence update min items
> > accordingly for oloop-vadj and cloop-vadj. Without
> > constraining min items, dt_binding_check gives errors
> >
> > 	opp-table-cpr4:opp-0:qcom,opp-cloop-vadj:0: [0] is too short
> > 	opp-table-cpr4:opp-0:qcom,opp-oloop-vadj:0: [0] is too short
> >
> > 	Failed validating 'minItems' in schema . . .
> > 		{'maxItems': 2, 'minItems': 2}
> >
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> > v4: Fix dt_bindings_check error
> > ---
> >  Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
>
> This is going to need to be rolled into your dependency because it needs
> the same fix.

Konrad,

Can you please squash this into https://lore.kernel.org/lkml/20230217-topic-cpr3h-v14-2-9fd23241493d@linaro.org/

Thanks
Varada

> > diff --git a/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml b/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml
> > index b203ea01b17a..1c1a9e12d57a 100644
> > --- a/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml
> > +++ b/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml
> > @@ -39,6 +39,7 @@ patternProperties:
> >            An array of per-thread values representing the closed-loop
> >            voltage adjustment value associated with this OPP node.
> >          $ref: /schemas/types.yaml#/definitions/int32-array
> > +        minItems: 1
> >          maxItems: 2
> >
> >        qcom,opp-oloop-vadj:
> > @@ -46,6 +47,7 @@ patternProperties:
> >            An array of per-thread values representing the open-loop
> >            voltage adjustment value associated with this OPP node.
> >          $ref: /schemas/types.yaml#/definitions/int32-array
> > +        minItems: 1
> >          maxItems: 2
> >
> >      required:
> > --
> > 2.34.1
> >

Re: [PATCH v4 01/10] soc: qcom: cpr3: Fix 'acc_desc' usage

[Varadarajan Narayanan]

On Fri, Jul 05, 2024 at 06:16:51PM +0300, Dmitry Baryshkov wrote:
> On Thu, Jul 04, 2024 at 10:30:50AM GMT, Varadarajan Narayanan wrote:
> > On Wed, Jul 03, 2024 at 01:46:54PM +0300, Dmitry Baryshkov wrote:
> > > On Wed, Jul 03, 2024 at 02:46:42PM GMT, Varadarajan Narayanan wrote:
> > > > cpr3 code assumes that 'acc_desc' is available for SoCs
> > > > implementing CPR version 4 or less. However, IPQ9574 SoC
> > > > implements CPRv4 without ACC. This causes NULL pointer accesses
> > > > resulting in crashes. Hence, check if 'acc_desc' is populated
> > > > before using it.
> > > >
> > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > > ---
> > > > v4: Undo the acc_desc validation in probe function as that could
> > > >     affect other SoC.
> > > > ---
> > > >  drivers/pmdomain/qcom/cpr3.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/pmdomain/qcom/cpr3.c b/drivers/pmdomain/qcom/cpr3.c
> > > > index c7790a71e74f..6ceb7605f84d 100644
> > > > --- a/drivers/pmdomain/qcom/cpr3.c
> > > > +++ b/drivers/pmdomain/qcom/cpr3.c
> > > > @@ -2399,12 +2399,12 @@ static int cpr_pd_attach_dev(struct generic_pm_domain *domain,
> > > >  		if (ret)
> > > >  			goto exit;
> > > >
> > > > -		if (acc_desc->config)
> > > > +		if (acc_desc && acc_desc->config)
> > > >  			regmap_multi_reg_write(drv->tcsr, acc_desc->config,
> > > >  					       acc_desc->num_regs_per_fuse);
> > > >
> > > >  		/* Enable ACC if required */
> > > > -		if (acc_desc->enable_mask)
> > > > +		if (acc_desc && acc_desc->enable_mask)
> > > >  			regmap_update_bits(drv->tcsr, acc_desc->enable_reg,
> > > >  					   acc_desc->enable_mask,
> > > >  					   acc_desc->enable_mask);
> > >
> > > Should the same fix be applied to other places which access acc_desc?
> > > For example cpr_pre_voltage() and cpr_post_voltage() which call
> > > cpr_set_acc()?
> >
> > With this patch alone, if acc_desc is NULL, cpr_probe() will fail
> > at the start itself because of this check
> >
> > 	if (!data->acc_desc && desc->cpr_type < CTRL_TYPE_CPRH)
> > 		return -EINVAL;
> >
> > After applying this patch series, cpr_probe will cross the above
> > check to accomodate IPQ9574. However, the check below will ensure
> > drv->tcsr is not initialized.
> >
> > 	if (desc->cpr_type < CTRL_TYPE_CPRH &&
> > 	    !of_device_is_compatible(dev->of_node, "qcom,ipq9574-cpr4"))
> >
> > cpr_pre_voltage() and cpr_post_voltage() call cpr_set_acc() only
> > if drv->tcsr is not NULL. Hence acc_desc need not be checked.
> >
> > Will add the check to cpr_pre_voltage() and cpr_post_voltage() if
> > you feel it will make it more robust regardless of the changes to
> > cpr_probe in future. Please let me know.
>
> Having !acc_desc check instead of the of_device_is_compatible would
> solve the issue.

Ok. Will post next version with that.

Thanks
Varada

Re: [PATCH v4 05/10] pmdomain: qcom: rpmpd: Add IPQ9574 power domains

[Konrad Dybcio]

On 3.07.2024 11:16 AM, Varadarajan Narayanan wrote:
> From: Praveenkumar I <quic_ipkumar@quicinc.com>
> 
> Add the APC power domain definitions used in IPQ9574.
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v4: Add Reviewed-by: Dmitry Baryshkov
> v3: Fix patch author
> v2: Fix Signed-off-by order
> ---
>  drivers/pmdomain/qcom/rpmpd.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/pmdomain/qcom/rpmpd.c b/drivers/pmdomain/qcom/rpmpd.c
> index 5e6280b4cf70..947d6a9c3897 100644
> --- a/drivers/pmdomain/qcom/rpmpd.c
> +++ b/drivers/pmdomain/qcom/rpmpd.c
> @@ -38,6 +38,7 @@ static struct qcom_smd_rpm *rpmpd_smd_rpm;
>  #define KEY_FLOOR_CORNER	0x636676   /* vfc */
>  #define KEY_FLOOR_LEVEL		0x6c6676   /* vfl */
>  #define KEY_LEVEL		0x6c766c76 /* vlvl */
> +#define RPM_KEY_UV		0x00007675 /* "uv" */

The "uv" key is handled in qcom_smd-regulator.c.. I'm assuming on this
platform, it accepts level idx instead of the regulator properties
and this is intentional?

Konrad

Re: [PATCH v4 09/10] dt-bindings: opp: v2-qcom-level: Update minItems for oloop-vadj & cloop-vadj

[Konrad Dybcio]

On 9.07.2024 9:01 AM, Varadarajan Narayanan wrote:
> On Mon, Jul 08, 2024 at 09:55:29AM -0600, Rob Herring wrote:
>> On Wed, Jul 03, 2024 at 02:46:50PM +0530, Varadarajan Narayanan wrote:
>>> Since IPQ9574 has only one CPR thread it will specify
>>> only one voltage adjustment value. Hence update min items
>>> accordingly for oloop-vadj and cloop-vadj. Without
>>> constraining min items, dt_binding_check gives errors
>>>
>>> 	opp-table-cpr4:opp-0:qcom,opp-cloop-vadj:0: [0] is too short
>>> 	opp-table-cpr4:opp-0:qcom,opp-oloop-vadj:0: [0] is too short
>>>
>>> 	Failed validating 'minItems' in schema . . .
>>> 		{'maxItems': 2, 'minItems': 2}
>>>
>>> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
>>> ---
>>> v4: Fix dt_bindings_check error
>>> ---
>>>  Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml | 2 ++
>>>  1 file changed, 2 insertions(+)
>>
>> This is going to need to be rolled into your dependency because it needs
>> the same fix.
> 
> Konrad,
> 
> Can you please squash this into https://lore.kernel.org/lkml/20230217-topic-cpr3h-v14-2-9fd23241493d@linaro.org/

Yes, I'll do that in the next revision.. forgot to validate this..

Konrad

Re: [PATCH v4 05/10] pmdomain: qcom: rpmpd: Add IPQ9574 power domains

[Varadarajan Narayanan]

On Tue, Jul 09, 2024 at 11:52:19AM +0200, Konrad Dybcio wrote:
> On 3.07.2024 11:16 AM, Varadarajan Narayanan wrote:
> > From: Praveenkumar I <quic_ipkumar@quicinc.com>
> >
> > Add the APC power domain definitions used in IPQ9574.
> >
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> > v4: Add Reviewed-by: Dmitry Baryshkov
> > v3: Fix patch author
> > v2: Fix Signed-off-by order
> > ---
> >  drivers/pmdomain/qcom/rpmpd.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/pmdomain/qcom/rpmpd.c b/drivers/pmdomain/qcom/rpmpd.c
> > index 5e6280b4cf70..947d6a9c3897 100644
> > --- a/drivers/pmdomain/qcom/rpmpd.c
> > +++ b/drivers/pmdomain/qcom/rpmpd.c
> > @@ -38,6 +38,7 @@ static struct qcom_smd_rpm *rpmpd_smd_rpm;
> >  #define KEY_FLOOR_CORNER	0x636676   /* vfc */
> >  #define KEY_FLOOR_LEVEL		0x6c6676   /* vfl */
> >  #define KEY_LEVEL		0x6c766c76 /* vlvl */
> > +#define RPM_KEY_UV		0x00007675 /* "uv" */
>
> The "uv" key is handled in qcom_smd-regulator.c.. I'm assuming on this
> platform, it accepts level idx instead of the regulator properties
> and this is intentional?

IPQ9574 RPM accepts regulator properties (uv) and not the level idx.
Hence added the "uv" key in the rpmpd.c

Thanks
Praveen/Varada

Re: [PATCH v4 05/10] pmdomain: qcom: rpmpd: Add IPQ9574 power domains

[Dmitry Baryshkov]

On Wed, 24 Jul 2024 at 07:27, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> On Tue, Jul 09, 2024 at 11:52:19AM +0200, Konrad Dybcio wrote:
> > On 3.07.2024 11:16 AM, Varadarajan Narayanan wrote:
> > > From: Praveenkumar I <quic_ipkumar@quicinc.com>
> > >
> > > Add the APC power domain definitions used in IPQ9574.
> > >
> > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > ---
> > > v4: Add Reviewed-by: Dmitry Baryshkov
> > > v3: Fix patch author
> > > v2: Fix Signed-off-by order
> > > ---
> > >  drivers/pmdomain/qcom/rpmpd.c | 19 +++++++++++++++++++
> > >  1 file changed, 19 insertions(+)
> > >
> > > diff --git a/drivers/pmdomain/qcom/rpmpd.c b/drivers/pmdomain/qcom/rpmpd.c
> > > index 5e6280b4cf70..947d6a9c3897 100644
> > > --- a/drivers/pmdomain/qcom/rpmpd.c
> > > +++ b/drivers/pmdomain/qcom/rpmpd.c
> > > @@ -38,6 +38,7 @@ static struct qcom_smd_rpm *rpmpd_smd_rpm;
> > >  #define KEY_FLOOR_CORNER   0x636676   /* vfc */
> > >  #define KEY_FLOOR_LEVEL            0x6c6676   /* vfl */
> > >  #define KEY_LEVEL          0x6c766c76 /* vlvl */
> > > +#define RPM_KEY_UV         0x00007675 /* "uv" */
> >
> > The "uv" key is handled in qcom_smd-regulator.c.. I'm assuming on this
> > platform, it accepts level idx instead of the regulator properties
> > and this is intentional?
>
> IPQ9574 RPM accepts regulator properties (uv) and not the level idx.
> Hence added the "uv" key in the rpmpd.c

Does it expect the actual voltage? If so, then it is not a power
domain and it should be modelled as a regulator instead.


-- 
With best wishes
Dmitry