RE: [PATCH 2/3] pinctrl:Intel: clear interrupt status for every IRQ setup

["Zheng, Qi" <qi.zheng@xxxxxxxxx>]

On Sat, Mar 12, 2016 at 01:06:01AM +0800, Qipeng Zha wrote:
> There is one unexpected GPIO interrupt coming in below scenario.
> 1. GPIO X is going to be used as falling edge interrupt.
> 2. Before request_irq call, this GPIO X interrupt was masked.
> 3. But the IRQ mode may be set for some mode in default (by BIOS).
> 4. Toggle GPIO X from high to low.
> 5. The GPIO X interrupt status will be set even if it was masked.
> 6. Register the interrupt for GPIO X, the interrupt will be unmasked.
> 7. Even if no change on GPIO X afterwards, but one GPIO X interrupt 
> will be triggered because the interrupt status was set.
> 
> To avoid above issue, the interrupt status need clear before request_irq.
> 
> Signed-off-by: Qi Zheng <qi.zheng@xxxxxxxxx>
> Signed-off-by: Qipeng Zha <qipeng.zha@xxxxxxxxx>
> ---
>  drivers/pinctrl/intel/pinctrl-intel.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c 
> b/drivers/pinctrl/intel/pinctrl-intel.c
> index ded5378..d6fe659 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -773,6 +773,8 @@ static int intel_gpio_irq_type(struct irq_data *d, unsigned type)
>  		return -EPERM;
>  	}
>  
> +	intel_gpio_irq_ack(d);

> If the pin toggles right here, we still have the same issue, no?

Yes. But it is very short time from here to the place IRQ got enabled.
To me, it is the only available platform dependent interface to do the "ACK" in the flow of request_irq.
Maybe you have other better option here?

> We should check in the interrupt handler whether the interrupt is actually enabled which I think we do already. Maybe that code has bug somewhere?

The check in the interrupt handler can't distinguish if the interrupt status was set before or after the request_irq.
On the Broxton RVP, one unexpected GPIO interrupt was captured during our GPIO unit test program.
This issue can be fixed by this patch.

> +
>  	spin_lock_irqsave(&pctrl->lock, flags);
>  
>  	value = readl(reg);
> --
> 1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html