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. Thanks, Guenter _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev