Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[Regression, post-3.5] System suspend broken on the Mackerel board

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


rjw at sisk

Jul 27, 2012, 3:53 PM

Post #1 of 8 (337 views)
Permalink
[Regression, post-3.5] System suspend broken on the Mackerel board

Hi Paul,

Unfortunately, your commit

commit ca5481c68e9fbcea62bb3c78ae6cccf99ca8fb73
Author: Paul Mundt <lethal [at] linux-sh>
Date: Tue Jul 10 12:08:14 2012 +0900

sh: pfc: Rudimentary pinctrl-backed GPIO support.

breaks system suspend on the Mackerel board (.config attached). The system
simply doesn't suspend and instead it hangs somewhere while suspending
devices (apparently before running the "late" callbacks).

If the above commit is reverted, system suspend works normally.

Thanks,
Rafael
Attachments: mackerel-config (50.0 KB)


lethal at linux-sh

Jul 27, 2012, 5:48 PM

Post #2 of 8 (323 views)
Permalink
Re: [Regression, post-3.5] System suspend broken on the Mackerel board [In reply to]

On Sat, Jul 28, 2012 at 12:53:11AM +0200, Rafael J. Wysocki wrote:
> Hi Paul,
>
> Unfortunately, your commit
>
> commit ca5481c68e9fbcea62bb3c78ae6cccf99ca8fb73
> Author: Paul Mundt <lethal [at] linux-sh>
> Date: Tue Jul 10 12:08:14 2012 +0900
>
> sh: pfc: Rudimentary pinctrl-backed GPIO support.
>
> breaks system suspend on the Mackerel board (.config attached). The system
> simply doesn't suspend and instead it hangs somewhere while suspending
> devices (apparently before running the "late" callbacks).
>
> If the above commit is reverted, system suspend works normally.
>
Ok, I'll take a look at it on Monday, thanks for reporting.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


lethal at linux-sh

Aug 1, 2012, 12:30 AM

Post #3 of 8 (333 views)
Permalink
Re: [Regression, post-3.5] System suspend broken on the Mackerel board [In reply to]

On Sat, Jul 28, 2012 at 12:53:11AM +0200, Rafael J. Wysocki wrote:
> Unfortunately, your commit
>
> commit ca5481c68e9fbcea62bb3c78ae6cccf99ca8fb73
> Author: Paul Mundt <lethal [at] linux-sh>
> Date: Tue Jul 10 12:08:14 2012 +0900
>
> sh: pfc: Rudimentary pinctrl-backed GPIO support.
>
> breaks system suspend on the Mackerel board (.config attached). The system
> simply doesn't suspend and instead it hangs somewhere while suspending
> devices (apparently before running the "late" callbacks).
>
> If the above commit is reverted, system suspend works normally.

On Tue, Jul 31, 2012 at 08:57:02PM -0700, kuninori.morimoto.gx [at] renesas wrote:
> gpio: sh7724_pfc handling gpio 0 -> 486
> core: sh7724_pfc support registered
> HW Breakpoints: SH-4A UBC support registered
> autorequest GPIO-53
> ------------[ cut here ]------------
> WARNING: at /opt/usr/src/WORK/morimoto/gitlinux/linux-2.6/drivers/gpio/gpiolib.3
> Modules linked in:
>
> Pid : 1, Comm: swapper
> CPU : 0 Not tainted (3.5.0-rc6+ #1407)
>
> PC is at gpio_ensure_requested+0x30/0x78
> PR is at gpio_ensure_requested+0x30/0x78

Morimoto-san's logs off-list made it clear what happened. Both of these
platforms are going gpio_request() calls at arch_initcall() time which
completely screwed up the ordering of the pfc core. We seem to -ENODEV
out in one place due to missing a pfc pointer initialization elsewhere
resulting in -EPROBE_DEFER from gpiolib.

Turns out we can just collapse the probe/init stuff anyways, so this
ought to fix it. I've verified that it fixes Morimoto-san's issue, my
expectation is that the mackerel case is likewise getting tripped up but
no one bothered implementing any error detecting logic for gpio_request()
failing, so it doesn't fail gracefully.

I'll be pushing this out to Linus shortly:

---

commit 1e32dfe323d156d5d7b25b9feffe015d19713db2
Author: Paul Mundt <lethal [at] linux-sh>
Date: Wed Aug 1 16:27:38 2012 +0900

sh: pfc: Fix up init ordering mess.

Commit ca5481c68e9fbcea62bb3c78ae6cccf99ca8fb73 ("sh: pfc: Rudimentary
pinctrl-backed GPIO support.") introduced a regression for platforms that
were doing early GPIO API calls (from arch_initcall() or earlier),
leading to a situation where our two-stage registration logic would trip
itself up and we'd -ENODEV out of the pinctrl registration path,
resulting in endless -EPROBE_DEFER errors. Further lack of checking any
sort of errors from gpio_request() resulted in boot time warnings,
tripping on the FLAG_REQUESTED test-and-set in gpio_ensure_requested().

As it turns out there's no particular need to bother with the two-stage
registration, as the platform bus is already available at the point that
we have to start caring. As such, it's easiest to simply fold these
together in to a single init path, the ordering of which is ensured
through the platform's mux registration, as usual.

Reported-by: Rafael J. Wysocki <rjw [at] sisk>
Reported-by: Kuninori Morimoto <kuninori.morimoto.gx [at] renesas>
Signed-off-by: Paul Mundt <lethal [at] linux-sh>

diff --git a/drivers/sh/pfc/pinctrl.c b/drivers/sh/pfc/pinctrl.c
index 814b292..2804eaa 100644
--- a/drivers/sh/pfc/pinctrl.c
+++ b/drivers/sh/pfc/pinctrl.c
@@ -325,20 +325,6 @@ static struct pinctrl_desc sh_pfc_pinctrl_desc = {
.confops = &sh_pfc_pinconf_ops,
};

-int sh_pfc_register_pinctrl(struct sh_pfc *pfc)
-{
- sh_pfc_pmx = kzalloc(sizeof(struct sh_pfc_pinctrl), GFP_KERNEL);
- if (unlikely(!sh_pfc_pmx))
- return -ENOMEM;
-
- spin_lock_init(&sh_pfc_pmx->lock);
-
- sh_pfc_pmx->pfc = pfc;
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(sh_pfc_register_pinctrl);
-
static inline void __devinit sh_pfc_map_one_gpio(struct sh_pfc *pfc,
struct sh_pfc_pinctrl *pmx,
struct pinmux_gpio *gpio,
@@ -505,7 +491,7 @@ static struct platform_device sh_pfc_pinctrl_device = {
.id = -1,
};

-static int __init sh_pfc_pinctrl_init(void)
+static int sh_pfc_pinctrl_init(void)
{
int rc;

@@ -519,10 +505,22 @@ static int __init sh_pfc_pinctrl_init(void)
return rc;
}

+int sh_pfc_register_pinctrl(struct sh_pfc *pfc)
+{
+ sh_pfc_pmx = kzalloc(sizeof(struct sh_pfc_pinctrl), GFP_KERNEL);
+ if (unlikely(!sh_pfc_pmx))
+ return -ENOMEM;
+
+ spin_lock_init(&sh_pfc_pmx->lock);
+
+ sh_pfc_pmx->pfc = pfc;
+
+ return sh_pfc_pinctrl_init();
+}
+EXPORT_SYMBOL_GPL(sh_pfc_register_pinctrl);
+
static void __exit sh_pfc_pinctrl_exit(void)
{
platform_driver_unregister(&sh_pfc_pinctrl_driver);
}
-
-subsys_initcall(sh_pfc_pinctrl_init);
module_exit(sh_pfc_pinctrl_exit);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rjw at sisk

Aug 4, 2012, 3:02 PM

Post #4 of 8 (311 views)
Permalink
Re: [Regression, post-3.5] System suspend broken on the Mackerel board [In reply to]

On Wednesday, August 01, 2012, Paul Mundt wrote:
> On Sat, Jul 28, 2012 at 12:53:11AM +0200, Rafael J. Wysocki wrote:
> > Unfortunately, your commit
> >
> > commit ca5481c68e9fbcea62bb3c78ae6cccf99ca8fb73
> > Author: Paul Mundt <lethal [at] linux-sh>
> > Date: Tue Jul 10 12:08:14 2012 +0900
> >
> > sh: pfc: Rudimentary pinctrl-backed GPIO support.
> >
> > breaks system suspend on the Mackerel board (.config attached). The system
> > simply doesn't suspend and instead it hangs somewhere while suspending
> > devices (apparently before running the "late" callbacks).
> >
> > If the above commit is reverted, system suspend works normally.
>
> On Tue, Jul 31, 2012 at 08:57:02PM -0700, kuninori.morimoto.gx [at] renesas wrote:
> > gpio: sh7724_pfc handling gpio 0 -> 486
> > core: sh7724_pfc support registered
> > HW Breakpoints: SH-4A UBC support registered
> > autorequest GPIO-53
> > ------------[ cut here ]------------
> > WARNING: at /opt/usr/src/WORK/morimoto/gitlinux/linux-2.6/drivers/gpio/gpiolib.3
> > Modules linked in:
> >
> > Pid : 1, Comm: swapper
> > CPU : 0 Not tainted (3.5.0-rc6+ #1407)
> >
> > PC is at gpio_ensure_requested+0x30/0x78
> > PR is at gpio_ensure_requested+0x30/0x78
>
> Morimoto-san's logs off-list made it clear what happened. Both of these
> platforms are going gpio_request() calls at arch_initcall() time which
> completely screwed up the ordering of the pfc core. We seem to -ENODEV
> out in one place due to missing a pfc pointer initialization elsewhere
> resulting in -EPROBE_DEFER from gpiolib.
>
> Turns out we can just collapse the probe/init stuff anyways, so this
> ought to fix it. I've verified that it fixes Morimoto-san's issue, my
> expectation is that the mackerel case is likewise getting tripped up but
> no one bothered implementing any error detecting logic for gpio_request()
> failing, so it doesn't fail gracefully.
>
> I'll be pushing this out to Linus shortly:

Thanks, this helped. Resume works correctly on Mackerel with 3.6-rc1.

However, I'm now seeing a different problem related to system suspend on that,
board which is that sh7372_enter_a3sm_common() returns immediately, as though
at least one of the wakeup signals was permanently asserted. This hadn't
happened before your last pull request was merged, so I suspect that one
of the irqdomain patches might introduce this behavior.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


lethal at linux-sh

Aug 6, 2012, 6:55 PM

Post #5 of 8 (305 views)
Permalink
Re: [Regression, post-3.5] System suspend broken on the Mackerel board [In reply to]

On Sun, Aug 05, 2012 at 12:02:43AM +0200, Rafael J. Wysocki wrote:
> On Wednesday, August 01, 2012, Paul Mundt wrote:
> > Turns out we can just collapse the probe/init stuff anyways, so this
> > ought to fix it. I've verified that it fixes Morimoto-san's issue, my
> > expectation is that the mackerel case is likewise getting tripped up but
> > no one bothered implementing any error detecting logic for gpio_request()
> > failing, so it doesn't fail gracefully.
> >
> > I'll be pushing this out to Linus shortly:
>
> Thanks, this helped. Resume works correctly on Mackerel with 3.6-rc1.
>
> However, I'm now seeing a different problem related to system suspend on that,
> board which is that sh7372_enter_a3sm_common() returns immediately, as though
> at least one of the wakeup signals was permanently asserted. This hadn't
> happened before your last pull request was merged, so I suspect that one
> of the irqdomain patches might introduce this behavior.
>
Ok, I'll back off the irqdomain selection until we have a chance to test
it more on the other platforms.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rjw at sisk

Aug 8, 2012, 2:21 AM

Post #6 of 8 (309 views)
Permalink
Re: [Regression, post-3.5] System suspend broken on the Mackerel board [In reply to]

On Tuesday, August 07, 2012, Paul Mundt wrote:
> On Sun, Aug 05, 2012 at 12:02:43AM +0200, Rafael J. Wysocki wrote:
> > On Wednesday, August 01, 2012, Paul Mundt wrote:
> > > Turns out we can just collapse the probe/init stuff anyways, so this
> > > ought to fix it. I've verified that it fixes Morimoto-san's issue, my
> > > expectation is that the mackerel case is likewise getting tripped up but
> > > no one bothered implementing any error detecting logic for gpio_request()
> > > failing, so it doesn't fail gracefully.
> > >
> > > I'll be pushing this out to Linus shortly:
> >
> > Thanks, this helped. Resume works correctly on Mackerel with 3.6-rc1.
> >
> > However, I'm now seeing a different problem related to system suspend on that,
> > board which is that sh7372_enter_a3sm_common() returns immediately, as though
> > at least one of the wakeup signals was permanently asserted. This hadn't
> > happened before your last pull request was merged, so I suspect that one
> > of the irqdomain patches might introduce this behavior.
> >
> Ok, I'll back off the irqdomain selection until we have a chance to test
> it more on the other platforms.

If you mean removing the select from drivers/sh/intc/Kconfig, that alone doesn't
help. Reverting commit 1d6a21b0a672fb29b01ccf397d478e0541e17716
(sh: intc: initial irqdomain support.) helps, though.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


lethal at linux-sh

Aug 8, 2012, 9:40 PM

Post #7 of 8 (298 views)
Permalink
Re: [Regression, post-3.5] System suspend broken on the Mackerel board [In reply to]

On Wed, Aug 08, 2012 at 11:23:04AM +0200, Rafael J. Wysocki wrote:
> On Tuesday, August 07, 2012, Paul Mundt wrote:
> > On Sun, Aug 05, 2012 at 12:02:43AM +0200, Rafael J. Wysocki wrote:
> > > On Wednesday, August 01, 2012, Paul Mundt wrote:
> > > > Turns out we can just collapse the probe/init stuff anyways, so this
> > > > ought to fix it. I've verified that it fixes Morimoto-san's issue, my
> > > > expectation is that the mackerel case is likewise getting tripped up but
> > > > no one bothered implementing any error detecting logic for gpio_request()
> > > > failing, so it doesn't fail gracefully.
> > > >
> > > > I'll be pushing this out to Linus shortly:
> > >
> > > Thanks, this helped. Resume works correctly on Mackerel with 3.6-rc1.
> > >
> > > However, I'm now seeing a different problem related to system suspend on that,
> > > board which is that sh7372_enter_a3sm_common() returns immediately, as though
> > > at least one of the wakeup signals was permanently asserted. This hadn't
> > > happened before your last pull request was merged, so I suspect that one
> > > of the irqdomain patches might introduce this behavior.
> > >
> > Ok, I'll back off the irqdomain selection until we have a chance to test
> > it more on the other platforms.
>
> If you mean removing the select from drivers/sh/intc/Kconfig, that alone doesn't
> help. Reverting commit 1d6a21b0a672fb29b01ccf397d478e0541e17716
> (sh: intc: initial irqdomain support.) helps, though.
>
Yeah, I see what happened now. I inadvertently trampled the -EEXIST
behaviour. I've taken care of it now, so hopefully you'll have better
luck with -rc2!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rjw at sisk

Aug 9, 2012, 2:20 PM

Post #8 of 8 (296 views)
Permalink
Re: [Regression, post-3.5] System suspend broken on the Mackerel board [In reply to]

On Thursday, August 09, 2012, Paul Mundt wrote:
> On Wed, Aug 08, 2012 at 11:23:04AM +0200, Rafael J. Wysocki wrote:
> > On Tuesday, August 07, 2012, Paul Mundt wrote:
> > > On Sun, Aug 05, 2012 at 12:02:43AM +0200, Rafael J. Wysocki wrote:
> > > > On Wednesday, August 01, 2012, Paul Mundt wrote:
> > > > > Turns out we can just collapse the probe/init stuff anyways, so this
> > > > > ought to fix it. I've verified that it fixes Morimoto-san's issue, my
> > > > > expectation is that the mackerel case is likewise getting tripped up but
> > > > > no one bothered implementing any error detecting logic for gpio_request()
> > > > > failing, so it doesn't fail gracefully.
> > > > >
> > > > > I'll be pushing this out to Linus shortly:
> > > >
> > > > Thanks, this helped. Resume works correctly on Mackerel with 3.6-rc1.
> > > >
> > > > However, I'm now seeing a different problem related to system suspend on that,
> > > > board which is that sh7372_enter_a3sm_common() returns immediately, as though
> > > > at least one of the wakeup signals was permanently asserted. This hadn't
> > > > happened before your last pull request was merged, so I suspect that one
> > > > of the irqdomain patches might introduce this behavior.
> > > >
> > > Ok, I'll back off the irqdomain selection until we have a chance to test
> > > it more on the other platforms.
> >
> > If you mean removing the select from drivers/sh/intc/Kconfig, that alone doesn't
> > help. Reverting commit 1d6a21b0a672fb29b01ccf397d478e0541e17716
> > (sh: intc: initial irqdomain support.) helps, though.
> >
> Yeah, I see what happened now. I inadvertently trampled the -EEXIST
> behaviour. I've taken care of it now, so hopefully you'll have better
> luck with -rc2!

I tested your "sh: intc: Handle domain association for sparseirq pre-allocated
vectors" patch in the meantime and it does fix the issue.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.