Skip to contentSkip to conversation

cpufreq: qcom-nvmem: add support for IPQ8074

PATCH Superseded Robert Marko 6 participants 23 messages Sep 30, 2023 – Oct 12, 2023

Subsystems (6)
Device tree bindings Arm Qualcomm mailing list Arm Qualcomm support CPU frequency scaling framework +2 more subsystems

v5 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Robert Marko

author
#0 lore raw

PATCH v5 1/4 cpufreq: qcom-nvmem: add support for IPQ8074

IPQ8074 comes in 2 families:
* IPQ8070A/IPQ8071A (Acorn) up to 1.4GHz
* IPQ8072A/IPQ8074A/IPQ8076A/IPQ8078A (Hawkeye) up to 2.2GHz

So, in order to be able to share one OPP table lets add support for IPQ8074 family based of SMEM SoC ID-s as speedbin fuse is always 0 on IPQ8074.

IPQ8074 compatible is blacklisted from DT platdev as the cpufreq device will get created by NVMEM CPUFreq driver.

Signed-off-by Robert Marko <[email protected]>

---
Changes in v4:
* Add support for IPQ8174 (Oak) family

Changes in v3:
* Use enum for SoC versions

Changes in v2:
* Print an error if SMEM ID is not part of the IPQ8074 family and restrict the speed to Acorn variant (1.4GHz)

 drivers/cpufreq/cpufreq-dt-platdev.c |  1 +
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 45 ++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)
drivers/cpufreq/cpufreq-dt-platdev.c +1 -0
index 2016d47889c0..157c91b9962c 100644
@@ -180,6 +180,7 @@ static const struct of_device_id blocklist[] __initconst = {
 	{ .compatible = "ti,am62a7", },
 
 	{ .compatible = "qcom,ipq8064", },
+	{ .compatible = "qcom,ipq8074", },
 	{ .compatible = "qcom,apq8064", },
 	{ .compatible = "qcom,msm8974", },
 	{ .compatible = "qcom,msm8960", },
drivers/cpufreq/qcom-cpufreq-nvmem.c +45 -0

69 lines not on this page · raw message

Signature · 1 line
2.41.0
author
#1 lore raw

PATCH v5 2/4 dt-bindings: opp: opp-v2-kryo-cpu: Document named opp-microvolt property

From: Christian Marangi <[email protected]>

Document named opp-microvolt property for opp-v2-kryo-cpu schema. This property is used to declare multiple voltage ranges selected on the different values read from efuses. The selection is done based on the speed pvs values and the named opp-microvolt property is selected by the qcom-cpufreq-nvmem driver.

Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Robert Marko <[email protected]>

---
Changes v5:
* Fix typo in opp items

Changes v4:
* Address comments from Rob (meaning of pvs, drop of driver specific info, drop of legacy single voltage OPP, better specify max regulators supported)

 .../bindings/opp/opp-v2-kryo-cpu.yaml         | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml +39 -0
index 27ea7eca73e5..8d2a47e9a854 100644
@@ -65,6 +65,12 @@ patternProperties:
           5:  MSM8996SG, speedbin 1
           6:  MSM8996SG, speedbin 2
           7-31:  unused
+
+          Bitmap for IPQ806X SoC:
+          0:  IPQ8062
+          1:  IPQ8064/IPQ8066/IPQ8068
+          2:  IPQ8065/IPQ8069
+          3-31:  unused
         enum: [0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7,
                0x9, 0xd, 0xe, 0xf,
                0x10, 0x20, 0x30, 0x70]

@@ -73,6 +79,23 @@ patternProperties:
 
       required-opps: true
 
+    patternProperties:
+      '^opp-microvolt-speed[0-9]+-pvs[0-9]+$':
+        description: |
+          Named opp-microvolt property following the same generic
+          binding for named opp-microvolt.
+
+          The correct voltage range is selected based on the values
+          in the efuse for the speed and the pvs (power variable
+          scaling).
+        minItems: 1
+        maxItems: 4 # Up to 4 regulators: Core, Mem, Dig and HFPLL
+        items:
+          items:
+            - description: nominal voltage
+            - description: minimum voltage
+            - description: maximum voltage
+
     required:
       - opp-hz
 

@@ -258,6 +281,22 @@ examples:
             };
         };
 
+        /* Dummy opp table to give example for named opp-microvolt */
+        opp-table-2 {
+            compatible = "operating-points-v2-kryo-cpu";
+            nvmem-cells = <&speedbin_efuse>;
+
+            opp-384000000 {
+                opp-hz = /bits/ 64 <384000000>;
+                opp-microvolt-speed0-pvs0 = <1000000 950000 1050000>;
+                opp-microvolt-speed0-pvs1 = <925000 878750 971250>;
+                opp-microvolt-speed0-pvs2 = <875000 831250 918750>;
+                opp-microvolt-speed0-pvs3 = <800000 760000 840000>;
+                opp-supported-hw = <0x7>;
+                clock-latency-ns = <100000>;
+            };
+        };
+
         smem {
             compatible = "qcom,smem";
             memory-region = <&smem_mem>;
Signature · 1 line
2.41.0
maintainer Reviewed-by on v5 2/4
#2 lore raw

Re: [PATCH v5 2/4] dt-bindings: opp: opp-v2-kryo-cpu: Document named opp-microvolt property

Quoted 14 lines · Robert Marko

From: Christian Marangi <[email protected]>

Document named opp-microvolt property for opp-v2-kryo-cpu schema. This property is used to declare multiple voltage ranges selected on the different values read from efuses. The selection is done based on the speed pvs values and the named opp-microvolt property is selected by the qcom-cpufreq-nvmem driver.

Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Robert Marko <[email protected]>

---
Changes v5:
* Fix typo in opp items

Quoted 8 lines

Changes v4:
* Address comments from Rob (meaning of pvs, drop of driver specific info, drop of legacy single voltage OPP, better specify max regulators supported)

 .../bindings/opp/opp-v2-kryo-cpu.yaml         | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
Reviewed-by Rob Herring <[email protected]>
#3 lore raw

Re: [PATCH v5 2/4] dt-bindings: opp: opp-v2-kryo-cpu: Document named opp-microvolt property

In reply to Robert Marko
Quoted 43 lines · Robert Marko

From: Christian Marangi <[email protected]>

Document named opp-microvolt property for opp-v2-kryo-cpu schema. This property is used to declare multiple voltage ranges selected on the different values read from efuses. The selection is done based on the speed pvs values and the named opp-microvolt property is selected by the qcom-cpufreq-nvmem driver.

Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Robert Marko <[email protected]>

---
Changes v5:
* Fix typo in opp items

Changes v4:
* Address comments from Rob (meaning of pvs, drop of driver specific info, drop of legacy single voltage OPP, better specify max regulators supported)

 .../bindings/opp/opp-v2-kryo-cpu.yaml         | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml +6 -0
index 27ea7eca73e5..8d2a47e9a854 100644
@@ -65,6 +65,12 @@ patternProperties:
           5:  MSM8996SG, speedbin 1
           6:  MSM8996SG, speedbin 2
           7-31:  unused
+
+          Bitmap for IPQ806X SoC:
+          0:  IPQ8062
+          1:  IPQ8064/IPQ8066/IPQ8068
+          2:  IPQ8065/IPQ8069
+          3-31:  unused
         enum: [0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7,
                0x9, 0xd, 0xe, 0xf,
                0x10, 0x20, 0x30, 0x70]

@@ -73,6 +79,23 @@ patternProperties:
 
       required-opps: true

Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml patternProperties: · line 90

Quoted 10 lines
Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml +9 -0
 
+    patternProperties:
+      '^opp-microvolt-speed[0-9]+-pvs[0-9]+$':
+        description: |
+          Named opp-microvolt property following the same generic
+          binding for named opp-microvolt.
+
+          The correct voltage range is selected based on the values
+          in the efuse for the speed and the pvs (power variable
+          scaling).

I suppose that simple 'true' schema should be enough since this is already mostly described in opp/opp-v2-base.yaml

Quoted 37 lines
Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml +24 -1
+        minItems: 1
+        maxItems: 4 # Up to 4 regulators: Core, Mem, Dig and HFPLL
+        items:
+          items:
+            - description: nominal voltage
+            - description: minimum voltage
+            - description: maximum voltage
+
     required:
       - opp-hz
 

@@ -258,6 +281,22 @@ examples:
             };
         };
 
+        /* Dummy opp table to give example for named opp-microvolt */
+        opp-table-2 {
+            compatible = "operating-points-v2-kryo-cpu";
+            nvmem-cells = <&speedbin_efuse>;
+
+            opp-384000000 {
+                opp-hz = /bits/ 64 <384000000>;
+                opp-microvolt-speed0-pvs0 = <1000000 950000 1050000>;
+                opp-microvolt-speed0-pvs1 = <925000 878750 971250>;
+                opp-microvolt-speed0-pvs2 = <875000 831250 918750>;
+                opp-microvolt-speed0-pvs3 = <800000 760000 840000>;
+                opp-supported-hw = <0x7>;
+                clock-latency-ns = <100000>;
+            };
+        };
+
         smem {
             compatible = "qcom,smem";
             memory-region = <&smem_mem>;
--

2.41.0

Signature · 2 lines
With best wishes
Dmitry
#4 lore raw

Re: [PATCH v5 2/4] dt-bindings: opp: opp-v2-kryo-cpu: Document named opp-microvolt property

Quoted 45 lines · Dmitry Baryshkov
Quoted 44 lines · Robert Marko

From: Christian Marangi <[email protected]>

Document named opp-microvolt property for opp-v2-kryo-cpu schema. This property is used to declare multiple voltage ranges selected on the different values read from efuses. The selection is done based on the speed pvs values and the named opp-microvolt property is selected by the qcom-cpufreq-nvmem driver.

Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Robert Marko <[email protected]>

---
Changes v5:
* Fix typo in opp items

Changes v4:
* Address comments from Rob (meaning of pvs, drop of driver specific info, drop of legacy single voltage OPP, better specify max regulators supported)

 .../bindings/opp/opp-v2-kryo-cpu.yaml         | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml +6 -0
index 27ea7eca73e5..8d2a47e9a854 100644
@@ -65,6 +65,12 @@ patternProperties:
           5:  MSM8996SG, speedbin 1
           6:  MSM8996SG, speedbin 2
           7-31:  unused
+
+          Bitmap for IPQ806X SoC:
+          0:  IPQ8062
+          1:  IPQ8064/IPQ8066/IPQ8068
+          2:  IPQ8065/IPQ8069
+          3-31:  unused
         enum: [0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7,
                0x9, 0xd, 0xe, 0xf,
                0x10, 0x20, 0x30, 0x70]

@@ -73,6 +79,23 @@ patternProperties:
 
       required-opps: true
Quoted 13 lines

Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml patternProperties: · line 90

Quoted 9 lines
Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml +9 -0
+    patternProperties:
+      '^opp-microvolt-speed[0-9]+-pvs[0-9]+$':
+        description: |
+          Named opp-microvolt property following the same generic
+          binding for named opp-microvolt.
+
+          The correct voltage range is selected based on the values
+          in the efuse for the speed and the pvs (power variable
+          scaling).

I suppose that simple 'true' schema should be enough since this is already mostly described in opp/opp-v2-base.yaml

Mhhh an example of the following implementation?

Quoted 42 lines
Quoted 37 lines
Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml +24 -1
+        minItems: 1
+        maxItems: 4 # Up to 4 regulators: Core, Mem, Dig and HFPLL
+        items:
+          items:
+            - description: nominal voltage
+            - description: minimum voltage
+            - description: maximum voltage
+
     required:
       - opp-hz
 

@@ -258,6 +281,22 @@ examples:
             };
         };
 
+        /* Dummy opp table to give example for named opp-microvolt */
+        opp-table-2 {
+            compatible = "operating-points-v2-kryo-cpu";
+            nvmem-cells = <&speedbin_efuse>;
+
+            opp-384000000 {
+                opp-hz = /bits/ 64 <384000000>;
+                opp-microvolt-speed0-pvs0 = <1000000 950000 1050000>;
+                opp-microvolt-speed0-pvs1 = <925000 878750 971250>;
+                opp-microvolt-speed0-pvs2 = <875000 831250 918750>;
+                opp-microvolt-speed0-pvs3 = <800000 760000 840000>;
+                opp-supported-hw = <0x7>;
+                clock-latency-ns = <100000>;
+            };
+        };
+
         smem {
             compatible = "qcom,smem";
             memory-region = <&smem_mem>;
--

2.41.0

Signature · 2 lines
With best wishes
Dmitry
Signature · 1 line
	Ansuel
maintainer
#5 lore raw

Re: [PATCH v5 2/4] dt-bindings: opp: opp-v2-kryo-cpu: Document named opp-microvolt property

Quoted 7 lines · Christian Marangi
Quoted 4 lines · Dmitry Baryshkov

On Sat, 30 Sept 2023 at 13:22, Robert Marko <[email protected]> wrote:
I suppose that simple 'true' schema should be enough since this is already mostly described in opp/opp-v2-base.yaml

Mhhh an example of the following implementation?

Documentation/devicetree/bindings/opp/allwinner,sun50i-h6-operating-points.yaml

Signature · 1 line
viresh
maintainer
#6 lore raw

Re: [PATCH v5 2/4] dt-bindings: opp: opp-v2-kryo-cpu: Document named opp-microvolt property

In reply to Dmitry Baryshkov
Quoted 2 lines · Dmitry Baryshkov

I suppose that simple 'true' schema should be enough since this is already mostly described in opp/opp-v2-base.yaml

Dmitry, Konrad,

Can you guys review the other patches in the series, since you are also actively working on this driver ?

Signature · 1 line
viresh
author
#7 lore raw

PATCH v5 3/4 cpufreq: qcom-nvmem: add support for IPQ8064

In reply to Robert Marko

From: Christian Marangi <[email protected]>

IPQ8064 comes in 3 families:
* IPQ8062 up to 1.0GHz
* IPQ8064/IPQ8066/IPQ8068 up to 1.4GHz
* IPQ8065/IPQ8069 up to 1.7Ghz

So, in order to be able to support one OPP table, add support for IPQ8064 family based of SMEM SoC ID-s and correctly set the version so opp-supported-hw can be correctly used.

Bit are set with the following logic:
* IPQ8062 BIT 0
* IPQ8064/IPQ8066/IPQ8068 BIT 1
* IPQ8065/IPQ8069 BIT 2

speed is never fused, only pvs values are fused.

IPQ806x SoC doesn't have pvs_version so we drop and we use the new pattern:
opp-microvolt-speed0-pvs<PSV_VALUE>

Example:
- for ipq8062 psv2
opp-microvolt-speed0-pvs2 = < 925000 878750 971250>

Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Robert Marko <[email protected]>

---
Changes in v4:
* Free speedbin in case of an error

Changes in v3:
* Use enum for SoC version
* Dont evaluate speed as its not fused, only pvs

Changes in v2:
* Include IPQ8064 support

 drivers/cpufreq/qcom-cpufreq-nvmem.c | 68 +++++++++++++++++++++++++++-
 1 file changed, 67 insertions(+), 1 deletion(-)
drivers/cpufreq/qcom-cpufreq-nvmem.c +67 -1
index ba9e1d60e5b5..3d93b511db86 100644
@@ -35,6 +35,12 @@ enum ipq8074_versions {
 	IPQ8074_ACORN_VERSION,
 };
 
+enum ipq806x_versions {
+	IPQ8062_VERSION = 0,
+	IPQ8064_VERSION,
+	IPQ8065_VERSION,
+};
+
 struct qcom_cpufreq_drv;
 
 struct qcom_cpufreq_match_data {

@@ -208,6 +214,62 @@ static int qcom_cpufreq_krait_name_version(struct device *cpu_dev,
 	return ret;
 }
 
+static int qcom_cpufreq_ipq8064_name_version(struct device *cpu_dev,
+					     struct nvmem_cell *speedbin_nvmem,
+					     char **pvs_name,
+					     struct qcom_cpufreq_drv *drv)
+{
+	int speed = 0, pvs = 0, pvs_ver = 0;
+	int msm_id, ret = 0;
+	u8 *speedbin;
+	size_t len;
+
+	speedbin = nvmem_cell_read(speedbin_nvmem, &len);
+
+	if (IS_ERR(speedbin))
+		return PTR_ERR(speedbin);
+
+	if (len != 4) {
+		dev_err(cpu_dev, "Unable to read nvmem data. Defaulting to 0!\n");
+		kfree(speedbin);
+		return -ENODEV;
+	}
+
+	get_krait_bin_format_a(cpu_dev, &speed, &pvs, &pvs_ver, speedbin);
+
+	ret = qcom_smem_get_soc_id(&msm_id);
+	if (ret)
+		return ret;
+
+	switch (msm_id) {
+	case QCOM_ID_IPQ8062:
+		drv->versions = BIT(IPQ8062_VERSION);
+		break;
+	case QCOM_ID_IPQ8064:
+	case QCOM_ID_IPQ8066:
+	case QCOM_ID_IPQ8068:
+		drv->versions = BIT(IPQ8064_VERSION);
+		break;
+	case QCOM_ID_IPQ8065:
+	case QCOM_ID_IPQ8069:
+		drv->versions = BIT(IPQ8065_VERSION);
+		break;
+	default:
+		dev_err(cpu_dev,
+			"SoC ID %u is not part of IPQ8064 family, limiting to 1.0GHz!\n",
+			msm_id);
+		drv->versions = BIT(IPQ8062_VERSION);
+		break;
+	}
+
+	/* IPQ8064 speed is never fused. Only pvs values are fused. */
+	snprintf(*pvs_name, sizeof("speedXX-pvsXX"), "speed%d-pvs%d",
+		 speed, pvs);
+
+	kfree(speedbin);
+	return ret;
+}
+
 static int qcom_cpufreq_ipq8074_name_version(struct device *cpu_dev,
 					     struct nvmem_cell *speedbin_nvmem,
 					     char **pvs_name,

@@ -257,6 +319,10 @@ static const struct qcom_cpufreq_match_data match_data_qcs404 = {
 	.genpd_names = qcs404_genpd_names,
 };
 
+static const struct qcom_cpufreq_match_data match_data_ipq8064 = {
+	.get_version = qcom_cpufreq_ipq8064_name_version,
+};
+
 static const struct qcom_cpufreq_match_data match_data_ipq8074 = {
 	.get_version = qcom_cpufreq_ipq8074_name_version,
 };

@@ -403,7 +469,7 @@ static const struct of_device_id qcom_cpufreq_match_list[] __initconst = {
 	{ .compatible = "qcom,apq8096", .data = &match_data_kryo },
 	{ .compatible = "qcom,msm8996", .data = &match_data_kryo },
 	{ .compatible = "qcom,qcs404", .data = &match_data_qcs404 },
-	{ .compatible = "qcom,ipq8064", .data = &match_data_krait },
+	{ .compatible = "qcom,ipq8064", .data = &match_data_ipq8064 },
 	{ .compatible = "qcom,ipq8074", .data = &match_data_ipq8074 },
 	{ .compatible = "qcom,apq8064", .data = &match_data_krait },
 	{ .compatible = "qcom,msm8974", .data = &match_data_krait },
Signature · 1 line
2.41.0
#8 lore raw

Re: [PATCH v5 3/4] cpufreq: qcom-nvmem: add support for IPQ8064

Quoted 29 lines · Robert Marko

From: Christian Marangi <[email protected]>

IPQ8064 comes in 3 families:
* IPQ8062 up to 1.0GHz
* IPQ8064/IPQ8066/IPQ8068 up to 1.4GHz
* IPQ8065/IPQ8069 up to 1.7Ghz

So, in order to be able to support one OPP table, add support for IPQ8064 family based of SMEM SoC ID-s and correctly set the version so opp-supported-hw can be correctly used.

Bit are set with the following logic:
* IPQ8062 BIT 0
* IPQ8064/IPQ8066/IPQ8068 BIT 1
* IPQ8065/IPQ8069 BIT 2

speed is never fused, only pvs values are fused.

IPQ806x SoC doesn't have pvs_version so we drop and we use the new pattern:
opp-microvolt-speed0-pvs<PSV_VALUE>

Example:
- for ipq8062 psv2
opp-microvolt-speed0-pvs2 = < 925000 878750 971250>

Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Robert Marko <[email protected]>

---

[...]

drivers/cpufreq/qcom-cpufreq-nvmem.c static int qcom_cpufreq_krait_name_version(struct device *cpu_dev, · line 229

Quoted 9 lines
drivers/cpufreq/qcom-cpufreq-nvmem.c +9 -0
+{
+	int speed = 0, pvs = 0, pvs_ver = 0;
+	int msm_id, ret = 0;
+	u8 *speedbin;
+	size_t len;
+
+	speedbin = nvmem_cell_read(speedbin_nvmem, &len);
+
+	if (IS_ERR(speedbin))

The stray newline above this line triggers my OCD :D

drivers/cpufreq/qcom-cpufreq-nvmem.c static int qcom_cpufreq_krait_name_version(struct device *cpu_dev, · line 242

Quoted 13 lines
drivers/cpufreq/qcom-cpufreq-nvmem.c +13 -0
+		return PTR_ERR(speedbin);
+
+	if (len != 4) {
+		dev_err(cpu_dev, "Unable to read nvmem data. Defaulting to 0!\n");
+		kfree(speedbin);
+		return -ENODEV;
+	}
+
+	get_krait_bin_format_a(cpu_dev, &speed, &pvs, &pvs_ver, speedbin);
+
+	ret = qcom_smem_get_soc_id(&msm_id);
+	if (ret)
+		return ret;

speedbin leaks here

you can free it right after the get_krait.. call

Quoted 15 lines
drivers/cpufreq/qcom-cpufreq-nvmem.c +15 -0
+
+	switch (msm_id) {
+	case QCOM_ID_IPQ8062:
+		drv->versions = BIT(IPQ8062_VERSION);
+		break;
+	case QCOM_ID_IPQ8064:
+	case QCOM_ID_IPQ8066:
+	case QCOM_ID_IPQ8068:
+		drv->versions = BIT(IPQ8064_VERSION);
+		break;
+	case QCOM_ID_IPQ8065:
+	case QCOM_ID_IPQ8069:
+		drv->versions = BIT(IPQ8065_VERSION);
+		break;
+	default:

drivers/cpufreq/qcom-cpufreq-nvmem.c static int qcom_cpufreq_krait_name_version(struct device *cpu_dev, · line 267

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-nvmem.c +10 -0
+		dev_err(cpu_dev,
+			"SoC ID %u is not part of IPQ8064 family, limiting to 1.0GHz!\n",
+			msm_id);
+		drv->versions = BIT(IPQ8062_VERSION);
+		break;
+	}
+
+	/* IPQ8064 speed is never fused. Only pvs values are fused. */
+	snprintf(*pvs_name, sizeof("speedXX-pvsXX"), "speed%d-pvs%d",
+		 speed, pvs);

Then drop the format for speed and just throw in a zero!

[...]

drivers/cpufreq/qcom-cpufreq-nvmem.c static const struct of_device_id qcom_cpufreq_match_list[] __initconst = { · line 472

Quoted 2 lines
drivers/cpufreq/qcom-cpufreq-nvmem.c +1 -1
-	{ .compatible = "qcom,ipq8064", .data = &match_data_krait },
+	{ .compatible = "qcom,ipq8064", .data = &match_data_ipq8064 },

This change demands a Fixes tag, because you're essentially saying "the support for this SoC was supposedly there, but it could have never worked and was broken all along".

Konrad

#9 lore raw

Re: [PATCH v5 3/4] cpufreq: qcom-nvmem: add support for IPQ8064

Quoted 88 lines · Konrad Dybcio
Quoted 29 lines · Robert Marko

From: Christian Marangi <[email protected]>

IPQ8064 comes in 3 families:
* IPQ8062 up to 1.0GHz
* IPQ8064/IPQ8066/IPQ8068 up to 1.4GHz
* IPQ8065/IPQ8069 up to 1.7Ghz

So, in order to be able to support one OPP table, add support for IPQ8064 family based of SMEM SoC ID-s and correctly set the version so opp-supported-hw can be correctly used.

Bit are set with the following logic:
* IPQ8062 BIT 0
* IPQ8064/IPQ8066/IPQ8068 BIT 1
* IPQ8065/IPQ8069 BIT 2

speed is never fused, only pvs values are fused.

IPQ806x SoC doesn't have pvs_version so we drop and we use the new pattern:
opp-microvolt-speed0-pvs<PSV_VALUE>

Example:
- for ipq8062 psv2
opp-microvolt-speed0-pvs2 = < 925000 878750 971250>

Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Robert Marko <[email protected]>

---

[...]

drivers/cpufreq/qcom-cpufreq-nvmem.c static int qcom_cpufreq_krait_name_version(struct device *cpu_dev, · line 229

Quoted 9 lines
drivers/cpufreq/qcom-cpufreq-nvmem.c +9 -0
+{
+	int speed = 0, pvs = 0, pvs_ver = 0;
+	int msm_id, ret = 0;
+	u8 *speedbin;
+	size_t len;
+
+	speedbin = nvmem_cell_read(speedbin_nvmem, &len);
+
+	if (IS_ERR(speedbin))

The stray newline above this line triggers my OCD :D

drivers/cpufreq/qcom-cpufreq-nvmem.c static int qcom_cpufreq_krait_name_version(struct device *cpu_dev, · line 242

Quoted 13 lines
drivers/cpufreq/qcom-cpufreq-nvmem.c +13 -0
+		return PTR_ERR(speedbin);
+
+	if (len != 4) {
+		dev_err(cpu_dev, "Unable to read nvmem data. Defaulting to 0!\n");
+		kfree(speedbin);
+		return -ENODEV;
+	}
+
+	get_krait_bin_format_a(cpu_dev, &speed, &pvs, &pvs_ver, speedbin);
+
+	ret = qcom_smem_get_soc_id(&msm_id);
+	if (ret)
+		return ret;

speedbin leaks here

you can free it right after the get_krait.. call

Quoted 15 lines
drivers/cpufreq/qcom-cpufreq-nvmem.c +15 -0
+
+	switch (msm_id) {
+	case QCOM_ID_IPQ8062:
+		drv->versions = BIT(IPQ8062_VERSION);
+		break;
+	case QCOM_ID_IPQ8064:
+	case QCOM_ID_IPQ8066:
+	case QCOM_ID_IPQ8068:
+		drv->versions = BIT(IPQ8064_VERSION);
+		break;
+	case QCOM_ID_IPQ8065:
+	case QCOM_ID_IPQ8069:
+		drv->versions = BIT(IPQ8065_VERSION);
+		break;
+	default:

drivers/cpufreq/qcom-cpufreq-nvmem.c static int qcom_cpufreq_krait_name_version(struct device *cpu_dev, · line 267

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-nvmem.c +10 -0
+		dev_err(cpu_dev,
+			"SoC ID %u is not part of IPQ8064 family, limiting to 1.0GHz!\n",
+			msm_id);
+		drv->versions = BIT(IPQ8062_VERSION);
+		break;
+	}
+
+	/* IPQ8064 speed is never fused. Only pvs values are fused. */
+	snprintf(*pvs_name, sizeof("speedXX-pvsXX"), "speed%d-pvs%d",
+		 speed, pvs);

Then drop the format for speed and just throw in a zero!

Quoted 8 lines

[...]

drivers/cpufreq/qcom-cpufreq-nvmem.c static const struct of_device_id qcom_cpufreq_match_list[] __initconst = { · line 472

Quoted 2 lines
drivers/cpufreq/qcom-cpufreq-nvmem.c +1 -1
-	{ .compatible = "qcom,ipq8064", .data = &match_data_krait },
+	{ .compatible = "qcom,ipq8064", .data = &match_data_ipq8064 },

This change demands a Fixes tag, because you're essentially saying "the support for this SoC was supposedly there, but it could have never worked and was broken all along".

Mhhh actually no. We are just changing the opp binding and introducing hardcoded versions. But the thing worked and actually it's what was used before this change in openwrt. Also current ipq806x dtsi doesn't have any opp definition so no regression there. (and also 99% downstream either use openwrt or use qcom sdk where this implementation is not used at all)

Given these thing should we still add a fixes tag referencing the commit that introduced the compatible for qcom,ipq8064? It's quite problematic as this depends on qcom_smem_get_soc_id().

Signature · 1 line
	Ansuel
#10 lore raw

Re: [PATCH v5 3/4] cpufreq: qcom-nvmem: add support for IPQ8064

Quoted 105 lines · Christian Marangi
Quoted 88 lines · Konrad Dybcio
Quoted 29 lines · Robert Marko

From: Christian Marangi <[email protected]>

IPQ8064 comes in 3 families:
* IPQ8062 up to 1.0GHz
* IPQ8064/IPQ8066/IPQ8068 up to 1.4GHz
* IPQ8065/IPQ8069 up to 1.7Ghz

So, in order to be able to support one OPP table, add support for IPQ8064 family based of SMEM SoC ID-s and correctly set the version so opp-supported-hw can be correctly used.

Bit are set with the following logic:
* IPQ8062 BIT 0
* IPQ8064/IPQ8066/IPQ8068 BIT 1
* IPQ8065/IPQ8069 BIT 2

speed is never fused, only pvs values are fused.

IPQ806x SoC doesn't have pvs_version so we drop and we use the new pattern:
opp-microvolt-speed0-pvs<PSV_VALUE>

Example:
- for ipq8062 psv2

    opp-microvolt-speed0-pvs2 = < 925000 878750 971250>
Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Robert Marko <[email protected]>

---

[...]

drivers/cpufreq/qcom-cpufreq-nvmem.c static int qcom_cpufreq_krait_name_version(struct device *cpu_dev, · line 229

Quoted 9 lines
drivers/cpufreq/qcom-cpufreq-nvmem.c +9 -0
+{
+	int speed = 0, pvs = 0, pvs_ver = 0;
+	int msm_id, ret = 0;
+	u8 *speedbin;
+	size_t len;
+
+	speedbin = nvmem_cell_read(speedbin_nvmem, &len);
+
+	if (IS_ERR(speedbin))

The stray newline above this line triggers my OCD :D

drivers/cpufreq/qcom-cpufreq-nvmem.c static int qcom_cpufreq_krait_name_version(struct device *cpu_dev, · line 242

Quoted 13 lines
drivers/cpufreq/qcom-cpufreq-nvmem.c +13 -0
+		return PTR_ERR(speedbin);
+
+	if (len != 4) {
+		dev_err(cpu_dev, "Unable to read nvmem data. Defaulting to 0!\n");
+		kfree(speedbin);
+		return -ENODEV;
+	}
+
+	get_krait_bin_format_a(cpu_dev, &speed, &pvs, &pvs_ver, speedbin);
+
+	ret = qcom_smem_get_soc_id(&msm_id);
+	if (ret)
+		return ret;

speedbin leaks here

you can free it right after the get_krait.. call

Quoted 15 lines
drivers/cpufreq/qcom-cpufreq-nvmem.c +15 -0
+
+	switch (msm_id) {
+	case QCOM_ID_IPQ8062:
+		drv->versions = BIT(IPQ8062_VERSION);
+		break;
+	case QCOM_ID_IPQ8064:
+	case QCOM_ID_IPQ8066:
+	case QCOM_ID_IPQ8068:
+		drv->versions = BIT(IPQ8064_VERSION);
+		break;
+	case QCOM_ID_IPQ8065:
+	case QCOM_ID_IPQ8069:
+		drv->versions = BIT(IPQ8065_VERSION);
+		break;
+	default:

drivers/cpufreq/qcom-cpufreq-nvmem.c static int qcom_cpufreq_krait_name_version(struct device *cpu_dev, · line 267

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-nvmem.c +10 -0
+		dev_err(cpu_dev,
+			"SoC ID %u is not part of IPQ8064 family, limiting to 1.0GHz!\n",
+			msm_id);
+		drv->versions = BIT(IPQ8062_VERSION);
+		break;
+	}
+
+	/* IPQ8064 speed is never fused. Only pvs values are fused. */
+	snprintf(*pvs_name, sizeof("speedXX-pvsXX"), "speed%d-pvs%d",
+		 speed, pvs);

Then drop the format for speed and just throw in a zero!

Quoted 8 lines

[...]

drivers/cpufreq/qcom-cpufreq-nvmem.c static const struct of_device_id qcom_cpufreq_match_list[] __initconst = { · line 472

Quoted 2 lines
drivers/cpufreq/qcom-cpufreq-nvmem.c +1 -1
-	{ .compatible = "qcom,ipq8064", .data = &match_data_krait },
+	{ .compatible = "qcom,ipq8064", .data = &match_data_ipq8064 },

This change demands a Fixes tag, because you're essentially saying "the support for this SoC was supposedly there, but it could have never worked and was broken all along".

Mhhh actually no. We are just changing the opp binding and introducing hardcoded versions. But the thing worked and actually it's what was used before this change in openwrt. Also current ipq806x dtsi doesn't have any opp definition so no regression there. (and also 99% downstream either use openwrt or use qcom sdk where this implementation is not used at all)

Quoted 3 lines

Given these thing should we still add a fixes tag referencing the commit that introduced the compatible for qcom,ipq8064? It's quite problematic as this depends on qcom_smem_get_soc_id().

Fixes only hints auto backports, you shouldn't be worried about putting fixes on commits that fix bugs.

I see this as a "didnt work" -> "works" commit, which in my eyes qualifies as a fix.

Konrad

author
#11 lore raw

PATCH v5 4/4 ARM: dts: qcom: ipq8064: Add CPU OPP table

In reply to Robert Marko

From: Christian Marangi <[email protected]>

Add CPU OPP table for IPQ8062, IPQ8064 and IPQ8065 SoC. Use opp-supported-hw binding to correctly enable and disable the frequency as IPQ8062 supports up to 1.0Ghz, IPQ8064 supports up to 1.4GHz with 1.2GHz as an additional frequency and IPQ8065 supports 1.7GHZ but doesn't have 1.2GHZ frequency and has to be disabled.

Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Robert Marko <[email protected]>

---
Changes v4:
* Add OPP DTS patch for IPQ8064

 arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi | 30 +++++++++++
 arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi | 67 ++++++++++++++++++++++++
 arch/arm/boot/dts/qcom/qcom-ipq8065.dtsi | 65 +++++++++++++++++++++++
 3 files changed, 162 insertions(+)
arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi +30 -0
index 5d3ebd3e2e51..72d9782c3d6f 100644
@@ -6,3 +6,33 @@ / {
 	model = "Qualcomm Technologies, Inc. IPQ8062";
 	compatible = "qcom,ipq8062", "qcom,ipq8064";
 };
+
+&opp_table_cpu {
+	opp-384000000 {
+		opp-microvolt-speed0-pvs0 = <1000000 950000 1050000>;
+		opp-microvolt-speed0-pvs1 = <925000 878750 971250>;
+		opp-microvolt-speed0-pvs2 = <875000 831250 918750>;
+		opp-microvolt-speed0-pvs3 = <800000 760000 840000>;
+	};
+
+	opp-600000000 {
+		opp-microvolt-speed0-pvs0 = <1050000 997500 1102500>;
+		opp-microvolt-speed0-pvs1 = <975000 926250 1023750>;
+		opp-microvolt-speed0-pvs2 = <925000 878750 971250>;
+		opp-microvolt-speed0-pvs3 = <850000 807500 892500>;
+	};
+
+	opp-800000000 {
+		opp-microvolt-speed0-pvs0 = <1100000 1045000 1155000>;
+		opp-microvolt-speed0-pvs1 = <1025000 973750 1076250>;
+		opp-microvolt-speed0-pvs2 = <995000 945250 1044750>;
+		opp-microvolt-speed0-pvs3 = <900000 855000 945000>;
+	};
+
+	opp-1000000000 {
+		opp-microvolt-speed0-pvs0 = <1150000 1092500 1207500>;
+		opp-microvolt-speed0-pvs1 = <1075000 1021250 1128750>;
+		opp-microvolt-speed0-pvs2 = <1025000 973750 1076250>;
+		opp-microvolt-speed0-pvs3 = <950000 902500 997500>;
+	};
+};
arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi +67 -0

85 lines not on this page · raw message

arch/arm/boot/dts/qcom/qcom-ipq8065.dtsi +65 -0

68 lines not on this page · raw message

Signature · 1 line
2.41.0
#12 lore raw

Re: [PATCH v5 4/4] ARM: dts: qcom: ipq8064: Add CPU OPP table

Quoted 11 lines · Robert Marko

From: Christian Marangi <[email protected]>

Add CPU OPP table for IPQ8062, IPQ8064 and IPQ8065 SoC. Use opp-supported-hw binding to correctly enable and disable the frequency as IPQ8062 supports up to 1.0Ghz, IPQ8064 supports up to 1.4GHz with 1.2GHz as an additional frequency and IPQ8065 supports 1.7GHZ but doesn't have 1.2GHZ frequency and has to be disabled.

Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Robert Marko <[email protected]>

---

Christian/Robert, can you provide a downstream source for this?

Konrad

#13 lore raw

Re: [PATCH v5 4/4] ARM: dts: qcom: ipq8064: Add CPU OPP table

Quoted 11 lines · Konrad Dybcio
Quoted 8 lines · Robert Marko

From: Christian Marangi <[email protected]>

Add CPU OPP table for IPQ8062, IPQ8064 and IPQ8065 SoC. Use opp-supported-hw binding to correctly enable and disable the frequency as IPQ8062 supports up to 1.0Ghz, IPQ8064 supports up to 1.4GHz with 1.2GHz as an additional frequency and IPQ8065 supports 1.7GHZ but doesn't have 1.2GHZ frequency and has to be disabled.

Quoted 5 lines
Quoted 3 lines
Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Robert Marko <[email protected]>

---

Christian/Robert, can you provide a downstream source for this?

Sure, consider that everything is with +/-5%.

Qsdk ships with these values but I will link a more secure source that is from a very old qsdk source where march-msm was still used instead of dt.

Here the source [1]. Confirmed by internal verification and also other qsdk. At first view you might be scared by confusion but...

- _lite = ipq8062
- nothing = ipq8064
- tn_3 = ipq8065

and the psv are both in number and slow, nominal, fast and faster and they all reflect efuse values.

There is on the left frequency and on the right voltage.

[1] https://github.com/Getnear/R7800/blob/master/git_home/linux.git/sourcecode/arch/arm/mach-msm/acpuclock-ipq806x.c

Signature · 1 line
	Ansuel
#14 lore raw

Re: [PATCH v5 4/4] ARM: dts: qcom: ipq8064: Add CPU OPP table

Quoted 12 lines · Christian Marangi
Quoted 11 lines · Konrad Dybcio
Quoted 8 lines · Robert Marko

From: Christian Marangi <[email protected]>

Add CPU OPP table for IPQ8062, IPQ8064 and IPQ8065 SoC. Use opp-supported-hw binding to correctly enable and disable the frequency as IPQ8062 supports up to 1.0Ghz, IPQ8064 supports up to 1.4GHz with 1.2GHz as an additional frequency and IPQ8065 supports 1.7GHZ but doesn't have 1.2GHZ frequency and has to be disabled.

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Robert Marko <[email protected]>

---

Christian/Robert, can you provide a downstream source for this?

Sure, consider that everything is with +/-5%.

Hm, so you're e.g. putting ipq8062 384MHz voltage for PVS3 equal to 0.95*800000 = 760000, but I'm not sure if it's a good idea?

The comment in downstream:

"These are based on +/-5% Margin on the VDD_APCx that is advertised in our Datasheet across Temperature"

suggests this is already not very accurate, and betting that the lower threshold works on all chips is probably not the best idea.

Konrad

#15 lore raw

Re: [PATCH v5 4/4] ARM: dts: qcom: ipq8064: Add CPU OPP table

Quoted 25 lines · Konrad Dybcio
Quoted 12 lines · Christian Marangi
Quoted 11 lines · Konrad Dybcio
Quoted 8 lines · Robert Marko

From: Christian Marangi <[email protected]>

Add CPU OPP table for IPQ8062, IPQ8064 and IPQ8065 SoC. Use opp-supported-hw binding to correctly enable and disable the frequency as IPQ8062 supports up to 1.0Ghz, IPQ8064 supports up to 1.4GHz with 1.2GHz as an additional frequency and IPQ8065 supports 1.7GHZ but doesn't have 1.2GHZ frequency and has to be disabled.

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Robert Marko <[email protected]>

---

Christian/Robert, can you provide a downstream source for this?

Sure, consider that everything is with +/-5%.

Hm, so you're e.g. putting ipq8062 384MHz voltage for PVS3 equal to 0.95*800000 = 760000, but I'm not sure if it's a good idea?

Quoted 8 lines

The comment in downstream:

"These are based on +/-5% Margin on the VDD_APCx that is advertised in our Datasheet across Temperature"

suggests this is already not very accurate, and betting that the lower threshold works on all chips is probably not the best idea.

Consider that everything is driven by the rpm. The original qsdk used the same approach of taking the value, apply +-5% and pass it as a voltage triplet to the rpm regulator. Also the driver have ranges so it autodecide the best voltage in the range of the voltage triplet based on the one supported by the regulator. Normally the normal voltage is always used.

Signature · 1 line
	Ansuel
#16 lore raw

Re: [PATCH v5 4/4] ARM: dts: qcom: ipq8064: Add CPU OPP table

Quoted 35 lines · Christian Marangi
Quoted 25 lines · Konrad Dybcio
Quoted 12 lines · Christian Marangi
Quoted 11 lines · Konrad Dybcio
Quoted 8 lines · Robert Marko

From: Christian Marangi <[email protected]>

Add CPU OPP table for IPQ8062, IPQ8064 and IPQ8065 SoC. Use opp-supported-hw binding to correctly enable and disable the frequency as IPQ8062 supports up to 1.0Ghz, IPQ8064 supports up to 1.4GHz with 1.2GHz as an additional frequency and IPQ8065 supports 1.7GHZ but doesn't have 1.2GHZ frequency and has to be disabled.

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Robert Marko <[email protected]>

---

Christian/Robert, can you provide a downstream source for this?

Sure, consider that everything is with +/-5%.

Hm, so you're e.g. putting ipq8062 384MHz voltage for PVS3 equal to 0.95*800000 = 760000, but I'm not sure if it's a good idea?

Quoted 8 lines

The comment in downstream:

"These are based on +/-5% Margin on the VDD_APCx that is advertised in our Datasheet across Temperature"

suggests this is already not very accurate, and betting that the lower threshold works on all chips is probably not the best idea.

Quoted 6 lines

Consider that everything is driven by the rpm. The original qsdk used the same approach of taking the value, apply +-5% and pass it as a voltage triplet to the rpm regulator. Also the driver have ranges so it autodecide the best voltage in the range of the voltage triplet based on the one supported by the regulator. Normally the normal voltage is always used.

Eeh? So you pass any half-random value to it and RPM edits it in flight?

Please be more specific, I'm not very familiar with this platform

Konrad

#17 lore raw

Re: [PATCH v5 4/4] ARM: dts: qcom: ipq8064: Add CPU OPP table

Quoted 44 lines · Konrad Dybcio
Quoted 35 lines · Christian Marangi
Quoted 25 lines · Konrad Dybcio
Quoted 12 lines · Christian Marangi
Quoted 11 lines · Konrad Dybcio
Quoted 8 lines · Robert Marko

From: Christian Marangi <[email protected]>

Add CPU OPP table for IPQ8062, IPQ8064 and IPQ8065 SoC. Use opp-supported-hw binding to correctly enable and disable the frequency as IPQ8062 supports up to 1.0Ghz, IPQ8064 supports up to 1.4GHz with 1.2GHz as an additional frequency and IPQ8065 supports 1.7GHZ but doesn't have 1.2GHZ frequency and has to be disabled.

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Robert Marko <[email protected]>

---

Christian/Robert, can you provide a downstream source for this?

Sure, consider that everything is with +/-5%.

Hm, so you're e.g. putting ipq8062 384MHz voltage for PVS3 equal to 0.95*800000 = 760000, but I'm not sure if it's a good idea?

Quoted 8 lines

The comment in downstream:

"These are based on +/-5% Margin on the VDD_APCx that is advertised in our Datasheet across Temperature"

suggests this is already not very accurate, and betting that the lower threshold works on all chips is probably not the best idea.

Quoted 6 lines

Consider that everything is driven by the rpm. The original qsdk used the same approach of taking the value, apply +-5% and pass it as a voltage triplet to the rpm regulator. Also the driver have ranges so it autodecide the best voltage in the range of the voltage triplet based on the one supported by the regulator. Normally the normal voltage is always used.

Quoted 4 lines

Eeh? So you pass any half-random value to it and RPM edits it in flight?

Please be more specific, I'm not very familiar with this platform

Sorry, probably I was a bit confusing.

ipq806x mount on 99% of the devices (this is the suggested design by qcom) smb208 regulator. These have selector and step since not every voltage is supported. So the closest one is selected in the range of the provided min and max.

Most of the time the normal voltage is correctly used by sometimes an higher one is used. The ranges are described here [1].

Consider that in later version of the qsdk where the moved to DT definition, they started using OPP v1 where voltage-tollerance binding is used. The voltage-tollerance was set to 5.

You can find how this value was used for OPPv1 here [2].

As you can see they internally calculate the min and max value and set them. OPP v2 dropped this and make the dev directly provide min and max.

Effectively we pass the same voltage values.

For the voltage and how it's set, we use cpufreq-dt where if an attached regulator is found, the set_voltage_triplet is used by providing normal min and max value and then internally the best value is selected. This same implementation was used in the qsdk source when they moved to dt implementation.

Hope it's more clear now how the voltages are set on this platform. For the RPM part, the voltage is requested but there isn't a direct control of the system on the regulator since everything is handled by RPM so there is also that extra step.

[1] https://elixir.bootlin.com/linux/latest/source/drivers/regulator/qcom_rpm-regulator.c#L178
[2] https://elixir.bootlin.com/linux/latest/source/drivers/opp/core.c#L1956

Signature · 1 line
	Ansuel
#18 lore raw

Re: [PATCH v5 4/4] ARM: dts: qcom: ipq8064: Add CPU OPP table

In reply to Robert Marko
Quoted 24 lines · Robert Marko

From: Christian Marangi <[email protected]>

Add CPU OPP table for IPQ8062, IPQ8064 and IPQ8065 SoC. Use opp-supported-hw binding to correctly enable and disable the frequency as IPQ8062 supports up to 1.0Ghz, IPQ8064 supports up to 1.4GHz with 1.2GHz as an additional frequency and IPQ8065 supports 1.7GHZ but doesn't have 1.2GHZ frequency and has to be disabled.

Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Robert Marko <[email protected]>

---
Changes v4:
* Add OPP DTS patch for IPQ8064

  arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi | 30 +++++++++++
  arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi | 67 ++++++++++++++++++++++++
  arch/arm/boot/dts/qcom/qcom-ipq8065.dtsi | 65 +++++++++++++++++++++++
  3 files changed, 162 insertions(+)
arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi +0 -0
index 5d3ebd3e2e51..72d9782c3d6f 100644

arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi / { · line 15

Quoted 10 lines
arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi +7 -0
@@ -6,3 +6,33 @@ / {
  	model = "Qualcomm Technologies, Inc. IPQ8062";
  	compatible = "qcom,ipq8062", "qcom,ipq8064";
  };
+
+&opp_table_cpu {
+	opp-384000000 {
+		opp-microvolt-speed0-pvs0 = <1000000 950000 1050000>;
+		opp-microvolt-speed0-pvs1 = <925000 878750 971250>;
+		opp-microvolt-speed0-pvs2 = <875000 831250 918750>;
+		opp-microvolt-speed0-pvs3 = <800000 760000 840000>;

We can just make use of opp-supported-hw and define opp-384...-0, opp-384..-1 etc. with a valid corresponding bitmask in opp-supported-hw

otherwise it's somewhat confusing to follow, I think..

Konrad

#19 lore raw

Re: [PATCH v5 4/4] ARM: dts: qcom: ipq8064: Add CPU OPP table

Quoted 40 lines · Konrad Dybcio
Quoted 24 lines · Robert Marko

From: Christian Marangi <[email protected]>

Add CPU OPP table for IPQ8062, IPQ8064 and IPQ8065 SoC. Use opp-supported-hw binding to correctly enable and disable the frequency as IPQ8062 supports up to 1.0Ghz, IPQ8064 supports up to 1.4GHz with 1.2GHz as an additional frequency and IPQ8065 supports 1.7GHZ but doesn't have 1.2GHZ frequency and has to be disabled.

Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Robert Marko <[email protected]>

---
Changes v4:
* Add OPP DTS patch for IPQ8064

  arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi | 30 +++++++++++
  arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi | 67 ++++++++++++++++++++++++
  arch/arm/boot/dts/qcom/qcom-ipq8065.dtsi | 65 +++++++++++++++++++++++
  3 files changed, 162 insertions(+)
arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi +0 -0
index 5d3ebd3e2e51..72d9782c3d6f 100644

arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi / { · line 15

Quoted 10 lines
arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi +7 -0
@@ -6,3 +6,33 @@ / {
  	model = "Qualcomm Technologies, Inc. IPQ8062";
  	compatible = "qcom,ipq8062", "qcom,ipq8064";
  };
+
+&opp_table_cpu {
+	opp-384000000 {
+		opp-microvolt-speed0-pvs0 = <1000000 950000 1050000>;
+		opp-microvolt-speed0-pvs1 = <925000 878750 971250>;
+		opp-microvolt-speed0-pvs2 = <875000 831250 918750>;
+		opp-microvolt-speed0-pvs3 = <800000 760000 840000>;

We can just make use of opp-supported-hw and define opp-384...-0, opp-384..-1 etc. with a valid corresponding bitmask in opp-supported-hw

Quoted 2 lines

otherwise it's somewhat confusing to follow, I think..

Ehh should we really double the nodes for ipq8062 and ipq8065? The idea here was to overwrite the one since the pvs always match and at worst(ipq8065) have 7 pvs instead of 4. From the system it would be easier to read since only one table is present in the final dts and not 2 and referring to the opp-supported-hw.

The original idea was to declare one opp table and reuse pvs version (by faking it with hardcoded values) to put additional pvs for ipq8062 and ipq8065, but Dmitry didn't like it and asked to move the opp in different dtsi.

Signature · 1 line
	Ansuel
#20 lore raw

Re: [PATCH v5 4/4] ARM: dts: qcom: ipq8064: Add CPU OPP table

Quoted 41 lines · Christian Marangi
Quoted 40 lines · Konrad Dybcio
Quoted 24 lines · Robert Marko

From: Christian Marangi <[email protected]>

Add CPU OPP table for IPQ8062, IPQ8064 and IPQ8065 SoC. Use opp-supported-hw binding to correctly enable and disable the frequency as IPQ8062 supports up to 1.0Ghz, IPQ8064 supports up to 1.4GHz with 1.2GHz as an additional frequency and IPQ8065 supports 1.7GHZ but doesn't have 1.2GHZ frequency and has to be disabled.

Signed-off-by Christian Marangi <[email protected]>
Signed-off-by Robert Marko <[email protected]>

---
Changes v4:
* Add OPP DTS patch for IPQ8064

   arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi | 30 +++++++++++
   arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi | 67 ++++++++++++++++++++++++
   arch/arm/boot/dts/qcom/qcom-ipq8065.dtsi | 65 +++++++++++++++++++++++
   3 files changed, 162 insertions(+)
arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi +0 -0
index 5d3ebd3e2e51..72d9782c3d6f 100644

arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi / { · line 15

Quoted 10 lines
arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi +7 -0
@@ -6,3 +6,33 @@ / {
   	model = "Qualcomm Technologies, Inc. IPQ8062";
   	compatible = "qcom,ipq8062", "qcom,ipq8064";
   };
+
+&opp_table_cpu {
+	opp-384000000 {
+		opp-microvolt-speed0-pvs0 = <1000000 950000 1050000>;
+		opp-microvolt-speed0-pvs1 = <925000 878750 971250>;
+		opp-microvolt-speed0-pvs2 = <875000 831250 918750>;
+		opp-microvolt-speed0-pvs3 = <800000 760000 840000>;

We can just make use of opp-supported-hw and define opp-384...-0, opp-384..-1 etc. with a valid corresponding bitmask in opp-supported-hw

Quoted 4 lines
Quoted 2 lines

otherwise it's somewhat confusing to follow, I think..

Ehh should we really double the nodes for ipq8062 and ipq8065?

Hm.. I'm not 100% sure, both solutions are kinda meh, but perhaps overwriting it will be less of a hassle for looking at the decompiled dt indeed..

Quoted 9 lines

The idea here was to overwrite the one since the pvs always match and at worst(ipq8065) have 7 pvs instead of 4. From the system it would be easier to read since only one table is present in the final dts and not 2 and referring to the opp-supported-hw.

The original idea was to declare one opp table and reuse pvs version (by faking it with hardcoded values) to put additional pvs for ipq8062 and ipq8065, but Dmitry didn't like it and asked to move the opp in different dtsi.

Yeah this fusing thing is complex enough already

KOnrad

Acked-by on v5 1/4
#21 lore raw
In reply to Robert Marko
Quoted 31 lines · Robert Marko

IPQ8074 comes in 2 families:
* IPQ8070A/IPQ8071A (Acorn) up to 1.4GHz
* IPQ8072A/IPQ8074A/IPQ8076A/IPQ8078A (Hawkeye) up to 2.2GHz

So, in order to be able to share one OPP table lets add support for IPQ8074 family based of SMEM SoC ID-s as speedbin fuse is always 0 on IPQ8074.

IPQ8074 compatible is blacklisted from DT platdev as the cpufreq device will get created by NVMEM CPUFreq driver.

Signed-off-by Robert Marko <[email protected]>

---
Changes in v4:
* Add support for IPQ8174 (Oak) family

Changes in v3:
* Use enum for SoC versions

Changes in v2:
* Print an error if SMEM ID is not part of the IPQ8074 family and restrict the speed to Acorn variant (1.4GHz)

  drivers/cpufreq/cpufreq-dt-platdev.c |  1 +
  drivers/cpufreq/qcom-cpufreq-nvmem.c | 45 ++++++++++++++++++++++++++++
  2 files changed, 46 insertions(+)
drivers/cpufreq/cpufreq-dt-platdev.c +0 -0
index 2016d47889c0..157c91b9962c 100644

drivers/cpufreq/cpufreq-dt-platdev.c static const struct of_device_id blocklist[] __initconst = { · line 186

Quoted 7 lines
drivers/cpufreq/cpufreq-dt-platdev.c +1 -0
@@ -180,6 +180,7 @@ static const struct of_device_id blocklist[] __initconst = {
  	{ .compatible = "ti,am62a7", },
 
  	{ .compatible = "qcom,ipq8064", },
+	{ .compatible = "qcom,ipq8074", },
  	{ .compatible = "qcom,apq8064", },
  	{ .compatible = "qcom,msm8974", },
  	{ .compatible = "qcom,msm8960", },

Generally this lands in a separate commit, but I guess since Viresh takes changes to both of the files, it's even better..

Acked-by Konrad Dybcio <[email protected]>

Konrad

maintainer
#22 lore raw
In reply to Robert Marko
Quoted 7 lines · Robert Marko

IPQ8074 comes in 2 families:
* IPQ8070A/IPQ8071A (Acorn) up to 1.4GHz
* IPQ8072A/IPQ8074A/IPQ8076A/IPQ8078A (Hawkeye) up to 2.2GHz

So, in order to be able to share one OPP table lets add support for IPQ8074 family based of SMEM SoC ID-s as speedbin fuse is always 0 on IPQ8074.

Quoted 7 lines

IPQ8074 compatible is blacklisted from DT platdev as the cpufreq device will get created by NVMEM CPUFreq driver.

Signed-off-by Robert Marko <[email protected]>

---
Changes in v4:
* Add support for IPQ8174 (Oak) family

Doesn't apply cleanly over my branch anymore, please resend. Thanks.

Signature · 1 line
viresh