Re: [PATCH] gpio: convince line to become input in irq helper

[Bjorn Andersson <bjorn.andersson@linaro.org>]

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