* [PATCH v11 0/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW @ 2018-12-02 3:55 Taniya Das 2018-12-02 3:55 ` [PATCH v11 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings Taniya Das 2018-12-02 3:55 ` [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver Taniya Das 0 siblings, 2 replies; 21+ messages in thread From: Taniya Das @ 2018-12-02 3:55 UTC (permalink / raw) To: Rafael J. Wysocki, Viresh Kumar, linux-kernel, linux-pm, Stephen Boyd Cc: Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen, Taniya Das [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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v11 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings 2018-12-02 3:55 [PATCH v11 0/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW Taniya Das @ 2018-12-02 3:55 ` Taniya Das 2018-12-03 16:55 ` Stephen Boyd 2018-12-03 23:09 ` Rob Herring 2018-12-02 3:55 ` [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver Taniya Das 1 sibling, 2 replies; 21+ messages in thread From: Taniya Das @ 2018-12-02 3:55 UTC (permalink / raw) To: Rafael J. Wysocki, Viresh Kumar, linux-kernel, linux-pm, Stephen Boyd Cc: Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen, 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 <tdas@codeaurora.org> --- .../bindings/cpufreq/cpufreq-qcom-hw.txt | 172 +++++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file mode 100644 index 0000000..2b82965 --- /dev/null +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt @@ -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>; + }; +} -- Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation. ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings 2018-12-02 3:55 ` [PATCH v11 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings Taniya Das @ 2018-12-03 16:55 ` Stephen Boyd 2018-12-03 23:09 ` Rob Herring 1 sibling, 0 replies; 21+ messages in thread From: Stephen Boyd @ 2018-12-03 16:55 UTC (permalink / raw) To: Rafael J. Wysocki, Taniya Das, Viresh Kumar, linux-kernel, linux-pm Cc: Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen, Taniya Das Quoting Taniya Das (2018-12-01 19:55:02) > 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 <tdas@codeaurora.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings 2018-12-02 3:55 ` [PATCH v11 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings Taniya Das 2018-12-03 16:55 ` Stephen Boyd @ 2018-12-03 23:09 ` Rob Herring 2018-12-03 23:57 ` Stephen Boyd 1 sibling, 1 reply; 21+ messages in thread From: Rob Herring @ 2018-12-03 23:09 UTC (permalink / raw) To: Taniya Das Cc: Rafael J. Wysocki, Viresh Kumar, linux-kernel, linux-pm, Stephen Boyd, Rajendra Nayak, devicetree, skannan, linux-arm-msm, evgreen On Sun, Dec 02, 2018 at 09:25:02AM +0530, Taniya Das wrote: > 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 <tdas@codeaurora.org> > --- > .../bindings/cpufreq/cpufreq-qcom-hw.txt | 172 +++++++++++++++++++++ > 1 file changed, 172 insertions(+) > create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt > > diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt > new file mode 100644 > index 0000000..2b82965 > --- /dev/null > +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt > @@ -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: #freq-domain-cells Otherwise, Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings 2018-12-03 23:09 ` Rob Herring @ 2018-12-03 23:57 ` Stephen Boyd 0 siblings, 0 replies; 21+ messages in thread From: Stephen Boyd @ 2018-12-03 23:57 UTC (permalink / raw) To: Rob Herring, Taniya Das Cc: Rafael J. Wysocki, Viresh Kumar, linux-kernel, linux-pm, Rajendra Nayak, devicetree, skannan, linux-arm-msm, evgreen Quoting Rob Herring (2018-12-03 15:09:07) > On Sun, Dec 02, 2018 at 09:25:02AM +0530, Taniya Das wrote: > > 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 <tdas@codeaurora.org> > > --- > > .../bindings/cpufreq/cpufreq-qcom-hw.txt | 172 +++++++++++++++++++++ > > 1 file changed, 172 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt > > > > diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt > > new file mode 100644 > > index 0000000..2b82965 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt > > @@ -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: > > #freq-domain-cells > > Otherwise, > > Reviewed-by: Rob Herring <robh@kernel.org> Or should it be #qcom,freq-domain-cells? That would match the same stem of the property used in the cpu node. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver 2018-12-02 3:55 [PATCH v11 0/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW Taniya Das 2018-12-02 3:55 ` [PATCH v11 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings Taniya Das @ 2018-12-02 3:55 ` Taniya Das 2018-12-03 16:57 ` Stephen Boyd ` (2 more replies) 1 sibling, 3 replies; 21+ messages in thread From: Taniya Das @ 2018-12-02 3:55 UTC (permalink / raw) To: Rafael J. Wysocki, Viresh Kumar, linux-kernel, linux-pm, Stephen Boyd Cc: Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen, Taniya Das, Stephen Boyd 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 <skannan@codeaurora.org> Signed-off-by: Stephen Boyd <swboyd@chromium.org> Signed-off-by: Taniya Das <tdas@codeaurora.org> --- 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 diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 4e1131e..688f102 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -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 diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index d5ee456..08c071b 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -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 diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c new file mode 100644 index 0000000..8dc6b73 --- /dev/null +++ b/drivers/cpufreq/qcom-cpufreq-hw.c @@ -0,0 +1,334 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2018, The Linux Foundation. All rights reserved. + */ + +#include <linux/bitfield.h> +#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 LUT_MAX_ENTRIES 40U +#define LUT_SRC GENMASK(31, 30) +#define LUT_L_VAL GENMASK(7, 0) +#define LUT_CORE_COUNT GENMASK(18, 16) +#define LUT_ROW_SIZE 32 +#define CLK_HW_DIV 2 + +/* Register offsets */ +#define REG_ENABLE 0x0 +#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 int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy, + unsigned int index) +{ + void __iomem *perf_state_reg = policy->driver_data; + + writel_relaxed(index, perf_state_reg); + + return 0; +} + +static unsigned int qcom_cpufreq_hw_get(unsigned int cpu) +{ + void __iomem *perf_state_reg; + struct cpufreq_policy *policy; + unsigned int index; + + policy = cpufreq_cpu_get_raw(cpu); + if (!policy) + return 0; + + perf_state_reg = policy->driver_data; + + index = readl_relaxed(perf_state_reg); + 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) +{ + void __iomem *perf_state_reg = policy->driver_data; + int index; + + index = policy->cached_resolved_idx; + if (index < 0) + return 0; + + writel_relaxed(index, perf_state_reg); + + 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) +{ + 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 + 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); + } + + /* + * Two of the same frequencies with the same core counts means + * end of table + */ + if (i > 0 && prev_freq == freq && prev_cc == core_count) { + struct cpufreq_frequency_table *prev = &c->table[i - 1]; + + /* + * Only treat the last frequency that might be a boost + * as the boost frequency + */ + if (prev_cc != max_cores) { + prev->frequency = prev_freq; + prev->flags = CPUFREQ_BOOST_FREQ; + } + + break; + } + + prev_cc = core_count; + prev_freq = freq; + } + + c->table[i].frequency = CPUFREQ_TABLE_END; + + 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) +{ + 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; + + res = platform_get_resource(pdev, IORESOURCE_MEM, index); + base = devm_ioremap_resource(dev, res); + if (IS_ERR(base)) + return PTR_ERR(base); + + /* 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; + } + + 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); + 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; + + return 0; +} + +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"); + if (IS_ERR(clk)) + return PTR_ERR(clk); + + xo_rate = clk_get_rate(clk); + clk_put(clk); + + clk = clk_get(&pdev->dev, "alternate"); + if (IS_ERR(clk)) + return PTR_ERR(clk); + + 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; + } + + ret = cpufreq_register_driver(&cpufreq_qcom_hw_driver); + if (ret) { + dev_err(&pdev->dev, "CPUFreq HW driver failed to register\n"); + return ret; + } + + dev_dbg(&pdev->dev, "QCOM CPUFreq HW driver initialized\n"); + + return 0; +} + +static int qcom_cpufreq_hw_driver_remove(struct platform_device *pdev) +{ + return cpufreq_unregister_driver(&cpufreq_qcom_hw_driver); +} + +static const struct of_device_id qcom_cpufreq_hw_match[] = { + { .compatible = "qcom,cpufreq-hw" }, + {} +}; +MODULE_DEVICE_TABLE(of, qcom_cpufreq_hw_match); + +static struct platform_driver qcom_cpufreq_hw_driver = { + .probe = qcom_cpufreq_hw_driver_probe, + .remove = qcom_cpufreq_hw_driver_remove, + .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); + +static void __exit qcom_cpufreq_hw_exit(void) +{ + platform_driver_unregister(&qcom_cpufreq_hw_driver); +} +module_exit(qcom_cpufreq_hw_exit); + +MODULE_DESCRIPTION("QCOM CPUFREQ HW Driver"); +MODULE_LICENSE("GPL v2"); -- Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation. ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver 2018-12-02 3:55 ` [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver Taniya Das @ 2018-12-03 16:57 ` Stephen Boyd 2018-12-03 17:46 ` Matthias Kaehlcke 2018-12-04 5:12 ` Viresh Kumar 2 siblings, 0 replies; 21+ messages in thread From: Stephen Boyd @ 2018-12-03 16:57 UTC (permalink / raw) To: Rafael J. Wysocki, Taniya Das, Viresh Kumar, linux-kernel, linux-pm Cc: Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen, Taniya Das Quoting Taniya Das (2018-12-01 19:55:03) > 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 <skannan@codeaurora.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > Signed-off-by: Taniya Das <tdas@codeaurora.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver 2018-12-02 3:55 ` [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver Taniya Das 2018-12-03 16:57 ` Stephen Boyd @ 2018-12-03 17:46 ` Matthias Kaehlcke 2018-12-04 5:12 ` Viresh Kumar 2 siblings, 0 replies; 21+ messages in thread From: Matthias Kaehlcke @ 2018-12-03 17:46 UTC (permalink / raw) To: Taniya Das Cc: Rafael J. Wysocki, Viresh Kumar, linux-kernel, linux-pm, Stephen Boyd, Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen, Stephen Boyd On Sun, Dec 02, 2018 at 09:25:03AM +0530, Taniya Das wrote: > 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 <skannan@codeaurora.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > Signed-off-by: Taniya Das <tdas@codeaurora.org> > --- > 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 > > ... > > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > new file mode 100644 > index 0000000..8dc6b73 > --- /dev/null > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > > ... > > +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 > + 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. > +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. Reviewed-by: Matthias Kaehlcke <mka@chromium.org> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver 2018-12-02 3:55 ` [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver Taniya Das 2018-12-03 16:57 ` Stephen Boyd 2018-12-03 17:46 ` Matthias Kaehlcke @ 2018-12-04 5:12 ` Viresh Kumar 2018-12-04 9:27 ` Taniya Das 2018-12-04 22:28 ` Stephen Boyd 2 siblings, 2 replies; 21+ messages in thread From: Viresh Kumar @ 2018-12-04 5:12 UTC (permalink / raw) To: Taniya Das Cc: Rafael J. Wysocki, linux-kernel, linux-pm, Stephen Boyd, Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen, Stephen Boyd 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.. On 02-12-18, 09:25, Taniya Das wrote: > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > +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 ? -- viresh ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver 2018-12-04 5:12 ` Viresh Kumar @ 2018-12-04 9:27 ` Taniya Das 2018-12-04 9:29 ` Viresh Kumar 2018-12-04 22:28 ` Stephen Boyd 1 sibling, 1 reply; 21+ messages in thread From: Taniya Das @ 2018-12-04 9:27 UTC (permalink / raw) To: Viresh Kumar Cc: Rafael J. Wysocki, linux-kernel, linux-pm, Stephen Boyd, Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen, Stephen Boyd Hello Viresh, On 12/4/2018 10:42 AM, Viresh Kumar wrote: > 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.. > > On 02-12-18, 09:25, Taniya Das wrote: >> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > >> +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 ? > Yes, we still would require the per-cpu. -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. -- ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver 2018-12-04 9:27 ` Taniya Das @ 2018-12-04 9:29 ` Viresh Kumar 0 siblings, 0 replies; 21+ messages in thread From: Viresh Kumar @ 2018-12-04 9:29 UTC (permalink / raw) To: Taniya Das Cc: Rafael J. Wysocki, linux-kernel, linux-pm, Stephen Boyd, Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen, Stephen Boyd On 04-12-18, 14:57, Taniya Das wrote: > Hello Viresh, > > On 12/4/2018 10:42 AM, Viresh Kumar wrote: > > 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.. > > > > On 02-12-18, 09:25, Taniya Das wrote: > > > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > > > > > +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 ? > > > > 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. -- viresh ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver 2018-12-04 5:12 ` Viresh Kumar 2018-12-04 9:27 ` Taniya Das @ 2018-12-04 22:28 ` Stephen Boyd 2018-12-04 23:05 ` Stephen Boyd 1 sibling, 1 reply; 21+ messages in thread From: Stephen Boyd @ 2018-12-04 22:28 UTC (permalink / raw) To: Taniya Das, Viresh Kumar Cc: Rafael J. Wysocki, linux-kernel, linux-pm, Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen Quoting Viresh Kumar (2018-12-03 21:12:31) > 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.. > > On 02-12-18, 09:25, Taniya Das wrote: > > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > > > +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 ? > 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<---- diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c index 8dc6b73c2f22..04e7cfd70316 100644 --- a/drivers/cpufreq/qcom-cpufreq-hw.c +++ b/drivers/cpufreq/qcom-cpufreq-hw.c @@ -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; } ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver 2018-12-04 22:28 ` Stephen Boyd @ 2018-12-04 23:05 ` Stephen Boyd 2018-12-05 3:37 ` Taniya Das 0 siblings, 1 reply; 21+ messages in thread From: Stephen Boyd @ 2018-12-04 23:05 UTC (permalink / raw) To: Taniya Das, Viresh Kumar Cc: Rafael J. Wysocki, linux-kernel, linux-pm, Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen Quoting Stephen Boyd (2018-12-04 14:28:11) > Quoting Viresh Kumar (2018-12-03 21:12:31) > > 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.. > > > > On 02-12-18, 09:25, Taniya Das wrote: > > > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > > > > > +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 ? > > > > 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<---- > > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > index 8dc6b73c2f22..04e7cfd70316 100644 > --- a/drivers/cpufreq/qcom-cpufreq-hw.c > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > @@ -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]; 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. diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c index 8dc6b73c2f22..bc0d734f7e3c 100644 --- a/drivers/cpufreq/qcom-cpufreq-hw.c +++ b/drivers/cpufreq/qcom-cpufreq-hw.c @@ -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; } ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver 2018-12-04 23:05 ` Stephen Boyd @ 2018-12-05 3:37 ` Taniya Das 2018-12-05 6:16 ` Viresh Kumar 2018-12-05 8:07 ` Stephen Boyd 0 siblings, 2 replies; 21+ messages in thread From: Taniya Das @ 2018-12-05 3:37 UTC (permalink / raw) To: Stephen Boyd, Viresh Kumar Cc: Rafael J. Wysocki, linux-kernel, linux-pm, Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen 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]. On 12/5/2018 4:35 AM, Stephen Boyd wrote: > Quoting Stephen Boyd (2018-12-04 14:28:11) >> Quoting Viresh Kumar (2018-12-03 21:12:31) >>> 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.. >>> >>> On 02-12-18, 09:25, Taniya Das wrote: >>>> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c >>> >>>> +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 ? >>> >> >> 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<---- >> >> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c >> index 8dc6b73c2f22..04e7cfd70316 100644 >> --- a/drivers/cpufreq/qcom-cpufreq-hw.c >> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c >> @@ -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]; > > 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. > > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > index 8dc6b73c2f22..bc0d734f7e3c 100644 > --- a/drivers/cpufreq/qcom-cpufreq-hw.c > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > @@ -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; > } > -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. -- ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver 2018-12-05 3:37 ` Taniya Das @ 2018-12-05 6:16 ` Viresh Kumar 2018-12-05 17:00 ` Stephen Boyd 2018-12-11 13:35 ` Taniya Das 2018-12-05 8:07 ` Stephen Boyd 1 sibling, 2 replies; 21+ messages in thread From: Viresh Kumar @ 2018-12-05 6:16 UTC (permalink / raw) To: Taniya Das Cc: Stephen Boyd, Rafael J. Wysocki, linux-kernel, linux-pm, Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen On 05-12-18, 09:07, Taniya Das wrote: > 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. -- viresh -------------------------8<------------------------- drivers/cpufreq/qcom-cpufreq-hw.c | 156 +++++++++++------------------- 1 file changed, 57 insertions(+), 99 deletions(-) diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c index 8dc6b73c2f22..bcf9bb37298a 100644 --- a/drivers/cpufreq/qcom-cpufreq-hw.c +++ b/drivers/cpufreq/qcom-cpufreq-hw.c @@ -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) { ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver 2018-12-05 6:16 ` Viresh Kumar @ 2018-12-05 17:00 ` Stephen Boyd 2018-12-06 4:22 ` Viresh Kumar 2018-12-11 13:35 ` Taniya Das 1 sibling, 1 reply; 21+ messages in thread From: Stephen Boyd @ 2018-12-05 17:00 UTC (permalink / raw) To: Taniya Das, Viresh Kumar Cc: Rafael J. Wysocki, linux-kernel, linux-pm, Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen Quoting Viresh Kumar (2018-12-04 22:16:00) > On 05-12-18, 09:07, Taniya Das wrote: > > 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. 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! > +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); This one. > + 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); > + 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. > if (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(-) diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c index bcf9bb37298a..1e865e216839 100644 --- a/drivers/cpufreq/qcom-cpufreq-hw.c +++ b/drivers/cpufreq/qcom-cpufreq-hw.c @@ -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) ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver 2018-12-05 17:00 ` Stephen Boyd @ 2018-12-06 4:22 ` Viresh Kumar 0 siblings, 0 replies; 21+ messages in thread From: Viresh Kumar @ 2018-12-06 4:22 UTC (permalink / raw) To: Stephen Boyd Cc: Taniya Das, Rafael J. Wysocki, linux-kernel, linux-pm, Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen On 05-12-18, 09:00, Stephen Boyd wrote: > 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(-) > > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > index bcf9bb37298a..1e865e216839 100644 > --- a/drivers/cpufreq/qcom-cpufreq-hw.c > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > @@ -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) LGTM -- viresh ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver 2018-12-05 6:16 ` Viresh Kumar 2018-12-05 17:00 ` Stephen Boyd @ 2018-12-11 13:35 ` Taniya Das 2018-12-12 4:47 ` Viresh Kumar 1 sibling, 1 reply; 21+ messages in thread From: Taniya Das @ 2018-12-11 13:35 UTC (permalink / raw) To: Viresh Kumar Cc: Stephen Boyd, Rafael J. Wysocki, linux-kernel, linux-pm, Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen Hi Viresh, Sorry for the delayed response. Thanks for the patch. On 12/5/2018 11:46 AM, Viresh Kumar wrote: > On 05-12-18, 09:07, Taniya Das wrote: >> 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. > 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. -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. -- ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver 2018-12-11 13:35 ` Taniya Das @ 2018-12-12 4:47 ` Viresh Kumar 2018-12-13 7:48 ` Taniya Das 0 siblings, 1 reply; 21+ messages in thread From: Viresh Kumar @ 2018-12-12 4:47 UTC (permalink / raw) To: Taniya Das Cc: Stephen Boyd, Rafael J. Wysocki, linux-kernel, linux-pm, Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen On 11-12-18, 19:05, Taniya Das wrote: > 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. > 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. > 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 :) -- viresh ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver 2018-12-12 4:47 ` Viresh Kumar @ 2018-12-13 7:48 ` Taniya Das 0 siblings, 0 replies; 21+ messages in thread From: Taniya Das @ 2018-12-13 7:48 UTC (permalink / raw) To: Viresh Kumar Cc: Stephen Boyd, Rafael J. Wysocki, linux-kernel, linux-pm, Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen Hello Viresh, On 12/12/2018 10:17 AM, Viresh Kumar wrote: > On 11-12-18, 19:05, Taniya Das wrote: >> 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. > >> 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. > >> 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 :). -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. -- ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver 2018-12-05 3:37 ` Taniya Das 2018-12-05 6:16 ` Viresh Kumar @ 2018-12-05 8:07 ` Stephen Boyd 1 sibling, 0 replies; 21+ messages in thread From: Stephen Boyd @ 2018-12-05 8:07 UTC (permalink / raw) To: Taniya Das, Viresh Kumar Cc: Rafael J. Wysocki, linux-kernel, linux-pm, Rajendra Nayak, devicetree, robh, skannan, linux-arm-msm, evgreen Quoting Taniya Das (2018-12-04 19:37:00) > 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. diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c index 8dc6b73c2f22..c0f6a07eb3c5 100644 --- a/drivers/cpufreq/qcom-cpufreq-hw.c +++ b/drivers/cpufreq/qcom-cpufreq-hw.c @@ -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; } ^ permalink raw reply related [flat|nested] 21+ messages in thread
end of thread, other threads:[~2018-12-13 7:48 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-02 3:55 [PATCH v11 0/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW Taniya Das 2018-12-02 3:55 ` [PATCH v11 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings Taniya Das 2018-12-03 16:55 ` Stephen Boyd 2018-12-03 23:09 ` Rob Herring 2018-12-03 23:57 ` Stephen Boyd 2018-12-02 3:55 ` [PATCH v11 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver Taniya Das 2018-12-03 16:57 ` Stephen Boyd 2018-12-03 17:46 ` Matthias Kaehlcke 2018-12-04 5:12 ` Viresh Kumar 2018-12-04 9:27 ` Taniya Das 2018-12-04 9:29 ` Viresh Kumar 2018-12-04 22:28 ` Stephen Boyd 2018-12-04 23:05 ` Stephen Boyd 2018-12-05 3:37 ` Taniya Das 2018-12-05 6:16 ` Viresh Kumar 2018-12-05 17:00 ` Stephen Boyd 2018-12-06 4:22 ` Viresh Kumar 2018-12-11 13:35 ` Taniya Das 2018-12-12 4:47 ` Viresh Kumar 2018-12-13 7:48 ` Taniya Das 2018-12-05 8:07 ` Stephen Boyd
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).