From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pd0-x230.google.com (mail-pd0-x230.google.com [IPv6:2607:f8b0:400e:c02::230]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 035F61A0509 for ; Thu, 2 Oct 2014 01:54:42 +1000 (EST) Received: by mail-pd0-f176.google.com with SMTP id fp1so453221pdb.7 for ; Wed, 01 Oct 2014 08:54:39 -0700 (PDT) Sender: Guenter Roeck Date: Wed, 1 Oct 2014 08:54:34 -0700 From: Guenter Roeck To: Alexander Graf Subject: Re: [PATCH 00/20] powerpc: Convert power off logic to pm_power_off Message-ID: <20141001155434.GA11039@roeck-us.net> References: <1412170086-57971-1-git-send-email-agraf@suse.de> <542C13FB.80608@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <542C13FB.80608@suse.de> Cc: Arnd Bergmann , Geoff Levand , Alistair Popple , Geert Uytterhoeven , Scott Wood , Anatolij Gustschin , "linuxppc-dev@lists.ozlabs.org" List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 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