| Message ID | 20090923190113.GA19932@oksana.dev.rtsoft.ru (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | f25c525c1412675d2b23d5d88660fb5c9f3a5341 |
| Delegated to: | Kumar Gala |
| Headers | show |
On Sep 23, 2009, at 2:01 PM, Anton Vorontsov wrote: > Currently 83xx PMC driver clears deep_sleeping variable very early, > before devices are resumed. This makes fsl_deep_sleep() unusable in > drivers' resume() callback. > > Sure, drivers can store fsl_deep_sleep() value on suspend and use > the stored value on resume. But a better solution is to postpone > clearing the deep_sleeping variable, i.e. move it into finish() > callback. > > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> > --- > arch/powerpc/platforms/83xx/suspend.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) Scott, any comments or an ack? - k
Kumar Gala wrote: > > On Sep 23, 2009, at 2:01 PM, Anton Vorontsov wrote: > >> Currently 83xx PMC driver clears deep_sleeping variable very early, >> before devices are resumed. This makes fsl_deep_sleep() unusable in >> drivers' resume() callback. >> >> Sure, drivers can store fsl_deep_sleep() value on suspend and use >> the stored value on resume. But a better solution is to postpone >> clearing the deep_sleeping variable, i.e. move it into finish() >> callback. >> >> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> >> --- >> arch/powerpc/platforms/83xx/suspend.c | 4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) > > Scott, any comments or an ack? ACK -Scott
On Nov 5, 2009, at 10:57 AM, Scott Wood wrote: > Kumar Gala wrote: >> On Sep 23, 2009, at 2:01 PM, Anton Vorontsov wrote: >>> Currently 83xx PMC driver clears deep_sleeping variable very early, >>> before devices are resumed. This makes fsl_deep_sleep() unusable in >>> drivers' resume() callback. >>> >>> Sure, drivers can store fsl_deep_sleep() value on suspend and use >>> the stored value on resume. But a better solution is to postpone >>> clearing the deep_sleeping variable, i.e. move it into finish() >>> callback. >>> >>> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> >>> --- >>> arch/powerpc/platforms/83xx/suspend.c | 4 ++-- >>> 1 files changed, 2 insertions(+), 2 deletions(-) >> Scott, any comments or an ack? > > ACK thanks, is that an ACK for all 3? - k
Kumar Gala wrote: > > On Nov 5, 2009, at 10:57 AM, Scott Wood wrote: > >> Kumar Gala wrote: >>> On Sep 23, 2009, at 2:01 PM, Anton Vorontsov wrote: >>>> Currently 83xx PMC driver clears deep_sleeping variable very early, >>>> before devices are resumed. This makes fsl_deep_sleep() unusable in >>>> drivers' resume() callback. >>>> >>>> Sure, drivers can store fsl_deep_sleep() value on suspend and use >>>> the stored value on resume. But a better solution is to postpone >>>> clearing the deep_sleeping variable, i.e. move it into finish() >>>> callback. >>>> >>>> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> >>>> --- >>>> arch/powerpc/platforms/83xx/suspend.c | 4 ++-- >>>> 1 files changed, 2 insertions(+), 2 deletions(-) >>> Scott, any comments or an ack? >> >> ACK > > thanks, is that an ACK for all 3? The first. Patch 2 looks OK as well. As for patch 3, Ben objected to the sleep-nexus stuff on IRC. -Scott
On Nov 5, 2009, at 1:48 PM, Scott Wood wrote: > Kumar Gala wrote: >> On Nov 5, 2009, at 10:57 AM, Scott Wood wrote: >>> Kumar Gala wrote: >>>> On Sep 23, 2009, at 2:01 PM, Anton Vorontsov wrote: >>>>> Currently 83xx PMC driver clears deep_sleeping variable very >>>>> early, >>>>> before devices are resumed. This makes fsl_deep_sleep() unusable >>>>> in >>>>> drivers' resume() callback. >>>>> >>>>> Sure, drivers can store fsl_deep_sleep() value on suspend and use >>>>> the stored value on resume. But a better solution is to postpone >>>>> clearing the deep_sleeping variable, i.e. move it into finish() >>>>> callback. >>>>> >>>>> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> >>>>> --- >>>>> arch/powerpc/platforms/83xx/suspend.c | 4 ++-- >>>>> 1 files changed, 2 insertions(+), 2 deletions(-) >>>> Scott, any comments or an ack? >>> >>> ACK >> thanks, is that an ACK for all 3? > > The first. Patch 2 looks OK as well. > > As for patch 3, Ben objected to the sleep-nexus stuff on IRC. Is sleep-nexus new? I thought we've had that for a bit. - k
Kumar Gala wrote: > On Nov 5, 2009, at 1:48 PM, Scott Wood wrote: >> As for patch 3, Ben objected to the sleep-nexus stuff on IRC. > > Is sleep-nexus new? I thought we've had that for a bit. It's been around in a few dts files, but as was noted, nothing uses this stuff yet. -Scott
On Thu, Nov 05, 2009 at 02:03:14PM -0600, Scott Wood wrote: > Kumar Gala wrote: > >On Nov 5, 2009, at 1:48 PM, Scott Wood wrote: > >>As for patch 3, Ben objected to the sleep-nexus stuff on IRC. > > > >Is sleep-nexus new? I thought we've had that for a bit. > > It's been around in a few dts files, but as was noted, nothing uses > this stuff yet. So I should just drop the sleep-nexus changes? I can also prepare a patch that removes sleep-nexus from 8313rdb.dts. But how should we handle the sleep = <> properties then?
Anton Vorontsov wrote: > On Thu, Nov 05, 2009 at 02:03:14PM -0600, Scott Wood wrote: >> Kumar Gala wrote: >>> On Nov 5, 2009, at 1:48 PM, Scott Wood wrote: >>>> As for patch 3, Ben objected to the sleep-nexus stuff on IRC. >>> Is sleep-nexus new? I thought we've had that for a bit. >> It's been around in a few dts files, but as was noted, nothing uses >> this stuff yet. > > So I should just drop the sleep-nexus changes? I can also > prepare a patch that removes sleep-nexus from 8313rdb.dts. > But how should we handle the sleep = <> properties then? We could still have some sort of nexus node that is off to the side (pointed to with sleep-parent) and not inserting itself into the hierarchy. Or, we could allow multiple nodes to refer to the same sleep ID, and use the ID rather than a node to tie things together. If we do that, we'll probably want a simple index rather than a set of bits in the sleep property, so it can correspond to some kernel object that has some bookeeping info. Perhaps we could tie into the clock bindings that were discussed on devtree-discuss in August. -Scott
On Thu, Nov 05, 2009 at 02:25:15PM -0600, Scott Wood wrote: > Anton Vorontsov wrote: > >On Thu, Nov 05, 2009 at 02:03:14PM -0600, Scott Wood wrote: > >>Kumar Gala wrote: > >>>On Nov 5, 2009, at 1:48 PM, Scott Wood wrote: > >>>>As for patch 3, Ben objected to the sleep-nexus stuff on IRC. > >>>Is sleep-nexus new? I thought we've had that for a bit. > >>It's been around in a few dts files, but as was noted, nothing uses > >>this stuff yet. > > > >So I should just drop the sleep-nexus changes? I can also > >prepare a patch that removes sleep-nexus from 8313rdb.dts. > >But how should we handle the sleep = <> properties then? > > We could still have some sort of nexus node that is off to the side > (pointed to with sleep-parent) and not inserting itself into the > hierarchy. > > Or, we could allow multiple nodes to refer to the same sleep ID, and > use the ID rather than a node to tie things together. If we do > that, we'll probably want a simple index rather than a set of bits > in the sleep property, so it can correspond to some kernel object > that has some bookeeping info. Perhaps we could tie into the clock > bindings that were discussed on devtree-discuss in August. Yeah, reusing the clk api would be the best. Anyway, since we don't use the sleep-nexus stuff, I'd rather just add the power management controller nodes.
diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c index d306f07..b0c2619 100644 --- a/arch/powerpc/platforms/83xx/suspend.c +++ b/arch/powerpc/platforms/83xx/suspend.c @@ -194,7 +194,7 @@ out: return ret; } -static void mpc83xx_suspend_finish(void) +static void mpc83xx_suspend_end(void) { deep_sleeping = 0; } @@ -278,7 +278,7 @@ static struct platform_suspend_ops mpc83xx_suspend_ops = { .valid = mpc83xx_suspend_valid, .begin = mpc83xx_suspend_begin, .enter = mpc83xx_suspend_enter, - .finish = mpc83xx_suspend_finish, + .end = mpc83xx_suspend_end, }; static int pmc_probe(struct of_device *ofdev,
Currently 83xx PMC driver clears deep_sleeping variable very early, before devices are resumed. This makes fsl_deep_sleep() unusable in drivers' resume() callback. Sure, drivers can store fsl_deep_sleep() value on suspend and use the stored value on resume. But a better solution is to postpone clearing the deep_sleeping variable, i.e. move it into finish() callback. Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> --- arch/powerpc/platforms/83xx/suspend.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)