From 88763a5cf80ca59a7c3bea32681ce8f697d9995f Mon Sep 17 00:00:00 2001 From: Daniel Lezcano Date: Tue, 26 Jun 2018 12:53:29 +0200 Subject: powercap / idle_inject: Add an idle injection framework Initially, the cpu_cooling device for ARM was changed by adding a new policy inserting idle cycles. The intel_powerclamp driver does a similar action. Instead of implementing idle injections privately in the cpu_cooling device, move the idle injection code in a dedicated framework and give the opportunity to other frameworks to make use of it. The framework relies on the smpboot kthreads which handles via its main loop the common code for hotplugging and [un]parking. This code was previously tested with the cpu cooling device and went through several iterations. It results now in split code and API exported in the header file. It was tested with the cpu cooling device with success. Signed-off-by: Daniel Lezcano Reviewed-by: Viresh Kumar [ rjw: Rewrite of all comments ] Signed-off-by: Rafael J. Wysocki --- drivers/powercap/Kconfig | 10 ++ drivers/powercap/Makefile | 1 + drivers/powercap/idle_inject.c | 356 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 367 insertions(+) create mode 100644 drivers/powercap/idle_inject.c (limited to 'drivers') diff --git a/drivers/powercap/Kconfig b/drivers/powercap/Kconfig index 85727ef6ce8e..6ac27e5908f5 100644 --- a/drivers/powercap/Kconfig +++ b/drivers/powercap/Kconfig @@ -29,4 +29,14 @@ config INTEL_RAPL controller, CPU core (Power Plance 0), graphics uncore (Power Plane 1), etc. +config IDLE_INJECT + bool "Idle injection framework" + depends on CPU_IDLE + default n + help + This enables support for the idle injection framework. It + provides a way to force idle periods on a set of specified + CPUs for power capping. Idle period can be injected + synchronously on a set of specified CPUs or alternatively + on a per CPU basis. endif diff --git a/drivers/powercap/Makefile b/drivers/powercap/Makefile index 0a21ef31372b..1b328854b36e 100644 --- a/drivers/powercap/Makefile +++ b/drivers/powercap/Makefile @@ -1,2 +1,3 @@ obj-$(CONFIG_POWERCAP) += powercap_sys.o obj-$(CONFIG_INTEL_RAPL) += intel_rapl.o +obj-$(CONFIG_IDLE_INJECT) += idle_inject.o diff --git a/drivers/powercap/idle_inject.c b/drivers/powercap/idle_inject.c new file mode 100644 index 000000000000..24ff2a068978 --- /dev/null +++ b/drivers/powercap/idle_inject.c @@ -0,0 +1,356 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright 2018 Linaro Limited + * + * Author: Daniel Lezcano + * + * The idle injection framework provides a way to force CPUs to enter idle + * states for a specified fraction of time over a specified period. + * + * It relies on the smpboot kthreads feature providing common code for CPU + * hotplug and thread [un]parking. + * + * All of the kthreads used for idle injection are created at init time. + * + * Next, the users of the the idle injection framework provide a cpumask via + * its register function. The kthreads will be synchronized with respect to + * this cpumask. + * + * The idle + run duration is specified via separate helpers and that allows + * idle injection to be started. + * + * The idle injection kthreads will call play_idle() with the idle duration + * specified as per the above. + * + * After all of them have been woken up, a timer is set to start the next idle + * injection cycle. + * + * The timer interrupt handler will wake up the idle injection kthreads for + * all of the CPUs in the cpumask provided by the user. + * + * Idle injection is stopped synchronously and no leftover idle injection + * kthread activity after its completion is guaranteed. + * + * It is up to the user of this framework to provide a lock for higher-level + * synchronization to prevent race conditions like starting idle injection + * while unregistering from the framework. + */ +#define pr_fmt(fmt) "ii_dev: " fmt + +#include +#include +#include +#include +#include +#include + +#include + +/** + * struct idle_inject_thread - task on/off switch structure + * @tsk: task injecting the idle cycles + * @should_run: whether or not to run the task (for the smpboot kthread API) + */ +struct idle_inject_thread { + struct task_struct *tsk; + int should_run; +}; + +/** + * struct idle_inject_device - idle injection data + * @timer: idle injection period timer + * @idle_duration_ms: duration of CPU idle time to inject + * @run_duration_ms: duration of CPU run time to allow + * @cpumask: mask of CPUs affected by idle injection + */ +struct idle_inject_device { + struct hrtimer timer; + unsigned int idle_duration_ms; + unsigned int run_duration_ms; + unsigned long int cpumask[0]; +}; + +static DEFINE_PER_CPU(struct idle_inject_thread, idle_inject_thread); +static DEFINE_PER_CPU(struct idle_inject_device *, idle_inject_device); + +/** + * idle_inject_wakeup - Wake up idle injection threads + * @ii_dev: target idle injection device + * + * Every idle injection task associated with the given idle injection device + * and running on an online CPU will be woken up. + */ +static void idle_inject_wakeup(struct idle_inject_device *ii_dev) +{ + struct idle_inject_thread *iit; + unsigned int cpu; + + for_each_cpu_and(cpu, to_cpumask(ii_dev->cpumask), cpu_online_mask) { + iit = per_cpu_ptr(&idle_inject_thread, cpu); + iit->should_run = 1; + wake_up_process(iit->tsk); + } +} + +/** + * idle_inject_timer_fn - idle injection timer function + * @timer: idle injection hrtimer + * + * This function is called when the idle injection timer expires. It wakes up + * idle injection tasks associated with the timer and they, in turn, invoke + * play_idle() to inject a specified amount of CPU idle time. + * + * Return: HRTIMER_RESTART. + */ +static enum hrtimer_restart idle_inject_timer_fn(struct hrtimer *timer) +{ + unsigned int duration_ms; + struct idle_inject_device *ii_dev = + container_of(timer, struct idle_inject_device, timer); + + duration_ms = READ_ONCE(ii_dev->run_duration_ms); + duration_ms += READ_ONCE(ii_dev->idle_duration_ms); + + idle_inject_wakeup(ii_dev); + + hrtimer_forward_now(timer, ms_to_ktime(duration_ms)); + + return HRTIMER_RESTART; +} + +/** + * idle_inject_fn - idle injection work function + * @cpu: the CPU owning the task + * + * This function calls play_idle() to inject a specified amount of CPU idle + * time. + */ +static void idle_inject_fn(unsigned int cpu) +{ + struct idle_inject_device *ii_dev; + struct idle_inject_thread *iit; + + ii_dev = per_cpu(idle_inject_device, cpu); + iit = per_cpu_ptr(&idle_inject_thread, cpu); + + /* + * Let the smpboot main loop know that the task should not run again. + */ + iit->should_run = 0; + + play_idle(READ_ONCE(ii_dev->idle_duration_ms)); +} + +/** + * idle_inject_set_duration - idle and run duration update helper + * @run_duration_ms: CPU run time to allow in milliseconds + * @idle_duration_ms: CPU idle time to inject in milliseconds + */ +void idle_inject_set_duration(struct idle_inject_device *ii_dev, + unsigned int run_duration_ms, + unsigned int idle_duration_ms) +{ + if (run_duration_ms && idle_duration_ms) { + WRITE_ONCE(ii_dev->run_duration_ms, run_duration_ms); + WRITE_ONCE(ii_dev->idle_duration_ms, idle_duration_ms); + } +} + +/** + * idle_inject_get_duration - idle and run duration retrieval helper + * @run_duration_ms: memory location to store the current CPU run time + * @idle_duration_ms: memory location to store the current CPU idle time + */ +void idle_inject_get_duration(struct idle_inject_device *ii_dev, + unsigned int *run_duration_ms, + unsigned int *idle_duration_ms) +{ + *run_duration_ms = READ_ONCE(ii_dev->run_duration_ms); + *idle_duration_ms = READ_ONCE(ii_dev->idle_duration_ms); +} + +/** + * idle_inject_start - start idle injections + * @ii_dev: idle injection control device structure + * + * The function starts idle injection by first waking up all of the idle + * injection kthreads associated with @ii_dev to let them inject CPU idle time + * sets up a timer to start the next idle injection period. + * + * Return: -EINVAL if the CPU idle or CPU run time is not set or 0 on success. + */ +int idle_inject_start(struct idle_inject_device *ii_dev) +{ + unsigned int idle_duration_ms = READ_ONCE(ii_dev->idle_duration_ms); + unsigned int run_duration_ms = READ_ONCE(ii_dev->run_duration_ms); + + if (!idle_duration_ms || !run_duration_ms) + return -EINVAL; + + pr_debug("Starting injecting idle cycles on CPUs '%*pbl'\n", + cpumask_pr_args(to_cpumask(ii_dev->cpumask))); + + idle_inject_wakeup(ii_dev); + + hrtimer_start(&ii_dev->timer, + ms_to_ktime(idle_duration_ms + run_duration_ms), + HRTIMER_MODE_REL); + + return 0; +} + +/** + * idle_inject_stop - stops idle injections + * @ii_dev: idle injection control device structure + * + * The function stops idle injection and waits for the threads to finish work. + * If CPU idle time is being injected when this function runs, then it will + * wait until the end of the cycle. + * + * When it returns, there is no more idle injection kthread activity. The + * kthreads are scheduled out and the periodic timer is off. + */ +void idle_inject_stop(struct idle_inject_device *ii_dev) +{ + struct idle_inject_thread *iit; + unsigned int cpu; + + pr_debug("Stopping idle injection on CPUs '%*pbl'\n", + cpumask_pr_args(to_cpumask(ii_dev->cpumask))); + + hrtimer_cancel(&ii_dev->timer); + + /* + * Stopping idle injection requires all of the idle injection kthreads + * associated with the given cpumask to be parked and stay that way, so + * prevent CPUs from going online at this point. Any CPUs going online + * after the loop below will be covered by clearing the should_run flag + * that will cause the smpboot main loop to schedule them out. + */ + cpu_hotplug_disable(); + + /* + * Iterate over all (online + offline) CPUs here in case one of them + * goes offline with the should_run flag set so as to prevent its idle + * injection kthread from running when the CPU goes online again after + * the ii_dev has been freed. + */ + for_each_cpu(cpu, to_cpumask(ii_dev->cpumask)) { + iit = per_cpu_ptr(&idle_inject_thread, cpu); + iit->should_run = 0; + + wait_task_inactive(iit->tsk, 0); + } + + cpu_hotplug_enable(); +} + +/** + * idle_inject_setup - prepare the current task for idle injection + * @cpu: not used + * + * Called once, this function is in charge of setting the current task's + * scheduler parameters to make it an RT task. + */ +static void idle_inject_setup(unsigned int cpu) +{ + struct sched_param param = { .sched_priority = MAX_USER_RT_PRIO / 2 }; + + sched_setscheduler(current, SCHED_FIFO, ¶m); +} + +/** + * idle_inject_should_run - function helper for the smpboot API + * @cpu: CPU the kthread is running on + * + * Return: whether or not the thread can run. + */ +static int idle_inject_should_run(unsigned int cpu) +{ + struct idle_inject_thread *iit = + per_cpu_ptr(&idle_inject_thread, cpu); + + return iit->should_run; +} + +/** + * idle_inject_register - initialize idle injection on a set of CPUs + * @cpumask: CPUs to be affected by idle injection + * + * This function creates an idle injection control device structure for the + * given set of CPUs and initializes the timer associated with it. It does not + * start any injection cycles. + * + * Return: NULL if memory allocation fails, idle injection control device + * pointer on success. + */ +struct idle_inject_device *idle_inject_register(struct cpumask *cpumask) +{ + struct idle_inject_device *ii_dev; + int cpu, cpu_rb; + + ii_dev = kzalloc(sizeof(*ii_dev) + cpumask_size(), GFP_KERNEL); + if (!ii_dev) + return NULL; + + cpumask_copy(to_cpumask(ii_dev->cpumask), cpumask); + hrtimer_init(&ii_dev->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); + ii_dev->timer.function = idle_inject_timer_fn; + + for_each_cpu(cpu, to_cpumask(ii_dev->cpumask)) { + + if (per_cpu(idle_inject_device, cpu)) { + pr_err("cpu%d is already registered\n", cpu); + goto out_rollback; + } + + per_cpu(idle_inject_device, cpu) = ii_dev; + } + + return ii_dev; + +out_rollback: + for_each_cpu(cpu_rb, to_cpumask(ii_dev->cpumask)) { + if (cpu == cpu_rb) + break; + per_cpu(idle_inject_device, cpu_rb) = NULL; + } + + kfree(ii_dev); + + return NULL; +} + +/** + * idle_inject_unregister - unregister idle injection control device + * @ii_dev: idle injection control device to unregister + * + * The function stops idle injection for the given control device, + * unregisters its kthreads and frees memory allocated when that device was + * created. + */ +void idle_inject_unregister(struct idle_inject_device *ii_dev) +{ + unsigned int cpu; + + idle_inject_stop(ii_dev); + + for_each_cpu(cpu, to_cpumask(ii_dev->cpumask)) + per_cpu(idle_inject_device, cpu) = NULL; + + kfree(ii_dev); +} + +static struct smp_hotplug_thread idle_inject_threads = { + .store = &idle_inject_thread.tsk, + .setup = idle_inject_setup, + .thread_fn = idle_inject_fn, + .thread_comm = "idle_inject/%u", + .thread_should_run = idle_inject_should_run, +}; + +static int __init idle_inject_init(void) +{ + return smpboot_register_percpu_thread(&idle_inject_threads); +} +early_initcall(idle_inject_init); -- cgit 1.4.1 From 12ba2c65f9d3d6996d1e909921151487b009dd30 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Tue, 13 Feb 2018 22:10:42 +0100 Subject: PM / devfreq: exynos-ppmu: Delete an error message for a failed memory allocation in exynos_ppmu_probe() Omit an extra message for a memory allocation failure in this function. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring Acked-by: Chanwoo Choi Signed-off-by: MyungJoo Ham --- drivers/devfreq/event/exynos-ppmu.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/devfreq/event/exynos-ppmu.c b/drivers/devfreq/event/exynos-ppmu.c index 3cd6a184fe7c..a9c64f0d3284 100644 --- a/drivers/devfreq/event/exynos-ppmu.c +++ b/drivers/devfreq/event/exynos-ppmu.c @@ -627,11 +627,9 @@ static int exynos_ppmu_probe(struct platform_device *pdev) size = sizeof(struct devfreq_event_dev *) * info->num_events; info->edev = devm_kzalloc(&pdev->dev, size, GFP_KERNEL); - if (!info->edev) { - dev_err(&pdev->dev, - "failed to allocate memory devfreq-event devices\n"); + if (!info->edev) return -ENOMEM; - } + edev = info->edev; platform_set_drvdata(pdev, info); -- cgit 1.4.1 From 2d803dc8f7a5f622ac47c3b650834ada3a2659b9 Mon Sep 17 00:00:00 2001 From: Arvind Yadav Date: Fri, 30 Mar 2018 17:14:03 +0530 Subject: PM / devfreq: use put_device() instead of kfree() Never directly free @dev after calling device_register() or device_unregister(), even if device_register() returned an error. Always use put_device() to give up the reference initialized. Signed-off-by: Arvind Yadav Reviewed-by: Chanwoo Choi Signed-off-by: MyungJoo Ham --- drivers/devfreq/devfreq.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index 0b5b3abe054e..e26adf67e218 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -625,7 +625,8 @@ struct devfreq *devfreq_add_device(struct device *dev, err = device_register(&devfreq->dev); if (err) { mutex_unlock(&devfreq->lock); - goto err_dev; + put_device(&devfreq->dev); + goto err_out; } devfreq->trans_table = @@ -672,6 +673,7 @@ err_init: mutex_unlock(&devfreq_list_lock); device_unregister(&devfreq->dev); + devfreq = NULL; err_dev: if (devfreq) kfree(devfreq); -- cgit 1.4.1 From 90dd72e1290dd86c4b6e5c421fcd13e60e625782 Mon Sep 17 00:00:00 2001 From: Enric Balletbo i Serra Date: Wed, 9 May 2018 14:57:45 +0200 Subject: PM / devfreq: rk3399_dmc: remove wait for dcf irq event. We have already wait dcf done in ATF, so don't need wait dcf irq in kernel, besides, clear dcf irq in kernel will import competiton between kernel and ATF, only handle dcf irq in ATF is a better way. Signed-off-by: Lin Huang Signed-off-by: Enric Balletbo i Serra Reviewed-by: Chanwoo Choi Signed-off-by: MyungJoo Ham --- drivers/devfreq/rk3399_dmc.c | 53 +------------------------------------------- 1 file changed, 1 insertion(+), 52 deletions(-) (limited to 'drivers') diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c index 5dfbfa3cc878..44a379657cd5 100644 --- a/drivers/devfreq/rk3399_dmc.c +++ b/drivers/devfreq/rk3399_dmc.c @@ -68,15 +68,6 @@ struct rk3399_dmcfreq { struct devfreq_event_dev *edev; struct mutex lock; struct dram_timing timing; - - /* - * DDR Converser of Frequency (DCF) is used to implement DDR frequency - * conversion without the participation of CPU, we will implement and - * control it in arm trust firmware. - */ - wait_queue_head_t wait_dcf_queue; - int irq; - int wait_dcf_flag; struct regulator *vdd_center; unsigned long rate, target_rate; unsigned long volt, target_volt; @@ -117,7 +108,6 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, goto out; } } - dmcfreq->wait_dcf_flag = 1; err = clk_set_rate(dmcfreq->dmc_clk, target_rate); if (err) { @@ -128,14 +118,6 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, goto out; } - /* - * Wait until bcf irq happen, it means freq scaling finish in - * arm trust firmware, use 100ms as timeout time. - */ - if (!wait_event_timeout(dmcfreq->wait_dcf_queue, - !dmcfreq->wait_dcf_flag, HZ / 10)) - dev_warn(dev, "Timeout waiting for dcf interrupt\n"); - /* * Check the dpll rate, * There only two result we will get, @@ -241,22 +223,6 @@ static __maybe_unused int rk3399_dmcfreq_resume(struct device *dev) static SIMPLE_DEV_PM_OPS(rk3399_dmcfreq_pm, rk3399_dmcfreq_suspend, rk3399_dmcfreq_resume); -static irqreturn_t rk3399_dmc_irq(int irq, void *dev_id) -{ - struct rk3399_dmcfreq *dmcfreq = dev_id; - struct arm_smccc_res res; - - dmcfreq->wait_dcf_flag = 0; - wake_up(&dmcfreq->wait_dcf_queue); - - /* Clear the DCF interrupt */ - arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, 0, 0, - ROCKCHIP_SIP_CONFIG_DRAM_CLR_IRQ, - 0, 0, 0, 0, &res); - - return IRQ_HANDLED; -} - static int of_get_ddr_timings(struct dram_timing *timing, struct device_node *np) { @@ -330,16 +296,10 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct device_node *np = pdev->dev.of_node; struct rk3399_dmcfreq *data; - int ret, irq, index, size; + int ret, index, size; uint32_t *timing; struct dev_pm_opp *opp; - irq = platform_get_irq(pdev, 0); - if (irq < 0) { - dev_err(&pdev->dev, - "Cannot get the dmc interrupt resource: %d\n", irq); - return irq; - } data = devm_kzalloc(dev, sizeof(struct rk3399_dmcfreq), GFP_KERNEL); if (!data) return -ENOMEM; @@ -358,17 +318,6 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev) return PTR_ERR(data->dmc_clk); }; - data->irq = irq; - ret = devm_request_irq(dev, irq, rk3399_dmc_irq, 0, - dev_name(dev), data); - if (ret) { - dev_err(dev, "Failed to request dmc irq: %d\n", ret); - return ret; - } - - init_waitqueue_head(&data->wait_dcf_queue); - data->wait_dcf_flag = 0; - data->edev = devfreq_event_get_edev_by_phandle(dev, 0); if (IS_ERR(data->edev)) return -EPROBE_DEFER; -- cgit 1.4.1 From 49edc52312c34c981722833b0d9344c2aa83892d Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Wed, 9 May 2018 14:57:47 +0200 Subject: PM / devfreq: rk3399_dmc: do not print error when get supply and clk defer. We just return -EPROBE_DEFER error code to caller and do not print error message when try to get center logic regulator and DMC clock defer. Signed-off-by: Lin Huang Signed-off-by: Enric Balletbo i Serra Reviewed-by: Chanwoo Choi Signed-off-by: MyungJoo Ham --- drivers/devfreq/rk3399_dmc.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'drivers') diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c index 44a379657cd5..5bfca028eaaf 100644 --- a/drivers/devfreq/rk3399_dmc.c +++ b/drivers/devfreq/rk3399_dmc.c @@ -308,12 +308,18 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev) data->vdd_center = devm_regulator_get(dev, "center"); if (IS_ERR(data->vdd_center)) { + if (PTR_ERR(data->vdd_center) == -EPROBE_DEFER) + return -EPROBE_DEFER; + dev_err(dev, "Cannot get the regulator \"center\"\n"); return PTR_ERR(data->vdd_center); } data->dmc_clk = devm_clk_get(dev, "dmc_clk"); if (IS_ERR(data->dmc_clk)) { + if (PTR_ERR(data->dmc_clk) == -EPROBE_DEFER) + return -EPROBE_DEFER; + dev_err(dev, "Cannot get the clk dmc_clk\n"); return PTR_ERR(data->dmc_clk); }; -- cgit 1.4.1 From dfa7d764caf00b12da276ea473d7f1fd7fd40200 Mon Sep 17 00:00:00 2001 From: Enric Balletbo i Serra Date: Wed, 9 May 2018 14:57:48 +0200 Subject: PM / devfreq: rk3399_dmc: fix spelling mistakes. Fix some spelling mistakes in error and debug messages. Signed-off-by: Enric Balletbo i Serra Signed-off-by: MyungJoo Ham --- drivers/devfreq/rk3399_dmc.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers') diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c index 5bfca028eaaf..d5c03e5abe13 100644 --- a/drivers/devfreq/rk3399_dmc.c +++ b/drivers/devfreq/rk3399_dmc.c @@ -103,7 +103,7 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, err = regulator_set_voltage(dmcfreq->vdd_center, target_volt, target_volt); if (err) { - dev_err(dev, "Cannot to set voltage %lu uV\n", + dev_err(dev, "Cannot set voltage %lu uV\n", target_volt); goto out; } @@ -111,8 +111,8 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, err = clk_set_rate(dmcfreq->dmc_clk, target_rate); if (err) { - dev_err(dev, "Cannot to set frequency %lu (%d)\n", - target_rate, err); + dev_err(dev, "Cannot set frequency %lu (%d)\n", target_rate, + err); regulator_set_voltage(dmcfreq->vdd_center, dmcfreq->volt, dmcfreq->volt); goto out; @@ -128,8 +128,8 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, /* If get the incorrect rate, set voltage to old value. */ if (dmcfreq->rate != target_rate) { - dev_err(dev, "Get wrong ddr frequency, Request frequency %lu,\ - Current frequency %lu\n", target_rate, dmcfreq->rate); + dev_err(dev, "Got wrong frequency, Request %lu, Current %lu\n", + target_rate, dmcfreq->rate); regulator_set_voltage(dmcfreq->vdd_center, dmcfreq->volt, dmcfreq->volt); goto out; @@ -137,7 +137,7 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, err = regulator_set_voltage(dmcfreq->vdd_center, target_volt, target_volt); if (err) - dev_err(dev, "Cannot to set vol %lu uV\n", target_volt); + dev_err(dev, "Cannot set voltage %lu uV\n", target_volt); dmcfreq->rate = target_rate; dmcfreq->volt = target_volt; -- cgit 1.4.1 From 2c2cb1e6b05b90d55b4b943646faa3cfbdf78f6e Mon Sep 17 00:00:00 2001 From: Matthias Kaehlcke Date: Fri, 25 May 2018 13:30:33 -0700 Subject: PM / devfreq: Init user limits from OPP limits, not viceversa Commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when adding the devfreq device") introduced the initialization of the user limits min/max_freq from the lowest/highest available OPPs. Later commit f1d981eaecf8 ("PM / devfreq: Use the available min/max frequency") added scaling_min/max_freq, which actually represent the frequencies of the lowest/highest available OPP. scaling_min/ max_freq are initialized with the values from min/max_freq, which is totally correct in the context, but a bit awkward to read. Swap the initialization and assign scaling_min/max_freq with the OPP freqs and then the user limts min/max_freq with scaling_min/ max_freq. Needless to say that this change is a NOP, intended to improve readability. Signed-off-by: Matthias Kaehlcke Reviewed-by: Chanwoo Choi Reviewed-by: Brian Norris Signed-off-by: MyungJoo Ham --- drivers/devfreq/devfreq.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers') diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index e26adf67e218..4c49bb1330b5 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -604,21 +604,21 @@ struct devfreq *devfreq_add_device(struct device *dev, mutex_lock(&devfreq->lock); } - devfreq->min_freq = find_available_min_freq(devfreq); - if (!devfreq->min_freq) { + devfreq->scaling_min_freq = find_available_min_freq(devfreq); + if (!devfreq->scaling_min_freq) { mutex_unlock(&devfreq->lock); err = -EINVAL; goto err_dev; } - devfreq->scaling_min_freq = devfreq->min_freq; + devfreq->min_freq = devfreq->scaling_min_freq; - devfreq->max_freq = find_available_max_freq(devfreq); - if (!devfreq->max_freq) { + devfreq->scaling_max_freq = find_available_max_freq(devfreq); + if (!devfreq->scaling_max_freq) { mutex_unlock(&devfreq->lock); err = -EINVAL; goto err_dev; } - devfreq->scaling_max_freq = devfreq->max_freq; + devfreq->max_freq = devfreq->scaling_max_freq; dev_set_name(&devfreq->dev, "devfreq%d", atomic_inc_return(&devfreq_no)); -- cgit 1.4.1 From d6e98f3e6d825380b566dc59359917a116090154 Mon Sep 17 00:00:00 2001 From: Enric Balletbo i Serra Date: Fri, 15 Jun 2018 17:12:17 +0200 Subject: PM / devfreq: rk3399_dmc: Fix duplicated opp table on reload. The opp table is not removed when the driver is unloaded neither when there is an error within probe, so if the driver is reloaded the opp core shows the following warning: rk3399-dmc-freq dmc: _opp_add: duplicate OPPs detected. Existing: freq: 200000000, volt: 900000, enabled: 1. New: freq: 200000000, volt: 900000, enabled: 1 rk3399-dmc-freq dmc: _opp_add: duplicate OPPs detected. Existing: freq: 400000000, volt: 900000, enabled: 1. New: freq: 400000000, volt: 900000, enabled: 1 rk3399-dmc-freq dmc: _opp_add: duplicate OPPs detected. Existing: freq: 666000000, volt: 900000, enabled: 1. New: freq: 666000000, volt: 900000, enabled: 1 rk3399-dmc-freq dmc: _opp_add: duplicate OPPs detected. Existing: freq: 800000000, volt: 900000, enabled: 1. New: freq: 800000000, volt: 900000, enabled: 1 rk3399-dmc-freq dmc: _opp_add: duplicate OPPs detected. Existing: freq: 928000000, volt: 900000, enabled: 1. New: freq: 928000000, volt: 900000, enabled: 1 This patch fixes the error path in the probe function and adds a .remove function to properly cleanup the opp table on unloading. Fixes: 5a893e31a636c (PM / devfreq: rockchip: add devfreq driver for rk3399 dmc) Signed-off-by: Enric Balletbo i Serra Reviewed-by: Chanwoo Choi Signed-off-by: MyungJoo Ham --- drivers/devfreq/rk3399_dmc.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c index d5c03e5abe13..e795ad2b3f6b 100644 --- a/drivers/devfreq/rk3399_dmc.c +++ b/drivers/devfreq/rk3399_dmc.c @@ -375,8 +375,10 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev) data->rate = clk_get_rate(data->dmc_clk); opp = devfreq_recommended_opp(dev, &data->rate, 0); - if (IS_ERR(opp)) - return PTR_ERR(opp); + if (IS_ERR(opp)) { + ret = PTR_ERR(opp); + goto err_free_opp; + } data->rate = dev_pm_opp_get_freq(opp); data->volt = dev_pm_opp_get_voltage(opp); @@ -388,13 +390,33 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev) &rk3399_devfreq_dmc_profile, DEVFREQ_GOV_SIMPLE_ONDEMAND, &data->ondemand_data); - if (IS_ERR(data->devfreq)) - return PTR_ERR(data->devfreq); + if (IS_ERR(data->devfreq)) { + ret = PTR_ERR(data->devfreq); + goto err_free_opp; + } + devm_devfreq_register_opp_notifier(dev, data->devfreq); data->dev = dev; platform_set_drvdata(pdev, data); + return 0; + +err_free_opp: + dev_pm_opp_of_remove_table(&pdev->dev); + return ret; +} + +static int rk3399_dmcfreq_remove(struct platform_device *pdev) +{ + struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(&pdev->dev); + + /* + * Before remove the opp table we need to unregister the opp notifier. + */ + devm_devfreq_unregister_opp_notifier(dmcfreq->dev, dmcfreq->devfreq); + dev_pm_opp_of_remove_table(dmcfreq->dev); + return 0; } @@ -406,6 +428,7 @@ MODULE_DEVICE_TABLE(of, rk3399dmc_devfreq_of_match); static struct platform_driver rk3399_dmcfreq_driver = { .probe = rk3399_dmcfreq_probe, + .remove = rk3399_dmcfreq_remove, .driver = { .name = "rk3399-dmc-freq", .pm = &rk3399_dmcfreq_pm, -- cgit 1.4.1