On 10/01/2014 02:25 PM, Alexander Graf wrote:
On 01.10.14 17:54, Guenter Roeck wrote:
On Wed, Oct 01, 2014 at 04:47:23PM +0200, Alexander Graf wrote:
On 01.10.14 16:33, Geert Uytterhoeven wrote:
Hi Alex,
On Wed, Oct 1, 2014 at 3:27 PM, Alexander Graf <ag...@suse.de> wrote:
The generic Linux framework to power off the machine is a function pointer
called pm_power_off. The trick about this pointer is that device drivers can
potentially implement it rather than board files.
Today on PowerPC we set pm_power_off to invoke our generic full machine power
off logic which then calls ppc_md.power_off to invoke machine specific power
off.
However, when we want to add a power off GPIO via the "gpio-poweroff" driver,
this card house falls apart. That driver only registers itself if pm_power_off
is NULL to ensure it doesn't override board specific logic. However, since we
always set pm_power_off to the generic power off logic (which will just not
power off the machine if no ppc_md.power_off call is implemented), we can't
implement power off via the generic GPIO power off driver.
To fix this up, let's get rid of the ppc_md.power_off logic and just always use
pm_power_off as was intended. Then individual drivers such as the GPIO power off
driver can implement power off logic via that function pointer.
With this patch set applied and a few patches on top of QEMU that implement a
power off GPIO on the virt e500 machine, I can successfully turn off my virtual
machine after halt.
This is touching the same area as last night's
"[RFC PATCH 00/16] kernel: Add support for poweroff handler call chain"
https://lkml.org/lkml/2014/9/30/575
I agree, and I think your patch set is walking into a reasonable
direction. However, I really think it should convert all users of
pm_power_off - at which point you'll probably get to the same conclusion
that ppc_md.power_off is a bad idea :).
Yes, that would be the ultimate goal.
So in a way, this patch set is semantically a prerequisite to the full
conversion you'd probably like to do :).
Also, in your cover letter you describe that some methods power off the
CPU power while others power off the system power. How do you
distinguish between them with a call chain? You probably won't get
around to trigger the system power off callback after the CPU power off
callback ran ;).
Those are examples. Don't get hung up on it. I may actually replace the
CPU example with something better in the next version; it is not really
a good example and may get people stuck on "why on earth would anyone want
or need a means to turn off the CPU power" instead of focusing on the problem
the patch set tries to solve.
The basic problem is that there can be different poweroff handlers,
some of which may not be available on some systems, and some may not
be as desirable as others for various reasons. The code registering
those poweroff handlers does not specify the poweroff method, but its
priority. It would be up to the programmer (hopefully together with
the board designer) to determine which method should have higher priority.
Taking the above example, the callback to turn off CPU power would presumably
be one of last resort, and have a very low priority.
A better example may actually be patch 15/16 of the series. The affected
driver (drivers/power/reset/restart-poweroff.c) does not really power off
the system, but restarts it instead. Obviously that would only be a poweroff
handler of last resort, which should only be executed if no other means
to power off the system is available.
Sounds like a good plan :). You probably want to have some global list
of priority numbers like "try this first" or "this is a non-optimal, but
working method" and "only ever do this as last resort".
Yes, this is already in the patch set, similar to the restart handler.
Maybe you could as a first step convert every user of pm_power_off to
this new framework with a global notifier_block, similar to how
pm_power_off is a global today? Then we can at least get rid of
pm_power_off altogether and move to only notifiers, whereas new
notifiers can come before or after the old machine set implementations.
As a nice bonus this automatically converts every user of pm_power_off()
to instead call the notifier chain.
Interesting idea, but I am not really sure if it would make much of
a difference. I would still have to implement that handler for each
platform. I might as well convert all users directly; at least this would
ensure that all users are converted.
Guenter
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
