Thread (20 messages) 20 messages, 5 authors, 2015-06-03

Re: [PATCH 2/7] gpio: omap: fix error handling in omap_gpio_irq_type

From: Javier Martinez Canillas <javier@dowhile0.org>
Date: 2015-06-02 09:40:35
Also in: linux-omap

Hello Grygorii,

On Fri, May 22, 2015 at 4:35 PM, Grygorii Strashko
[off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
---
 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.
quoted hunk ↗ jump to hunk
+               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 <javier@dowhile0.org>

Best regards,
Javier
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help