Hi Rafael,
I am sending this again, but without the controversial part this time. I have dropped last two patches which were about detecting clock sharing among CPUs. Would fix up that later once other Maintainers get in sync about the bindings.
So, what's left here ? These are mostly updates/reorders which can be applied the patches which I have dropped now. These are all well reviewed and tested by others. Getting them in shouldn't break anything, I believe. Its better people start using the updates we already have, otherwise they will keep on sending fixes they get (The way Pramod Gurav tried it today).
Can we please get them in 3.17 if its still allowed? Or -next otherwise.
V2: https://lkml.org/lkml/2014/7/1/358
Rebased over: 3.17-rc2
git://git.linaro.org/people/viresh.kumar/linux.git cpufreq/cpu0-updates-v3
V2->V3: Dropped few patches, nothing much.
-- Thanks Viresh
Viresh Kumar (10): cpufreq: Add support for per-policy driver data cpufreq: cpu0: Update Module Author cpufreq: cpu0: don't validate clock on clk_put() cpufreq: cpu0: print relevant error when we defer probe cpufreq: cpu0: use dev_{err|warn|dbg} instead of pr_{err|warn|debug} cpufreq: cpu0: Move per-cluster initialization code to ->init() cpufreq: cpu0: try regulators with name "cpu-supply" cpufreq: cpu0: Make allocate_resources() work for any CPU cpufreq: cpu0: rename driver and internals to 'cpufreq_generic' cpufreq: generic: set platform_{driver|device} '.name' to 'cpufreq-generic'
.../{cpufreq-cpu0.txt => cpufreq-generic.txt} | 8 +- arch/arm/mach-imx/imx27-dt.c | 2 +- arch/arm/mach-imx/mach-imx51.c | 2 +- arch/arm/mach-omap2/pm.c | 2 +- arch/arm/mach-shmobile/board-ape6evm-reference.c | 2 +- arch/arm/mach-shmobile/cpufreq.c | 2 +- arch/arm/mach-shmobile/setup-sh73a0.c | 4 +- arch/arm/mach-zynq/common.c | 2 +- drivers/cpufreq/Kconfig | 10 +- drivers/cpufreq/Kconfig.arm | 2 +- drivers/cpufreq/Makefile | 2 +- drivers/cpufreq/cpufreq-cpu0.c | 248 -------------- drivers/cpufreq/cpufreq-generic.c | 363 +++++++++++++++++++++ drivers/cpufreq/exynos4210-cpufreq.c | 2 +- drivers/cpufreq/exynos4x12-cpufreq.c | 2 +- drivers/cpufreq/exynos5250-cpufreq.c | 2 +- drivers/cpufreq/highbank-cpufreq.c | 6 +- drivers/cpufreq/s5pv210-cpufreq.c | 2 +- include/linux/cpufreq.h | 3 + 19 files changed, 392 insertions(+), 274 deletions(-) rename Documentation/devicetree/bindings/cpufreq/{cpufreq-cpu0.txt => cpufreq-generic.txt} (85%) delete mode 100644 drivers/cpufreq/cpufreq-cpu0.c create mode 100644 drivers/cpufreq/cpufreq-generic.c
Drivers supporting multiple clusters or multiple 'struct cpufreq_policy' instances may need to keep per-policy data. If the core doesn't provide support for that, they might do it in the most unoptimized way: 'per-cpu' data.
This patch adds another field in struct cpufreq_policy: 'driver_data'. It isn't accessed by core and is for driver's internal use only.
Tested-by: Stephen Boyd sboyd@codeaurora.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- include/linux/cpufreq.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 7d1955a..138336b 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -112,6 +112,9 @@ struct cpufreq_policy { spinlock_t transition_lock; wait_queue_head_t transition_wait; struct task_struct *transition_task; /* Task which is doing the transition */ + + /* For cpufreq driver's internal use */ + void *driver_data; };
/* Only for ACPI */
Two people are maintaining it now, Viresh and Shawn. Add Viresh's details in MODULE_AUTHOR() and copyright section.
Suggested-by: Shawn Guo shawn.guo@linaro.org Tested-by: Stephen Boyd sboyd@codeaurora.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq-cpu0.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c index 0d2172b..4cfde63 100644 --- a/drivers/cpufreq/cpufreq-cpu0.c +++ b/drivers/cpufreq/cpufreq-cpu0.c @@ -1,6 +1,9 @@ /* * Copyright (C) 2012 Freescale Semiconductor, Inc. * + * Copyright (C) 2014 Linaro. + * Viresh Kumar viresh.kumar@linaro.org + * * The OPP code in function cpu0_set_target() is reused from * drivers/cpufreq/omap-cpufreq.c * @@ -243,6 +246,7 @@ static struct platform_driver cpu0_cpufreq_platdrv = { }; module_platform_driver(cpu0_cpufreq_platdrv);
+MODULE_AUTHOR("Viresh Kumar viresh.kumar@linaro.org"); MODULE_AUTHOR("Shawn Guo shawn.guo@linaro.org"); MODULE_DESCRIPTION("Generic CPU0 cpufreq driver"); MODULE_LICENSE("GPL");
CPU clk is not optional for this driver and probe would fail if it couldn't find a suitable clock.
And so, while calling clk_put() we don't need to validate clocks.
Acked-by: Santosh Shilimkar santosh.shilimkar@ti.com Tested-by: Stephen Boyd sboyd@codeaurora.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq-cpu0.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c index 4cfde63..d2dc921 100644 --- a/drivers/cpufreq/cpufreq-cpu0.c +++ b/drivers/cpufreq/cpufreq-cpu0.c @@ -217,8 +217,7 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev) out_free_table: dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table); out_put_clk: - if (!IS_ERR(cpu_clk)) - clk_put(cpu_clk); + clk_put(cpu_clk); out_put_reg: if (!IS_ERR(cpu_reg)) regulator_put(cpu_reg);
Currently, we defer probe if regulator_get() returned -EPROBE_DEFER, i.e. regulator isn't registered yet. We do a dev_err() in this case. Sending a message to the log on probe defer just duplicates what the driver core is already doing. Convert it to dev_dbg() instead.
We should defer in case of clk_get() as well.
Current code already does it, but it wasn't intentional probably. Its just that we are returning the right error with wrong print message.
Fix print message to convey right error.
Tested-by: Stephen Boyd sboyd@codeaurora.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq-cpu0.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c index d2dc921..eb07e3f 100644 --- a/drivers/cpufreq/cpufreq-cpu0.c +++ b/drivers/cpufreq/cpufreq-cpu0.c @@ -151,7 +151,16 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev) cpu_clk = clk_get(cpu_dev, NULL); if (IS_ERR(cpu_clk)) { ret = PTR_ERR(cpu_clk); - pr_err("failed to get cpu0 clock: %d\n", ret); + + /* + * If cpu's clk node is present, but clock is not yet + * registered, we should try defering probe. + */ + if (ret == -EPROBE_DEFER) + dev_dbg(cpu_dev, "cpu0 clock not ready, retry\n"); + else + dev_err(cpu_dev, "failed to get cpu0 clock: %d\n", ret); + goto out_put_reg; }
We already have cpu_dev and is used at multiple places for printing errors using dev_*(). But some prints are still using pr_*(). Lets make it consistent and replace those pr_*() macros with dev_*() macros.
Acked-by: Santosh Shilimkar santosh.shilimkar@ti.com Tested-by: Stephen Boyd sboyd@codeaurora.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq-cpu0.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c index eb07e3f..741ff22 100644 --- a/drivers/cpufreq/cpufreq-cpu0.c +++ b/drivers/cpufreq/cpufreq-cpu0.c @@ -58,7 +58,8 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index) opp = dev_pm_opp_find_freq_ceil(cpu_dev, &freq_Hz); if (IS_ERR(opp)) { rcu_read_unlock(); - pr_err("failed to find OPP for %ld\n", freq_Hz); + dev_err(cpu_dev, "failed to find OPP for %ld\n", + freq_Hz); return PTR_ERR(opp); } volt = dev_pm_opp_get_voltage(opp); @@ -67,22 +68,23 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index) volt_old = regulator_get_voltage(cpu_reg); }
- pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n", - old_freq / 1000, volt_old ? volt_old / 1000 : -1, - new_freq / 1000, volt ? volt / 1000 : -1); + dev_dbg(cpu_dev, "%u MHz, %ld mV --> %u MHz, %ld mV\n", + old_freq / 1000, volt_old ? volt_old / 1000 : -1, + new_freq / 1000, volt ? volt / 1000 : -1);
/* scaling up? scale voltage before frequency */ if (!IS_ERR(cpu_reg) && new_freq > old_freq) { ret = regulator_set_voltage_tol(cpu_reg, volt, tol); if (ret) { - pr_err("failed to scale voltage up: %d\n", ret); + dev_err(cpu_dev, "failed to scale voltage up: %d\n", + ret); return ret; } }
ret = clk_set_rate(cpu_clk, freq_exact); if (ret) { - pr_err("failed to set clock rate: %d\n", ret); + dev_err(cpu_dev, "failed to set clock rate: %d\n", ret); if (!IS_ERR(cpu_reg)) regulator_set_voltage_tol(cpu_reg, volt_old, tol); return ret; @@ -92,7 +94,8 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index) if (!IS_ERR(cpu_reg) && new_freq < old_freq) { ret = regulator_set_voltage_tol(cpu_reg, volt, tol); if (ret) { - pr_err("failed to scale voltage down: %d\n", ret); + dev_err(cpu_dev, "failed to scale voltage down: %d\n", + ret); clk_set_rate(cpu_clk, old_freq * 1000); } } @@ -129,7 +132,7 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
np = of_node_get(cpu_dev->of_node); if (!np) { - pr_err("failed to find cpu0 node\n"); + dev_err(cpu_dev, "failed to find cpu0 node\n"); return -ENOENT; }
@@ -144,8 +147,8 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev) ret = -EPROBE_DEFER; goto out_put_node; } - pr_warn("failed to get cpu0 regulator: %ld\n", - PTR_ERR(cpu_reg)); + dev_warn(cpu_dev, "failed to get cpu0 regulator: %ld\n", + PTR_ERR(cpu_reg)); }
cpu_clk = clk_get(cpu_dev, NULL); @@ -169,7 +172,7 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table); if (ret) { - pr_err("failed to init cpufreq table: %d\n", ret); + dev_err(cpu_dev, "failed to init cpufreq table: %d\n", ret); goto out_put_clk; }
@@ -205,7 +208,7 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
ret = cpufreq_register_driver(&cpu0_cpufreq_driver); if (ret) { - pr_err("failed register driver: %d\n", ret); + dev_err(cpu_dev, "failed to register driver: %d\n", ret); goto out_free_table; }
@@ -216,8 +219,9 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev) if (of_find_property(np, "#cooling-cells", NULL)) { cdev = of_cpufreq_cooling_register(np, cpu_present_mask); if (IS_ERR(cdev)) - pr_err("running cpufreq without cooling device: %ld\n", - PTR_ERR(cdev)); + dev_err(cpu_dev, + "running cpufreq without cooling device: %ld\n", + PTR_ERR(cdev)); }
of_node_put(np);
Currently this driver only support platforms on which all CPUs share clock & voltage lines and there is requirement to support platforms which have separate clock & voltage lines for CPUs, like Qualcomm's Krait and ARM's big LITTLE.
Each group of CPUs sharing clock/voltage lines are represented by 'struct cpufreq_policy' in cpufreq framework. And core calls ->init() once for each policy.
Currently we do all initialization/allocation from probe() which wouldn't work for above scenario. To make it work for these platforms, the first step is to move all initialization/allocation to ->init() and add ->exit() to do the reverse of it.
Also, remove all global variables and allocate space for them at runtime.
This patch creates 'struct private_data' for keeping all such information and a pointer to that would be stored in policy->driver_data.
The changed probe() routine now tries to see if regulator/clocks are available or we need to defer probe. In case they are available, it registers cpufreq driver. Otherwise, returns with -EPROBE_DEFER.
We still *don't* support platforms with separate clock/voltage lines for CPUs. This would be done in a separate patch later.
Tested-by: Stephen Boyd sboyd@codeaurora.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq-cpu0.c | 189 +++++++++++++++++++++++++++++------------ 1 file changed, 136 insertions(+), 53 deletions(-)
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c index 741ff22..03e352b 100644 --- a/drivers/cpufreq/cpufreq-cpu0.c +++ b/drivers/cpufreq/cpufreq-cpu0.c @@ -28,18 +28,21 @@ #include <linux/slab.h> #include <linux/thermal.h>
-static unsigned int transition_latency; -static unsigned int voltage_tolerance; /* in percentage */ - -static struct device *cpu_dev; -static struct clk *cpu_clk; -static struct regulator *cpu_reg; -static struct cpufreq_frequency_table *freq_table; -static struct thermal_cooling_device *cdev; +struct private_data { + struct device *cpu_dev; + struct regulator *cpu_reg; + struct thermal_cooling_device *cdev; + unsigned int voltage_tolerance; /* in percentage */ +};
static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index) { struct dev_pm_opp *opp; + struct cpufreq_frequency_table *freq_table = policy->freq_table; + struct clk *cpu_clk = policy->clk; + struct private_data *priv = policy->driver_data; + struct device *cpu_dev = priv->cpu_dev; + struct regulator *cpu_reg = priv->cpu_reg; unsigned long volt = 0, volt_old = 0, tol = 0; unsigned int old_freq, new_freq; long freq_Hz, freq_exact; @@ -64,7 +67,7 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index) } volt = dev_pm_opp_get_voltage(opp); rcu_read_unlock(); - tol = volt * voltage_tolerance / 100; + tol = volt * priv->voltage_tolerance / 100; volt_old = regulator_get_voltage(cpu_reg); }
@@ -103,26 +106,13 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index) return ret; }
-static int cpu0_cpufreq_init(struct cpufreq_policy *policy) -{ - policy->clk = cpu_clk; - return cpufreq_generic_init(policy, freq_table, transition_latency); -} - -static struct cpufreq_driver cpu0_cpufreq_driver = { - .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK, - .verify = cpufreq_generic_frequency_table_verify, - .target_index = cpu0_set_target, - .get = cpufreq_generic_get, - .init = cpu0_cpufreq_init, - .name = "generic_cpu0", - .attr = cpufreq_generic_attr, -}; - -static int cpu0_cpufreq_probe(struct platform_device *pdev) +static int allocate_resources(struct device **cdev, + struct regulator **creg, struct clk **cclk) { - struct device_node *np; - int ret; + struct device *cpu_dev; + struct regulator *cpu_reg; + struct clk *cpu_clk; + int ret = 0;
cpu_dev = get_cpu_device(0); if (!cpu_dev) { @@ -130,12 +120,6 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev) return -ENODEV; }
- np = of_node_get(cpu_dev->of_node); - if (!np) { - dev_err(cpu_dev, "failed to find cpu0 node\n"); - return -ENOENT; - } - cpu_reg = regulator_get_optional(cpu_dev, "cpu0"); if (IS_ERR(cpu_reg)) { /* @@ -144,8 +128,7 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev) */ if (PTR_ERR(cpu_reg) == -EPROBE_DEFER) { dev_dbg(cpu_dev, "cpu0 regulator not ready, retry\n"); - ret = -EPROBE_DEFER; - goto out_put_node; + return -EPROBE_DEFER; } dev_warn(cpu_dev, "failed to get cpu0 regulator: %ld\n", PTR_ERR(cpu_reg)); @@ -153,6 +136,10 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
cpu_clk = clk_get(cpu_dev, NULL); if (IS_ERR(cpu_clk)) { + /* put regulator */ + if (!IS_ERR(cpu_reg)) + regulator_put(cpu_reg); + ret = PTR_ERR(cpu_clk);
/* @@ -163,8 +150,39 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev) dev_dbg(cpu_dev, "cpu0 clock not ready, retry\n"); else dev_err(cpu_dev, "failed to get cpu0 clock: %d\n", ret); + } else { + *cdev = cpu_dev; + *creg = cpu_reg; + *cclk = cpu_clk; + } + + return ret; +} + +static int cpu0_cpufreq_init(struct cpufreq_policy *policy) +{ + struct cpufreq_frequency_table *freq_table; + struct thermal_cooling_device *cdev; + struct device_node *np; + struct private_data *priv; + struct device *cpu_dev; + struct regulator *cpu_reg; + struct clk *cpu_clk; + unsigned int transition_latency; + int ret; + + /* We only support cpu0 currently */ + ret = allocate_resources(&cpu_dev, &cpu_reg, &cpu_clk); + if (ret) { + pr_err("%s: Failed to allocate resources\n: %d", __func__, ret); + return ret; + }
- goto out_put_reg; + np = of_node_get(cpu_dev->of_node); + if (!np) { + dev_err(cpu_dev, "failed to find cpu%d node\n", policy->cpu); + ret = -ENOENT; + goto out_put_reg_clk; }
/* OPPs might be populated at runtime, don't check for error here */ @@ -173,10 +191,16 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev) ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table); if (ret) { dev_err(cpu_dev, "failed to init cpufreq table: %d\n", ret); - goto out_put_clk; + goto out_put_node; + } + + priv = kzalloc(sizeof(*priv), GFP_KERNEL); + if (!priv) { + ret = -ENOMEM; + goto out_free_table; }
- of_property_read_u32(np, "voltage-tolerance", &voltage_tolerance); + of_property_read_u32(np, "voltage-tolerance", &priv->voltage_tolerance);
if (of_property_read_u32(np, "clock-latency", &transition_latency)) transition_latency = CPUFREQ_ETERNAL; @@ -206,12 +230,6 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev) transition_latency += ret * 1000; }
- ret = cpufreq_register_driver(&cpu0_cpufreq_driver); - if (ret) { - dev_err(cpu_dev, "failed to register driver: %d\n", ret); - goto out_free_table; - } - /* * For now, just loading the cooling device; * thermal DT code takes care of matching them. @@ -222,29 +240,94 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev) dev_err(cpu_dev, "running cpufreq without cooling device: %ld\n", PTR_ERR(cdev)); + else + priv->cdev = cdev; } - of_node_put(np); + + priv->cpu_dev = cpu_dev; + priv->cpu_reg = cpu_reg; + policy->driver_data = priv; + + policy->clk = cpu_clk; + ret = cpufreq_generic_init(policy, freq_table, transition_latency); + if (ret) + goto out_cooling_unregister; + return 0;
+out_cooling_unregister: + cpufreq_cooling_unregister(priv->cdev); + kfree(priv); out_free_table: dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table); -out_put_clk: +out_put_node: + of_node_put(np); +out_put_reg_clk: clk_put(cpu_clk); -out_put_reg: if (!IS_ERR(cpu_reg)) regulator_put(cpu_reg); -out_put_node: - of_node_put(np); + + return ret; +} + +static int cpu0_cpufreq_exit(struct cpufreq_policy *policy) +{ + struct private_data *priv = policy->driver_data; + + cpufreq_cooling_unregister(priv->cdev); + dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table); + clk_put(policy->clk); + if (!IS_ERR(priv->cpu_reg)) + regulator_put(priv->cpu_reg); + kfree(priv); + + return 0; +} + +static struct cpufreq_driver cpu0_cpufreq_driver = { + .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK, + .verify = cpufreq_generic_frequency_table_verify, + .target_index = cpu0_set_target, + .get = cpufreq_generic_get, + .init = cpu0_cpufreq_init, + .exit = cpu0_cpufreq_exit, + .name = "generic_cpu0", + .attr = cpufreq_generic_attr, +}; + +static int cpu0_cpufreq_probe(struct platform_device *pdev) +{ + struct device *cpu_dev; + struct regulator *cpu_reg; + struct clk *cpu_clk; + int ret; + + /* + * All per-cluster (CPUs sharing clock/voltages) initialization is done + * from ->init(). In probe(), we just need to make sure that clk and + * regulators are available. Else defer probe and retry. + * + * FIXME: Is checking this only for CPU0 sufficient ? + */ + ret = allocate_resources(&cpu_dev, &cpu_reg, &cpu_clk); + if (ret) + return ret; + + clk_put(cpu_clk); + if (!IS_ERR(cpu_reg)) + regulator_put(cpu_reg); + + ret = cpufreq_register_driver(&cpu0_cpufreq_driver); + if (ret) + dev_err(cpu_dev, "failed register driver: %d\n", ret); + return ret; }
static int cpu0_cpufreq_remove(struct platform_device *pdev) { - cpufreq_cooling_unregister(cdev); cpufreq_unregister_driver(&cpu0_cpufreq_driver); - dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table); - return 0; }
Currently, we expect regulator name to be "cpu0", but as we are going to support multiple cpu-blocks (all CPUs in a block share clock/voltage) later, we need to pass some generic string instead of that.
For backwards compatibility try for "cpu0" first and if it fails, then try for "cpu".
Suggested-by: Stephen Boyd sboyd@codeaurora.org Tested-by: Stephen Boyd sboyd@codeaurora.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq-cpu0.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c index 03e352b..de38952 100644 --- a/drivers/cpufreq/cpufreq-cpu0.c +++ b/drivers/cpufreq/cpufreq-cpu0.c @@ -113,6 +113,7 @@ static int allocate_resources(struct device **cdev, struct regulator *cpu_reg; struct clk *cpu_clk; int ret = 0; + char *reg_cpu0 = "cpu0", *reg_cpu = "cpu", *reg;
cpu_dev = get_cpu_device(0); if (!cpu_dev) { @@ -120,7 +121,11 @@ static int allocate_resources(struct device **cdev, return -ENODEV; }
- cpu_reg = regulator_get_optional(cpu_dev, "cpu0"); + /* Try "cpu0" for older DTs */ + reg = reg_cpu0; + +try_again: + cpu_reg = regulator_get_optional(cpu_dev, reg); if (IS_ERR(cpu_reg)) { /* * If cpu0 regulator supply node is present, but regulator is @@ -130,6 +135,13 @@ static int allocate_resources(struct device **cdev, dev_dbg(cpu_dev, "cpu0 regulator not ready, retry\n"); return -EPROBE_DEFER; } + + /* Try with "cpu-supply" */ + if (reg == reg_cpu0) { + reg = reg_cpu; + goto try_again; + } + dev_warn(cpu_dev, "failed to get cpu0 regulator: %ld\n", PTR_ERR(cpu_reg)); }
Currently allocate_resources() supports only CPU0 and it would need to allocate resources for any CPU going forward.
Add another argument to it, i.e. cpu, and update code accordingly.
Tested-by: Stephen Boyd sboyd@codeaurora.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq-cpu0.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c index de38952..a5f8c5f 100644 --- a/drivers/cpufreq/cpufreq-cpu0.c +++ b/drivers/cpufreq/cpufreq-cpu0.c @@ -106,7 +106,7 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index) return ret; }
-static int allocate_resources(struct device **cdev, +static int allocate_resources(int cpu, struct device **cdev, struct regulator **creg, struct clk **cclk) { struct device *cpu_dev; @@ -115,24 +115,28 @@ static int allocate_resources(struct device **cdev, int ret = 0; char *reg_cpu0 = "cpu0", *reg_cpu = "cpu", *reg;
- cpu_dev = get_cpu_device(0); + cpu_dev = get_cpu_device(cpu); if (!cpu_dev) { - pr_err("failed to get cpu0 device\n"); + pr_err("failed to get cpu%d device\n", cpu); return -ENODEV; }
/* Try "cpu0" for older DTs */ - reg = reg_cpu0; + if (!cpu) + reg = reg_cpu0; + else + reg = reg_cpu;
try_again: cpu_reg = regulator_get_optional(cpu_dev, reg); if (IS_ERR(cpu_reg)) { /* - * If cpu0 regulator supply node is present, but regulator is + * If cpu's regulator supply node is present, but regulator is * not yet registered, we should try defering probe. */ if (PTR_ERR(cpu_reg) == -EPROBE_DEFER) { - dev_dbg(cpu_dev, "cpu0 regulator not ready, retry\n"); + dev_dbg(cpu_dev, "cpu%d regulator not ready, retry\n", + cpu); return -EPROBE_DEFER; }
@@ -142,8 +146,8 @@ static int allocate_resources(struct device **cdev, goto try_again; }
- dev_warn(cpu_dev, "failed to get cpu0 regulator: %ld\n", - PTR_ERR(cpu_reg)); + dev_warn(cpu_dev, "failed to get cpu%d regulator: %ld\n", + cpu, PTR_ERR(cpu_reg)); }
cpu_clk = clk_get(cpu_dev, NULL); @@ -159,9 +163,10 @@ static int allocate_resources(struct device **cdev, * registered, we should try defering probe. */ if (ret == -EPROBE_DEFER) - dev_dbg(cpu_dev, "cpu0 clock not ready, retry\n"); + dev_dbg(cpu_dev, "cpu%d clock not ready, retry\n", cpu); else - dev_err(cpu_dev, "failed to get cpu0 clock: %d\n", ret); + dev_err(cpu_dev, "failed to get cpu%d clock: %d\n", ret, + cpu); } else { *cdev = cpu_dev; *creg = cpu_reg; @@ -183,8 +188,7 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy) unsigned int transition_latency; int ret;
- /* We only support cpu0 currently */ - ret = allocate_resources(&cpu_dev, &cpu_reg, &cpu_clk); + ret = allocate_resources(policy->cpu, &cpu_dev, &cpu_reg, &cpu_clk); if (ret) { pr_err("%s: Failed to allocate resources\n: %d", __func__, ret); return ret; @@ -322,7 +326,7 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev) * * FIXME: Is checking this only for CPU0 sufficient ? */ - ret = allocate_resources(&cpu_dev, &cpu_reg, &cpu_clk); + ret = allocate_resources(0, &cpu_dev, &cpu_reg, &cpu_clk); if (ret) return ret;
The naming convention of this driver was always under the scanner, people complained that it should have a more generic name than cpu0, as it manages all CPUs that are sharing clock lines.
Also, in future it will be modified to support any number of clusters with separate clock/voltage lines.
Lets rename it to 'cpufreq_generic' from 'cpufreq_cpu0'.
'.name' field of platform driver/devices isn't renamed yet, would be done separately.
Tested-by: Stephen Boyd sboyd@codeaurora.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- .../{cpufreq-cpu0.txt => cpufreq-generic.txt} | 8 ++--- drivers/cpufreq/Kconfig | 10 +++--- drivers/cpufreq/Kconfig.arm | 2 +- drivers/cpufreq/Makefile | 2 +- .../cpufreq/{cpufreq-cpu0.c => cpufreq-generic.c} | 36 +++++++++++----------- 5 files changed, 29 insertions(+), 29 deletions(-) rename Documentation/devicetree/bindings/cpufreq/{cpufreq-cpu0.txt => cpufreq-generic.txt} (85%) rename drivers/cpufreq/{cpufreq-cpu0.c => cpufreq-generic.c} (90%)
diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-generic.txt similarity index 85% rename from Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt rename to Documentation/devicetree/bindings/cpufreq/cpufreq-generic.txt index 366690c..9278f79 100644 --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-generic.txt @@ -1,8 +1,8 @@ -Generic CPU0 cpufreq driver +Generic cpufreq driver
-It is a generic cpufreq driver for CPU0 frequency management. It -supports both uniprocessor (UP) and symmetric multiprocessor (SMP) -systems which share clock and voltage across all CPUs. +It is a generic cpufreq driver for frequency management. It supports both +uniprocessor (UP) and symmetric multiprocessor (SMP) systems which share clock +and voltage across all CPUs.
Both required and optional properties listed below must be defined under node /cpus/cpu@0. diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig index ffe350f..22b42d5 100644 --- a/drivers/cpufreq/Kconfig +++ b/drivers/cpufreq/Kconfig @@ -183,16 +183,16 @@ config CPU_FREQ_GOV_CONSERVATIVE
If in doubt, say N.
-config GENERIC_CPUFREQ_CPU0 - tristate "Generic CPU0 cpufreq driver" +config CPUFREQ_GENERIC + tristate "Generic cpufreq driver" depends on HAVE_CLK && OF - # if CPU_THERMAL is on and THERMAL=m, CPU0 cannot be =y: + # if CPU_THERMAL is on and THERMAL=m, GENERIC cannot be =y: depends on !CPU_THERMAL || THERMAL select PM_OPP help - This adds a generic cpufreq driver for CPU0 frequency management. + This adds a generic cpufreq driver for frequency management. It supports both uniprocessor (UP) and symmetric multiprocessor (SMP) - systems which share clock and voltage across all CPUs. + systems which may or maynot share clock and voltage across all CPUs.
If in doubt, say N.
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 7364a53..b0b0ca1 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -92,7 +92,7 @@ config ARM_EXYNOS_CPU_FREQ_BOOST_SW
config ARM_HIGHBANK_CPUFREQ tristate "Calxeda Highbank-based" - depends on ARCH_HIGHBANK && GENERIC_CPUFREQ_CPU0 && REGULATOR + depends on ARCH_HIGHBANK && CPUFREQ_GENERIC && REGULATOR default m help This adds the CPUFreq driver for Calxeda Highbank SoC diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index db6d9a2..e8efe9b 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -13,7 +13,7 @@ obj-$(CONFIG_CPU_FREQ_GOV_ONDEMAND) += cpufreq_ondemand.o obj-$(CONFIG_CPU_FREQ_GOV_CONSERVATIVE) += cpufreq_conservative.o obj-$(CONFIG_CPU_FREQ_GOV_COMMON) += cpufreq_governor.o
-obj-$(CONFIG_GENERIC_CPUFREQ_CPU0) += cpufreq-cpu0.o +obj-$(CONFIG_CPUFREQ_GENERIC) += cpufreq-generic.o
################################################################################## # x86 drivers. diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-generic.c similarity index 90% rename from drivers/cpufreq/cpufreq-cpu0.c rename to drivers/cpufreq/cpufreq-generic.c index a5f8c5f..9b85055 100644 --- a/drivers/cpufreq/cpufreq-cpu0.c +++ b/drivers/cpufreq/cpufreq-generic.c @@ -4,7 +4,7 @@ * Copyright (C) 2014 Linaro. * Viresh Kumar viresh.kumar@linaro.org * - * The OPP code in function cpu0_set_target() is reused from + * The OPP code in function set_target() is reused from * drivers/cpufreq/omap-cpufreq.c * * This program is free software; you can redistribute it and/or modify @@ -35,7 +35,7 @@ struct private_data { unsigned int voltage_tolerance; /* in percentage */ };
-static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index) +static int set_target(struct cpufreq_policy *policy, unsigned int index) { struct dev_pm_opp *opp; struct cpufreq_frequency_table *freq_table = policy->freq_table; @@ -176,7 +176,7 @@ static int allocate_resources(int cpu, struct device **cdev, return ret; }
-static int cpu0_cpufreq_init(struct cpufreq_policy *policy) +static int cpufreq_init(struct cpufreq_policy *policy) { struct cpufreq_frequency_table *freq_table; struct thermal_cooling_device *cdev; @@ -287,7 +287,7 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy) return ret; }
-static int cpu0_cpufreq_exit(struct cpufreq_policy *policy) +static int cpufreq_exit(struct cpufreq_policy *policy) { struct private_data *priv = policy->driver_data;
@@ -301,18 +301,18 @@ static int cpu0_cpufreq_exit(struct cpufreq_policy *policy) return 0; }
-static struct cpufreq_driver cpu0_cpufreq_driver = { +static struct cpufreq_driver generic_cpufreq_driver = { .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK, .verify = cpufreq_generic_frequency_table_verify, - .target_index = cpu0_set_target, + .target_index = set_target, .get = cpufreq_generic_get, - .init = cpu0_cpufreq_init, - .exit = cpu0_cpufreq_exit, - .name = "generic_cpu0", + .init = cpufreq_init, + .exit = cpufreq_exit, + .name = "cpufreq-generic", .attr = cpufreq_generic_attr, };
-static int cpu0_cpufreq_probe(struct platform_device *pdev) +static int generic_cpufreq_probe(struct platform_device *pdev) { struct device *cpu_dev; struct regulator *cpu_reg; @@ -334,30 +334,30 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev) if (!IS_ERR(cpu_reg)) regulator_put(cpu_reg);
- ret = cpufreq_register_driver(&cpu0_cpufreq_driver); + ret = cpufreq_register_driver(&generic_cpufreq_driver); if (ret) dev_err(cpu_dev, "failed register driver: %d\n", ret);
return ret; }
-static int cpu0_cpufreq_remove(struct platform_device *pdev) +static int generic_cpufreq_remove(struct platform_device *pdev) { - cpufreq_unregister_driver(&cpu0_cpufreq_driver); + cpufreq_unregister_driver(&generic_cpufreq_driver); return 0; }
-static struct platform_driver cpu0_cpufreq_platdrv = { +static struct platform_driver generic_cpufreq_platdrv = { .driver = { .name = "cpufreq-cpu0", .owner = THIS_MODULE, }, - .probe = cpu0_cpufreq_probe, - .remove = cpu0_cpufreq_remove, + .probe = generic_cpufreq_probe, + .remove = generic_cpufreq_remove, }; -module_platform_driver(cpu0_cpufreq_platdrv); +module_platform_driver(generic_cpufreq_platdrv);
MODULE_AUTHOR("Viresh Kumar viresh.kumar@linaro.org"); MODULE_AUTHOR("Shawn Guo shawn.guo@linaro.org"); -MODULE_DESCRIPTION("Generic CPU0 cpufreq driver"); +MODULE_DESCRIPTION("Generic cpufreq driver"); MODULE_LICENSE("GPL");
On Thursday, August 28, 2014 11:22:31 AM Viresh Kumar wrote:
The naming convention of this driver was always under the scanner, people complained that it should have a more generic name than cpu0, as it manages all CPUs that are sharing clock lines.
Also, in future it will be modified to support any number of clusters with separate clock/voltage lines.
Lets rename it to 'cpufreq_generic' from 'cpufreq_cpu0'.
I'm not a big fan of changes like this to be honest, because they kind of hide history. For example, if you do a "git blame" on the new file, it will show your commit and nothing before it.
'.name' field of platform driver/devices isn't renamed yet, would be done separately.
Tested-by: Stephen Boyd sboyd@codeaurora.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
.../{cpufreq-cpu0.txt => cpufreq-generic.txt} | 8 ++--- drivers/cpufreq/Kconfig | 10 +++--- drivers/cpufreq/Kconfig.arm | 2 +- drivers/cpufreq/Makefile | 2 +- .../cpufreq/{cpufreq-cpu0.c => cpufreq-generic.c} | 36 +++++++++++----------- 5 files changed, 29 insertions(+), 29 deletions(-) rename Documentation/devicetree/bindings/cpufreq/{cpufreq-cpu0.txt => cpufreq-generic.txt} (85%) rename drivers/cpufreq/{cpufreq-cpu0.c => cpufreq-generic.c} (90%)
diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-generic.txt similarity index 85% rename from Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt rename to Documentation/devicetree/bindings/cpufreq/cpufreq-generic.txt index 366690c..9278f79 100644 --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-generic.txt @@ -1,8 +1,8 @@ -Generic CPU0 cpufreq driver +Generic cpufreq driver -It is a generic cpufreq driver for CPU0 frequency management. It -supports both uniprocessor (UP) and symmetric multiprocessor (SMP) -systems which share clock and voltage across all CPUs. +It is a generic cpufreq driver for frequency management. It supports both +uniprocessor (UP) and symmetric multiprocessor (SMP) systems which share clock +and voltage across all CPUs. Both required and optional properties listed below must be defined under node /cpus/cpu@0. diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig index ffe350f..22b42d5 100644 --- a/drivers/cpufreq/Kconfig +++ b/drivers/cpufreq/Kconfig @@ -183,16 +183,16 @@ config CPU_FREQ_GOV_CONSERVATIVE If in doubt, say N. -config GENERIC_CPUFREQ_CPU0
- tristate "Generic CPU0 cpufreq driver"
+config CPUFREQ_GENERIC
- tristate "Generic cpufreq driver" depends on HAVE_CLK && OF
- # if CPU_THERMAL is on and THERMAL=m, CPU0 cannot be =y:
- # if CPU_THERMAL is on and THERMAL=m, GENERIC cannot be =y: depends on !CPU_THERMAL || THERMAL select PM_OPP help
This adds a generic cpufreq driver for CPU0 frequency management.
This adds a generic cpufreq driver for frequency management.
systems which share clock and voltage across all CPUs.
systems which may or maynot share clock and voltage across all CPUs.If in doubt, say N. diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 7364a53..b0b0ca1 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -92,7 +92,7 @@ config ARM_EXYNOS_CPU_FREQ_BOOST_SW config ARM_HIGHBANK_CPUFREQ tristate "Calxeda Highbank-based"
- depends on ARCH_HIGHBANK && GENERIC_CPUFREQ_CPU0 && REGULATOR
- depends on ARCH_HIGHBANK && CPUFREQ_GENERIC && REGULATOR default m help This adds the CPUFreq driver for Calxeda Highbank SoC
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index db6d9a2..e8efe9b 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -13,7 +13,7 @@ obj-$(CONFIG_CPU_FREQ_GOV_ONDEMAND) += cpufreq_ondemand.o obj-$(CONFIG_CPU_FREQ_GOV_CONSERVATIVE) += cpufreq_conservative.o obj-$(CONFIG_CPU_FREQ_GOV_COMMON) += cpufreq_governor.o -obj-$(CONFIG_GENERIC_CPUFREQ_CPU0) += cpufreq-cpu0.o +obj-$(CONFIG_CPUFREQ_GENERIC) += cpufreq-generic.o ################################################################################## # x86 drivers. diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-generic.c similarity index 90% rename from drivers/cpufreq/cpufreq-cpu0.c rename to drivers/cpufreq/cpufreq-generic.c index a5f8c5f..9b85055 100644 --- a/drivers/cpufreq/cpufreq-cpu0.c +++ b/drivers/cpufreq/cpufreq-generic.c @@ -4,7 +4,7 @@
- Copyright (C) 2014 Linaro.
- Viresh Kumar viresh.kumar@linaro.org
- The OPP code in function cpu0_set_target() is reused from
- The OPP code in function set_target() is reused from
- drivers/cpufreq/omap-cpufreq.c
- This program is free software; you can redistribute it and/or modify
@@ -35,7 +35,7 @@ struct private_data { unsigned int voltage_tolerance; /* in percentage */ }; -static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index) +static int set_target(struct cpufreq_policy *policy, unsigned int index) { struct dev_pm_opp *opp; struct cpufreq_frequency_table *freq_table = policy->freq_table; @@ -176,7 +176,7 @@ static int allocate_resources(int cpu, struct device **cdev, return ret; } -static int cpu0_cpufreq_init(struct cpufreq_policy *policy) +static int cpufreq_init(struct cpufreq_policy *policy) { struct cpufreq_frequency_table *freq_table; struct thermal_cooling_device *cdev; @@ -287,7 +287,7 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy) return ret; } -static int cpu0_cpufreq_exit(struct cpufreq_policy *policy) +static int cpufreq_exit(struct cpufreq_policy *policy) { struct private_data *priv = policy->driver_data; @@ -301,18 +301,18 @@ static int cpu0_cpufreq_exit(struct cpufreq_policy *policy) return 0; } -static struct cpufreq_driver cpu0_cpufreq_driver = { +static struct cpufreq_driver generic_cpufreq_driver = { .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK, .verify = cpufreq_generic_frequency_table_verify,
- .target_index = cpu0_set_target,
- .target_index = set_target, .get = cpufreq_generic_get,
- .init = cpu0_cpufreq_init,
- .exit = cpu0_cpufreq_exit,
- .name = "generic_cpu0",
- .init = cpufreq_init,
- .exit = cpufreq_exit,
- .name = "cpufreq-generic", .attr = cpufreq_generic_attr,
}; -static int cpu0_cpufreq_probe(struct platform_device *pdev) +static int generic_cpufreq_probe(struct platform_device *pdev) { struct device *cpu_dev; struct regulator *cpu_reg; @@ -334,30 +334,30 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev) if (!IS_ERR(cpu_reg)) regulator_put(cpu_reg);
- ret = cpufreq_register_driver(&cpu0_cpufreq_driver);
- ret = cpufreq_register_driver(&generic_cpufreq_driver); if (ret) dev_err(cpu_dev, "failed register driver: %d\n", ret);
return ret; } -static int cpu0_cpufreq_remove(struct platform_device *pdev) +static int generic_cpufreq_remove(struct platform_device *pdev) {
- cpufreq_unregister_driver(&cpu0_cpufreq_driver);
- cpufreq_unregister_driver(&generic_cpufreq_driver); return 0;
} -static struct platform_driver cpu0_cpufreq_platdrv = { +static struct platform_driver generic_cpufreq_platdrv = { .driver = { .name = "cpufreq-cpu0", .owner = THIS_MODULE, },
- .probe = cpu0_cpufreq_probe,
- .remove = cpu0_cpufreq_remove,
- .probe = generic_cpufreq_probe,
- .remove = generic_cpufreq_remove,
}; -module_platform_driver(cpu0_cpufreq_platdrv); +module_platform_driver(generic_cpufreq_platdrv); MODULE_AUTHOR("Viresh Kumar viresh.kumar@linaro.org"); MODULE_AUTHOR("Shawn Guo shawn.guo@linaro.org"); -MODULE_DESCRIPTION("Generic CPU0 cpufreq driver"); +MODULE_DESCRIPTION("Generic cpufreq driver"); MODULE_LICENSE("GPL");
On 09/08/14 16:34, Rafael J. Wysocki wrote:
On Thursday, August 28, 2014 11:22:31 AM Viresh Kumar wrote:
The naming convention of this driver was always under the scanner, people complained that it should have a more generic name than cpu0, as it manages all CPUs that are sharing clock lines.
Also, in future it will be modified to support any number of clusters with separate clock/voltage lines.
Lets rename it to 'cpufreq_generic' from 'cpufreq_cpu0'.
I'm not a big fan of changes like this to be honest, because they kind of hide history. For example, if you do a "git blame" on the new file, it will show your commit and nothing before it.
Sounds like a tooling problem. But git blame doesn't have that behavior so there isn't actually any problem?
$ git blame -- drivers/cpufreq/cpufreq-generic.c | head -10 95ceafd46359d drivers/cpufreq/cpufreq-cpu0.c (Shawn Guo 2012-09-06 07:09:11 +0000 1) /* 95ceafd46359d drivers/cpufreq/cpufreq-cpu0.c (Shawn Guo 2012-09-06 07:09:11 +0000 2) * Copyright (C) 2012 Freescale Semiconductor, Inc. 95ceafd46359d drivers/cpufreq/cpufreq-cpu0.c (Shawn Guo 2012-09-06 07:09:11 +0000 3) * da1a765bf681c drivers/cpufreq/cpufreq-cpu0.c (Viresh Kumar 2014-06-30 10:15:22 +0530 4) * Copyright (C) 2014 Linaro. da1a765bf681c drivers/cpufreq/cpufreq-cpu0.c (Viresh Kumar 2014-06-30 10:15:22 +0530 5) * Viresh Kumar viresh.kumar@linaro.org da1a765bf681c drivers/cpufreq/cpufreq-cpu0.c (Viresh Kumar 2014-06-30 10:15:22 +0530 6) * ac4f646b8a5f3 drivers/cpufreq/cpufreq-generic.c (Viresh Kumar 2014-06-27 10:58:51 +0530 7) * The OPP code in function set_target() is reused from 95ceafd46359d drivers/cpufreq/cpufreq-cpu0.c (Shawn Guo 2012-09-06 07:09:11 +0000 8) * drivers/cpufreq/omap-cpufreq.c 95ceafd46359d drivers/cpufreq/cpufreq-cpu0.c (Shawn Guo 2012-09-06 07:09:11 +0000 9) * 95ceafd46359d drivers/cpufreq/cpufreq-cpu0.c (Shawn Guo 2012-09-06 07:09:11 +0000 10) * This program is free software; you can redistribute it and/or modify
On Monday, September 08, 2014 04:18:11 PM Stephen Boyd wrote:
On 09/08/14 16:34, Rafael J. Wysocki wrote:
On Thursday, August 28, 2014 11:22:31 AM Viresh Kumar wrote:
The naming convention of this driver was always under the scanner, people complained that it should have a more generic name than cpu0, as it manages all CPUs that are sharing clock lines.
Also, in future it will be modified to support any number of clusters with separate clock/voltage lines.
Lets rename it to 'cpufreq_generic' from 'cpufreq_cpu0'.
I'm not a big fan of changes like this to be honest, because they kind of hide history. For example, if you do a "git blame" on the new file, it will show your commit and nothing before it.
Sounds like a tooling problem. But git blame doesn't have that behavior so there isn't actually any problem?
Ah, OK
It used to some time ago or maybe it depends on how you carry out the rename.
$ git blame -- drivers/cpufreq/cpufreq-generic.c | head -10 95ceafd46359d drivers/cpufreq/cpufreq-cpu0.c (Shawn Guo 2012-09-06 07:09:11 +0000 1) /* 95ceafd46359d drivers/cpufreq/cpufreq-cpu0.c (Shawn Guo 2012-09-06 07:09:11 +0000 2) * Copyright (C) 2012 Freescale Semiconductor, Inc. 95ceafd46359d drivers/cpufreq/cpufreq-cpu0.c (Shawn Guo 2012-09-06 07:09:11 +0000 3) * da1a765bf681c drivers/cpufreq/cpufreq-cpu0.c (Viresh Kumar 2014-06-30 10:15:22 +0530 4) * Copyright (C) 2014 Linaro. da1a765bf681c drivers/cpufreq/cpufreq-cpu0.c (Viresh Kumar 2014-06-30 10:15:22 +0530 5) * Viresh Kumar viresh.kumar@linaro.org da1a765bf681c drivers/cpufreq/cpufreq-cpu0.c (Viresh Kumar 2014-06-30 10:15:22 +0530 6) * ac4f646b8a5f3 drivers/cpufreq/cpufreq-generic.c (Viresh Kumar 2014-06-27 10:58:51 +0530 7) * The OPP code in function set_target() is reused from 95ceafd46359d drivers/cpufreq/cpufreq-cpu0.c (Shawn Guo 2012-09-06 07:09:11 +0000 8) * drivers/cpufreq/omap-cpufreq.c 95ceafd46359d drivers/cpufreq/cpufreq-cpu0.c (Shawn Guo 2012-09-06 07:09:11 +0000 9) * 95ceafd46359d drivers/cpufreq/cpufreq-cpu0.c (Shawn Guo 2012-09-06 07:09:11 +0000 10) * This program is free software; you can redistribute it and/or modify
On Tuesday, September 09, 2014 01:39:32 AM Rafael J. Wysocki wrote:
On Monday, September 08, 2014 04:18:11 PM Stephen Boyd wrote:
On 09/08/14 16:34, Rafael J. Wysocki wrote:
On Thursday, August 28, 2014 11:22:31 AM Viresh Kumar wrote:
The naming convention of this driver was always under the scanner, people complained that it should have a more generic name than cpu0, as it manages all CPUs that are sharing clock lines.
Also, in future it will be modified to support any number of clusters with separate clock/voltage lines.
Lets rename it to 'cpufreq_generic' from 'cpufreq_cpu0'.
I'm not a big fan of changes like this to be honest, because they kind of hide history. For example, if you do a "git blame" on the new file, it will show your commit and nothing before it.
Sounds like a tooling problem. But git blame doesn't have that behavior so there isn't actually any problem?
Ah, OK
It used to some time ago or maybe it depends on how you carry out the rename.
So more bikeshedding. :-)
Isn't "generic" somewhat too broad? Surely it doesn't cover x86. And does it actually cover anything without clocks/regulators?
On 09/08/14 16:52, Rafael J. Wysocki wrote:
So more bikeshedding. :-)
Isn't "generic" somewhat too broad? Surely it doesn't cover x86. And does it actually cover anything without clocks/regulators?
I thought when you bikeshed you have to come up with a different color ;-)
Perhaps "cpufreq_cpus"?
On 9 September 2014 05:31, Stephen Boyd sboyd@codeaurora.org wrote:
On 09/08/14 16:52, Rafael J. Wysocki wrote:
So more bikeshedding. :-)
:)
Isn't "generic" somewhat too broad? Surely it doesn't cover x86. And does it actually cover anything without clocks/regulators?
It can work without regulators but not clocks.
I thought when you bikeshed you have to come up with a different color ;-)
Perhaps "cpufreq_cpus"?
I don't have any affection to the word 'generic' here and it can be anything we want it to be. But yeah, we shouldn't let these patches wait for that for too long :)
What name do you suggest Rafael?
- cpufreq_generic - cpufreq_cpus - cpufreq_dt - cpufreq_platform - cpufreq_cpuX
Let me know and I will prepare pending patches accordingly :)
On Tuesday, September 09, 2014 10:17:54 AM Viresh Kumar wrote:
On 9 September 2014 05:31, Stephen Boyd sboyd@codeaurora.org wrote:
On 09/08/14 16:52, Rafael J. Wysocki wrote:
So more bikeshedding. :-)
:)
Isn't "generic" somewhat too broad? Surely it doesn't cover x86. And does it actually cover anything without clocks/regulators?
It can work without regulators but not clocks.
I thought when you bikeshed you have to come up with a different color ;-)
Perhaps "cpufreq_cpus"?
I don't have any affection to the word 'generic' here and it can be anything we want it to be. But yeah, we shouldn't let these patches wait for that for too long :)
What name do you suggest Rafael?
- cpufreq_generic
- cpufreq_cpus
- cpufreq_dt
If it relies on DTs being used, this probably is the right name to call it.
- cpufreq_platform
- cpufreq_cpuX
Let me know and I will prepare pending patches accordingly :)
On 9 September 2014 18:59, Rafael J. Wysocki rjw@rjwysocki.net wrote:
- cpufreq_dt
If it relies on DTs being used, this probably is the right name to call it.
Okay then. Let it be cpufreq-dt.c.. Will send patches shortly..
On Tuesday, September 09, 2014 01:52:10 AM Rafael J. Wysocki wrote:
On Tuesday, September 09, 2014 01:39:32 AM Rafael J. Wysocki wrote:
On Monday, September 08, 2014 04:18:11 PM Stephen Boyd wrote:
On 09/08/14 16:34, Rafael J. Wysocki wrote:
On Thursday, August 28, 2014 11:22:31 AM Viresh Kumar wrote:
The naming convention of this driver was always under the scanner, people complained that it should have a more generic name than cpu0, as it manages all CPUs that are sharing clock lines.
Also, in future it will be modified to support any number of clusters with separate clock/voltage lines.
Lets rename it to 'cpufreq_generic' from 'cpufreq_cpu0'.
I'm not a big fan of changes like this to be honest, because they kind of hide history. For example, if you do a "git blame" on the new file, it will show your commit and nothing before it.
Sounds like a tooling problem. But git blame doesn't have that behavior so there isn't actually any problem?
Ah, OK
It used to some time ago or maybe it depends on how you carry out the rename.
So more bikeshedding. :-)
Isn't "generic" somewhat too broad? Surely it doesn't cover x86. And does it actually cover anything without clocks/regulators?
For now, I'm going to queue up patches [1-8/10] from this series for 3.18.
We have already renamed cpufreq-cpu0 to cpufreq-generic at most of the places, the only one left is in the '.name' field of platform driver and devices.
Lets do it now for all users.
Updates cpufreq-cpu0 from comments as well.
Tested-by: Stephen Boyd sboyd@codeaurora.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- arch/arm/mach-imx/imx27-dt.c | 2 +- arch/arm/mach-imx/mach-imx51.c | 2 +- arch/arm/mach-omap2/pm.c | 2 +- arch/arm/mach-shmobile/board-ape6evm-reference.c | 2 +- arch/arm/mach-shmobile/cpufreq.c | 2 +- arch/arm/mach-shmobile/setup-sh73a0.c | 4 ++-- arch/arm/mach-zynq/common.c | 2 +- drivers/cpufreq/cpufreq-generic.c | 2 +- drivers/cpufreq/exynos4210-cpufreq.c | 2 +- drivers/cpufreq/exynos4x12-cpufreq.c | 2 +- drivers/cpufreq/exynos5250-cpufreq.c | 2 +- drivers/cpufreq/highbank-cpufreq.c | 6 +++--- drivers/cpufreq/s5pv210-cpufreq.c | 2 +- 13 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/arch/arm/mach-imx/imx27-dt.c b/arch/arm/mach-imx/imx27-dt.c index 080e66c..e15db8a 100644 --- a/arch/arm/mach-imx/imx27-dt.c +++ b/arch/arm/mach-imx/imx27-dt.c @@ -20,7 +20,7 @@
static void __init imx27_dt_init(void) { - struct platform_device_info devinfo = { .name = "cpufreq-cpu0", }; + struct platform_device_info devinfo = { .name = "cpufreq-generic", };
mxc_arch_reset_init_dt();
diff --git a/arch/arm/mach-imx/mach-imx51.c b/arch/arm/mach-imx/mach-imx51.c index c77deb3..7298136 100644 --- a/arch/arm/mach-imx/mach-imx51.c +++ b/arch/arm/mach-imx/mach-imx51.c @@ -51,7 +51,7 @@ static void __init imx51_ipu_mipi_setup(void)
static void __init imx51_dt_init(void) { - struct platform_device_info devinfo = { .name = "cpufreq-cpu0", }; + struct platform_device_info devinfo = { .name = "cpufreq-generic", };
mxc_arch_reset_init_dt(); imx51_ipu_mipi_setup(); diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c index 828aee9..7ec2fbb 100644 --- a/arch/arm/mach-omap2/pm.c +++ b/arch/arm/mach-omap2/pm.c @@ -282,7 +282,7 @@ static inline void omap_init_cpufreq(void) if (!of_have_populated_dt()) devinfo.name = "omap-cpufreq"; else - devinfo.name = "cpufreq-cpu0"; + devinfo.name = "cpufreq-generic"; platform_device_register_full(&devinfo); }
diff --git a/arch/arm/mach-shmobile/board-ape6evm-reference.c b/arch/arm/mach-shmobile/board-ape6evm-reference.c index 2f7723e..4e98972 100644 --- a/arch/arm/mach-shmobile/board-ape6evm-reference.c +++ b/arch/arm/mach-shmobile/board-ape6evm-reference.c @@ -50,7 +50,7 @@ static void __init ape6evm_add_standard_devices(void)
r8a73a4_add_dt_devices(); of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); - platform_device_register_simple("cpufreq-cpu0", -1, NULL, 0); + platform_device_register_simple("cpufreq-generic", -1, NULL, 0); }
static const char *ape6evm_boards_compat_dt[] __initdata = { diff --git a/arch/arm/mach-shmobile/cpufreq.c b/arch/arm/mach-shmobile/cpufreq.c index 8a24b2b..6817204 100644 --- a/arch/arm/mach-shmobile/cpufreq.c +++ b/arch/arm/mach-shmobile/cpufreq.c @@ -12,6 +12,6 @@
int __init shmobile_cpufreq_init(void) { - platform_device_register_simple("cpufreq-cpu0", -1, NULL, 0); + platform_device_register_simple("cpufreq-generic", -1, NULL, 0); return 0; } diff --git a/arch/arm/mach-shmobile/setup-sh73a0.c b/arch/arm/mach-shmobile/setup-sh73a0.c index 2c802ae..297c629 100644 --- a/arch/arm/mach-shmobile/setup-sh73a0.c +++ b/arch/arm/mach-shmobile/setup-sh73a0.c @@ -775,7 +775,7 @@ void __init sh73a0_add_early_devices(void)
void __init sh73a0_add_standard_devices_dt(void) { - struct platform_device_info devinfo = { .name = "cpufreq-cpu0", .id = -1, }; + struct platform_device_info devinfo = { .name = "cpufreq-generic", .id = -1, };
/* clocks are setup late during boot in the case of DT */ sh73a0_clock_init(); @@ -784,7 +784,7 @@ void __init sh73a0_add_standard_devices_dt(void) ARRAY_SIZE(sh73a0_devices_dt)); of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
- /* Instantiate cpufreq-cpu0 */ + /* Instantiate cpufreq-generic */ platform_device_register_full(&devinfo); }
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c index 31a6fa4..79f3c61 100644 --- a/arch/arm/mach-zynq/common.c +++ b/arch/arm/mach-zynq/common.c @@ -104,7 +104,7 @@ static int __init zynq_get_revision(void) */ static void __init zynq_init_machine(void) { - struct platform_device_info devinfo = { .name = "cpufreq-cpu0", }; + struct platform_device_info devinfo = { .name = "cpufreq-generic", }; struct soc_device_attribute *soc_dev_attr; struct soc_device *soc_dev; struct device *parent = NULL; diff --git a/drivers/cpufreq/cpufreq-generic.c b/drivers/cpufreq/cpufreq-generic.c index 9b85055..549c2e9 100644 --- a/drivers/cpufreq/cpufreq-generic.c +++ b/drivers/cpufreq/cpufreq-generic.c @@ -349,7 +349,7 @@ static int generic_cpufreq_remove(struct platform_device *pdev)
static struct platform_driver generic_cpufreq_platdrv = { .driver = { - .name = "cpufreq-cpu0", + .name = "cpufreq-generic", .owner = THIS_MODULE, }, .probe = generic_cpufreq_probe, diff --git a/drivers/cpufreq/exynos4210-cpufreq.c b/drivers/cpufreq/exynos4210-cpufreq.c index 61a5431..728c9f4 100644 --- a/drivers/cpufreq/exynos4210-cpufreq.c +++ b/drivers/cpufreq/exynos4210-cpufreq.c @@ -127,7 +127,7 @@ int exynos4210_cpufreq_init(struct exynos_dvfs_info *info) * dependencies on platform headers. It is necessary to enable * Exynos multi-platform support and will be removed together with * this whole driver as soon as Exynos gets migrated to use - * cpufreq-cpu0 driver. + * cpufreq-generic driver. */ np = of_find_compatible_node(NULL, NULL, "samsung,exynos4210-clock"); if (!np) { diff --git a/drivers/cpufreq/exynos4x12-cpufreq.c b/drivers/cpufreq/exynos4x12-cpufreq.c index 351a207..7bbe5ef 100644 --- a/drivers/cpufreq/exynos4x12-cpufreq.c +++ b/drivers/cpufreq/exynos4x12-cpufreq.c @@ -174,7 +174,7 @@ int exynos4x12_cpufreq_init(struct exynos_dvfs_info *info) * dependencies on platform headers. It is necessary to enable * Exynos multi-platform support and will be removed together with * this whole driver as soon as Exynos gets migrated to use - * cpufreq-cpu0 driver. + * cpufreq-generic driver. */ np = of_find_compatible_node(NULL, NULL, "samsung,exynos4412-clock"); if (!np) { diff --git a/drivers/cpufreq/exynos5250-cpufreq.c b/drivers/cpufreq/exynos5250-cpufreq.c index c91ce69..d39864c 100644 --- a/drivers/cpufreq/exynos5250-cpufreq.c +++ b/drivers/cpufreq/exynos5250-cpufreq.c @@ -153,7 +153,7 @@ int exynos5250_cpufreq_init(struct exynos_dvfs_info *info) * dependencies on platform headers. It is necessary to enable * Exynos multi-platform support and will be removed together with * this whole driver as soon as Exynos gets migrated to use - * cpufreq-cpu0 driver. + * cpufreq-generic driver. */ np = of_find_compatible_node(NULL, NULL, "samsung,exynos5250-clock"); if (!np) { diff --git a/drivers/cpufreq/highbank-cpufreq.c b/drivers/cpufreq/highbank-cpufreq.c index bf8902a..dc7a9ab 100644 --- a/drivers/cpufreq/highbank-cpufreq.c +++ b/drivers/cpufreq/highbank-cpufreq.c @@ -6,7 +6,7 @@ * published by the Free Software Foundation. * * This driver provides the clk notifier callbacks that are used when - * the cpufreq-cpu0 driver changes to frequency to alert the highbank + * the cpufreq-generic driver changes to frequency to alert the highbank * EnergyCore Management Engine (ECME) about the need to change * voltage. The ECME interfaces with the actual voltage regulators. */ @@ -60,7 +60,7 @@ static struct notifier_block hb_cpufreq_clk_nb = {
static int hb_cpufreq_driver_init(void) { - struct platform_device_info devinfo = { .name = "cpufreq-cpu0", }; + struct platform_device_info devinfo = { .name = "cpufreq-generic", }; struct device *cpu_dev; struct clk *cpu_clk; struct device_node *np; @@ -95,7 +95,7 @@ static int hb_cpufreq_driver_init(void) goto out_put_node; }
- /* Instantiate cpufreq-cpu0 */ + /* Instantiate cpufreq-generic */ platform_device_register_full(&devinfo);
out_put_node: diff --git a/drivers/cpufreq/s5pv210-cpufreq.c b/drivers/cpufreq/s5pv210-cpufreq.c index 9a68225..11aa3f4 100644 --- a/drivers/cpufreq/s5pv210-cpufreq.c +++ b/drivers/cpufreq/s5pv210-cpufreq.c @@ -597,7 +597,7 @@ static int s5pv210_cpufreq_probe(struct platform_device *pdev) * and dependencies on platform headers. It is necessary to enable * S5PV210 multi-platform support and will be removed together with * this whole driver as soon as S5PV210 gets migrated to use - * cpufreq-cpu0 driver. + * cpufreq-generic driver. */ np = of_find_compatible_node(NULL, NULL, "samsung,s5pv210-clock"); if (!np) {
On Thursday 28 August 2014 01:52 AM, Viresh Kumar wrote:
Hi Rafael,
I am sending this again, but without the controversial part this time. I have dropped last two patches which were about detecting clock sharing among CPUs. Would fix up that later once other Maintainers get in sync about the bindings.
So, what's left here ? These are mostly updates/reorders which can be applied the patches which I have dropped now. These are all well reviewed and tested by others. Getting them in shouldn't break anything, I believe. Its better people start using the updates we already have, otherwise they will keep on sending fixes they get (The way Pramod Gurav tried it today).
Can we please get them in 3.17 if its still allowed? Or -next otherwise.
V2: https://lkml.org/lkml/2014/7/1/358
Rebased over: 3.17-rc2
git://git.linaro.org/people/viresh.kumar/linux.git cpufreq/cpu0-updates-v3
V2->V3: Dropped few patches, nothing much.
I just hope that binding related discussion don't die because the series has been moved on. Ofcourse the rest of the patches can move forward.
Feel free to add my ack if you need one
Viresh Kumar (10): cpufreq: Add support for per-policy driver data cpufreq: cpu0: Update Module Author cpufreq: cpu0: don't validate clock on clk_put() cpufreq: cpu0: print relevant error when we defer probe cpufreq: cpu0: use dev_{err|warn|dbg} instead of pr_{err|warn|debug} cpufreq: cpu0: Move per-cluster initialization code to ->init() cpufreq: cpu0: try regulators with name "cpu-supply" cpufreq: cpu0: Make allocate_resources() work for any CPU cpufreq: cpu0: rename driver and internals to 'cpufreq_generic' cpufreq: generic: set platform_{driver|device} '.name' to 'cpufreq-generic'
.../{cpufreq-cpu0.txt => cpufreq-generic.txt} | 8 +- arch/arm/mach-imx/imx27-dt.c | 2 +- arch/arm/mach-imx/mach-imx51.c | 2 +- arch/arm/mach-omap2/pm.c | 2 +- arch/arm/mach-shmobile/board-ape6evm-reference.c | 2 +- arch/arm/mach-shmobile/cpufreq.c | 2 +- arch/arm/mach-shmobile/setup-sh73a0.c | 4 +- arch/arm/mach-zynq/common.c | 2 +- drivers/cpufreq/Kconfig | 10 +- drivers/cpufreq/Kconfig.arm | 2 +- drivers/cpufreq/Makefile | 2 +- drivers/cpufreq/cpufreq-cpu0.c | 248 -------------- drivers/cpufreq/cpufreq-generic.c | 363 +++++++++++++++++++++ drivers/cpufreq/exynos4210-cpufreq.c | 2 +- drivers/cpufreq/exynos4x12-cpufreq.c | 2 +- drivers/cpufreq/exynos5250-cpufreq.c | 2 +- drivers/cpufreq/highbank-cpufreq.c | 6 +- drivers/cpufreq/s5pv210-cpufreq.c | 2 +- include/linux/cpufreq.h | 3 + 19 files changed, 392 insertions(+), 274 deletions(-) rename Documentation/devicetree/bindings/cpufreq/{cpufreq-cpu0.txt => cpufreq-generic.txt} (85%) delete mode 100644 drivers/cpufreq/cpufreq-cpu0.c create mode 100644 drivers/cpufreq/cpufreq-generic.c
On Thursday, August 28, 2014, Santosh Shilimkar santosh.shilimkar@ti.com wrote:
I just hope that binding related discussion don't die because the series has been moved on. Ofcourse the rest of the patches can move forward.
No, the series isn't going to be complete without getting the bindings fixed. So I still need that.
And I need people to respond and tell me clearly how bindings should look like, otherwise it will get delayed indefinitely.
Feel free to add my ack if you need one
I already have :)
I just noticed that my last mail bounced as my ipad sent it in html somehow :( Resending it..
On 28 August 2014 19:46, Santosh Shilimkar santosh.shilimkar@ti.com wrote:
I just hope that binding related discussion don't die because the series has been moved on. Ofcourse the rest of the patches can move forward.
No, the series isn't going to be complete without getting the bindings fixed. So I still need that.
And I need people to respond and tell me clearly how bindings should look like, otherwise it will get delayed indefinitely.
Feel free to add my ack if you need one
I already have :)
linaro-kernel@lists.linaro.org
-
Rafael J. Wysocki -
Santosh Shilimkar -
Stephen Boyd -
Viresh Kumar