From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from bh-25.webhostbox.net (bh-25.webhostbox.net [208.91.199.152]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 569571A0ACA for ; Wed, 8 Oct 2014 06:03:47 +1100 (EST) Received: from mailnull by bh-25.webhostbox.net with sa-checked (Exim 4.82) (envelope-from ) id 1Xba2z-002hIg-Fd for linuxppc-dev@lists.ozlabs.org; Tue, 07 Oct 2014 19:03:45 +0000 Date: Tue, 7 Oct 2014 12:03:25 -0700 From: Guenter Roeck To: Alexander Graf Subject: Re: [PATCH 00/20] powerpc: Convert power off logic to pm_power_off Message-ID: <20141007190325.GA30399@roeck-us.net> References: <1412170086-57971-1-git-send-email-agraf@suse.de> <1412311350.2783.4.camel@concordia> <54326850.1030801@suse.de> <1412663147.10747.1.camel@concordia> <5433CFEB.9040104@suse.de> <20141007170052.GB16931@roeck-us.net> <54342737.5010802@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <54342737.5010802@suse.de> Cc: Arnd Bergmann , Geoff Levand , Alistair Popple , 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 Tue, Oct 07, 2014 at 07:47:35PM +0200, Alexander Graf wrote: > > > On 07.10.14 19:00, Guenter Roeck wrote: > > On Tue, Oct 07, 2014 at 01:35:07PM +0200, Alexander Graf wrote: > >> > >> > >> On 07.10.14 08:25, Michael Ellerman wrote: > >>> On Mon, 2014-10-06 at 12:00 +0200, Alexander Graf wrote: > >>>> > >>>> On 03.10.14 06:42, Michael Ellerman wrote: > >>>>> On Wed, 2014-10-01 at 15:27 +0200, 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. > >>>>>> > >>>>>> 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. > >>>>> > >>>>> This looks OK to me with one caveat. > >>>>> > >>>>> In several of the patches you're replacing a static initialisation with a > >>>>> runtime one, and you're doing the runtime initialisation in xxx_setup_arch(). > >>>>> That's reasonably late, so I'd prefer you did it in xxx_probe(). > >>>> > >>>> Heh, I had it in xxx_probe() originally and then realized that > >>>> > >>>> a) the power off function is basically a driver. Driver initialization > >>>> happens in xxx_setup_arch() and > >>>> > >>>> b) the maple target already does overwrite its power_off callback in > >>>> xxx_setup_arch and > >>>> > >>>> c) on all targets xxx_probe() is very slim and doesn't do much > >>>> > >>>> but I'll happily change it back to put the bits in xxx_probe() instead. > >>> > >>> Thanks. > >>> > >>> That way you shouldn't be changing behaviour. > >>> > >>> It may still be the case that some power off routines don't actually work until > >>> later, but that's an existing problem. Some power off routines *do* work before > >>> setup_arch(), so they will continue to work. > >> > >> Ok, works for me :). Just wanted to make sure you're aware of the > >> reasoning why I didn't do it in probe(). > >> > >>> Also, how does your series interact with Guenter's that removes pm_power_off ? > >>> It seems at the moment they are unaware of each other. > >> > >> Guenters patches convert users of pm_power_off to his new scheme. We're > >> not even at that stage at all yet in the powerpc tree. Converting > >> everything to pm_power_off is basically a first step. His patch set > >> maintains pm_power_off, so there shouldn't be nasty conflicts. > >> > > Onlly the first m68k patch, though. The very last patch in the series > > remvoes pm_power_off. > > And there go my patch reading skills ... :). > > For which window are you targeting this? 3.18 or 3.19? If you're trying > to hit 3.18, I can easily wait with my patch set and base it on top of > yours. > Definitely 3.19; this is way too late for 3.18 and will need some time to mature in -next. I can merge with your code once it is ready to go and you can make an immutable branch available. Thanks, Guenter