From patchwork Fri Apr 19 00:49:29 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Hunter, Jon" X-Patchwork-Id: 2462911 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) by patchwork1.kernel.org (Postfix) with ESMTP id DA8A13FD8C for ; Fri, 19 Apr 2013 00:50:06 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1USzWZ-0000iM-Co; Fri, 19 Apr 2013 00:49:59 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1USzWD-0002Zf-Cc; Fri, 19 Apr 2013 00:49:37 +0000 Received: from comal.ext.ti.com ([198.47.26.152]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1USzW8-0002Yn-Tq for linux-arm-kernel@lists.infradead.org; Fri, 19 Apr 2013 00:49:34 +0000 Received: from dflxv15.itg.ti.com ([128.247.5.124]) by comal.ext.ti.com (8.13.7/8.13.7) with ESMTP id r3J0nTCa030072; Thu, 18 Apr 2013 19:49:29 -0500 Received: from DLEE70.ent.ti.com (dlee70.ent.ti.com [157.170.170.113]) by dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id r3J0nTSZ009134; Thu, 18 Apr 2013 19:49:29 -0500 Received: from [172.24.1.11] (172.24.1.11) by DLEE70.ent.ti.com (157.170.170.113) with Microsoft SMTP Server id 14.2.342.3; Thu, 18 Apr 2013 19:49:29 -0500 Message-ID: <51709499.8030208@ti.com> Date: Thu, 18 Apr 2013 19:49:29 -0500 From: Jon Hunter User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130329 Thunderbird/17.0.5 MIME-Version: 1.0 To: Kevin Hilman Subject: Re: [PATCH] gpio/omap: ensure gpio context is initialised References: <1366230707-32681-1-git-send-email-jon-hunter@ti.com> <871ua734mv.fsf@linaro.org> <51707D7C.1050600@ti.com> <5170911C.6010507@ti.com> In-Reply-To: <5170911C.6010507@ti.com> X-Originating-IP: [172.24.1.11] X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130418_204933_345504_7182E1A8 X-CRM114-Status: GOOD ( 30.16 ) X-Spam-Score: -7.6 (-------) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-7.6 points) pts rule name description ---- ---------------------- -------------------------------------------------- -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/, high trust [198.47.26.152 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -0.7 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Tony Lindgren , Linus Walleij , Grant Likely , Santosh Shilimkar , linux-omap , linux-arm X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On 04/18/2013 07:34 PM, Jon Hunter wrote: > > On 04/18/2013 06:10 PM, Jon Hunter wrote: >> On 04/18/2013 04:34 PM, Kevin Hilman wrote: > > ... > >>> Why not just init context right here if bank->loses_context && >>> !bank->context_valid? > > I really like this idea a lot. It can really clean-up the code > and really make it much more readable. Before we were playing > some tricks with when we init'ed the get_context_loss_count() > function pointer. How about the below? > > Tony, care to re-test? > > Cheers > Jon > > From d7a940531d354e6be5e16ee50fa8344041df963a Mon Sep 17 00:00:00 2001 > From: Jon Hunter > Date: Mon, 15 Apr 2013 13:06:54 -0500 > Subject: [PATCH] gpio/omap: ensure gpio context is initialised > > Commit a2797be (gpio/omap: force restore if context loss is not > detectable) broke gpio support for OMAP when booting with device-tree > because a restore of the gpio context being performed without ever > initialising the gpio context. In other words, the context restored was > bad. > > This problem could also occur in the non device-tree case, however, it > is much less likely because when booting without device-tree we can > detect context loss via a platform specific API and so context restore > is performed less often. > > Nevertheless we should ensure that the gpio context is initialised > on the first pm-runtime resume for gpio banks that could lose their > state regardless of whether we are booting with device-tree or not. > > The context loss count was being initialised on the first pm-runtime > suspend following a resume, by populating the get_count_loss_count() > function pointer after the first pm-runtime resume. To make the code > more readable and logical, initialise the context loss count on the > first pm-runtime resume if the context is not yet valid. > > Reported-by: Tony Lindgren > Signed-off-by: Jon Hunter > --- > drivers/gpio/gpio-omap.c | 45 ++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 42 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c > index 0557529..db3c732 100644 > --- a/drivers/gpio/gpio-omap.c > +++ b/drivers/gpio/gpio-omap.c > @@ -70,6 +70,7 @@ struct gpio_bank { > bool is_mpuio; > bool dbck_flag; > bool loses_context; > + bool context_valid; > int stride; > u32 width; > int context_loss_count; > @@ -1129,6 +1130,7 @@ static int omap_gpio_probe(struct platform_device *pdev) > bank->loses_context = true; > } else { > bank->loses_context = pdata->loses_context; > + bank->get_context_loss_count = pdata->get_context_loss_count; Still need to check loses_context for populating get_context_loss_count here. Updated patch below. Jon From d02ef7b7dfcf8e13bf019aedfdecb38ca3c6749f Mon Sep 17 00:00:00 2001 From: Jon Hunter Date: Mon, 15 Apr 2013 13:06:54 -0500 Subject: [PATCH] gpio/omap: ensure gpio context is initialised Commit a2797be (gpio/omap: force restore if context loss is not detectable) broke gpio support for OMAP when booting with device-tree because a restore of the gpio context being performed without ever initialising the gpio context. In other words, the context restored was bad. This problem could also occur in the non device-tree case, however, it is much less likely because when booting without device-tree we can detect context loss via a platform specific API and so context restore is performed less often. Nevertheless we should ensure that the gpio context is initialised on the first pm-runtime resume for gpio banks that could lose their state regardless of whether we are booting with device-tree or not. The context loss count was being initialised on the first pm-runtime suspend following a resume, by populating the get_count_loss_count() function pointer after the first pm-runtime resume. To make the code more readable and logical, initialise the context loss count on the first pm-runtime resume if the context is not yet valid. Reported-by: Tony Lindgren Signed-off-by: Jon Hunter Acked-by: Santosh Shilimkar Reviewed-by: Kevin Hilman Tested-by: Tony Lindgren --- drivers/gpio/gpio-omap.c | 48 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 0557529..c3c3ffe 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -70,6 +70,7 @@ struct gpio_bank { bool is_mpuio; bool dbck_flag; bool loses_context; + bool context_valid; int stride; u32 width; int context_loss_count; @@ -1129,6 +1130,10 @@ static int omap_gpio_probe(struct platform_device *pdev) bank->loses_context = true; } else { bank->loses_context = pdata->loses_context; + + if (bank->loses_context) + bank->get_context_loss_count = + pdata->get_context_loss_count; } @@ -1179,9 +1184,6 @@ static int omap_gpio_probe(struct platform_device *pdev) omap_gpio_chip_init(bank); omap_gpio_show_rev(bank); - if (bank->loses_context) - bank->get_context_loss_count = pdata->get_context_loss_count; - pm_runtime_put(bank->dev); list_add_tail(&bank->node, &omap_gpio_list); @@ -1260,6 +1262,8 @@ update_gpio_context_count: return 0; } +static void omap_gpio_init_context(struct gpio_bank *p); + static int omap_gpio_runtime_resume(struct device *dev) { struct platform_device *pdev = to_platform_device(dev); @@ -1269,6 +1273,20 @@ static int omap_gpio_runtime_resume(struct device *dev) int c; spin_lock_irqsave(&bank->lock, flags); + + /* + * On the first resume during the probe, the context has not + * been initialised and so initialise it now. Also initialise + * the context loss count. + */ + if (bank->loses_context && !bank->context_valid) { + omap_gpio_init_context(bank); + + if (bank->get_context_loss_count) + bank->context_loss_count = + bank->get_context_loss_count(bank->dev); + } + _gpio_dbck_enable(bank); /* @@ -1385,6 +1403,29 @@ void omap2_gpio_resume_after_idle(void) } #if defined(CONFIG_PM_RUNTIME) +static void omap_gpio_init_context(struct gpio_bank *p) +{ + struct omap_gpio_reg_offs *regs = p->regs; + void __iomem *base = p->base; + + p->context.ctrl = __raw_readl(base + regs->ctrl); + p->context.oe = __raw_readl(base + regs->direction); + p->context.wake_en = __raw_readl(base + regs->wkup_en); + p->context.leveldetect0 = __raw_readl(base + regs->leveldetect0); + p->context.leveldetect1 = __raw_readl(base + regs->leveldetect1); + p->context.risingdetect = __raw_readl(base + regs->risingdetect); + p->context.fallingdetect = __raw_readl(base + regs->fallingdetect); + p->context.irqenable1 = __raw_readl(base + regs->irqenable); + p->context.irqenable2 = __raw_readl(base + regs->irqenable2); + + if (regs->set_dataout && p->regs->clr_dataout) + p->context.dataout = __raw_readl(base + regs->set_dataout); + else + p->context.dataout = __raw_readl(base + regs->dataout); + + p->context_valid = true; +} + static void omap_gpio_restore_context(struct gpio_bank *bank) { __raw_writel(bank->context.wake_en, @@ -1422,6 +1463,7 @@ static void omap_gpio_restore_context(struct gpio_bank *bank) #else #define omap_gpio_runtime_suspend NULL #define omap_gpio_runtime_resume NULL +static void omap_gpio_init_context(struct gpio_bank *p) {} #endif static const struct dev_pm_ops gpio_pm_ops = {