cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
v7 cover cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
[v7]
* Updated the logic to check for related CPUs.
[v6]
* Renamed match table 'qcom_cpufreq_hw_match'.
* Renamed 'qcom_read_lut' to 'qcom_cpufreq_hw_read_lut'.
* Updated the logic to check for related CPUs at the beginning of the 'qcom_cpu_resources_init'.
* Use devm_ioremap_resource instead of devm_ioremap.
* Update the use of of_node_put to handle error conditions.
* Use policy->cached_resolved_idx in fast switch callback.
* Keep precalculated offsets 'reg_bases'.
* XO clock is taken from Device tree.
* Update documentation binding for clocks/clock-names.
* Minor comments in Kconfig.arm.
* Comments to move dev_info to dev_dbg.
[v5]
* Remove mapping different register regions of perf/lut/enable, instead map the entire HW region.
* Add reg_offset/cpufreq_qcom_std_offsets to be supplied as device data.
* Check of src == 0 during lut read.
* Add of_node_put(cpu_np) in qcom_get_related_cpus
* Update the qcom_cpu_resources_init for register offset data, and cleanup the related cpus to keep a single copy of CPUfreq.
* Replace FW with HW, update Kconfig, rename filename qcom-cpufreq-hw.c
* Update the documentation binding to reflect the changes of mapping the
* entire HW region.
[v4]
* Fixed console messages as per comments.
* Return error from qcom_resources_init()
in the cases where failed to get frequency domain.
* Rename cpu_dev to cpu_np in qcom_resources_init, qcom_get_related_cpus(). Also use temp variable freq_np in qcom_get_related_cpus().
* Update qcom_cpufreq_fw_get() to use the policy data to incoporate the hotplug use case.
* Update code to use of fast_switching.
* Check for !c->max_cores instead of cpumask_empty in qcom_get_related_cpus().
* Update the logic of assigning 'c' to qcom_freq_domain_map[cpu].
[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'.
Taniya Das (2):
dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings
cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
.../bindings/cpufreq/cpufreq-qcom-hw.txt | 172 ++++++++++ drivers/cpufreq/Kconfig.arm | 11 + drivers/cpufreq/Makefile | 1 + drivers/cpufreq/qcom-cpufreq-hw.c | 348 +++++++++++++++++++++ 4 files changed, 532 insertions(+) create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.c
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
PATCH v7 1/2 dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware 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 the hardware engine.
--- .../bindings/cpufreq/cpufreq-qcom-hw.txt | 172 +++++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +172 -0
@@ -0,0 +1,172 @@ +Qualcomm Technologies, Inc. CPUFREQ Bindings + +CPUFREQ HW 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-hw". + +- clocks + Usage: required + Value type: <phandle> From common clock binding. + Definition: clock handle for XO clock. + +- clock-names + Usage: required + Value type: <string> From common clock binding. + Definition: must be "xo". + +* 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 HW bases. + +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-hw { + compatible = "qcom,cpufreq-hw"; + + clocks = <&rpmhcc RPMH_CXO_CLK>; + clock-names = "xo"; + + #address-cells = <2>; + #size-cells = <2>; + ranges; + freq_domain_table0: freq_table0 { + reg = <0 0x17d43000 0 0x1400>; + }; + + freq_domain_table1: freq_table1 { + reg = <0 0x17d45800 0 0x1400>; + }; + }; +};
Signature · 2 lines
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
Re: [PATCH v7 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings
Quoting Taniya Das (2018-07-24 03:42:49)
Quoted 6 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +0
-0@@ -0,0 +1,172 @@
[...]
Quoted 21 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +21
-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-hw { + compatible = "qcom,cpufreq-hw"; + + clocks = <&rpmhcc RPMH_CXO_CLK>; + clock-names = "xo"; +
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt line 170
Quoted 10 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +10
-0+ #address-cells = <2>; + #size-cells = <2>; + ranges; + freq_domain_table0: freq_table0 { + reg = <0 0x17d43000 0 0x1400>; + }; + + freq_domain_table1: freq_table1 { + reg = <0 0x17d45800 0 0x1400>; + };
Sorry, this is just not proper DT design. The whole node should have a reg property, and it should contain two (or three if we're handling the L3 clk domain?) different offsets for the different power clusters. The problem seems to still be that we don't have a way to map the CPUs to the clk domains they're in provided by this hardware block. Making subnodes is not the solution.
Re: [PATCH v7 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings
Quoted 38 lines · Stephen Boyd
Quoting Taniya Das (2018-07-24 03:42:49)
Quoted 6 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +0
-0@@ -0,0 +1,172 @@[...]
Quoted 30 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +30
-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-hw { + compatible = "qcom,cpufreq-hw"; + + clocks = <&rpmhcc RPMH_CXO_CLK>; + clock-names = "xo"; + + #address-cells = <2>; + #size-cells = <2>; + ranges; + freq_domain_table0: freq_table0 { + reg = <0 0x17d43000 0 0x1400>; + }; + + freq_domain_table1: freq_table1 { + reg = <0 0x17d45800 0 0x1400>;
Quoted 8 lines
Quoted 1 line
+ };
Sorry, this is just not proper DT design. The whole node should have a reg property, and it should contain two (or three if we're handling the L3 clk domain?) different offsets for the different power clusters. The problem seems to still be that we don't have a way to map the CPUs to the clk domains they're in provided by this hardware block. Making subnodes is not the solution.
The problem is mapping clock domains to logical CPUs that CPUfreq uses. The physical CPU to logical CPU mapping can be changed by the kernel (even through DT if I'm not mistaken). So we need to have a way to tell in DT which physical CPUs are connected to which CPU freq clock domain.
As for subnodes or not, we don't have any strong opinion, but couple of other points to consider. Two or more CPUfreq policies might have a common frequency table (read from HW), but separate control of frequency. So, you also need a way to group frequency table with CPU freq policies. If you have a better design, we are open to that suggestion.
Thanks,
Saravana
Re: [PATCH v7 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings
Quoted 50 lines · On Mon, Aug 06, 2018 at 01:54:24PM -0700, [email protected] wrote:
Quoted 40 lines · Stephen Boyd
Quoting Taniya Das (2018-07-24 03:42:49)
Quoted 8 lines
diff --git
a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txtb/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txtnew file mode 100644
index 0000000..22d4355Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +0
-0@@ -0,0 +1,172 @@[...]
Quoted 30 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +30
-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-hw { + compatible = "qcom,cpufreq-hw"; + + clocks = <&rpmhcc RPMH_CXO_CLK>; + clock-names = "xo"; + + #address-cells = <2>; + #size-cells = <2>; + ranges; + freq_domain_table0: freq_table0 { + reg = <0 0x17d43000 0 0x1400>; + }; + + freq_domain_table1: freq_table1 { + reg = <0 0x17d45800 0 0x1400>;Quoted 8 lines
Quoted 1 line
+ };
Sorry, this is just not proper DT design. The whole node should have a reg property, and it should contain two (or three if we're handling the L3 clk domain?) different offsets for the different power clusters. The problem seems to still be that we don't have a way to map the CPUs to the clk domains they're in provided by this hardware block. Making subnodes is not the solution.
Quoted 5 lines
The problem is mapping clock domains to logical CPUs that CPUfreq uses. The physical CPU to logical CPU mapping can be changed by the kernel (even through DT if I'm not mistaken). So we need to have a way to tell in DT which physical CPUs are connected to which CPU freq clock domain.
How about passing CPU freq clock domain id as along with phandle in qcom,freq-domain ?
--
Regards,
Sudeep
Re: [PATCH v7 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings
Quoted 51 lines · Sudeep Holla
Quoted 50 lines · On Mon, Aug 06, 2018 at 01:54:24PM -0700, [email protected] wrote:
Quoted 48 lines · Stephen Boyd
Quoting Taniya Das (2018-07-24 03:42:49)
Quoted 8 lines
diff --git
a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txtb/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txtnew file mode 100644
index 0000000..22d4355Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +0
-0@@ -0,0 +1,172 @@[...]
Quoted 21 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +21
-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-hw { + compatible = "qcom,cpufreq-hw"; + + clocks = <&rpmhcc RPMH_CXO_CLK>; + clock-names = "xo"; +Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt line 170
Quoted 10 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +10
-0+ #address-cells = <2>; + #size-cells = <2>; + ranges; + freq_domain_table0: freq_table0 { + reg = <0 0x17d43000 0 0x1400>; + }; + + freq_domain_table1: freq_table1 { + reg = <0 0x17d45800 0 0x1400>; + };Sorry, this is just not proper DT design. The whole node should have a reg property, and it should contain two (or three if we're handling the L3 clk domain?) different offsets for the different power clusters. The problem seems to still be that we don't have a way to map the CPUs to the clk domains they're in provided by this hardware block. Making subnodes is not the solution.
Quoted 8 lines
Quoted 5 lines
The problem is mapping clock domains to logical CPUs that CPUfreq uses. The physical CPU to logical CPU mapping can be changed by the kernel (even through DT if I'm not mistaken). So we need to have a way to tell in DT which physical CPUs are connected to which CPU freq clock domain.
How about passing CPU freq clock domain id as along with phandle in qcom,freq-domain ?
Now sure what you mean here. There's no such this as CPUfreq clock domain id. It has policies that are made up of logical CPU numbers. Logical CPU is not something that you can fix in DT.
-Saravana
Re: [PATCH v7 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings
Quoted 58 lines · On 8/8/2018 12:54 AM, [email protected] wrote:
Quoted 57 lines · Sudeep Holla
Quoted 55 lines · On Mon, Aug 06, 2018 at 01:54:24PM -0700, [email protected] wrote:
Quoted 40 lines · Stephen Boyd
Quoting Taniya Das (2018-07-24 03:42:49)
Quoted 8 lines
diff --git
a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txtb/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txtnew file mode 100644
index 0000000..22d4355Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +0
-0@@ -0,0 +1,172 @@[...]
Quoted 30 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +30
-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-hw { + compatible = "qcom,cpufreq-hw"; + + clocks = <&rpmhcc RPMH_CXO_CLK>; + clock-names = "xo"; + + #address-cells = <2>; + #size-cells = <2>; + ranges; + freq_domain_table0: freq_table0 { + reg = <0 0x17d43000 0 0x1400>; + }; + + freq_domain_table1: freq_table1 { + reg = <0 0x17d45800 0 0x1400>;Quoted 8 lines
Quoted 1 line
+ };
Sorry, this is just not proper DT design. The whole node should have a reg property, and it should contain two (or three if we're handling the L3 clk domain?) different offsets for the different power clusters. The problem seems to still be that we don't have a way to map the CPUs to the clk domains they're in provided by this hardware block. Making subnodes is not the solution.
The problem is mapping clock domains to logical CPUs that CPUfreq uses. The physical CPU to logical CPU mapping can be changed by the kernel (even through DT if I'm not mistaken). So we need to have a way to tell in DT which physical CPUs are connected to which CPU freq clock domain.
Quoted 6 lines
Quoted 2 lines
How about passing CPU freq clock domain id as along with phandle in qcom,freq-domain ?
Now sure what you mean here. There's no such this as CPUfreq clock domain id. It has policies that are made up of logical CPU numbers. Logical CPU is not something that you can fix in DT.
-Saravana
Sudeep,
Earlier the design was the freq_domain would take the CPU phandles
freq_domain:
cpus = <&cpu0 &cpu1....>;
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
Re: [PATCH v7 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings
Quoting Taniya Das (2018-08-07 19:46:01)
Quoted 69 lines · On 8/8/2018 12:54 AM, [email protected] wrote:
Quoted 66 lines · Sudeep Holla
Quoted 58 lines · On Mon, Aug 06, 2018 at 01:54:24PM -0700, [email protected] wrote:
Quoted 50 lines · Stephen Boyd
Quoted 40 lines
Quoting Taniya Das (2018-07-24 03:42:49)
Quoted 8 lines
diff --git
a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txtb/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txtnew file mode 100644
index 0000000..22d4355Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +0
-0@@ -0,0 +1,172 @@[...]
Quoted 30 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +30
-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-hw { + compatible = "qcom,cpufreq-hw"; + + clocks = <&rpmhcc RPMH_CXO_CLK>; + clock-names = "xo"; + + #address-cells = <2>; + #size-cells = <2>; + ranges; + freq_domain_table0: freq_table0 { + reg = <0 0x17d43000 0 0x1400>; + }; + + freq_domain_table1: freq_table1 { + reg = <0 0x17d45800 0 0x1400>;Quoted 8 lines
Quoted 1 line
+ };
Sorry, this is just not proper DT design. The whole node should have a reg property, and it should contain two (or three if we're handling the L3 clk domain?) different offsets for the different power clusters. The problem seems to still be that we don't have a way to map the CPUs to the clk domains they're in provided by this hardware block. Making subnodes is not the solution.
Quoted 6 lines
The problem is mapping clock domains to logical CPUs that CPUfreq uses. The
physical CPU to logical CPU mapping can be changed by the kernel (even through DT if I'm not mistaken). So we need to have a way to tell in DT which physical CPUs are connected to which CPU freq clock domain.Quoted 2 lines
How about passing CPU freq clock domain id as along with phandle in qcom,freq-domain ?
Now sure what you mean here. There's no such this as CPUfreq clock domain id. It has policies that are made up of logical CPU numbers. Logical CPU is not something that you can fix in DT.
Quoted 9 lines
Quoted 1 line
-Saravana
Sudeep,
Earlier the design was the
freq_domainwould take the CPU phandles
freq_domain:cpus = <&cpu0 &cpu1....>;
I believe Sudeep is recommending something I recommended earlier. It would look like:
cpu7 {
qcom,freq-domain = <&cpufreq_hw 1>;}
to indicate that cpu7 is in cpufreq_hw's frequency domain #1. That should probably be called clk domain BTW.
If that was done with a phandle and a single cell, then we should have something similar on the cpufreq_hw node side indicating how to parse the cells in qcom,freq-domain. A property like #qcom,freq-domain-cells = <1> to indicate that one u32 follows the phandle.
Re: [PATCH v7 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings
Quoted 1 line · Stephen Boyd
Quoting Taniya Das (2018-08-07 19:46:01)
[...]
Quoted 11 lines
Quoted 7 lines
Sudeep,
Earlier the design was the
freq_domainwould take the CPU phandles
freq_domain:cpus = <&cpu0 &cpu1....>;I believe Sudeep is recommending something I recommended earlier. It would look like:
Quoted 7 lines
cpu7 { qcom,freq-domain = <&cpufreq_hw 1>; }to indicate that cpu7 is in cpufreq_hw's frequency domain #1. That should probably be called clk domain BTW.
Thanks Stephen, that's exactly what I meant. You have explained all the details saving me time :)
Quoted 5 lines
If that was done with a phandle and a single cell, then we should have something similar on the
cpufreq_hwnode side indicating how to parse the cells in qcom,freq-domain. A property like #qcom,freq-domain-cells = <1> to indicate that one u32 follows the phandle.
As Saravana mentions in the other email I can't believe it's just made up of logical CPU numbers. If that's the case is it software configurable.
--
Regards,
Sudeep
PATCH v7 2/2 cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
In reply to Taniya DasThe CPUfreq HW present in some QCOM chipsets offloads the steps necessary for changing the frequency of CPUs. The driver implements the cpufreq driver interface for this hardware engine.
--- drivers/cpufreq/Kconfig.arm | 11 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/qcom-cpufreq-hw.c | 348 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 360 insertions(+) create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.c
drivers/cpufreq/Kconfig.arm +11 -0
@@ -298,3 +298,14 @@ config ARM_PXA2xx_CPUFREQ This add the CPUFreq driver support for Intel PXA2xx SOCs. If in doubt, say N. + +config ARM_QCOM_CPUFREQ_HW + bool "QCOM CPUFreq HW driver" + depends on ARCH_QCOM + help + Support for the CPUFreq HW driver. + Some QCOM chipsets have a HW engine to offload the steps + necessary for changing the frequency of the CPUs. Firmware loaded + in this engine exposes a programming interface to the OS. + The driver implements the cpufreq interface for this HW engine. + Say Y if you want to support CPUFreq HW.
drivers/cpufreq/Makefile +1 -0
@@ -85,6 +85,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_HW) += qcom-cpufreq-hw.o ##################################################################################
drivers/cpufreq/qcom-cpufreq-hw.c new file +348 -0
348 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.
Hi Taniya,
Quoted 14 lines · Taniya Das
The CPUfreq HW present in some QCOM chipsets offloads the steps necessary for changing the frequency of CPUs. The driver implements the cpufreq driver interface for this hardware engine.
Signed-off-by Saravana Kannan <[email protected]>Signed-off-by Taniya Das <[email protected]>--- drivers/cpufreq/Kconfig.arm | 11 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/qcom-cpufreq-hw.c | 348 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 360 insertions(+) create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.c
...
Quoted 5 lines
drivers/cpufreq/qcom-cpufreq-hw.c new file +0
-0
...
Quoted 14 lines
drivers/cpufreq/qcom-cpufreq-hw.c new file +14
-0+static int qcom_cpufreq_hw_read_lut (struct platform_device *pdev, +struct cpufreq_qcom *c) +{ +struct device *dev = &pdev->dev; +void __iomem *base; +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 ; + + base = c->reg_bases[REG_LUT_TABLE ]; +
drivers/cpufreq/qcom-cpufreq-hw.c line 149
Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-hw.c new file +10
-0+for (i =0 ; i <LUT_MAX_ENTRIES ; i++) { + data =readl_relaxed (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 = c->xo_rate * lval /1000 ; +else + c->table[i].frequency =INIT_RATE /1000 ;
I don't know much about how this hardware works, but based on the mask, src has 4 possible values. So does 0 mean INIT_RATE, and 1, 2, and 3 all mean xo_rate?
Also, is INIT_RATE really constant? It sounds like gpll0 (or gpll0_out_even?). You're already getting the xo clock, why not get gpll0's real rate as well?
Quoted 56 lines · Evan Green
Hi Taniya,
Quoted 15 lines · Taniya Das
The CPUfreq HW present in some QCOM chipsets offloads the steps necessary for changing the frequency of CPUs. The driver implements the cpufreq driver interface for this hardware engine.
Signed-off-by Saravana Kannan <[email protected]>Signed-off-by Taniya Das <[email protected]>--- drivers/cpufreq/Kconfig.arm | 11 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/qcom-cpufreq-hw.c | 348 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 360 insertions(+) create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.c...
Quoted 5 lines
drivers/cpufreq/qcom-cpufreq-hw.c new file +0
-0...
Quoted 24 lines
drivers/cpufreq/qcom-cpufreq-hw.c new file +6
-0+static int qcom_cpufreq_hw_read_lut (struct platform_device *pdev, +struct cpufreq_qcom *c) +{ +struct device *dev = &pdev->dev; +void __iomem *base; + u32 data, src, lval, i, core_count, prev_cc, prev_freq,
cur_freq;drivers/cpufreq/qcom-cpufreq-hw.c new file +16
-0+ + c->table =devm_kcalloc (dev,LUT_MAX_ENTRIES +1 , +sizeof (*c->table),GFP_KERNEL ); +if (!c->table) +return -ENOMEM ; + + base = c->reg_bases[REG_LUT_TABLE ]; + +for (i =0 ; i <LUT_MAX_ENTRIES ; i++) { + data =readl_relaxed (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 = c->xo_rate * lval /1000;
drivers/cpufreq/qcom-cpufreq-hw.c line 149
Quoted 2 lines
drivers/cpufreq/qcom-cpufreq-hw.c new file +2
-0+else + c->table[i].frequency =INIT_RATE /1000 ;I don't know much about how this hardware works, but based on the mask, src has 4 possible values. So does 0 mean
INIT_RATE, and 1, 2, and 3 all meanxo_rate?
Quoted 3 lines
Also, is
INIT_RATEreally constant? It sounds like gpll0 (orgpll0_out_even?). You're already getting the xo clock, why not get gpll0's real rate as well?
Actually I was about to comment and say NOT to get clocks just to get their rate. The XO_RATE is just a multiplication factor. This HW/FW can change in the future and make the multiplication factor to 1KHz. We also can't really control any of the clocks going to this block from Linux (it's all locked down). Adding clk_get significantly delays when this driver can be probed and increases boot up time. The INIT_RATE will always be 300 MHz independent of what source it's coming from (as in, if GPLL0 can't give 300, the HW design will be so that we'll find a different source).
So, I'd like to remove any clock bindings for this driver.
Thanks,
Saravana
Quoting [email protected] (2018-08-03 12:52:48)
Quoted 15 lines · Evan Green
Quoted 14 lines
Hi Taniya,
drivers/cpufreq/qcom-cpufreq-hw.c line 149
Quoted 6 lines · Taniya Das
(quoted diff) +2
-0+ if (src) + c->table[i].frequency = c->xo_rate * lval /1000;
drivers/cpufreq/qcom-cpufreq-hw.c new file +2
-0+else + c->table[i].frequency =INIT_RATE /1000 ;I don't know much about how this hardware works, but based on the mask, src has 4 possible values. So does 0 mean
INIT_RATE, and 1, 2, and 3 all meanxo_rate?
Quoted 7 lines
Quoted 3 lines
Also, is
INIT_RATEreally constant? It sounds like gpll0 (orgpll0_out_even?). You're already getting the xo clock, why not get gpll0's real rate as well?Actually I was about to comment and say NOT to get clocks just to get their rate. The
XO_RATEis just a multiplication factor. This HW/FW can change in the future and make the multiplication factor to 1KHz.
So future changes to this hardware are going to make this register return the final frequency that we should use? Sounds great! But that isn't how it's working right now. We need to have XO in the binding here so the driver can work with different XO frequencies in case that ever happens. Same story for GPLL0. Hardcoding this stuff in the driver just means we'll have to swizzle things in the driver when it changes.
Quoted 3 lines
We also
can't really control any of the clocks going to this block from Linux (it's all locked down).
This shouldn't matter. The clocks going to this hardware block are described by the firmware to the OS by means of the DT node. If the firmware or the hardware decides to change the input clks then the binding can have different clk nodes used.
Quoted 2 lines
Adding
clk_getsignificantly delays when this
driver can be probed and increases boot up time.
Huh? Please fix your bootloader to increase the CPU frequency before booting the kernel. I really doubt probe defer is going to happen if we send GPLL0 here (XO is a DT clk so it's definitely registered before this driver), and trying to work around probe defer because the CPU won't go fast before that is papering over a larger problem with both probe defer and bootloaders not supplying enough CPU speed for you.
Quoted 6 lines
The
INIT_RATEwill
always be 300 MHz independent of what source it's coming from (as in, if GPLL0 can't give 300, the HW design will be so that we'll find a different source).So, I'd like to remove any clock bindings for this driver.
No. Bindings describe how the hardware is connected. Please don't remove clocks from the binding just because probe defer is a concern.
Quoted 24 lines · Stephen Boyd
Quoting [email protected] (2018-08-03 12:52:48)
Quoted 15 lines · Evan Green
Quoted 14 lines
Hi Taniya,
drivers/cpufreq/qcom-cpufreq-hw.c line 149
Quoted 6 lines · Taniya Das
(quoted diff) +2
-0+ if (src) + c->table[i].frequency = c->xo_rate * lval /1000;
drivers/cpufreq/qcom-cpufreq-hw.c new file +2
-0+else + c->table[i].frequency =INIT_RATE /1000 ;I don't know much about how this hardware works, but based on the mask, src has 4 possible values. So does 0 mean
INIT_RATE, and 1, 2, and 3 all meanxo_rate?Quoted 7 lines
Quoted 3 lines
Also, is
INIT_RATEreally constant? It sounds like gpll0 (orgpll0_out_even?). You're already getting the xo clock, why not get gpll0's real rate as well?Actually I was about to comment and say NOT to get clocks just to get their rate. The
XO_RATEis just a multiplication factor. This HW/FW can change in the future and make the multiplication factor to 1KHz.
Quoted 6 lines
So future changes to this hardware are going to make this register return the final frequency that we should use? Sounds great! But that isn't how it's working right now. We need to have XO in the binding here so the driver can work with different XO frequencies in case that ever happens. Same story for GPLL0. Hardcoding this stuff in the driver just means we'll have to swizzle things in the driver when it changes.
XO being a different frequency in a Qualcomm chip is way way less likely than anything else we are discussing here. In the 10+years I've been there this has never changed.
So, how about making this binding optional? If it's not present we can make assumptions for XO rate and init rate. That'll handle your hypothetical use case while also being optimized.
Quoted 8 lines
Quoted 3 lines
We also
can't really control any of the clocks going to this block from Linux (it's all locked down).This shouldn't matter. The clocks going to this hardware block are described by the firmware to the OS by means of the DT node. If the firmware or the hardware decides to change the input clks then the binding can have different clk nodes used.
There are tons of clocks that are input to blocks in an SoC that are never controlled by Linux. Or are expose in DT because they are set up ahead of time and can't change. So, why make an exception here?
Quoted 9 lines
Quoted 2 lines
Adding
clk_getsignificantly delays when this
driver can be probed and increases boot up time.Huh? Please fix your bootloader to increase the CPU frequency before booting the kernel. I really doubt probe defer is going to happen if we send GPLL0 here (XO is a DT clk so it's definitely registered before this driver), and trying to work around probe defer because the CPU won't go fast before that is papering over a larger problem with both probe defer and bootloaders not supplying enough CPU speed for you.
There are multiple reason the bootloader can't run the CPU faster. For one, it doesn't have thermal handling, etc. And adding all those features into the bootloader doesn't make sense when Linux already has support for it.
Quoted 9 lines
Quoted 6 lines
The
INIT_RATEwill
always be 300 MHz independent of what source it's coming from (as in, if GPLL0 can't give 300, the HW design will be so that we'll find a different source).So, I'd like to remove any clock bindings for this driver.
No. Bindings describe how the hardware is connected. Please don't remove clocks from the binding just because probe defer is a concern.
Binding describes hardware controllable by the OS. That's the reality. Let's not add mandatory clock bindings for clocks that the OS can't do anything about.
-Saravana
Quoting [email protected] (2018-08-06 13:46:05)
Quoted 34 lines · Stephen Boyd
Quoted 25 lines
Quoting [email protected] (2018-08-03 12:52:48)
Quoted 15 lines · Evan Green
Quoted 14 lines
Hi Taniya,
drivers/cpufreq/qcom-cpufreq-hw.c line 149
Quoted 6 lines · Taniya Das
(quoted diff) +2
-0+ if (src) + c->table[i].frequency = c->xo_rate * lval /1000;
drivers/cpufreq/qcom-cpufreq-hw.c new file +2
-0+else + c->table[i].frequency =INIT_RATE /1000 ;I don't know much about how this hardware works, but based on the mask, src has 4 possible values. So does 0 mean
INIT_RATE, and 1, 2, and 3 all meanxo_rate?Quoted 8 lines
Quoted 3 lines
Also, is
INIT_RATEreally constant? It sounds like gpll0 (orgpll0_out_even?). You're already getting the xo clock, why not get gpll0's real rate as well?Actually I was about to comment and say NOT to get clocks just to get their rate. The
XO_RATEis just a multiplication factor. This HW/FW can
change in the future and make the multiplication factor to 1KHz.Quoted 7 lines
So future changes to this hardware are going to make this register return the final frequency that we should use? Sounds great! But that isn't how it's working right now. We need to have XO in the binding here
so the driver can work with different XO frequencies in case that ever happens. Same story for GPLL0. Hardcoding this stuff in the driver just means we'll have to swizzle things in the driver when it changes.
Quoted 7 lines
XO being a different frequency in a Qualcomm chip is way way less likely than anything else we are discussing here. In the 10+years I've been there this has never changed.
So, how about making this binding optional? If it's not present we can make assumptions for XO rate and init rate. That'll handle your hypothetical use case while also being optimized.
Optional properties are almost never correct for clks. Either the clk goes there or it doesn't go there. The only time it's optional is if the HW has the choice of generating the clk itself internally or to use an external clk as input.
In this case, it's not optional, it's actually used so marking it optional is highly confusing.
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
We also
can't really control any of the clocks going to this block from Linux (it's all locked down).
Quoted 8 lines
Quoted 4 lines
This shouldn't matter. The clocks going to this hardware block are described by the firmware to the OS by means of the DT node. If the firmware or the hardware decides to change the input clks then the binding can have different clk nodes used.
There are tons of clocks that are input to blocks in an SoC that are never controlled by Linux. Or are expose in DT because they are set up ahead of time and can't change. So, why make an exception here?
Because the driver doesn't have to hard-code some frequency that can easily come from the DT, making it more flexible for frequency plan changes. It doesn't matter about control of clks at all here, so this comparison is just plain wrong.
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
The
INIT_RATEwill
always be 300 MHz independent of what source it's coming from (as in, if
GPLL0 can't give 300, the HW design will be so that we'll find a different source).So, I'd like to remove any clock bindings for this driver.
Quoted 8 lines
Quoted 3 lines
No. Bindings describe how the hardware is connected. Please don't remove
clocks from the binding just because probe defer is a concern.Binding describes hardware controllable by the OS. That's the reality. Let's not add mandatory clock bindings for clocks that the OS can't do anything about.
It seems that you believe clks should only be used to turn on/off and control rates. That is not the whole truth. Sometimes clks are there just to express the clk frequencies that are present in the design so that drivers can figure out what to do.
Quoted 88 lines · Stephen Boyd
Quoting [email protected] (2018-08-06 13:46:05)
Quoted 34 lines · Stephen Boyd
Quoted 25 lines
Quoting [email protected] (2018-08-03 12:52:48)
Quoted 15 lines · Evan Green
Quoted 14 lines
Hi Taniya,
drivers/cpufreq/qcom-cpufreq-hw.c line 149
Quoted 6 lines · Taniya Das
(quoted diff) +2
-0+ if (src) + c->table[i].frequency = c->xo_rate * lval /1000;
drivers/cpufreq/qcom-cpufreq-hw.c new file +2
-0+else + c->table[i].frequency =INIT_RATE /1000 ;I don't know much about how this hardware works, but based on the mask, src has 4 possible values. So does 0 mean
INIT_RATE, and 1, 2, and 3 all meanxo_rate?Quoted 8 lines
Quoted 3 lines
Also, is
INIT_RATEreally constant? It sounds like gpll0 (orgpll0_out_even?). You're already getting the xo clock, why not get gpll0's real rate as well?Actually I was about to comment and say NOT to get clocks just to get their rate. The
XO_RATEis just a multiplication factor. This HW/FW can
change in the future and make the multiplication factor to 1KHz.Quoted 7 lines
So future changes to this hardware are going to make this register return the final frequency that we should use? Sounds great! But that isn't how it's working right now. We need to have XO in the binding here
so the driver can work with different XO frequencies in case that ever happens. Same story for GPLL0. Hardcoding this stuff in the driver just means we'll have to swizzle things in the driver when it changes.Quoted 7 lines
XO being a different frequency in a Qualcomm chip is way way less likely than anything else we are discussing here. In the 10+years I've been there this has never changed.
So, how about making this binding optional? If it's not present we can make assumptions for XO rate and init rate. That'll handle your hypothetical use case while also being optimized.
Optional properties are almost never correct for clks. Either the clk goes there or it doesn't go there. The only time it's optional is if the HW has the choice of generating the clk itself internally or to use an external clk as input.
In this case, it's not optional, it's actually used so marking it optional is highly confusing.
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
We also
can't really control any of the clocks going to this block from Linux (it's all locked down).Quoted 8 lines
Quoted 4 lines
This shouldn't matter. The clocks going to this hardware block are described by the firmware to the OS by means of the DT node. If the firmware or the hardware decides to change the input clks then the binding can have different clk nodes used.
There are tons of clocks that are input to blocks in an SoC that are never controlled by Linux. Or are expose in DT because they are set up ahead of time and can't change. So, why make an exception here?
Because the driver doesn't have to hard-code some frequency that can easily come from the DT, making it more flexible for frequency plan changes. It doesn't matter about control of clks at all here, so this comparison is just plain wrong.
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
The
INIT_RATEwill
always be 300 MHz independent of what source it's coming from (as in, if
GPLL0 can't give 300, the HW design will be so that we'll find a different source).So, I'd like to remove any clock bindings for this driver.
Quoted 8 lines
Quoted 3 lines
No. Bindings describe how the hardware is connected. Please don't remove
clocks from the binding just because probe defer is a concern.Binding describes hardware controllable by the OS. That's the reality. Let's not add mandatory clock bindings for clocks that the OS can't do anything about.
Quoted 5 lines
It seems that you believe clks should only be used to turn on/off and control rates. That is not the whole truth. Sometimes clks are there just to express the clk frequencies that are present in the design so that drivers can figure out what to do.
Stephen,
As this clock is not configurable by linux clock drivers and we really do not care the parent src(as mentioned by Saravana) to generate the 300MHz, would it be good to define a fixed rate clock so as to express the HW connectivity & frequency?
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
Quoting Taniya Das (2018-08-08 03:15:26)
Quoted 15 lines · Stephen Boyd
Quoted 11 lines
Quoted 5 lines
Binding describes hardware controllable by the OS. That's the reality. Let's not add mandatory clock bindings for clocks that the OS can't do anything about.
It seems that you believe clks should only be used to turn on/off and control rates. That is not the whole truth. Sometimes clks are there just to express the clk frequencies that are present in the design so that drivers can figure out what to do.
Quoted 7 lines
Stephen,
As this clock is not configurable by linux clock drivers and we really do not care the parent src(as mentioned by Saravana) to generate the 300MHz, would it be good to define a fixed rate clock so as to express the HW connectivity & frequency?
As a hack that works great, but why do we need to workaround problems by adding a fixed rate clk to DT for this PLL? The PLL is provided by GCC node so it should be connected to the GCC node.
Hello Stephen,
Quoted 24 lines · Stephen Boyd
Quoting Taniya Das (2018-08-08 03:15:26)
Quoted 15 lines · Stephen Boyd
Quoted 11 lines
Quoted 5 lines
Binding describes hardware controllable by the OS. That's the reality. Let's not add mandatory clock bindings for clocks that the OS can't do anything about.
It seems that you believe clks should only be used to turn on/off and control rates. That is not the whole truth. Sometimes clks are there just to express the clk frequencies that are present in the design so that drivers can figure out what to do.
Quoted 7 lines
Stephen,
As this clock is not configurable by linux clock drivers and we really do not care the parent src(as mentioned by Saravana) to generate the 300MHz, would it be good to define a fixed rate clock so as to express the HW connectivity & frequency?
Quoted 4 lines
As a hack that works great, but why do we need to workaround problems by adding a fixed rate clk to DT for this PLL? The PLL is provided by GCC node so it should be connected to the GCC node.
Please help with review the next patch series which would take the PLL phandle from DT.
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
Hi Taniya,
Quoted 68 lines · Taniya Das
The CPUfreq HW present in some QCOM chipsets offloads the steps necessary for changing the frequency of CPUs. The driver implements the cpufreq driver interface for this hardware engine.
Signed-off-by Saravana Kannan <[email protected]>Signed-off-by Taniya Das <[email protected]>--- drivers/cpufreq/Kconfig.arm | 11 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/qcom-cpufreq-hw.c | 348 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 360 insertions(+) create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.cdrivers/cpufreq/Kconfig.arm +11
-0@@ -298,3 +298,14 @@ config ARM_PXA2xx_CPUFREQ This add the CPUFreq driver support for Intel PXA2xx SOCs. If in doubt, say N. + +config ARM_QCOM_CPUFREQ_HW + bool "QCOM CPUFreq HW driver" + depends on ARCH_QCOM + help + Support for the CPUFreq HW driver. + Some QCOM chipsets have a HW engine to offload the steps + necessary for changing the frequency of the CPUs. Firmware loaded + in this engine exposes a programming interface to the OS. + The driver implements the cpufreq interface for this HW engine. + Say Y if you want to support CPUFreq HW.drivers/cpufreq/Makefile +1
-0@@ -85,6 +85,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_HW) += qcom-cpufreq-hw.o ##################################################################################drivers/cpufreq/qcom-cpufreq-hw.c new file +18
-0@@ -0,0 +1,348 @@ +// 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 LUT_MAX_ENTRIES 40U +#define CORE_COUNT_VAL (val) (((val) & (GENMASK(18, 16))) >> 16) +#define LUT_ROW_SIZE 32 + +enum {
drivers/cpufreq/qcom-cpufreq-hw.c line 28
Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-hw.c new file +10
-0+REG_ENABLE , +REG_LUT_TABLE , +REG_PERF_STATE , + + REG_ARRAY_SIZE, +}; + +struct cpufreq_qcom { +struct cpufreq_frequency_table *table; +struct device *dev;
'dev' is not used and can be removed.
Quoted 58 lines
...
drivers/cpufreq/qcom-cpufreq-hw.c new file +56
-0+static int qcom_cpu_resources_init (struct platform_device *pdev, +struct device_node *np,unsigned int cpu, +unsigned long xo_rate) +{ +struct cpufreq_qcom *c; +struct resource res; +struct device *dev = &pdev->dev; +const u16 *offsets; +int ret, i, cpu_first, cpu_r; +void __iomem *base; + +if (qcom_freq_domain_map[cpu]) +return 0 ; + + c =devm_kzalloc (dev,sizeof (*c),GFP_KERNEL ); +if (!c) +return -ENOMEM ; + + offsets =of_device_get_match_data (&pdev->dev); +if (!offsets) +return -EINVAL ; + +if (of_address_to_resource (np,0 , &res)) +return -ENOMEM ; + + base =devm_ioremap_resource (dev, &res); +if (!base) +return -ENOMEM ; + +for (i =REG_ENABLE ; i <REG_ARRAY_SIZE ; i++) + c->reg_bases[i] = base + offsets[i]; + +/* HW should be in enabled state to proceed */ +if (!(readl_relaxed (c->reg_bases[REG_ENABLE ]) &0x1 )) { +dev_err (dev,"%s cpufreq hardware not enabled\n" , np->name); +return -ENODEV ; + } + + ret =qcom_get_related_cpus (np, &c->related_cpus); +if (ret) { +dev_err (dev,"%s failed to get related CPUs\n" , np->name); +return ret; + } + + c->max_cores =cpumask_weight (&c->related_cpus); +if (!c->max_cores) +return -ENOENT ; + + c->xo_rate = xo_rate; + + ret =qcom_cpufreq_hw_read_lut (pdev, c); +if (ret) { +dev_err (dev,"%s failed to read LUT\n" , np->name); +return ret; + } +
drivers/cpufreq/qcom-cpufreq-hw.c line 266
Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-hw.c new file +10
-0+ qcom_freq_domain_map[cpu] = c; + +/* Related CPUs */ + cpu_first =cpumask_first (&c->related_cpus); + +for_each_cpu (cpu_r, &c->related_cpus) { +if (cpu_r != cpu_first) + qcom_freq_domain_map[cpu_r] = + qcom_freq_domain_map[cpu_first]; + }
The above ten lines could be simplified to:
for_each_cpu(cpu_r, &c->related_cpus) qcom_freq_domain_map[cpu_r] = c;
drivers/cpufreq/qcom-cpufreq-hw.c line 346
Quoted 7 lines
...
drivers/cpufreq/qcom-cpufreq-hw.c new file +5
-0+static int __init qcom_cpufreq_hw_init (void ) +{ +return platform_driver_register (&qcom_cpufreq_hw_driver); +} +subsys_initcall (qcom_cpufreq_hw_init);
Is subsys_initcall used for a particular reason? It will cause problems when registering cooling devices, since the thermal device class is initialized through an fs_initcall, which are executed later.
Most cpufreq drivers use module_init, device_initcall or late_initcall, can't this driver use one of those?
Cheers
Matthias
Hello Matthias,
Thanks for your review comments.
Quoted 75 lines · Matthias Kaehlcke
Hi Taniya,
Quoted 72 lines · Taniya Das
The CPUfreq HW present in some QCOM chipsets offloads the steps necessary for changing the frequency of CPUs. The driver implements the cpufreq driver interface for this hardware engine.
Signed-off-by Saravana Kannan <[email protected]>Signed-off-by Taniya Das <[email protected]>--- drivers/cpufreq/Kconfig.arm | 11 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/qcom-cpufreq-hw.c | 348 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 360 insertions(+)create mode 100644
drivers/cpufreq/qcom-cpufreq-hw.cdrivers/cpufreq/Kconfig.arm +11
-0@@ -298,3 +298,14 @@ config ARM_PXA2xx_CPUFREQ This add the CPUFreq driver support for Intel PXA2xx SOCs. If in doubt, say N. + +config ARM_QCOM_CPUFREQ_HW + bool "QCOM CPUFreq HW driver" + depends on ARCH_QCOM + help + Support for the CPUFreq HW driver. + Some QCOM chipsets have a HW engine to offload the steps + necessary for changing the frequency of the CPUs. Firmware loaded + in this engine exposes a programming interface to the OS. + The driver implements the cpufreq interface for this HW engine. + Say Y if you want to support CPUFreq HW.drivers/cpufreq/Makefile +1
-0@@ -85,6 +85,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_HW) += qcom-cpufreq-hw.o ##################################################################################drivers/cpufreq/qcom-cpufreq-hw.c new file +22
-0@@ -0,0 +1,348 @@ +// 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 LUT_MAX_ENTRIES 40U +#define CORE_COUNT_VAL (val) (((val) & (GENMASK(18, 16))) >> 16) +#define LUT_ROW_SIZE 32 + +enum { +REG_ENABLE , +REG_LUT_TABLE , +REG_PERF_STATE , +
Quoted 9 lines
drivers/cpufreq/qcom-cpufreq-hw.c line 28
Quoted 6 lines
drivers/cpufreq/qcom-cpufreq-hw.c new file +6
-0+ REG_ARRAY_SIZE, +}; + +struct cpufreq_qcom { +struct cpufreq_frequency_table *table; +struct device *dev;'dev' is not used and can be removed.
Thanks, would remove in the next patch.
Quoted 71 lines
Quoted 58 lines
...
drivers/cpufreq/qcom-cpufreq-hw.c new file +56
-0+static int qcom_cpu_resources_init (struct platform_device *pdev, +struct device_node *np,unsigned int cpu, +unsigned long xo_rate) +{ +struct cpufreq_qcom *c; +struct resource res; +struct device *dev = &pdev->dev; +const u16 *offsets; +int ret, i, cpu_first, cpu_r; +void __iomem *base; + +if (qcom_freq_domain_map[cpu]) +return 0 ; + + c =devm_kzalloc (dev,sizeof (*c),GFP_KERNEL ); +if (!c) +return -ENOMEM ; + + offsets =of_device_get_match_data (&pdev->dev); +if (!offsets) +return -EINVAL ; + +if (of_address_to_resource (np,0 , &res)) +return -ENOMEM ; + + base =devm_ioremap_resource (dev, &res); +if (!base) +return -ENOMEM ; + +for (i =REG_ENABLE ; i <REG_ARRAY_SIZE ; i++) + c->reg_bases[i] = base + offsets[i]; + +/* HW should be in enabled state to proceed */ +if (!(readl_relaxed (c->reg_bases[REG_ENABLE ]) &0x1 )) { +dev_err (dev,"%s cpufreq hardware not enabled\n" , np->name); +return -ENODEV ; + } + + ret =qcom_get_related_cpus (np, &c->related_cpus); +if (ret) { +dev_err (dev,"%s failed to get related CPUs\n" , np->name); +return ret; + } + + c->max_cores =cpumask_weight (&c->related_cpus); +if (!c->max_cores) +return -ENOENT ; + + c->xo_rate = xo_rate; + + ret =qcom_cpufreq_hw_read_lut (pdev, c); +if (ret) { +dev_err (dev,"%s failed to read LUT\n" , np->name); +return ret; + } +drivers/cpufreq/qcom-cpufreq-hw.c line 266
Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-hw.c new file +10
-0+ qcom_freq_domain_map[cpu] = c; + +/* Related CPUs */ + cpu_first =cpumask_first (&c->related_cpus); + +for_each_cpu (cpu_r, &c->related_cpus) { +if (cpu_r != cpu_first) + qcom_freq_domain_map[cpu_r] = + qcom_freq_domain_map[cpu_first]; + }The above ten lines could be simplified to:
Quoted 3 lines
for_each_cpu(cpu_r, &c->related_cpus) qcom_freq_domain_map[cpu_r] = c;
Would clean it up in the next patch.
Quoted 13 lines
drivers/cpufreq/qcom-cpufreq-hw.c line 346
Quoted 7 lines
...
drivers/cpufreq/qcom-cpufreq-hw.c new file +5
-0+static int __init qcom_cpufreq_hw_init (void ) +{ +return platform_driver_register (&qcom_cpufreq_hw_driver); +} +subsys_initcall (qcom_cpufreq_hw_init);Is
subsys_initcallused for a particular reason? It will cause problems when registering cooling devices, since the thermal device class is initialized through anfs_initcall, which are executed later.
Quoted 3 lines
Most cpufreq drivers use
module_init,device_initcallorlate_initcall, can't this driver use one of those?
We want the CPU to be scaling to the highest frequency at the earliest.
Quoted 4 lines
Cheers
Matthias
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
Quoted 182 lines · Taniya Das
Hello Matthias,
Thanks for your review comments.
Quoted 75 lines · Matthias Kaehlcke
Hi Taniya,
Quoted 72 lines · Taniya Das
The CPUfreq HW present in some QCOM chipsets offloads the steps necessary for changing the frequency of CPUs. The driver implements the cpufreq driver interface for this hardware engine.
Signed-off-by Saravana Kannan <[email protected]>Signed-off-by Taniya Das <[email protected]>--- drivers/cpufreq/Kconfig.arm | 11 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/qcom-cpufreq-hw.c | 348 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 360 insertions(+)create mode 100644
drivers/cpufreq/qcom-cpufreq-hw.cdrivers/cpufreq/Kconfig.arm +11
-0@@ -298,3 +298,14 @@ config ARM_PXA2xx_CPUFREQ This add the CPUFreq driver support for Intel PXA2xx SOCs. If in doubt, say N. + +config ARM_QCOM_CPUFREQ_HW + bool "QCOM CPUFreq HW driver" + depends on ARCH_QCOM + help + Support for the CPUFreq HW driver. + Some QCOM chipsets have a HW engine to offload the steps + necessary for changing the frequency of the CPUs. Firmware loaded + in this engine exposes a programming interface to the OS. + The driver implements the cpufreq interface for this HW engine. + Say Y if you want to support CPUFreq HW.drivers/cpufreq/Makefile +1
-0@@ -85,6 +85,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_HW) += qcom-cpufreq-hw.o ##################################################################################drivers/cpufreq/qcom-cpufreq-hw.c new file +22
-0@@ -0,0 +1,348 @@ +// 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 LUT_MAX_ENTRIES 40U +#define CORE_COUNT_VAL (val) (((val) & (GENMASK(18, 16))) >> 16) +#define LUT_ROW_SIZE 32 + +enum { +REG_ENABLE , +REG_LUT_TABLE , +REG_PERF_STATE , +Quoted 9 lines
drivers/cpufreq/qcom-cpufreq-hw.c line 28
Quoted 6 lines
drivers/cpufreq/qcom-cpufreq-hw.c new file +6
-0+ REG_ARRAY_SIZE, +}; + +struct cpufreq_qcom { +struct cpufreq_frequency_table *table; +struct device *dev;'dev' is not used and can be removed.
Thanks, would remove in the next patch.
Quoted 71 lines
Quoted 58 lines
...
drivers/cpufreq/qcom-cpufreq-hw.c new file +56
-0+static int qcom_cpu_resources_init (struct platform_device *pdev, +struct device_node *np,unsigned int cpu, +unsigned long xo_rate) +{ +struct cpufreq_qcom *c; +struct resource res; +struct device *dev = &pdev->dev; +const u16 *offsets; +int ret, i, cpu_first, cpu_r; +void __iomem *base; + +if (qcom_freq_domain_map[cpu]) +return 0 ; + + c =devm_kzalloc (dev,sizeof (*c),GFP_KERNEL ); +if (!c) +return -ENOMEM ; + + offsets =of_device_get_match_data (&pdev->dev); +if (!offsets) +return -EINVAL ; + +if (of_address_to_resource (np,0 , &res)) +return -ENOMEM ; + + base =devm_ioremap_resource (dev, &res); +if (!base) +return -ENOMEM ; + +for (i =REG_ENABLE ; i <REG_ARRAY_SIZE ; i++) + c->reg_bases[i] = base + offsets[i]; + +/* HW should be in enabled state to proceed */ +if (!(readl_relaxed (c->reg_bases[REG_ENABLE ]) &0x1 )) { +dev_err (dev,"%s cpufreq hardware not enabled\n" , np->name); +return -ENODEV ; + } + + ret =qcom_get_related_cpus (np, &c->related_cpus); +if (ret) { +dev_err (dev,"%s failed to get related CPUs\n" , np->name); +return ret; + } + + c->max_cores =cpumask_weight (&c->related_cpus); +if (!c->max_cores) +return -ENOENT ; + + c->xo_rate = xo_rate; + + ret =qcom_cpufreq_hw_read_lut (pdev, c); +if (ret) { +dev_err (dev,"%s failed to read LUT\n" , np->name); +return ret; + } +drivers/cpufreq/qcom-cpufreq-hw.c line 266
Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-hw.c new file +10
-0+ qcom_freq_domain_map[cpu] = c; + +/* Related CPUs */ + cpu_first =cpumask_first (&c->related_cpus); + +for_each_cpu (cpu_r, &c->related_cpus) { +if (cpu_r != cpu_first) + qcom_freq_domain_map[cpu_r] = + qcom_freq_domain_map[cpu_first]; + }The above ten lines could be simplified to:
Quoted 3 lines
for_each_cpu(cpu_r, &c->related_cpus) qcom_freq_domain_map[cpu_r] = c;Would clean it up in the next patch.
Quoted 13 lines
drivers/cpufreq/qcom-cpufreq-hw.c line 346
Quoted 7 lines
...
drivers/cpufreq/qcom-cpufreq-hw.c new file +5
-0+static int __init qcom_cpufreq_hw_init (void ) +{ +return platform_driver_register (&qcom_cpufreq_hw_driver); +} +subsys_initcall (qcom_cpufreq_hw_init);Is
subsys_initcallused for a particular reason? It will cause problems when registering cooling devices, since the thermal device class is initialized through anfs_initcall, which are executed later.
Quoted 6 lines
Quoted 3 lines
Most cpufreq drivers use
module_init,device_initcallorlate_initcall, can't this driver use one of those?We want the CPU to be scaling to the highest frequency at the earliest.
I guess you also want thermal management for the CPU. With the subsys_initcall registration of cooling devices fails, as mentioned in my earlier comment. Do you plan to defer the registration of cooling devices?
Cheers
Matthias
Hi Taniya,
How much have you stressed this driver?
I tried it on top of an integration branch based on 4.19-rc2 that we maintain[1] and was able to get the board to reboot fairly easily with just a few "yes > /dev/null &" instances running in the background.
I even tried with interconnect framework disabled to be sure, and it still reboots.
I'll continue trying to pinpoint the source of the problem, but it would be nice to know what tree you're testing against.
Regards,
Amit
[1] https://git.linaro.org/people/amit.kucheria/kernel.git/log/?h=integration/qcomlt-automerge-result
Quoted 402 lines · Taniya Das
The CPUfreq HW present in some QCOM chipsets offloads the steps necessary for changing the frequency of CPUs. The driver implements the cpufreq driver interface for this hardware engine.
Signed-off-by Saravana Kannan <[email protected]>Signed-off-by Taniya Das <[email protected]>--- drivers/cpufreq/Kconfig.arm | 11 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/qcom-cpufreq-hw.c | 348 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 360 insertions(+) create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.cdrivers/cpufreq/Kconfig.arm +11
-0@@ -298,3 +298,14 @@ config ARM_PXA2xx_CPUFREQ This add the CPUFreq driver support for Intel PXA2xx SOCs. If in doubt, say N. + +config ARM_QCOM_CPUFREQ_HW + bool "QCOM CPUFreq HW driver" + depends on ARCH_QCOM + help + Support for the CPUFreq HW driver. + Some QCOM chipsets have a HW engine to offload the steps + necessary for changing the frequency of the CPUs. Firmware loaded + in this engine exposes a programming interface to the OS. + The driver implements the cpufreq interface for this HW engine. + Say Y if you want to support CPUFreq HW.drivers/cpufreq/Makefile +1
-0@@ -85,6 +85,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_HW) += qcom-cpufreq-hw.o ##################################################################################drivers/cpufreq/qcom-cpufreq-hw.c new file +348
-1@@ -0,0 +1,348 @@ +// 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 LUT_MAX_ENTRIES 40U +#define CORE_COUNT_VAL (val) (((val) & (GENMASK(18, 16))) >> 16) +#define LUT_ROW_SIZE 32 + +enum { +REG_ENABLE , +REG_LUT_TABLE , +REG_PERF_STATE , + +REG_ARRAY_SIZE , +}; + +struct cpufreq_qcom { +struct cpufreq_frequency_table *table; +struct device *dev; +void __iomem *reg_bases[REG_ARRAY_SIZE ]; +cpumask_t related_cpus; +unsigned int max_cores; +unsigned long xo_rate; +}; + +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE ] = { + [REG_ENABLE ] =0x0 , + [REG_LUT_TABLE ] =0x110 , + [REG_PERF_STATE ] =0x920 , +}; + +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS ]; + +static int +qcom_cpufreq_hw_target_index (struct cpufreq_policy *policy, +unsigned int index) +{ +struct cpufreq_qcom *c = policy->driver_data; + +writel_relaxed (index, c->reg_bases[REG_PERF_STATE ]); + +return 0 ; +} + +static unsigned int qcom_cpufreq_hw_get (unsigned int cpu) +{ +struct cpufreq_qcom *c; +struct cpufreq_policy *policy; +unsigned int index; + + policy =cpufreq_cpu_get_raw (cpu); +if (!policy) +return 0 ; + + c = policy->driver_data; + + index =readl_relaxed (c->reg_bases[REG_PERF_STATE ]); + index =min (index,LUT_MAX_ENTRIES -1 ); + +return policy->freq_table[index].frequency; +} + +static unsigned int +qcom_cpufreq_hw_fast_switch (struct cpufreq_policy *policy, +unsigned int target_freq) +{ +struct cpufreq_qcom *c = policy->driver_data; +int index; + + index = policy->cached_resolved_idx; +if (index <0 ) +return 0 ; + +writel_relaxed (index, c->reg_bases[REG_PERF_STATE ]); + +return policy->freq_table[index].frequency; +} + +static int qcom_cpufreq_hw_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 ; + } + +cpumask_copy (policy->cpus, &c->related_cpus); + + policy->fast_switch_possible = true; + policy->freq_table = c->table; + policy->driver_data = c; + +return 0 ; +} + +static struct freq_attr *qcom_cpufreq_hw_attr[] = { + &cpufreq_freq_attr_scaling_available_freqs, + &cpufreq_freq_attr_scaling_boost_freqs, +NULL +}; + +static struct cpufreq_driver cpufreq_qcom_hw_driver = { + .flags =CPUFREQ_STICKY |CPUFREQ_NEED_INITIAL_FREQ_CHECK | +CPUFREQ_HAVE_GOVERNOR_PER_POLICY , + .verify = cpufreq_generic_frequency_table_verify, + .target_index = qcom_cpufreq_hw_target_index, + .get = qcom_cpufreq_hw_get, + .init = qcom_cpufreq_hw_cpu_init, + .fast_switch = qcom_cpufreq_hw_fast_switch, + .name ="qcom-cpufreq-hw" , + .attr = qcom_cpufreq_hw_attr, + .boost_enabled = true, +}; + +static int qcom_cpufreq_hw_read_lut (struct platform_device *pdev, +struct cpufreq_qcom *c) +{ +struct device *dev = &pdev->dev; +void __iomem *base; +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 ; + + base = c->reg_bases[REG_LUT_TABLE ]; + +for (i =0 ; i <LUT_MAX_ENTRIES ; i++) { + data =readl_relaxed (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 = c->xo_rate * lval /1000 ; +else + c->table[i].frequency =INIT_RATE /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 ; + +return 0 ; +} + +static int qcom_get_related_cpus (struct device_node *np,struct cpumask *m) +{ +struct device_node *cpu_np, *freq_np; +int cpu; + +for_each_possible_cpu (cpu) { + cpu_np =of_cpu_device_node_get (cpu); +if (!cpu_np) +continue ; + freq_np =of_parse_phandle (cpu_np,"qcom,freq-domain" ,0 ); +of_node_put (cpu_np); +if (!freq_np) +continue ; + +if (freq_np == np) +cpumask_set_cpu (cpu, m); + } + +return 0 ; +} + +static int qcom_cpu_resources_init (struct platform_device *pdev, +struct device_node *np,unsigned int cpu, +unsigned long xo_rate) +{ +struct cpufreq_qcom *c; +struct resource res; +struct device *dev = &pdev->dev; +const u16 *offsets; +int ret, i, cpu_first, cpu_r; +void __iomem *base; + +if (qcom_freq_domain_map[cpu]) +return 0 ; + + c =devm_kzalloc (dev,sizeof (*c),GFP_KERNEL ); +if (!c) +return -ENOMEM ; + + offsets =of_device_get_match_data (&pdev->dev); +if (!offsets) +return -EINVAL ; + +if (of_address_to_resource (np,0 , &res)) +return -ENOMEM ; + + base =devm_ioremap_resource (dev, &res); +if (!base) +return -ENOMEM ; + +for (i =REG_ENABLE ; i <REG_ARRAY_SIZE ; i++) + c->reg_bases[i] = base + offsets[i]; + +/* HW should be in enabled state to proceed */ +if (!(readl_relaxed (c->reg_bases[REG_ENABLE ]) &0x1 )) { +dev_err (dev,"%s cpufreq hardware not enabled\n" , np->name); +return -ENODEV ; + } + + ret =qcom_get_related_cpus (np, &c->related_cpus); +if (ret) { +dev_err (dev,"%s failed to get related CPUs\n" , np->name); +return ret; + } + + c->max_cores =cpumask_weight (&c->related_cpus); +if (!c->max_cores) +return -ENOENT ; + + c->xo_rate = xo_rate; + + ret =qcom_cpufreq_hw_read_lut (pdev, c); +if (ret) { +dev_err (dev,"%s failed to read LUT\n" , np->name); +return ret; + } + + qcom_freq_domain_map[cpu] = c; + +/* Related CPUs */ + cpu_first =cpumask_first (&c->related_cpus); + +for_each_cpu (cpu_r, &c->related_cpus) { +if (cpu_r != cpu_first) + qcom_freq_domain_map[cpu_r] = + qcom_freq_domain_map[cpu_first]; + } + +return 0 ; +} + +static int qcom_resources_init (struct platform_device *pdev) +{ +struct device_node *np, *cpu_np; +struct clk *clk; +unsigned int cpu; +int ret; + + clk =devm_clk_get (&pdev->dev,"xo" ); +if (IS_ERR (clk)) +return PTR_ERR (clk); + +for_each_possible_cpu (cpu) { + cpu_np =of_cpu_device_node_get (cpu); +if (!cpu_np) { +dev_dbg (&pdev->dev,"Failed to get cpu %d device\n" , + cpu); +continue ; + } + + np =of_parse_phandle (cpu_np,"qcom,freq-domain" ,0 ); +of_node_put (cpu_np); +if (!np) { +dev_err (&pdev->dev,"Failed to get freq-domain device\n" ); +return -EINVAL ; + } + + ret =qcom_cpu_resources_init (pdev, np, cpu,clk_get_rate (clk)); +if (ret) +return ret; + } + +devm_clk_put (&pdev->dev, clk); + +return 0 ; +} + +static int qcom_cpufreq_hw_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_hw_driver); +if (rc) { +dev_err (&pdev->dev,"CPUFreq HW driver failed to register\n" ); +return rc; + } + +dev_dbg (&pdev->dev,"QCOM CPUFreq HW driver initialized\n" ); + +return 0 ; +} + +static const struct of_device_id qcom_cpufreq_hw_match[] = { + { .compatible ="qcom,cpufreq-hw" , .data = &cpufreq_qcom_std_offsets }, + {} +}; + +static struct platform_driver qcom_cpufreq_hw_driver = { + .probe = qcom_cpufreq_hw_driver_probe, + .driver = { + .name ="qcom-cpufreq-hw" , + .of_match_table = qcom_cpufreq_hw_match, + }, +}; + +static int __init qcom_cpufreq_hw_init (void ) +{ +return platform_driver_register (&qcom_cpufreq_hw_driver); +} +subsys_initcall (qcom_cpufreq_hw_init); + +MODULE_DESCRIPTION ("QCOM firmware-based CPU Frequency driver" ); --Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
Hi Amit,
Quoted 11 lines · Amit Kucheria
Hi Taniya,
How much have you stressed this driver?
I tried it on top of an integration branch based on 4.19-rc2 that we maintain[1] and was able to get the board to reboot fairly easily with just a few "yes >
/dev/null&" instances running in the background.I even tried with interconnect framework disabled to be sure, and it still reboots.
The driver is being tested with 4.14 kernel version and I have not yet got any reports of reboots. Though I would also try to check with teams internally.
Quoted 410 lines
I'll continue trying to pinpoint the source of the problem, but it would be nice to know what tree you're testing against.
Regards,
Amit
[1] https://git.linaro.org/people/amit.kucheria/kernel.git/log/?h=integration/qcomlt-automerge-resultQuoted 402 lines · Taniya Das
The CPUfreq HW present in some QCOM chipsets offloads the steps necessary for changing the frequency of CPUs. The driver implements the cpufreq driver interface for this hardware engine.
Signed-off-by Saravana Kannan <[email protected]>Signed-off-by Taniya Das <[email protected]>--- drivers/cpufreq/Kconfig.arm | 11 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/qcom-cpufreq-hw.c | 348 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 360 insertions(+)create mode 100644
drivers/cpufreq/qcom-cpufreq-hw.cdrivers/cpufreq/Kconfig.arm +11
-0@@ -298,3 +298,14 @@ config ARM_PXA2xx_CPUFREQ This add the CPUFreq driver support for Intel PXA2xx SOCs. If in doubt, say N. + +config ARM_QCOM_CPUFREQ_HW + bool "QCOM CPUFreq HW driver" + depends on ARCH_QCOM + help + Support for the CPUFreq HW driver. + Some QCOM chipsets have a HW engine to offload the steps + necessary for changing the frequency of the CPUs. Firmware loaded + in this engine exposes a programming interface to the OS. + The driver implements the cpufreq interface for this HW engine. + Say Y if you want to support CPUFreq HW.drivers/cpufreq/Makefile +1
-0@@ -85,6 +85,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_HW) += qcom-cpufreq-hw.o ##################################################################################drivers/cpufreq/qcom-cpufreq-hw.c new file +348
-1@@ -0,0 +1,348 @@ +// 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 LUT_MAX_ENTRIES 40U +#define CORE_COUNT_VAL (val) (((val) & (GENMASK(18, 16))) >> 16) +#define LUT_ROW_SIZE 32 + +enum { +REG_ENABLE , +REG_LUT_TABLE , +REG_PERF_STATE , + +REG_ARRAY_SIZE , +}; + +struct cpufreq_qcom { +struct cpufreq_frequency_table *table; +struct device *dev; +void __iomem *reg_bases[REG_ARRAY_SIZE ]; +cpumask_t related_cpus; +unsigned int max_cores; +unsigned long xo_rate; +}; + +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE ] = { + [REG_ENABLE ] =0x0 , + [REG_LUT_TABLE ] =0x110 , + [REG_PERF_STATE ] =0x920 , +}; + +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS ]; + +static int +qcom_cpufreq_hw_target_index (struct cpufreq_policy *policy, +unsigned int index) +{ +struct cpufreq_qcom *c = policy->driver_data; + +writel_relaxed (index, c->reg_bases[REG_PERF_STATE ]); + +return 0 ; +} + +static unsigned int qcom_cpufreq_hw_get (unsigned int cpu) +{ +struct cpufreq_qcom *c; +struct cpufreq_policy *policy; +unsigned int index; + + policy =cpufreq_cpu_get_raw (cpu); +if (!policy) +return 0 ; + + c = policy->driver_data; + + index =readl_relaxed (c->reg_bases[REG_PERF_STATE ]); + index =min (index,LUT_MAX_ENTRIES -1 ); + +return policy->freq_table[index].frequency; +} + +static unsigned int +qcom_cpufreq_hw_fast_switch (struct cpufreq_policy *policy, +unsigned int target_freq) +{ +struct cpufreq_qcom *c = policy->driver_data; +int index; + + index = policy->cached_resolved_idx; +if (index <0 ) +return 0 ; + +writel_relaxed (index, c->reg_bases[REG_PERF_STATE ]); + +return policy->freq_table[index].frequency; +} + +static int qcom_cpufreq_hw_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 ; + } + +cpumask_copy (policy->cpus, &c->related_cpus); + + policy->fast_switch_possible = true; + policy->freq_table = c->table; + policy->driver_data = c; + +return 0 ; +} + +static struct freq_attr *qcom_cpufreq_hw_attr[] = { + &cpufreq_freq_attr_scaling_available_freqs, + &cpufreq_freq_attr_scaling_boost_freqs, +NULL +}; + +static struct cpufreq_driver cpufreq_qcom_hw_driver = { + .flags =CPUFREQ_STICKY |CPUFREQ_NEED_INITIAL_FREQ_CHECK | +CPUFREQ_HAVE_GOVERNOR_PER_POLICY , + .verify = cpufreq_generic_frequency_table_verify, + .target_index = qcom_cpufreq_hw_target_index, + .get = qcom_cpufreq_hw_get, + .init = qcom_cpufreq_hw_cpu_init, + .fast_switch = qcom_cpufreq_hw_fast_switch, + .name ="qcom-cpufreq-hw" , + .attr = qcom_cpufreq_hw_attr, + .boost_enabled = true, +}; + +static int qcom_cpufreq_hw_read_lut (struct platform_device *pdev, +struct cpufreq_qcom *c) +{ +struct device *dev = &pdev->dev; +void __iomem *base; +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 ; + + base = c->reg_bases[REG_LUT_TABLE ]; + +for (i =0 ; i <LUT_MAX_ENTRIES ; i++) { + data =readl_relaxed (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 = c->xo_rate * lval /1000 ; +else + c->table[i].frequency =INIT_RATE /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 ; + +return 0 ; +} + +static int qcom_get_related_cpus (struct device_node *np,struct cpumask *m) +{ +struct device_node *cpu_np, *freq_np; +int cpu; + +for_each_possible_cpu (cpu) { + cpu_np =of_cpu_device_node_get (cpu); +if (!cpu_np) +continue ; + freq_np =of_parse_phandle (cpu_np,"qcom,freq-domain" ,0 ); +of_node_put (cpu_np); +if (!freq_np) +continue ; + +if (freq_np == np) +cpumask_set_cpu (cpu, m); + } + +return 0 ; +} + +static int qcom_cpu_resources_init (struct platform_device *pdev, +struct device_node *np,unsigned int cpu, +unsigned long xo_rate) +{ +struct cpufreq_qcom *c; +struct resource res; +struct device *dev = &pdev->dev; +const u16 *offsets; +int ret, i, cpu_first, cpu_r; +void __iomem *base; + +if (qcom_freq_domain_map[cpu]) +return 0 ; + + c =devm_kzalloc (dev,sizeof (*c),GFP_KERNEL ); +if (!c) +return -ENOMEM ; + + offsets =of_device_get_match_data (&pdev->dev); +if (!offsets) +return -EINVAL ; + +if (of_address_to_resource (np,0 , &res)) +return -ENOMEM ; + + base =devm_ioremap_resource (dev, &res); +if (!base) +return -ENOMEM ; + +for (i =REG_ENABLE ; i <REG_ARRAY_SIZE ; i++) + c->reg_bases[i] = base + offsets[i]; + +/* HW should be in enabled state to proceed */ +if (!(readl_relaxed (c->reg_bases[REG_ENABLE ]) &0x1 )) { +dev_err (dev,"%s cpufreq hardware not enabled\n" , np->name); +return -ENODEV ; + } + + ret =qcom_get_related_cpus (np, &c->related_cpus); +if (ret) { +dev_err (dev,"%s failed to get related CPUs\n" , np->name); +return ret; + } + + c->max_cores =cpumask_weight (&c->related_cpus); +if (!c->max_cores) +return -ENOENT ; + + c->xo_rate = xo_rate; + + ret =qcom_cpufreq_hw_read_lut (pdev, c); +if (ret) { +dev_err (dev,"%s failed to read LUT\n" , np->name); +return ret; + } + + qcom_freq_domain_map[cpu] = c; + +/* Related CPUs */ + cpu_first =cpumask_first (&c->related_cpus); + +for_each_cpu (cpu_r, &c->related_cpus) { +if (cpu_r != cpu_first) + qcom_freq_domain_map[cpu_r] = + qcom_freq_domain_map[cpu_first]; + } + +return 0 ; +} + +static int qcom_resources_init (struct platform_device *pdev) +{ +struct device_node *np, *cpu_np; +struct clk *clk; +unsigned int cpu; +int ret; + + clk =devm_clk_get (&pdev->dev,"xo" ); +if (IS_ERR (clk)) +return PTR_ERR (clk); + +for_each_possible_cpu (cpu) { + cpu_np =of_cpu_device_node_get (cpu); +if (!cpu_np) { +dev_dbg (&pdev->dev,"Failed to get cpu %d device\n" , + cpu); +continue ; + } + + np =of_parse_phandle (cpu_np,"qcom,freq-domain" ,0 ); +of_node_put (cpu_np); +if (!np) { +dev_err (&pdev->dev,"Failed to get freq-domain device\n" ); +return -EINVAL ; + } + + ret =qcom_cpu_resources_init (pdev, np, cpu,clk_get_rate (clk)); +if (ret) +return ret; + } + +devm_clk_put (&pdev->dev, clk); + +return 0 ; +} + +static int qcom_cpufreq_hw_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_hw_driver); +if (rc) { +dev_err (&pdev->dev,"CPUFreq HW driver failed to register\n" ); +return rc; + } + +dev_dbg (&pdev->dev,"QCOM CPUFreq HW driver initialized\n" ); + +return 0 ; +} + +static const struct of_device_id qcom_cpufreq_hw_match[] = { + { .compatible ="qcom,cpufreq-hw" , .data = &cpufreq_qcom_std_offsets }, + {} +}; + +static struct platform_driver qcom_cpufreq_hw_driver = { + .probe = qcom_cpufreq_hw_driver_probe, + .driver = { + .name ="qcom-cpufreq-hw" , + .of_match_table = qcom_cpufreq_hw_match, + }, +}; + +static int __init qcom_cpufreq_hw_init (void ) +{ +return platform_driver_register (&qcom_cpufreq_hw_driver); +} +subsys_initcall (qcom_cpufreq_hw_init); + +MODULE_DESCRIPTION ("QCOM firmware-based CPU Frequency driver" ); --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. --
Quoted 198 lines · Taniya Das
The CPUfreq HW present in some QCOM chipsets offloads the steps necessary for changing the frequency of CPUs. The driver implements the cpufreq driver interface for this hardware engine.
Signed-off-by Saravana Kannan <[email protected]>Signed-off-by Taniya Das <[email protected]>--- drivers/cpufreq/Kconfig.arm | 11 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/qcom-cpufreq-hw.c | 348 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 360 insertions(+) create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.cdrivers/cpufreq/Kconfig.arm +11
-0@@ -298,3 +298,14 @@ config ARM_PXA2xx_CPUFREQ This add the CPUFreq driver support for Intel PXA2xx SOCs. If in doubt, say N. + +config ARM_QCOM_CPUFREQ_HW + bool "QCOM CPUFreq HW driver" + depends on ARCH_QCOM + help + Support for the CPUFreq HW driver. + Some QCOM chipsets have a HW engine to offload the steps + necessary for changing the frequency of the CPUs. Firmware loaded + in this engine exposes a programming interface to the OS. + The driver implements the cpufreq interface for this HW engine. + Say Y if you want to support CPUFreq HW.drivers/cpufreq/Makefile +1
-0@@ -85,6 +85,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_HW) += qcom-cpufreq-hw.o ##################################################################################drivers/cpufreq/qcom-cpufreq-hw.c new file +148
-0@@ -0,0 +1,348 @@ +// 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 LUT_MAX_ENTRIES 40U +#define CORE_COUNT_VAL (val) (((val) & (GENMASK(18, 16))) >> 16) +#define LUT_ROW_SIZE 32 + +enum { +REG_ENABLE , +REG_LUT_TABLE , +REG_PERF_STATE , + +REG_ARRAY_SIZE , +}; + +struct cpufreq_qcom { +struct cpufreq_frequency_table *table; +struct device *dev; +void __iomem *reg_bases[REG_ARRAY_SIZE ]; +cpumask_t related_cpus; +unsigned int max_cores; +unsigned long xo_rate; +}; + +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE ] = { + [REG_ENABLE ] =0x0 , + [REG_LUT_TABLE ] =0x110 , + [REG_PERF_STATE ] =0x920 , +}; + +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS ]; + +static int +qcom_cpufreq_hw_target_index (struct cpufreq_policy *policy, +unsigned int index) +{ +struct cpufreq_qcom *c = policy->driver_data; + +writel_relaxed (index, c->reg_bases[REG_PERF_STATE ]); + +return 0 ; +} + +static unsigned int qcom_cpufreq_hw_get (unsigned int cpu) +{ +struct cpufreq_qcom *c; +struct cpufreq_policy *policy; +unsigned int index; + + policy =cpufreq_cpu_get_raw (cpu); +if (!policy) +return 0 ; + + c = policy->driver_data; + + index =readl_relaxed (c->reg_bases[REG_PERF_STATE ]); + index =min (index,LUT_MAX_ENTRIES -1 ); + +return policy->freq_table[index].frequency; +} + +static unsigned int +qcom_cpufreq_hw_fast_switch (struct cpufreq_policy *policy, +unsigned int target_freq) +{ +struct cpufreq_qcom *c = policy->driver_data; +int index; + + index = policy->cached_resolved_idx; +if (index <0 ) +return 0 ; + +writel_relaxed (index, c->reg_bases[REG_PERF_STATE ]); + +return policy->freq_table[index].frequency; +} + +static int qcom_cpufreq_hw_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 ; + } + +cpumask_copy (policy->cpus, &c->related_cpus); + + policy->fast_switch_possible = true; + policy->freq_table = c->table; + policy->driver_data = c; + +return 0 ; +} + +static struct freq_attr *qcom_cpufreq_hw_attr[] = { + &cpufreq_freq_attr_scaling_available_freqs, + &cpufreq_freq_attr_scaling_boost_freqs, +NULL +}; + +static struct cpufreq_driver cpufreq_qcom_hw_driver = { + .flags =CPUFREQ_STICKY |CPUFREQ_NEED_INITIAL_FREQ_CHECK | +CPUFREQ_HAVE_GOVERNOR_PER_POLICY , + .verify = cpufreq_generic_frequency_table_verify, + .target_index = qcom_cpufreq_hw_target_index, + .get = qcom_cpufreq_hw_get, + .init = qcom_cpufreq_hw_cpu_init, + .fast_switch = qcom_cpufreq_hw_fast_switch, + .name ="qcom-cpufreq-hw" , + .attr = qcom_cpufreq_hw_attr, + .boost_enabled = true, +}; + +static int qcom_cpufreq_hw_read_lut (struct platform_device *pdev, +struct cpufreq_qcom *c) +{ +struct device *dev = &pdev->dev; +void __iomem *base; +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 ; + + base = c->reg_bases[REG_LUT_TABLE ]; + +for (i =0 ; i <LUT_MAX_ENTRIES ; i++) { + data =readl_relaxed (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 = c->xo_rate * lval /1000 ; +else
drivers/cpufreq/qcom-cpufreq-hw.c line 158
Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-hw.c new file +10
-0+ c->table[i].frequency =INIT_RATE /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 ; +
I noticed that the 'power_allocator' thermal governor currently can't be used with this driver since there is no OPP table with frequency and voltage information. Does the LUT contain information about the voltage or is there another mechanism to retrieve it?
Quoted 206 lines · Matthias Kaehlcke
Quoted 205 lines · Taniya Das
The CPUfreq HW present in some QCOM chipsets offloads the steps necessary for changing the frequency of CPUs. The driver implements the cpufreq driver interface for this hardware engine.
Signed-off-by Saravana Kannan <[email protected]>Signed-off-by Taniya Das <[email protected]>--- drivers/cpufreq/Kconfig.arm | 11 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/qcom-cpufreq-hw.c | 348 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 360 insertions(+)create mode 100644
drivers/cpufreq/qcom-cpufreq-hw.cdrivers/cpufreq/Kconfig.arm +11
-0@@ -298,3 +298,14 @@ config ARM_PXA2xx_CPUFREQ This add the CPUFreq driver support for Intel PXA2xx SOCs. If in doubt, say N. + +config ARM_QCOM_CPUFREQ_HW + bool "QCOM CPUFreq HW driver" + depends on ARCH_QCOM + help + Support for the CPUFreq HW driver. + Some QCOM chipsets have a HW engine to offload the steps + necessary for changing the frequency of the CPUs. Firmware loaded + in this engine exposes a programming interface to the OS. + The driver implements the cpufreq interface for this HW engine. + Say Y if you want to support CPUFreq HW.drivers/cpufreq/Makefile +1
-0@@ -85,6 +85,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_HW) += qcom-cpufreq-hw.o ##################################################################################drivers/cpufreq/qcom-cpufreq-hw.c new file +155
-0@@ -0,0 +1,348 @@ +// 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 LUT_MAX_ENTRIES 40U +#define CORE_COUNT_VAL (val) (((val) & (GENMASK(18, 16))) >> 16) +#define LUT_ROW_SIZE 32 + +enum { +REG_ENABLE , +REG_LUT_TABLE , +REG_PERF_STATE , + +REG_ARRAY_SIZE , +}; + +struct cpufreq_qcom { +struct cpufreq_frequency_table *table; +struct device *dev; +void __iomem *reg_bases[REG_ARRAY_SIZE ]; +cpumask_t related_cpus; +unsigned int max_cores; +unsigned long xo_rate; +}; + +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE ] = { + [REG_ENABLE ] =0x0 , + [REG_LUT_TABLE ] =0x110 , + [REG_PERF_STATE ] =0x920 , +}; + +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS ]; + +static int +qcom_cpufreq_hw_target_index (struct cpufreq_policy *policy, +unsigned int index) +{ +struct cpufreq_qcom *c = policy->driver_data; + +writel_relaxed (index, c->reg_bases[REG_PERF_STATE ]); + +return 0 ; +} + +static unsigned int qcom_cpufreq_hw_get (unsigned int cpu) +{ +struct cpufreq_qcom *c; +struct cpufreq_policy *policy; +unsigned int index; + + policy =cpufreq_cpu_get_raw (cpu); +if (!policy) +return 0 ; + + c = policy->driver_data; + + index =readl_relaxed (c->reg_bases[REG_PERF_STATE ]); + index =min (index,LUT_MAX_ENTRIES -1 ); + +return policy->freq_table[index].frequency; +} + +static unsigned int +qcom_cpufreq_hw_fast_switch (struct cpufreq_policy *policy, +unsigned int target_freq) +{ +struct cpufreq_qcom *c = policy->driver_data; +int index; + + index = policy->cached_resolved_idx; +if (index <0 ) +return 0 ; + +writel_relaxed (index, c->reg_bases[REG_PERF_STATE ]); + +return policy->freq_table[index].frequency; +} + +static int qcom_cpufreq_hw_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 ; + } + +cpumask_copy (policy->cpus, &c->related_cpus); + + policy->fast_switch_possible = true; + policy->freq_table = c->table; + policy->driver_data = c; + +return 0 ; +} + +static struct freq_attr *qcom_cpufreq_hw_attr[] = { + &cpufreq_freq_attr_scaling_available_freqs, + &cpufreq_freq_attr_scaling_boost_freqs, +NULL +}; + +static struct cpufreq_driver cpufreq_qcom_hw_driver = { + .flags =CPUFREQ_STICKY |CPUFREQ_NEED_INITIAL_FREQ_CHECK | +CPUFREQ_HAVE_GOVERNOR_PER_POLICY , + .verify = cpufreq_generic_frequency_table_verify, + .target_index = qcom_cpufreq_hw_target_index, + .get = qcom_cpufreq_hw_get, + .init = qcom_cpufreq_hw_cpu_init, + .fast_switch = qcom_cpufreq_hw_fast_switch, + .name ="qcom-cpufreq-hw" , + .attr = qcom_cpufreq_hw_attr, + .boost_enabled = true, +}; + +static int qcom_cpufreq_hw_read_lut (struct platform_device *pdev, +struct cpufreq_qcom *c) +{ +struct device *dev = &pdev->dev; +void __iomem *base; +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 ; + + base = c->reg_bases[REG_LUT_TABLE ]; + +for (i =0 ; i <LUT_MAX_ENTRIES ; i++) { + data =readl_relaxed (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 = c->xo_rate * lval /1000 ; +else + c->table[i].frequency =INIT_RATE /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); +
Quoted 9 lines
drivers/cpufreq/qcom-cpufreq-hw.c line 158
Quoted 3 lines
drivers/cpufreq/qcom-cpufreq-hw.c new file +3
-0+if (core_count != c->max_cores) + cur_freq =CPUFREQ_ENTRY_INVALID ; +I noticed that the '
power_allocator' thermal governor currently can't be used with this driver since there is no OPP table with frequency and voltage information. Does the LUT contain information about the voltage or is there another mechanism to retrieve it?
No, currently there is no way of reading the voltage information.
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
Quoted 219 lines · Taniya Das
Quoted 206 lines · Matthias Kaehlcke
Quoted 205 lines · Taniya Das
The CPUfreq HW present in some QCOM chipsets offloads the steps necessary for changing the frequency of CPUs. The driver implements the cpufreq driver interface for this hardware engine.
Signed-off-by Saravana Kannan <[email protected]>Signed-off-by Taniya Das <[email protected]>--- drivers/cpufreq/Kconfig.arm | 11 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/qcom-cpufreq-hw.c | 348 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 360 insertions(+)create mode 100644
drivers/cpufreq/qcom-cpufreq-hw.cdrivers/cpufreq/Kconfig.arm +11
-0@@ -298,3 +298,14 @@ config ARM_PXA2xx_CPUFREQ This add the CPUFreq driver support for Intel PXA2xx SOCs. If in doubt, say N. + +config ARM_QCOM_CPUFREQ_HW + bool "QCOM CPUFreq HW driver" + depends on ARCH_QCOM + help + Support for the CPUFreq HW driver. + Some QCOM chipsets have a HW engine to offload the steps + necessary for changing the frequency of the CPUs. Firmware loaded + in this engine exposes a programming interface to the OS. + The driver implements the cpufreq interface for this HW engine. + Say Y if you want to support CPUFreq HW.drivers/cpufreq/Makefile +1
-0@@ -85,6 +85,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_HW) += qcom-cpufreq-hw.o ##################################################################################drivers/cpufreq/qcom-cpufreq-hw.c new file +155
-0@@ -0,0 +1,348 @@ +// 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 LUT_MAX_ENTRIES 40U +#define CORE_COUNT_VAL (val) (((val) & (GENMASK(18, 16))) >> 16) +#define LUT_ROW_SIZE 32 + +enum { +REG_ENABLE , +REG_LUT_TABLE , +REG_PERF_STATE , + +REG_ARRAY_SIZE , +}; + +struct cpufreq_qcom { +struct cpufreq_frequency_table *table; +struct device *dev; +void __iomem *reg_bases[REG_ARRAY_SIZE ]; +cpumask_t related_cpus; +unsigned int max_cores; +unsigned long xo_rate; +}; + +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE ] = { + [REG_ENABLE ] =0x0 , + [REG_LUT_TABLE ] =0x110 , + [REG_PERF_STATE ] =0x920 , +}; + +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS ]; + +static int +qcom_cpufreq_hw_target_index (struct cpufreq_policy *policy, +unsigned int index) +{ +struct cpufreq_qcom *c = policy->driver_data; + +writel_relaxed (index, c->reg_bases[REG_PERF_STATE ]); + +return 0 ; +} + +static unsigned int qcom_cpufreq_hw_get (unsigned int cpu) +{ +struct cpufreq_qcom *c; +struct cpufreq_policy *policy; +unsigned int index; + + policy =cpufreq_cpu_get_raw (cpu); +if (!policy) +return 0 ; + + c = policy->driver_data; + + index =readl_relaxed (c->reg_bases[REG_PERF_STATE ]); + index =min (index,LUT_MAX_ENTRIES -1 ); + +return policy->freq_table[index].frequency; +} + +static unsigned int +qcom_cpufreq_hw_fast_switch (struct cpufreq_policy *policy, +unsigned int target_freq) +{ +struct cpufreq_qcom *c = policy->driver_data; +int index; + + index = policy->cached_resolved_idx; +if (index <0 ) +return 0 ; + +writel_relaxed (index, c->reg_bases[REG_PERF_STATE ]); + +return policy->freq_table[index].frequency; +} + +static int qcom_cpufreq_hw_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 ; + } + +cpumask_copy (policy->cpus, &c->related_cpus); + + policy->fast_switch_possible = true; + policy->freq_table = c->table; + policy->driver_data = c; + +return 0 ; +} + +static struct freq_attr *qcom_cpufreq_hw_attr[] = { + &cpufreq_freq_attr_scaling_available_freqs, + &cpufreq_freq_attr_scaling_boost_freqs, +NULL +}; + +static struct cpufreq_driver cpufreq_qcom_hw_driver = { + .flags =CPUFREQ_STICKY |CPUFREQ_NEED_INITIAL_FREQ_CHECK | +CPUFREQ_HAVE_GOVERNOR_PER_POLICY , + .verify = cpufreq_generic_frequency_table_verify, + .target_index = qcom_cpufreq_hw_target_index, + .get = qcom_cpufreq_hw_get, + .init = qcom_cpufreq_hw_cpu_init, + .fast_switch = qcom_cpufreq_hw_fast_switch, + .name ="qcom-cpufreq-hw" , + .attr = qcom_cpufreq_hw_attr, + .boost_enabled = true, +}; + +static int qcom_cpufreq_hw_read_lut (struct platform_device *pdev, +struct cpufreq_qcom *c) +{ +struct device *dev = &pdev->dev; +void __iomem *base; +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 ; + + base = c->reg_bases[REG_LUT_TABLE ]; + +for (i =0 ; i <LUT_MAX_ENTRIES ; i++) { + data =readl_relaxed (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 = c->xo_rate * lval /1000 ; +else + c->table[i].frequency =INIT_RATE /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); +Quoted 9 lines
drivers/cpufreq/qcom-cpufreq-hw.c line 158
Quoted 3 lines
drivers/cpufreq/qcom-cpufreq-hw.c new file +3
-0+if (core_count != c->max_cores) + cur_freq =CPUFREQ_ENTRY_INVALID ; +I noticed that the '
power_allocator' thermal governor currently can't be used with this driver since there is no OPP table with frequency and voltage information. Does the LUT contain information about the voltage or is there another mechanism to retrieve it?
Quoted 1 line
No, currently there is no way of reading the voltage information.
That leaves the 'power_allocator' out of question :(
Which thermal governor is/should typically be used on these systems? Step wise and user space should work out of the box, however the response of step wise could be slow (step by step) and user space requires a thermal daemon (and could suffer from latencies). Fair share could be an option if the thermal cooling devices are registered with a 'weight', which could come from the device tree.