Re: [RFCv3 PATCH 3/4] gpiolib: override irq_enable/disable

[Hans Verkuil <hverkuil@xxxxxxxxx>]

On 09/05/2018 12:28 PM, Linus Walleij wrote:
> On Fri, Aug 31, 2018 at 4:37 PM Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
> 
>> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>>
>> When using the gpiolib irqchip helpers install irq_enable/disable
>> hooks for the irqchip to ensure that gpiolib knows when the irq
>> is enabled or disabled, allowing drivers to disable the irq and then
>> use it as an output pin, and later switch the direction to input and
>> re-enable the irq.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> 
> Overall I'm a big fan of this as you already know!
> 
>> +       /*
>> +        * It is possible for a driver to override this, but only if the
>> +        * alternative functions are both implemented.
>> +        */
>> +       if (!irqchip->irq_request_resources &&
>> +           !irqchip->irq_release_resources) {
>> +               irqchip->irq_request_resources = gpiochip_irq_reqres;
>> +               irqchip->irq_release_resources = gpiochip_irq_relres;
>> +       }
>> +
>> +       gpiochip->irq.irq_enable = irqchip->irq_enable;
>> +       gpiochip->irq.irq_disable = irqchip->irq_disable;
>> +
>> +       irqchip->irq_enable = gpiochip_irq_enable;
>> +       irqchip->irq_disable = gpiochip_irq_disable;
> 
> Actully the latter method, to use local copies of the enable/disable
> function pointers in gpiochip->irq is what we should use also
> for the request/release_resources callbacks.
> 
> The chips that actually use local irq_request_resources and
> irq_release resources AND GPIOLIB_IRQCHIP are limited to:
> 
> drivers/gpio/gpio-dwapb.c

Are you sure? I don't see this one selecting GPIOLIB_IRQCHIP.

> drivers/pinctrl/intel/pinctrl-intel.c
> drivers/pinctrl/pinctrl-st.c
> drivers/pinctrl/qcom/pinctrl-msm.c

Ditto for the qcom driver.

The ST and Intel drivers do use it and those two driver needs
updating.

Regards,

	Hans

> 
> As far as I can tell.
> 
> They probably all have some boilerplate related to reimplementing
> part of the core functions.
> 
> But we can fix that in a separate patch after this.
> 
> Yours,
> Linus Walleij
>