* Re: [PATCH 3/3] PM: Introduce Intel PowerClamp Driver
From: Paul E. McKenney @ 2012-11-15 3:22 UTC (permalink / raw)
To: Arjan van de Ven
Cc: Jacob Pan, Linux PM, LKML, Rafael Wysocki, Len Brown,
Thomas Gleixner, H. Peter Anvin, Ingo Molnar, Zhang Rui,
Rob Landley
In-Reply-To: <50A308FA.40001@linux.intel.com>
On Tue, Nov 13, 2012 at 06:59:06PM -0800, Arjan van de Ven wrote:
> On 11/13/2012 5:34 PM, Paul E. McKenney wrote:
> > On Tue, Nov 13, 2012 at 05:14:50PM -0800, Jacob Pan wrote:
> >> On Tue, 13 Nov 2012 16:08:54 -0800
> >> Arjan van de Ven <arjan@linux.intel.com> wrote:
> >>
> >>>> I think I know, but I feel the need to ask anyway. Why not tell
> >>>> RCU about the clamping?
> >>>
> >>> I don't mind telling RCU, but what cannot happen is a bunch of CPU
> >>> time suddenly getting used (since that is the opposite of what is
> >>> needed at the specific point in time of going idle)
> >
> > Another round of RCU_FAST_NO_HZ rework, you are asking for? ;-)
>
> well
> we can tell you we're about to mwait
> and we can tell you when we're done being idle.
> you could just do the actual work at that point, we don't care anymore ;-)
> just at the start of the mandated idle period we can't afford to have more
> jitter than we already have (which is more than I'd like, but it's manageable.
> More jitter means more performance hit, since during the time of the jitter, some cpus
> are forced idle, e.g. costing performance, without the actual big-step power savings
> kicking in yet....)
Fair enough -- but probably best to see what problems arise rather than
trying to guess too far ahead. Who knows? It might "just work".
> > If you are only having the system take 6-millisecond "vacations", probably
>
> it's not all that different from running a while (1) loop for 6 msec inside
> a kernel thread.... other than the power level of course...
Well, a while (1) on all CPUs simultaneously, anyway.
Thanx, Paul
^ permalink raw reply
* Re: [PATCH 1/1] thermal: Exynos: Add missing dependency
From: Sachin Kamat @ 2012-11-15 3:22 UTC (permalink / raw)
To: Zhang Rui; +Cc: linux-pm, durgadoss.r, patches, akpm, Amit Daniel Kachhap
In-Reply-To: <1352938284.2143.5.camel@rzhang1-mobl4>
Hi Zhang,
Thanks for your review comments.
On 15 November 2012 05:41, Zhang Rui <rui.zhang@intel.com> wrote:
> Hi, Sachin,
>
> thanks for catching the problem.
>
> On Wed, 2012-11-14 at 12:18 +0530, Sachin Kamat wrote:
>> CPU_FREQ_TABLE depends on CPU_FREQ. Selecting CPU_FREQ_TABLE without checking
>> for dependencies gives the following compilation warnings:
>> warning: (ARCH_TEGRA_2x_SOC && ARCH_TEGRA_3x_SOC && UX500_SOC_DB8500 &&
>> CPU_THERMAL && EXYNOS_THERMAL) selects CPU_FREQ_TABLE which has unmet
>> direct dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ)
>>
> Amit,
>
> how is exynos driver supposed to work?
> do you want the exynos driver still be loaded without CPU_THERMAL?
> If yes, EXYNOS_THERMAL should not select CPU_FREQ_TABLE.
> If no, EXYNOS_THERMAL should depends on CPU_THERMAL instead of THERMAL.
> and CPU_THERMAL will select CPU_FREQ_TABLE instead.
>
> IMO, either of the above solution will be more proper to fix this
> warning.
I will discuss with Amit and send an updated patch to fix this.
>
> thanks,
> rui
>
>> Cc: Amit Daniel Kachhap <amit.kachhap@linaro.org>
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> ---
>> Build tested using exynos4_defconfig on linux-next tree of 20121114.
>> ---
>> drivers/thermal/Kconfig | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index 266c15e..197b7db 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -50,7 +50,7 @@ config RCAR_THERMAL
>>
>> config EXYNOS_THERMAL
>> tristate "Temperature sensor on Samsung EXYNOS"
>> - depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
>> + depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL && CPU_FREQ
>> select CPU_FREQ_TABLE
>> help
>> If you say yes here you get support for TMU (Thermal Managment
>
>
--
With warm regards,
Sachin
^ permalink raw reply
* Re: [PATCH 1/1] thermal: Exynos: Add missing dependency
From: Zhang Rui @ 2012-11-15 4:14 UTC (permalink / raw)
To: Sachin Kamat; +Cc: linux-pm, durgadoss.r, patches, akpm, Amit Daniel Kachhap
In-Reply-To: <CAK9yfHzCmWwaQa79JFoa2Y1Q6-tjdFS+M8vScXwwCTnfgP9nUA@mail.gmail.com>
On Thu, 2012-11-15 at 08:52 +0530, Sachin Kamat wrote:
> Hi Zhang,
>
> Thanks for your review comments.
>
> On 15 November 2012 05:41, Zhang Rui <rui.zhang@intel.com> wrote:
> > Hi, Sachin,
> >
> > thanks for catching the problem.
> >
> > On Wed, 2012-11-14 at 12:18 +0530, Sachin Kamat wrote:
> >> CPU_FREQ_TABLE depends on CPU_FREQ. Selecting CPU_FREQ_TABLE without checking
> >> for dependencies gives the following compilation warnings:
> >> warning: (ARCH_TEGRA_2x_SOC && ARCH_TEGRA_3x_SOC && UX500_SOC_DB8500 &&
> >> CPU_THERMAL && EXYNOS_THERMAL) selects CPU_FREQ_TABLE which has unmet
> >> direct dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ)
> >>
> > Amit,
> >
> > how is exynos driver supposed to work?
> > do you want the exynos driver still be loaded without CPU_THERMAL?
> > If yes, EXYNOS_THERMAL should not select CPU_FREQ_TABLE.
> > If no, EXYNOS_THERMAL should depends on CPU_THERMAL instead of THERMAL.
> > and CPU_THERMAL will select CPU_FREQ_TABLE instead.
> >
> > IMO, either of the above solution will be more proper to fix this
> > warning.
>
> I will discuss with Amit and send an updated patch to fix this.
>
I checked the code of exynos driver, it fails to load if no
cpufreq_cooling device found, which means it depends on CPU_THERMAL.
I'm not sure if this is the behavior you want, but IMO, exynos driver
should register its critical trip point to the generic thermal layer
instead, when CPU_THERMAL is not set.
thanks,
rui
>
> >
> > thanks,
> > rui
> >
> >> Cc: Amit Daniel Kachhap <amit.kachhap@linaro.org>
> >> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> >> ---
> >> Build tested using exynos4_defconfig on linux-next tree of 20121114.
> >> ---
> >> drivers/thermal/Kconfig | 2 +-
> >> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> >> index 266c15e..197b7db 100644
> >> --- a/drivers/thermal/Kconfig
> >> +++ b/drivers/thermal/Kconfig
> >> @@ -50,7 +50,7 @@ config RCAR_THERMAL
> >>
> >> config EXYNOS_THERMAL
> >> tristate "Temperature sensor on Samsung EXYNOS"
> >> - depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
> >> + depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL && CPU_FREQ
> >> select CPU_FREQ_TABLE
> >> help
> >> If you say yes here you get support for TMU (Thermal Managment
> >
> >
>
>
>
^ permalink raw reply
* Re: [PATCH 1/1] thermal: Exynos: Add missing dependency
From: Amit Kachhap @ 2012-11-15 6:24 UTC (permalink / raw)
To: linux-pm, Sachin Kamat; +Cc: durgadoss.r, patches, akpm
In-Reply-To: <1352938284.2143.5.camel@rzhang1-mobl4>
On 15 November 2012 05:41, Zhang Rui <rui.zhang@intel.com> wrote:
> Hi, Sachin,
>
> thanks for catching the problem.
>
> On Wed, 2012-11-14 at 12:18 +0530, Sachin Kamat wrote:
>> CPU_FREQ_TABLE depends on CPU_FREQ. Selecting CPU_FREQ_TABLE without checking
>> for dependencies gives the following compilation warnings:
>> warning: (ARCH_TEGRA_2x_SOC && ARCH_TEGRA_3x_SOC && UX500_SOC_DB8500 &&
>> CPU_THERMAL && EXYNOS_THERMAL) selects CPU_FREQ_TABLE which has unmet
>> direct dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ)
>>
> Amit,
>
> how is exynos driver supposed to work?
> do you want the exynos driver still be loaded without CPU_THERMAL?
> If yes, EXYNOS_THERMAL should not select CPU_FREQ_TABLE.
> If no, EXYNOS_THERMAL should depends on CPU_THERMAL instead of THERMAL.
> and CPU_THERMAL will select CPU_FREQ_TABLE instead.
>
> IMO, either of the above solution will be more proper to fix this
> warning.
Yes Rui, Currently exynos driver has dependency on CPU_THERMAL. There
is some code-refactoring work needed to remove this dependency.
As of now to fix the warning following changes may be sufficient.
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -50,7 +50,8 @@ config RCAR_THERMAL
config EXYNOS_THERMAL
tristate "Temperature sensor on Samsung EXYNOS"
- depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
+ depends on THERMAL
+ depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && CPU_THERMAL
select CPU_FREQ_TABLE
help
If you say yes here you get support for TMU (Thermal Managment
Thanks,
Amit Daniel
>
> thanks,
> rui
>
>> Cc: Amit Daniel Kachhap <amit.kachhap@linaro.org>
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> ---
>> Build tested using exynos4_defconfig on linux-next tree of 20121114.
>> ---
>> drivers/thermal/Kconfig | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index 266c15e..197b7db 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -50,7 +50,7 @@ config RCAR_THERMAL
>>
>> config EXYNOS_THERMAL
>> tristate "Temperature sensor on Samsung EXYNOS"
>> - depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
>> + depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL && CPU_FREQ
>> select CPU_FREQ_TABLE
>> help
>> If you say yes here you get support for TMU (Thermal Managment
>
>
^ permalink raw reply
* Re: [PATCH V5 2/2] Thermal: Add ST-Ericsson DB8500 thermal properties and platform data.
From: Hongbo Zhang @ 2012-11-15 6:37 UTC (permalink / raw)
To: Linus Walleij, Zhang Rui, linux-pm
Cc: linux-acpi, linux-kernel, amit.kachhap, patches, linaro-dev,
linaro-kernel, STEricsson_nomadik_linux, kernel, hongbo.zhang
In-Reply-To: <CACRpkdb-C-z1bWA2+xbuGt8TKSWRaD2xYUKS4M=KNMO0mOKF2g@mail.gmail.com>
On 13 November 2012 04:46, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Nov 12, 2012 at 7:07 AM, Zhang Rui <rui.zhang@intel.com> wrote:
>> On Fri, 2012-11-09 at 19:29 +0800, hongbo.zhang wrote:
>>> From: "hongbo.zhang" <hongbo.zhang@linaro.com>
>>>
>>> This patch adds device tree properties for ST-Ericsson DB8500 thermal driver,
>>> also adds the platform data to support the old fashion.
>>>
>>> Signed-off-by: hongbo.zhang <hongbo.zhang@linaro.com>
>>> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
>>
>> hmmm,
>>
>> who should take this patch?
>> I'd like to see ACK from the maintainer of these code before applying
>> them to thermal tree.
>
> I guess it needs to go through Thermal.
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
Thank you Linus Walleij.
Hi Rui, I got it.
> Let's hope it's not creating too much conflict...
>
> Yours,
> Linus Walleij
^ permalink raw reply
* Re: [PATCH 1/1] thermal: Exynos: Add missing dependency
From: Sachin Kamat @ 2012-11-15 6:39 UTC (permalink / raw)
To: Amit Kachhap; +Cc: linux-pm, durgadoss.r, patches, akpm
In-Reply-To: <CAK44p22wzbTE-u5QcasJhzYa9Ht7e9sNrJ63V2QX2+ZEp=vhSg@mail.gmail.com>
On 15 November 2012 11:54, Amit Kachhap <amit.kachhap@linaro.org> wrote:
> On 15 November 2012 05:41, Zhang Rui <rui.zhang@intel.com> wrote:
>> Hi, Sachin,
>>
>> thanks for catching the problem.
>>
>> On Wed, 2012-11-14 at 12:18 +0530, Sachin Kamat wrote:
>>> CPU_FREQ_TABLE depends on CPU_FREQ. Selecting CPU_FREQ_TABLE without checking
>>> for dependencies gives the following compilation warnings:
>>> warning: (ARCH_TEGRA_2x_SOC && ARCH_TEGRA_3x_SOC && UX500_SOC_DB8500 &&
>>> CPU_THERMAL && EXYNOS_THERMAL) selects CPU_FREQ_TABLE which has unmet
>>> direct dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ)
>>>
>> Amit,
>>
>> how is exynos driver supposed to work?
>> do you want the exynos driver still be loaded without CPU_THERMAL?
>> If yes, EXYNOS_THERMAL should not select CPU_FREQ_TABLE.
>> If no, EXYNOS_THERMAL should depends on CPU_THERMAL instead of THERMAL.
>> and CPU_THERMAL will select CPU_FREQ_TABLE instead.
>>
>> IMO, either of the above solution will be more proper to fix this
>> warning.
>
> Yes Rui, Currently exynos driver has dependency on CPU_THERMAL. There
> is some code-refactoring work needed to remove this dependency.
> As of now to fix the warning following changes may be sufficient.
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -50,7 +50,8 @@ config RCAR_THERMAL
>
> config EXYNOS_THERMAL
> tristate "Temperature sensor on Samsung EXYNOS"
> - depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
> + depends on THERMAL
This is not needed as CPU_THERMAL depends on THERMAL.
> + depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && CPU_THERMAL
> select CPU_FREQ_TABLE
This can also be removed as CPU_THERMAL selects it.
I will resend my patch with the above changes.
> help
> If you say yes here you get support for TMU (Thermal Managment
>
> Thanks,
> Amit Daniel
>
>>
>> thanks,
>> rui
>>
>>> Cc: Amit Daniel Kachhap <amit.kachhap@linaro.org>
>>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>>> ---
>>> Build tested using exynos4_defconfig on linux-next tree of 20121114.
>>> ---
>>> drivers/thermal/Kconfig | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>>> index 266c15e..197b7db 100644
>>> --- a/drivers/thermal/Kconfig
>>> +++ b/drivers/thermal/Kconfig
>>> @@ -50,7 +50,7 @@ config RCAR_THERMAL
>>>
>>> config EXYNOS_THERMAL
>>> tristate "Temperature sensor on Samsung EXYNOS"
>>> - depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
>>> + depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL && CPU_FREQ
>>> select CPU_FREQ_TABLE
>>> help
>>> If you say yes here you get support for TMU (Thermal Managment
>>
>>
--
With warm regards,
Sachin
^ permalink raw reply
* RE: [PATCH 1/1] thermal: Exynos: Add missing dependency
From: Zhang, Rui @ 2012-11-15 6:45 UTC (permalink / raw)
To: Sachin Kamat, Amit Kachhap
Cc: linux-pm@vger.kernel.org, R, Durgadoss, patches@linaro.org,
akpm@linux-foundation.org
In-Reply-To: <CAK9yfHyU_rPhR=j4QoPM4sPVAEX7kQ6Vhv1OaakfensXKvqNXg@mail.gmail.com>
> -----Original Message-----
> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
> owner@vger.kernel.org] On Behalf Of Sachin Kamat
> Sent: Thursday, November 15, 2012 2:39 PM
> To: Amit Kachhap
> Cc: linux-pm@vger.kernel.org; R, Durgadoss; patches@linaro.org;
> akpm@linux-foundation.org
> Subject: Re: [PATCH 1/1] thermal: Exynos: Add missing dependency
>
> On 15 November 2012 11:54, Amit Kachhap <amit.kachhap@linaro.org> wrote:
> > On 15 November 2012 05:41, Zhang Rui <rui.zhang@intel.com> wrote:
> >> Hi, Sachin,
> >>
> >> thanks for catching the problem.
> >>
> >> On Wed, 2012-11-14 at 12:18 +0530, Sachin Kamat wrote:
> >>> CPU_FREQ_TABLE depends on CPU_FREQ. Selecting CPU_FREQ_TABLE
> without
> >>> checking for dependencies gives the following compilation warnings:
> >>> warning: (ARCH_TEGRA_2x_SOC && ARCH_TEGRA_3x_SOC &&
> UX500_SOC_DB8500
> >>> && CPU_THERMAL && EXYNOS_THERMAL) selects CPU_FREQ_TABLE which has
> >>> unmet direct dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ)
> >>>
> >> Amit,
> >>
> >> how is exynos driver supposed to work?
> >> do you want the exynos driver still be loaded without CPU_THERMAL?
> >> If yes, EXYNOS_THERMAL should not select CPU_FREQ_TABLE.
> >> If no, EXYNOS_THERMAL should depends on CPU_THERMAL instead of
> THERMAL.
> >> and CPU_THERMAL will select CPU_FREQ_TABLE instead.
> >>
> >> IMO, either of the above solution will be more proper to fix this
> >> warning.
> >
> > Yes Rui, Currently exynos driver has dependency on CPU_THERMAL. There
> > is some code-refactoring work needed to remove this dependency.
> > As of now to fix the warning following changes may be sufficient.
> > --- a/drivers/thermal/Kconfig
> > +++ b/drivers/thermal/Kconfig
> > @@ -50,7 +50,8 @@ config RCAR_THERMAL
> >
> > config EXYNOS_THERMAL
> > tristate "Temperature sensor on Samsung EXYNOS"
> > - depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
> > + depends on THERMAL
>
> This is not needed as CPU_THERMAL depends on THERMAL.
>
Agreed.
> > + depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && CPU_THERMAL
>
>
> > select CPU_FREQ_TABLE
> This can also be removed as CPU_THERMAL selects it.
>
Agreed.
> I will resend my patch with the above changes.
No, you don't need to.
I'm refactoring drivers/thermal/Kconfig, and I'll send a patch based on this idea.
Will send out the patch soon.
Thanks,
rui
> > help
> > If you say yes here you get support for TMU (Thermal
> > Managment
> >
> > Thanks,
> > Amit Daniel
> >
> >>
> >> thanks,
> >> rui
> >>
> >>> Cc: Amit Daniel Kachhap <amit.kachhap@linaro.org>
> >>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> >>> ---
> >>> Build tested using exynos4_defconfig on linux-next tree of 20121114.
> >>> ---
> >>> drivers/thermal/Kconfig | 2 +-
> >>> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index
> >>> 266c15e..197b7db 100644
> >>> --- a/drivers/thermal/Kconfig
> >>> +++ b/drivers/thermal/Kconfig
> >>> @@ -50,7 +50,7 @@ config RCAR_THERMAL
> >>>
> >>> config EXYNOS_THERMAL
> >>> tristate "Temperature sensor on Samsung EXYNOS"
> >>> - depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
> >>> + depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL &&
> >>> + CPU_FREQ
> >>> select CPU_FREQ_TABLE
> >>> help
> >>> If you say yes here you get support for TMU (Thermal
> >>> Managment
> >>
> >>
>
>
>
> --
> With warm regards,
> Sachin
> --
> 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
* [PATCH-Resend] thermal: Exynos: Add missing dependency
From: Sachin Kamat @ 2012-11-15 6:39 UTC (permalink / raw)
To: linux-pm; +Cc: rui.zhang, sachin.kamat, patches, akpm, Amit Daniel Kachhap
CPU_FREQ_TABLE depends on CPU_FREQ. Selecting CPU_FREQ_TABLE without checking
for dependencies gives the following compilation warnings:
warning: (ARCH_TEGRA_2x_SOC && ARCH_TEGRA_3x_SOC && UX500_SOC_DB8500 &&
CPU_THERMAL && EXYNOS_THERMAL) selects CPU_FREQ_TABLE which has unmet
direct dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ)
Cc: Amit Daniel Kachhap <amit.kachhap@linaro.org>
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
drivers/thermal/Kconfig | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 266c15e..6124c4b 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -50,8 +50,7 @@ config RCAR_THERMAL
config EXYNOS_THERMAL
tristate "Temperature sensor on Samsung EXYNOS"
- depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
- select CPU_FREQ_TABLE
+ depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && CPU_THERMAL
help
If you say yes here you get support for TMU (Thermal Managment
Unit) on SAMSUNG EXYNOS series of SoC.
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH 1/1] thermal: Exynos: Add missing dependency
From: Sachin Kamat @ 2012-11-15 6:46 UTC (permalink / raw)
To: Zhang, Rui
Cc: Amit Kachhap, linux-pm@vger.kernel.org, R, Durgadoss,
patches@linaro.org, akpm@linux-foundation.org
In-Reply-To: <744357E9AAD1214791ACBA4B0B9092632572FD@SHSMSX101.ccr.corp.intel.com>
On 15 November 2012 12:15, Zhang, Rui <rui.zhang@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
>> owner@vger.kernel.org] On Behalf Of Sachin Kamat
>> Sent: Thursday, November 15, 2012 2:39 PM
>> To: Amit Kachhap
>> Cc: linux-pm@vger.kernel.org; R, Durgadoss; patches@linaro.org;
>> akpm@linux-foundation.org
>> Subject: Re: [PATCH 1/1] thermal: Exynos: Add missing dependency
>>
>> On 15 November 2012 11:54, Amit Kachhap <amit.kachhap@linaro.org> wrote:
>> > On 15 November 2012 05:41, Zhang Rui <rui.zhang@intel.com> wrote:
>> >> Hi, Sachin,
>> >>
>> >> thanks for catching the problem.
>> >>
>> >> On Wed, 2012-11-14 at 12:18 +0530, Sachin Kamat wrote:
>> >>> CPU_FREQ_TABLE depends on CPU_FREQ. Selecting CPU_FREQ_TABLE
>> without
>> >>> checking for dependencies gives the following compilation warnings:
>> >>> warning: (ARCH_TEGRA_2x_SOC && ARCH_TEGRA_3x_SOC &&
>> UX500_SOC_DB8500
>> >>> && CPU_THERMAL && EXYNOS_THERMAL) selects CPU_FREQ_TABLE which has
>> >>> unmet direct dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ)
>> >>>
>> >> Amit,
>> >>
>> >> how is exynos driver supposed to work?
>> >> do you want the exynos driver still be loaded without CPU_THERMAL?
>> >> If yes, EXYNOS_THERMAL should not select CPU_FREQ_TABLE.
>> >> If no, EXYNOS_THERMAL should depends on CPU_THERMAL instead of
>> THERMAL.
>> >> and CPU_THERMAL will select CPU_FREQ_TABLE instead.
>> >>
>> >> IMO, either of the above solution will be more proper to fix this
>> >> warning.
>> >
>> > Yes Rui, Currently exynos driver has dependency on CPU_THERMAL. There
>> > is some code-refactoring work needed to remove this dependency.
>> > As of now to fix the warning following changes may be sufficient.
>> > --- a/drivers/thermal/Kconfig
>> > +++ b/drivers/thermal/Kconfig
>> > @@ -50,7 +50,8 @@ config RCAR_THERMAL
>> >
>> > config EXYNOS_THERMAL
>> > tristate "Temperature sensor on Samsung EXYNOS"
>> > - depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
>> > + depends on THERMAL
>>
>> This is not needed as CPU_THERMAL depends on THERMAL.
>>
> Agreed.
>> > + depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && CPU_THERMAL
>>
>>
>> > select CPU_FREQ_TABLE
>> This can also be removed as CPU_THERMAL selects it.
>>
> Agreed.
>
>> I will resend my patch with the above changes.
>
> No, you don't need to.
> I'm refactoring drivers/thermal/Kconfig, and I'll send a patch based on this idea.
> Will send out the patch soon.
Oops.. sorry just sent it out..
>
> Thanks,
> rui
>
>> > help
>> > If you say yes here you get support for TMU (Thermal
>> > Managment
>> >
>> > Thanks,
>> > Amit Daniel
>> >
>> >>
>> >> thanks,
>> >> rui
>> >>
>> >>> Cc: Amit Daniel Kachhap <amit.kachhap@linaro.org>
>> >>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> >>> ---
>> >>> Build tested using exynos4_defconfig on linux-next tree of 20121114.
>> >>> ---
>> >>> drivers/thermal/Kconfig | 2 +-
>> >>> 1 files changed, 1 insertions(+), 1 deletions(-)
>> >>>
>> >>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index
>> >>> 266c15e..197b7db 100644
>> >>> --- a/drivers/thermal/Kconfig
>> >>> +++ b/drivers/thermal/Kconfig
>> >>> @@ -50,7 +50,7 @@ config RCAR_THERMAL
>> >>>
>> >>> config EXYNOS_THERMAL
>> >>> tristate "Temperature sensor on Samsung EXYNOS"
>> >>> - depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
>> >>> + depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL &&
>> >>> + CPU_FREQ
>> >>> select CPU_FREQ_TABLE
>> >>> help
>> >>> If you say yes here you get support for TMU (Thermal
>> >>> Managment
>> >>
>> >>
>>
>>
>>
>> --
>> With warm regards,
>> Sachin
>> --
>> 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
--
With warm regards,
Sachin
^ permalink raw reply
* [PATCH 1/1] thermal: cpu_cooling: Make 'notify_device' static
From: Sachin Kamat @ 2012-11-15 6:49 UTC (permalink / raw)
To: linux-pm; +Cc: rui.zhang, hongbo.zhang, sachin.kamat, patches
Silences the following sparse warning:
drivers/thermal/cpu_cooling.c:67:31: warning:
symbol 'notify_device' was not declared. Should it be static?
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
drivers/thermal/cpu_cooling.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 392d57d..6f94c2c 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -64,7 +64,7 @@ static unsigned int cpufreq_dev_count;
/* notify_table passes value to the CPUFREQ_ADJUST callback function. */
#define NOTIFY_INVALID NULL
-struct cpufreq_cooling_device *notify_device;
+static struct cpufreq_cooling_device *notify_device;
/**
* get_idr - function to get a unique id.
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH-Resend] thermal: Exynos: Add missing dependency
From: Amit Kachhap @ 2012-11-15 7:01 UTC (permalink / raw)
To: Sachin Kamat, Zhang Rui; +Cc: linux-pm, patches, akpm, linux-samsung-soc
In-Reply-To: <1352961579-13807-1-git-send-email-sachin.kamat@linaro.org>
On 15 November 2012 12:09, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> CPU_FREQ_TABLE depends on CPU_FREQ. Selecting CPU_FREQ_TABLE without checking
> for dependencies gives the following compilation warnings:
> warning: (ARCH_TEGRA_2x_SOC && ARCH_TEGRA_3x_SOC && UX500_SOC_DB8500 &&
> CPU_THERMAL && EXYNOS_THERMAL) selects CPU_FREQ_TABLE which has unmet
> direct dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ)
>
> Cc: Amit Daniel Kachhap <amit.kachhap@linaro.org>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
> drivers/thermal/Kconfig | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 266c15e..6124c4b 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -50,8 +50,7 @@ config RCAR_THERMAL
>
> config EXYNOS_THERMAL
> tristate "Temperature sensor on Samsung EXYNOS"
> - depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
> - select CPU_FREQ_TABLE
Hi Sachin/Rui,
"select CPU_FREQ_TABLE" should be there even if CONFIG_CPU_FREQ is
enabled. Only some governors enable it. See drivers/cpufreq/Kconfig
Also "depends THERMAL" should be there otherwise menuconfig
parent/child relationship is broken.
Thanks,
Amit Daniel
> + depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && CPU_THERMAL
> help
> If you say yes here you get support for TMU (Thermal Managment
> Unit) on SAMSUNG EXYNOS series of SoC.
> --
> 1.7.4.1
>
^ permalink raw reply
* [PATCH 1/2] Thermal: drivers/thermal/Kconfig refactor
From: Zhang Rui @ 2012-11-15 7:01 UTC (permalink / raw)
To: Linux PM list; +Cc: Sachin Kamat, Amit Kachhap, Zhang, Rui, patches
drivers/thermal/Kconfig refactor.
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/thermal/Kconfig | 103 +++++++++++++++++++++++------------------------
1 file changed, 51 insertions(+), 52 deletions(-)
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 266c15e..937a23d 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -13,15 +13,62 @@ menuconfig THERMAL
All platforms with ACPI thermal support can use this driver.
If you want this support, you should say Y or M here.
+if THERMAL
+
config THERMAL_HWMON
bool
- depends on THERMAL
depends on HWMON=y || HWMON=THERMAL
default y
+choice
+ prompt "Default Thermal governor"
+ default THERMAL_DEFAULT_GOV_STEP_WISE
+ help
+ This option sets which thermal governor shall be loaded at
+ startup. If in doubt, select 'step_wise'.
+
+config THERMAL_DEFAULT_GOV_STEP_WISE
+ bool "step_wise"
+ select STEP_WISE
+ help
+ Use the step_wise governor as default. This throttles the
+ devices one step at a time.
+
+config THERMAL_DEFAULT_GOV_FAIR_SHARE
+ bool "fair_share"
+ select FAIR_SHARE
+ help
+ Use the fair_share governor as default. This throttles the
+ devices based on their 'contribution' to a zone. The
+ contribution should be provided through platform data.
+
+config THERMAL_DEFAULT_GOV_USER_SPACE
+ bool "user_space"
+ select USER_SPACE
+ help
+ Select this if you want to let the user space manage the
+ lpatform thermals.
+
+endchoice
+
+config FAIR_SHARE
+ bool "Fair-share thermal governor"
+ help
+ Enable this to manage platform thermals using fair-share governor.
+
+config STEP_WISE
+ bool "Step_wise thermal governor"
+ help
+ Enable this to manage platform thermals using a simple linear
+
+config USER_SPACE
+ bool "User_space thermal governor"
+ help
+ Enable this to let the user space manage the platform thermals.
+
config CPU_THERMAL
bool "generic cpu cooling support"
- depends on THERMAL && CPU_FREQ
+ depends on CPU_FREQ
select CPU_FREQ_TABLE
help
This implements the generic cpu cooling mechanism through frequency
@@ -33,7 +80,6 @@ config CPU_THERMAL
config SPEAR_THERMAL
bool "SPEAr thermal sensor driver"
- depends on THERMAL
depends on PLAT_SPEAR
depends on OF
help
@@ -42,7 +88,6 @@ config SPEAR_THERMAL
config RCAR_THERMAL
tristate "Renesas R-Car thermal driver"
- depends on THERMAL
depends on ARCH_SHMOBILE
help
Enable this to plug the R-Car thermal sensor driver into the Linux
@@ -50,57 +95,11 @@ config RCAR_THERMAL
config EXYNOS_THERMAL
tristate "Temperature sensor on Samsung EXYNOS"
- depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
+ depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5)
select CPU_FREQ_TABLE
help
If you say yes here you get support for TMU (Thermal Managment
Unit) on SAMSUNG EXYNOS series of SoC.
-config FAIR_SHARE
- bool "Fair-share thermal governor"
- depends on THERMAL
- help
- Enable this to manage platform thermals using fair-share governor.
-
-config STEP_WISE
- bool "Step_wise thermal governor"
- depends on THERMAL
- help
- Enable this to manage platform thermals using a simple linear
-
-config USER_SPACE
- bool "User_space thermal governor"
- depends on THERMAL
- help
- Enable this to let the user space manage the platform thermals.
-
-choice
- prompt "Default Thermal governor"
- depends on THERMAL
- default THERMAL_DEFAULT_GOV_STEP_WISE
- help
- This option sets which thermal governor shall be loaded at
- startup. If in doubt, select 'step_wise'.
-
-config THERMAL_DEFAULT_GOV_STEP_WISE
- bool "step_wise"
- select STEP_WISE
- help
- Use the step_wise governor as default. This throttles the
- devices one step at a time.
-
-config THERMAL_DEFAULT_GOV_FAIR_SHARE
- bool "fair_share"
- select FAIR_SHARE
- help
- Use the fair_share governor as default. This throttles the
- devices based on their 'contribution' to a zone. The
- contribution should be provided through platform data.
-config THERMAL_DEFAULT_GOV_USER_SPACE
- bool "user_space"
- select USER_SPACE
- help
- Select this if you want to let the user space manage the
- platform thermals.
-endchoice
+endif
--
1.7.9.5
^ permalink raw reply related
* [RFC PATCH 2/2] Thermal: Exynos : add missing dependency
From: Zhang Rui @ 2012-11-15 7:02 UTC (permalink / raw)
To: Linux PM list; +Cc: Sachin Kamat, Amit Kachhap, Zhang, Rui, patches
there is still something unclear to me.
Amit, the next step should be remove this "depends on CPU_THERMAL"
line for EXYNOS driver, right?
If we do this, the exynos driver should work w/ or w/o CPU_THERMAL,
but should we have something like,
"hey, I like CPU_THERMAL, I work better with it set"?
"Select" does not work here
because it may block exynos driver when CPU_FREQ cleared.
any ideas on this?
thanks,
rui
CPU_FREQ_TABLE depends on CPU_FREQ. Selecting
CPU_FREQ_TABLE without checking for dependencies gives
the following compilation warnings: warning:
(ARCH_TEGRA_2x_SOC && ARCH_TEGRA_3x_SOC &&
UX500_SOC_DB8500 && CPU_THERMAL && EXYNOS_THERMAL)
selects CPU_FREQ_TABLE which has unmet direct
dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ)
Patch-based-on: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/thermal/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 937a23d..99b6587 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -96,7 +96,7 @@ config RCAR_THERMAL
config EXYNOS_THERMAL
tristate "Temperature sensor on Samsung EXYNOS"
depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5)
- select CPU_FREQ_TABLE
+ depends on CPU_THERMAL
help
If you say yes here you get support for TMU (Thermal Managment
Unit) on SAMSUNG EXYNOS series of SoC.
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH-Resend] thermal: Exynos: Add missing dependency
From: Zhang Rui @ 2012-11-15 7:04 UTC (permalink / raw)
To: Amit Kachhap; +Cc: Sachin Kamat, linux-pm, patches, akpm, linux-samsung-soc
In-Reply-To: <CAK44p21smO8rrKoJF=47ywRCeagDudvFzZ7DhJ7TZCT7Mqv7kA@mail.gmail.com>
On Thu, 2012-11-15 at 12:31 +0530, Amit Kachhap wrote:
> On 15 November 2012 12:09, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> > CPU_FREQ_TABLE depends on CPU_FREQ. Selecting CPU_FREQ_TABLE without checking
> > for dependencies gives the following compilation warnings:
> > warning: (ARCH_TEGRA_2x_SOC && ARCH_TEGRA_3x_SOC && UX500_SOC_DB8500 &&
> > CPU_THERMAL && EXYNOS_THERMAL) selects CPU_FREQ_TABLE which has unmet
> > direct dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ)
> >
> > Cc: Amit Daniel Kachhap <amit.kachhap@linaro.org>
> > Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> > ---
> > drivers/thermal/Kconfig | 3 +--
> > 1 files changed, 1 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > index 266c15e..6124c4b 100644
> > --- a/drivers/thermal/Kconfig
> > +++ b/drivers/thermal/Kconfig
> > @@ -50,8 +50,7 @@ config RCAR_THERMAL
> >
> > config EXYNOS_THERMAL
> > tristate "Temperature sensor on Samsung EXYNOS"
> > - depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
> > - select CPU_FREQ_TABLE
> Hi Sachin/Rui,
>
> "select CPU_FREQ_TABLE" should be there even if CONFIG_CPU_FREQ is
> enabled. Only some governors enable it.
But CPU_THERMAL will select it, right?
> See drivers/cpufreq/Kconfig
> Also "depends THERMAL" should be there otherwise menuconfig
> parent/child relationship is broken.
>
this should be solved by my patch 1/2. :)
thanks,
rui
> Thanks,
> Amit Daniel
>
> > + depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && CPU_THERMAL
> > help
> > If you say yes here you get support for TMU (Thermal Managment
> > Unit) on SAMSUNG EXYNOS series of SoC.
> > --
> > 1.7.4.1
> >
^ permalink raw reply
* Re: [PATCH-Resend] thermal: Exynos: Add missing dependency
From: Amit Kachhap @ 2012-11-15 7:07 UTC (permalink / raw)
To: Zhang Rui; +Cc: Sachin Kamat, linux-pm, patches, akpm, linux-samsung-soc
In-Reply-To: <1352963065.2080.12.camel@rzhang1-mobl4>
On 15 November 2012 12:34, Zhang Rui <rui.zhang@intel.com> wrote:
> On Thu, 2012-11-15 at 12:31 +0530, Amit Kachhap wrote:
>> On 15 November 2012 12:09, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>> > CPU_FREQ_TABLE depends on CPU_FREQ. Selecting CPU_FREQ_TABLE without checking
>> > for dependencies gives the following compilation warnings:
>> > warning: (ARCH_TEGRA_2x_SOC && ARCH_TEGRA_3x_SOC && UX500_SOC_DB8500 &&
>> > CPU_THERMAL && EXYNOS_THERMAL) selects CPU_FREQ_TABLE which has unmet
>> > direct dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ)
>> >
>> > Cc: Amit Daniel Kachhap <amit.kachhap@linaro.org>
>> > Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> > ---
>> > drivers/thermal/Kconfig | 3 +--
>> > 1 files changed, 1 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> > index 266c15e..6124c4b 100644
>> > --- a/drivers/thermal/Kconfig
>> > +++ b/drivers/thermal/Kconfig
>> > @@ -50,8 +50,7 @@ config RCAR_THERMAL
>> >
>> > config EXYNOS_THERMAL
>> > tristate "Temperature sensor on Samsung EXYNOS"
>> > - depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && THERMAL
>> > - select CPU_FREQ_TABLE
>> Hi Sachin/Rui,
>>
>> "select CPU_FREQ_TABLE" should be there even if CONFIG_CPU_FREQ is
>> enabled. Only some governors enable it.
>
> But CPU_THERMAL will select it, right?
Ohh yes correct. Thanks
>
>> See drivers/cpufreq/Kconfig
>> Also "depends THERMAL" should be there otherwise menuconfig
>> parent/child relationship is broken.
>>
> this should be solved by my patch 1/2. :)
>
> thanks,
> rui
>
>> Thanks,
>> Amit Daniel
>>
>> > + depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5) && CPU_THERMAL
>> > help
>> > If you say yes here you get support for TMU (Thermal Managment
>> > Unit) on SAMSUNG EXYNOS series of SoC.
>> > --
>> > 1.7.4.1
>> >
>
>
^ permalink raw reply
* Re: [PATCH 1/1] thermal: cpu_cooling: Make 'notify_device' static
From: Zhang Rui @ 2012-11-15 7:43 UTC (permalink / raw)
To: Sachin Kamat; +Cc: linux-pm, hongbo.zhang, patches
In-Reply-To: <1352962184-20970-1-git-send-email-sachin.kamat@linaro.org>
On Thu, 2012-11-15 at 12:19 +0530, Sachin Kamat wrote:
> Silences the following sparse warning:
>
> drivers/thermal/cpu_cooling.c:67:31: warning:
> symbol 'notify_device' was not declared. Should it be static?
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
applied to therml-next.
thanks,
rui
> ---
> drivers/thermal/cpu_cooling.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index 392d57d..6f94c2c 100644
> --- a/drivers/thermal/cpu_cooling.c
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -64,7 +64,7 @@ static unsigned int cpufreq_dev_count;
>
> /* notify_table passes value to the CPUFREQ_ADJUST callback function. */
> #define NOTIFY_INVALID NULL
> -struct cpufreq_cooling_device *notify_device;
> +static struct cpufreq_cooling_device *notify_device;
>
> /**
> * get_idr - function to get a unique id.
^ permalink raw reply
* Re: [PATCH V5 1/2] Thermal: Add ST-Ericsson DB8500 thermal driver.
From: Zhang Rui @ 2012-11-15 8:13 UTC (permalink / raw)
To: hongbo.zhang
Cc: linux-acpi, linux-kernel, linux-pm, amit.kachhap, patches,
linaro-dev, linaro-kernel, STEricsson_nomadik_linux, kernel,
hongbo.zhang
In-Reply-To: <1352460548-3494-2-git-send-email-hongbo.zhang@linaro.com>
On Fri, 2012-11-09 at 19:29 +0800, hongbo.zhang wrote:
> From: "hongbo.zhang" <hongbo.zhang@linaro.com>
>
> This driver is based on the thermal management framework in thermal_sys.c. A
> thermal zone device is created with the trip points to which cooling devices
> can be bound, the current cooling device is cpufreq, e.g. CPU frequency is
> clipped down to cool the CPU, and other cooling devices can be added and bound
> to the trip points dynamically. The platform specific PRCMU interrupts are
> used to active thermal update when trip points are reached.
>
> Signed-off-by: hongbo.zhang <hongbo.zhang@linaro.com>
> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
> Reviewed-by: Francesco Lavra <francescolavra.fl@gmail.com>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index e1cb6bd..54c8fd0 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -31,6 +31,26 @@ config CPU_THERMAL
> and not the ACPI interface.
> If you want this support, you should say Y here.
>
> +config DB8500_THERMAL
> + bool "DB8500 thermal management"
why is this bool?
thanks,
rui
^ permalink raw reply
* Re: [PATCH V5 1/2] Thermal: Add ST-Ericsson DB8500 thermal driver.
From: Hongbo Zhang @ 2012-11-15 8:32 UTC (permalink / raw)
To: Zhang Rui
Cc: linux-acpi, linux-kernel, linux-pm, amit.kachhap, patches,
linaro-dev, linaro-kernel, STEricsson_nomadik_linux, kernel,
hongbo.zhang
In-Reply-To: <1352967218.2080.15.camel@rzhang1-mobl4>
On 15 November 2012 16:13, Zhang Rui <rui.zhang@intel.com> wrote:
> On Fri, 2012-11-09 at 19:29 +0800, hongbo.zhang wrote:
>> From: "hongbo.zhang" <hongbo.zhang@linaro.com>
>>
>> This driver is based on the thermal management framework in thermal_sys.c. A
>> thermal zone device is created with the trip points to which cooling devices
>> can be bound, the current cooling device is cpufreq, e.g. CPU frequency is
>> clipped down to cool the CPU, and other cooling devices can be added and bound
>> to the trip points dynamically. The platform specific PRCMU interrupts are
>> used to active thermal update when trip points are reached.
>>
>> Signed-off-by: hongbo.zhang <hongbo.zhang@linaro.com>
>> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
>> Reviewed-by: Francesco Lavra <francescolavra.fl@gmail.com>
>
>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index e1cb6bd..54c8fd0 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -31,6 +31,26 @@ config CPU_THERMAL
>> and not the ACPI interface.
>> If you want this support, you should say Y here.
>>
>> +config DB8500_THERMAL
>> + bool "DB8500 thermal management"
>
> why is this bool?
platform specific PRCMU interfaces are used, and those interfaces are
not exported, so my driver cannot be compiled as a module.
>
> thanks,
> rui
>
>
> --
> 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
* Re: [RFC PATCH 2/2] Thermal: Exynos : add missing dependency
From: Amit Kachhap @ 2012-11-15 8:41 UTC (permalink / raw)
To: Zhang Rui; +Cc: Linux PM list, Sachin Kamat, patches
In-Reply-To: <1352962947.2080.10.camel@rzhang1-mobl4>
On 15 November 2012 12:32, Zhang Rui <rui.zhang@intel.com> wrote:
> there is still something unclear to me.
>
> Amit, the next step should be remove this "depends on CPU_THERMAL"
> line for EXYNOS driver, right?
Yes I will refactor the sensor code and framework registration code.
Now it is somewhat tied together.
> If we do this, the exynos driver should work w/ or w/o CPU_THERMAL,
> but should we have something like,
> "hey, I like CPU_THERMAL, I work better with it set"?
>
> "Select" does not work here
> because it may block exynos driver when CPU_FREQ cleared.
Good idea but as Select does not enable all dependencies recursively
so we ourself needs to resolve all. "depends on " is a easy way to do
it with just 1 macro.
BTW current exynos driver needs both CPU_FREQ and CPU_THERMAL so your
patch is fine.
Thanks,
Amit Daniel
>
> any ideas on this?
>
> thanks,
> rui
>
> CPU_FREQ_TABLE depends on CPU_FREQ. Selecting
> CPU_FREQ_TABLE without checking for dependencies gives
> the following compilation warnings: warning:
> (ARCH_TEGRA_2x_SOC && ARCH_TEGRA_3x_SOC &&
> UX500_SOC_DB8500 && CPU_THERMAL && EXYNOS_THERMAL)
> selects CPU_FREQ_TABLE which has unmet direct
> dependencies (ARCH_HAS_CPUFREQ && CPU_FREQ)
>
> Patch-based-on: Sachin Kamat <sachin.kamat@linaro.org>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
> drivers/thermal/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 937a23d..99b6587 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -96,7 +96,7 @@ config RCAR_THERMAL
> config EXYNOS_THERMAL
> tristate "Temperature sensor on Samsung EXYNOS"
> depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5)
> - select CPU_FREQ_TABLE
> + depends on CPU_THERMAL
> help
> If you say yes here you get support for TMU (Thermal Managment
> Unit) on SAMSUNG EXYNOS series of SoC.
> --
> 1.7.9.5
>
>
>
^ permalink raw reply
* Re: [PATCH] cpuidle: Measure idle state durations with monotonic clock
From: Preeti Murthy @ 2012-11-15 9:04 UTC (permalink / raw)
To: Julius Werner
Cc: linux-kernel, Len Brown, Rafael J. Wysocki, Kevin Hilman,
Andrew Morton, Srivatsa S. Bhat, linux-acpi, linux-pm,
linuxppc-dev, Deepthi Dharwar, Trinabh Gupta, Sameer Nanda,
Lists Linaro-dev, Daniel Lezcano, Peter Zijlstra
In-Reply-To: <1352944590-8776-1-git-send-email-jwerner@chromium.org>
Hi all,
The code looks correct and inviting to me as it has led to good cleanups.
I dont think passing 0 as the argument to the function
sched_clock_idle_wakeup_event()
should lead to problems,as it does not do anything useful with the
passed arguments.
My only curiosity is what was the purpose of passing idle residency time to
sched_clock_idle_wakeup_event() when this data could always be retrieved from
dev->last_residency for each cpu,which gets almost immediately updated.
But this does not seem to come in way of this patch for now.Anyway I
have added Peter to
the list so that he can opine about this issue if possible and needed.
Reviewed-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
Regards
Preeti U Murthy
^ permalink raw reply
* Re: [PATCH V5 1/2] Thermal: Add ST-Ericsson DB8500 thermal driver.
From: Zhang Rui @ 2012-11-15 9:13 UTC (permalink / raw)
To: Hongbo Zhang
Cc: linux-acpi, linux-kernel, linux-pm, amit.kachhap, patches,
linaro-dev, linaro-kernel, STEricsson_nomadik_linux, kernel,
hongbo.zhang
In-Reply-To: <CAJLyvQyu_oU7xx92JTOj6bvF+7-hOJfM=kTHj6JMogaYjC+Jww@mail.gmail.com>
On Thu, 2012-11-15 at 16:32 +0800, Hongbo Zhang wrote:
> On 15 November 2012 16:13, Zhang Rui <rui.zhang@intel.com> wrote:
> > On Fri, 2012-11-09 at 19:29 +0800, hongbo.zhang wrote:
> >> From: "hongbo.zhang" <hongbo.zhang@linaro.com>
> >>
> >> This driver is based on the thermal management framework in thermal_sys.c. A
> >> thermal zone device is created with the trip points to which cooling devices
> >> can be bound, the current cooling device is cpufreq, e.g. CPU frequency is
> >> clipped down to cool the CPU, and other cooling devices can be added and bound
> >> to the trip points dynamically. The platform specific PRCMU interrupts are
> >> used to active thermal update when trip points are reached.
> >>
> >> Signed-off-by: hongbo.zhang <hongbo.zhang@linaro.com>
> >> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
> >> Reviewed-by: Francesco Lavra <francescolavra.fl@gmail.com>
> >
> >> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> >> index e1cb6bd..54c8fd0 100644
> >> --- a/drivers/thermal/Kconfig
> >> +++ b/drivers/thermal/Kconfig
> >> @@ -31,6 +31,26 @@ config CPU_THERMAL
> >> and not the ACPI interface.
> >> If you want this support, you should say Y here.
> >>
> >> +config DB8500_THERMAL
> >> + bool "DB8500 thermal management"
> >
> > why is this bool?
>
> platform specific PRCMU interfaces are used, and those interfaces are
> not exported, so my driver cannot be compiled as a module.
>
ok.
But I still alway get this error message when set DB8500_THERMAL,
include/linux/mfd/dbx500-prcmu.h:459:19: error: redefinition of
‘prcmu_abb_read’
include/linux/mfd/db8500-prcmu.h:673:19: note: previous definition of
‘prcmu_abb_read’ was here
include/linux/mfd/dbx500-prcmu.h:464:19: error: redefinition of
‘prcmu_abb_write’
include/linux/mfd/db8500-prcmu.h:678:19: note: previous definition of
‘prcmu_abb_write’ was here
include/linux/mfd/dbx500-prcmu.h:475:19: error: redefinition of
‘prcmu_config_clkout’
include/linux/mfd/db8500-prcmu.h:643:19: note: previous definition of
‘prcmu_config_clkout’ was here
include/linux/mfd/dbx500-prcmu.h:537:19: error: redefinition of
‘prcmu_ac_wake_req’
include/linux/mfd/db8500-prcmu.h:683:19: note: previous definition of
‘prcmu_ac_wake_req’ was here
include/linux/mfd/dbx500-prcmu.h:542:20: error: redefinition of
‘prcmu_ac_sleep_req’
include/linux/mfd/db8500-prcmu.h:688:20: note: previous definition of
‘prcmu_ac_sleep_req’ was here
$ grep prcmu_abb_read include/linux/mfd/*
include/linux/mfd/db8500-prcmu.h:int prcmu_abb_read(u8 slave, u8 reg, u8
*value, u8 size);
include/linux/mfd/db8500-prcmu.h:static inline int prcmu_abb_read(u8
slave, u8 reg, u8 *value, u8 size)
include/linux/mfd/dbx500-prcmu.h:int prcmu_abb_read(u8 slave, u8 reg, u8
*value, u8 size);
include/linux/mfd/dbx500-prcmu.h:static inline int prcmu_abb_read(u8
slave, u8 reg, u8 *value, u8 size)
this functions are defined in both db8500-prmcu.h and dbx500-prcmu.h,
plus linux/mfd/dbx500-prcmu.h includes linux/mfd/db8500-prcmu.h.
I do not know how it works before, but this is a bug to me.
thanks,
rui
^ permalink raw reply
* Re: [PATCH V5 1/2] Thermal: Add ST-Ericsson DB8500 thermal driver.
From: Hongbo Zhang @ 2012-11-15 9:23 UTC (permalink / raw)
To: Zhang Rui
Cc: linux-acpi, linux-kernel, linux-pm, amit.kachhap, patches,
linaro-dev, linaro-kernel, STEricsson_nomadik_linux, kernel,
hongbo.zhang
In-Reply-To: <1352970823.2080.22.camel@rzhang1-mobl4>
this is a driver for ST-Ericsson u8500 board(Snowball), with a ARM core inside.
so you should compile like this:
make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- u8500_defconfig
make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- uImage
On 15 November 2012 17:13, Zhang Rui <rui.zhang@intel.com> wrote:
> On Thu, 2012-11-15 at 16:32 +0800, Hongbo Zhang wrote:
>> On 15 November 2012 16:13, Zhang Rui <rui.zhang@intel.com> wrote:
>> > On Fri, 2012-11-09 at 19:29 +0800, hongbo.zhang wrote:
>> >> From: "hongbo.zhang" <hongbo.zhang@linaro.com>
>> >>
>> >> This driver is based on the thermal management framework in thermal_sys.c. A
>> >> thermal zone device is created with the trip points to which cooling devices
>> >> can be bound, the current cooling device is cpufreq, e.g. CPU frequency is
>> >> clipped down to cool the CPU, and other cooling devices can be added and bound
>> >> to the trip points dynamically. The platform specific PRCMU interrupts are
>> >> used to active thermal update when trip points are reached.
>> >>
>> >> Signed-off-by: hongbo.zhang <hongbo.zhang@linaro.com>
>> >> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
>> >> Reviewed-by: Francesco Lavra <francescolavra.fl@gmail.com>
>> >
>> >> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> >> index e1cb6bd..54c8fd0 100644
>> >> --- a/drivers/thermal/Kconfig
>> >> +++ b/drivers/thermal/Kconfig
>> >> @@ -31,6 +31,26 @@ config CPU_THERMAL
>> >> and not the ACPI interface.
>> >> If you want this support, you should say Y here.
>> >>
>> >> +config DB8500_THERMAL
>> >> + bool "DB8500 thermal management"
>> >
>> > why is this bool?
>>
>> platform specific PRCMU interfaces are used, and those interfaces are
>> not exported, so my driver cannot be compiled as a module.
>>
> ok.
> But I still alway get this error message when set DB8500_THERMAL,
> include/linux/mfd/dbx500-prcmu.h:459:19: error: redefinition of
> ‘prcmu_abb_read’
> include/linux/mfd/db8500-prcmu.h:673:19: note: previous definition of
> ‘prcmu_abb_read’ was here
> include/linux/mfd/dbx500-prcmu.h:464:19: error: redefinition of
> ‘prcmu_abb_write’
> include/linux/mfd/db8500-prcmu.h:678:19: note: previous definition of
> ‘prcmu_abb_write’ was here
> include/linux/mfd/dbx500-prcmu.h:475:19: error: redefinition of
> ‘prcmu_config_clkout’
> include/linux/mfd/db8500-prcmu.h:643:19: note: previous definition of
> ‘prcmu_config_clkout’ was here
> include/linux/mfd/dbx500-prcmu.h:537:19: error: redefinition of
> ‘prcmu_ac_wake_req’
> include/linux/mfd/db8500-prcmu.h:683:19: note: previous definition of
> ‘prcmu_ac_wake_req’ was here
> include/linux/mfd/dbx500-prcmu.h:542:20: error: redefinition of
> ‘prcmu_ac_sleep_req’
> include/linux/mfd/db8500-prcmu.h:688:20: note: previous definition of
> ‘prcmu_ac_sleep_req’ was here
>
> $ grep prcmu_abb_read include/linux/mfd/*
> include/linux/mfd/db8500-prcmu.h:int prcmu_abb_read(u8 slave, u8 reg, u8
> *value, u8 size);
> include/linux/mfd/db8500-prcmu.h:static inline int prcmu_abb_read(u8
> slave, u8 reg, u8 *value, u8 size)
> include/linux/mfd/dbx500-prcmu.h:int prcmu_abb_read(u8 slave, u8 reg, u8
> *value, u8 size);
> include/linux/mfd/dbx500-prcmu.h:static inline int prcmu_abb_read(u8
> slave, u8 reg, u8 *value, u8 size)
>
> this functions are defined in both db8500-prmcu.h and dbx500-prcmu.h,
> plus linux/mfd/dbx500-prcmu.h includes linux/mfd/db8500-prcmu.h.
>
> I do not know how it works before, but this is a bug to me.
>
> thanks,
> rui
>
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" 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: [BUGFIX] PM: Fix active child counting when disabled and forbidden
From: Rafael J. Wysocki @ 2012-11-15 9:51 UTC (permalink / raw)
To: Huang Ying, Alan Stern; +Cc: linux-kernel, linux-pm
In-Reply-To: <1352941424.7176.261.camel@yhuang-dev>
On Thursday, November 15, 2012 09:03:44 AM Huang Ying wrote:
> On Thu, 2012-11-15 at 00:10 +0100, Rafael J. Wysocki wrote:
> > On Wednesday, November 14, 2012 04:45:01 PM Alan Stern wrote:
> > > On Wed, 14 Nov 2012, Rafael J. Wysocki wrote:
> > >
> > > > > This has the side effect that when a driver unbinds, it can't leave the
> > > > > device in a special low-power state. The device will always end up in
> > > > > the generic low-power state supported by the PCI core.
> > > >
> > > > Well, I'm not sure I'd like that.
> > > >
> > > > Let's just go back even one step more and think what we'd like to have in
> > > > general terms and then how to implement it. :-)
> > > >
> > > > Suppose that pci_pm_init() calls pm_runtime_enable() for all devices (in
> > > > addition to what it does currently). The runtime PM status of each device is
> > > > RPM_SUSPENDED at this point. Then:
> > >
> > > Wait a moment. When the device is detected and initialized, it is in
> > > D0, right? Currently we don't care much because the device starts out
> > > disabled for runtime PM. But now you are going to enable it. While
> > > the device is enabled, its runtime status should match the physical
> > > power level.
> >
> > OK
>
> If my memory were correct, RPM_SUSPENDED just means device stop working,
> but need not be put into low-power state. So for RPM_ACTIVE, PCI
> devices should be in D0, but for RPM_SUSPENDED, PCI devices can in any
> power state.
Yes, that's correct and I was wrong when I thought we could require the
status to be RPM_ACTIVE all the time when there's no driver, because that
would prevent parents from being suspended. And we want them to be able to
suspend for driverless children, _unless_ user space has its attribute set
to "on" (i.e. the default).
So it looks like what we want to do is:
(1) Enable runtime PM in pci_pm_init() and set the status to RPM_ACTIVE right
before, so that it is in agreement with the pm_runtime_forbid() we do in
there.
(2) If user space switches its attribute to "off" later, but before any
drivers are probed, we want the status to switch to RPM_SUSPENDED
_without_ actually changing the devices power state. For that,
I think, we can make the PCI bus type's runtime PM callback ignore
devices without drivers (i.e. return 0 for them).
(3) When local_pci_probe() starts, after we've resumed the parent,
the device will be in D0 (it may be D0-uninitialized, though).
If the user space's attribute is "on" at this point, the parent's
resume doesn't change anything. If it is "auto", the parent's
resume may actually transition the device, although its status
will still be RPM_SUSPENDED. For consistency _and_ compatibility
with the current code, the driver's .probe() routine needs to see
the device RPM_ACTIVE and usage_count incremented, but we don't
want to run its PM callbacks _before_ .probe() runs. For that
to work, I think, we can do something like pm_runtime_get_skip_callbacks(),
treating the device as though it had the power.no_callbacks flag set,
right before calling ddi->drv->probe().
If the device has been RPM_ACTIVE at that point (i.e. user space has
had its attribute set to "on") it will just bump up usage_count (which
is what we want). If the device has been RPM_SUSPENDED, it will
bump up usage_count _and_ change the status to RPM_ACTIVE without
executing any callbacks (the device is in D0 anyway, right?), which
is what we want too.
(4) If ddi->drv->probe() succeeds, we don't want to change anything, so
as not to confuse the driver, which is now in control of the device.
(5) If ddi->drv->probe() fails, we need to restore the situation from
before calling local_pci_probe(), but we want the pm_runtime_put(parent)
at the end of it to actually suspend the parent if user space has
its attribute (for the child!) set to "auto".
Assume that the driver is not buggy and the failing ddi->drv->probe()
leaves the device in the same configuration as it's received it in.
Then, the device is RPM_ACTIVE and in D0 (which may be uninitialized).
For the parent's suspend to work, we need to transition it to
RPM_SUSPENDED, but again we don't want the driver's PM callbacks to
run at this point. Moreover, we don't want the PCI bus type's
callbacks to run at this point, because dev->driver is still set.
So again, doing something like pm_runtime_put_skip_callbacks(),
treating the device as though it had power.no_callbacks set, seems
to be appropriate.
Namely, if the user space's attribute is "on", it will just drop
usage_count by 1, which is what we want in that case. If the user
space's attribute is "auto", on the other hand, it will drop
usage_count by 1 and change the status to RPM_SUSPENDED without
running callbacks, which again is what we want.
(6) In drv->remove() the driver is supposed to bump up usage_count by 1,
so as to restore the situation from before its .probe() routine
was called. It also should leave the device as RPM_ACTIVE, because
that's what it's got in .probe(). Then, after drv->remove exits,
(and also if drv was NULL to start with), we want to drop usage_count
by 1. Moreover, if the user space's attribute is "on", we don't
want anything more to happen, _but_ if that's "auto", we want to
suspend the parent.
Note that dev->driver is still not NULL at this point (although
pci_dev->driver is!) so again we can't run the PCI bus type's callbacks.
It looks like, then, what we want to do here is
pm_runtime_put_skip_callbacks() again, because if the user space's
attribute is "on", it will just drop usage_count by 1, which is what
we want, and if that's "auto", it will additionally change the status
to RPM_SUSPENDED (without executing callbacks, which we want) _and_
it will queue up the parent's suspend (which, again, is what we want).
Did I miss anything?
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [BUGFIX] PM: Fix active child counting when disabled and forbidden
From: Rafael J. Wysocki @ 2012-11-15 10:09 UTC (permalink / raw)
To: Huang Ying; +Cc: Alan Stern, linux-kernel, linux-pm
In-Reply-To: <50117675.OVEdDFPCp3@vostro.rjw.lan>
On Thursday, November 15, 2012 10:51:42 AM Rafael J. Wysocki wrote:
> On Thursday, November 15, 2012 09:03:44 AM Huang Ying wrote:
> > On Thu, 2012-11-15 at 00:10 +0100, Rafael J. Wysocki wrote:
> > > On Wednesday, November 14, 2012 04:45:01 PM Alan Stern wrote:
> > > > On Wed, 14 Nov 2012, Rafael J. Wysocki wrote:
> > > >
> > > > > > This has the side effect that when a driver unbinds, it can't leave the
> > > > > > device in a special low-power state. The device will always end up in
> > > > > > the generic low-power state supported by the PCI core.
> > > > >
> > > > > Well, I'm not sure I'd like that.
> > > > >
> > > > > Let's just go back even one step more and think what we'd like to have in
> > > > > general terms and then how to implement it. :-)
> > > > >
> > > > > Suppose that pci_pm_init() calls pm_runtime_enable() for all devices (in
> > > > > addition to what it does currently). The runtime PM status of each device is
> > > > > RPM_SUSPENDED at this point. Then:
> > > >
> > > > Wait a moment. When the device is detected and initialized, it is in
> > > > D0, right? Currently we don't care much because the device starts out
> > > > disabled for runtime PM. But now you are going to enable it. While
> > > > the device is enabled, its runtime status should match the physical
> > > > power level.
> > >
> > > OK
> >
> > If my memory were correct, RPM_SUSPENDED just means device stop working,
> > but need not be put into low-power state. So for RPM_ACTIVE, PCI
> > devices should be in D0, but for RPM_SUSPENDED, PCI devices can in any
> > power state.
>
> Yes, that's correct and I was wrong when I thought we could require the
> status to be RPM_ACTIVE all the time when there's no driver, because that
> would prevent parents from being suspended. And we want them to be able to
> suspend for driverless children, _unless_ user space has its attribute set
> to "on" (i.e. the default).
>
> So it looks like what we want to do is:
>
> (1) Enable runtime PM in pci_pm_init() and set the status to RPM_ACTIVE right
> before, so that it is in agreement with the pm_runtime_forbid() we do in
> there.
>
> (2) If user space switches its attribute to "off" later, but before any
> drivers are probed, we want the status to switch to RPM_SUSPENDED
> _without_ actually changing the devices power state. For that,
> I think, we can make the PCI bus type's runtime PM callback ignore
> devices without drivers (i.e. return 0 for them).
>
> (3) When local_pci_probe() starts, after we've resumed the parent,
> the device will be in D0 (it may be D0-uninitialized, though).
> If the user space's attribute is "on" at this point, the parent's
> resume doesn't change anything. If it is "auto", the parent's
> resume may actually transition the device, although its status
> will still be RPM_SUSPENDED. For consistency _and_ compatibility
> with the current code, the driver's .probe() routine needs to see
> the device RPM_ACTIVE and usage_count incremented, but we don't
> want to run its PM callbacks _before_ .probe() runs. For that
> to work, I think, we can do something like pm_runtime_get_skip_callbacks(),
> treating the device as though it had the power.no_callbacks flag set,
> right before calling ddi->drv->probe().
>
> If the device has been RPM_ACTIVE at that point (i.e. user space has
> had its attribute set to "on") it will just bump up usage_count (which
> is what we want). If the device has been RPM_SUSPENDED, it will
> bump up usage_count _and_ change the status to RPM_ACTIVE without
> executing any callbacks (the device is in D0 anyway, right?), which
> is what we want too.
>
> (4) If ddi->drv->probe() succeeds, we don't want to change anything, so
> as not to confuse the driver, which is now in control of the device.
>
> (5) If ddi->drv->probe() fails, we need to restore the situation from
> before calling local_pci_probe(), but we want the pm_runtime_put(parent)
> at the end of it to actually suspend the parent if user space has
> its attribute (for the child!) set to "auto".
>
> Assume that the driver is not buggy and the failing ddi->drv->probe()
> leaves the device in the same configuration as it's received it in.
> Then, the device is RPM_ACTIVE and in D0 (which may be uninitialized).
> For the parent's suspend to work, we need to transition it to
> RPM_SUSPENDED, but again we don't want the driver's PM callbacks to
> run at this point. Moreover, we don't want the PCI bus type's
> callbacks to run at this point, because dev->driver is still set.
> So again, doing something like pm_runtime_put_skip_callbacks(),
> treating the device as though it had power.no_callbacks set, seems
> to be appropriate.
>
> Namely, if the user space's attribute is "on", it will just drop
> usage_count by 1, which is what we want in that case. If the user
> space's attribute is "auto", on the other hand, it will drop
> usage_count by 1 and change the status to RPM_SUSPENDED without
> running callbacks, which again is what we want.
>
> (6) In drv->remove() the driver is supposed to bump up usage_count by 1,
> so as to restore the situation from before its .probe() routine
> was called. It also should leave the device as RPM_ACTIVE, because
> that's what it's got in .probe(). Then, after drv->remove exits,
> (and also if drv was NULL to start with), we want to drop usage_count
> by 1. Moreover, if the user space's attribute is "on", we don't
> want anything more to happen, _but_ if that's "auto", we want to
> suspend the parent.
>
> Note that dev->driver is still not NULL at this point (although
> pci_dev->driver is!) so again we can't run the PCI bus type's callbacks.
> It looks like, then, what we want to do here is
> pm_runtime_put_skip_callbacks() again, because if the user space's
> attribute is "on", it will just drop usage_count by 1, which is what
> we want, and if that's "auto", it will additionally change the status
> to RPM_SUSPENDED (without executing callbacks, which we want) _and_
> it will queue up the parent's suspend (which, again, is what we want).
>
> Did I miss anything?
Apparently, I did. In (6), if drv is NULL to start with, we don't want
to do anything with runtime PM, except for checking if the parent can be
suspended, so we only need to do pm_request_idle(parent) in that case.
And we seem to have a bug in there right now, because we shouldn't
do the "Undo the runtime PM settings in local_pci_probe()" stuff in that
case I think.
That seems to be the only thing I missed, though, hopefully.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH V5 1/2] Thermal: Add ST-Ericsson DB8500 thermal driver.
From: Viresh Kumar @ 2012-11-15 10:17 UTC (permalink / raw)
To: Hongbo Zhang
Cc: Zhang Rui, linaro-kernel, linaro-dev, patches, linux-pm,
linux-kernel, linux-acpi, amit.kachhap, STEricsson_nomadik_linux,
kernel, hongbo.zhang
In-Reply-To: <CAJLyvQxJeNgGc8sQiV_qmKCfvfWQKyR8S5ZMNCtETvJLMQH-oQ@mail.gmail.com>
On 15 November 2012 14:53, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
> this is a driver for ST-Ericsson u8500 board(Snowball), with a ARM core inside.
> so you should compile like this:
> make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- u8500_defconfig
> make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- uImage
Then we must have a DEPENDS_ON in Kconfig entry for these.
--
viresh
^ 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