Re: [PATCH] gpio/omap: implement irq_enable/disable using mask/unmask.

[Santosh Shilimkar <santosh.shilimkar@ti.com>]

On Friday 12 April 2013 02:43 PM, Andreas Fenkart wrote:
> In PM suspend, some omaps can't detect sdio irqs via the sdio interface.
> The way to implement this, is to declare the corresponding pin as part
> of the sdio interface and as a gpio input via pinctrl-single states.
> The MMC driver request states "default" "active" and "idle" during the
> probe, then toggles between active and idle during the runtime. This
> requires low overhead functions for enable/disable of gpio irqs.
> 
> For level triggered interrupt there is no difference between masking
> and disabling an interrupt. For edge interrupt interrupts there is.
> When masked, interrupts should still be latched to the interrupt status
> register so when unmasked later there is an interrupt straight away.
> However, if the interrupt is disabled then gpio events occurring will not
> be latched/stored. Hence proposed patch is incomplete for edge type
> interrupts.
> 
> Signed-off-by: Andreas Fenkart <andreas.fenkart@streamunlimited.com>
> ---
Patch is incomplete and still confusing ;-) if some one reads the
patch without the thread. I think you have already ask the question/
suggestion in past but its better to split masking/disabling functions
and make them behave properly. Mapping enable/disable to mask/unmask
to get around the issue seems more of a hack.

Also I think, we can address the edge type IRQ issue along with this
patch to be complete.

Before you go ahead with the update, I would like to hear Jon's
opinion on the edge IRQ related fixing.

>  drivers/gpio/gpio-omap.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 159f5c5..69ef2d8 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -815,6 +815,8 @@ static struct irq_chip gpio_irq_chip = {
>  	.irq_unmask	= gpio_unmask_irq,
>  	.irq_set_type	= gpio_irq_type,
>  	.irq_set_wake	= gpio_wake_enable,
> +	.irq_disable    = gpio_mask_irq, /* FIXME for edge type IRQ */
> +	.irq_enable     = gpio_unmask_irq,
>  };
>  
>  /*---------------------------------------------------------------------*/
> 
Sorry to make you wait for the proposal discussion.

Regards,
Santosh