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

From: Guenter Roeck <[email-masked]>
Date: Tue, 7 Oct 2014 10:00:52 -0700

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.

Guenter

> Once we converted from ppc_md tables to actual code, we can just run a
> simple coccinelle patch to convert from pm_power_off to his new scheme
> later.
> 
> 
> Alex