* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Mark Brown @ 2012-11-21 1:31 UTC (permalink / raw)
To: Grant Likely
Cc: Alexandre Courbot, Anton Vorontsov, Stephen Warren,
Thierry Reding, Mark Zhang, Rob Herring, David Woodhouse,
Arnd Bergmann, linux-tegra, linux-arm-kernel, linux-kernel,
linux-fbdev, devicetree-discuss, linux-pm, Alexandre Courbot
In-Reply-To: <20121120215429.B621F3E1821@localhost>
[-- Attachment #1: Type: text/plain, Size: 2354 bytes --]
On Tue, Nov 20, 2012 at 09:54:29PM +0000, Grant Likely wrote:
> On Sat, 17 Nov 2012 19:55:45 +0900, Alexandre Courbot <acourbot@nvidia.com> wrote:
> > With the advent of the device tree and of ARM kernels that are not
> > board-tied, we cannot rely on these board-specific hooks anymore but
> This isn't strictly true. It is still perfectly fine to have board
> specific code when necessary. However, there is strong encouragement to
> enable that code in device drivers as much as possible and new board
> files need to have very strong justification.
This isn't the message that's gone over, and even for device drivers
everyone seems to be taking the whole device tree thing as a move to
pull all data out of the kernel. In some cases there are some real
practical advantages to doing this but a lot of the people making these
changes seem to view having things in DT as a goal in itself.
> I'm thinking about the scripts as trivial-to-parse ascii strings that
> have a very simple set of commands. The commands use resources already
> defined in the node. ie. 'g0' refers to the first entry in the gpios
> property. 'r0' for the regulator, 'p0' for the pwms, 'd' means delay. By
> no means take this as the format to use, it is just an example off the
> top of my head, but it is already way easier to work with than putting
> each command into a node.
It does appear to have some legibility challenges, especially with using
the indexes into the arrays of resources. On the other hand the arrays
should be fairly small.
> > +Platform Data Format
> > +--------------------
> > +All relevant data structures for declaring power sequences are located in
> > +include/linux/power_seq.h.
> Hmm... If sequences are switched to a string instead, then platform_data
> should also use it. The power sequence data structures can be created at
> runtime by parsing the string.
Seems like a step backwards to me - why not let people store the more
parsed version of the data? It's going to be less convenient for users
on non-DT systems.
As I said in my earlier reviews I think this is a useful thing to have
as a utility library for drivers independantly of the DT bindings, it
would allow drivers to become more data driven. Perhaps we can rework
the series so that the DT bindings are added as a final patch? All the
rest of the code seems OK.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* drivers/devfreq/governor_simpleondemand.c:36:11: error: dereferencing pointer to incomplete type
From: kbuild test robot @ 2012-11-21 1:16 UTC (permalink / raw)
To: Nishanth Menon; +Cc: linux-pm, MyungJoo Ham
tree: git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
head: aa0745b51987fbaf628ee0943907db177335522d
commit: eff607fdb1f787da1fedf46ab6e64adc2afd1c5a PM / devfreq: governors: add GPL module license and allow module build
date: 16 hours ago
config: make ARCH=x86_64 allmodconfig
All error/warnings:
drivers/devfreq/governor_simpleondemand.c: In function 'devfreq_simple_ondemand_func':
drivers/devfreq/governor_simpleondemand.c:36:11: error: dereferencing pointer to incomplete type
drivers/devfreq/governor_simpleondemand.c:37:27: error: dereferencing pointer to incomplete type
drivers/devfreq/governor_simpleondemand.c:38:11: error: dereferencing pointer to incomplete type
drivers/devfreq/governor_simpleondemand.c:39:32: error: dereferencing pointer to incomplete type
vim +36 drivers/devfreq/governor_simpleondemand.c
6530b9de MyungJoo Ham 2011-12-09 30 unsigned long max = (df->max_freq) ? df->max_freq : UINT_MAX;
ce26c5bb MyungJoo Ham 2011-10-02 31
ce26c5bb MyungJoo Ham 2011-10-02 32 if (err)
ce26c5bb MyungJoo Ham 2011-10-02 33 return err;
ce26c5bb MyungJoo Ham 2011-10-02 34
ce26c5bb MyungJoo Ham 2011-10-02 35 if (data) {
ce26c5bb MyungJoo Ham 2011-10-02 @36 if (data->upthreshold)
ce26c5bb MyungJoo Ham 2011-10-02 37 dfso_upthreshold = data->upthreshold;
ce26c5bb MyungJoo Ham 2011-10-02 38 if (data->downdifferential)
ce26c5bb MyungJoo Ham 2011-10-02 39 dfso_downdifferential = data->downdifferential;
---
0-DAY kernel build testing backend Open Source Technology Center
Fengguang Wu, Yuanhan Liu Intel Corporation
^ permalink raw reply
* Re: [PATCH 052/493] cpufreq: remove use of __devexit_p
From: Rafael J. Wysocki @ 2012-11-21 0:50 UTC (permalink / raw)
To: Bill Pemberton; +Cc: gregkh, cpufreq, linux-pm
In-Reply-To: <1353349642-3677-52-git-send-email-wfp5p@virginia.edu>
On Monday, November 19, 2012 01:20:01 PM Bill Pemberton wrote:
> CONFIG_HOTPLUG is going away as an option so __devexit_p is no longer
> needed.
Applied to the linux-next branch of the linux-pm.git tree as v3.8 material.
Thanks,
Rafael
> Signed-off-by: Bill Pemberton <wfp5p@virginia.edu>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: cpufreq@vger.kernel.org
> Cc: linux-pm@vger.kernel.org
> ---
> drivers/cpufreq/longhaul.c | 2 +-
> drivers/cpufreq/powernow-k8.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/longhaul.c b/drivers/cpufreq/longhaul.c
> index 53ddbc7..8d7ebb2 100644
> --- a/drivers/cpufreq/longhaul.c
> +++ b/drivers/cpufreq/longhaul.c
> @@ -946,7 +946,7 @@ static struct cpufreq_driver longhaul_driver = {
> .target = longhaul_target,
> .get = longhaul_get,
> .init = longhaul_cpu_init,
> - .exit = __devexit_p(longhaul_cpu_exit),
> + .exit = longhaul_cpu_exit,
> .name = "longhaul",
> .owner = THIS_MODULE,
> .attr = longhaul_attr,
> diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
> index e3ebb4f..1fd0c8a 100644
> --- a/drivers/cpufreq/powernow-k8.c
> +++ b/drivers/cpufreq/powernow-k8.c
> @@ -1242,7 +1242,7 @@ static struct cpufreq_driver cpufreq_amd64_driver = {
> .target = powernowk8_target,
> .bios_limit = acpi_processor_get_bios_limit,
> .init = powernowk8_cpu_init,
> - .exit = __devexit_p(powernowk8_cpu_exit),
> + .exit = powernowk8_cpu_exit,
> .get = powernowk8_get,
> .name = "powernow-k8",
> .owner = THIS_MODULE,
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: PM QOS flags
From: Rafael J. Wysocki @ 2012-11-21 0:55 UTC (permalink / raw)
To: Oliver Neukum; +Cc: linux-pm
In-Reply-To: <1905473.JbZu5BcXPL@linux-lqwf.site>
Hi,
On Wednesday, November 14, 2012 09:22:13 AM Oliver Neukum wrote:
> Hi,
>
> it seems to me that we are missing some flags. I would propose
>
> PM_QOS_FLAG_NO_EXTERNAL_EFFECT
> in some cases going to powersave has externally visible effects
> modems hang up, LEDs on keyboards extinguish, screens blank
>
> PM_QOS_FLAG_NO_HOTPLUG_DETECT
> hotplug is not detected while in powersave
>
> PM_QOS_FLAG_NO_MEDIUM_CHANGE
> the device does not detect medium change events in powersave
> USB storage devices are prime candidates for this
I'm fine with these, but please add them along with the users, so that
it is clear how you're planning to utilize them.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH 396/493] cpufreq: remove use of __devexit
From: Rafael J. Wysocki @ 2012-11-21 0:50 UTC (permalink / raw)
To: Bill Pemberton; +Cc: gregkh, cpufreq, linux-pm
In-Reply-To: <1353349642-3677-396-git-send-email-wfp5p@virginia.edu>
On Monday, November 19, 2012 01:25:45 PM Bill Pemberton wrote:
> CONFIG_HOTPLUG is going away as an option so __devexit is no
> longer needed.
Applied to the linux-next branch of the linux-pm.git tree as v3.8 material.
Thanks,
Rafael
> Signed-off-by: Bill Pemberton <wfp5p@virginia.edu>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: cpufreq@vger.kernel.org
> Cc: linux-pm@vger.kernel.org
> ---
> drivers/cpufreq/longhaul.c | 2 +-
> drivers/cpufreq/powernow-k8.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/longhaul.c b/drivers/cpufreq/longhaul.c
> index 8d7ebb2..f1fa500 100644
> --- a/drivers/cpufreq/longhaul.c
> +++ b/drivers/cpufreq/longhaul.c
> @@ -930,7 +930,7 @@ static int __cpuinit longhaul_cpu_init(struct cpufreq_policy *policy)
> return 0;
> }
>
> -static int __devexit longhaul_cpu_exit(struct cpufreq_policy *policy)
> +static int longhaul_cpu_exit(struct cpufreq_policy *policy)
> {
> cpufreq_frequency_table_put_attr(policy->cpu);
> return 0;
> diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
> index 1fd0c8a..056faf6 100644
> --- a/drivers/cpufreq/powernow-k8.c
> +++ b/drivers/cpufreq/powernow-k8.c
> @@ -1186,7 +1186,7 @@ err_out:
> return -ENODEV;
> }
>
> -static int __devexit powernowk8_cpu_exit(struct cpufreq_policy *pol)
> +static int powernowk8_cpu_exit(struct cpufreq_policy *pol)
> {
> struct powernow_k8_data *data = per_cpu(powernow_data, pol->cpu);
>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH RESEND v4 0/5] Migrate SCSI drivers to use dev_pm_ops
From: Rafael J. Wysocki @ 2012-11-21 0:53 UTC (permalink / raw)
To: James Bottomley; +Cc: Aaron Lu, Alan Stern, linux-pm, linux-scsi, Aaron Lu
In-Reply-To: <1352446075-1814-1-git-send-email-aaron.lu@intel.com>
Hi James,
On Friday, November 09, 2012 03:27:50 PM Aaron Lu wrote:
> This patchset has been quiet for a while, so resend them.
>
> v4:
> Only Patch 4 is modified:
> Fixed a line over 80 characters warning by checkpatch.pl;
> Update the changelog so that it is no more a try :-)
>
> v3:
> Only patch 4 is modified:
> Remove the special case for system freeze in scsi_bus_suspend_common
> as pointed out by Alan Stern;
> Updated some comments;
> Removed the use of typedef (*pm_callback_t)(struct device *).
>
> v2:
> Change the runtime suspend behaviour of sd driver by putting the device
> into stopped power state.
> Revert 2 patches which are no longer needed as pointed out by Alan Stern.
> Find out device callbacks in bus callbacks as suggested by Alan Stern.
>
> Due to these changes, patch number grows from 2 -> 5.
>
> v1:
> The 2 patches will migrate SCSI drivers to use the pm callbacks defined
> in dev_pm_ops as pm_message is deprecated and should not be used by driver.
> Bus level callback is changed to use callbacks defined in dev_pm_ops when
> needed and sd's pm callback is updated to use what are defined in dev_pm_ops.
>
>
> Aaron Lu (5):
> sd: put to stopped power state when runtime suspend
> Revert "[SCSI] scsi_pm: set device runtime state before parent
> suspended"
> Revert "[SCSI] runtime resume parent for child's system-resume"
> pm: use callbacks from dev_pm_ops for scsi devices
> sd: update sd to use the new pm callbacks
>
> drivers/scsi/scsi_pm.c | 98 +++++++++++++++++++++++++++-----------------------
> drivers/scsi/sd.c | 18 +++++++---
> 2 files changed, 67 insertions(+), 49 deletions(-)
Do you have any plans with respect to this patchset?
It has been acked by Alan and me, do you have any objections against it?
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH 250/493] cpufreq: remove use of __devinit
From: Rafael J. Wysocki @ 2012-11-21 0:50 UTC (permalink / raw)
To: Bill Pemberton; +Cc: gregkh, cpufreq, linux-pm
In-Reply-To: <1353349642-3677-250-git-send-email-wfp5p@virginia.edu>
On Monday, November 19, 2012 01:23:19 PM Bill Pemberton wrote:
> CONFIG_HOTPLUG is going away as an option so __devinit is no longer
> needed.
Applied to the linux-next branch of the linux-pm.git tree as v3.8 material.
Thanks,
Rafael
> Signed-off-by: Bill Pemberton <wfp5p@virginia.edu>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: cpufreq@vger.kernel.org
> Cc: linux-pm@vger.kernel.org
> ---
> drivers/cpufreq/cpufreq-cpu0.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
> index e915827..52bf36d 100644
> --- a/drivers/cpufreq/cpufreq-cpu0.c
> +++ b/drivers/cpufreq/cpufreq-cpu0.c
> @@ -174,7 +174,7 @@ static struct cpufreq_driver cpu0_cpufreq_driver = {
> .attr = cpu0_cpufreq_attr,
> };
>
> -static int __devinit cpu0_cpufreq_driver_init(void)
> +static int cpu0_cpufreq_driver_init(void)
> {
> struct device_node *np;
> int ret;
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PULL REQUEST for Rafael] PM / devfreq: pull request
From: Rafael J. Wysocki @ 2012-11-21 0:48 UTC (permalink / raw)
To: MyungJoo Ham
Cc: linux-pm, myungjoo.ham, nm, rajagopal.venkat, kyungmin.park,
sh007.yi, jonghwa3.lee, sachin.kamat
In-Reply-To: <1353397850-21949-1-git-send-email-myungjoo.ham@samsung.com>
On Tuesday, November 20, 2012 04:50:50 PM MyungJoo Ham wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> The following changes since commit 77b67063bb6bce6d475e910d3b886a606d0d91f7:
>
> Linux 3.7-rc5 (2012-11-11 13:44:33 +0100)
>
> are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/mzx/devfreq.git tags/pull_req_20121120
> , which is based on Rafael's linux-pm.git pm-devfreq.
>
> Jonghwa Lee (1):
> PM / devfreq: Add sysfs node for representing frequency transition information.
>
> Nishanth Menon (12):
> PM / devfreq: kernel-doc typo corrections
> PM / devfreq: fix sscanf handling for writable sysfs entries
> PM / devfreq: make devfreq_class static
> PM / devfreq: documentation cleanups for devfreq header
> PM / devfreq: Add sysfs node to expose available frequencies
> PM / devfreq: export update_devfreq
> PM / devfreq: provide hooks for governors to be registered
> PM / devfreq: register governors with devfreq framework
> PM / devfreq: map devfreq drivers to governor using name
> PM / devfreq: governors: add GPL module license and allow module build
> PM / devfreq: allow sysfs governor node to switch governor
> PM / devfreq: Add sysfs node to expose available governors
>
> Rajagopal Venkat (3):
> PM / devfreq: Core updates to support devices which can idle
> PM / devfreq: Add suspend and resume apis
> PM / devfreq: Add current freq callback in device profile
>
> Sachin Kamat (1):
> PM / devfreq: Use devm_* functions in exynos4_bus.c
>
> Sangho Yi (1):
> PM / devfreq: exynos4_bus.c: Fixed an alignment of the func call args.
Pulled, thanks a lot MyungJoo!
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH] cpuidle: Measure idle state durations with monotonic clock
From: Rafael J. Wysocki @ 2012-11-21 0:17 UTC (permalink / raw)
To: Len Brown, Len Brown
Cc: Julius Werner, linux-kernel, Kevin Hilman, Andrew Morton,
Srivatsa S. Bhat, linux-acpi, linux-pm, linuxppc-dev,
Deepthi Dharwar, Trinabh Gupta, Sameer Nanda, Lists Linaro-dev,
Daniel Lezcano
In-Reply-To: <1352944590-8776-1-git-send-email-jwerner@chromium.org>
On Wednesday, November 14, 2012 05:56:30 PM Julius Werner wrote:
> Many cpuidle drivers measure their time spent in an idle state by
> reading the wallclock time before and after idling and calculating the
> difference. This leads to erroneous results when the wallclock time gets
> updated by another processor in the meantime, adding that clock
> adjustment to the idle state's time counter.
>
> If the clock adjustment was negative, the result is even worse due to an
> erroneous cast from int to unsigned long long of the last_residency
> variable. The negative 32 bit integer will zero-extend and result in a
> forward time jump of roughly four billion milliseconds or 1.3 hours on
> the idle state residency counter.
>
> This patch changes all affected cpuidle drivers to either use the
> monotonic clock for their measurements or make use of the generic time
> measurement wrapper in cpuidle.c, which was already working correctly.
> Some superfluous CLIs/STIs in the ACPI code are removed (interrupts
> should always already be disabled before entering the idle function, and
> not get reenabled until the generic wrapper has performed its second
> measurement). It also removes the erroneous cast, making sure that
> negative residency values are applied correctly even though they should
> not appear anymore.
>
> Signed-off-by: Julius Werner <jwerner@chromium.org>
Len, I need your ACK on intel_idle change in this patch if you agree with it.
Thanks,
Rafael
> ---
> arch/powerpc/platforms/pseries/processor_idle.c | 4 +-
> drivers/acpi/processor_idle.c | 57 +---------------------
> drivers/cpuidle/cpuidle.c | 3 +-
> drivers/idle/intel_idle.c | 14 +-----
> 4 files changed, 7 insertions(+), 71 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/processor_idle.c b/arch/powerpc/platforms/pseries/processor_idle.c
> index 45d00e5..4d806b4 100644
> --- a/arch/powerpc/platforms/pseries/processor_idle.c
> +++ b/arch/powerpc/platforms/pseries/processor_idle.c
> @@ -36,7 +36,7 @@ static struct cpuidle_state *cpuidle_state_table;
> static inline void idle_loop_prolog(unsigned long *in_purr, ktime_t *kt_before)
> {
>
> - *kt_before = ktime_get_real();
> + *kt_before = ktime_get();
> *in_purr = mfspr(SPRN_PURR);
> /*
> * Indicate to the HV that we are idle. Now would be
> @@ -50,7 +50,7 @@ static inline s64 idle_loop_epilog(unsigned long in_purr, ktime_t kt_before)
> get_lppaca()->wait_state_cycles += mfspr(SPRN_PURR) - in_purr;
> get_lppaca()->idle = 0;
>
> - return ktime_to_us(ktime_sub(ktime_get_real(), kt_before));
> + return ktime_to_us(ktime_sub(ktime_get(), kt_before));
> }
>
> static int snooze_loop(struct cpuidle_device *dev,
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index e8086c7..f1a5da4 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -735,31 +735,18 @@ static inline void acpi_idle_do_entry(struct acpi_processor_cx *cx)
> static int acpi_idle_enter_c1(struct cpuidle_device *dev,
> struct cpuidle_driver *drv, int index)
> {
> - ktime_t kt1, kt2;
> - s64 idle_time;
> struct acpi_processor *pr;
> struct cpuidle_state_usage *state_usage = &dev->states_usage[index];
> struct acpi_processor_cx *cx = cpuidle_get_statedata(state_usage);
>
> pr = __this_cpu_read(processors);
> - dev->last_residency = 0;
>
> if (unlikely(!pr))
> return -EINVAL;
>
> - local_irq_disable();
> -
> -
> lapic_timer_state_broadcast(pr, cx, 1);
> - kt1 = ktime_get_real();
> acpi_idle_do_entry(cx);
> - kt2 = ktime_get_real();
> - idle_time = ktime_to_us(ktime_sub(kt2, kt1));
> -
> - /* Update device last_residency*/
> - dev->last_residency = (int)idle_time;
>
> - local_irq_enable();
> lapic_timer_state_broadcast(pr, cx, 0);
>
> return index;
> @@ -806,19 +793,12 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev,
> struct acpi_processor *pr;
> struct cpuidle_state_usage *state_usage = &dev->states_usage[index];
> struct acpi_processor_cx *cx = cpuidle_get_statedata(state_usage);
> - ktime_t kt1, kt2;
> - s64 idle_time_ns;
> - s64 idle_time;
>
> pr = __this_cpu_read(processors);
> - dev->last_residency = 0;
>
> if (unlikely(!pr))
> return -EINVAL;
>
> - local_irq_disable();
> -
> -
> if (cx->entry_method != ACPI_CSTATE_FFH) {
> current_thread_info()->status &= ~TS_POLLING;
> /*
> @@ -829,7 +809,6 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev,
>
> if (unlikely(need_resched())) {
> current_thread_info()->status |= TS_POLLING;
> - local_irq_enable();
> return -EINVAL;
> }
> }
> @@ -843,22 +822,12 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev,
> if (cx->type == ACPI_STATE_C3)
> ACPI_FLUSH_CPU_CACHE();
>
> - kt1 = ktime_get_real();
> /* Tell the scheduler that we are going deep-idle: */
> sched_clock_idle_sleep_event();
> acpi_idle_do_entry(cx);
> - kt2 = ktime_get_real();
> - idle_time_ns = ktime_to_ns(ktime_sub(kt2, kt1));
> - idle_time = idle_time_ns;
> - do_div(idle_time, NSEC_PER_USEC);
>
> - /* Update device last_residency*/
> - dev->last_residency = (int)idle_time;
> + sched_clock_idle_wakeup_event(0);
>
> - /* Tell the scheduler how much we idled: */
> - sched_clock_idle_wakeup_event(idle_time_ns);
> -
> - local_irq_enable();
> if (cx->entry_method != ACPI_CSTATE_FFH)
> current_thread_info()->status |= TS_POLLING;
>
> @@ -883,13 +852,8 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
> struct acpi_processor *pr;
> struct cpuidle_state_usage *state_usage = &dev->states_usage[index];
> struct acpi_processor_cx *cx = cpuidle_get_statedata(state_usage);
> - ktime_t kt1, kt2;
> - s64 idle_time_ns;
> - s64 idle_time;
> -
>
> pr = __this_cpu_read(processors);
> - dev->last_residency = 0;
>
> if (unlikely(!pr))
> return -EINVAL;
> @@ -899,16 +863,11 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
> return drv->states[drv->safe_state_index].enter(dev,
> drv, drv->safe_state_index);
> } else {
> - local_irq_disable();
> acpi_safe_halt();
> - local_irq_enable();
> return -EBUSY;
> }
> }
>
> - local_irq_disable();
> -
> -
> if (cx->entry_method != ACPI_CSTATE_FFH) {
> current_thread_info()->status &= ~TS_POLLING;
> /*
> @@ -919,7 +878,6 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
>
> if (unlikely(need_resched())) {
> current_thread_info()->status |= TS_POLLING;
> - local_irq_enable();
> return -EINVAL;
> }
> }
> @@ -934,7 +892,6 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
> */
> lapic_timer_state_broadcast(pr, cx, 1);
>
> - kt1 = ktime_get_real();
> /*
> * disable bus master
> * bm_check implies we need ARB_DIS
> @@ -965,18 +922,9 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
> c3_cpu_count--;
> raw_spin_unlock(&c3_lock);
> }
> - kt2 = ktime_get_real();
> - idle_time_ns = ktime_to_ns(ktime_sub(kt2, kt1));
> - idle_time = idle_time_ns;
> - do_div(idle_time, NSEC_PER_USEC);
> -
> - /* Update device last_residency*/
> - dev->last_residency = (int)idle_time;
>
> - /* Tell the scheduler how much we idled: */
> - sched_clock_idle_wakeup_event(idle_time_ns);
> + sched_clock_idle_wakeup_event(0);
>
> - local_irq_enable();
> if (cx->entry_method != ACPI_CSTATE_FFH)
> current_thread_info()->status |= TS_POLLING;
>
> @@ -987,6 +935,7 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
> struct cpuidle_driver acpi_idle_driver = {
> .name = "acpi_idle",
> .owner = THIS_MODULE,
> + .en_core_tk_irqen = 1,
> };
>
> /**
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 7f15b85..1536edd 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -109,8 +109,7 @@ int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv,
> /* This can be moved to within driver enter routine
> * but that results in multiple copies of same code.
> */
> - dev->states_usage[entered_state].time +=
> - (unsigned long long)dev->last_residency;
> + dev->states_usage[entered_state].time += dev->last_residency;
> dev->states_usage[entered_state].usage++;
> } else {
> dev->last_residency = 0;
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index b0f6b4c..c49c04d 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -56,7 +56,6 @@
> #include <linux/kernel.h>
> #include <linux/cpuidle.h>
> #include <linux/clockchips.h>
> -#include <linux/hrtimer.h> /* ktime_get_real() */
> #include <trace/events/power.h>
> #include <linux/sched.h>
> #include <linux/notifier.h>
> @@ -72,6 +71,7 @@
> static struct cpuidle_driver intel_idle_driver = {
> .name = "intel_idle",
> .owner = THIS_MODULE,
> + .en_core_tk_irqen = 1,
> };
> /* intel_idle.max_cstate=0 disables driver */
> static int max_cstate = MWAIT_MAX_NUM_CSTATES - 1;
> @@ -281,8 +281,6 @@ static int intel_idle(struct cpuidle_device *dev,
> struct cpuidle_state_usage *state_usage = &dev->states_usage[index];
> unsigned long eax = (unsigned long)cpuidle_get_statedata(state_usage);
> unsigned int cstate;
> - ktime_t kt_before, kt_after;
> - s64 usec_delta;
> int cpu = smp_processor_id();
>
> cstate = (((eax) >> MWAIT_SUBSTATE_SIZE) & MWAIT_CSTATE_MASK) + 1;
> @@ -297,8 +295,6 @@ static int intel_idle(struct cpuidle_device *dev,
> if (!(lapic_timer_reliable_states & (1 << (cstate))))
> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu);
>
> - kt_before = ktime_get_real();
> -
> stop_critical_timings();
> if (!need_resched()) {
>
> @@ -310,17 +306,9 @@ static int intel_idle(struct cpuidle_device *dev,
>
> start_critical_timings();
>
> - kt_after = ktime_get_real();
> - usec_delta = ktime_to_us(ktime_sub(kt_after, kt_before));
> -
> - local_irq_enable();
> -
> if (!(lapic_timer_reliable_states & (1 << (cstate))))
> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu);
>
> - /* Update cpuidle counters */
> - dev->last_residency = (int)usec_delta;
> -
> return index;
> }
>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCHv9 0/3] Runtime Interpreted Power Sequences
From: Grant Likely @ 2012-11-20 21:58 UTC (permalink / raw)
To: Anton Vorontsov, Stephen Warren, Thierry Reding, Mark Zhang,
Rob Herring, Mark Brown, David Woodhouse, Arnd Bergmann
Cc: linux-tegra, linux-arm-kernel, linux-kernel, linux-fbdev,
devicetree-discuss, linux-pm, Alexandre Courbot,
Alexandre Courbot
In-Reply-To: <1353149747-31871-1-git-send-email-acourbot@nvidia.com>
On Sat, 17 Nov 2012 19:55:44 +0900, Alexandre Courbot <acourbot@nvidia.com> wrote:
> Apologies for sending two patchsets in two days - the main purpose
> of this new revision is to add the linux-arm-kernel list to the
> audience. A few suggestions from v8 have also been added.
Just to be clear from my previous reply, don't merge this series. The DT
binding is not good.
g.
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Grant Likely @ 2012-11-20 21:54 UTC (permalink / raw)
To: Anton Vorontsov, Stephen Warren, Thierry Reding, Mark Zhang,
Rob Herring, Mark Brown, David Woodhouse, Arnd Bergmann
Cc: linux-tegra, linux-arm-kernel, linux-kernel, linux-fbdev,
devicetree-discuss, linux-pm, Alexandre Courbot,
Alexandre Courbot
In-Reply-To: <1353149747-31871-2-git-send-email-acourbot@nvidia.com>
On Sat, 17 Nov 2012 19:55:45 +0900, Alexandre Courbot <acourbot@nvidia.com> wrote:
> Some device drivers (e.g. panel or backlights) need to follow precise
> sequences for powering on and off, involving GPIOs, regulators, PWMs
> and other power-related resources, with a defined powering order and
> delays to respect between steps. These sequences are device-specific,
> and do not belong to a particular driver - therefore they have been
> performed by board-specific hook functions so far.
I must be honest, this series really makes me nervous...
> With the advent of the device tree and of ARM kernels that are not
> board-tied, we cannot rely on these board-specific hooks anymore but
This isn't strictly true. It is still perfectly fine to have board
specific code when necessary. However, there is strong encouragement to
enable that code in device drivers as much as possible and new board
files need to have very strong justification.
> need a way to implement these sequences in a portable manner. This patch
> introduces a simple interpreter that can execute such power sequences
> encoded either as platform data or within the device tree. It also
> introduces first support for regulator, GPIO and PWM resources.
This is where I start getting nervous. Up to now I've strongly resisted
adding any kind of interpreted code to the device tree. The model is to
identify hardware, but require the driver to know how to control it. (as
compared to ACPI which is entirely designed around executable
bytecode).
While the power sequences described here certainly cannot be confused
with a Turing complete bytecode, it is a step in that direction. Plus,
there will always be that new use case that needs just a "little new
feature" to make this work for that too. Without thinking about how to
handle that ahead of time is just asking for something to turn into a
maintenance nightmare. It's just as important to specify what the limits
of this approach are and when to punt to real driver code to handle a
device.
> +Power Sequences Steps
> +---------------------
> +Steps of a sequence describe an action to be performed on a resource. They
> +always include a "type" property which indicates what kind of resource this
> +step works on. Depending on the resource type, additional properties are defined
> +to control the action to be performed.
> +
> +"delay" type required properties:
> + - delay: delay to wait (in microseconds)
> +
> +"regulator" type required properties:
> + - id: name of the regulator to use.
> + - enable / disable: one of these two empty properties must be present to
> + enable or disable the resource
> +
> +"pwm" type required properties:
> + - id: name of the PWM to use.
> + - enable / disable: one of these two empty properties must be present to
> + enable or disable the resource
> +
> +"gpio" type required properties:
> + - gpio: phandle of the GPIO to use.
> + - value: value this GPIO should take. Must be 0 or 1.
> +
> +Example
> +-------
> +Here are example sequences declared within a backlight device that use all the
> +supported resources types:
> +
> + backlight {
> + compatible = "pwm-backlight";
> + ...
> +
> + /* resources used by the power sequences */
> + pwms = <&pwm 2 5000000>;
> + pwm-names = "backlight";
> + power-supply = <&backlight_reg>;
> +
> + power-sequences {
> + power-on {
> + step0 {
> + type = "regulator";
> + id = "power";
> + enable;
> + };
> + step1 {
> + type = "delay";
> + delay = <10000>;
> + };
> + step2 {
> + type = "pwm";
> + id = "backlight";
> + enable;
> + };
> + step3 {
> + type = "gpio";
> + gpio = <&gpio 28 0>;
> + value = <1>;
> + };
> + };
> +
> + power-off {
> + step0 {
> + type = "gpio";
> + gpio = <&gpio 28 0>;
> + value = <0>;
> + };
> + step1 {
> + type = "pwm";
> + id = "backlight";
> + disable;
> + };
> + step2 {
> + type = "delay";
> + delay = <10000>;
> + };
> + step3 {
> + type = "regulator";
> + id = "power";
> + disable;
> + };
> + };
> + };
> + };
I think this will get very verbose in a hurry. Already this simple
example is 45 lines long. Using the device tree structure to encode the
language doesn't look like a very good fit. Not to mention that the
order of operations is entirely based on the node name. Want to insert
an operation between step0 and step1? Need to rename step1, step2, and
step3 to do so.
This implementation also isn't very consistent. The gpio is referenced
with a phandle in step3/step0, but the regulator and pwm are referenced
by id.
As an alternative, what about something like the following?
backlight {
compatible = "pwm-backlight";
...
/* resources used by the power sequences */
pwms = <&pwm 2 5000000>;
pwm-names = "backlight";
regulators = <&backlight_reg>;
gpios = <&gpio 28 0>;
power-on-sequence = "r0e;d10000m;p0e;g0s";
power-off-sequence = "g0c;p0d;d10000m;r0d";
};
I'm thinking about the scripts as trivial-to-parse ascii strings that
have a very simple set of commands. The commands use resources already
defined in the node. ie. 'g0' refers to the first entry in the gpios
property. 'r0' for the regulator, 'p0' for the pwms, 'd' means delay. By
no means take this as the format to use, it is just an example off the
top of my head, but it is already way easier to work with than putting
each command into a node.
The trick is still to define a syntax that doesn't fall apart when it
needs to be extended. I would also like to get opinions on whether or
not conditionals or loops should be supported (ie. loop waiting for a
status to change). If they should then we need to be a lot more careful
about the design (and due to my aforementioned nervousness, somebody may
need to get me therapy).
(Mitch, I'll let you make the argument for using Forth here. To be
honest, I'm not keen on defining any kind of new language, however
simple, but neither am I keen to pull in Forth).
> +Platform Data Format
> +--------------------
> +All relevant data structures for declaring power sequences are located in
> +include/linux/power_seq.h.
Hmm... If sequences are switched to a string instead, then platform_data
should also use it. The power sequence data structures can be created at
runtime by parsing the string.
g.
^ permalink raw reply
* Re: [PATCH] PCI/PM: Keep runtime PM enabled for unbound PCI devices
From: Rafael J. Wysocki @ 2012-11-20 21:38 UTC (permalink / raw)
To: Huang Ying
Cc: Bjorn Helgaas, linux-kernel, linux-pci, linux-pm,
Rafael J. Wysocki, Alan Stern
In-Reply-To: <1353398902-21253-1-git-send-email-ying.huang@intel.com>
On Tuesday, November 20, 2012 04:08:22 PM Huang Ying wrote:
> For unbound PCI devices, what we need is:
>
> - Always in D0 state, because some devices does not work again after
> being put into D3 by the PCI bus.
>
> - In SUSPENDED state if allowed, so that the parent devices can still
> be put into low power state.
>
> To satisfy these requirement, the runtime PM for the unbound PCI
> devices are disabled and set to SUSPENDED state. One issue of this
> solution is that the PCI devices will be put into SUSPENDED state even
> if the SUSPENDED state is forbidden via the sysfs interface
> (.../power/control) of the device. This is not an issue for most
> devices, because most PCI devices are not used at all if unbounded.
> But there are exceptions. For example, unbound VGA card can be used
> for display, but suspend its parents make it stop working.
>
> To fix the issue, we keep the runtime PM enabled when the PCI devices
> are unbound. But the runtime PM callbacks will do nothing if the PCI
> devices are unbound. This way, we can put the PCI devices into
> SUSPENDED state without put the PCI devices into D3 state.
>
> Signed-off-by: Huang Ying <ying.huang@intel.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> CC: stable@vger.kernel.org # v3.6+
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/pci/pci-driver.c | 69 +++++++++++++++++++++++++++--------------------
> drivers/pci/pci.c | 2 +
> 2 files changed, 43 insertions(+), 28 deletions(-)
>
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -1900,6 +1900,8 @@ void pci_pm_init(struct pci_dev *dev)
> u16 pmc;
>
> pm_runtime_forbid(&dev->dev);
> + pm_runtime_set_active(&dev->dev);
> + pm_runtime_enable(&dev->dev);
> device_enable_async_suspend(&dev->dev);
> dev->wakeup_prepared = false;
>
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -256,31 +256,26 @@ struct drv_dev_and_id {
> static long local_pci_probe(void *_ddi)
> {
> struct drv_dev_and_id *ddi = _ddi;
> - struct device *dev = &ddi->dev->dev;
> - struct device *parent = dev->parent;
> + struct pci_dev *pci_dev = ddi->dev;
> + struct pci_driver *pci_drv = ddi->drv;
> + struct device *dev = &pci_dev->dev;
> int rc;
>
> - /* The parent bridge must be in active state when probing */
> - if (parent)
> - pm_runtime_get_sync(parent);
> - /* Unbound PCI devices are always set to disabled and suspended.
> - * During probe, the device is set to enabled and active and the
> - * usage count is incremented. If the driver supports runtime PM,
> - * it should call pm_runtime_put_noidle() in its probe routine and
> - * pm_runtime_get_noresume() in its remove routine.
> - */
> - pm_runtime_get_noresume(dev);
> - pm_runtime_set_active(dev);
> - pm_runtime_enable(dev);
> -
> - rc = ddi->drv->probe(ddi->dev, ddi->id);
> + /*
> + * Unbound PCI devices are always put in D0, regardless of
> + * runtime PM status. During probe, the device is set to
> + * active and the usage count is incremented. If the driver
> + * supports runtime PM, it should call pm_runtime_put_noidle()
> + * in its probe routine and pm_runtime_get_noresume() in its
> + * remove routine.
> + */
> + pm_runtime_get_sync(dev);
> + pci_dev->driver = pci_drv;
> + rc = pci_drv->probe(pci_dev, ddi->id);
> if (rc) {
> - pm_runtime_disable(dev);
> - pm_runtime_set_suspended(dev);
> - pm_runtime_put_noidle(dev);
> + pci_dev->driver = NULL;
> + pm_runtime_put_sync(dev);
> }
> - if (parent)
> - pm_runtime_put(parent);
> return rc;
> }
>
> @@ -330,10 +325,8 @@ __pci_device_probe(struct pci_driver *dr
> id = pci_match_device(drv, pci_dev);
> if (id)
> error = pci_call_probe(drv, pci_dev, id);
> - if (error >= 0) {
> - pci_dev->driver = drv;
> + if (error >= 0)
> error = 0;
> - }
> }
> return error;
> }
> @@ -369,9 +362,7 @@ static int pci_device_remove(struct devi
> }
>
> /* Undo the runtime PM settings in local_pci_probe() */
> - pm_runtime_disable(dev);
> - pm_runtime_set_suspended(dev);
> - pm_runtime_put_noidle(dev);
> + pm_runtime_put_sync(dev);
>
> /*
> * If the device is still on, set the power state as "unknown",
> @@ -994,6 +985,13 @@ static int pci_pm_runtime_suspend(struct
> pci_power_t prev = pci_dev->current_state;
> int error;
>
> + /*
> + * If pci_dev->driver is not set (unbound), the device should
> + * always remain in D0 regardless of the runtime PM status
> + */
> + if (!pci_dev->driver)
> + return 0;
> +
> if (!pm || !pm->runtime_suspend)
> return -ENOSYS;
>
> @@ -1029,6 +1027,13 @@ static int pci_pm_runtime_resume(struct
> struct pci_dev *pci_dev = to_pci_dev(dev);
> const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
>
> + /*
> + * If pci_dev->driver is not set (unbound), the device should
> + * always remain in D0 regardless of the runtime PM status
> + */
> + if (!pci_dev->driver)
> + return 0;
> +
> if (!pm || !pm->runtime_resume)
> return -ENOSYS;
>
> @@ -1046,8 +1051,16 @@ static int pci_pm_runtime_resume(struct
>
> static int pci_pm_runtime_idle(struct device *dev)
> {
> + struct pci_dev *pci_dev = to_pci_dev(dev);
> const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
>
> + /*
> + * If pci_dev->driver is not set (unbound), the device should
> + * always remain in D0 regardless of the runtime PM status
> + */
> + if (!pci_dev->driver)
> + goto out;
> +
> if (!pm)
> return -ENOSYS;
>
> @@ -1057,8 +1070,8 @@ static int pci_pm_runtime_idle(struct de
> return ret;
> }
>
> +out:
> pm_runtime_suspend(dev);
> -
> return 0;
> }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH] PCI/PM: Keep runtime PM enabled for unbound PCI devices
From: Alan Stern @ 2012-11-20 16:04 UTC (permalink / raw)
To: Huang Ying
Cc: Bjorn Helgaas, linux-kernel, linux-pci, linux-pm,
Rafael J. Wysocki, Rafael J. Wysocki
In-Reply-To: <1353398902-21253-1-git-send-email-ying.huang@intel.com>
On Tue, 20 Nov 2012, Huang Ying wrote:
> For unbound PCI devices, what we need is:
>
> - Always in D0 state, because some devices does not work again after
> being put into D3 by the PCI bus.
>
> - In SUSPENDED state if allowed, so that the parent devices can still
> be put into low power state.
>
> To satisfy these requirement, the runtime PM for the unbound PCI
> devices are disabled and set to SUSPENDED state. One issue of this
> solution is that the PCI devices will be put into SUSPENDED state even
> if the SUSPENDED state is forbidden via the sysfs interface
> (.../power/control) of the device. This is not an issue for most
> devices, because most PCI devices are not used at all if unbounded.
> But there are exceptions. For example, unbound VGA card can be used
> for display, but suspend its parents make it stop working.
>
> To fix the issue, we keep the runtime PM enabled when the PCI devices
> are unbound. But the runtime PM callbacks will do nothing if the PCI
> devices are unbound. This way, we can put the PCI devices into
> SUSPENDED state without put the PCI devices into D3 state.
Acked-by: Alan Stern <stern@rowland.harvard.edu>
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-11-20 14:48 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Anton Vorontsov, Stephen Warren, Thierry Reding, Mark Zhang,
Grant Likely, Rob Herring, Mark Brown, David Woodhouse,
Arnd Bergmann, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Alexandre Courbot
In-Reply-To: <1353149747-31871-2-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1623 bytes --]
Hi,
On 2012-11-17 12:55, Alexandre Courbot wrote:
A few questions after looking at the documentation:
> +Example
> +-------
> +Here are example sequences declared within a backlight device that use all the
> +supported resources types:
> +
> + backlight {
> + compatible = "pwm-backlight";
> + ...
> +
> + /* resources used by the power sequences */
> + pwms = <&pwm 2 5000000>;
> + pwm-names = "backlight";
> + power-supply = <&backlight_reg>;
> +
> + power-sequences {
> + power-on {
> + step0 {
> + type = "regulator";
> + id = "power";
> + enable;
> + };
> + step1 {
> + type = "delay";
> + delay = <10000>;
> + };
> + step2 {
> + type = "pwm";
> + id = "backlight";
> + enable;
> + };
> + step3 {
> + type = "gpio";
> + gpio = <&gpio 28 0>;
> + value = <1>;
> + };
> + };
I guess there's a reason, but the above looks a bit inconsistent. For
gpio you define the gpio resource inside the step. For power and pwm the
resource is defined before the steps. Why wouldn't "pwm = <&pwm 2
5000000>;" work in step2?
> +When a power sequence is run, its steps is executed one after the other until
> +one step fails or the end of the sequence is reached.
The document doesn't give any hint of what the driver should do if
running the power sequence fails. Run the "opposite" power sequence?
Will that work for all resources? I'm mainly thinking of a case where
each enable of the resource should be matched by a disable, i.e. you
can't call disable if no enable was called.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply
* [PATCH v3] cpufreq: ondemand: handle SW coordinated CPUs
From: Fabio Baltieri @ 2012-11-20 12:06 UTC (permalink / raw)
To: Rafael J. Wysocki, cpufreq, linux-pm
Cc: Rickard Andersson, Vincent Guittot, Linus Walleij, Lee Jones,
linux-kernel, Fabio Baltieri
Hi all,
this patch fixes an issue with the ondemand governor, which currently
does not handle properly software coordinated CPUs - i.e. CPUs groups
with a common clock and a single governor, as in the u8500:
root@genericarmv7a:~# cpufreq-info -d
DB8500
root@genericarmv7a:~# cpufreq-info -a
0 1
In this case, with a non-patched kernel, if a process is loading only
the secondary CPU while the first one is idle, the ondemand governor may
take a long time before firing up the load sampling routine
(dbs_check_cpu()), leaving both cores running at minimum frequency even
if one of them is fully loaded.
The problem can be reproduced with standard utils, as in:
root@genericarmv7a:~# cpufreq-info --cpu 1 -f
200000
root@genericarmv7a:~# taskset 2 yes > /dev/null &
root@genericarmv7a:~# sleep 3
root@genericarmv7a:~# cpufreq-info --cpu 1 -f
200000
while there is no other process loading the other core - here I'm using
a minimal oe-core image.
To fix the problem, this patch modifies the governor to use an
individual deferrable work for each CPU, instead that just one for the
main one.
This patch has been tested on an U8500 system (dual cortex-A9) and on a
standard x86_64 dual-core laptop.
The patch is based on the original one by Rickard Andersson, developed
for ST-Ericsson and posted some months ago on the list, hence I'm
tagging as "v3" to avoid confusion.
Regards,
Fabio
---
Rickard Andersson (1):
cpufreq: ondemand: handle SW coordinated CPUs
drivers/cpufreq/cpufreq_ondemand.c | 141 ++++++++++++++++++++++++++++++++-----
1 file changed, 122 insertions(+), 19 deletions(-)
--
1.7.12.1
^ permalink raw reply
* [PATCH v3] cpufreq: ondemand: handle SW coordinated CPUs
From: Fabio Baltieri @ 2012-11-20 12:06 UTC (permalink / raw)
To: Rafael J. Wysocki, cpufreq, linux-pm
Cc: Rickard Andersson, Vincent Guittot, Linus Walleij, Lee Jones,
linux-kernel, Fabio Baltieri
In-Reply-To: <1353413176-21723-1-git-send-email-fabio.baltieri@linaro.org>
From: Rickard Andersson <rickard.andersson@stericsson.com>
This patch fixes a bug that occurred when we had load on a secondary CPU
and the primary CPU was sleeping. Only one sampling timer was spawned
and it was spawned as a deferred timer on the primary CPU, so when a
secondary CPU had a change in load this was not detected by the ondemand
governor.
This patch make sure that deferred timers are run on all CPUs in the
case of software controlled CPUs that run on the same frequency.
Signed-off-by: Rickard Andersson <rickard.andersson@stericsson.com>
Signed-off-by: Fabio Baltieri <fabio.baltieri@linaro.org>
---
drivers/cpufreq/cpufreq_ondemand.c | 141 ++++++++++++++++++++++++++++++++-----
1 file changed, 122 insertions(+), 19 deletions(-)
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 396322f..430f614 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -93,6 +93,7 @@ struct cpu_dbs_info_s {
* when user is changing the governor or limits.
*/
struct mutex timer_mutex;
+ ktime_t time_stamp;
};
static DEFINE_PER_CPU(struct cpu_dbs_info_s, od_cpu_dbs_info);
@@ -285,7 +286,7 @@ static void update_sampling_rate(unsigned int new_rate)
policy = cpufreq_cpu_get(cpu);
if (!policy)
continue;
- dbs_info = &per_cpu(od_cpu_dbs_info, policy->cpu);
+ dbs_info = &per_cpu(od_cpu_dbs_info, cpu);
cpufreq_cpu_put(policy);
mutex_lock(&dbs_info->timer_mutex);
@@ -305,7 +306,7 @@ static void update_sampling_rate(unsigned int new_rate)
cancel_delayed_work_sync(&dbs_info->work);
mutex_lock(&dbs_info->timer_mutex);
- schedule_delayed_work_on(dbs_info->cpu, &dbs_info->work,
+ schedule_delayed_work_on(cpu, &dbs_info->work,
usecs_to_jiffies(new_rate));
}
@@ -449,6 +450,16 @@ static struct attribute_group dbs_attr_group = {
/************************** sysfs end ************************/
+static bool dbs_sw_coordinated_cpus(struct cpu_dbs_info_s *dbs_info)
+{
+ struct cpufreq_policy *policy = dbs_info->cur_policy;
+
+ if (cpumask_weight(policy->cpus) > 1)
+ return true;
+ else
+ return false;
+}
+
static void dbs_freq_increase(struct cpufreq_policy *p, unsigned int freq)
{
if (dbs_tuners_ins.powersave_bias)
@@ -598,20 +609,41 @@ static void dbs_check_cpu(struct cpu_dbs_info_s *this_dbs_info)
static void do_dbs_timer(struct work_struct *work)
{
+ struct delayed_work *dw = to_delayed_work(work);
struct cpu_dbs_info_s *dbs_info =
container_of(work, struct cpu_dbs_info_s, work.work);
- unsigned int cpu = dbs_info->cpu;
- int sample_type = dbs_info->sample_type;
-
+ int sample_type;
int delay;
+ bool sample = true;
+
+ if (dbs_sw_coordinated_cpus(dbs_info)) {
+ ktime_t time_now;
+ s64 delta_us;
+
+ /* use leader CPU's dbs_info */
+ dbs_info = &per_cpu(od_cpu_dbs_info, dbs_info->cpu);
+ mutex_lock(&dbs_info->timer_mutex);
- mutex_lock(&dbs_info->timer_mutex);
+ time_now = ktime_get();
+ delta_us = ktime_us_delta(time_now, dbs_info->time_stamp);
+
+ /* Do nothing if we recently have sampled */
+ if (delta_us < (s64)(dbs_tuners_ins.sampling_rate / 2))
+ sample = false;
+ else
+ dbs_info->time_stamp = time_now;
+ } else {
+ mutex_lock(&dbs_info->timer_mutex);
+ }
+
+ sample_type = dbs_info->sample_type;
/* Common NORMAL_SAMPLE setup */
dbs_info->sample_type = DBS_NORMAL_SAMPLE;
if (!dbs_tuners_ins.powersave_bias ||
sample_type == DBS_NORMAL_SAMPLE) {
- dbs_check_cpu(dbs_info);
+ if (sample)
+ dbs_check_cpu(dbs_info);
if (dbs_info->freq_lo) {
/* Setup timer for SUB_SAMPLE */
dbs_info->sample_type = DBS_SUB_SAMPLE;
@@ -627,32 +659,41 @@ static void do_dbs_timer(struct work_struct *work)
delay -= jiffies % delay;
}
} else {
- __cpufreq_driver_target(dbs_info->cur_policy,
- dbs_info->freq_lo, CPUFREQ_RELATION_H);
+ if (sample)
+ __cpufreq_driver_target(dbs_info->cur_policy,
+ dbs_info->freq_lo,
+ CPUFREQ_RELATION_H);
delay = dbs_info->freq_lo_jiffies;
}
- schedule_delayed_work_on(cpu, &dbs_info->work, delay);
+ schedule_delayed_work_on(smp_processor_id(), dw, delay);
mutex_unlock(&dbs_info->timer_mutex);
}
-static inline void dbs_timer_init(struct cpu_dbs_info_s *dbs_info)
+static inline void dbs_timer_init(struct cpu_dbs_info_s *dbs_info, int cpu)
{
/* We want all CPUs to do sampling nearly on same jiffy */
int delay = usecs_to_jiffies(dbs_tuners_ins.sampling_rate);
+ struct cpu_dbs_info_s *dbs_info_local = &per_cpu(od_cpu_dbs_info, cpu);
if (num_online_cpus() > 1)
delay -= jiffies % delay;
+ cancel_delayed_work_sync(&dbs_info_local->work);
dbs_info->sample_type = DBS_NORMAL_SAMPLE;
- INIT_DEFERRABLE_WORK(&dbs_info->work, do_dbs_timer);
- schedule_delayed_work_on(dbs_info->cpu, &dbs_info->work, delay);
+ schedule_delayed_work_on(cpu, &dbs_info_local->work, delay);
}
-static inline void dbs_timer_exit(struct cpu_dbs_info_s *dbs_info)
+static inline void dbs_timer_exit(int cpu)
{
+ struct cpu_dbs_info_s *dbs_info = &per_cpu(od_cpu_dbs_info, cpu);
cancel_delayed_work_sync(&dbs_info->work);
}
+static void dbs_timer_exit_per_cpu(struct work_struct *dummy)
+{
+ dbs_timer_exit(smp_processor_id());
+}
+
/*
* Not all CPUs want IO time to be accounted as busy; this dependson how
* efficient idling at a higher frequency/voltage is.
@@ -676,6 +717,43 @@ static int should_io_be_busy(void)
return 0;
}
+static int __cpuinit cpu_callback(struct notifier_block *nfb,
+ unsigned long action, void *hcpu)
+{
+ unsigned int cpu = (unsigned long)hcpu;
+ struct device *cpu_dev;
+ struct cpu_dbs_info_s *dbs_info;
+
+ dbs_info = &per_cpu(od_cpu_dbs_info, cpu);
+
+ /* use leader CPU's dbs_info */
+ if (dbs_sw_coordinated_cpus(dbs_info))
+ dbs_info = &per_cpu(od_cpu_dbs_info, dbs_info->cpu);
+
+ cpu_dev = get_cpu_device(cpu);
+ if (cpu_dev) {
+ switch (action) {
+ case CPU_ONLINE:
+ case CPU_ONLINE_FROZEN:
+ dbs_timer_init(dbs_info, cpu);
+ break;
+ case CPU_DOWN_PREPARE:
+ case CPU_DOWN_PREPARE_FROZEN:
+ dbs_timer_exit(cpu);
+ break;
+ case CPU_DOWN_FAILED:
+ case CPU_DOWN_FAILED_FROZEN:
+ dbs_timer_init(dbs_info, cpu);
+ break;
+ }
+ }
+ return NOTIFY_OK;
+}
+
+static struct notifier_block __refdata ondemand_cpu_notifier = {
+ .notifier_call = cpu_callback,
+};
+
static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
unsigned int event)
{
@@ -704,9 +782,13 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
if (dbs_tuners_ins.ignore_nice)
j_dbs_info->prev_cpu_nice =
kcpustat_cpu(j).cpustat[CPUTIME_NICE];
+
+ mutex_init(&j_dbs_info->timer_mutex);
+ INIT_DEFERRABLE_WORK(&j_dbs_info->work, do_dbs_timer);
+
+ j_dbs_info->rate_mult = 1;
}
this_dbs_info->cpu = cpu;
- this_dbs_info->rate_mult = 1;
ondemand_powersave_bias_init_cpu(cpu);
/*
* Start the timerschedule work, when this governor
@@ -736,21 +818,42 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
}
mutex_unlock(&dbs_mutex);
- mutex_init(&this_dbs_info->timer_mutex);
- dbs_timer_init(this_dbs_info);
+ /* If SW coordinated CPUs then register notifier */
+ if (dbs_sw_coordinated_cpus(this_dbs_info)) {
+ register_hotcpu_notifier(&ondemand_cpu_notifier);
+
+ /* Initiate timer time stamp */
+ this_dbs_info->time_stamp = ktime_get();
+
+ for_each_cpu(j, policy->cpus) {
+ struct cpu_dbs_info_s *j_dbs_info;
+
+ j_dbs_info = &per_cpu(od_cpu_dbs_info, cpu);
+ dbs_timer_init(j_dbs_info, j);
+ }
+ } else {
+ dbs_timer_init(this_dbs_info, cpu);
+ }
break;
case CPUFREQ_GOV_STOP:
- dbs_timer_exit(this_dbs_info);
+ dbs_timer_exit(cpu);
mutex_lock(&dbs_mutex);
mutex_destroy(&this_dbs_info->timer_mutex);
dbs_enable--;
mutex_unlock(&dbs_mutex);
- if (!dbs_enable)
+ if (!dbs_enable) {
sysfs_remove_group(cpufreq_global_kobject,
&dbs_attr_group);
+ if (dbs_sw_coordinated_cpus(this_dbs_info)) {
+ /* Make sure all pending timers/works are
+ * stopped. */
+ schedule_on_each_cpu(dbs_timer_exit_per_cpu);
+ unregister_hotcpu_notifier(&ondemand_cpu_notifier);
+ }
+ }
break;
case CPUFREQ_GOV_LIMITS:
--
1.7.12.1
^ permalink raw reply related
* Re: [PATCH v9 06/10] ata: zpodd: check zero power ready status
From: Aaron Lu @ 2012-11-20 8:59 UTC (permalink / raw)
To: Tejun Heo, Rafael J. Wysocki, Alan Stern
Cc: Jeff Garzik, James Bottomley, Jeff Wu, linux-ide, linux-pm,
linux-scsi, linux-acpi
In-Reply-To: <50AB1C78.9060905@intel.com>
On 11/20/2012 02:00 PM, Aaron Lu wrote:
> On 11/19/2012 10:56 PM, Tejun Heo wrote:
>> I really think we need a way for (auto)pm and event polling to talk to
>> each other so that autopm can tell event poll to sod off while pm is
>> in effect. Trying to solve this from inside libata doesn't seem
>> right. The problem, again, seems to be figuring out which hardware
>> device maps to which block device. Hmmm... Any good ideas?
>
> A possible way of doing this is using pm qos.
>
> We currently have 2 pm qos flags, NO_POWER_OFF and REMOTE_WAKEUP, and we
> can add another one: NO_POLL, use it like the following:
> 1 Set the NO_POLL pm qos flag when the underlying driver thinks it is no
> longer necessary. In the ZPODD's case, it should be set when the
> device is to be powered off;
> 2 Clear it when poll is necessary again. In the ZPODD's case, when power
> is re-gained, this flag will be cleared.
> 3 In the disk_events_workfn, check if this flag is set, if so, simply
> return.
It should be, skip calling disk->fops->check_events, but still queue the
work for next time's poll.
-Aaron
>
> The disk->driverfs_dev can be used to host the pm qos flag, ATA layer
> can access it through ata_device->sdev->sdev_gendev.
>
> Is this OK?
>
> Thanks,
> Aaron
>
^ permalink raw reply
* [PATCH] PCI/PM: Keep runtime PM enabled for unbound PCI devices
From: Huang Ying @ 2012-11-20 8:08 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-kernel, linux-pci, linux-pm, Rafael J. Wysocki, Huang Ying,
Rafael J. Wysocki, Alan Stern
For unbound PCI devices, what we need is:
- Always in D0 state, because some devices does not work again after
being put into D3 by the PCI bus.
- In SUSPENDED state if allowed, so that the parent devices can still
be put into low power state.
To satisfy these requirement, the runtime PM for the unbound PCI
devices are disabled and set to SUSPENDED state. One issue of this
solution is that the PCI devices will be put into SUSPENDED state even
if the SUSPENDED state is forbidden via the sysfs interface
(.../power/control) of the device. This is not an issue for most
devices, because most PCI devices are not used at all if unbounded.
But there are exceptions. For example, unbound VGA card can be used
for display, but suspend its parents make it stop working.
To fix the issue, we keep the runtime PM enabled when the PCI devices
are unbound. But the runtime PM callbacks will do nothing if the PCI
devices are unbound. This way, we can put the PCI devices into
SUSPENDED state without put the PCI devices into D3 state.
Signed-off-by: Huang Ying <ying.huang@intel.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
CC: stable@vger.kernel.org # v3.6+
---
drivers/pci/pci-driver.c | 69 +++++++++++++++++++++++++++--------------------
drivers/pci/pci.c | 2 +
2 files changed, 43 insertions(+), 28 deletions(-)
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1900,6 +1900,8 @@ void pci_pm_init(struct pci_dev *dev)
u16 pmc;
pm_runtime_forbid(&dev->dev);
+ pm_runtime_set_active(&dev->dev);
+ pm_runtime_enable(&dev->dev);
device_enable_async_suspend(&dev->dev);
dev->wakeup_prepared = false;
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -256,31 +256,26 @@ struct drv_dev_and_id {
static long local_pci_probe(void *_ddi)
{
struct drv_dev_and_id *ddi = _ddi;
- struct device *dev = &ddi->dev->dev;
- struct device *parent = dev->parent;
+ struct pci_dev *pci_dev = ddi->dev;
+ struct pci_driver *pci_drv = ddi->drv;
+ struct device *dev = &pci_dev->dev;
int rc;
- /* The parent bridge must be in active state when probing */
- if (parent)
- pm_runtime_get_sync(parent);
- /* Unbound PCI devices are always set to disabled and suspended.
- * During probe, the device is set to enabled and active and the
- * usage count is incremented. If the driver supports runtime PM,
- * it should call pm_runtime_put_noidle() in its probe routine and
- * pm_runtime_get_noresume() in its remove routine.
- */
- pm_runtime_get_noresume(dev);
- pm_runtime_set_active(dev);
- pm_runtime_enable(dev);
-
- rc = ddi->drv->probe(ddi->dev, ddi->id);
+ /*
+ * Unbound PCI devices are always put in D0, regardless of
+ * runtime PM status. During probe, the device is set to
+ * active and the usage count is incremented. If the driver
+ * supports runtime PM, it should call pm_runtime_put_noidle()
+ * in its probe routine and pm_runtime_get_noresume() in its
+ * remove routine.
+ */
+ pm_runtime_get_sync(dev);
+ pci_dev->driver = pci_drv;
+ rc = pci_drv->probe(pci_dev, ddi->id);
if (rc) {
- pm_runtime_disable(dev);
- pm_runtime_set_suspended(dev);
- pm_runtime_put_noidle(dev);
+ pci_dev->driver = NULL;
+ pm_runtime_put_sync(dev);
}
- if (parent)
- pm_runtime_put(parent);
return rc;
}
@@ -330,10 +325,8 @@ __pci_device_probe(struct pci_driver *dr
id = pci_match_device(drv, pci_dev);
if (id)
error = pci_call_probe(drv, pci_dev, id);
- if (error >= 0) {
- pci_dev->driver = drv;
+ if (error >= 0)
error = 0;
- }
}
return error;
}
@@ -369,9 +362,7 @@ static int pci_device_remove(struct devi
}
/* Undo the runtime PM settings in local_pci_probe() */
- pm_runtime_disable(dev);
- pm_runtime_set_suspended(dev);
- pm_runtime_put_noidle(dev);
+ pm_runtime_put_sync(dev);
/*
* If the device is still on, set the power state as "unknown",
@@ -994,6 +985,13 @@ static int pci_pm_runtime_suspend(struct
pci_power_t prev = pci_dev->current_state;
int error;
+ /*
+ * If pci_dev->driver is not set (unbound), the device should
+ * always remain in D0 regardless of the runtime PM status
+ */
+ if (!pci_dev->driver)
+ return 0;
+
if (!pm || !pm->runtime_suspend)
return -ENOSYS;
@@ -1029,6 +1027,13 @@ static int pci_pm_runtime_resume(struct
struct pci_dev *pci_dev = to_pci_dev(dev);
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
+ /*
+ * If pci_dev->driver is not set (unbound), the device should
+ * always remain in D0 regardless of the runtime PM status
+ */
+ if (!pci_dev->driver)
+ return 0;
+
if (!pm || !pm->runtime_resume)
return -ENOSYS;
@@ -1046,8 +1051,16 @@ static int pci_pm_runtime_resume(struct
static int pci_pm_runtime_idle(struct device *dev)
{
+ struct pci_dev *pci_dev = to_pci_dev(dev);
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
+ /*
+ * If pci_dev->driver is not set (unbound), the device should
+ * always remain in D0 regardless of the runtime PM status
+ */
+ if (!pci_dev->driver)
+ goto out;
+
if (!pm)
return -ENOSYS;
@@ -1057,8 +1070,8 @@ static int pci_pm_runtime_idle(struct de
return ret;
}
+out:
pm_runtime_suspend(dev);
-
return 0;
}
^ permalink raw reply
* [PULL REQUEST for Rafael] PM / devfreq: pull request
From: MyungJoo Ham @ 2012-11-20 7:50 UTC (permalink / raw)
To: rjw
Cc: linux-pm, myungjoo.ham, myungjoo.ham, nm, rajagopal.venkat,
kyungmin.park, sh007.yi, jonghwa3.lee, sachin.kamat
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
The following changes since commit 77b67063bb6bce6d475e910d3b886a606d0d91f7:
Linux 3.7-rc5 (2012-11-11 13:44:33 +0100)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mzx/devfreq.git tags/pull_req_20121120
, which is based on Rafael's linux-pm.git pm-devfreq.
Jonghwa Lee (1):
PM / devfreq: Add sysfs node for representing frequency transition information.
Nishanth Menon (12):
PM / devfreq: kernel-doc typo corrections
PM / devfreq: fix sscanf handling for writable sysfs entries
PM / devfreq: make devfreq_class static
PM / devfreq: documentation cleanups for devfreq header
PM / devfreq: Add sysfs node to expose available frequencies
PM / devfreq: export update_devfreq
PM / devfreq: provide hooks for governors to be registered
PM / devfreq: register governors with devfreq framework
PM / devfreq: map devfreq drivers to governor using name
PM / devfreq: governors: add GPL module license and allow module build
PM / devfreq: allow sysfs governor node to switch governor
PM / devfreq: Add sysfs node to expose available governors
Rajagopal Venkat (3):
PM / devfreq: Core updates to support devices which can idle
PM / devfreq: Add suspend and resume apis
PM / devfreq: Add current freq callback in device profile
Sachin Kamat (1):
PM / devfreq: Use devm_* functions in exynos4_bus.c
Sangho Yi (1):
PM / devfreq: exynos4_bus.c: Fixed an alignment of the func call args.
Documentation/ABI/testing/sysfs-class-devfreq | 44 +-
drivers/devfreq/Kconfig | 8 +-
drivers/devfreq/devfreq.c | 885 +++++++++++++++++--------
drivers/devfreq/exynos4_bus.c | 45 +-
drivers/devfreq/governor.h | 17 +
drivers/devfreq/governor_performance.c | 38 +-
drivers/devfreq/governor_powersave.c | 38 +-
drivers/devfreq/governor_simpleondemand.c | 55 ++-
drivers/devfreq/governor_userspace.c | 45 ++-
include/linux/devfreq.h | 134 ++--
10 files changed, 915 insertions(+), 394 deletions(-)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
iQIcBAEBAgAGBQJQqzYmAAoJEBOhurvWBoCKFHUP/3dM8TjCCbpLCXISig+3qCy9
RLjH18vbWqDNmEfticjPM/t+kbKwnW/opVbpRxwpncHrrA7hUqKOdefs6pyo1/wH
p1xFoZfzHLiQoGMpHeNnAm3ItBO9H/UYqfw+5DWvU2bGZVHs2OAynom3KCKkL6JZ
pkvE33m56cP9kFoa4tI5TmR7cItUj6ShIMj6m/TB2VcbZ//xJ3X1ONk53/5Au3iW
SSW9UM/O2rR+z6Xr1/AvVcy9zuhfhxt5/37GjsYwKHF1J7ZRF40l4+jTG6Nw1geq
a2H2AVNEQrUjU9JmcnarvkFGffcVNmLJDMYBII2DvdLScW7Bx8qfORL0blxLdDcr
LbOkQjA1GAFf3e/ZuKNABL6PM+V/tCay+HrWMC9bEy7yCkhE+4iBx5IRhixhogsm
Qcx8hOpYca1Tmj9v1yXrDcyFPfKqqtXvDoTnX9UgWQKTyYROpJ3Kc/vwWJAJ4CnV
ppk8Ocdl6P2X0E0BelJGpsONrhyyrzMcGEtJBxNhM1NcmiAN037Ym6zAq78BcqHr
O6aLdSQuODY/vUc37Y0Zd6FqSdc4EfPaveJz03Q7x+CQ9NT/yzg06AWAtqbJnDQM
nVf1oPwMve29aK7v7A5ObKl9Tg9/KbOPb/Lycym7fFlEiCN9RbNEcQ2PR1894AvG
NDujl9d0CmYBo7jVlnuw
=zoRw
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [PATCH v3 1/2] thermal: exynos: Fix wrong bit to control tmu core
From: Kyungmin Park @ 2012-11-20 7:06 UTC (permalink / raw)
To: Zhang Rui
Cc: Jonghwan Choi, jonghwa3.lee, open list, Amit Daniel Kachhap,
Sachin Kamat, Linux PM list
In-Reply-To: <1353393150.3833.5.camel@rzhang1-mobl4>
On 11/20/12, Zhang Rui <rui.zhang@intel.com> wrote:
> On Tue, 2012-11-20 at 15:16 +0900, Kyungmin Park wrote:
>> On 11/20/12, Zhang Rui <rui.zhang@intel.com> wrote:
>> > On Tue, 2012-11-20 at 10:39 +0900, Kyungmin Park wrote:
>> >> On 11/20/12, Jonghwan Choi <jhbird.choi@samsung.com> wrote:
>> >> > [0]bit is used to enable/disable tmu core. [1] bit is a reserved
>> >> > bit.
>> >> >
>> >> > Signed-off-by: Jonghwan Choi <jhbird.choi@samsung.com>
>> >> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> >
>> > Amit and Donggeun Kim,
>> FYI: Donggeun was working together with me and he left the company an
>> year ago and now Mr. Lee are take over.
>
> I see.
>
> so Amit and Lee,
> will any of you be the maintainer of this driver?
Okay, after discussing it with Amit and Mr. Lee. let you know.
basically Mr. Lee want to maintain thermal task and he's working on it now.
Thank you,
Kyungmin Park
>
> I'd like to see your comments for the changes, as you know more about
> the hardware details.
>
> thanks,
> rui
>> > any comments on this patch?
>> >
>> > thanks,
>> > rui
>> >
>> >> > ---
>> >> > drivers/thermal/exynos_thermal.c | 16 ++++++++++++----
>> >> > 1 files changed, 12 insertions(+), 4 deletions(-)
>> >> >
>> >> > diff --git a/drivers/thermal/exynos_thermal.c
>> >> > b/drivers/thermal/exynos_thermal.c
>> >> > index 6dd29e4..129e827 100644
>> >> > --- a/drivers/thermal/exynos_thermal.c
>> >> > +++ b/drivers/thermal/exynos_thermal.c
>> >> > @@ -52,9 +52,12 @@
>> >> >
>> >> > #define EXYNOS_TMU_TRIM_TEMP_MASK 0xff
>> >> > #define EXYNOS_TMU_GAIN_SHIFT 8
>> >> > +#define EXYNOS_TMU_GAIN_MASK (0xF <<
>> >> > EXYNOS_TMU_GAIN_SHIFT)
>> >> > #define EXYNOS_TMU_REF_VOLTAGE_SHIFT 24
>> >> > -#define EXYNOS_TMU_CORE_ON 3
>> >> > -#define EXYNOS_TMU_CORE_OFF 2
>> >> > +#define EXYNOS_TMU_REF_VOLTAGE_MASK (0x1F <<
>> >> > EXYNOS_TMU_REF_VOLTAGE_SHIFT)
>> >> > +#define EXYNOS_TMU_CORE_ON BIT(0)
>> >> > +#define EXYNOS_TMU_CORE_ON_SHIFT 0
>> >> > +#define EXYNOS_TMU_CORE_ON_MASK (0x1 <<
>> >> > EXYNOS_TMU_CORE_ON_SHIFT)
>> >> > #define EXYNOS_TMU_DEF_CODE_TO_TEMP_OFFSET 50
>> >> >
>> >> > /* Exynos4210 specific registers */
>> >> > @@ -85,7 +88,9 @@
>> >> > #define EXYNOS_TMU_CLEAR_FALL_INT (0x111 << 16)
>> >> > #define EXYNOS_MUX_ADDR_VALUE 6
>> >> > #define EXYNOS_MUX_ADDR_SHIFT 20
>> >> > +#define EXYNOS_MUX_ADDR_MASK (0x7 <<
>> >> > EXYNOS_MUX_ADDR_SHIFT)
>> >> > #define EXYNOS_TMU_TRIP_MODE_SHIFT 13
>> >> > +#define EXYNOS_TMU_TRIP_MODE_MASK (0x7 <<
>> >> > EXYNOS_TMU_TRIP_MODE_SHIFT)
>> >> >
>> >> > #define EFUSE_MIN_VALUE 40
>> >> > #define EFUSE_MAX_VALUE 100
>> >> > @@ -650,10 +655,14 @@ static void exynos_tmu_control(struct
>> >> > platform_device
>> >> > *pdev, bool on)
>> >> > mutex_lock(&data->lock);
>> >> > clk_enable(data->clk);
>> >> >
>> >> > - con = pdata->reference_voltage <<
>> >> > EXYNOS_TMU_REF_VOLTAGE_SHIFT
>> >> > |
>> >> > + con = readl(data->base + EXYNOS_TMU_REG_CONTROL);
>> >> > + con &= ~(EXYNOS_TMU_REF_VOLTAGE_MASK | EXYNOS_TMU_GAIN_MASK
>> >> > |
>> >> > + EXYNOS_TMU_CORE_ON_MASK);
>> >> > + con |= pdata->reference_voltage <<
>> >> > EXYNOS_TMU_REF_VOLTAGE_SHIFT
>> >> > |
>> >> > pdata->gain << EXYNOS_TMU_GAIN_SHIFT;
>> >> >
>> >> > if (data->soc == SOC_ARCH_EXYNOS) {
>> >> > + con &= ~(EXYNOS_TMU_TRIP_MODE_MASK |
>> >> > EXYNOS_MUX_ADDR_MASK);
>> >> > con |= pdata->noise_cancel_mode <<
>> >> > EXYNOS_TMU_TRIP_MODE_SHIFT;
>> >> > con |= (EXYNOS_MUX_ADDR_VALUE <<
>> >> > EXYNOS_MUX_ADDR_SHIFT);
>> >> > }
>> >> > @@ -665,7 +674,6 @@ static void exynos_tmu_control(struct
>> >> > platform_device
>> >> > *pdev, bool on)
>> >> > pdata->trigger_level1_en << 4 |
>> >> > pdata->trigger_level0_en;
>> >> > } else {
>> >> > - con |= EXYNOS_TMU_CORE_OFF;
>> >> > interrupt_en = 0; /* Disable all interrupts */
>> >> > }
>> >> > writel(interrupt_en, data->base + EXYNOS_TMU_REG_INTEN);
>> >> > --
>> >> > 1.7.4.1
>> >> >
>> >> > --
>> >> > To unsubscribe from this list: send the line "unsubscribe
>> >> > linux-kernel"
>> >> > in
>> >> > the body of a message to majordomo@vger.kernel.org
>> >> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>> >> > Please read the FAQ at http://www.tux.org/lkml/
>> >> >
>> >
>> >
>> >
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
^ permalink raw reply
* Re: [PATCH v3 1/2] thermal: exynos: Fix wrong bit to control tmu core
From: Zhang Rui @ 2012-11-20 6:32 UTC (permalink / raw)
To: Kyungmin Park
Cc: Jonghwan Choi, jonghwa3.lee, open list, Amit Daniel Kachhap,
Sachin Kamat, Linux PM list, dg77.kim
In-Reply-To: <CAH9JG2Xtdo0tpxEAW4m5LG4q--M9zXi-N+FiCtjhZ7Q=5DZ_Tw@mail.gmail.com>
On Tue, 2012-11-20 at 15:16 +0900, Kyungmin Park wrote:
> On 11/20/12, Zhang Rui <rui.zhang@intel.com> wrote:
> > On Tue, 2012-11-20 at 10:39 +0900, Kyungmin Park wrote:
> >> On 11/20/12, Jonghwan Choi <jhbird.choi@samsung.com> wrote:
> >> > [0]bit is used to enable/disable tmu core. [1] bit is a reserved bit.
> >> >
> >> > Signed-off-by: Jonghwan Choi <jhbird.choi@samsung.com>
> >> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> >
> > Amit and Donggeun Kim,
> FYI: Donggeun was working together with me and he left the company an
> year ago and now Mr. Lee are take over.
I see.
so Amit and Lee,
will any of you be the maintainer of this driver?
I'd like to see your comments for the changes, as you know more about
the hardware details.
thanks,
rui
> > any comments on this patch?
> >
> > thanks,
> > rui
> >
> >> > ---
> >> > drivers/thermal/exynos_thermal.c | 16 ++++++++++++----
> >> > 1 files changed, 12 insertions(+), 4 deletions(-)
> >> >
> >> > diff --git a/drivers/thermal/exynos_thermal.c
> >> > b/drivers/thermal/exynos_thermal.c
> >> > index 6dd29e4..129e827 100644
> >> > --- a/drivers/thermal/exynos_thermal.c
> >> > +++ b/drivers/thermal/exynos_thermal.c
> >> > @@ -52,9 +52,12 @@
> >> >
> >> > #define EXYNOS_TMU_TRIM_TEMP_MASK 0xff
> >> > #define EXYNOS_TMU_GAIN_SHIFT 8
> >> > +#define EXYNOS_TMU_GAIN_MASK (0xF << EXYNOS_TMU_GAIN_SHIFT)
> >> > #define EXYNOS_TMU_REF_VOLTAGE_SHIFT 24
> >> > -#define EXYNOS_TMU_CORE_ON 3
> >> > -#define EXYNOS_TMU_CORE_OFF 2
> >> > +#define EXYNOS_TMU_REF_VOLTAGE_MASK (0x1F <<
> >> > EXYNOS_TMU_REF_VOLTAGE_SHIFT)
> >> > +#define EXYNOS_TMU_CORE_ON BIT(0)
> >> > +#define EXYNOS_TMU_CORE_ON_SHIFT 0
> >> > +#define EXYNOS_TMU_CORE_ON_MASK (0x1 <<
> >> > EXYNOS_TMU_CORE_ON_SHIFT)
> >> > #define EXYNOS_TMU_DEF_CODE_TO_TEMP_OFFSET 50
> >> >
> >> > /* Exynos4210 specific registers */
> >> > @@ -85,7 +88,9 @@
> >> > #define EXYNOS_TMU_CLEAR_FALL_INT (0x111 << 16)
> >> > #define EXYNOS_MUX_ADDR_VALUE 6
> >> > #define EXYNOS_MUX_ADDR_SHIFT 20
> >> > +#define EXYNOS_MUX_ADDR_MASK (0x7 << EXYNOS_MUX_ADDR_SHIFT)
> >> > #define EXYNOS_TMU_TRIP_MODE_SHIFT 13
> >> > +#define EXYNOS_TMU_TRIP_MODE_MASK (0x7 <<
> >> > EXYNOS_TMU_TRIP_MODE_SHIFT)
> >> >
> >> > #define EFUSE_MIN_VALUE 40
> >> > #define EFUSE_MAX_VALUE 100
> >> > @@ -650,10 +655,14 @@ static void exynos_tmu_control(struct
> >> > platform_device
> >> > *pdev, bool on)
> >> > mutex_lock(&data->lock);
> >> > clk_enable(data->clk);
> >> >
> >> > - con = pdata->reference_voltage << EXYNOS_TMU_REF_VOLTAGE_SHIFT
> >> > |
> >> > + con = readl(data->base + EXYNOS_TMU_REG_CONTROL);
> >> > + con &= ~(EXYNOS_TMU_REF_VOLTAGE_MASK | EXYNOS_TMU_GAIN_MASK |
> >> > + EXYNOS_TMU_CORE_ON_MASK);
> >> > + con |= pdata->reference_voltage << EXYNOS_TMU_REF_VOLTAGE_SHIFT
> >> > |
> >> > pdata->gain << EXYNOS_TMU_GAIN_SHIFT;
> >> >
> >> > if (data->soc == SOC_ARCH_EXYNOS) {
> >> > + con &= ~(EXYNOS_TMU_TRIP_MODE_MASK |
> >> > EXYNOS_MUX_ADDR_MASK);
> >> > con |= pdata->noise_cancel_mode <<
> >> > EXYNOS_TMU_TRIP_MODE_SHIFT;
> >> > con |= (EXYNOS_MUX_ADDR_VALUE <<
> >> > EXYNOS_MUX_ADDR_SHIFT);
> >> > }
> >> > @@ -665,7 +674,6 @@ static void exynos_tmu_control(struct
> >> > platform_device
> >> > *pdev, bool on)
> >> > pdata->trigger_level1_en << 4 |
> >> > pdata->trigger_level0_en;
> >> > } else {
> >> > - con |= EXYNOS_TMU_CORE_OFF;
> >> > interrupt_en = 0; /* Disable all interrupts */
> >> > }
> >> > writel(interrupt_en, data->base + EXYNOS_TMU_REG_INTEN);
> >> > --
> >> > 1.7.4.1
> >> >
> >> > --
> >> > To unsubscribe from this list: send the line "unsubscribe linux-kernel"
> >> > in
> >> > the body of a message to majordomo@vger.kernel.org
> >> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >> > Please read the FAQ at http://www.tux.org/lkml/
> >> >
> >
> >
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 1/2] thermal: exynos: Fix wrong bit to control tmu core
From: Kyungmin Park @ 2012-11-20 6:16 UTC (permalink / raw)
To: Zhang Rui
Cc: Jonghwan Choi, jonghwa3.lee, open list, Amit Daniel Kachhap,
Sachin Kamat, Linux PM list, dg77.kim
In-Reply-To: <1353390792.3833.1.camel@rzhang1-mobl4>
On 11/20/12, Zhang Rui <rui.zhang@intel.com> wrote:
> On Tue, 2012-11-20 at 10:39 +0900, Kyungmin Park wrote:
>> On 11/20/12, Jonghwan Choi <jhbird.choi@samsung.com> wrote:
>> > [0]bit is used to enable/disable tmu core. [1] bit is a reserved bit.
>> >
>> > Signed-off-by: Jonghwan Choi <jhbird.choi@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>
> Amit and Donggeun Kim,
FYI: Donggeun was working together with me and he left the company an
year ago and now Mr. Lee are take over.
> any comments on this patch?
>
> thanks,
> rui
>
>> > ---
>> > drivers/thermal/exynos_thermal.c | 16 ++++++++++++----
>> > 1 files changed, 12 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/thermal/exynos_thermal.c
>> > b/drivers/thermal/exynos_thermal.c
>> > index 6dd29e4..129e827 100644
>> > --- a/drivers/thermal/exynos_thermal.c
>> > +++ b/drivers/thermal/exynos_thermal.c
>> > @@ -52,9 +52,12 @@
>> >
>> > #define EXYNOS_TMU_TRIM_TEMP_MASK 0xff
>> > #define EXYNOS_TMU_GAIN_SHIFT 8
>> > +#define EXYNOS_TMU_GAIN_MASK (0xF << EXYNOS_TMU_GAIN_SHIFT)
>> > #define EXYNOS_TMU_REF_VOLTAGE_SHIFT 24
>> > -#define EXYNOS_TMU_CORE_ON 3
>> > -#define EXYNOS_TMU_CORE_OFF 2
>> > +#define EXYNOS_TMU_REF_VOLTAGE_MASK (0x1F <<
>> > EXYNOS_TMU_REF_VOLTAGE_SHIFT)
>> > +#define EXYNOS_TMU_CORE_ON BIT(0)
>> > +#define EXYNOS_TMU_CORE_ON_SHIFT 0
>> > +#define EXYNOS_TMU_CORE_ON_MASK (0x1 <<
>> > EXYNOS_TMU_CORE_ON_SHIFT)
>> > #define EXYNOS_TMU_DEF_CODE_TO_TEMP_OFFSET 50
>> >
>> > /* Exynos4210 specific registers */
>> > @@ -85,7 +88,9 @@
>> > #define EXYNOS_TMU_CLEAR_FALL_INT (0x111 << 16)
>> > #define EXYNOS_MUX_ADDR_VALUE 6
>> > #define EXYNOS_MUX_ADDR_SHIFT 20
>> > +#define EXYNOS_MUX_ADDR_MASK (0x7 << EXYNOS_MUX_ADDR_SHIFT)
>> > #define EXYNOS_TMU_TRIP_MODE_SHIFT 13
>> > +#define EXYNOS_TMU_TRIP_MODE_MASK (0x7 <<
>> > EXYNOS_TMU_TRIP_MODE_SHIFT)
>> >
>> > #define EFUSE_MIN_VALUE 40
>> > #define EFUSE_MAX_VALUE 100
>> > @@ -650,10 +655,14 @@ static void exynos_tmu_control(struct
>> > platform_device
>> > *pdev, bool on)
>> > mutex_lock(&data->lock);
>> > clk_enable(data->clk);
>> >
>> > - con = pdata->reference_voltage << EXYNOS_TMU_REF_VOLTAGE_SHIFT
>> > |
>> > + con = readl(data->base + EXYNOS_TMU_REG_CONTROL);
>> > + con &= ~(EXYNOS_TMU_REF_VOLTAGE_MASK | EXYNOS_TMU_GAIN_MASK |
>> > + EXYNOS_TMU_CORE_ON_MASK);
>> > + con |= pdata->reference_voltage << EXYNOS_TMU_REF_VOLTAGE_SHIFT
>> > |
>> > pdata->gain << EXYNOS_TMU_GAIN_SHIFT;
>> >
>> > if (data->soc == SOC_ARCH_EXYNOS) {
>> > + con &= ~(EXYNOS_TMU_TRIP_MODE_MASK |
>> > EXYNOS_MUX_ADDR_MASK);
>> > con |= pdata->noise_cancel_mode <<
>> > EXYNOS_TMU_TRIP_MODE_SHIFT;
>> > con |= (EXYNOS_MUX_ADDR_VALUE <<
>> > EXYNOS_MUX_ADDR_SHIFT);
>> > }
>> > @@ -665,7 +674,6 @@ static void exynos_tmu_control(struct
>> > platform_device
>> > *pdev, bool on)
>> > pdata->trigger_level1_en << 4 |
>> > pdata->trigger_level0_en;
>> > } else {
>> > - con |= EXYNOS_TMU_CORE_OFF;
>> > interrupt_en = 0; /* Disable all interrupts */
>> > }
>> > writel(interrupt_en, data->base + EXYNOS_TMU_REG_INTEN);
>> > --
>> > 1.7.4.1
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-kernel"
>> > in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>> > Please read the FAQ at http://www.tux.org/lkml/
>> >
>
>
>
^ permalink raw reply
* [PATCH] [CPUFREQ] EXYNOS: Use static for functions used in only this file
From: Tushar Behera @ 2012-11-20 5:59 UTC (permalink / raw)
To: linux-pm, cpufreq; +Cc: rjw, patches
Fixes following sparse error.
drivers/cpufreq/exynos-cpufreq.c:34:5: warning: symbol
'exynos_verify_speed' was not declared. Should it be static?
drivers/cpufreq/exynos-cpufreq.c:40:14: warning: symbol
'exynos_getspeed' was not declared. Should it be static?
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
drivers/cpufreq/exynos-cpufreq.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/cpufreq/exynos-cpufreq.c b/drivers/cpufreq/exynos-cpufreq.c
index af2d81e..eb057bf 100644
--- a/drivers/cpufreq/exynos-cpufreq.c
+++ b/drivers/cpufreq/exynos-cpufreq.c
@@ -31,13 +31,13 @@ static unsigned int locking_frequency;
static bool frequency_locked;
static DEFINE_MUTEX(cpufreq_lock);
-int exynos_verify_speed(struct cpufreq_policy *policy)
+static int exynos_verify_speed(struct cpufreq_policy *policy)
{
return cpufreq_frequency_table_verify(policy,
exynos_info->freq_table);
}
-unsigned int exynos_getspeed(unsigned int cpu)
+static unsigned int exynos_getspeed(unsigned int cpu)
{
return clk_get_rate(exynos_info->cpu_clk) / 1000;
}
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH v9 06/10] ata: zpodd: check zero power ready status
From: Aaron Lu @ 2012-11-20 6:00 UTC (permalink / raw)
To: Tejun Heo, Rafael J. Wysocki, Alan Stern
Cc: Jeff Garzik, James Bottomley, Jeff Wu, Aaron Lu, linux-ide,
linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20121119145605.GC15971@htj.dyndns.org>
On 11/19/2012 10:56 PM, Tejun Heo wrote:
> Hey, Aaron.
>
> On Mon, Nov 19, 2012 at 11:09:40AM +0800, Aaron Lu wrote:
>>> What I'm confused about is what autopm does for devices w/o zpodd.
>>> What happens then? Is it gonna leave power on for the device and,
>>> say, go on to suspend the controller? But, how would that work for,
>>> say, future devices with async notification for media events?
>>
>> Maybe we shouldn't allow autopm for such devices?
>
> Yeah, maybe. It would be nice to be able to automatically power off
> disks and ports which aren't being used tho.
Yes, we can do this.
I'm just saying, if an ODD is using async notification, we probably
shouldn't enable autopm for it at the moment.
>
>>> That said, I can't say the snooping is pretty. It's a rather nasty
>>> thing to do. So, libata now wants information from the event polling
>>> in block layer, but reaching for block_device from ata_devices is
>>> nasty too. Hmmm... but aren't you already doing that to block polling
>>> on a powered down device?
>>
>> I was feeling brain damaged by this for some time :-)
>>
>> Basically, only ATA layer is aware of the power off thing, and sr knows
>> nothing about this(or it is not supposed to know this, at least, this is
>> what SCSI people think) and once powered off, I do not want the poll to
>> disturb the device, so I need to block the poll. I can't come up with
>> another way to achieve this except this nasty way.
>>
>> James suggests me to keep the poll, but emulate the command. The problem
>> with this is, the autopm for resume will kick in on each poll, so I'll
>> need to decide if power up the ODD for this time's resume is needed in
>> port's runtime resume callback. This made things complex and it also put
>> too much logic in the resume callback, which is not desired. And even if
>> I keep the ODD in powered off state by emulating this poll command, its
>> ancestor devices will still be resumed, and I may need to do some trick
>> in their resume callback to avoid needless power/state transitions. This
>> doesn't feel like an elegant way to solve this either.
>>
>> So yes, I'm still using this _nasty_ way to make the ODD stay in powered
>> off state as long as possible. But if there is other elegant ways to
>> solve this, I would appreciate it and happily using it. Personally, I
>> hope we can make sr aware of ZPODD, that would make the pain gone.
>
> I really think we need a way for (auto)pm and event polling to talk to
> each other so that autopm can tell event poll to sod off while pm is
> in effect. Trying to solve this from inside libata doesn't seem
> right. The problem, again, seems to be figuring out which hardware
> device maps to which block device. Hmmm... Any good ideas?
A possible way of doing this is using pm qos.
We currently have 2 pm qos flags, NO_POWER_OFF and REMOTE_WAKEUP, and we
can add another one: NO_POLL, use it like the following:
1 Set the NO_POLL pm qos flag when the underlying driver thinks it is no
longer necessary. In the ZPODD's case, it should be set when the
device is to be powered off;
2 Clear it when poll is necessary again. In the ZPODD's case, when power
is re-gained, this flag will be cleared.
3 In the disk_events_workfn, check if this flag is set, if so, simply
return.
The disk->driverfs_dev can be used to host the pm qos flag, ATA layer
can access it through ata_device->sdev->sdev_gendev.
Is this OK?
Thanks,
Aaron
^ permalink raw reply
* Re: [PATCH v3 1/2] thermal: exynos: Fix wrong bit to control tmu core
From: Zhang Rui @ 2012-11-20 5:53 UTC (permalink / raw)
To: Kyungmin Park
Cc: Jonghwan Choi, jonghwa3.lee, open list, Amit Daniel Kachhap,
Sachin Kamat, Linux PM list, dg77.kim
In-Reply-To: <CAH9JG2UYsQ6VFMO4COyEr9fBzCL03+37bXTN=AgjKbD7NjR1MA@mail.gmail.com>
On Tue, 2012-11-20 at 10:39 +0900, Kyungmin Park wrote:
> On 11/20/12, Jonghwan Choi <jhbird.choi@samsung.com> wrote:
> > [0]bit is used to enable/disable tmu core. [1] bit is a reserved bit.
> >
> > Signed-off-by: Jonghwan Choi <jhbird.choi@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Amit and Donggeun Kim,
any comments on this patch?
thanks,
rui
> > ---
> > drivers/thermal/exynos_thermal.c | 16 ++++++++++++----
> > 1 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/thermal/exynos_thermal.c
> > b/drivers/thermal/exynos_thermal.c
> > index 6dd29e4..129e827 100644
> > --- a/drivers/thermal/exynos_thermal.c
> > +++ b/drivers/thermal/exynos_thermal.c
> > @@ -52,9 +52,12 @@
> >
> > #define EXYNOS_TMU_TRIM_TEMP_MASK 0xff
> > #define EXYNOS_TMU_GAIN_SHIFT 8
> > +#define EXYNOS_TMU_GAIN_MASK (0xF << EXYNOS_TMU_GAIN_SHIFT)
> > #define EXYNOS_TMU_REF_VOLTAGE_SHIFT 24
> > -#define EXYNOS_TMU_CORE_ON 3
> > -#define EXYNOS_TMU_CORE_OFF 2
> > +#define EXYNOS_TMU_REF_VOLTAGE_MASK (0x1F <<
> > EXYNOS_TMU_REF_VOLTAGE_SHIFT)
> > +#define EXYNOS_TMU_CORE_ON BIT(0)
> > +#define EXYNOS_TMU_CORE_ON_SHIFT 0
> > +#define EXYNOS_TMU_CORE_ON_MASK (0x1 <<
> > EXYNOS_TMU_CORE_ON_SHIFT)
> > #define EXYNOS_TMU_DEF_CODE_TO_TEMP_OFFSET 50
> >
> > /* Exynos4210 specific registers */
> > @@ -85,7 +88,9 @@
> > #define EXYNOS_TMU_CLEAR_FALL_INT (0x111 << 16)
> > #define EXYNOS_MUX_ADDR_VALUE 6
> > #define EXYNOS_MUX_ADDR_SHIFT 20
> > +#define EXYNOS_MUX_ADDR_MASK (0x7 << EXYNOS_MUX_ADDR_SHIFT)
> > #define EXYNOS_TMU_TRIP_MODE_SHIFT 13
> > +#define EXYNOS_TMU_TRIP_MODE_MASK (0x7 << EXYNOS_TMU_TRIP_MODE_SHIFT)
> >
> > #define EFUSE_MIN_VALUE 40
> > #define EFUSE_MAX_VALUE 100
> > @@ -650,10 +655,14 @@ static void exynos_tmu_control(struct platform_device
> > *pdev, bool on)
> > mutex_lock(&data->lock);
> > clk_enable(data->clk);
> >
> > - con = pdata->reference_voltage << EXYNOS_TMU_REF_VOLTAGE_SHIFT |
> > + con = readl(data->base + EXYNOS_TMU_REG_CONTROL);
> > + con &= ~(EXYNOS_TMU_REF_VOLTAGE_MASK | EXYNOS_TMU_GAIN_MASK |
> > + EXYNOS_TMU_CORE_ON_MASK);
> > + con |= pdata->reference_voltage << EXYNOS_TMU_REF_VOLTAGE_SHIFT |
> > pdata->gain << EXYNOS_TMU_GAIN_SHIFT;
> >
> > if (data->soc == SOC_ARCH_EXYNOS) {
> > + con &= ~(EXYNOS_TMU_TRIP_MODE_MASK | EXYNOS_MUX_ADDR_MASK);
> > con |= pdata->noise_cancel_mode <<
> > EXYNOS_TMU_TRIP_MODE_SHIFT;
> > con |= (EXYNOS_MUX_ADDR_VALUE << EXYNOS_MUX_ADDR_SHIFT);
> > }
> > @@ -665,7 +674,6 @@ static void exynos_tmu_control(struct platform_device
> > *pdev, bool on)
> > pdata->trigger_level1_en << 4 |
> > pdata->trigger_level0_en;
> > } else {
> > - con |= EXYNOS_TMU_CORE_OFF;
> > interrupt_en = 0; /* Disable all interrupts */
> > }
> > writel(interrupt_en, data->base + EXYNOS_TMU_REG_INTEN);
> > --
> > 1.7.4.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox