From patchwork Wed Jun 3 19:34:23 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Grygorii.Strashko@linaro.org" X-Patchwork-Id: 49496 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-wg0-f70.google.com (mail-wg0-f70.google.com [74.125.82.70]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 5CE1024612 for ; Wed, 3 Jun 2015 19:34:35 +0000 (UTC) Received: by wgv5 with SMTP id 5sf4934216wgv.0 for ; Wed, 03 Jun 2015 12:34:34 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:from:message-id:date:user-agent :mime-version:to:cc:subject:references:in-reply-to:content-type :content-transfer-encoding:sender:precedence:list-id :x-original-sender:x-original-authentication-results:mailing-list :list-post:list-help:list-archive:list-unsubscribe; bh=XZ93P1hO7d27p1qTwSyDNH626Bg4ommmwxr4LONO/J0=; b=KhTC5WsVMsQGRpx1mT6XG0j2vxojHUKgRKJOfMfPM91A0OQUnRd8o/KHuBJ4xwRtGf qRrknyfC6GhGEWmrAc/ecztoB4m+mkU/RGXvdufoSOAsEn3Lq0pW0nbwuheYbW65tbje rFrH+5h5qCSDyryD07YbnLdOvp2HymhKpHpzLBxWH1yQ/ZjSOgindC5qrNIT8LQMhtMz 4XfGBaAhh30nGjAflPrB/Lxo4IVrxY8FLAocWsrglTGzj859+37uIc0vhS1VWhY03Cg8 3VJ+3QPBKM/y7Pmvq7NdRd0Mz+iVN8jv1Zbi7UsOmRTJ5NVneg3aIXSYivN8FEs0J0Z8 Xt0g== X-Gm-Message-State: ALoCoQn1dQF8HkbLstQ1UlDSjEIRi3jUkH7GPLf5+h9KnZsifxP35XMW5qGeDgRL1F9fvFtKu5UU X-Received: by 10.112.219.200 with SMTP id pq8mr32084854lbc.7.1433360074581; Wed, 03 Jun 2015 12:34:34 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.246.35 with SMTP id xt3ls100426lac.37.gmail; Wed, 03 Jun 2015 12:34:34 -0700 (PDT) X-Received: by 10.112.56.42 with SMTP id x10mr33809012lbp.123.1433360074435; Wed, 03 Jun 2015 12:34:34 -0700 (PDT) Received: from mail-lb0-f175.google.com (mail-lb0-f175.google.com. [209.85.217.175]) by mx.google.com with ESMTPS id th3si18794844lbb.146.2015.06.03.12.34.34 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 03 Jun 2015 12:34:34 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.175 as permitted sender) client-ip=209.85.217.175; Received: by lbbqq2 with SMTP id qq2so13835378lbb.3 for ; Wed, 03 Jun 2015 12:34:34 -0700 (PDT) X-Received: by 10.112.199.133 with SMTP id jk5mr34467257lbc.32.1433360074327; Wed, 03 Jun 2015 12:34:34 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.108.230 with SMTP id hn6csp49699lbb; Wed, 3 Jun 2015 12:34:33 -0700 (PDT) X-Received: by 10.70.51.67 with SMTP id i3mr62418015pdo.145.1433360072413; Wed, 03 Jun 2015 12:34:32 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id vr2si2338280pab.15.2015.06.03.12.34.31; Wed, 03 Jun 2015 12:34:32 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-gpio-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933061AbbFCTeb (ORCPT + 1 other); Wed, 3 Jun 2015 15:34:31 -0400 Received: from mail-wi0-f180.google.com ([209.85.212.180]:35486 "EHLO mail-wi0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933054AbbFCTea (ORCPT ); Wed, 3 Jun 2015 15:34:30 -0400 Received: by wiga1 with SMTP id a1so25541101wig.0 for ; Wed, 03 Jun 2015 12:34:29 -0700 (PDT) X-Received: by 10.180.96.196 with SMTP id du4mr44297071wib.77.1433360068917; Wed, 03 Jun 2015 12:34:28 -0700 (PDT) Received: from [172.22.39.17] ([195.238.92.128]) by mx.google.com with ESMTPSA id w11sm2422133wjr.48.2015.06.03.12.34.27 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 03 Jun 2015 12:34:28 -0700 (PDT) From: "Grygorii.Strashko@linaro.org" Message-ID: <556F56BF.8090803@linaro.org> Date: Wed, 03 Jun 2015 22:34:23 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Javier Martinez Canillas , Linus Walleij CC: "Grygorii.Strashko@linaro.org" , Alexandre Courbot , Tony Lindgren , Santosh Shilimkar , Kevin Hilman , "linux-omap@vger.kernel.org" , Linux GPIO List Subject: Re: [PATCH 2/7] gpio: omap: fix error handling in omap_gpio_irq_type References: <1432305354-5968-1-git-send-email-grygorii.strashko@linaro.org> <1432305354-5968-3-git-send-email-grygorii.strashko@linaro.org> <556DBD6D.8090306@linaro.org> In-Reply-To: <556DBD6D.8090306@linaro.org> Sender: linux-gpio-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-gpio@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: grygorii.strashko@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.175 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , Hi Linus, On 06/02/2015 05:27 PM, Grygorii.Strashko@linaro.org wrote: > On 06/02/2015 12:40 PM, Javier Martinez Canillas wrote: >> Hello Grygorii, >> >> On Fri, May 22, 2015 at 4:35 PM, Grygorii Strashko >> wrote: >>> The GPIO bank will be kept powered in case if input parameters >>> are invalid or error occurred in omap_gpio_irq_type. >>> >>> Hence, fix it by ensuring that GPIO bank will be unpowered >>> in case of errors and add additional check of value returned >>> from omap_set_gpio_triggering(). >>> >>> Signed-off-by: Grygorii Strashko >>> --- >>> drivers/gpio/gpio-omap.c | 16 ++++++++++++---- >>> 1 file changed, 12 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c >>> index bb60cbc..f6cc638 100644 >>> --- a/drivers/gpio/gpio-omap.c >>> +++ b/drivers/gpio/gpio-omap.c >>> @@ -488,9 +488,6 @@ static int omap_gpio_irq_type(struct irq_data *d, >>> unsigned type) >>> unsigned long flags; >>> unsigned offset = d->hwirq; >>> >>> - if (!BANK_USED(bank)) >>> - pm_runtime_get_sync(bank->dev); >>> - >>> if (type & ~IRQ_TYPE_SENSE_MASK) >>> return -EINVAL; >>> >>> @@ -498,12 +495,18 @@ static int omap_gpio_irq_type(struct irq_data >>> *d, unsigned type) >>> (type & (IRQ_TYPE_LEVEL_LOW|IRQ_TYPE_LEVEL_HIGH))) >>> return -EINVAL; >>> >>> + if (!BANK_USED(bank)) >>> + pm_runtime_get_sync(bank->dev); >>> + >>> spin_lock_irqsave(&bank->lock, flags); >>> retval = omap_set_gpio_triggering(bank, offset, type); >>> + if (retval) >> >> At this point the bank->lock spinlock is held so you need to add a >> spin_unlock_irqrestore() in the error path. > > Ops. Thanks for catching it. >> >>> + goto error; >>> omap_gpio_init_irq(bank, offset); >>> if (!omap_gpio_is_input(bank, offset)) { >>> spin_unlock_irqrestore(&bank->lock, flags); >>> - return -EINVAL; >>> + retval = -EINVAL; >>> + goto error; >>> } >>> spin_unlock_irqrestore(&bank->lock, flags); >>> >>> @@ -512,6 +515,11 @@ static int omap_gpio_irq_type(struct irq_data >>> *d, unsigned type) >>> else if (type & (IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING)) >>> __irq_set_handler_locked(d->irq, handle_edge_irq); >>> >>> + return 0; >>> + >>> +error: >>> + if (!BANK_USED(bank)) >>> + pm_runtime_put(bank->dev); >>> return retval; >>> } >>> >>> -- >> >> But you are correct about the runtime PM bug so after addressing the >> above comment, feel free to add: >> >> Acked-by: Javier Martinez Canillas >> > > Linus, How do prefer me to fix it? > Resend whole patch or send additional fix on top of patch 5. > Below is the additional patch to fix issue reported by Javier. Pls. inform me if you would like me to send v2 of this patch instead. diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index e26dc40..e3f8205 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -490,8 +490,10 @@ static int omap_gpio_irq_type(struct irq_data *d, unsigned type) spin_lock_irqsave(&bank->lock, flags); retval = omap_set_gpio_triggering(bank, offset, type); - if (retval) + if (retval) { + spin_unlock_irqrestore(&bank->lock, flags); goto error; + } spin_unlock_irqrestore(&bank->lock, flags); if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_LEVEL_HIGH))