| Message ID | 1466606015-16326-1-git-send-email-linus.walleij@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On Wed 22 Jun 07:33 PDT 2016, Linus Walleij wrote: > The generic IRQ helper library just checks if the IRQ line is > set as input before activating it for interrupts. As we > recently started to check things better with .get_dir() it > turns out that it's good to try to convince the line to become > an input before attempting to lock it as IRQ. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> I really like this. > --- > drivers/gpio/gpiolib.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index 58d822d7e8da..b9a6a5c69fbb 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -1010,6 +1010,23 @@ static int gpiochip_irq_reqres(struct irq_data *d) > if (!try_module_get(chip->gpiodev->owner)) > return -ENODEV; > > + /* > + * If it is possible to switch this GPIO to an input > + * this is a good time to do it. > + */ > + if (chip->direction_input) { > + struct gpio_desc *desc; > + int ret; > + > + desc = gpiochip_get_desc(chip, d->hwirq); > + if (IS_ERR(desc)) > + return PTR_ERR(desc); > + > + ret = chip->direction_input(chip, d->hwirq); > + if (!ret) > + clear_bit(FLAG_IS_OUT, &desc->flags); With this updated semantics I think it's a mistake to not propagate this error. Clients will not be able to rely on it and have to continue configuring their pins as input just to be sure. > + } > + > if (gpiochip_lock_as_irq(chip, d->hwirq)) { > chip_err(chip, > "unable to lock HW IRQ %lu for IRQ\n", Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 22, 2016 at 7:12 PM, Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > On Wed 22 Jun 07:33 PDT 2016, Linus Walleij wrote: > >> The generic IRQ helper library just checks if the IRQ line is >> set as input before activating it for interrupts. As we >> recently started to check things better with .get_dir() it >> turns out that it's good to try to convince the line to become >> an input before attempting to lock it as IRQ. >> >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > > I really like this. Thanks :) >> + /* >> + * If it is possible to switch this GPIO to an input >> + * this is a good time to do it. >> + */ >> + if (chip->direction_input) { >> + struct gpio_desc *desc; >> + int ret; >> + >> + desc = gpiochip_get_desc(chip, d->hwirq); >> + if (IS_ERR(desc)) >> + return PTR_ERR(desc); >> + >> + ret = chip->direction_input(chip, d->hwirq); >> + if (!ret) >> + clear_bit(FLAG_IS_OUT, &desc->flags); > > With this updated semantics I think it's a mistake to not propagate this > error. Clients will not be able to rely on it and have to continue > configuring their pins as input just to be sure. OK good point I'll propagate it. But it will only be used by those driver utilizing the generic IRQchip helper. It's especially for the usecase where you take an IRQ off a DT resource without gpiod_to_irq() first. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 58d822d7e8da..b9a6a5c69fbb 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1010,6 +1010,23 @@ static int gpiochip_irq_reqres(struct irq_data *d) if (!try_module_get(chip->gpiodev->owner)) return -ENODEV; + /* + * If it is possible to switch this GPIO to an input + * this is a good time to do it. + */ + if (chip->direction_input) { + struct gpio_desc *desc; + int ret; + + desc = gpiochip_get_desc(chip, d->hwirq); + if (IS_ERR(desc)) + return PTR_ERR(desc); + + ret = chip->direction_input(chip, d->hwirq); + if (!ret) + clear_bit(FLAG_IS_OUT, &desc->flags); + } + if (gpiochip_lock_as_irq(chip, d->hwirq)) { chip_err(chip, "unable to lock HW IRQ %lu for IRQ\n",
The generic IRQ helper library just checks if the IRQ line is set as input before activating it for interrupts. As we recently started to check things better with .get_dir() it turns out that it's good to try to convince the line to become an input before attempting to lock it as IRQ. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/gpio/gpiolib.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) -- 2.4.11 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html