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
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
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
On Mon, Feb 13, 2012 at 17:35, Dave Jones <[email protected]> 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