Re: [PATCH 00/20] powerpc: Convert power off logic to pm_power_off

[Guenter Roeck <linux@roeck-us.net>]

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