diff mbox

[3/6] ARM: SAMSUNG: pm: Adjust for pinctrl- and DT-enabled platforms

Message ID 1368807872-2601-4-git-send-email-t.figa@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tomasz Figa May 17, 2013, 4:24 p.m. UTC
This patch makes legacy code on suspend/resume path being executed
conditionally, on non-DT platforms only, to fix suspend/resume of
DT-enabled systems, for which the code is inappropriate.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/plat-samsung/pm.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

Comments

Doug Anderson May 17, 2013, 7:24 p.m. UTC | #1
Tomasz,

On Fri, May 17, 2013 at 9:24 AM, Tomasz Figa <t.figa@samsung.com> wrote:
> This patch makes legacy code on suspend/resume path being executed
> conditionally, on non-DT platforms only, to fix suspend/resume of
> DT-enabled systems, for which the code is inappropriate.
>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  arch/arm/plat-samsung/pm.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/plat-samsung/pm.c b/arch/arm/plat-samsung/pm.c
> index 53210ec..8ac2b2d 100644
> --- a/arch/arm/plat-samsung/pm.c
> +++ b/arch/arm/plat-samsung/pm.c
> @@ -261,7 +261,8 @@ static int s3c_pm_enter(suspend_state_t state)
>          * require a full power-cycle)
>         */
>
> -       if (!any_allowed(s3c_irqwake_intmask, s3c_irqwake_intallow) &&
> +       if (!of_have_populated_dt() &&
> +           !any_allowed(s3c_irqwake_intmask, s3c_irqwake_intallow) &&

I'd rather see you modify patch set #2 to provide some function to
retrieve just the eint mask and then use it here.  Your patch removes
this test and doesn't replace it with anything.  If you moved this
test to pinctrl directly you'd lose the test against intallow.

...or do you think this test is no longer useful for some reason?


>             !any_allowed(s3c_irqwake_eintmask, s3c_irqwake_eintallow)) {
>                 printk(KERN_ERR "%s: No wake-up sources!\n", __func__);
>                 printk(KERN_ERR "%s: Aborting sleep\n", __func__);
> @@ -270,8 +271,11 @@ static int s3c_pm_enter(suspend_state_t state)
>
>         /* save all necessary core registers not covered by the drivers */
>
> -       samsung_pm_save_gpios();
> -       samsung_pm_saved_gpios();
> +       if (!of_have_populated_dt()) {
> +               samsung_pm_save_gpios();
> +               samsung_pm_saved_gpios();
> +       }
> +

Ah, the important part here is the "saved", not the "save"!  The
"save" should get a NULL chip for all GPIOs and effectively be a
no-op.

Skipping the "saved" is important of s3c64xx and s5p64x0 where the
"saved" seems to transition things over to powerdown mode.  Hopefully
a future patch of yours adds that back for those platforms in the
pinmux world.  ...same for restore.


Summary: I've tested this on exynos5250-snow and it's reasonable but
I'd love a response about the missing test before adding Reviewed-by.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomasz Figa May 17, 2013, 8:23 p.m. UTC | #2
On Friday 17 of May 2013 12:24:04 Doug Anderson wrote:
> Tomasz,
> 
> On Fri, May 17, 2013 at 9:24 AM, Tomasz Figa <t.figa@samsung.com> wrote:
> > This patch makes legacy code on suspend/resume path being executed
> > conditionally, on non-DT platforms only, to fix suspend/resume of
> > DT-enabled systems, for which the code is inappropriate.
> > 
> > Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> > 
> >  arch/arm/plat-samsung/pm.c | 17 ++++++++++++-----
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm/plat-samsung/pm.c b/arch/arm/plat-samsung/pm.c
> > index 53210ec..8ac2b2d 100644
> > --- a/arch/arm/plat-samsung/pm.c
> > +++ b/arch/arm/plat-samsung/pm.c
> > @@ -261,7 +261,8 @@ static int s3c_pm_enter(suspend_state_t state)
> > 
> >          * require a full power-cycle)
> >         
> >         */
> > 
> > -       if (!any_allowed(s3c_irqwake_intmask, s3c_irqwake_intallow) &&
> > +       if (!of_have_populated_dt() &&
> > +           !any_allowed(s3c_irqwake_intmask, s3c_irqwake_intallow) &&
> 
> I'd rather see you modify patch set #2 to provide some function to
> retrieve just the eint mask and then use it here.  Your patch removes
> this test and doesn't replace it with anything.  If you moved this
> test to pinctrl directly you'd lose the test against intallow.

Well, looking from the perspective of status before my patch, it just 
bypasses a test that is incorrect on DT-enabled platforms.

I agree that this test is rather reasonable to have, but it would require 
defining a new interface and moving all platforms to it, which for now 
would be a bit of overkill.

IMHO a separate series that sanitizes the whole PM handling in plat-
samsung, including a rework of this check to make it cover all platforms 
in a generic and multiplatform-friendly way. What do you think?

> ...or do you think this test is no longer useful for some reason?
> 
> >             !any_allowed(s3c_irqwake_eintmask, s3c_irqwake_eintallow))
> >             {
> >             
> >                 printk(KERN_ERR "%s: No wake-up sources!\n",
> >                 __func__);
> >                 printk(KERN_ERR "%s: Aborting sleep\n", __func__);
> > 
> > @@ -270,8 +271,11 @@ static int s3c_pm_enter(suspend_state_t state)
> > 
> >         /* save all necessary core registers not covered by the
> >         drivers */
> > 
> > -       samsung_pm_save_gpios();
> > -       samsung_pm_saved_gpios();
> > +       if (!of_have_populated_dt()) {
> > +               samsung_pm_save_gpios();
> > +               samsung_pm_saved_gpios();
> > +       }
> > +
> 
> Ah, the important part here is the "saved", not the "save"!  The
> "save" should get a NULL chip for all GPIOs and effectively be a
> no-op.
> 
> Skipping the "saved" is important of s3c64xx and s5p64x0 where the
> "saved" seems to transition things over to powerdown mode.  Hopefully
> a future patch of yours adds that back for those platforms in the
> pinmux world.  ...same for restore.

S3C64xx can be switched to power down pin configuration manually, but if 
you don't do it, it will activate it automatically after entering sleep 
mode.

> Summary: I've tested this on exynos5250-snow and it's reasonable but
> I'd love a response about the missing test before adding Reviewed-by.

Thanks.

Best regards,
Tomasz

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Doug Anderson May 17, 2013, 8:56 p.m. UTC | #3
Tomasz,

On Fri, May 17, 2013 at 1:23 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>> I'd rather see you modify patch set #2 to provide some function to
>> retrieve just the eint mask and then use it here.  Your patch removes
>> this test and doesn't replace it with anything.  If you moved this
>> test to pinctrl directly you'd lose the test against intallow.
>
> Well, looking from the perspective of status before my patch, it just
> bypasses a test that is incorrect on DT-enabled platforms.

True.  What was there before was broken and this avoids the broken code.


> I agree that this test is rather reasonable to have, but it would require
> defining a new interface and moving all platforms to it, which for now
> would be a bit of overkill.

It seems like you could use the same type of solution as patch set #2?

...oh, but that's only for exynos!  I guess we would need something
similar for other platforms.  ...and until we do those other platforms
(including S3C64xx, I think) are still using the old
s3c_irqwake_eintmask, right?

...so overall your patch series only fully fixes exynos, though it
does make other platforms less broken which is a plus.  :)


> IMHO a separate series that sanitizes the whole PM handling in plat-
> samsung, including a rework of this check to make it cover all platforms
> in a generic and multiplatform-friendly way. What do you think?

Sure, I think that would be OK.


>> Ah, the important part here is the "saved", not the "save"!  The
>> "save" should get a NULL chip for all GPIOs and effectively be a
>> no-op.
>>
>> Skipping the "saved" is important of s3c64xx and s5p64x0 where the
>> "saved" seems to transition things over to powerdown mode.  Hopefully
>> a future patch of yours adds that back for those platforms in the
>> pinmux world.  ...same for restore.
>
> S3C64xx can be switched to power down pin configuration manually, but if
> you don't do it, it will activate it automatically after entering sleep
> mode.

How would restore work in that case?  I'd imagine that it would
automatically switch out of the powerdown config at wakeup before
running software?  That doesn't seem like a great idea.

I think that to make S3C64xx work properly we'd need to solve.


I wouldn't be opposed to a re-spin to address some of the above, but I
also won't object to this landing and remaining issues being addressed
in future patches.  This patch definitely does make things better and
no worse.  :)

On exynos5250-snow (pinmux backported to 3.8):

Tested-by: Doug Anderson <dianders@chromium.org>

Reviewed-by: Doug Anderson <dianders@chromium.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomasz Figa May 17, 2013, 9:07 p.m. UTC | #4
On Friday 17 of May 2013 13:56:39 Doug Anderson wrote:
> Tomasz,
> 
> On Fri, May 17, 2013 at 1:23 PM, Tomasz Figa <tomasz.figa@gmail.com> 
wrote:
> >> I'd rather see you modify patch set #2 to provide some function to
> >> retrieve just the eint mask and then use it here.  Your patch removes
> >> this test and doesn't replace it with anything.  If you moved this
> >> test to pinctrl directly you'd lose the test against intallow.
> > 
> > Well, looking from the perspective of status before my patch, it just
> > bypasses a test that is incorrect on DT-enabled platforms.
> 
> True.  What was there before was broken and this avoids the broken code.
> > I agree that this test is rather reasonable to have, but it would
> > require defining a new interface and moving all platforms to it,
> > which for now would be a bit of overkill.
> 
> It seems like you could use the same type of solution as patch set #2?
> 
> ...oh, but that's only for exynos!  I guess we would need something
> similar for other platforms.  ...and until we do those other platforms
> (including S3C64xx, I think) are still using the old
> s3c_irqwake_eintmask, right?

Correct.

Also as an extra side note, intallow is used here as a mask of valid 
eintmask bits on given platform. On Exynos SoCs there are 32 EINTs, so 
this extra mask is not needed.

> ...so overall your patch series only fully fixes exynos, though it
> does make other platforms less broken which is a plus.  :)
> 
> > IMHO a separate series that sanitizes the whole PM handling in plat-
> > samsung, including a rework of this check to make it cover all
> > platforms in a generic and multiplatform-friendly way. What do you
> > think?
> Sure, I think that would be OK.
> 
> >> Ah, the important part here is the "saved", not the "save"!  The
> >> "save" should get a NULL chip for all GPIOs and effectively be a
> >> no-op.
> >> 
> >> Skipping the "saved" is important of s3c64xx and s5p64x0 where the
> >> "saved" seems to transition things over to powerdown mode.  Hopefully
> >> a future patch of yours adds that back for those platforms in the
> >> pinmux world.  ...same for restore.
> > 
> > S3C64xx can be switched to power down pin configuration manually, but
> > if you don't do it, it will activate it automatically after entering
> > sleep mode.
> 
> How would restore work in that case?  I'd imagine that it would
> automatically switch out of the powerdown config at wakeup before
> running software?  That doesn't seem like a great idea.

This is configurable. There is a bit that determines whether it should 
switch back to normal config automatically or not. What's interesting is 
that AFAIR current code uses automatic switch, which even with the glitch 
prevention in resume can glitch things, due to the period of time between 
wake-up and resume when pins are misconfigured.

I think we should just configure this bit for manual switch back and 
trigger the switch from .resumed() callback of the pin controller in 
pinctrl-s3c64xx driver. I will post a patch for this some day.

> I think that to make S3C64xx work properly we'd need to solve.
> 
> 
> I wouldn't be opposed to a re-spin to address some of the above, but I
> also won't object to this landing and remaining issues being addressed
> in future patches.  This patch definitely does make things better and
> no worse.  :)

Let's see. If nobody takes this until Monday, which is very likely, I will 
send new version.

> On exynos5250-snow (pinmux backported to 3.8):
> 
> Tested-by: Doug Anderson <dianders@chromium.org>
> 
> Reviewed-by: Doug Anderson <dianders@chromium.org>

Thanks.

Best regards,
Tomasz

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij May 21, 2013, 11:29 a.m. UTC | #5
On Fri, May 17, 2013 at 11:07 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> On Friday 17 of May 2013 13:56:39 Doug Anderson wrote:
>> I wouldn't be opposed to a re-spin to address some of the above, but I
>> also won't object to this landing and remaining issues being addressed
>> in future patches.  This patch definitely does make things better and
>> no worse.  :)
>
> Let's see. If nobody takes this until Monday, which is very likely, I will
> send new version.

I applied 1 & 2 so pls repost your updated patches 3,4,5,6 on
monday and let's continue fixing.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomasz Figa May 21, 2013, 1:15 p.m. UTC | #6
Hi Linus,

On Tuesday 21 of May 2013 13:29:23 Linus Walleij wrote:
> On Fri, May 17, 2013 at 11:07 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> > On Friday 17 of May 2013 13:56:39 Doug Anderson wrote:
> >> I wouldn't be opposed to a re-spin to address some of the above, but I
> >> also won't object to this landing and remaining issues being addressed
> >> in future patches.  This patch definitely does make things better and
> >> no worse.  :)
> > 
> > Let's see. If nobody takes this until Monday, which is very likely, I will
> > send new version.
> 
> I applied 1 & 2 so pls repost your updated patches 3,4,5,6 on
> monday and let's continue fixing.

Thanks.

Feel free to apply current version of patches 3, 4 and 5 as well. I will post 
updated patch 6 in a while.

Best regards,
Tomasz Figa May 21, 2013, 5:06 p.m. UTC | #7
Just posted updated version of patch 6/6.

Best regards,
Tomasz Figa June 10, 2013, 2:45 p.m. UTC | #8
Hi,

On Friday 17 of May 2013 18:24:29 Tomasz Figa wrote:
> This patch makes legacy code on suspend/resume path being executed
> conditionally, on non-DT platforms only, to fix suspend/resume of
> DT-enabled systems, for which the code is inappropriate.
> 
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  arch/arm/plat-samsung/pm.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)

It seems like this patch did not make it into 3.10-rc5, while rest of patches 
did, which ended up with suspend/resume still being broken.

What should we do in this case? 

Best regards,
Tomasz

> diff --git a/arch/arm/plat-samsung/pm.c b/arch/arm/plat-samsung/pm.c
> index 53210ec..8ac2b2d 100644
> --- a/arch/arm/plat-samsung/pm.c
> +++ b/arch/arm/plat-samsung/pm.c
> @@ -261,7 +261,8 @@ static int s3c_pm_enter(suspend_state_t state)
>  	 * require a full power-cycle)
>  	*/
> 
> -	if (!any_allowed(s3c_irqwake_intmask, s3c_irqwake_intallow) &&
> +	if (!of_have_populated_dt() &&
> +	    !any_allowed(s3c_irqwake_intmask, s3c_irqwake_intallow) &&
>  	    !any_allowed(s3c_irqwake_eintmask, s3c_irqwake_eintallow)) {
>  		printk(KERN_ERR "%s: No wake-up sources!\n", __func__);
>  		printk(KERN_ERR "%s: Aborting sleep\n", __func__);
> @@ -270,8 +271,11 @@ static int s3c_pm_enter(suspend_state_t state)
> 
>  	/* save all necessary core registers not covered by the drivers */
> 
> -	samsung_pm_save_gpios();
> -	samsung_pm_saved_gpios();
> +	if (!of_have_populated_dt()) {
> +		samsung_pm_save_gpios();
> +		samsung_pm_saved_gpios();
> +	}
> +
>  	s3c_pm_save_uarts();
>  	s3c_pm_save_core();
> 
> @@ -310,8 +314,11 @@ static int s3c_pm_enter(suspend_state_t state)
> 
>  	s3c_pm_restore_core();
>  	s3c_pm_restore_uarts();
> -	samsung_pm_restore_gpios();
> -	s3c_pm_restored_gpios();
> +
> +	if (!of_have_populated_dt()) {
> +		samsung_pm_restore_gpios();
> +		s3c_pm_restored_gpios();
> +	}
> 
>  	s3c_pm_debug_init();
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij June 10, 2013, 4:13 p.m. UTC | #9
On Mon, Jun 10, 2013 at 4:45 PM, Tomasz Figa <t.figa@samsung.com> wrote:
> On Friday 17 of May 2013 18:24:29 Tomasz Figa wrote:
>> This patch makes legacy code on suspend/resume path being executed
>> conditionally, on non-DT platforms only, to fix suspend/resume of
>> DT-enabled systems, for which the code is inappropriate.
>>
>> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>  arch/arm/plat-samsung/pm.c | 17 ++++++++++++-----
>>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> It seems like this patch did not make it into 3.10-rc5, while rest of patches
> did, which ended up with suspend/resume still being broken.
>
> What should we do in this case?

I did not apply this patch to the pinctrl tree because it begins
with ARM: but it appears I promised to carry it anyway...

I need an ACK from the ARM SoC people to do that though,
or you can take it through ARM SoC fixes directly.

I would just ask Olof/Arnd to apply it directly as a separate
patch.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Olof Johansson June 11, 2013, 7:45 a.m. UTC | #10
On Mon, Jun 10, 2013 at 06:13:10PM +0200, Linus Walleij wrote:
> On Mon, Jun 10, 2013 at 4:45 PM, Tomasz Figa <t.figa@samsung.com> wrote:
> > On Friday 17 of May 2013 18:24:29 Tomasz Figa wrote:
> >> This patch makes legacy code on suspend/resume path being executed
> >> conditionally, on non-DT platforms only, to fix suspend/resume of
> >> DT-enabled systems, for which the code is inappropriate.
> >>
> >> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> >> ---
> >>  arch/arm/plat-samsung/pm.c | 17 ++++++++++++-----
> >>  1 file changed, 12 insertions(+), 5 deletions(-)
> >
> > It seems like this patch did not make it into 3.10-rc5, while rest of patches
> > did, which ended up with suspend/resume still being broken.
> >
> > What should we do in this case?
> 
> I did not apply this patch to the pinctrl tree because it begins
> with ARM: but it appears I promised to carry it anyway...
> 
> I need an ACK from the ARM SoC people to do that though,
> or you can take it through ARM SoC fixes directly.
> 
> I would just ask Olof/Arnd to apply it directly as a separate
> patch.

It makes sense to take this through arm-soc since it's no longer
dependent on anything else.

Applied to fixes. Please do follow through with the promised cleanups at some
point, Tomasz. :-)


-Olof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Olof Johansson June 11, 2013, 8:21 a.m. UTC | #11
On Tue, Jun 11, 2013 at 12:45 AM, Olof Johansson <olof@lixom.net> wrote:
> On Mon, Jun 10, 2013 at 06:13:10PM +0200, Linus Walleij wrote:
>> On Mon, Jun 10, 2013 at 4:45 PM, Tomasz Figa <t.figa@samsung.com> wrote:
>> > On Friday 17 of May 2013 18:24:29 Tomasz Figa wrote:
>> >> This patch makes legacy code on suspend/resume path being executed
>> >> conditionally, on non-DT platforms only, to fix suspend/resume of
>> >> DT-enabled systems, for which the code is inappropriate.
>> >>
>> >> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
>> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> >> ---
>> >>  arch/arm/plat-samsung/pm.c | 17 ++++++++++++-----
>> >>  1 file changed, 12 insertions(+), 5 deletions(-)
>> >
>> > It seems like this patch did not make it into 3.10-rc5, while rest of patches
>> > did, which ended up with suspend/resume still being broken.
>> >
>> > What should we do in this case?
>>
>> I did not apply this patch to the pinctrl tree because it begins
>> with ARM: but it appears I promised to carry it anyway...
>>
>> I need an ACK from the ARM SoC people to do that though,
>> or you can take it through ARM SoC fixes directly.
>>
>> I would just ask Olof/Arnd to apply it directly as a separate
>> patch.
>
> It makes sense to take this through arm-soc since it's no longer
> dependent on anything else.
>
> Applied to fixes. Please do follow through with the promised cleanups at some
> point, Tomasz. :-)


Btw, I had to add an include of <linux/of.h> or else all non-DT builds broke...


-Olof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomasz Figa June 12, 2013, 12:15 a.m. UTC | #12
Hi Olof,

On Tuesday 11 of June 2013 01:21:59 Olof Johansson wrote:
> On Tue, Jun 11, 2013 at 12:45 AM, Olof Johansson <olof@lixom.net> wrote:
> > On Mon, Jun 10, 2013 at 06:13:10PM +0200, Linus Walleij wrote:
> >> On Mon, Jun 10, 2013 at 4:45 PM, Tomasz Figa <t.figa@samsung.com> 
wrote:
> >> > On Friday 17 of May 2013 18:24:29 Tomasz Figa wrote:
> >> >> This patch makes legacy code on suspend/resume path being executed
> >> >> conditionally, on non-DT platforms only, to fix suspend/resume of
> >> >> DT-enabled systems, for which the code is inappropriate.
> >> >> 
> >> >> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> >> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> >> >> ---
> >> >> 
> >> >>  arch/arm/plat-samsung/pm.c | 17 ++++++++++++-----
> >> >>  1 file changed, 12 insertions(+), 5 deletions(-)
> >> > 
> >> > It seems like this patch did not make it into 3.10-rc5, while rest
> >> > of patches did, which ended up with suspend/resume still being
> >> > broken.
> >> > 
> >> > What should we do in this case?
> >> 
> >> I did not apply this patch to the pinctrl tree because it begins
> >> with ARM: but it appears I promised to carry it anyway...
> >> 
> >> I need an ACK from the ARM SoC people to do that though,
> >> or you can take it through ARM SoC fixes directly.
> >> 
> >> I would just ask Olof/Arnd to apply it directly as a separate
> >> patch.
> > 
> > It makes sense to take this through arm-soc since it's no longer
> > dependent on anything else.
> > 
> > Applied to fixes. 

Thanks.

> > Please do follow through with the promised cleanups
> > at some point, Tomasz. :-)

Yeah, just managed to reserve some time for it, so stay tuned ;) .

> 
> Btw, I had to add an include of <linux/of.h> or else all non-DT builds
> broke...

Sorry, I must have missed it.

I guess it's the right time to set up some autobuild framework to check 
all patches with several most interesting configs. I guess some git hooks 
would be enough...

Best regards,
Tomasz

> 
> -Olof
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Olof Johansson June 12, 2013, 12:20 a.m. UTC | #13
On Tue, Jun 11, 2013 at 5:15 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:

>> > Please do follow through with the promised cleanups
>> > at some point, Tomasz. :-)
>
> Yeah, just managed to reserve some time for it, so stay tuned ;) .

Cool.

>> Btw, I had to add an include of <linux/of.h> or else all non-DT builds
>> broke...
>
> Sorry, I must have missed it.
>
> I guess it's the right time to set up some autobuild framework to check
> all patches with several most interesting configs. I guess some git hooks
> would be enough...

No worries. I tend to build all defconfigs after doing a batch of
pulls, and that's when we find these issues. I also boot on the few
boards that I have to make sure nothing catastrophic is broken, but
there's obviously much less coverage on that side.


-Olof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/plat-samsung/pm.c b/arch/arm/plat-samsung/pm.c
index 53210ec..8ac2b2d 100644
--- a/arch/arm/plat-samsung/pm.c
+++ b/arch/arm/plat-samsung/pm.c
@@ -261,7 +261,8 @@  static int s3c_pm_enter(suspend_state_t state)
 	 * require a full power-cycle)
 	*/
 
-	if (!any_allowed(s3c_irqwake_intmask, s3c_irqwake_intallow) &&
+	if (!of_have_populated_dt() &&
+	    !any_allowed(s3c_irqwake_intmask, s3c_irqwake_intallow) &&
 	    !any_allowed(s3c_irqwake_eintmask, s3c_irqwake_eintallow)) {
 		printk(KERN_ERR "%s: No wake-up sources!\n", __func__);
 		printk(KERN_ERR "%s: Aborting sleep\n", __func__);
@@ -270,8 +271,11 @@  static int s3c_pm_enter(suspend_state_t state)
 
 	/* save all necessary core registers not covered by the drivers */
 
-	samsung_pm_save_gpios();
-	samsung_pm_saved_gpios();
+	if (!of_have_populated_dt()) {
+		samsung_pm_save_gpios();
+		samsung_pm_saved_gpios();
+	}
+
 	s3c_pm_save_uarts();
 	s3c_pm_save_core();
 
@@ -310,8 +314,11 @@  static int s3c_pm_enter(suspend_state_t state)
 
 	s3c_pm_restore_core();
 	s3c_pm_restore_uarts();
-	samsung_pm_restore_gpios();
-	s3c_pm_restored_gpios();
+
+	if (!of_have_populated_dt()) {
+		samsung_pm_restore_gpios();
+		s3c_pm_restored_gpios();
+	}
 
 	s3c_pm_debug_init();