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