Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 06 Jul 2021 14:06:01 +0100
Message-ID: <878s2j8udi.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Huacai Chen <chenhuacai@loongson.cn>
Cc:     Thomas Gleixner <tglx@linutronix.de>, linux-kernel@vger.kernel.org,
        Xuefeng Li <lixuefeng@loongson.cn>,
        Huacai Chen <chenhuacai@gmail.com>,
        Jiaxun Yang <jiaxun.yang@flygoat.com>,
        Chen Zhu <zhuchen@loongson.cn>
Subject: Re: [PATCH 2/9] irqchip/loongson-pch-pic: Improve edge triggered interrupt support
In-Reply-To: <20210706030904.1411775-3-chenhuacai@loongson.cn>
References: <20210706030904.1411775-1-chenhuacai@loongson.cn>
        <20210706030904.1411775-3-chenhuacai@loongson.cn>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Tue, 06 Jul 2021 04:08:57 +0100,
Huacai Chen <chenhuacai@loongson.cn> 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@loongson.cn>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
>  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.