cpufreq: qcom-hw: Add support for QCOM cpufreq HW
v11 cover cpufreq: qcom-hw: Add support for QCOM cpufreq HW
[v11]
* Updated the code logic as per Stephen.
* Default boost enabled is removed.
* Update the clock name to use "alternate" for GPLL0 source in code and Documentation binding.
* Module description updated.
* perf_base updated to perf_state_reg.
[v10]
* Update Documentation binding for cpufreq node.
* Make the driver 'tristate' instead of 'bool' and update code.
* Update the clock name to reflect the hardware name.
* Remove support for varying offset.
[v9]
* Update the Documentation binding for freq-domain-cells & cpufreq node.
* Address comments in Kconfig.arm & Makefile.
* Remove include file & MODULE_DESCRIPTION not required.
* Fix the code for 'of_node_put(cpu_np)'.
[v8]
* Address comments to update code to take cpufreq_hw phandle and index from the CPU nodes.
* Updated the Documentation for the above change in DT.
* Updated logic for assigning 'qcom_freq_domain_map' for related CPUs.
* Clock input to the HW block is taken from DT which has been updated in code and Device tree documentation.
[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 | 334 +++++++++++++++++++++ 4 files changed, 518 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 v11 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 and GPLL0 clock. + +- clock-names + Usage: required + Value type: <string> From common clock binding. + Definition: must be "xo", "alternate". + +- reg + Usage: required + Value type: <prop-encoded-array> + Definition: Addresses and sizes for the memory of the HW bases in + each frequency domain. +- reg-names + Usage: Optional + Value type: <string> + Definition: Frequency domain name i.e. + "freq-domain0", "freq-domain1". + +- freq-domain-cells: + Usage: required. + Definition: Number of cells in a freqency domain specifier. + +* Property qcom,freq-domain +Devices supporting freq-domain must set their "qcom,freq-domain" property with +phandle to a cpufreq_hw followed by the Domain ID(0/1) in the CPU DT node. + + +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 = <&cpufreq_hw 0>; + 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 = <&cpufreq_hw 0>; + 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 = <&cpufreq_hw 0>; + 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 = <&cpufreq_hw 0>; + 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 = <&cpufreq_hw 1>; + 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 = <&cpufreq_hw 1>; + 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 = <&cpufreq_hw 1>; + 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 = <&cpufreq_hw 1>; + L2_700: l2-cache { + compatible = "cache"; + next-level-cache = <&L3_0>; + }; + }; + }; + + soc { + cpufreq_hw: cpufreq@17d43000 { + compatible = "qcom,cpufreq-hw"; + reg = <0x17d43000 0x1400>, <0x17d45800 0x1400>; + reg-names = "freq-domain0", "freq-domain1"; + + clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GPLL0>; + clock-names = "xo", "alternate"; + + #freq-domain-cells = <1>; + }; +}
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 v11 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings
Quoting Taniya Das (2018-12-01 19:55:02)
Quoted 6 lines
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.
Signed-off-by Taniya Das <[email protected]>---
Re: [PATCH v11 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings
In reply to Taniya DasQuoted 40 lines · Taniya Das
Add QCOM cpufreq firmware device bindings for Qualcomm Technology Inc's SoCs. This is required for managing the cpu frequency transitions which are controlled by the hardware engine.
Signed-off-by Taniya Das <[email protected]>--- .../bindings/cpufreq/cpufreq-qcom-hw.txt | 172 +++++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txtDocumentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +24
-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 and GPLL0 clock. + +- clock-names + Usage: required + Value type: <string> From common clock binding. + Definition: must be "xo", "alternate". + +- reg + Usage: required
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt line 34
Quoted 10 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +10
-0+ Value type: <prop-encoded-array> + Definition: Addresses and sizes for the memory of the HW bases in + each frequency domain. +- reg-names + Usage: Optional + Value type: <string> + Definition: Frequency domain name i.e. + "freq-domain0", "freq-domain1". + +- freq-domain-cells:
#freq-domain-cells
Otherwise,
Re: [PATCH v11 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings
Quoting Rob Herring (2018-12-03 15:09:07)
Quoted 54 lines · Taniya Das
Quoted 40 lines
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.
Signed-off-by Taniya Das <[email protected]>--- .../bindings/cpufreq/cpufreq-qcom-hw.txt | 172 +++++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txtDocumentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +24
-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 and GPLL0 clock. + +- clock-names + Usage: required + Value type: <string> From common clock binding. + Definition: must be "xo", "alternate". + +- reg + Usage: requiredDocumentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt line 34
Quoted 10 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +10
-0+ Value type: <prop-encoded-array> + Definition: Addresses and sizes for the memory of the HW bases in + each frequency domain. +- reg-names + Usage: Optional + Value type: <string> + Definition: Frequency domain name i.e. + "freq-domain0", "freq-domain1". + +- freq-domain-cells:#freq-domain-cells
Quoted 3 lines
Otherwise,
Reviewed-by Rob Herring <[email protected]>
Or should it be #qcom,freq-domain-cells? That would match the same stem of the property used in the cpu node.
PATCH v11 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 | 334 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 346 insertions(+) create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.c
drivers/cpufreq/Kconfig.arm +11 -0
@@ -114,6 +114,17 @@ config ARM_QCOM_CPUFREQ_KRYO If in doubt, say N. +config ARM_QCOM_CPUFREQ_HW + tristate "QCOM CPUFreq HW driver" + depends on ARCH_QCOM || COMPILE_TEST + 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. + config ARM_S3C_CPUFREQ bool help
drivers/cpufreq/Makefile +1 -0
@@ -61,6 +61,7 @@ obj-$(CONFIG_MACH_MVEBU_V7) += mvebu-cpufreq.o obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o +obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW) += qcom-cpufreq-hw.o obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO) += qcom-cpufreq-kryo.o obj-$(CONFIG_ARM_S3C2410_CPUFREQ) += s3c2410-cpufreq.o obj-$(CONFIG_ARM_S3C2412_CPUFREQ) += s3c2412-cpufreq.o
drivers/cpufreq/qcom-cpufreq-hw.c new file +334 -0
334 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.
Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
Quoting Taniya Das (2018-12-01 19:55:03)
Quoted 8 lines
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 Stephen Boyd <[email protected]>Signed-off-by Taniya Das <[email protected]>---
Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
In reply to Taniya DasQuoted 45 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 Stephen Boyd <[email protected]>Signed-off-by Taniya Das <[email protected]>--- drivers/cpufreq/Kconfig.arm | 11 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/qcom-cpufreq-hw.c | 334 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 346 insertions(+) create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.c...
drivers/cpufreq/qcom-cpufreq-hw.c new file +21
-0+static int qcom_cpufreq_hw_read_lut (struct device *dev,struct cpufreq_qcom *c, +void __iomem *base,unsigned long xo_rate, +unsigned long cpu_hw_rate) +{ +u32 data, src, lval, i, core_count, prev_cc =0 , prev_freq =0 , freq; +unsigned int max_cores =cpumask_weight (&c->related_cpus); + + c->table =devm_kcalloc (dev,LUT_MAX_ENTRIES +1 , +sizeof (*c->table),GFP_KERNEL ); +if (!c->table) +return -ENOMEM ; + +for (i =0 ; i <LUT_MAX_ENTRIES ; i++) { + data =readl_relaxed (base +REG_LUT_TABLE + i *LUT_ROW_SIZE ); + src =FIELD_GET (LUT_SRC , data); + lval =FIELD_GET (LUT_L_VAL , data); + core_count =FIELD_GET (LUT_CORE_COUNT , data); + +if (src) + freq = xo_rate * lval /1000 ; +else
drivers/cpufreq/qcom-cpufreq-hw.c line 144
Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-hw.c new file +10
-0+ freq = cpu_hw_rate /1000 ; + +/* Ignore boosts in the middle of the table */ +if (core_count != max_cores) { + c->table[i].frequency =CPUFREQ_ENTRY_INVALID ; + }else { + c->table[i].frequency = freq; +dev_dbg (dev,"index=%d freq=%d, core_count %d\n" , i, + freq, core_count); + }
nit: IMO it would be better to put the normal case ("core_count != max_cores") first and the exception in the else branch.
Quoted 1 line
+MODULE_DESCRIPTION("QCOM CPUFREQ HW Driver");
nit: my suggestion was 'QCOM CPUFreq HW driver', which is what's used elsewhere in the driver.
Anyway, no need to respin just for the nits, we can address them (or not) with follow-up patches.
Viresh Kumar
maintainerRe: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
In reply to Taniya DasHi Taniya,
Sorry that I haven't been reviewing it much from last few iterations as I was letting others get this into a better shape. Thanks for your efforts..
drivers/cpufreq/qcom-cpufreq-hw.c line 32
Quoted 9 lines · Taniya Das
+++
b/drivers/cpufreq/qcom-cpufreq-hw.cdrivers/cpufreq/qcom-cpufreq-hw.c new file +7
-0+struct cpufreq_qcom { +struct cpufreq_frequency_table *table; +void __iomem *perf_state_reg; +cpumask_t related_cpus; +}; + +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS ];
Now that the code is much more simplified, I am not sure if you need this per-cpu structure at all. The only place where you are using it is in qcom_cpufreq_hw_cpu_init() and probe(). Why not merge qcom_cpu_resources_init() completely into qcom_cpufreq_hw_cpu_init() and get rid of this structure entirely ?
Signature · 1 line
viresh
Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
Hello Viresh,
Quoted 8 lines · Viresh Kumar
Hi Taniya,
Sorry that I haven't been reviewing it much from last few iterations as I was letting others get this into a better shape. Thanks for your efforts..
Quoted 1 line · Taniya Das
+++
b/drivers/cpufreq/qcom-cpufreq-hw.c
Quoted 14 lines
drivers/cpufreq/qcom-cpufreq-hw.c line 32
Quoted 7 lines
drivers/cpufreq/qcom-cpufreq-hw.c new file +7
-0+struct cpufreq_qcom { +struct cpufreq_frequency_table *table; +void __iomem *perf_state_reg; +cpumask_t related_cpus; +}; + +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS ];Now that the code is much more simplified, I am not sure if you need this per-cpu structure at all. The only place where you are using it is in
qcom_cpufreq_hw_cpu_init()andprobe(). Why not mergeqcom_cpu_resources_init()completely intoqcom_cpufreq_hw_cpu_init()and get rid of this structure entirely ?
Yes, we still would require the per-cpu.
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
Viresh Kumar
maintainerRe: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
Quoted 26 lines · Taniya Das
Hello Viresh,
Quoted 8 lines · Viresh Kumar
Hi Taniya,
Sorry that I haven't been reviewing it much from last few iterations as I was letting others get this into a better shape. Thanks for your efforts..
Quoted 1 line · Taniya Das
+++
b/drivers/cpufreq/qcom-cpufreq-hw.cQuoted 14 lines
drivers/cpufreq/qcom-cpufreq-hw.c line 32
Quoted 7 lines
drivers/cpufreq/qcom-cpufreq-hw.c new file +7
-0+struct cpufreq_qcom { +struct cpufreq_frequency_table *table; +void __iomem *perf_state_reg; +cpumask_t related_cpus; +}; + +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS ];Now that the code is much more simplified, I am not sure if you need this per-cpu structure at all. The only place where you are using it is in
qcom_cpufreq_hw_cpu_init()andprobe(). Why not mergeqcom_cpu_resources_init()completely intoqcom_cpufreq_hw_cpu_init()and get rid of this structure entirely ?
Quoted 1 line
Yes, we still would require the per-cpu.
An explanation on why do you feel so would have been nice :)
I am sure I am missing something obvious here.
Signature · 1 line
viresh
Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
In reply to Viresh KumarQuoting Viresh Kumar (2018-12-03 21:12:31)
Quoted 8 lines
Hi Taniya,
Sorry that I haven't been reviewing it much from last few iterations as I was letting others get this into a better shape. Thanks for your efforts..
Quoted 1 line · Taniya Das
+++
b/drivers/cpufreq/qcom-cpufreq-hw.c
Quoted 14 lines
Review
Quoted 7 lines
(quoted diff) +7
-0+struct cpufreq_qcom { + struct cpufreq_frequency_table *table; + void __iomem *perf_state_reg; + cpumask_t related_cpus; +}; + +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];Now that the code is much more simplified, I am not sure if you need this per-cpu structure at all. The only place where you are using it is in
qcom_cpufreq_hw_cpu_init()andprobe(). Why not mergeqcom_cpu_resources_init()completely intoqcom_cpufreq_hw_cpu_init()and get rid of this structure entirely ?
Good point. Here's an untested patch to handle that. It removes the related functionality and makes an array of pointers to the domains that are created each time qcom_cpu_resources_init() is called.
---8<----
drivers/cpufreq/qcom-cpufreq-hw.c +36 -41
@@ -23,14 +23,14 @@#define REG_LUT_TABLE 0x110#define REG_PERF_STATE 0x920 +#define MAX_FREQ_DOMAINS 2 +struct cpufreq_qcom {struct cpufreq_frequency_table *table;void __iomem *perf_state_reg;cpumask_t related_cpus; }; -static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS ]; -static int qcom_cpufreq_hw_target_index (struct cpufreq_policy *policy,unsigned int index) {
@@ -76,9 +76,14 @@ static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,static int qcom_cpufreq_hw_cpu_init (struct cpufreq_policy *policy) { +struct cpufreq_qcom **freq_domain_map;struct cpufreq_qcom *c; - c = qcom_freq_domain_map[policy->cpu]; + freq_domain_map =cpufreq_get_driver_data (); +if (!freq_domain_map) +return -ENODEV ; + + c = freq_domain_map[policy->cpu];if (!c) {pr_err ("No scaling support for CPU%d\n" , policy->cpu);return -ENODEV ;
@@ -171,39 +176,17 @@ static int qcom_cpufreq_hw_read_lut(struct device *dev, struct cpufreq_qcom *c,return 0 ; } -static void qcom_get_related_cpus (int index,struct cpumask *m) -{ -struct device_node *cpu_np; -struct of_phandle_args args; -int cpu, ret; - -for_each_possible_cpu (cpu) { - cpu_np =of_cpu_device_node_get (cpu); -if (!cpu_np) -continue ; - - ret =of_parse_phandle_with_args (cpu_np,"qcom,freq-domain" , -"#freq-domain-cells" ,0 , - &args); -of_node_put (cpu_np); -if (ret <0 ) -continue ; - -if (index == args.args[0 ]) -cpumask_set_cpu (cpu, m); - } -} -static int qcom_cpu_resources_init (struct platform_device *pdev,unsigned int cpu,int index,unsigned long xo_rate, -unsigned long cpu_hw_rate) +unsigned long cpu_hw_rate, +struct cpufreq_qcom **freq_domain_map) {struct cpufreq_qcom *c;struct resource *res;struct device *dev = &pdev->dev;void __iomem *base; -int ret, cpu_r; +int ret; c =devm_kzalloc (dev,sizeof (*c),GFP_KERNEL );if (!c)
@@ -220,12 +203,6 @@ static int qcom_cpu_resources_init(struct platform_device *pdev,return -ENODEV ; } -qcom_get_related_cpus (index, &c->related_cpus); -if (!cpumask_weight (&c->related_cpus)) { -dev_err (dev,"Domain-%d failed to get related CPUs\n" , index); -return -ENOENT ; - } - c->perf_state_reg = base +REG_PERF_STATE ; ret =qcom_cpufreq_hw_read_lut (dev, c, base, xo_rate, cpu_hw_rate);
@@ -234,8 +211,7 @@ static int qcom_cpu_resources_init(struct platform_device *pdev,return ret; } -for_each_cpu (cpu_r, &c->related_cpus) - qcom_freq_domain_map[cpu_r] = c; + freq_domain_map[index] = c;return 0 ; }
@@ -245,9 +221,16 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)struct device_node *cpu_np;struct of_phandle_args args;struct clk *clk; -unsigned int cpu; +unsigned int cpu, domain;unsigned long xo_rate, cpu_hw_rate;int ret; +struct cpufreq_qcom **freq_domain_map, *freq_domain; + + freq_domain_map =devm_kcalloc (&pdev->dev,MAX_FREQ_DOMAINS , +sizeof (*freq_domain_map),GFP_KERNEL ); + cpufreq_qcom_hw_driver.driver_data = freq_domain_map; +if (!freq_domain_map) +return -ENOMEM ; clk =clk_get (&pdev->dev,"xo" );if (IS_ERR (clk))
@@ -273,16 +256,28 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev) ret =of_parse_phandle_with_args (cpu_np,"qcom,freq-domain" ,"#freq-domain-cells" ,0 , - &args); + &args);of_node_put (cpu_np);if (ret)return ret; -if (qcom_freq_domain_map[cpu]) + domain = args.args[0 ]; +if (domain >=MAX_FREQ_DOMAINS )continue ; - ret =qcom_cpu_resources_init (pdev, cpu, args.args[0 ], - xo_rate, cpu_hw_rate); +/* +* If we've already populated the frequency table for this domain +* just mark it related and get out of here +*/ + freq_domain = freq_domain_map[domain]; +if (freq_domain) { +cpumask_set_cpu (cpu, &freq_domain->related_cpus); +continue ; + } + + ret =qcom_cpu_resources_init (pdev, cpu, domain, + xo_rate, cpu_hw_rate, + freq_domain_map);if (ret)return ret; }
Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
Quoting Stephen Boyd (2018-12-04 14:28:11)
Quoted 54 lines
Quoting Viresh Kumar (2018-12-03 21:12:31)
Quoted 8 lines
Hi Taniya,
Sorry that I haven't been reviewing it much from last few iterations as I was letting others get this into a better shape. Thanks for your efforts..
Quoted 1 line · Taniya Das
+++
b/drivers/cpufreq/qcom-cpufreq-hw.cQuoted 14 lines
Review
Quoted 7 lines
(quoted diff) +7
-0+struct cpufreq_qcom { + struct cpufreq_frequency_table *table; + void __iomem *perf_state_reg; + cpumask_t related_cpus; +}; + +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];Now that the code is much more simplified, I am not sure if you need this per-cpu structure at all. The only place where you are using it is in
qcom_cpufreq_hw_cpu_init()andprobe(). Why not mergeqcom_cpu_resources_init()completely intoqcom_cpufreq_hw_cpu_init()and get rid of this structure entirely ?Good point. Here's an untested patch to handle that. It removes the related functionality and makes an array of pointers to the domains that are created each time
qcom_cpu_resources_init()is called.---8<----
drivers/cpufreq/qcom-cpufreq-hw.c +2
-2@@ -23,14 +23,14 @@#define REG_LUT_TABLE 0x110#define REG_PERF_STATE 0x920 +#define MAX_FREQ_DOMAINS 2 +struct cpufreq_qcom {struct cpufreq_frequency_table *table;void __iomem *perf_state_reg;cpumask_t related_cpus; }; -static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS ]; -static int qcom_cpufreq_hw_target_index (struct cpufreq_policy *policy,unsigned int index) {@@ -76,9 +76,14 @@ static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,static int qcom_cpufreq_hw_cpu_init (struct cpufreq_policy *policy)
drivers/cpufreq/qcom-cpufreq-hw.c static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy, · line 86
Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-hw.c +6
-1{ +struct cpufreq_qcom **freq_domain_map;struct cpufreq_qcom *c; - c = qcom_freq_domain_map[policy->cpu]; + freq_domain_map =cpufreq_get_driver_data (); +if (!freq_domain_map) +return -ENODEV ; + + c = freq_domain_map[policy->cpu];
And this fails now because it indexes based on cpu number. We have to parse the frequency domain out of the cpunode again to fix that.
Here's the updated (still untested) patch and I also just allocated the freq domain array up front instead of in each domain init routine to simplify some more.
drivers/cpufreq/qcom-cpufreq-hw.c +47 -50
@@ -23,14 +23,14 @@#define REG_LUT_TABLE 0x110#define REG_PERF_STATE 0x920 +#define MAX_FREQ_DOMAINS 2 +struct cpufreq_qcom {struct cpufreq_frequency_table *table;void __iomem *perf_state_reg;cpumask_t related_cpus; }; -static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS ]; -static int qcom_cpufreq_hw_target_index (struct cpufreq_policy *policy,unsigned int index) {
@@ -76,9 +76,26 @@ static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,static int qcom_cpufreq_hw_cpu_init (struct cpufreq_policy *policy) { -struct cpufreq_qcom *c; +struct cpufreq_qcom *freq_domain_map, *c; +struct device_node *cpu_np; +struct of_phandle_args args; +int ret; + + freq_domain_map =cpufreq_get_driver_data (); +if (!freq_domain_map) +return -ENODEV ; + + cpu_np =of_cpu_device_node_get (policy->cpu); +if (!cpu_np) +return -ENODEV ; + + ret =of_parse_phandle_with_args (cpu_np,"qcom,freq-domain" , +"#freq-domain-cells" ,0 , &args); +of_node_put (cpu_np); +if (ret) +return ret; - c = qcom_freq_domain_map[policy->cpu]; + c = &freq_domain_map[args.args[0 ]];if (!c) {pr_err ("No scaling support for CPU%d\n" , policy->cpu);return -ENODEV ;
@@ -171,43 +188,15 @@ static int qcom_cpufreq_hw_read_lut(struct device *dev, struct cpufreq_qcom *c,return 0 ; } -static void qcom_get_related_cpus (int index,struct cpumask *m) -{ -struct device_node *cpu_np; -struct of_phandle_args args; -int cpu, ret; - -for_each_possible_cpu (cpu) { - cpu_np =of_cpu_device_node_get (cpu); -if (!cpu_np) -continue ; - - ret =of_parse_phandle_with_args (cpu_np,"qcom,freq-domain" , -"#freq-domain-cells" ,0 , - &args); -of_node_put (cpu_np); -if (ret <0 ) -continue ; - -if (index == args.args[0 ]) -cpumask_set_cpu (cpu, m); - } -} -static int qcom_cpu_resources_init (struct platform_device *pdev, -unsigned int cpu,int index, -unsigned long xo_rate, -unsigned long cpu_hw_rate) +int index,unsigned long xo_rate, +unsigned long cpu_hw_rate, +struct cpufreq_qcom *c) { -struct cpufreq_qcom *c;struct resource *res;struct device *dev = &pdev->dev;void __iomem *base; -int ret, cpu_r; - - c =devm_kzalloc (dev,sizeof (*c),GFP_KERNEL ); -if (!c) -return -ENOMEM ; +int ret; res =platform_get_resource (pdev,IORESOURCE_MEM , index); base =devm_ioremap_resource (dev, res);
@@ -220,12 +209,6 @@ static int qcom_cpu_resources_init(struct platform_device *pdev,return -ENODEV ; } -qcom_get_related_cpus (index, &c->related_cpus); -if (!cpumask_weight (&c->related_cpus)) { -dev_err (dev,"Domain-%d failed to get related CPUs\n" , index); -return -ENOENT ; - } - c->perf_state_reg = base +REG_PERF_STATE ; ret =qcom_cpufreq_hw_read_lut (dev, c, base, xo_rate, cpu_hw_rate);
@@ -234,9 +217,6 @@ static int qcom_cpu_resources_init(struct platform_device *pdev,return ret; } -for_each_cpu (cpu_r, &c->related_cpus) - qcom_freq_domain_map[cpu_r] = c; -return 0 ; }
@@ -245,9 +225,16 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)struct device_node *cpu_np;struct of_phandle_args args;struct clk *clk; -unsigned int cpu; +unsigned int cpu, domain;unsigned long xo_rate, cpu_hw_rate;int ret; +struct cpufreq_qcom *freq_domain_map, *freq_domain; + + freq_domain_map =devm_kcalloc (&pdev->dev,MAX_FREQ_DOMAINS , +sizeof (*freq_domain_map),GFP_KERNEL ); + cpufreq_qcom_hw_driver.driver_data = freq_domain_map; +if (!freq_domain_map) +return -ENOMEM ; clk =clk_get (&pdev->dev,"xo" );if (IS_ERR (clk))
@@ -273,16 +260,26 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev) ret =of_parse_phandle_with_args (cpu_np,"qcom,freq-domain" ,"#freq-domain-cells" ,0 , - &args); + &args);of_node_put (cpu_np);if (ret)return ret; -if (qcom_freq_domain_map[cpu]) + domain = args.args[0 ]; +if (WARN_ON (domain >=MAX_FREQ_DOMAINS )) +continue ; + + freq_domain = &freq_domain_map[domain]; +cpumask_set_cpu (cpu, &freq_domain->related_cpus); +/* +* If we've already populated the frequency table for this domain +* just mark it related and get out of here +*/ +if (cpumask_weight (&freq_domain->related_cpus) >1 )continue ; - ret =qcom_cpu_resources_init (pdev, cpu, args.args[0 ], - xo_rate, cpu_hw_rate); + ret =qcom_cpu_resources_init (pdev, domain, xo_rate, + cpu_hw_rate, freq_domain);if (ret)return ret; }
Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
Hello Stephen, Viresh
Thanks for the code and suggestions.
Having a NR_DOMAINS '2' makes the driver not scalable for re-use.
This assumption is only true for the current version of the HW and do not intend to update/clean-up this logic again. So want to stick keeping current logic of having the *qcom_freq_domain_map[NR_CPUS].
Quoted 244 lines · Stephen Boyd
Quoting Stephen Boyd (2018-12-04 14:28:11)
Quoted 54 lines
Quoting Viresh Kumar (2018-12-03 21:12:31)
Quoted 8 lines
Hi Taniya,
Sorry that I haven't been reviewing it much from last few iterations as I was letting others get this into a better shape. Thanks for your efforts..
Quoted 1 line · Taniya Das
+++
b/drivers/cpufreq/qcom-cpufreq-hw.cQuoted 14 lines
Review
Quoted 7 lines
(quoted diff) +7
-0+struct cpufreq_qcom { + struct cpufreq_frequency_table *table; + void __iomem *perf_state_reg; + cpumask_t related_cpus; +}; + +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];Now that the code is much more simplified, I am not sure if you need this per-cpu structure at all. The only place where you are using it is in
qcom_cpufreq_hw_cpu_init()andprobe(). Why not mergeqcom_cpu_resources_init()completely intoqcom_cpufreq_hw_cpu_init()and get rid of this structure entirely ?Good point. Here's an untested patch to handle that. It removes the related functionality and makes an array of pointers to the domains that are created each time
qcom_cpu_resources_init()is called.---8<----
drivers/cpufreq/qcom-cpufreq-hw.c +2
-2@@ -23,14 +23,14 @@#define REG_LUT_TABLE 0x110#define REG_PERF_STATE 0x920 +#define MAX_FREQ_DOMAINS 2 +struct cpufreq_qcom {struct cpufreq_frequency_table *table;void __iomem *perf_state_reg;cpumask_t related_cpus; }; -static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS ]; -static int qcom_cpufreq_hw_target_index (struct cpufreq_policy *policy,unsigned int index) {@@ -76,9 +76,14 @@ static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,static int qcom_cpufreq_hw_cpu_init (struct cpufreq_policy *policy)drivers/cpufreq/qcom-cpufreq-hw.c static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy, · line 86
Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-hw.c +6
-1{ +struct cpufreq_qcom **freq_domain_map;struct cpufreq_qcom *c; - c = qcom_freq_domain_map[policy->cpu]; + freq_domain_map =cpufreq_get_driver_data (); +if (!freq_domain_map) +return -ENODEV ; + + c = freq_domain_map[policy->cpu];And this fails now because it indexes based on cpu number. We have to parse the frequency domain out of the cpunode again to fix that.
Here's the updated (still untested) patch and I also just allocated the freq domain array up front instead of in each domain init routine to simplify some more.
drivers/cpufreq/qcom-cpufreq-hw.c +47
-50@@ -23,14 +23,14 @@#define REG_LUT_TABLE 0x110#define REG_PERF_STATE 0x920 +#define MAX_FREQ_DOMAINS 2 +struct cpufreq_qcom {struct cpufreq_frequency_table *table;void __iomem *perf_state_reg;cpumask_t related_cpus; }; -static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS ]; -static int qcom_cpufreq_hw_target_index (struct cpufreq_policy *policy,unsigned int index) {@@ -76,9 +76,26 @@ static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,static int qcom_cpufreq_hw_cpu_init (struct cpufreq_policy *policy) { -struct cpufreq_qcom *c; +struct cpufreq_qcom *freq_domain_map, *c; +struct device_node *cpu_np; +struct of_phandle_args args; +int ret; + + freq_domain_map =cpufreq_get_driver_data (); +if (!freq_domain_map) +return -ENODEV ; + + cpu_np =of_cpu_device_node_get (policy->cpu); +if (!cpu_np) +return -ENODEV ; + + ret =of_parse_phandle_with_args (cpu_np,"qcom,freq-domain" , +"#freq-domain-cells" ,0 , &args); +of_node_put (cpu_np); +if (ret) +return ret; - c = qcom_freq_domain_map[policy->cpu]; + c = &freq_domain_map[args.args[0 ]];if (!c) {pr_err ("No scaling support for CPU%d\n" , policy->cpu);return -ENODEV ;@@ -171,43 +188,15 @@ static int qcom_cpufreq_hw_read_lut(struct device *dev, struct cpufreq_qcom *c,return 0 ; } -static void qcom_get_related_cpus (int index,struct cpumask *m) -{ -struct device_node *cpu_np; -struct of_phandle_args args; -int cpu, ret; - -for_each_possible_cpu (cpu) { - cpu_np =of_cpu_device_node_get (cpu); -if (!cpu_np) -continue ; - - ret =of_parse_phandle_with_args (cpu_np,"qcom,freq-domain" , -"#freq-domain-cells" ,0 , - &args); -of_node_put (cpu_np); -if (ret <0 ) -continue ; - -if (index == args.args[0 ]) -cpumask_set_cpu (cpu, m); - } -} -static int qcom_cpu_resources_init (struct platform_device *pdev, -unsigned int cpu,int index, -unsigned long xo_rate, -unsigned long cpu_hw_rate) +int index,unsigned long xo_rate, +unsigned long cpu_hw_rate, +struct cpufreq_qcom *c) { -struct cpufreq_qcom *c;struct resource *res;struct device *dev = &pdev->dev;void __iomem *base; -int ret, cpu_r; - - c =devm_kzalloc (dev,sizeof (*c),GFP_KERNEL ); -if (!c) -return -ENOMEM ; +int ret; res =platform_get_resource (pdev,IORESOURCE_MEM , index); base =devm_ioremap_resource (dev, res);@@ -220,12 +209,6 @@ static int qcom_cpu_resources_init(struct platform_device *pdev,return -ENODEV ; } -qcom_get_related_cpus (index, &c->related_cpus); -if (!cpumask_weight (&c->related_cpus)) { -dev_err (dev,"Domain-%d failed to get related CPUs\n" , index); -return -ENOENT ; - } - c->perf_state_reg = base +REG_PERF_STATE ; ret =qcom_cpufreq_hw_read_lut (dev, c, base, xo_rate, cpu_hw_rate);@@ -234,9 +217,6 @@ static int qcom_cpu_resources_init(struct platform_device *pdev,return ret; } -for_each_cpu (cpu_r, &c->related_cpus) - qcom_freq_domain_map[cpu_r] = c; -return 0 ; }@@ -245,9 +225,16 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)struct device_node *cpu_np;struct of_phandle_args args;struct clk *clk; -unsigned int cpu; +unsigned int cpu, domain;unsigned long xo_rate, cpu_hw_rate;int ret; +struct cpufreq_qcom *freq_domain_map, *freq_domain; + + freq_domain_map =devm_kcalloc (&pdev->dev,MAX_FREQ_DOMAINS , +sizeof (*freq_domain_map),GFP_KERNEL ); + cpufreq_qcom_hw_driver.driver_data = freq_domain_map; +if (!freq_domain_map) +return -ENOMEM ; clk =clk_get (&pdev->dev,"xo" );if (IS_ERR (clk))@@ -273,16 +260,26 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev) ret =of_parse_phandle_with_args (cpu_np,"qcom,freq-domain" ,"#freq-domain-cells" ,0 , - &args); + &args);of_node_put (cpu_np);if (ret)return ret; -if (qcom_freq_domain_map[cpu]) + domain = args.args[0 ]; +if (WARN_ON (domain >=MAX_FREQ_DOMAINS )) +continue ; + + freq_domain = &freq_domain_map[domain]; +cpumask_set_cpu (cpu, &freq_domain->related_cpus); +/* +* If we've already populated the frequency table for this domain +* just mark it related and get out of here +*/ +if (cpumask_weight (&freq_domain->related_cpus) >1 )continue ; - ret =qcom_cpu_resources_init (pdev, cpu, args.args[0 ], - xo_rate, cpu_hw_rate); + ret =qcom_cpu_resources_init (pdev, domain, xo_rate, + cpu_hw_rate, freq_domain);if (ret)return ret; }
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
Viresh Kumar
maintainerRe: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
Quoted 5 lines · Taniya Das
Hello Stephen, Viresh
Thanks for the code and suggestions.
Having a
NR_DOMAINS'2' makes the driver not scalable for re-use.
Sure, I didn't like it either and that wasn't really what I was suggesting in the first place. I didn't wanted to write the code myself and pass it on, but I have it now anyway :)
It may have a bug or two in there, but compiles just fine and is rebased over your patch Taniya.
Signature · 6 lines
viresh -------------------------8<------------------------- drivers/cpufreq/qcom-cpufreq-hw.c | 156 +++++++++++------------------- 1 file changed, 57 insertions(+), 99 deletions(-)
drivers/cpufreq/qcom-cpufreq-hw.c +57 -99
@@ -23,13 +23,8 @@#define REG_LUT_TABLE 0x110#define REG_PERF_STATE 0x920 -struct cpufreq_qcom { -struct cpufreq_frequency_table *table; -void __iomem *perf_state_reg; -cpumask_t related_cpus; -}; - -static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS ]; +static unsigned long cpu_hw_rate, xo_rate; +static struct platform_device *global_pdev;static int qcom_cpufreq_hw_target_index (struct cpufreq_policy *policy,unsigned int index)
@@ -74,53 +69,17 @@ static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,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->perf_state_reg; - -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, -}; - -static int qcom_cpufreq_hw_read_lut (struct device *dev,struct cpufreq_qcom *c, -void __iomem *base,unsigned long xo_rate, -unsigned long cpu_hw_rate) +static int qcom_cpufreq_hw_read_lut (struct device *dev, +struct cpufreq_policy *policy, +void __iomem *base) {u32 data, src, lval, i, core_count, prev_cc =0 , prev_freq =0 , freq; -unsigned int max_cores =cpumask_weight (&c->related_cpus); +unsigned int max_cores =cpumask_weight (policy->cpus); +struct cpufreq_frequency_table *table; - c->table =devm_kcalloc (dev,LUT_MAX_ENTRIES +1 , -sizeof (*c->table),GFP_KERNEL ); -if (!c->table) + table =devm_kcalloc (dev,LUT_MAX_ENTRIES +1 , +sizeof (*table),GFP_KERNEL ); +if (!table)return -ENOMEM ;for (i =0 ; i <LUT_MAX_ENTRIES ; i++) {
@@ -136,9 +95,9 @@ static int qcom_cpufreq_hw_read_lut(struct device *dev, struct cpufreq_qcom *c,/* Ignore boosts in the middle of the table */ if (core_count != max_cores) { - c->table[i].frequency =CPUFREQ_ENTRY_INVALID ; + table[i].frequency =CPUFREQ_ENTRY_INVALID ; }else { - c->table[i].frequency = freq; + table[i].frequency = freq;dev_dbg (dev,"index=%d freq=%d, core_count %d\n" , i, freq, core_count); }
@@ -148,7 +107,7 @@ static int qcom_cpufreq_hw_read_lut(struct device *dev, struct cpufreq_qcom *c,* end of table */ if (i >0 && prev_freq == freq && prev_cc == core_count) { -struct cpufreq_frequency_table *prev = &c->table[i -1 ]; +struct cpufreq_frequency_table *prev = &table[i -1 ];/* * Only treat the last frequency that might be a boost
@@ -166,7 +125,8 @@ static int qcom_cpufreq_hw_read_lut(struct device *dev, struct cpufreq_qcom *c, prev_freq = freq; } - c->table[i].frequency =CPUFREQ_TABLE_END ; + table[i].frequency =CPUFREQ_TABLE_END ; + policy->freq_table = table;return 0 ; }
@@ -194,22 +154,29 @@ static void qcom_get_related_cpus(int index, struct cpumask *m) } } -static int qcom_cpu_resources_init (struct platform_device *pdev, -unsigned int cpu,int index, -unsigned long xo_rate, -unsigned long cpu_hw_rate) +static int qcom_cpufreq_hw_cpu_init (struct cpufreq_policy *policy) { -struct cpufreq_qcom *c; +struct device *dev = &global_pdev->dev; +struct of_phandle_args args; +struct device_node *cpu_np;struct resource *res; -struct device *dev = &pdev->dev;void __iomem *base; -int ret, cpu_r; +int ret, index; - c =devm_kzalloc (dev,sizeof (*c),GFP_KERNEL ); -if (!c) -return -ENOMEM ; + cpu_np =of_cpu_device_node_get (policy->cpu); +if (!cpu_np) +return -EINVAL ; + + ret =of_parse_phandle_with_args (cpu_np,"qcom,freq-domain" , +"#freq-domain-cells" ,0 , &args); +of_node_put (cpu_np); - res =platform_get_resource (pdev,IORESOURCE_MEM , index); +if (ret) +return ret; + + index = args.args[0 ]; + + res =platform_get_resource (global_pdev,IORESOURCE_MEM , index); base =devm_ioremap_resource (dev, res);if (IS_ERR (base))return PTR_ERR (base);
@@ -220,33 +187,46 @@ static int qcom_cpu_resources_init(struct platform_device *pdev,return -ENODEV ; } -qcom_get_related_cpus (index, &c->related_cpus); -if (!cpumask_weight (&c->related_cpus)) { +qcom_get_related_cpus (index, policy->cpus); +if (!cpumask_weight (policy->cpus)) {dev_err (dev,"Domain-%d failed to get related CPUs\n" , index);return -ENOENT ; } - c->perf_state_reg = base +REG_PERF_STATE ; + policy->driver_data = base +REG_PERF_STATE ; - ret =qcom_cpufreq_hw_read_lut (dev, c, base, xo_rate, cpu_hw_rate); + ret =qcom_cpufreq_hw_read_lut (dev, policy, base);if (ret) {dev_err (dev,"Domain-%d failed to read LUT\n" , index);return ret; } -for_each_cpu (cpu_r, &c->related_cpus) - qcom_freq_domain_map[cpu_r] = c; + policy->fast_switch_possible = true;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, +}; +static int qcom_cpufreq_hw_driver_probe (struct platform_device *pdev) { -struct device_node *cpu_np; -struct of_phandle_args args;struct clk *clk; -unsigned int cpu; -unsigned long xo_rate, cpu_hw_rate;int ret; clk =clk_get (&pdev->dev,"xo" );
@@ -263,29 +243,7 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev) cpu_hw_rate =clk_get_rate (clk) /CLK_HW_DIV ;clk_put (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 ; - } - - ret =of_parse_phandle_with_args (cpu_np,"qcom,freq-domain" , -"#freq-domain-cells" ,0 , - &args); -of_node_put (cpu_np); -if (ret) -return ret; - -if (qcom_freq_domain_map[cpu]) -continue ; - - ret =qcom_cpu_resources_init (pdev, cpu, args.args[0 ], - xo_rate, cpu_hw_rate); -if (ret) -return ret; - } + global_pdev = pdev; ret =cpufreq_register_driver (&cpufreq_qcom_hw_driver);if (ret) {
Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
Quoting Viresh Kumar (2018-12-04 22:16:00)
Quoted 5 lines · Taniya Das
Quoted 4 lines
Hello Stephen, Viresh
Thanks for the code and suggestions.
Quoted 8 lines
Quoted 1 line
Having a
NR_DOMAINS'2' makes the driver not scalable for re-use.Sure, I didn't like it either and that wasn't really what I was suggesting in the first place. I didn't wanted to write the code myself and pass it on, but I have it now anyway :)
It may have a bug or two in there, but compiles just fine and is rebased over your patch Taniya.
If we move the ioremap of registers to the cpufreq_driver::init path then we need to unmap it on cpufreq_driver::exit. In fact, all the devm_*() code in the init path needs to change to non-devm. Otherwise it's a nice simplification!
Quoted 4 lines
(quoted diff) +3
-0+static int qcom_cpufreq_hw_read_lut(struct device *dev, + struct cpufreq_policy *policy, + void __iomem *base) {
Review
Quoted 10 lines
(quoted diff) +4
-4u32 data, src, lval, i, core_count, prev_cc = 0, prev_freq = 0, freq; - unsigned int max_cores = cpumask_weight(&c->related_cpus); + unsigned int max_cores = cpumask_weight(policy->cpus); + struct cpufreq_frequency_table *table; - c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1, - sizeof(*c->table), GFP_KERNEL); - if (!c->table) + table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1, + sizeof(*table), GFP_KERNEL);
This one.
Quoted 33 lines
(quoted diff) +12
-10+ if (!table) return -ENOMEM; for (i = 0; i < LUT_MAX_ENTRIES; i++) {@@ -194,22 +154,29 @@ static void qcom_get_related_cpus(int index, struct cpumask *m) } } -static int qcom_cpu_resources_init(struct platform_device *pdev, - unsigned int cpu, int index, - unsigned long xo_rate, - unsigned long cpu_hw_rate) +static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) { - struct cpufreq_qcom *c; + struct device *dev = &global_pdev->dev; + struct of_phandle_args args; + struct device_node *cpu_np; struct resource *res; - struct device *dev = &pdev->dev; void __iomem *base; - int ret, cpu_r; + int ret, index; - c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL); - if (!c) - return -ENOMEM; + cpu_np = of_cpu_device_node_get(policy->cpu); + if (!cpu_np) + return -EINVAL; + + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain", + "#freq-domain-cells", 0, &args);
Review static void qcom_get_related_cpus(int index, struct cpumask *m) · line 180
Quoted 10 lines
(quoted diff) +7
-1+ of_node_put(cpu_np); - res = platform_get_resource(pdev, IORESOURCE_MEM, index); + if (ret) + return ret; + + index = args.args[0]; + + res = platform_get_resource(global_pdev, IORESOURCE_MEM, index); base = devm_ioremap_resource(dev, res);
And this one.
Review static void qcom_get_related_cpus(int index, struct cpumask *m) · line 182
Quoted 2 lines
(quoted diff) +0
-0if (IS_ERR(base)) return PTR_ERR(base);
Here's a patch to squash in to fix those tiny problems. I left it as devm_ioremap_resource() because that has all the nice features of resource requesting inside and it didn't seem too bad to devm_iounmap() on the exit path.
---------8<--------------
drivers/cpufreq/qcom-cpufreq-hw.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-)
drivers/cpufreq/qcom-cpufreq-hw.c +26 -11
@@ -10,6 +10,7 @@#include <linux/module.h> #include <linux/of_address.h> #include <linux/of_platform.h> +#include <linux/slab.h> #define LUT_MAX_ENTRIES 40U#define LUT_SRC GENMASK(31, 30)
@@ -77,8 +78,7 @@ static int qcom_cpufreq_hw_read_lut(struct device *dev,unsigned int max_cores =cpumask_weight (policy->cpus);struct cpufreq_frequency_table *table; - table =devm_kcalloc (dev,LUT_MAX_ENTRIES +1 , -sizeof (*table),GFP_KERNEL ); + table =kcalloc (LUT_MAX_ENTRIES +1 ,sizeof (*table),GFP_KERNEL );if (!table)return -ENOMEM ;
@@ -144,7 +144,7 @@ static void qcom_get_related_cpus(int index, struct cpumask *m) ret =of_parse_phandle_with_args (cpu_np,"qcom,freq-domain" ,"#freq-domain-cells" ,0 , - &args); + &args);of_node_put (cpu_np);if (ret <0 )continue ;
@@ -170,7 +170,6 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) ret =of_parse_phandle_with_args (cpu_np,"qcom,freq-domain" ,"#freq-domain-cells" ,0 , &args);of_node_put (cpu_np); -if (ret)return ret;
@@ -184,13 +183,15 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)/* HW should be in enabled state to proceed */ if (!(readl_relaxed (base +REG_ENABLE ) &0x1 )) {dev_err (dev,"Domain-%d cpufreq hardware not enabled\n" , index); -return -ENODEV ; + ret = -ENODEV ; + goto error; }qcom_get_related_cpus (index, policy->cpus);if (!cpumask_weight (policy->cpus)) {dev_err (dev,"Domain-%d failed to get related CPUs\n" , index); -return -ENOENT ; + ret = -ENOENT ; + goto error; } policy->driver_data = base +REG_PERF_STATE ;
@@ -198,11 +199,25 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) ret =qcom_cpufreq_hw_read_lut (dev, policy, base);if (ret) {dev_err (dev,"Domain-%d failed to read LUT\n" , index); -return ret; + goto error; } policy->fast_switch_possible = true; +return 0 ; + +error: +devm_iounmap (dev, base); +return ret; +} + +static int qcom_cpufreq_hw_cpu_exit (struct cpufreq_policy *policy) +{ +void __iomem *base = policy->driver_data -REG_PERF_STATE ; + +kfree (policy->freq_table); +devm_iounmap (&global_pdev->dev, base); +return 0 ; }
@@ -219,6 +234,7 @@ static struct cpufreq_driver cpufreq_qcom_hw_driver = { .target_index = qcom_cpufreq_hw_target_index, .get = qcom_cpufreq_hw_get, .init = qcom_cpufreq_hw_cpu_init, + .exit = qcom_cpufreq_hw_cpu_exit, .fast_switch = qcom_cpufreq_hw_fast_switch, .name = "qcom-cpufreq-hw", .attr = qcom_cpufreq_hw_attr,
@@ -248,12 +264,11 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev) ret =cpufreq_register_driver (&cpufreq_qcom_hw_driver);if (ret) {dev_err (&pdev->dev,"CPUFreq HW driver failed to register\n" ); -return ret; + }else { +dev_dbg (&pdev->dev,"QCOM CPUFreq HW driver initialized\n" ); } -dev_dbg (&pdev->dev,"QCOM CPUFreq HW driver initialized\n" ); - -return 0 ; +return ret; }static int qcom_cpufreq_hw_driver_remove (struct platform_device *pdev)
Viresh Kumar
maintainerRe: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
Quoted 108 lines · Stephen Boyd
Here's a patch to squash in to fix those tiny problems. I left it as
devm_ioremap_resource()because that has all the nice features of resource requesting inside and it didn't seem too bad todevm_iounmap()on the exit path.---------8<--------------
drivers/cpufreq/qcom-cpufreq-hw.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-)drivers/cpufreq/qcom-cpufreq-hw.c +24
-8@@ -10,6 +10,7 @@#include <linux/module.h> #include <linux/of_address.h> #include <linux/of_platform.h> +#include <linux/slab.h> #define LUT_MAX_ENTRIES 40U#define LUT_SRC GENMASK(31, 30)@@ -77,8 +78,7 @@ static int qcom_cpufreq_hw_read_lut(struct device *dev,unsigned int max_cores =cpumask_weight (policy->cpus);struct cpufreq_frequency_table *table; - table =devm_kcalloc (dev,LUT_MAX_ENTRIES +1 , -sizeof (*table),GFP_KERNEL ); + table =kcalloc (LUT_MAX_ENTRIES +1 ,sizeof (*table),GFP_KERNEL );if (!table)return -ENOMEM ;@@ -144,7 +144,7 @@ static void qcom_get_related_cpus(int index, struct cpumask *m) ret =of_parse_phandle_with_args (cpu_np,"qcom,freq-domain" ,"#freq-domain-cells" ,0 , - &args); + &args);of_node_put (cpu_np);if (ret <0 )continue ;@@ -170,7 +170,6 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) ret =of_parse_phandle_with_args (cpu_np,"qcom,freq-domain" ,"#freq-domain-cells" ,0 , &args);of_node_put (cpu_np); -if (ret)return ret;@@ -184,13 +183,15 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)/* HW should be in enabled state to proceed */ if (!(readl_relaxed (base +REG_ENABLE ) &0x1 )) {dev_err (dev,"Domain-%d cpufreq hardware not enabled\n" , index); -return -ENODEV ; + ret = -ENODEV ; + goto error; }qcom_get_related_cpus (index, policy->cpus);if (!cpumask_weight (policy->cpus)) {dev_err (dev,"Domain-%d failed to get related CPUs\n" , index); -return -ENOENT ; + ret = -ENOENT ; + goto error; } policy->driver_data = base +REG_PERF_STATE ;@@ -198,11 +199,25 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) ret =qcom_cpufreq_hw_read_lut (dev, policy, base);if (ret) {dev_err (dev,"Domain-%d failed to read LUT\n" , index); -return ret; + goto error; } policy->fast_switch_possible = true; +return 0 ; + +error: +devm_iounmap (dev, base); +return ret; +} + +static int qcom_cpufreq_hw_cpu_exit (struct cpufreq_policy *policy) +{ +void __iomem *base = policy->driver_data -REG_PERF_STATE ; + +kfree (policy->freq_table); +devm_iounmap (&global_pdev->dev, base); +return 0 ; }@@ -219,6 +234,7 @@ static struct cpufreq_driver cpufreq_qcom_hw_driver = { .target_index = qcom_cpufreq_hw_target_index, .get = qcom_cpufreq_hw_get, .init = qcom_cpufreq_hw_cpu_init, + .exit = qcom_cpufreq_hw_cpu_exit, .fast_switch = qcom_cpufreq_hw_fast_switch, .name = "qcom-cpufreq-hw", .attr = qcom_cpufreq_hw_attr,@@ -248,12 +264,11 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev) ret =cpufreq_register_driver (&cpufreq_qcom_hw_driver);if (ret) {dev_err (&pdev->dev,"CPUFreq HW driver failed to register\n" ); -return ret; + }else {
drivers/cpufreq/qcom-cpufreq-hw.c static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev) · line 274
Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-hw.c +2
-3+dev_dbg (&pdev->dev,"QCOM CPUFreq HW driver initialized\n" ); } -dev_dbg (&pdev->dev,"QCOM CPUFreq HW driver initialized\n" ); - -return 0 ; +return ret; }static int qcom_cpufreq_hw_driver_remove (struct platform_device *pdev)
LGTM
Signature · 1 line
viresh
Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
In reply to Viresh KumarHi Viresh,
Sorry for the delayed response. Thanks for the patch.
Quoted 5 lines · Viresh Kumar
Quoted 4 lines · Taniya Das
Hello Stephen, Viresh
Thanks for the code and suggestions.
Quoted 9 lines
Quoted 1 line
Having a
NR_DOMAINS'2' makes the driver not scalable for re-use.Sure, I didn't like it either and that wasn't really what I was suggesting in the first place. I didn't wanted to write the code myself and pass it on, but I have it now anyway :)
It may have a bug or two in there, but compiles just fine and is rebased over your patch Taniya.
The design here assumes that there would not be any per-cpu/per-cluster based SW requirement for the HW during frequency transitions, which again makes me think that we would require to re-introduce these structures again in case we have such requirements in near future.
Also I think leaving the structures also helps us debug any boot up issues looking at the ram contents of the per-(cpu/cluster) structures with the contents from the firmware.
Hope these above helps us to go ahead with the current SW design.
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
Viresh Kumar
maintainerRe: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
Quoted 4 lines · Taniya Das
The design here assumes that there would not be any per-cpu/per-cluster based SW requirement for the HW during frequency transitions, which again makes me think that we would require to re-introduce these structures again in case we have such requirements in near future.
Firstly, even in such cases we can go ahead with the design we proposed. And I am not at all concerned about some hardware which we don't have right now. We will see what to do when such hardware comes, maybe reintroduce the structures, but that doesn't matter right now.
Quoted 3 lines
Also I think leaving the structures also helps us debug any boot up issues looking at the ram contents of the per-(cpu/cluster) structures with the contents from the firmware.
I don't see how debugging would be hard without those structures in place.
Quoted 1 line
Hope these above helps us to go ahead with the current SW design.
Sorry, but I don't see any satisfactory reason on why you shouldn't make the suggested changes. We are trying to make your (and any other developer who will work on that driver) life simple by simplifying the code. Nothing beyond that :)
Signature · 1 line
viresh
Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
Hello Viresh,
Quoted 15 lines · Viresh Kumar
Quoted 4 lines · Taniya Das
The design here assumes that there would not be any per-cpu/per-cluster based SW requirement for the HW during frequency transitions, which again makes me think that we would require to re-introduce these structures again in case we have such requirements in near future.
Firstly, even in such cases we can go ahead with the design we proposed. And I am not at all concerned about some hardware which we don't have right now. We will see what to do when such hardware comes, maybe reintroduce the structures, but that doesn't matter right now.
Quoted 3 lines
Also I think leaving the structures also helps us debug any boot up issues looking at the ram contents of the per-(cpu/cluster) structures with the contents from the firmware.
Quoted 8 lines
I don't see how debugging would be hard without those structures in place.
Quoted 1 line
Hope these above helps us to go ahead with the current SW design.
Sorry, but I don't see any satisfactory reason on why you shouldn't make the suggested changes. We are trying to make your (and any other developer who will work on that driver) life simple by simplifying the code. Nothing beyond that :)
Sure, Thanks, will submit the next patch series for your ACK :).
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 v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
In reply to Taniya DasQuoting Taniya Das (2018-12-04 19:37:00)
Quoted 8 lines
Hello Stephen, Viresh
Thanks for the code and suggestions.
Having a
NR_DOMAINS'2' makes the driver not scalable for re-use. This assumption is only true for the current version of the HW and do not intend to update/clean-up this logic again. So want to stick keeping current logic of having the *qcom_freq_domain_map[NR_CPUS].
Ok, so let's just count the number of reg properties there are in the node and use that to know how many frequency domains exist. That already maps exactly to how many frequency domains this driver needs to worry about. Alternatively, we could count the number of named register regions like 'freq_domain0', 'freq_domain1', etc. are listed in reg-names so that other register regions in this device could be exposed in DT when/if they are added. Here's the reg counting patch.
drivers/cpufreq/qcom-cpufreq-hw.c +49 -50
@@ -29,8 +29,6 @@ struct cpufreq_qcom {cpumask_t related_cpus; }; -static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS ]; -static int qcom_cpufreq_hw_target_index (struct cpufreq_policy *policy,unsigned int index) {
@@ -76,9 +74,26 @@ static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,static int qcom_cpufreq_hw_cpu_init (struct cpufreq_policy *policy) { -struct cpufreq_qcom *c; +struct cpufreq_qcom *freq_domains, *c; +struct device_node *cpu_np; +struct of_phandle_args args; +int ret; + + freq_domains =cpufreq_get_driver_data (); +if (!freq_domains) +return -ENODEV ; + + cpu_np =of_cpu_device_node_get (policy->cpu); +if (!cpu_np) +return -ENODEV ; + + ret =of_parse_phandle_with_args (cpu_np,"qcom,freq-domain" , +"#freq-domain-cells" ,0 , &args); +of_node_put (cpu_np); +if (ret) +return ret; - c = qcom_freq_domain_map[policy->cpu]; + c = &freq_domains[args.args[0 ]];if (!c) {pr_err ("No scaling support for CPU%d\n" , policy->cpu);return -ENODEV ;
@@ -171,43 +186,15 @@ static int qcom_cpufreq_hw_read_lut(struct device *dev, struct cpufreq_qcom *c,return 0 ; } -static void qcom_get_related_cpus (int index,struct cpumask *m) -{ -struct device_node *cpu_np; -struct of_phandle_args args; -int cpu, ret; - -for_each_possible_cpu (cpu) { - cpu_np =of_cpu_device_node_get (cpu); -if (!cpu_np) -continue ; - - ret =of_parse_phandle_with_args (cpu_np,"qcom,freq-domain" , -"#freq-domain-cells" ,0 , - &args); -of_node_put (cpu_np); -if (ret <0 ) -continue ; - -if (index == args.args[0 ]) -cpumask_set_cpu (cpu, m); - } -} -static int qcom_cpu_resources_init (struct platform_device *pdev, -unsigned int cpu,int index, -unsigned long xo_rate, -unsigned long cpu_hw_rate) +int index,unsigned long xo_rate, +unsigned long cpu_hw_rate, +struct cpufreq_qcom *c) { -struct cpufreq_qcom *c;struct resource *res;struct device *dev = &pdev->dev;void __iomem *base; -int ret, cpu_r; - - c =devm_kzalloc (dev,sizeof (*c),GFP_KERNEL ); -if (!c) -return -ENOMEM ; +int ret; res =platform_get_resource (pdev,IORESOURCE_MEM , index); base =devm_ioremap_resource (dev, res);
@@ -220,12 +207,6 @@ static int qcom_cpu_resources_init(struct platform_device *pdev,return -ENODEV ; } -qcom_get_related_cpus (index, &c->related_cpus); -if (!cpumask_weight (&c->related_cpus)) { -dev_err (dev,"Domain-%d failed to get related CPUs\n" , index); -return -ENOENT ; - } - c->perf_state_reg = base +REG_PERF_STATE ; ret =qcom_cpufreq_hw_read_lut (dev, c, base, xo_rate, cpu_hw_rate);
@@ -234,9 +215,6 @@ static int qcom_cpu_resources_init(struct platform_device *pdev,return ret; } -for_each_cpu (cpu_r, &c->related_cpus) - qcom_freq_domain_map[cpu_r] = c; -return 0 ; }
@@ -245,9 +223,20 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)struct device_node *cpu_np;struct of_phandle_args args;struct clk *clk; -unsigned int cpu; +unsigned int cpu, domain, max_domains;unsigned long xo_rate, cpu_hw_rate;int ret; +struct cpufreq_qcom *freq_domains, *freq_domain; + + max_domains =0 ; +while (platform_get_resource (pdev,IORESOURCE_MEM , max_domains)) + max_domains++; + + freq_domains =devm_kcalloc (&pdev->dev, max_domains, +sizeof (*freq_domains),GFP_KERNEL ); + cpufreq_qcom_hw_driver.driver_data = freq_domains; +if (!freq_domains) +return -ENOMEM ; clk =clk_get (&pdev->dev,"xo" );if (IS_ERR (clk))
@@ -273,16 +262,26 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev) ret =of_parse_phandle_with_args (cpu_np,"qcom,freq-domain" ,"#freq-domain-cells" ,0 , - &args); + &args);of_node_put (cpu_np);if (ret)return ret; -if (qcom_freq_domain_map[cpu]) + domain = args.args[0 ]; +if (WARN_ON (domain >= max_domains)) +continue ; + + freq_domain = &freq_domains[domain]; +cpumask_set_cpu (cpu, &freq_domain->related_cpus); +/* +* If we've already populated the frequency table for this domain +* just mark it related and get out of here +*/ +if (cpumask_weight (&freq_domain->related_cpus) >1 )continue ; - ret =qcom_cpu_resources_init (pdev, cpu, args.args[0 ], - xo_rate, cpu_hw_rate); + ret =qcom_cpu_resources_init (pdev, domain, xo_rate, + cpu_hw_rate, freq_domain);if (ret)return ret; }