[PATCH 1/2] irqchip/sifive-plic: Fix duplicate mask/unmask for claim/complete

[Anup Patel]

On Mon, Oct 11, 2021 at 6:54 PM <guoren at kernel.org> wrote:
>
> From: Guo Ren <guoren at linux.alibaba.com>
>
> PLIC only has enable-registers not mask/unmask registers. Mixing
> mask/unmask with irq_eoi is wrong, because readl(claim) could mask

This is an incorrect assumption about readl(claim). When SW does
read(claim) the HW updates internal state that IRQ has been claimed.
The HW can still get same (already claimed) IRQ again before
writel(claim) which will be delivered after writel(claim).

> irq by hardware. We only need mask/unmask to fixup the hardware
> which couldn't claim + mask correctly.

The handle_fasteoi_irq() only calls unmask_irq() mostly when the
underlying IRQ is threaded. Is there any other case ?

Another fact is that all irqchip drivers using handle_fasteoi_irq()
implement irq_mask/unmask().

Regards,
Anup

>
> If hardware supports claim + mask, it would cause unnecessary
> mask/umak operations.
>
> Signed-off-by: Guo Ren <guoren at linux.alibaba.com>
> Cc: Thomas Gleixner <tglx at linutronix.de>
> Cc: Marc Zyngier <maz at kernel.org>
> Cc: Palmer Dabbelt <palmer at dabbelt.com>
> Cc: Anup Patel <anup at brainfault.org>
> Cc: Atish Patra <atish.patra at wdc.com>
> ---
>  drivers/irqchip/irq-sifive-plic.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index cf74cfa82045..0fa46912f452 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -64,6 +64,7 @@ struct plic_priv {
>         struct cpumask lmask;
>         struct irq_domain *irqdomain;
>         void __iomem *regs;
> +       bool claim_mask_support;
>  };
>
>  struct plic_handler {
> @@ -111,7 +112,7 @@ static inline void plic_irq_toggle(const struct cpumask *mask,
>         }
>  }
>
> -static void plic_irq_unmask(struct irq_data *d)
> +static void plic_irq_enable(struct irq_data *d)
>  {
>         struct cpumask amask;
>         unsigned int cpu;
> @@ -125,7 +126,7 @@ static void plic_irq_unmask(struct irq_data *d)
>         plic_irq_toggle(cpumask_of(cpu), d, 1);
>  }
>
> -static void plic_irq_mask(struct irq_data *d)
> +static void plic_irq_disable(struct irq_data *d)
>  {
>         struct plic_priv *priv = irq_data_get_irq_chip_data(d);
>
> @@ -168,8 +169,8 @@ static void plic_irq_eoi(struct irq_data *d)
>
>  static struct irq_chip plic_chip = {
>         .name           = "SiFive PLIC",
> -       .irq_mask       = plic_irq_mask,
> -       .irq_unmask     = plic_irq_unmask,
> +       .irq_enable     = plic_irq_enable,
> +       .irq_disable    = plic_irq_disable,
>         .irq_eoi        = plic_irq_eoi,
>  #ifdef CONFIG_SMP
>         .irq_set_affinity = plic_set_affinity,
> @@ -181,6 +182,11 @@ static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq,
>  {
>         struct plic_priv *priv = d->host_data;
>
> +       if (!priv->claim_mask_support) {
> +               plic_chip.irq_mask      = plic_irq_disable;
> +               plic_chip.irq_unmask    = plic_irq_enable;
> +       }
> +
>         irq_domain_set_info(d, irq, hwirq, &plic_chip, d->host_data,
>                             handle_fasteoi_irq, NULL, NULL);
>         irq_set_noprobe(irq);
> @@ -298,6 +304,8 @@ static int __init plic_init(struct device_node *node,
>         if (WARN_ON(!nr_contexts))
>                 goto out_iounmap;
>
> +       priv->claim_mask_support = of_property_read_bool(node, "claim-mask-support");
> +
>         error = -ENOMEM;
>         priv->irqdomain = irq_domain_add_linear(node, nr_irqs + 1,
>                         &plic_irqdomain_ops, priv);
> --
> 2.25.1
>