Skip to contentSkip to conversation

cpufreq: qcom-fw: Add support for QCOM cpufreq FW driver

PATCH Superseded Taniya Das 3 participants 12 messages Jun 4, 2018 – Jun 19, 2018

Subsystems (3)
Device tree bindings Arm Qualcomm mailing list CPU frequency scaling framework

v3 of 13 Superseded Latest is v13 →

lore
Share & tools
lore

Taniya Das

author
#0 lore raw

v3 cover cpufreq: qcom-fw: Add support for QCOM cpufreq FW driver

[v3]
* Remove index check from 'qcom_cpufreq_fw_target_index'.
* Update the Documentation binding to add the platform specific properties in the CPU nodes, node name "qcom,freq-domain".
* Update return value to '0' from -ENODEV from 'qcom_cpufreq_fw_get'.
* Update the logic for boost frequency to use local variables instead of cpufreq driver data in 'qcom_read_lut'.
* Update the logic in 'qcom_get_related_cpus' to find the related cpus.
* Update the reg-names to remove "_base" and also update the binding with the description of these registers.
* Update the logic in 'qcom_resources_init' to address the new device tree notation of handling the frequency domain phandles.

[v2]
* Fixed the alignment issues in "qcom_cpufreq_fw_target_index" for dev_err and also for "qcom_cpu_resources_init".
* Removed ret = 0 from qcom_get_related_cpus and added to check for cpu_mask_empty to return -ENOENT.
* Fixes qcom_cpu_resources_init function
* Remove initialization of 'index'
* Check for valid 'c'
* Removed initialization of 'prev_cc' from 'qcom_read_lut'.
* Remove initialization of 'ret' from function qcom_resources_init and add return -ENODEV based on 'of_get_available_child_count'.
* Removed initialization of 'rc' from qcom_cpufreq_fw_driver_probe
* Removed module_exit as this driver would not be used as module, also updated the Kconfig to bool from tristate.
* Updated the subsystem in device tree bindings.

[v1]
* Fixed compilation reported by Amit K.

Taniya Das (2):
dt-bindings: cpufreq: Introduce QCOM CPUFREQ FW bindings
cpufreq: qcom-fw: Add support for QCOM cpufreq FW driver

 .../bindings/cpufreq/cpufreq-qcom-fw.txt           | 173 +++++++++++
 drivers/cpufreq/Kconfig.arm                        |   9 +
 drivers/cpufreq/Makefile                           |   1 +
 drivers/cpufreq/qcom-cpufreq-fw.c                  | 318 +++++++++++++++++++++
 4 files changed, 501 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt
 create mode 100644 drivers/cpufreq/qcom-cpufreq-fw.c

--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.

author
#1 lore raw

PATCH v3 1/2 dt-bindings: cpufreq: Introduce QCOM CPUFREQ FW bindings

Add QCOM cpufreq firmware device bindings for Qualcomm Technology Inc's SoCs. This is required for managing the cpu frequency transitions which are controlled by firmware.

Signed-off-by Taniya Das <[email protected]>
---
 .../bindings/cpufreq/cpufreq-qcom-fw.txt           | 173 +++++++++++++++++++++
 1 file changed, 173 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt new file +173 -0
new file mode 100644
index 0000000..e3087ec
@@ -0,0 +1,173 @@
+Qualcomm Technologies, Inc. CPUFREQ Bindings
+
+CPUFREQ FW is a hardware engine used by some Qualcomm Technologies, Inc. (QTI)
+SoCs to manage frequency in hardware. It is capable of controlling frequency
+for multiple clusters.
+
+Properties:
+- compatible
+	Usage:		required
+	Value type:	<string>
+	Definition:	must be "qcom,cpufreq-fw".
+
+* Property qcom,freq-domain
+Devices supporting freq-domain must set their "qcom,freq-domain" property with
+phandle to a freq_domain_table in their DT node.
+
+* Frequency Domain Table Node
+
+This describes the frequency domain belonging to a device.
+This node can have following properties:
+
+- reg
+	Usage:		required
+	Value type:	<prop-encoded-array>
+	Definition:	Addresses and sizes for the memory of the perf
+			, lut and enable bases.
+			perf - indicates the base address for the desired
+			performance state to be set.
+			lut - indicates the look up table base address for the
+			cpufreq	driver to read frequencies.
+			enable - indicates the enable register for firmware.
+- reg-names
+	Usage:		required
+	Value type:	<stringlist>
+	Definition:	Address names. Must be "perf", "lut", "enable".
+			Must be specified in the same order as the reg property.
+
+Example:
+
+Example 1: Dual-cluster, Quad-core per cluster. CPUs within a cluster switch
+DCVS state together.
+
+/ {
+	cpus {
+		#address-cells = <2>;
+		#size-cells = <0>;
+
+		CPU0: cpu@0 {
+			device_type = "cpu";
+			compatible = "qcom,kryo385";
+			reg = <0x0 0x0>;
+			enable-method = "psci";
+			next-level-cache = <&L2_0>;
+			qcom,freq-domain = <&freq_domain_table0>;
+			L2_0: l2-cache {
+				compatible = "cache";
+				next-level-cache = <&L3_0>;
+				L3_0: l3-cache {
+				      compatible = "cache";
+				};
+			};
+		};
+
+		CPU1: cpu@100 {
+			device_type = "cpu";
+			compatible = "qcom,kryo385";
+			reg = <0x0 0x100>;
+			enable-method = "psci";
+			next-level-cache = <&L2_100>;
+			qcom,freq-domain = <&freq_domain_table0>;
+			L2_100: l2-cache {
+				compatible = "cache";
+				next-level-cache = <&L3_0>;
+			};
+		};
+
+		CPU2: cpu@200 {
+			device_type = "cpu";
+			compatible = "qcom,kryo385";
+			reg = <0x0 0x200>;
+			enable-method = "psci";
+			next-level-cache = <&L2_200>;
+			qcom,freq-domain = <&freq_domain_table0>;
+			L2_200: l2-cache {
+				compatible = "cache";
+				next-level-cache = <&L3_0>;
+			};
+		};
+
+		CPU3: cpu@300 {
+			device_type = "cpu";
+			compatible = "qcom,kryo385";
+			reg = <0x0 0x300>;
+			enable-method = "psci";
+			next-level-cache = <&L2_300>;
+			qcom,freq-domain = <&freq_domain_table0>;
+			L2_300: l2-cache {
+				compatible = "cache";
+				next-level-cache = <&L3_0>;
+			};
+		};
+
+		CPU4: cpu@400 {
+			device_type = "cpu";
+			compatible = "qcom,kryo385";
+			reg = <0x0 0x400>;
+			enable-method = "psci";
+			next-level-cache = <&L2_400>;
+			qcom,freq-domain = <&freq_domain_table1>;
+			L2_400: l2-cache {
+				compatible = "cache";
+				next-level-cache = <&L3_0>;
+			};
+		};
+
+		CPU5: cpu@500 {
+			device_type = "cpu";
+			compatible = "qcom,kryo385";
+			reg = <0x0 0x500>;
+			enable-method = "psci";
+			next-level-cache = <&L2_500>;
+			qcom,freq-domain = <&freq_domain_table1>;
+			L2_500: l2-cache {
+				compatible = "cache";
+				next-level-cache = <&L3_0>;
+			};
+		};
+
+		CPU6: cpu@600 {
+			device_type = "cpu";
+			compatible = "qcom,kryo385";
+			reg = <0x0 0x600>;
+			enable-method = "psci";
+			next-level-cache = <&L2_600>;
+			qcom,freq-domain = <&freq_domain_table1>;
+			L2_600: l2-cache {
+				compatible = "cache";
+				next-level-cache = <&L3_0>;
+			};
+		};
+
+		CPU7: cpu@700 {
+			device_type = "cpu";
+			compatible = "qcom,kryo385";
+			reg = <0x0 0x700>;
+			enable-method = "psci";
+			next-level-cache = <&L2_700>;
+			qcom,freq-domain = <&freq_domain_table1>;
+			L2_700: l2-cache {
+				compatible = "cache";
+				next-level-cache = <&L3_0>;
+			};
+		};
+	};
+
+	qcom,cpufreq-fw {
+		compatible = "qcom,cpufreq-fw";
+
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		freq_domain_table0 : freq_table0 {
+			reg = <0x17d43920 0x4>, <0x17d43110 0x500>,
+				 <0x17d41000 0x4>;
+			reg-names = "perf", "lut", "enable";
+		};
+
+		freq_domain_table1 : freq_table1 {
+			reg = <0x17d46120 0x4>, <0x17d45910 0x500>,
+				<0x17d45800 0x4> ;
+			reg-names = "perf", "lut", "enable";
+		};
+	};

--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.

#2 lore raw

Re: [PATCH 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ FW bindings

Quoted 53 lines · Taniya Das

Add QCOM cpufreq firmware device bindings for Qualcomm Technology Inc's SoCs. This is required for managing the cpu frequency transitions which are controlled by firmware.

Signed-off-by Taniya Das <[email protected]>
---
 .../bindings/cpufreq/cpufreq-qcom-fw.txt           | 173 +++++++++++++++++++++
 1 file changed, 173 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt new file +37 -0
new file mode 100644
index 0000000..e3087ec
@@ -0,0 +1,173 @@
+Qualcomm Technologies, Inc. CPUFREQ Bindings
+
+CPUFREQ FW is a hardware engine used by some Qualcomm Technologies, Inc. (QTI)
+SoCs to manage frequency in hardware. It is capable of controlling frequency
+for multiple clusters.
+
+Properties:
+- compatible
+	Usage:		required
+	Value type:	<string>
+	Definition:	must be "qcom,cpufreq-fw".
+
+* Property qcom,freq-domain
+Devices supporting freq-domain must set their "qcom,freq-domain" property with
+phandle to a freq_domain_table in their DT node.
+
+* Frequency Domain Table Node
+
+This describes the frequency domain belonging to a device.
+This node can have following properties:
+
+- reg
+	Usage:		required
+	Value type:	<prop-encoded-array>
+	Definition:	Addresses and sizes for the memory of the perf
+			, lut and enable bases.
+			perf - indicates the base address for the desired
+			performance state to be set.
+			lut - indicates the look up table base address for the
+			cpufreq	driver to read frequencies.
+			enable - indicates the enable register for firmware.
+- reg-names
+	Usage:		required
+	Value type:	<stringlist>
+	Definition:	Address names. Must be "perf", "lut", "enable".
+			Must be specified in the same order as the reg property.
+

[...]

Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt line 164

Quoted 10 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt new file +10 -0
+
+	qcom,cpufreq-fw {
+		compatible = "qcom,cpufreq-fw";
+
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		freq_domain_table0 : freq_table0 {
+			reg = <0x17d43920 0x4>, <0x17d43110 0x500>,
+				 <0x17d41000 0x4>;

Are "perf", "lut", "enable" registers part of single IP block / share memory ? I am just trying to understand the reason for separate entries in this fashion as part of DT register property. I am wondering if there will be multiple entries that fall with the page size.

--
Regards,
Sudeep

author
#3 lore raw

Re: [PATCH 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ FW bindings

Hello Sudeep,

Thanks for the review comments.

Quoted 57 lines · Sudeep Holla
Quoted 53 lines · Taniya Das

Add QCOM cpufreq firmware device bindings for Qualcomm Technology Inc's SoCs. This is required for managing the cpu frequency transitions which are controlled by firmware.

Signed-off-by Taniya Das <[email protected]>
---
  .../bindings/cpufreq/cpufreq-qcom-fw.txt           | 173 +++++++++++++++++++++
  1 file changed, 173 insertions(+)

create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt

Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt new file +37 -0
new file mode 100644
index 0000000..e3087ec
@@ -0,0 +1,173 @@
+Qualcomm Technologies, Inc. CPUFREQ Bindings
+
+CPUFREQ FW is a hardware engine used by some Qualcomm Technologies, Inc. (QTI)
+SoCs to manage frequency in hardware. It is capable of controlling frequency
+for multiple clusters.
+
+Properties:
+- compatible
+	Usage:		required
+	Value type:	<string>
+	Definition:	must be "qcom,cpufreq-fw".
+
+* Property qcom,freq-domain
+Devices supporting freq-domain must set their "qcom,freq-domain" property with
+phandle to a freq_domain_table in their DT node.
+
+* Frequency Domain Table Node
+
+This describes the frequency domain belonging to a device.
+This node can have following properties:
+
+- reg
+	Usage:		required
+	Value type:	<prop-encoded-array>
+	Definition:	Addresses and sizes for the memory of the perf
+			, lut and enable bases.
+			perf - indicates the base address for the desired
+			performance state to be set.
+			lut - indicates the look up table base address for the
+			cpufreq	driver to read frequencies.
+			enable - indicates the enable register for firmware.
+- reg-names
+	Usage:		required
+	Value type:	<stringlist>
+	Definition:	Address names. Must be "perf", "lut", "enable".
+			Must be specified in the same order as the reg property.
+

[...]

Quoted 16 lines

Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt line 164

Quoted 10 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt new file +10 -0
+
+	qcom,cpufreq-fw {
+		compatible = "qcom,cpufreq-fw";
+
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		freq_domain_table0 : freq_table0 {
+			reg = <0x17d43920 0x4>, <0x17d43110 0x500>,
+				 <0x17d41000 0x4>;

Are "perf", "lut", "enable" registers part of single IP block / share memory ? I am just trying to understand the reason for separate entries in this fashion as part of DT register property. I am wondering if there will be multiple entries that fall with the page size.

They are part of the same IP block and these are the only register offsets which is required to be accessed by HLOS.

Quoted 4 lines

--
Regards,
Sudeep

Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--
#4 lore raw

Re: [PATCH 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ FW bindings

Quoted 89 lines · Taniya Das

Hello Sudeep,

Thanks for the review comments.

Quoted 74 lines · Sudeep Holla
Quoted 61 lines · Taniya Das

Add QCOM cpufreq firmware device bindings for Qualcomm Technology Inc's SoCs. This is required for managing the cpu frequency transitions which are
controlled by firmware.

Signed-off-by Taniya Das <[email protected]>

---
  .../bindings/cpufreq/cpufreq-qcom-fw.txt           | 173
+++++++++++++++++++++

  1 file changed, 173 insertions(+)

  create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt

diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt new file mode 100644
index 0000000..e3087ec

Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt new file +3 -0
@@ -0,0 +1,173 @@
+Qualcomm Technologies, Inc. CPUFREQ Bindings
+
+CPUFREQ FW is a hardware engine used by some Qualcomm Technologies,

Inc. (QTI)
+SoCs to manage frequency in hardware. It is capable of controlling frequency

Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt new file +10 -0
+for multiple clusters.
+
+Properties:
+- compatible
+    Usage:        required
+    Value type:    <string>
+    Definition:    must be "qcom,cpufreq-fw".
+
+* Property qcom,freq-domain
+Devices supporting freq-domain must set their "qcom,freq-domain"

property with

Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt new file +23 -0
+phandle to a freq_domain_table in their DT node.
+
+* Frequency Domain Table Node
+
+This describes the frequency domain belonging to a device.
+This node can have following properties:
+
+- reg
+    Usage:        required
+    Value type:    <prop-encoded-array>
+    Definition:    Addresses and sizes for the memory of the perf
+            , lut and enable bases.
+            perf - indicates the base address for the desired
+            performance state to be set.
+            lut - indicates the look up table base address for the
+            cpufreq    driver to read frequencies.
+            enable - indicates the enable register for firmware.
+- reg-names
+    Usage:        required
+    Value type:    <stringlist>
+    Definition:    Address names. Must be "perf", "lut", "enable".
+            Must be specified in the same order as the reg property.
+

[...]

Quoted 9 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt new file +9 -0
+
+    qcom,cpufreq-fw {
+        compatible = "qcom,cpufreq-fw";
+
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        freq_domain_table0 : freq_table0 {
+            reg = <0x17d43920 0x4>, <0x17d43110 0x500>,
Quoted 9 lines
Quoted 1 line

+                 <0x17d41000 0x4>;

Are "perf", "lut", "enable" registers part of single IP block / share memory ?
I am just trying to understand the reason for separate entries in this fashion
as part of DT register property. I am wondering if there will be multiple entries that fall with the page size.

Quoted 3 lines

They are part of the same IP block and these are the only register offsets which is required to be accessed by HLOS.

HLOS ?

Anyways, OS might touch one or 2 registers in lots of IP blocks. I am not sure why those are any different from these. Are you trying to align with any other bindings or specification. Are you trying to make this binding generic here ? I understand if it was trying to generalize the firmware interface, but you state it's a hardware engine. So I fail to see the need for such specificity here. Why not define the whole IP block and the driver knows where to access these specific ones as they are specific to this hardware block.

Signature · 2 lines
Regards,
Sudeep
maintainer Acked-by on v3 1/2
#5 lore raw

Re: [PATCH 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ FW bindings

In reply to Taniya Das
Quoted 179 lines · Taniya Das

Add QCOM cpufreq firmware device bindings for Qualcomm Technology Inc's SoCs. This is required for managing the cpu frequency transitions which are controlled by firmware.

Signed-off-by Taniya Das <[email protected]>
---
 .../bindings/cpufreq/cpufreq-qcom-fw.txt           | 173 +++++++++++++++++++++
 1 file changed, 173 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt new file +163 -0
new file mode 100644
index 0000000..e3087ec
@@ -0,0 +1,173 @@
+Qualcomm Technologies, Inc. CPUFREQ Bindings
+
+CPUFREQ FW is a hardware engine used by some Qualcomm Technologies, Inc. (QTI)
+SoCs to manage frequency in hardware. It is capable of controlling frequency
+for multiple clusters.
+
+Properties:
+- compatible
+	Usage:		required
+	Value type:	<string>
+	Definition:	must be "qcom,cpufreq-fw".
+
+* Property qcom,freq-domain
+Devices supporting freq-domain must set their "qcom,freq-domain" property with
+phandle to a freq_domain_table in their DT node.
+
+* Frequency Domain Table Node
+
+This describes the frequency domain belonging to a device.
+This node can have following properties:
+
+- reg
+	Usage:		required
+	Value type:	<prop-encoded-array>
+	Definition:	Addresses and sizes for the memory of the perf
+			, lut and enable bases.
+			perf - indicates the base address for the desired
+			performance state to be set.
+			lut - indicates the look up table base address for the
+			cpufreq	driver to read frequencies.
+			enable - indicates the enable register for firmware.
+- reg-names
+	Usage:		required
+	Value type:	<stringlist>
+	Definition:	Address names. Must be "perf", "lut", "enable".
+			Must be specified in the same order as the reg property.
+
+Example:
+
+Example 1: Dual-cluster, Quad-core per cluster. CPUs within a cluster switch
+DCVS state together.
+
+/ {
+	cpus {
+		#address-cells = <2>;
+		#size-cells = <0>;
+
+		CPU0: cpu@0 {
+			device_type = "cpu";
+			compatible = "qcom,kryo385";
+			reg = <0x0 0x0>;
+			enable-method = "psci";
+			next-level-cache = <&L2_0>;
+			qcom,freq-domain = <&freq_domain_table0>;
+			L2_0: l2-cache {
+				compatible = "cache";
+				next-level-cache = <&L3_0>;
+				L3_0: l3-cache {
+				      compatible = "cache";
+				};
+			};
+		};
+
+		CPU1: cpu@100 {
+			device_type = "cpu";
+			compatible = "qcom,kryo385";
+			reg = <0x0 0x100>;
+			enable-method = "psci";
+			next-level-cache = <&L2_100>;
+			qcom,freq-domain = <&freq_domain_table0>;
+			L2_100: l2-cache {
+				compatible = "cache";
+				next-level-cache = <&L3_0>;
+			};
+		};
+
+		CPU2: cpu@200 {
+			device_type = "cpu";
+			compatible = "qcom,kryo385";
+			reg = <0x0 0x200>;
+			enable-method = "psci";
+			next-level-cache = <&L2_200>;
+			qcom,freq-domain = <&freq_domain_table0>;
+			L2_200: l2-cache {
+				compatible = "cache";
+				next-level-cache = <&L3_0>;
+			};
+		};
+
+		CPU3: cpu@300 {
+			device_type = "cpu";
+			compatible = "qcom,kryo385";
+			reg = <0x0 0x300>;
+			enable-method = "psci";
+			next-level-cache = <&L2_300>;
+			qcom,freq-domain = <&freq_domain_table0>;
+			L2_300: l2-cache {
+				compatible = "cache";
+				next-level-cache = <&L3_0>;
+			};
+		};
+
+		CPU4: cpu@400 {
+			device_type = "cpu";
+			compatible = "qcom,kryo385";
+			reg = <0x0 0x400>;
+			enable-method = "psci";
+			next-level-cache = <&L2_400>;
+			qcom,freq-domain = <&freq_domain_table1>;
+			L2_400: l2-cache {
+				compatible = "cache";
+				next-level-cache = <&L3_0>;
+			};
+		};
+
+		CPU5: cpu@500 {
+			device_type = "cpu";
+			compatible = "qcom,kryo385";
+			reg = <0x0 0x500>;
+			enable-method = "psci";
+			next-level-cache = <&L2_500>;
+			qcom,freq-domain = <&freq_domain_table1>;
+			L2_500: l2-cache {
+				compatible = "cache";
+				next-level-cache = <&L3_0>;
+			};
+		};
+
+		CPU6: cpu@600 {
+			device_type = "cpu";
+			compatible = "qcom,kryo385";
+			reg = <0x0 0x600>;
+			enable-method = "psci";
+			next-level-cache = <&L2_600>;
+			qcom,freq-domain = <&freq_domain_table1>;
+			L2_600: l2-cache {
+				compatible = "cache";
+				next-level-cache = <&L3_0>;
+			};
+		};
+
+		CPU7: cpu@700 {
+			device_type = "cpu";
+			compatible = "qcom,kryo385";
+			reg = <0x0 0x700>;
+			enable-method = "psci";
+			next-level-cache = <&L2_700>;
+			qcom,freq-domain = <&freq_domain_table1>;
+			L2_700: l2-cache {
+				compatible = "cache";
+				next-level-cache = <&L3_0>;
+			};
+		};
+	};
+
+	qcom,cpufreq-fw {
+		compatible = "qcom,cpufreq-fw";
+
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		freq_domain_table0 : freq_table0 {
+			reg = <0x17d43920 0x4>, <0x17d43110 0x500>,

Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt line 173

Quoted 10 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt new file +10 -0
+				 <0x17d41000 0x4>;
+			reg-names = "perf", "lut", "enable";
+		};
+
+		freq_domain_table1 : freq_table1 {
+			reg = <0x17d46120 0x4>, <0x17d45910 0x500>,
+				<0x17d45800 0x4> ;
+			reg-names = "perf", "lut", "enable";
+		};
+	};

Looks better now from design point of view now, no ugly CPU lists :)

Unless Rob finds something wrong with the bindings:

Acked-by Viresh Kumar <[email protected]>
Signature · 1 line
viresh
author
#6 lore raw

PATCH v3 2/2 cpufreq: qcom-fw: Add support for QCOM cpufreq FW driver

In reply to Taniya Das

The CPUfreq FW present in some QCOM chipsets offloads the steps necessary for changing the frequency of CPUs. The driver implements the cpufreq driver interface for this firmware.

Signed-off-by Saravana Kannan <[email protected]>
Signed-off-by Taniya Das <[email protected]>
---
 drivers/cpufreq/Kconfig.arm       |   9 ++
 drivers/cpufreq/Makefile          |   1 +
 drivers/cpufreq/qcom-cpufreq-fw.c | 316 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 326 insertions(+)
 create mode 100644 drivers/cpufreq/qcom-cpufreq-fw.c
drivers/cpufreq/Kconfig.arm +9 -0
index c7ce928..82c391e 100644
@@ -312,3 +312,12 @@ config ARM_PXA2xx_CPUFREQ
 	  This add the CPUFreq driver support for Intel PXA2xx SOCs.
 
 	  If in doubt, say N.
+
+config ARM_QCOM_CPUFREQ_FW
+	bool "QCOM CPUFreq FW driver"
+	help
+	 Support for the CPUFreq FW driver.
+	 The CPUfreq FW preset in some QCOM chipsets offloads the steps
+	 necessary for changing the frequency of CPUs. The driver
+	 implements the cpufreq driver interface for this firmware.
+	 Say Y if you want to support CPUFreq FW.
drivers/cpufreq/Makefile +1 -0
index fb4a2ec..34691a2 100644
@@ -86,6 +86,7 @@ obj-$(CONFIG_ARM_TEGRA124_CPUFREQ)	+= tegra124-cpufreq.o
 obj-$(CONFIG_ARM_TEGRA186_CPUFREQ)	+= tegra186-cpufreq.o
 obj-$(CONFIG_ARM_TI_CPUFREQ)		+= ti-cpufreq.o
 obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ)	+= vexpress-spc-cpufreq.o
+obj-$(CONFIG_ARM_QCOM_CPUFREQ_FW)	+= qcom-cpufreq-fw.o
 
 
 ##################################################################################
drivers/cpufreq/qcom-cpufreq-fw.c new file +316 -0

316 lines not on this page · raw message

--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.

maintainer
#7 lore raw
Quoted 107 lines · Taniya Das

The CPUfreq FW present in some QCOM chipsets offloads the steps necessary for changing the frequency of CPUs. The driver implements the cpufreq driver interface for this firmware.

Signed-off-by Saravana Kannan <[email protected]>
Signed-off-by Taniya Das <[email protected]>
---
 drivers/cpufreq/Kconfig.arm       |   9 ++
 drivers/cpufreq/Makefile          |   1 +
 drivers/cpufreq/qcom-cpufreq-fw.c | 316 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 326 insertions(+)
 create mode 100644 drivers/cpufreq/qcom-cpufreq-fw.c
drivers/cpufreq/Kconfig.arm +9 -0
index c7ce928..82c391e 100644
@@ -312,3 +312,12 @@ config ARM_PXA2xx_CPUFREQ
 	  This add the CPUFreq driver support for Intel PXA2xx SOCs.
 
 	  If in doubt, say N.
+
+config ARM_QCOM_CPUFREQ_FW
+	bool "QCOM CPUFreq FW driver"
+	help
+	 Support for the CPUFreq FW driver.
+	 The CPUfreq FW preset in some QCOM chipsets offloads the steps
+	 necessary for changing the frequency of CPUs. The driver
+	 implements the cpufreq driver interface for this firmware.
+	 Say Y if you want to support CPUFreq FW.
drivers/cpufreq/Makefile +1 -0
index fb4a2ec..34691a2 100644
@@ -86,6 +86,7 @@ obj-$(CONFIG_ARM_TEGRA124_CPUFREQ)	+= tegra124-cpufreq.o
 obj-$(CONFIG_ARM_TEGRA186_CPUFREQ)	+= tegra186-cpufreq.o
 obj-$(CONFIG_ARM_TI_CPUFREQ)		+= ti-cpufreq.o
 obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ)	+= vexpress-spc-cpufreq.o
+obj-$(CONFIG_ARM_QCOM_CPUFREQ_FW)	+= qcom-cpufreq-fw.o
 
 
 ##################################################################################
drivers/cpufreq/qcom-cpufreq-fw.c new file +59 -0
new file mode 100644
index 0000000..2135a08
@@ -0,0 +1,316 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/cpufreq.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+
+#define INIT_RATE			300000000UL
+#define XO_RATE				19200000UL
+#define LUT_MAX_ENTRIES			40U
+#define CORE_COUNT_VAL(val)		(((val) & (GENMASK(18, 16))) >> 16)
+#define LUT_ROW_SIZE			32
+
+struct cpufreq_qcom {
+	struct cpufreq_frequency_table *table;
+	struct device *dev;
+	void __iomem *perf_base;
+	void __iomem *lut_base;
+	cpumask_t related_cpus;
+	unsigned int max_cores;
+};
+
+static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];
+
+static int
+qcom_cpufreq_fw_target_index(struct cpufreq_policy *policy,
+			     unsigned int index)
+{
+	struct cpufreq_qcom *c = policy->driver_data;
+
+	writel_relaxed(index, c->perf_base);
+
+	return 0;
+}
+
+static unsigned int qcom_cpufreq_fw_get(unsigned int cpu)
+{
+	struct cpufreq_qcom *c;
+	unsigned int index;
+
+	c = qcom_freq_domain_map[cpu];
+	if (!c)
+		return 0;
+
+	index = readl_relaxed(c->perf_base);
+	index = min(index, LUT_MAX_ENTRIES - 1);
+
+	return c->table[index].frequency;
+}
+
+static int qcom_cpufreq_fw_cpu_init(struct cpufreq_policy *policy)
+{
+	struct cpufreq_qcom *c;
+

drivers/cpufreq/qcom-cpufreq-fw.c line 69

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +10 -0
+	c = qcom_freq_domain_map[policy->cpu];
+	if (!c) {
+		pr_err("No scaling support for CPU%d\n", policy->cpu);
+		return -ENODEV;
+	}
+
+	cpumask_copy(policy->cpus, &c->related_cpus);
+
+	policy->freq_table = c->table;
+	policy->driver_data = c;

What about fast cpufreq switching ? I think you can enable that option as well here..

Quoted 7 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +7 -0
+
+	return 0;
+}
+
+static struct freq_attr *qcom_cpufreq_fw_attr[] = {
+	&cpufreq_freq_attr_scaling_available_freqs,
+	&cpufreq_freq_attr_scaling_boost_freqs,

drivers/cpufreq/qcom-cpufreq-fw.c line 86

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +10 -0
+	NULL
+};
+
+static struct cpufreq_driver cpufreq_qcom_fw_driver = {
+	.flags		= CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
+			  CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
+	.verify		= cpufreq_generic_frequency_table_verify,
+	.target_index	= qcom_cpufreq_fw_target_index,
+	.get		= qcom_cpufreq_fw_get,
+	.init		= qcom_cpufreq_fw_cpu_init,

What about CPU hotplug ? We can still do that, right ? So what will happen if all CPUs of a freq-domain are removed (hence cpufreq policy is removed) and then someone calls qcom_cpufreq_fw_get() ? You should really work on cpufreq_policy there to get 'c'.

Quoted 45 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +45 -0
+	.name		= "qcom-cpufreq-fw",
+	.attr		= qcom_cpufreq_fw_attr,
+	.boost_enabled	= true,
+};
+
+static int qcom_read_lut(struct platform_device *pdev,
+			 struct cpufreq_qcom *c)
+{
+	struct device *dev = &pdev->dev;
+	u32 data, src, lval, i, core_count, prev_cc, prev_freq, cur_freq;
+
+	c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
+				sizeof(*c->table), GFP_KERNEL);
+	if (!c->table)
+		return -ENOMEM;
+
+	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
+		data = readl_relaxed(c->lut_base + i * LUT_ROW_SIZE);
+		src = ((data & GENMASK(31, 30)) >> 30);
+		lval = (data & GENMASK(7, 0));
+		core_count = CORE_COUNT_VAL(data);
+
+		if (!src)
+			c->table[i].frequency = INIT_RATE / 1000;
+		else
+			c->table[i].frequency = XO_RATE * lval / 1000;
+
+		cur_freq = c->table[i].frequency;
+
+		dev_dbg(dev, "index=%d freq=%d, core_count %d\n",
+			i, c->table[i].frequency, core_count);
+
+		if (core_count != c->max_cores)
+			cur_freq = CPUFREQ_ENTRY_INVALID;
+
+		/*
+		 * Two of the same frequencies with the same core counts means
+		 * end of table.
+		 */
+		if (i > 0 && c->table[i - 1].frequency ==
+		   c->table[i].frequency && prev_cc == core_count) {
+			struct cpufreq_frequency_table *prev = &c->table[i - 1];
+
+			if (prev_freq == CPUFREQ_ENTRY_INVALID)
+				prev->flags = CPUFREQ_BOOST_FREQ;

drivers/cpufreq/qcom-cpufreq-fw.c line 141

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +10 -0
+			break;
+		}
+		prev_cc = core_count;
+		prev_freq = cur_freq;
+	}
+
+	c->table[i].frequency = CPUFREQ_TABLE_END;
+
+	return 0;
+}

Looks like there are many problems here.
- You are assigning prev_freq with cur_freq (which may be uninitialized local variable here).
- In this version, you never write CPUFREQ_ENTRY_INVALID to table[i].frequency, which looks wrong as well.

drivers/cpufreq/qcom-cpufreq-fw.c line 145

Quoted 4 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +4 -0
+
+static int qcom_get_related_cpus(struct device_node *np, struct cpumask *m)
+{
+	struct device_node  *cpu_dev;

s/cpu_dev/cpu_np/

drivers/cpufreq/qcom-cpufreq-fw.c line 152

Quoted 7 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +7 -0
+	int cpu;
+
+	for_each_possible_cpu(cpu) {
+		cpu_dev = of_cpu_device_node_get(cpu);
+		if (!cpu_dev)
+			continue;
+		cpu_dev = of_parse_phandle(cpu_dev, "qcom,freq-domain", 0);

What's returned here is a pointer to the qcom,freq-domain node, and you assign that to a variable named cpu_dev. Either use two variables for different node types, or rename it to temp_np or something similar.

Quoted 64 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +64 -0
+		if (!cpu_dev)
+			continue;
+		if (cpu_dev == np)
+			cpumask_set_cpu(cpu, m);
+	}
+
+	if (cpumask_empty(m))
+		return -ENOENT;
+
+	return 0;
+}
+
+static int qcom_cpu_resources_init(struct platform_device *pdev,
+				   struct device_node *np)
+{
+	struct cpufreq_qcom *c;
+	struct resource res;
+	struct device *dev = &pdev->dev;
+	void __iomem *en_base;
+	int cpu, index, ret;
+
+	c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
+	if (!c)
+		return -ENOMEM;
+
+	index = of_property_match_string(np, "reg-names", "enable");
+	if (index < 0)
+		return index;
+
+	if (of_address_to_resource(np, index, &res))
+		return -ENOMEM;
+
+	en_base = devm_ioremap(dev, res.start, resource_size(&res));
+	if (!en_base) {
+		dev_err(dev, "Unable to map %s enable-base\n", np->name);
+		return -ENOMEM;
+	}
+
+	/* FW should be in enabled state to proceed */
+	if (!(readl_relaxed(en_base) & 0x1)) {
+		dev_err(dev, "%s firmware not enabled\n", np->name);
+		return -ENODEV;
+	}
+	devm_iounmap(&pdev->dev, en_base);
+
+	index = of_property_match_string(np, "reg-names", "perf");
+	if (index < 0)
+		return index;
+
+	if (of_address_to_resource(np, index, &res))
+		return -ENOMEM;
+
+	c->perf_base = devm_ioremap(dev, res.start, resource_size(&res));
+	if (!c->perf_base) {
+		dev_err(dev, "Unable to map %s perf-base\n", np->name);
+		return -ENOMEM;
+	}
+
+	index = of_property_match_string(np, "reg-names", "lut");
+	if (index < 0)
+		return index;
+
+	if (of_address_to_resource(np, index, &res))
+		return -ENOMEM;

drivers/cpufreq/qcom-cpufreq-fw.c line 226

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +10 -0
+
+	c->lut_base = devm_ioremap(dev, res.start, resource_size(&res));
+	if (!c->lut_base) {
+		dev_err(dev, "Unable to map %s lut-base\n", np->name);
+		return -ENOMEM;
+	}
+
+	ret = qcom_get_related_cpus(np, &c->related_cpus);
+	if (ret) {
+		dev_err(dev, "%s failed to get core phandles\n", np->name);

Maybe write a more relevant error message here ?

drivers/cpufreq/qcom-cpufreq-fw.c line 230

Quoted 4 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +4 -0
+		return ret;
+	}
+
+	c->max_cores = cpumask_weight(&c->related_cpus);

Maybe remove the error checking conditional from qcom_get_related_cpus() and check !c->max_cores here for the same.

drivers/cpufreq/qcom-cpufreq-fw.c line 236

Quoted 6 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +6 -0
+
+	ret = qcom_read_lut(pdev, c);
+	if (ret) {
+		dev_err(dev, "%s failed to read LUT\n", np->name);
+		return ret;
+	}

Enter a blank line here.

drivers/cpufreq/qcom-cpufreq-fw.c line 238

Quoted 2 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +2 -0
+	for_each_cpu(cpu, &c->related_cpus)
+		qcom_freq_domain_map[cpu] = c;

This whole setup looks a bit confusing to me. This is what you are doing essentially:

qcom_resources_init()

{
        for_each_possible_cpu() {
                qcom_cpu_resources_init()
                {
                        populate c->related_cpus;

                        for_each_related_cpu() {
                                qcom_freq_domain_map[cpu] = c;
                        }
                }
        }
}

So if there are 4 CPUs that share a freq domain, then you are allocating 'c' 4 times and (over)writing qcom_freq_domain_map[] for all these CPUs 4 times and finally keeping value of 'c' only once.

You must be running most of the work done in qcom_resources_init() only once per freq-domain.

drivers/cpufreq/qcom-cpufreq-fw.c line 245

Quoted 7 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +7 -0
+
+	return 0;
+}
+
+static int qcom_resources_init(struct platform_device *pdev)
+{
+	struct device_node *np, *cpu_dev;

cpu_dev is normally used in kernel for struct device *, maybe use cpu_np ?

Quoted 5 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +5 -0
+	unsigned int cpu;
+	int ret;
+
+	for_each_possible_cpu(cpu) {
+		cpu_dev = of_cpu_device_node_get(cpu);

drivers/cpufreq/qcom-cpufreq-fw.c line 260

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +10 -0
+		if (!cpu_dev) {
+			dev_err(&pdev->dev, "Failed to get cpu %d device\n",
+				cpu);
+			continue;
+		}
+
+		np = of_parse_phandle(cpu_dev, "qcom,freq-domain", 0);
+		if (!np) {
+			dev_err(&pdev->dev, "Failed to get freq-domain device\n");
+			continue;

I am not sure if we should continue or error out here. Why would you want only a group of CPUs to have this property set ? Or if you really have a case for that currently ?

Quoted 20 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +20 -0
+		}
+
+		of_node_put(cpu_dev);
+
+		ret = qcom_cpu_resources_init(pdev, np);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int qcom_cpufreq_fw_driver_probe(struct platform_device *pdev)
+{
+	int rc;
+
+	/* Get the bases of cpufreq for domains */
+	rc = qcom_resources_init(pdev);
+	if (rc) {
+		dev_err(&pdev->dev, "CPUFreq resource init failed\n");

drivers/cpufreq/qcom-cpufreq-fw.c line 290

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +10 -0
+		return rc;
+	}
+
+	rc = cpufreq_register_driver(&cpufreq_qcom_fw_driver);
+	if (rc) {
+		dev_err(&pdev->dev, "CPUFreq FW driver failed to register\n");
+		return rc;
+	}
+
+	dev_info(&pdev->dev, "QCOM CPUFreq FW driver inited\n");

s/inited/initialized/ ?

Quoted 29 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +26 -1
+
+	return 0;
+}
+
+static const struct of_device_id match_table[] = {
+	{ .compatible = "qcom,cpufreq-fw" },
+	{}
+};
+
+static struct platform_driver qcom_cpufreq_fw_driver = {
+	.probe = qcom_cpufreq_fw_driver_probe,
+	.driver = {
+		.name = "qcom-cpufreq-fw",
+		.of_match_table = match_table,
+		.owner = THIS_MODULE,
+	},
+};
+
+static int __init qcom_cpufreq_fw_init(void)
+{
+	return platform_driver_register(&qcom_cpufreq_fw_driver);
+}
+subsys_initcall(qcom_cpufreq_fw_init);
+
+MODULE_DESCRIPTION("QCOM CPU Frequency FW");
+MODULE_LICENSE("GPL v2");
--

Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.

Signature · 1 line
viresh
author
#8 lore raw

Hello Viresh,

Thank you for the review comments.

Quoted 113 lines · Viresh Kumar
Quoted 112 lines · Taniya Das

The CPUfreq FW present in some QCOM chipsets offloads the steps necessary for changing the frequency of CPUs. The driver implements the cpufreq driver interface for this firmware.

Signed-off-by Saravana Kannan <[email protected]>
Signed-off-by Taniya Das <[email protected]>
---
  drivers/cpufreq/Kconfig.arm       |   9 ++
  drivers/cpufreq/Makefile          |   1 +
  drivers/cpufreq/qcom-cpufreq-fw.c | 316 ++++++++++++++++++++++++++++++++++++++
  3 files changed, 326 insertions(+)

create mode 100644 drivers/cpufreq/qcom-cpufreq-fw.c

drivers/cpufreq/Kconfig.arm +9 -0
index c7ce928..82c391e 100644
@@ -312,3 +312,12 @@ config ARM_PXA2xx_CPUFREQ
  	  This add the CPUFreq driver support for Intel PXA2xx SOCs.
 
  	  If in doubt, say N.
+
+config ARM_QCOM_CPUFREQ_FW
+	bool "QCOM CPUFreq FW driver"
+	help
+	 Support for the CPUFreq FW driver.
+	 The CPUfreq FW preset in some QCOM chipsets offloads the steps
+	 necessary for changing the frequency of CPUs. The driver
+	 implements the cpufreq driver interface for this firmware.
+	 Say Y if you want to support CPUFreq FW.
drivers/cpufreq/Makefile +1 -0
index fb4a2ec..34691a2 100644
@@ -86,6 +86,7 @@ obj-$(CONFIG_ARM_TEGRA124_CPUFREQ)	+= tegra124-cpufreq.o
  obj-$(CONFIG_ARM_TEGRA186_CPUFREQ)	+= tegra186-cpufreq.o
  obj-$(CONFIG_ARM_TI_CPUFREQ)		+= ti-cpufreq.o
  obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ)	+= vexpress-spc-cpufreq.o
+obj-$(CONFIG_ARM_QCOM_CPUFREQ_FW)	+= qcom-cpufreq-fw.o
 
 
  ##################################################################################
drivers/cpufreq/qcom-cpufreq-fw.c new file +64 -0
new file mode 100644
index 0000000..2135a08
@@ -0,0 +1,316 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/cpufreq.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+
+#define INIT_RATE			300000000UL
+#define XO_RATE				19200000UL
+#define LUT_MAX_ENTRIES			40U
+#define CORE_COUNT_VAL(val)		(((val) & (GENMASK(18, 16))) >> 16)
+#define LUT_ROW_SIZE			32
+
+struct cpufreq_qcom {
+	struct cpufreq_frequency_table *table;
+	struct device *dev;
+	void __iomem *perf_base;
+	void __iomem *lut_base;
+	cpumask_t related_cpus;
+	unsigned int max_cores;
+};
+
+static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];
+
+static int
+qcom_cpufreq_fw_target_index(struct cpufreq_policy *policy,
+			     unsigned int index)
+{
+	struct cpufreq_qcom *c = policy->driver_data;
+
+	writel_relaxed(index, c->perf_base);
+
+	return 0;
+}
+
+static unsigned int qcom_cpufreq_fw_get(unsigned int cpu)
+{
+	struct cpufreq_qcom *c;
+	unsigned int index;
+
+	c = qcom_freq_domain_map[cpu];
+	if (!c)
+		return 0;
+
+	index = readl_relaxed(c->perf_base);
+	index = min(index, LUT_MAX_ENTRIES - 1);
+
+	return c->table[index].frequency;
+}
+
+static int qcom_cpufreq_fw_cpu_init(struct cpufreq_policy *policy)
+{
+	struct cpufreq_qcom *c;
+
+	c = qcom_freq_domain_map[policy->cpu];
+	if (!c) {
+		pr_err("No scaling support for CPU%d\n", policy->cpu);
+		return -ENODEV;
+	}
Quoted 9 lines

drivers/cpufreq/qcom-cpufreq-fw.c line 69

Quoted 5 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +5 -0
+
+	cpumask_copy(policy->cpus, &c->related_cpus);
+
+	policy->freq_table = c->table;
+	policy->driver_data = c;

What about fast cpufreq switching ? I think you can enable that option as well here..

Sure I will take a look and update.

Quoted 14 lines
Quoted 14 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +14 -0
+
+	return 0;
+}
+
+static struct freq_attr *qcom_cpufreq_fw_attr[] = {
+	&cpufreq_freq_attr_scaling_available_freqs,
+	&cpufreq_freq_attr_scaling_boost_freqs,
+	NULL
+};
+
+static struct cpufreq_driver cpufreq_qcom_fw_driver = {
+	.flags		= CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
+			  CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
+	.verify		= cpufreq_generic_frequency_table_verify,
Quoted 9 lines

drivers/cpufreq/qcom-cpufreq-fw.c line 86

Quoted 3 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +3 -0
+	.target_index	= qcom_cpufreq_fw_target_index,
+	.get		= qcom_cpufreq_fw_get,
+	.init		= qcom_cpufreq_fw_cpu_init,

What about CPU hotplug ? We can still do that, right ? So what will happen if all CPUs of a freq-domain are removed (hence cpufreq policy is removed) and then someone calls qcom_cpufreq_fw_get() ? You should really work on cpufreq_policy there to get 'c'.

You want the _get to do something as below.
Please correct me if my understanding is wrong.
....

policy = cpufreq_cpu_get_raw(cpu);
if (!policy)

    return 0;

c = policy->driver_data;

index = readl_relaxed(c->perf_base);
index = min(index, LUT_MAX_ENTRIES - 1);

return c->table[index].frequency;

....

Quoted 53 lines
Quoted 53 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +53 -0
+	.name		= "qcom-cpufreq-fw",
+	.attr		= qcom_cpufreq_fw_attr,
+	.boost_enabled	= true,
+};
+
+static int qcom_read_lut(struct platform_device *pdev,
+			 struct cpufreq_qcom *c)
+{
+	struct device *dev = &pdev->dev;
+	u32 data, src, lval, i, core_count, prev_cc, prev_freq, cur_freq;
+
+	c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
+				sizeof(*c->table), GFP_KERNEL);
+	if (!c->table)
+		return -ENOMEM;
+
+	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
+		data = readl_relaxed(c->lut_base + i * LUT_ROW_SIZE);
+		src = ((data & GENMASK(31, 30)) >> 30);
+		lval = (data & GENMASK(7, 0));
+		core_count = CORE_COUNT_VAL(data);
+
+		if (!src)
+			c->table[i].frequency = INIT_RATE / 1000;
+		else
+			c->table[i].frequency = XO_RATE * lval / 1000;
+
+		cur_freq = c->table[i].frequency;
+
+		dev_dbg(dev, "index=%d freq=%d, core_count %d\n",
+			i, c->table[i].frequency, core_count);
+
+		if (core_count != c->max_cores)
+			cur_freq = CPUFREQ_ENTRY_INVALID;
+
+		/*
+		 * Two of the same frequencies with the same core counts means
+		 * end of table.
+		 */
+		if (i > 0 && c->table[i - 1].frequency ==
+		   c->table[i].frequency && prev_cc == core_count) {
+			struct cpufreq_frequency_table *prev = &c->table[i - 1];
+
+			if (prev_freq == CPUFREQ_ENTRY_INVALID)
+				prev->flags = CPUFREQ_BOOST_FREQ;
+			break;
+		}
+		prev_cc = core_count;
+		prev_freq = cur_freq;
+	}
+
+	c->table[i].frequency = CPUFREQ_TABLE_END;
+
Quoted 9 lines

drivers/cpufreq/qcom-cpufreq-fw.c line 141

Quoted 2 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +2 -0
+	return 0;
+}

Looks like there are many problems here.
- You are assigning prev_freq with cur_freq (which may be uninitialized local variable here).
- In this version, you never write CPUFREQ_ENTRY_INVALID to table[i].frequency, which looks wrong as well.

- The code to detect boost, would only enter for i > 0 and the prev_freq
would be initialized with the cur_freq.
- In the case where the core_count != max_cores, the cur_freq is marked INVALID, and when both prev_freq == cur_freq && prev_cc && cur_cc match, that is the time the prev table flags need to be updated. Marking the table[i].frequency as INVALID is not required as cur_freq is already marked with the same. Please correct me if you think otherwise.

Quoted 7 lines

drivers/cpufreq/qcom-cpufreq-fw.c line 145

Quoted 4 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +4 -0
+
+static int qcom_get_related_cpus(struct device_node *np, struct cpumask *m)
+{
+	struct device_node  *cpu_dev;

s/cpu_dev/cpu_np/

Quoted 12 lines

drivers/cpufreq/qcom-cpufreq-fw.c line 152

Quoted 7 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +7 -0
+	int cpu;
+
+	for_each_possible_cpu(cpu) {
+		cpu_dev = of_cpu_device_node_get(cpu);
+		if (!cpu_dev)
+			continue;
+		cpu_dev = of_parse_phandle(cpu_dev, "qcom,freq-domain", 0);

What's returned here is a pointer to the qcom,freq-domain node, and you assign that to a variable named cpu_dev. Either use two variables for different node types, or rename it to temp_np or something similar.

I would use a different node.

Quoted 68 lines
Quoted 68 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +68 -0
+		if (!cpu_dev)
+			continue;
+		if (cpu_dev == np)
+			cpumask_set_cpu(cpu, m);
+	}
+
+	if (cpumask_empty(m))
+		return -ENOENT;
+
+	return 0;
+}
+
+static int qcom_cpu_resources_init(struct platform_device *pdev,
+				   struct device_node *np)
+{
+	struct cpufreq_qcom *c;
+	struct resource res;
+	struct device *dev = &pdev->dev;
+	void __iomem *en_base;
+	int cpu, index, ret;
+
+	c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
+	if (!c)
+		return -ENOMEM;
+
+	index = of_property_match_string(np, "reg-names", "enable");
+	if (index < 0)
+		return index;
+
+	if (of_address_to_resource(np, index, &res))
+		return -ENOMEM;
+
+	en_base = devm_ioremap(dev, res.start, resource_size(&res));
+	if (!en_base) {
+		dev_err(dev, "Unable to map %s enable-base\n", np->name);
+		return -ENOMEM;
+	}
+
+	/* FW should be in enabled state to proceed */
+	if (!(readl_relaxed(en_base) & 0x1)) {
+		dev_err(dev, "%s firmware not enabled\n", np->name);
+		return -ENODEV;
+	}
+	devm_iounmap(&pdev->dev, en_base);
+
+	index = of_property_match_string(np, "reg-names", "perf");
+	if (index < 0)
+		return index;
+
+	if (of_address_to_resource(np, index, &res))
+		return -ENOMEM;
+
+	c->perf_base = devm_ioremap(dev, res.start, resource_size(&res));
+	if (!c->perf_base) {
+		dev_err(dev, "Unable to map %s perf-base\n", np->name);
+		return -ENOMEM;
+	}
+
+	index = of_property_match_string(np, "reg-names", "lut");
+	if (index < 0)
+		return index;
+
+	if (of_address_to_resource(np, index, &res))
+		return -ENOMEM;
+
+	c->lut_base = devm_ioremap(dev, res.start, resource_size(&res));
+	if (!c->lut_base) {
+		dev_err(dev, "Unable to map %s lut-base\n", np->name);
Quoted 9 lines

drivers/cpufreq/qcom-cpufreq-fw.c line 226

Quoted 6 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +6 -0
+		return -ENOMEM;
+	}
+
+	ret = qcom_get_related_cpus(np, &c->related_cpus);
+	if (ret) {
+		dev_err(dev, "%s failed to get core phandles\n", np->name);

Maybe write a more relevant error message here ?

Sure, would take care of it in the next patch.

Quoted 8 lines

drivers/cpufreq/qcom-cpufreq-fw.c line 230

Quoted 4 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +4 -0
+		return ret;
+	}
+
+	c->max_cores = cpumask_weight(&c->related_cpus);

Maybe remove the error checking conditional from qcom_get_related_cpus() and check !c->max_cores here for the same.

I would update it in the next patch.

Quoted 8 lines

drivers/cpufreq/qcom-cpufreq-fw.c line 236

Quoted 6 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +6 -0
+
+	ret = qcom_read_lut(pdev, c);
+	if (ret) {
+		dev_err(dev, "%s failed to read LUT\n", np->name);
+		return ret;
+	}

Enter a blank line here.

I would update in the next patch.

Quoted 21 lines

drivers/cpufreq/qcom-cpufreq-fw.c line 238

Quoted 3 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +2 -0
+	for_each_cpu(cpu, &c->related_cpus)
+		qcom_freq_domain_map[cpu] = c;

This whole setup looks a bit confusing to me. This is what you are doing essentially:

qcom_resources_init()

{
         for_each_possible_cpu() {
                 qcom_cpu_resources_init()
                 {
                         populate c->related_cpus;

                         for_each_related_cpu() {
                                 qcom_freq_domain_map[cpu] = c;
                         }
                 }
         }
}
Quoted 7 lines

So if there are 4 CPUs that share a freq domain, then you are allocating 'c' 4 times and (over)writing qcom_freq_domain_map[] for all these CPUs 4 times and finally keeping value of 'c' only once.

You must be running most of the work done in qcom_resources_init() only once per freq-domain.

Thanks, you are correct, I carried the earlier logic, would cleanup in the next patch.

Quoted 9 lines

drivers/cpufreq/qcom-cpufreq-fw.c line 245

Quoted 7 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +7 -0
+
+	return 0;
+}
+
+static int qcom_resources_init(struct platform_device *pdev)
+{
+	struct device_node *np, *cpu_dev;

cpu_dev is normally used in kernel for struct device *, maybe use cpu_np ?

I would update it in the next patch.

Quoted 12 lines
Quoted 12 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +11 -0
+	unsigned int cpu;
+	int ret;
+
+	for_each_possible_cpu(cpu) {
+		cpu_dev = of_cpu_device_node_get(cpu);
+		if (!cpu_dev) {
+			dev_err(&pdev->dev, "Failed to get cpu %d device\n",
+				cpu);
+			continue;
+		}
+
Quoted 9 lines

drivers/cpufreq/qcom-cpufreq-fw.c line 260

Quoted 4 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +4 -0
+		np = of_parse_phandle(cpu_dev, "qcom,freq-domain", 0);
+		if (!np) {
+			dev_err(&pdev->dev, "Failed to get freq-domain device\n");
+			continue;

I am not sure if we should continue or error out here. Why would you want only a group of CPUs to have this property set ? Or if you really have a case for that currently ?

I don't have any usecase, but I was of the opinion in case there is no freq domains attached to a cluster. But it might be better I error out.

Quoted 24 lines
Quoted 24 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +24 -0
+		}
+
+		of_node_put(cpu_dev);
+
+		ret = qcom_cpu_resources_init(pdev, np);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int qcom_cpufreq_fw_driver_probe(struct platform_device *pdev)
+{
+	int rc;
+
+	/* Get the bases of cpufreq for domains */
+	rc = qcom_resources_init(pdev);
+	if (rc) {
+		dev_err(&pdev->dev, "CPUFreq resource init failed\n");
+		return rc;
+	}
+
+	rc = cpufreq_register_driver(&cpufreq_qcom_fw_driver);
Quoted 9 lines

drivers/cpufreq/qcom-cpufreq-fw.c line 290

Quoted 6 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +6 -0
+	if (rc) {
+		dev_err(&pdev->dev, "CPUFreq FW driver failed to register\n");
+		return rc;
+	}
+
+	dev_info(&pdev->dev, "QCOM CPUFreq FW driver inited\n");

s/inited/initialized/ ?

Sure, will fix in the next patch.

Quoted 30 lines
Quoted 29 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +26 -1
+
+	return 0;
+}
+
+static const struct of_device_id match_table[] = {
+	{ .compatible = "qcom,cpufreq-fw" },
+	{}
+};
+
+static struct platform_driver qcom_cpufreq_fw_driver = {
+	.probe = qcom_cpufreq_fw_driver_probe,
+	.driver = {
+		.name = "qcom-cpufreq-fw",
+		.of_match_table = match_table,
+		.owner = THIS_MODULE,
+	},
+};
+
+static int __init qcom_cpufreq_fw_init(void)
+{
+	return platform_driver_register(&qcom_cpufreq_fw_driver);
+}
+subsys_initcall(qcom_cpufreq_fw_init);
+
+MODULE_DESCRIPTION("QCOM CPU Frequency FW");
+MODULE_LICENSE("GPL v2");
--

Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.

Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--
maintainer
#9 lore raw

Sorry for being late..

Quoted 25 lines · Taniya Das
Quoted 15 lines · Viresh Kumar

drivers/cpufreq/qcom-cpufreq-fw.c line 86

Quoted 7 lines · Taniya Das
drivers/cpufreq/qcom-cpufreq-fw.c new file +7 -0
+static struct cpufreq_driver cpufreq_qcom_fw_driver = {
+	.flags		= CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
+			  CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
+	.verify		= cpufreq_generic_frequency_table_verify,
+	.target_index	= qcom_cpufreq_fw_target_index,
+	.get		= qcom_cpufreq_fw_get,
+	.init		= qcom_cpufreq_fw_cpu_init,

What about CPU hotplug ? We can still do that, right ? So what will happen if all CPUs of a freq-domain are removed (hence cpufreq policy is removed) and then someone calls qcom_cpufreq_fw_get() ? You should really work on cpufreq_policy there to get 'c'.

You want the _get to do something as below.
Please correct me if my understanding is wrong.
....

policy = cpufreq_cpu_get_raw(cpu);
if (!policy)

    return 0;
Quoted 8 lines

c = policy->driver_data;

index = readl_relaxed(c->perf_base);
index = min(index, LUT_MAX_ENTRIES - 1);

return c->table[index].frequency;

....

Right.

Quoted 58 lines
Quoted 48 lines
Quoted 48 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +48 -0
+static int qcom_read_lut(struct platform_device *pdev,
+			 struct cpufreq_qcom *c)
+{
+	struct device *dev = &pdev->dev;
+	u32 data, src, lval, i, core_count, prev_cc, prev_freq, cur_freq;
+
+	c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
+				sizeof(*c->table), GFP_KERNEL);
+	if (!c->table)
+		return -ENOMEM;
+
+	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
+		data = readl_relaxed(c->lut_base + i * LUT_ROW_SIZE);
+		src = ((data & GENMASK(31, 30)) >> 30);
+		lval = (data & GENMASK(7, 0));
+		core_count = CORE_COUNT_VAL(data);
+
+		if (!src)
+			c->table[i].frequency = INIT_RATE / 1000;
+		else
+			c->table[i].frequency = XO_RATE * lval / 1000;
+
+		cur_freq = c->table[i].frequency;
+
+		dev_dbg(dev, "index=%d freq=%d, core_count %d\n",
+			i, c->table[i].frequency, core_count);
+
+		if (core_count != c->max_cores)
+			cur_freq = CPUFREQ_ENTRY_INVALID;
+
+		/*
+		 * Two of the same frequencies with the same core counts means
+		 * end of table.
+		 */
+		if (i > 0 && c->table[i - 1].frequency ==
+		   c->table[i].frequency && prev_cc == core_count) {
+			struct cpufreq_frequency_table *prev = &c->table[i - 1];
+
+			if (prev_freq == CPUFREQ_ENTRY_INVALID)
+				prev->flags = CPUFREQ_BOOST_FREQ;
+			break;
+		}
+		prev_cc = core_count;
+		prev_freq = cur_freq;
+	}
+
+	c->table[i].frequency = CPUFREQ_TABLE_END;
+
Quoted 9 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+	return 0;
+}

Looks like there are many problems here.
- You are assigning prev_freq with cur_freq (which may be uninitialized local variable here).
- In this version, you never write CPUFREQ_ENTRY_INVALID to table[i].frequency, which looks wrong as well.

Quoted 7 lines

- The code to detect boost, would only enter for i > 0 and the prev_freq would be initialized with the cur_freq.
- In the case where the core_count != max_cores, the cur_freq is marked INVALID, and when both prev_freq == cur_freq && prev_cc && cur_cc match, that is the time the prev table flags need to be updated. Marking the table[i].frequency as INVALID is not required as cur_freq is already marked with the same. Please correct me if you think otherwise.

Yeah but the value of cur_freq isn't written to the table entries now. This wasn't the case in the earlier version. Have a look at that one.

Signature · 1 line
viresh
author
#10 lore raw
Quoted 105 lines · Viresh Kumar

Sorry for being late..

Quoted 32 lines · Taniya Das
Quoted 15 lines · Viresh Kumar

drivers/cpufreq/qcom-cpufreq-fw.c line 86

Quoted 7 lines · Taniya Das
drivers/cpufreq/qcom-cpufreq-fw.c new file +7 -0
+static struct cpufreq_driver cpufreq_qcom_fw_driver = {
+	.flags		= CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
+			  CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
+	.verify		= cpufreq_generic_frequency_table_verify,
+	.target_index	= qcom_cpufreq_fw_target_index,
+	.get		= qcom_cpufreq_fw_get,
+	.init		= qcom_cpufreq_fw_cpu_init,

What about CPU hotplug ? We can still do that, right ? So what will happen if all CPUs of a freq-domain are removed (hence cpufreq policy is removed) and then someone calls qcom_cpufreq_fw_get() ? You should really work on cpufreq_policy there to get 'c'.

You want the _get to do something as below.
Please correct me if my understanding is wrong.
....

  policy = cpufreq_cpu_get_raw(cpu);
  if (!policy)
     return 0;

  c = policy->driver_data;

  index = readl_relaxed(c->perf_base);
  index = min(index, LUT_MAX_ENTRIES - 1);

  return c->table[index].frequency;
Quoted 1 line

....

Right.

Quoted 58 lines
Quoted 48 lines
Quoted 48 lines
drivers/cpufreq/qcom-cpufreq-fw.c new file +48 -0
+static int qcom_read_lut(struct platform_device *pdev,
+			 struct cpufreq_qcom *c)
+{
+	struct device *dev = &pdev->dev;
+	u32 data, src, lval, i, core_count, prev_cc, prev_freq, cur_freq;
+
+	c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
+				sizeof(*c->table), GFP_KERNEL);
+	if (!c->table)
+		return -ENOMEM;
+
+	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
+		data = readl_relaxed(c->lut_base + i * LUT_ROW_SIZE);
+		src = ((data & GENMASK(31, 30)) >> 30);
+		lval = (data & GENMASK(7, 0));
+		core_count = CORE_COUNT_VAL(data);
+
+		if (!src)
+			c->table[i].frequency = INIT_RATE / 1000;
+		else
+			c->table[i].frequency = XO_RATE * lval / 1000;
+
+		cur_freq = c->table[i].frequency;
+
+		dev_dbg(dev, "index=%d freq=%d, core_count %d\n",
+			i, c->table[i].frequency, core_count);
+
+		if (core_count != c->max_cores)
+			cur_freq = CPUFREQ_ENTRY_INVALID;
+
+		/*
+		 * Two of the same frequencies with the same core counts means
+		 * end of table.
+		 */
+		if (i > 0 && c->table[i - 1].frequency ==
+		   c->table[i].frequency && prev_cc == core_count) {
+			struct cpufreq_frequency_table *prev = &c->table[i - 1];
+
+			if (prev_freq == CPUFREQ_ENTRY_INVALID)
+				prev->flags = CPUFREQ_BOOST_FREQ;
+			break;
+		}
+		prev_cc = core_count;
+		prev_freq = cur_freq;
+	}
+
+	c->table[i].frequency = CPUFREQ_TABLE_END;
+
Quoted 9 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+	return 0;
+}

Looks like there are many problems here.
- You are assigning prev_freq with cur_freq (which may be uninitialized local variable here).
- In this version, you never write CPUFREQ_ENTRY_INVALID to table[i].frequency, which looks wrong as well.

Quoted 7 lines

- The code to detect boost, would only enter for i > 0 and the prev_freq would be initialized with the cur_freq.
- In the case where the core_count != max_cores, the cur_freq is marked INVALID, and when both prev_freq == cur_freq && prev_cc && cur_cc match, that is the time the prev table flags need to be updated. Marking the table[i].frequency as INVALID is not required as cur_freq is already marked with the same. Please correct me if you think otherwise.

Quoted 3 lines

Yeah but the value of cur_freq isn't written to the table entries now. This wasn't the case in the earlier version. Have a look at that one.

Yes, Viresh, earlier code was updating the table frequency as I was marking the table frequency INVALID.

	if (core_count != c->max_cores)	
		c->table[i].frequency = CPUFREQ_ENTRY_INVALID;

And thus I had to update the table frequency.

But now I have used the cur_freq instead and the table frequency is not touched.

	if (core_count != c->max_cores)
		cur_freq = CPUFREQ_ENTRY_INVALID;
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--
maintainer
#11 lore raw
Quoted 11 lines · Taniya Das

Yes, Viresh, earlier code was updating the table frequency as I was marking the table frequency INVALID.

	if (core_count != c->max_cores)	
		c->table[i].frequency = CPUFREQ_ENTRY_INVALID;

And thus I had to update the table frequency.

But now I have used the cur_freq instead and the table frequency is not touched.

	if (core_count != c->max_cores)
		cur_freq = CPUFREQ_ENTRY_INVALID;

Unless I am reading your versions incorrectly, they behave differently.

Until V2, if core_count wasn't equal to max_cores then the frequency was getting marked as INVALID straight away in the table itself. Now if the next freq is also same then you abort and overwrite the previous one as valid, but otherwise it remains INVALID for ever. And this last thing doesn't happen anymore.

So if in your table there are few frequency entries which aren't repeating, but the core_count != max_cores for some of them, they remain valid in the newer versions.

Signature · 1 line
viresh