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: 6540891 Return-Path: X-Original-To: patchwork-linux-omap@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 0244A9F1CC for ; Wed, 3 Jun 2015 19:34:34 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 11AD2206DD for ; Wed, 3 Jun 2015 19:34:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 08D94206F0 for ; Wed, 3 Jun 2015 19:34:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933054AbbFCTeb (ORCPT ); Wed, 3 Jun 2015 15:34:31 -0400 Received: from mail-wg0-f48.google.com ([74.125.82.48]:33094 "EHLO mail-wg0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933060AbbFCTea (ORCPT ); Wed, 3 Jun 2015 15:34:30 -0400 Received: by wgez8 with SMTP id z8so17571161wge.0 for ; Wed, 03 Jun 2015 12:34:29 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:message-id:date:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=9BOu9qczTr0Fu7xbhPmq+wsyEQFFIZdPavCgJ0X1EgE=; b=Rxjd9i48KvdO4prZr+f+7XJixz7VnZ2xdWNnLaZXElcG8JP0dO9i4So9771mDEHHri YQRpQPst/033S4RbtguwsdkvjRjGR2LCvJrjftx9A4Znifc94XbM7fXw2eza5AT7z0oD 70MwnCZAw5qWVwvh6v4ofj8QZ3DOz8O8V/4ynJOyU5I0q4T9XAym682l83qaUwlPjNlP ePEpD3edPkoq2VtiraL/Yh5gQtFAZSNW5jcYDn2nS2PaTKtMfCDRrMmFJbGzxE8IfiNf nrZr1TFqq7GwJkjthONgoKPo1jvdich3VZ3tEUjqxm9xcGj/Lpv4bb4vQVKx8JPuD4W6 xz9A== X-Gm-Message-State: ALoCoQlmxjIHcS0AUv2w0MsTfLbDtbgyH+6w7ujA/KFI4nuoQ7GcirgMz3KWs0luHcGgK7FBukpc 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" X-Google-Original-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-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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))