[PATCH 0/1] pinctrl: iproc: Fix level interrupt handling

[PATCH 0/1] pinctrl: iproc: Fix level interrupt handling

[Hamish Martin]

An issue is seen on the BCM56060 SoC using the pinctrl-iproc-gpio.c driver
for one of its GPIO blocks.
When a GPIO on this block is used as a level sensitive interrupt the
downstream handler is not invoked until the level returns to its previous
(non-interrupting) state. This makes the level sensitive interrupts
unusable. Edge sensitive interrupts are not affected.

The fix is to simply ensure that we use a more appropriate handler
(handle_level_irq) for the interrupt based on what trigger type has been
configured.
Similar setting of the handler is done in numerous other drivers.

Hamish Martin (1):
  pinctrl: iproc: Set irq handler based on trig type

 drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 5 +++++
 1 file changed, 5 insertions(+)

-- 
2.24.1

[PATCH 1/1] pinctrl: iproc: Set irq handler based on trig type

[Hamish Martin]

Rather than always using handle_simple_irq() as the gpio_irq_chip
handler, set a more appropriate handler based on the IRQ trigger type
requested.
This is important for level triggered interrupts which need to be
masked during handling.

Signed-off-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz>
---
 drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
index 831a9318c384..c79e91eb1a47 100644
--- a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
+++ b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
@@ -288,6 +288,11 @@ static int iproc_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 		       rising_or_high);
 	raw_spin_unlock_irqrestore(&chip->lock, flags);
 
+	if (type & IRQ_TYPE_EDGE_BOTH)
+		irq_set_handler_locked(d, handle_edge_irq);
+	else
+		irq_set_handler_locked(d, handle_level_irq);
+
 	dev_dbg(chip->dev,
 		"gpio:%u level_triggered:%d dual_edge:%d rising_or_high:%d\n",
 		gpio, level_triggered, dual_edge, rising_or_high);
-- 
2.24.1

Re: [PATCH 1/1] pinctrl: iproc: Set irq handler based on trig type

[Ray Jui]

On 2019-12-12 2:09 p.m., Hamish Martin wrote:
> Rather than always using handle_simple_irq() as the gpio_irq_chip
> handler, set a more appropriate handler based on the IRQ trigger type
> requested.
> This is important for level triggered interrupts which need to be
> masked during handling.
> 
> Signed-off-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz>
> ---
>   drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
> index 831a9318c384..c79e91eb1a47 100644
> --- a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
> +++ b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
> @@ -288,6 +288,11 @@ static int iproc_gpio_irq_set_type(struct irq_data *d, unsigned int type)
>   		       rising_or_high);
>   	raw_spin_unlock_irqrestore(&chip->lock, flags);
>   
> +	if (type & IRQ_TYPE_EDGE_BOTH)
> +		irq_set_handler_locked(d, handle_edge_irq);
> +	else
> +		irq_set_handler_locked(d, handle_level_irq);
> +

I think it would make more sense to put them under the above spinlock 
protection, right?

In addition, with this, can the default assignment in 'iproc_gpio_probe' 
be removed?

girq->handler = handle_simple_irq;

Thanks,

Ray

>   	dev_dbg(chip->dev,
>   		"gpio:%u level_triggered:%d dual_edge:%d rising_or_high:%d\n",
>   		gpio, level_triggered, dual_edge, rising_or_high);
>