Re: [PATCH 2/9] irqchip/loongson-pch-pic: Improve edge triggered interrupt support

[Marc Zyngier]

On Tue, 06 Jul 2021 04:08:57 +0100,
Huacai Chen <chenhuacai@xxxxxxxxxxx> wrote:
> 
> Edge-triggered mode and level-triggered mode need different handlers,
> and edge-triggered mode need a specific ack operation. So improve it.
>

Is this a fix? How does it work currently?


> Signed-off-by: Chen Zhu <zhuchen@xxxxxxxxxxx>
> Signed-off-by: Huacai Chen <chenhuacai@xxxxxxxxxxx>
> ---
>  drivers/irqchip/irq-loongson-pch-pic.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
> index f790ca6d78aa..a4eb8a2181c7 100644
> --- a/drivers/irqchip/irq-loongson-pch-pic.c
> +++ b/drivers/irqchip/irq-loongson-pch-pic.c
> @@ -92,18 +92,22 @@ static int pch_pic_set_type(struct irq_data *d, unsigned int type)
>  	case IRQ_TYPE_EDGE_RISING:
>  		pch_pic_bitset(priv, PCH_PIC_EDGE, d->hwirq);
>  		pch_pic_bitclr(priv, PCH_PIC_POL, d->hwirq);
> +		irq_set_handler_locked(d, handle_edge_irq);
>  		break;
>  	case IRQ_TYPE_EDGE_FALLING:
>  		pch_pic_bitset(priv, PCH_PIC_EDGE, d->hwirq);
>  		pch_pic_bitset(priv, PCH_PIC_POL, d->hwirq);
> +		irq_set_handler_locked(d, handle_edge_irq);
>  		break;
>  	case IRQ_TYPE_LEVEL_HIGH:
>  		pch_pic_bitclr(priv, PCH_PIC_EDGE, d->hwirq);
>  		pch_pic_bitclr(priv, PCH_PIC_POL, d->hwirq);
> +		irq_set_handler_locked(d, handle_level_irq);
>  		break;
>  	case IRQ_TYPE_LEVEL_LOW:
>  		pch_pic_bitclr(priv, PCH_PIC_EDGE, d->hwirq);
>  		pch_pic_bitset(priv, PCH_PIC_POL, d->hwirq);
> +		irq_set_handler_locked(d, handle_level_irq);

You are changing the flow for the whole hierarchy. Are all the
irqchips in the stack supporting this?

>  		break;
>  	default:
>  		ret = -EINVAL;
> @@ -113,11 +117,24 @@ static int pch_pic_set_type(struct irq_data *d, unsigned int type)
>  	return ret;
>  }
>  
> +static void pch_pic_ack_irq(struct irq_data *d)
> +{
> +	unsigned int reg;
> +	struct pch_pic *priv = irq_data_get_irq_chip_data(d);
> +
> +	reg = readl(priv->base + PCH_PIC_EDGE + PIC_REG_IDX(d->hwirq) * 4);
> +	if (reg & BIT(PIC_REG_BIT(d->hwirq))) {
> +		writel(BIT(PIC_REG_BIT(d->hwirq)),
> +			priv->base + PCH_PIC_CLR + PIC_REG_IDX(d->hwirq) * 4);
> +	}
> +	irq_chip_ack_parent(d);
> +}
> +
>  static struct irq_chip pch_pic_irq_chip = {
>  	.name			= "PCH PIC",
>  	.irq_mask		= pch_pic_mask_irq,
>  	.irq_unmask		= pch_pic_unmask_irq,
> -	.irq_ack		= irq_chip_ack_parent,
> +	.irq_ack		= pch_pic_ack_irq,
>  	.irq_set_affinity	= irq_chip_set_affinity_parent,
>  	.irq_set_type		= pch_pic_set_type,
>  };

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.