Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754613Ab2HAHas (ORCPT ); Wed, 1 Aug 2012 03:30:48 -0400 Received: from linux-sh.org ([111.68.239.195]:33697 "EHLO linux-sh.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753612Ab2HAHaO (ORCPT ); Wed, 1 Aug 2012 03:30:14 -0400 Date: Wed, 1 Aug 2012 16:30:04 +0900 From: Paul Mundt To: "Rafael J. Wysocki" , kuninori.morimoto.gx@renesas.com Cc: Magnus Damm , "Linux-sh list" , Linux PM list , LKML , Kuninori Morimoto Subject: Re: [Regression, post-3.5] System suspend broken on the Mackerel board Message-ID: <20120801073004.GE15380@linux-sh.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <878vdze039.wl%kuninori.morimoto.gx@renesas.com> <201207280053.11701.rjw@sisk.pl> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4865 Lines: 135 On Sat, Jul 28, 2012 at 12:53:11AM +0200, Rafael J. Wysocki wrote: > Unfortunately, your commit > > commit ca5481c68e9fbcea62bb3c78ae6cccf99ca8fb73 > Author: Paul Mundt > 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 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 Reported-by: Kuninori Morimoto Signed-off-by: Paul Mundt 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/