Cpufreq shutdown patch (kernel 2.6.35.3)

Cpufreq shutdown patch (kernel 2.6.35.3)

[N. Coesel]

Dear readers,
I've found a problem in drivers/cpufreq/cpufreq.c. The driver does 
not execute the exit member of the low level driver when shutting 
down for a reboot (reset). This potentially leaves the power supply 
at a too low voltage to boot the system properly. The patch below 
adds a shutdown function which executes the exit member of the low 
level driver which allows a system to be properly prepared for a reset.

--- drivers/cpufreq/cpufreq.c.orig      2010-08-20 20:55:55.000000000 +0200
+++ drivers/cpufreq/cpufreq.c   2012-01-19 21:50:46.000000000 +0100
@@ -1431,11 +1431,34 @@ fail:
         return ret;
  }

+
+static int cpufreq_shutdown(struct sys_device *sysdev)
+{
+       struct cpufreq_policy *cpu_policy;
+       int cpu = sysdev->id;
+       int ret =0;
+
+       if (!cpu_online(cpu))
+               return 0;
+
+       cpu_policy = cpufreq_cpu_get(cpu);
+       if (!cpu_policy)
+               return -EINVAL;
+
+//     printk("cpufreq_shutdown %d \n", cpu);
+       if (cpufreq_driver->exit)
+               ret = cpufreq_driver->exit(cpu_policy);
+
+       return ret;
+}
+
+
  static struct sysdev_driver cpufreq_sysdev_driver = {
         .add            = cpufreq_add_dev,
         .remove         = cpufreq_remove_dev,
         .suspend        = cpufreq_suspend,
         .resume         = cpufreq_resume,
+       .shutdown = cpufreq_shutdown,
  };



Signed of by: Nico Coesel nico@nctdev.nl

Re: Cpufreq shutdown patch (kernel 2.6.35.3)

[Dave Jones]

On Thu, Jan 19, 2012 at 10:18:10PM +0100, N. Coesel wrote:
 
 > I've found a problem in drivers/cpufreq/cpufreq.c. The driver does 
 > not execute the exit member of the low level driver when shutting 
 > down for a reboot (reset). This potentially leaves the power supply 
 > at a too low voltage to boot the system properly. The patch below 
 > adds a shutdown function which executes the exit member of the low 
 > level driver which allows a system to be properly prepared for a reset.

I'm curious what hardware you saw this problem on ?

I've just seen a report on an x86 system which looks like it might
be solved by this.
https://bugzilla.redhat.com/show_bug.cgi?id=789964

 > --- drivers/cpufreq/cpufreq.c.orig      2010-08-20 20:55:55.000000000 +0200
 > +++ drivers/cpufreq/cpufreq.c   2012-01-19 21:50:46.000000000 +0100
 > @@ -1431,11 +1431,34 @@ fail:
 >          return ret;
 >   }
 > 
 > +
 > +static int cpufreq_shutdown(struct sys_device *sysdev)
 > +{
 > +       struct cpufreq_policy *cpu_policy;
 > +       int cpu = sysdev->id;
 > +       int ret =0;
 > +
 > +       if (!cpu_online(cpu))
 > +               return 0;
 > +
 > +       cpu_policy = cpufreq_cpu_get(cpu);
 > +       if (!cpu_policy)
 > +               return -EINVAL;
 > +
 > +//     printk("cpufreq_shutdown %d \n", cpu);
 > +       if (cpufreq_driver->exit)
 > +               ret = cpufreq_driver->exit(cpu_policy);
 > +
 > +       return ret;
 > +}
 > +
 > +
 >   static struct sysdev_driver cpufreq_sysdev_driver = {
 >          .add            = cpufreq_add_dev,
 >          .remove         = cpufreq_remove_dev,
 >          .suspend        = cpufreq_suspend,
 >          .resume         = cpufreq_resume,
 > +       .shutdown = cpufreq_shutdown,
 >   };

I'll queue this up, with the whitespace fixed, and commented out line removed.

	Dave

Re: Cpufreq shutdown patch (kernel 2.6.35.3)

[Dave Jones]

On Mon, Feb 13, 2012 at 11:28:53AM -0500, Dave Jones wrote:
 > On Thu, Jan 19, 2012 at 10:18:10PM +0100, N. Coesel wrote:
 >  
 >  > I've found a problem in drivers/cpufreq/cpufreq.c. The driver does 
 >  > not execute the exit member of the low level driver when shutting 
 >  > down for a reboot (reset). This potentially leaves the power supply 
 >  > at a too low voltage to boot the system properly. The patch below 
 >  > adds a shutdown function which executes the exit member of the low 
 >  > level driver which allows a system to be properly prepared for a reset.
 > 
 > I'm curious what hardware you saw this problem on ?
 > 
 > I've just seen a report on an x86 system which looks like it might
 > be solved by this.
 > https://bugzilla.redhat.com/show_bug.cgi?id=789964
 > 
 >  > --- drivers/cpufreq/cpufreq.c.orig      2010-08-20 20:55:55.000000000 +0200
 >  > +++ drivers/cpufreq/cpufreq.c   2012-01-19 21:50:46.000000000 +0100
 >  > @@ -1431,11 +1431,34 @@ fail:
 >  >          return ret;
 >  >   }
 >  > 
 >  > +
 >  > +static int cpufreq_shutdown(struct sys_device *sysdev)
 >  > +{
 >  > +       struct cpufreq_policy *cpu_policy;
 >  > +       int cpu = sysdev->id;
 >  > +       int ret =0;
 >  > +
 >  > +       if (!cpu_online(cpu))
 >  > +               return 0;
 >  > +
 >  > +       cpu_policy = cpufreq_cpu_get(cpu);
 >  > +       if (!cpu_policy)
 >  > +               return -EINVAL;
 >  > +
 >  > +//     printk("cpufreq_shutdown %d \n", cpu);
 >  > +       if (cpufreq_driver->exit)
 >  > +               ret = cpufreq_driver->exit(cpu_policy);
 >  > +
 >  > +       return ret;
 >  > +}
 >  > +
 >  > +
 >  >   static struct sysdev_driver cpufreq_sysdev_driver = {
 >  >          .add            = cpufreq_add_dev,
 >  >          .remove         = cpufreq_remove_dev,
 >  >          .suspend        = cpufreq_suspend,
 >  >          .resume         = cpufreq_resume,
 >  > +       .shutdown = cpufreq_shutdown,
 >  >   };
 > 
 > I'll queue this up, with the whitespace fixed, and commented out line removed.

Except I can't of course, because we no longer have a sysdev driver
since 8a25a2fd126c621f44f3aeaef80d51f00fc11639
Kay, given the subsys_interface doesn't have a shutdown method, what should
this be doing ?

	Dave 

Re: Cpufreq shutdown patch (kernel 2.6.35.3)

[Kay Sievers]

On Mon, Feb 13, 2012 at 17:35, Dave Jones <davej@redhat.com> wrote:
> On Mon, Feb 13, 2012 at 11:28:53AM -0500, Dave Jones wrote:

> Except I can't of course, because we no longer have a sysdev driver
> since 8a25a2fd126c621f44f3aeaef80d51f00fc11639
> Kay, given the subsys_interface doesn't have a shutdown method, what should
> this be doing ?

The former sysdev device's power management methods have been split
off to a separate 'struct syscore_ops' and all users converted to it,
to be able to remove the sysdev stuff.

Kay

Re: Cpufreq shutdown patch (kernel 2.6.35.3)

[N. Coesel]

Dave,

At 17:28 13-2-2012, Dave Jones wrote:
>On Thu, Jan 19, 2012 at 10:18:10PM +0100, N. Coesel wrote:
>
>  > I've found a problem in drivers/cpufreq/cpufreq.c. The driver does
>  > not execute the exit member of the low level driver when shutting
>  > down for a reboot (reset). This potentially leaves the power supply
>  > at a too low voltage to boot the system properly. The patch below
>  > adds a shutdown function which executes the exit member of the low
>  > level driver which allows a system to be properly prepared for a reset.
>
>I'm curious what hardware you saw this problem on ?

This was on an IMX51 embedded platform but I'm quite sure the same 
problem can occur on other platforms as well. There has to be some 
way to allow an orderly shutdown of the low level cpufreq system 
without a cold reset.

Nico Coesel



o---------------------------------------------------------------o
|                       N C T  Developments                     |
|Innovative embedded solutions                                  |
o---------------------------------------------------------------o