From: Paul Mundt <lethal@linux-sh.org> To: "Rafael J. Wysocki" <rjw@sisk.pl>, kuninori.morimoto.gx@renesas.com Cc: Magnus Damm <magnus.damm@gmail.com>, Linux-sh list <linux-sh@vger.kernel.org>, Linux PM list <linux-pm@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, Kuninori Morimoto <kuninori.morimoto.gx@gmail.com> Subject: Re: [Regression, post-3.5] System suspend broken on the Mackerel board Date: Wed, 01 Aug 2012 07:30:04 +0000 [thread overview] Message-ID: <20120801073004.GE15380@linux-sh.org> (raw) In-Reply-To: <878vdze039.wl%kuninori.morimoto.gx@renesas.com> <201207280053.11701.rjw@sisk.pl> In-Reply-To: <201207280053.11701.rjw@sisk.pl> On Sat, Jul 28, 2012 at 12:53:11AM +0200, Rafael J. Wysocki wrote: > Unfortunately, your commit > > commit ca5481c68e9fbcea62bb3c78ae6cccf99ca8fb73 > Author: Paul Mundt <lethal@linux-sh.org> > 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@renesas.com 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@linux-sh.org> 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@sisk.pl> Reported-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Signed-off-by: Paul Mundt <lethal@linux-sh.org> 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);
WARNING: multiple messages have this Message-ID (diff)
From: Paul Mundt <lethal@linux-sh.org> To: "Rafael J. Wysocki" <rjw@sisk.pl>, kuninori.morimoto.gx@renesas.com Cc: Magnus Damm <magnus.damm@gmail.com>, "Linux-sh list" <linux-sh@vger.kernel.org>, Linux PM list <linux-pm@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, Kuninori Morimoto <kuninori.morimoto.gx@gmail.com> Subject: Re: [Regression, post-3.5] System suspend broken on the Mackerel board Date: Wed, 1 Aug 2012 16:30:04 +0900 [thread overview] Message-ID: <20120801073004.GE15380@linux-sh.org> (raw) In-Reply-To: <878vdze039.wl%kuninori.morimoto.gx@renesas.com> <201207280053.11701.rjw@sisk.pl> On Sat, Jul 28, 2012 at 12:53:11AM +0200, Rafael J. Wysocki wrote: > Unfortunately, your commit > > commit ca5481c68e9fbcea62bb3c78ae6cccf99ca8fb73 > Author: Paul Mundt <lethal@linux-sh.org> > 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@renesas.com 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@linux-sh.org> 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@sisk.pl> Reported-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Signed-off-by: Paul Mundt <lethal@linux-sh.org> 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);
next prev parent reply other threads:[~2012-08-01 7:30 UTC|newest] Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top 2012-07-27 22:53 [Regression, post-3.5] System suspend broken on the Mackerel board Rafael J. Wysocki 2012-07-27 22:53 ` Rafael J. Wysocki 2012-07-28 0:48 ` Paul Mundt 2012-07-28 0:48 ` Paul Mundt 2012-08-01 3:57 ` bug report: ecovec boot got WARNING kuninori.morimoto.gx 2012-08-01 5:04 ` Paul Mundt 2012-08-01 5:39 ` Kuninori Morimoto 2012-08-01 7:30 ` Paul Mundt [this message] 2012-08-01 7:30 ` [Regression, post-3.5] System suspend broken on the Mackerel board Paul Mundt 2012-08-04 22:02 ` Rafael J. Wysocki 2012-08-04 22:02 ` Rafael J. Wysocki 2012-08-07 1:55 ` Paul Mundt 2012-08-07 1:55 ` Paul Mundt 2012-08-08 9:23 ` Rafael J. Wysocki 2012-08-08 9:23 ` Rafael J. Wysocki 2012-08-09 4:40 ` Paul Mundt 2012-08-09 4:40 ` Paul Mundt 2012-08-09 21:20 ` Rafael J. Wysocki 2012-08-09 21:20 ` Rafael J. Wysocki
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20120801073004.GE15380@linux-sh.org \ --to=lethal@linux-sh.org \ --cc=kuninori.morimoto.gx@gmail.com \ --cc=kuninori.morimoto.gx@renesas.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=linux-sh@vger.kernel.org \ --cc=magnus.damm@gmail.com \ --cc=rjw@sisk.pl \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.