Re: [PATCH] [RFC] cpufreq: can't raise max frequency with cpu_thermal

2012-12-29 Thread Sonny Rao
On Wed, Dec 26, 2012 at 11:32 AM, amit daniel kachhap
 wrote:
>
> On Tue, Dec 18, 2012 at 9:45 PM, Doug Anderson  wrote:
> > Amit,
> >
> > On Tue, Dec 18, 2012 at 8:17 PM, amit daniel kachhap
> >  wrote:
> >> On Tue, Dec 18, 2012 at 12:29 AM, Sonny Rao  wrote:
> >>> The cpu_thermal generic thermal management code has a bug where once
> >>> max cpu frequency has been lowered in sysfs (scaling_max_freq) it is
> >>> not possible to raise the max back up later.  The bug is that the
> >>> notifer gets called by __cpufreq_set_policy() before the user policy
> >>> max is raised, and is incorrectly trying to enforce the max frequency
> >>> policy even when we are trying to change the policy.  It is also not
> >>> clear why this driver is looking at the user policy since it is
> >>> primarily supposed to enforce thermal policy, not user set policy.
> >>
> >> Hi Sunny,
> >>
> >> I am not sure if this change is needed.
> >
> > Do you have a machine that's running with your code?  Can you go into
> > sysfs (/sys/devices/system/cpu/cpu0/cpufreq/) and try lowering then
> > raising the max frequency by doing something like this (assumes that
> > you can scale down to 200MHz):
> >
> >   cd /sys/devices/system/cpu/cpu0/cpufreq/
> >   OLD_VAL=$(cat scaling_max_freq)
> >   cat scaling_min_freq > scaling_max_freq
> >   echo ${OLD_VAL} > scaling_max_freq
> >
> >   echo "$(cat scaling_max_freq) should be ${OLD_VAL}.  Is it?"
> >
> > ...when I run the above without Sonny's patch on my system I see:
> >   20 should be 170. Is it?
> >
> > ...after Sonny's patch then the above works.
> Hi Doug,
>
> I tested the above steps on exynos origen board with all cpufreq
> cooling configs enabled in kernel version 3.8-rc1.
> In my tests I am able to vary scaling_max_freq to all values. Also I
> am in normal temperature threshold. So basically I am not able to
> reproduce the error reported,
>
> Thanks,
> Amit Daniel


Hi, thanks for checking it out.  I'm a bit surprised that you couldn't
reproduce it, but it might just be that it will only manifest on a
platform which is using that driver - like Exynos 5?  We'll try it out
on a 3.8-rc on an Exynos and let you know if we see it or not.

Thanks again,
Sonny


>
> >
> >> There is a check in cpufreq_thermal_notifier function to return 0 if
> >> notify_device == NOTIFY_INVALID. So the user will be always able to
> >> change the max frequency in normal situation. Did you tested this for
> >> some corner cases?
> >> The reason behind putting this check is that I don't want to override
> >> the user constraints.
> >>
> >> Thanks,
> >> Amit Daniel
> >>
> >>>
> >>> Signed-off-by: Sonny Rao 
> >>> ---
> >>>  drivers/thermal/cpu_cooling.c |4 
> >>>  1 files changed, 0 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> >>> index 836828e..63bc708 100644
> >>> --- a/drivers/thermal/cpu_cooling.c
> >>> +++ b/drivers/thermal/cpu_cooling.c
> >>> @@ -219,10 +219,6 @@ static int cpufreq_thermal_notifier(struct 
> >>> notifier_block *nb,
> >>> if (cpumask_test_cpu(policy->cpu, _device->allowed_cpus))
> >>> max_freq = notify_device->cpufreq_val;
> >>>
> >>> -   /* Never exceed user_policy.max*/
> >>> -   if (max_freq > policy->user_policy.max)
> >>> -   max_freq = policy->user_policy.max;
> >>> -
> >>> if (policy->max != max_freq)
> >>> cpufreq_verify_within_limits(policy, 0, max_freq);
> >>>
> >>> --
> >>> 1.7.7.3
> >>>
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> >>> the body of a message to majord...@vger.kernel.org
> >>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>> Please read the FAQ at  http://www.tux.org/lkml/
> >
> > -Doug
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majord...@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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [RFC] cpufreq: can't raise max frequency with cpu_thermal

2012-12-29 Thread Sonny Rao
On Wed, Dec 26, 2012 at 11:32 AM, amit daniel kachhap
amit.dan...@samsung.com wrote:

 On Tue, Dec 18, 2012 at 9:45 PM, Doug Anderson diand...@chromium.org wrote:
  Amit,
 
  On Tue, Dec 18, 2012 at 8:17 PM, amit daniel kachhap
  amit.dan...@samsung.com wrote:
  On Tue, Dec 18, 2012 at 12:29 AM, Sonny Rao sonny...@chromium.org wrote:
  The cpu_thermal generic thermal management code has a bug where once
  max cpu frequency has been lowered in sysfs (scaling_max_freq) it is
  not possible to raise the max back up later.  The bug is that the
  notifer gets called by __cpufreq_set_policy() before the user policy
  max is raised, and is incorrectly trying to enforce the max frequency
  policy even when we are trying to change the policy.  It is also not
  clear why this driver is looking at the user policy since it is
  primarily supposed to enforce thermal policy, not user set policy.
 
  Hi Sunny,
 
  I am not sure if this change is needed.
 
  Do you have a machine that's running with your code?  Can you go into
  sysfs (/sys/devices/system/cpu/cpu0/cpufreq/) and try lowering then
  raising the max frequency by doing something like this (assumes that
  you can scale down to 200MHz):
 
cd /sys/devices/system/cpu/cpu0/cpufreq/
OLD_VAL=$(cat scaling_max_freq)
cat scaling_min_freq  scaling_max_freq
echo ${OLD_VAL}  scaling_max_freq
 
echo $(cat scaling_max_freq) should be ${OLD_VAL}.  Is it?
 
  ...when I run the above without Sonny's patch on my system I see:
20 should be 170. Is it?
 
  ...after Sonny's patch then the above works.
 Hi Doug,

 I tested the above steps on exynos origen board with all cpufreq
 cooling configs enabled in kernel version 3.8-rc1.
 In my tests I am able to vary scaling_max_freq to all values. Also I
 am in normal temperature threshold. So basically I am not able to
 reproduce the error reported,

 Thanks,
 Amit Daniel


Hi, thanks for checking it out.  I'm a bit surprised that you couldn't
reproduce it, but it might just be that it will only manifest on a
platform which is using that driver - like Exynos 5?  We'll try it out
on a 3.8-rc on an Exynos and let you know if we see it or not.

Thanks again,
Sonny



 
  There is a check in cpufreq_thermal_notifier function to return 0 if
  notify_device == NOTIFY_INVALID. So the user will be always able to
  change the max frequency in normal situation. Did you tested this for
  some corner cases?
  The reason behind putting this check is that I don't want to override
  the user constraints.
 
  Thanks,
  Amit Daniel
 
 
  Signed-off-by: Sonny Rao sonny...@chromium.org
  ---
   drivers/thermal/cpu_cooling.c |4 
   1 files changed, 0 insertions(+), 4 deletions(-)
 
  diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
  index 836828e..63bc708 100644
  --- a/drivers/thermal/cpu_cooling.c
  +++ b/drivers/thermal/cpu_cooling.c
  @@ -219,10 +219,6 @@ static int cpufreq_thermal_notifier(struct 
  notifier_block *nb,
  if (cpumask_test_cpu(policy-cpu, notify_device-allowed_cpus))
  max_freq = notify_device-cpufreq_val;
 
  -   /* Never exceed user_policy.max*/
  -   if (max_freq  policy-user_policy.max)
  -   max_freq = policy-user_policy.max;
  -
  if (policy-max != max_freq)
  cpufreq_verify_within_limits(policy, 0, max_freq);
 
  --
  1.7.7.3
 
  --
  To unsubscribe from this list: send the line unsubscribe linux-kernel in
  the body of a message to majord...@vger.kernel.org
  More majordomo info at  http://vger.kernel.org/majordomo-info.html
  Please read the FAQ at  http://www.tux.org/lkml/
 
  -Doug
  --
  To unsubscribe from this list: send the line unsubscribe linux-kernel in
  the body of a message to majord...@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-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [RFC] cpufreq: can't raise max frequency with cpu_thermal

2012-12-26 Thread amit daniel kachhap
On Tue, Dec 18, 2012 at 9:45 PM, Doug Anderson  wrote:
> Amit,
>
> On Tue, Dec 18, 2012 at 8:17 PM, amit daniel kachhap
>  wrote:
>> On Tue, Dec 18, 2012 at 12:29 AM, Sonny Rao  wrote:
>>> The cpu_thermal generic thermal management code has a bug where once
>>> max cpu frequency has been lowered in sysfs (scaling_max_freq) it is
>>> not possible to raise the max back up later.  The bug is that the
>>> notifer gets called by __cpufreq_set_policy() before the user policy
>>> max is raised, and is incorrectly trying to enforce the max frequency
>>> policy even when we are trying to change the policy.  It is also not
>>> clear why this driver is looking at the user policy since it is
>>> primarily supposed to enforce thermal policy, not user set policy.
>>
>> Hi Sunny,
>>
>> I am not sure if this change is needed.
>
> Do you have a machine that's running with your code?  Can you go into
> sysfs (/sys/devices/system/cpu/cpu0/cpufreq/) and try lowering then
> raising the max frequency by doing something like this (assumes that
> you can scale down to 200MHz):
>
>   cd /sys/devices/system/cpu/cpu0/cpufreq/
>   OLD_VAL=$(cat scaling_max_freq)
>   cat scaling_min_freq > scaling_max_freq
>   echo ${OLD_VAL} > scaling_max_freq
>
>   echo "$(cat scaling_max_freq) should be ${OLD_VAL}.  Is it?"
>
> ...when I run the above without Sonny's patch on my system I see:
>   20 should be 170. Is it?
>
> ...after Sonny's patch then the above works.
Hi Doug,

I tested the above steps on exynos origen board with all cpufreq
cooling configs enabled in kernel version 3.8-rc1.
In my tests I am able to vary scaling_max_freq to all values. Also I
am in normal temperature threshold. So basically I am not able to
reproduce the error reported,

Thanks,
Amit Daniel
>
>> There is a check in cpufreq_thermal_notifier function to return 0 if
>> notify_device == NOTIFY_INVALID. So the user will be always able to
>> change the max frequency in normal situation. Did you tested this for
>> some corner cases?
>> The reason behind putting this check is that I don't want to override
>> the user constraints.
>>
>> Thanks,
>> Amit Daniel
>>
>>>
>>> Signed-off-by: Sonny Rao 
>>> ---
>>>  drivers/thermal/cpu_cooling.c |4 
>>>  1 files changed, 0 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
>>> index 836828e..63bc708 100644
>>> --- a/drivers/thermal/cpu_cooling.c
>>> +++ b/drivers/thermal/cpu_cooling.c
>>> @@ -219,10 +219,6 @@ static int cpufreq_thermal_notifier(struct 
>>> notifier_block *nb,
>>> if (cpumask_test_cpu(policy->cpu, _device->allowed_cpus))
>>> max_freq = notify_device->cpufreq_val;
>>>
>>> -   /* Never exceed user_policy.max*/
>>> -   if (max_freq > policy->user_policy.max)
>>> -   max_freq = policy->user_policy.max;
>>> -
>>> if (policy->max != max_freq)
>>> cpufreq_verify_within_limits(policy, 0, max_freq);
>>>
>>> --
>>> 1.7.7.3
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at  http://www.tux.org/lkml/
>
> -Doug
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [RFC] cpufreq: can't raise max frequency with cpu_thermal

2012-12-26 Thread amit daniel kachhap
On Tue, Dec 18, 2012 at 9:45 PM, Doug Anderson diand...@chromium.org wrote:
 Amit,

 On Tue, Dec 18, 2012 at 8:17 PM, amit daniel kachhap
 amit.dan...@samsung.com wrote:
 On Tue, Dec 18, 2012 at 12:29 AM, Sonny Rao sonny...@chromium.org wrote:
 The cpu_thermal generic thermal management code has a bug where once
 max cpu frequency has been lowered in sysfs (scaling_max_freq) it is
 not possible to raise the max back up later.  The bug is that the
 notifer gets called by __cpufreq_set_policy() before the user policy
 max is raised, and is incorrectly trying to enforce the max frequency
 policy even when we are trying to change the policy.  It is also not
 clear why this driver is looking at the user policy since it is
 primarily supposed to enforce thermal policy, not user set policy.

 Hi Sunny,

 I am not sure if this change is needed.

 Do you have a machine that's running with your code?  Can you go into
 sysfs (/sys/devices/system/cpu/cpu0/cpufreq/) and try lowering then
 raising the max frequency by doing something like this (assumes that
 you can scale down to 200MHz):

   cd /sys/devices/system/cpu/cpu0/cpufreq/
   OLD_VAL=$(cat scaling_max_freq)
   cat scaling_min_freq  scaling_max_freq
   echo ${OLD_VAL}  scaling_max_freq

   echo $(cat scaling_max_freq) should be ${OLD_VAL}.  Is it?

 ...when I run the above without Sonny's patch on my system I see:
   20 should be 170. Is it?

 ...after Sonny's patch then the above works.
Hi Doug,

I tested the above steps on exynos origen board with all cpufreq
cooling configs enabled in kernel version 3.8-rc1.
In my tests I am able to vary scaling_max_freq to all values. Also I
am in normal temperature threshold. So basically I am not able to
reproduce the error reported,

Thanks,
Amit Daniel

 There is a check in cpufreq_thermal_notifier function to return 0 if
 notify_device == NOTIFY_INVALID. So the user will be always able to
 change the max frequency in normal situation. Did you tested this for
 some corner cases?
 The reason behind putting this check is that I don't want to override
 the user constraints.

 Thanks,
 Amit Daniel


 Signed-off-by: Sonny Rao sonny...@chromium.org
 ---
  drivers/thermal/cpu_cooling.c |4 
  1 files changed, 0 insertions(+), 4 deletions(-)

 diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
 index 836828e..63bc708 100644
 --- a/drivers/thermal/cpu_cooling.c
 +++ b/drivers/thermal/cpu_cooling.c
 @@ -219,10 +219,6 @@ static int cpufreq_thermal_notifier(struct 
 notifier_block *nb,
 if (cpumask_test_cpu(policy-cpu, notify_device-allowed_cpus))
 max_freq = notify_device-cpufreq_val;

 -   /* Never exceed user_policy.max*/
 -   if (max_freq  policy-user_policy.max)
 -   max_freq = policy-user_policy.max;
 -
 if (policy-max != max_freq)
 cpufreq_verify_within_limits(policy, 0, max_freq);

 --
 1.7.7.3

 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/

 -Doug
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@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-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [RFC] cpufreq: can't raise max frequency with cpu_thermal

2012-12-18 Thread Doug Anderson
Amit,

On Tue, Dec 18, 2012 at 8:17 PM, amit daniel kachhap
 wrote:
> On Tue, Dec 18, 2012 at 12:29 AM, Sonny Rao  wrote:
>> The cpu_thermal generic thermal management code has a bug where once
>> max cpu frequency has been lowered in sysfs (scaling_max_freq) it is
>> not possible to raise the max back up later.  The bug is that the
>> notifer gets called by __cpufreq_set_policy() before the user policy
>> max is raised, and is incorrectly trying to enforce the max frequency
>> policy even when we are trying to change the policy.  It is also not
>> clear why this driver is looking at the user policy since it is
>> primarily supposed to enforce thermal policy, not user set policy.
>
> Hi Sunny,
>
> I am not sure if this change is needed.

Do you have a machine that's running with your code?  Can you go into
sysfs (/sys/devices/system/cpu/cpu0/cpufreq/) and try lowering then
raising the max frequency by doing something like this (assumes that
you can scale down to 200MHz):

  cd /sys/devices/system/cpu/cpu0/cpufreq/
  OLD_VAL=$(cat scaling_max_freq)
  cat scaling_min_freq > scaling_max_freq
  echo ${OLD_VAL} > scaling_max_freq

  echo "$(cat scaling_max_freq) should be ${OLD_VAL}.  Is it?"

...when I run the above without Sonny's patch on my system I see:
  20 should be 170. Is it?

...after Sonny's patch then the above works.

> There is a check in cpufreq_thermal_notifier function to return 0 if
> notify_device == NOTIFY_INVALID. So the user will be always able to
> change the max frequency in normal situation. Did you tested this for
> some corner cases?
> The reason behind putting this check is that I don't want to override
> the user constraints.
>
> Thanks,
> Amit Daniel
>
>>
>> Signed-off-by: Sonny Rao 
>> ---
>>  drivers/thermal/cpu_cooling.c |4 
>>  1 files changed, 0 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
>> index 836828e..63bc708 100644
>> --- a/drivers/thermal/cpu_cooling.c
>> +++ b/drivers/thermal/cpu_cooling.c
>> @@ -219,10 +219,6 @@ static int cpufreq_thermal_notifier(struct 
>> notifier_block *nb,
>> if (cpumask_test_cpu(policy->cpu, _device->allowed_cpus))
>> max_freq = notify_device->cpufreq_val;
>>
>> -   /* Never exceed user_policy.max*/
>> -   if (max_freq > policy->user_policy.max)
>> -   max_freq = policy->user_policy.max;
>> -
>> if (policy->max != max_freq)
>> cpufreq_verify_within_limits(policy, 0, max_freq);
>>
>> --
>> 1.7.7.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [RFC] cpufreq: can't raise max frequency with cpu_thermal

2012-12-18 Thread amit daniel kachhap
On Tue, Dec 18, 2012 at 12:29 AM, Sonny Rao  wrote:
> The cpu_thermal generic thermal management code has a bug where once
> max cpu frequency has been lowered in sysfs (scaling_max_freq) it is
> not possible to raise the max back up later.  The bug is that the
> notifer gets called by __cpufreq_set_policy() before the user policy
> max is raised, and is incorrectly trying to enforce the max frequency
> policy even when we are trying to change the policy.  It is also not
> clear why this driver is looking at the user policy since it is
> primarily supposed to enforce thermal policy, not user set policy.

Hi Sunny,

I am not sure if this change is needed.
There is a check in cpufreq_thermal_notifier function to return 0 if
notify_device == NOTIFY_INVALID. So the user will be always able to
change the max frequency in normal situation. Did you tested this for
some corner cases?
The reason behind putting this check is that I don't want to override
the user constraints.

Thanks,
Amit Daniel

>
> Signed-off-by: Sonny Rao 
> ---
>  drivers/thermal/cpu_cooling.c |4 
>  1 files changed, 0 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index 836828e..63bc708 100644
> --- a/drivers/thermal/cpu_cooling.c
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -219,10 +219,6 @@ static int cpufreq_thermal_notifier(struct 
> notifier_block *nb,
> if (cpumask_test_cpu(policy->cpu, _device->allowed_cpus))
> max_freq = notify_device->cpufreq_val;
>
> -   /* Never exceed user_policy.max*/
> -   if (max_freq > policy->user_policy.max)
> -   max_freq = policy->user_policy.max;
> -
> if (policy->max != max_freq)
> cpufreq_verify_within_limits(policy, 0, max_freq);
>
> --
> 1.7.7.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [RFC] cpufreq: can't raise max frequency with cpu_thermal

2012-12-18 Thread Doug Anderson
On Tue, Dec 18, 2012 at 12:29 AM, Sonny Rao  wrote:
> The cpu_thermal generic thermal management code has a bug where once
> max cpu frequency has been lowered in sysfs (scaling_max_freq) it is
> not possible to raise the max back up later.  The bug is that the
> notifer gets called by __cpufreq_set_policy() before the user policy
> max is raised, and is incorrectly trying to enforce the max frequency
> policy even when we are trying to change the policy.  It is also not
> clear why this driver is looking at the user policy since it is
> primarily supposed to enforce thermal policy, not user set policy.
>
> Signed-off-by: Sonny Rao 
> ---
>  drivers/thermal/cpu_cooling.c |4 
>  1 files changed, 0 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index 836828e..63bc708 100644
> --- a/drivers/thermal/cpu_cooling.c
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -219,10 +219,6 @@ static int cpufreq_thermal_notifier(struct 
> notifier_block *nb,
> if (cpumask_test_cpu(policy->cpu, _device->allowed_cpus))
> max_freq = notify_device->cpufreq_val;
>
> -   /* Never exceed user_policy.max*/
> -   if (max_freq > policy->user_policy.max)
> -   max_freq = policy->user_policy.max;
> -
> if (policy->max != max_freq)
> cpufreq_verify_within_limits(policy, 0, max_freq);
>
> --
> 1.7.7.3
>

Sonny's change matches what the "ACPI version" of this code
(drivers/acpi/processor_thermal.c) does as well.  I would certainly be
interested to know why the code was added here in the first place.
Amit: do you know?

Reviewed-by: Doug Anderson 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] [RFC] cpufreq: can't raise max frequency with cpu_thermal

2012-12-18 Thread Sonny Rao
The cpu_thermal generic thermal management code has a bug where once
max cpu frequency has been lowered in sysfs (scaling_max_freq) it is
not possible to raise the max back up later.  The bug is that the
notifer gets called by __cpufreq_set_policy() before the user policy
max is raised, and is incorrectly trying to enforce the max frequency
policy even when we are trying to change the policy.  It is also not
clear why this driver is looking at the user policy since it is
primarily supposed to enforce thermal policy, not user set policy.

Signed-off-by: Sonny Rao 
---
 drivers/thermal/cpu_cooling.c |4 
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 836828e..63bc708 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -219,10 +219,6 @@ static int cpufreq_thermal_notifier(struct notifier_block 
*nb,
if (cpumask_test_cpu(policy->cpu, _device->allowed_cpus))
max_freq = notify_device->cpufreq_val;
 
-   /* Never exceed user_policy.max*/
-   if (max_freq > policy->user_policy.max)
-   max_freq = policy->user_policy.max;
-
if (policy->max != max_freq)
cpufreq_verify_within_limits(policy, 0, max_freq);
 
-- 
1.7.7.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] [RFC] cpufreq: can't raise max frequency with cpu_thermal

2012-12-18 Thread Sonny Rao
The cpu_thermal generic thermal management code has a bug where once
max cpu frequency has been lowered in sysfs (scaling_max_freq) it is
not possible to raise the max back up later.  The bug is that the
notifer gets called by __cpufreq_set_policy() before the user policy
max is raised, and is incorrectly trying to enforce the max frequency
policy even when we are trying to change the policy.  It is also not
clear why this driver is looking at the user policy since it is
primarily supposed to enforce thermal policy, not user set policy.

Signed-off-by: Sonny Rao sonny...@chromium.org
---
 drivers/thermal/cpu_cooling.c |4 
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 836828e..63bc708 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -219,10 +219,6 @@ static int cpufreq_thermal_notifier(struct notifier_block 
*nb,
if (cpumask_test_cpu(policy-cpu, notify_device-allowed_cpus))
max_freq = notify_device-cpufreq_val;
 
-   /* Never exceed user_policy.max*/
-   if (max_freq  policy-user_policy.max)
-   max_freq = policy-user_policy.max;
-
if (policy-max != max_freq)
cpufreq_verify_within_limits(policy, 0, max_freq);
 
-- 
1.7.7.3

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [RFC] cpufreq: can't raise max frequency with cpu_thermal

2012-12-18 Thread Doug Anderson
On Tue, Dec 18, 2012 at 12:29 AM, Sonny Rao sonny...@chromium.org wrote:
 The cpu_thermal generic thermal management code has a bug where once
 max cpu frequency has been lowered in sysfs (scaling_max_freq) it is
 not possible to raise the max back up later.  The bug is that the
 notifer gets called by __cpufreq_set_policy() before the user policy
 max is raised, and is incorrectly trying to enforce the max frequency
 policy even when we are trying to change the policy.  It is also not
 clear why this driver is looking at the user policy since it is
 primarily supposed to enforce thermal policy, not user set policy.

 Signed-off-by: Sonny Rao sonny...@chromium.org
 ---
  drivers/thermal/cpu_cooling.c |4 
  1 files changed, 0 insertions(+), 4 deletions(-)

 diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
 index 836828e..63bc708 100644
 --- a/drivers/thermal/cpu_cooling.c
 +++ b/drivers/thermal/cpu_cooling.c
 @@ -219,10 +219,6 @@ static int cpufreq_thermal_notifier(struct 
 notifier_block *nb,
 if (cpumask_test_cpu(policy-cpu, notify_device-allowed_cpus))
 max_freq = notify_device-cpufreq_val;

 -   /* Never exceed user_policy.max*/
 -   if (max_freq  policy-user_policy.max)
 -   max_freq = policy-user_policy.max;
 -
 if (policy-max != max_freq)
 cpufreq_verify_within_limits(policy, 0, max_freq);

 --
 1.7.7.3


Sonny's change matches what the ACPI version of this code
(drivers/acpi/processor_thermal.c) does as well.  I would certainly be
interested to know why the code was added here in the first place.
Amit: do you know?

Reviewed-by: Doug Anderson diand...@chromium.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [RFC] cpufreq: can't raise max frequency with cpu_thermal

2012-12-18 Thread amit daniel kachhap
On Tue, Dec 18, 2012 at 12:29 AM, Sonny Rao sonny...@chromium.org wrote:
 The cpu_thermal generic thermal management code has a bug where once
 max cpu frequency has been lowered in sysfs (scaling_max_freq) it is
 not possible to raise the max back up later.  The bug is that the
 notifer gets called by __cpufreq_set_policy() before the user policy
 max is raised, and is incorrectly trying to enforce the max frequency
 policy even when we are trying to change the policy.  It is also not
 clear why this driver is looking at the user policy since it is
 primarily supposed to enforce thermal policy, not user set policy.

Hi Sunny,

I am not sure if this change is needed.
There is a check in cpufreq_thermal_notifier function to return 0 if
notify_device == NOTIFY_INVALID. So the user will be always able to
change the max frequency in normal situation. Did you tested this for
some corner cases?
The reason behind putting this check is that I don't want to override
the user constraints.

Thanks,
Amit Daniel


 Signed-off-by: Sonny Rao sonny...@chromium.org
 ---
  drivers/thermal/cpu_cooling.c |4 
  1 files changed, 0 insertions(+), 4 deletions(-)

 diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
 index 836828e..63bc708 100644
 --- a/drivers/thermal/cpu_cooling.c
 +++ b/drivers/thermal/cpu_cooling.c
 @@ -219,10 +219,6 @@ static int cpufreq_thermal_notifier(struct 
 notifier_block *nb,
 if (cpumask_test_cpu(policy-cpu, notify_device-allowed_cpus))
 max_freq = notify_device-cpufreq_val;

 -   /* Never exceed user_policy.max*/
 -   if (max_freq  policy-user_policy.max)
 -   max_freq = policy-user_policy.max;
 -
 if (policy-max != max_freq)
 cpufreq_verify_within_limits(policy, 0, max_freq);

 --
 1.7.7.3

 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@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-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [RFC] cpufreq: can't raise max frequency with cpu_thermal

2012-12-18 Thread Doug Anderson
Amit,

On Tue, Dec 18, 2012 at 8:17 PM, amit daniel kachhap
amit.dan...@samsung.com wrote:
 On Tue, Dec 18, 2012 at 12:29 AM, Sonny Rao sonny...@chromium.org wrote:
 The cpu_thermal generic thermal management code has a bug where once
 max cpu frequency has been lowered in sysfs (scaling_max_freq) it is
 not possible to raise the max back up later.  The bug is that the
 notifer gets called by __cpufreq_set_policy() before the user policy
 max is raised, and is incorrectly trying to enforce the max frequency
 policy even when we are trying to change the policy.  It is also not
 clear why this driver is looking at the user policy since it is
 primarily supposed to enforce thermal policy, not user set policy.

 Hi Sunny,

 I am not sure if this change is needed.

Do you have a machine that's running with your code?  Can you go into
sysfs (/sys/devices/system/cpu/cpu0/cpufreq/) and try lowering then
raising the max frequency by doing something like this (assumes that
you can scale down to 200MHz):

  cd /sys/devices/system/cpu/cpu0/cpufreq/
  OLD_VAL=$(cat scaling_max_freq)
  cat scaling_min_freq  scaling_max_freq
  echo ${OLD_VAL}  scaling_max_freq

  echo $(cat scaling_max_freq) should be ${OLD_VAL}.  Is it?

...when I run the above without Sonny's patch on my system I see:
  20 should be 170. Is it?

...after Sonny's patch then the above works.

 There is a check in cpufreq_thermal_notifier function to return 0 if
 notify_device == NOTIFY_INVALID. So the user will be always able to
 change the max frequency in normal situation. Did you tested this for
 some corner cases?
 The reason behind putting this check is that I don't want to override
 the user constraints.

 Thanks,
 Amit Daniel


 Signed-off-by: Sonny Rao sonny...@chromium.org
 ---
  drivers/thermal/cpu_cooling.c |4 
  1 files changed, 0 insertions(+), 4 deletions(-)

 diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
 index 836828e..63bc708 100644
 --- a/drivers/thermal/cpu_cooling.c
 +++ b/drivers/thermal/cpu_cooling.c
 @@ -219,10 +219,6 @@ static int cpufreq_thermal_notifier(struct 
 notifier_block *nb,
 if (cpumask_test_cpu(policy-cpu, notify_device-allowed_cpus))
 max_freq = notify_device-cpufreq_val;

 -   /* Never exceed user_policy.max*/
 -   if (max_freq  policy-user_policy.max)
 -   max_freq = policy-user_policy.max;
 -
 if (policy-max != max_freq)
 cpufreq_verify_within_limits(policy, 0, max_freq);

 --
 1.7.7.3

 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/

-Doug
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/