[1/2] gpio: don't override irq_*_resources() callbacks

If the driver has specified its own irq_{request/release}_resources()
functions, don't override them.  The gpio-etraxfs driver will use this.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 drivers/gpio/gpiolib.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

On 07/31/2015 03:48 PM, Rabin Vincent wrote:
> If the driver has specified its own irq_{request/release}_resources()
> functions, don't override them.  The gpio-etraxfs driver will use this.
> 
> Signed-off-by: Rabin Vincent <rabin@rab.in>
> ---
>   drivers/gpio/gpiolib.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index bf4bd1d..6865874 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -636,8 +636,12 @@ int gpiochip_irqchip_add(struct gpio_chip *gpiochip,
>   		gpiochip->irqchip = NULL;
>   		return -EINVAL;
>   	}
> -	irqchip->irq_request_resources = gpiochip_irq_reqres;
> -	irqchip->irq_release_resources = gpiochip_irq_relres;
> +
> +	if (!irqchip->irq_request_resources &&
> +	    !irqchip->irq_release_resources) {
> +		irqchip->irq_request_resources = gpiochip_irq_reqres;
> +		irqchip->irq_release_resources = gpiochip_irq_relres;
> +	}

I think, it will be better to handle req/rel cases separately.

>   
>   	/*
>   	 * Prepare the mapping since the irqchip shall be orthogonal to
> 

Nice, thanks. I need the same actually, but I propose to make
gpiochip_irq_reqres/gpiochip_irq_relres public also, so drivers can re-use them.

On Fri, Jul 31, 2015 at 2:48 PM, Rabin Vincent <rabin@rab.in> wrote:

> If the driver has specified its own irq_{request/release}_resources()
> functions, don't override them.  The gpio-etraxfs driver will use this.
>
> Signed-off-by: Rabin Vincent <rabin@rab.in>

Perfectly reasonable given the usecase in patch 2/2. Patch applied.

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

On Fri, Jul 31, 2015 at 4:54 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
> On 07/31/2015 03:48 PM, Rabin Vincent wrote:

>> +     if (!irqchip->irq_request_resources &&
>> +         !irqchip->irq_release_resources) {
>> +             irqchip->irq_request_resources = gpiochip_irq_reqres;
>> +             irqchip->irq_release_resources = gpiochip_irq_relres;
>> +     }
>
> I think, it will be better to handle req/rel cases separately.

No, I think that could be dangerous. The semantics of the both
functions are intertwined, if we change something in the core
we may break drivers.

It would be better with a mechanism saying "also do this
on irq_request/release resource" so a secondary vtable
for these two. Where the latter would be optional per-callback.

That way the ETRAXFS does not need to reimplement
irq locking.

I'll see what I can come up with.

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

On Mon, Aug 3, 2015 at 10:53 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Jul 31, 2015 at 2:48 PM, Rabin Vincent <rabin@rab.in> wrote:
>
>> If the driver has specified its own irq_{request/release}_resources()
>> functions, don't override them.  The gpio-etraxfs driver will use this.
>>
>> Signed-off-by: Rabin Vincent <rabin@rab.in>
>
> Perfectly reasonable given the usecase in patch 2/2. Patch applied.

So for drivers currently using GPIOLIB_IRQCHIP the calbacks were
always overridden, even if they supplied their own?

Hence after this change, that's no longer the case, and pinctrl-at91.c
will use its own, which are identical to the generic ones, modulo the bug fix
in 5b76e79c77264899 ("gpiolib: irqchip: prevent driver unloading if gpio is
used as irq only"). Oops...

I already wrote an untested patch to convert pinctrl-at91 to the generic
ones, shall I send that right away?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
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 Mon, Aug 17, 2015 at 10:40 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Mon, Aug 3, 2015 at 10:53 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Fri, Jul 31, 2015 at 2:48 PM, Rabin Vincent <rabin@rab.in> wrote:
>>
>>> If the driver has specified its own irq_{request/release}_resources()
>>> functions, don't override them.  The gpio-etraxfs driver will use this.
>>>
>>> Signed-off-by: Rabin Vincent <rabin@rab.in>
>>
>> Perfectly reasonable given the usecase in patch 2/2. Patch applied.
>
> So for drivers currently using GPIOLIB_IRQCHIP the calbacks were
> always overridden, even if they supplied their own?

Yeah, I guess we just assumed noone supplied their own and
expected them to be nulled out.

> Hence after this change, that's no longer the case, and pinctrl-at91.c
> will use its own, which are identical to the generic ones, modulo the bug fix
> in 5b76e79c77264899 ("gpiolib: irqchip: prevent driver unloading if gpio is
> used as irq only"). Oops...
>
> I already wrote an untested patch to convert pinctrl-at91 to the generic
> ones, shall I send that right away?

Please test it first, but yeah :)

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