[PATCH v2 1/1] irqchip/riscv-aplic: Retrigger MSI interrupt on source configuration

[Anup Patel]

On Fri, Aug 9, 2024 at 12:40 PM Yong-Xuan Wang <yongxuan.wang at sifive.com> wrote:
>
> The section 4.5.2 of the RISC-V AIA specification says that "any write
> to a sourcecfg register of an APLIC might (or might not) cause the
> corresponding interrupt-pending bit to be set to one if the rectified
> input value is high (= 1) under the new source mode."
>
> When the interrupt type is changed in sourcecfg register, the APLIC
> device might not set the corresponding pending bit, so the interrupt
> might never become pending.
>
> To handle sourcecfg register changes for level-triggered interrupts in
> MSI mode, manually set the pending bit for retriggering interrupt if it
> was already asserted.
>
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang at sifive.com>
> Reviewed-by: Vincent Chen <vincent.chen at sifive.com>

LGTM.

Reviewed-by: Anup Patel <anup at brainfault.org>

Regards,
Anup

> ---
> v2:
> - update commit message (Anup, Thomas)
> - rename aplic_retrigger_asserting_irq() to aplic_msi_irq_retrigger_level()
>   and make it as a static function since only APLIC MSI mode require it.
>   (Anup, Thomas)
>
> ---
>  drivers/irqchip/irq-riscv-aplic-msi.c | 35 +++++++++++++++++++++------
>  1 file changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c
> index 028444af48bd..9d63dc37dea5 100644
> --- a/drivers/irqchip/irq-riscv-aplic-msi.c
> +++ b/drivers/irqchip/irq-riscv-aplic-msi.c
> @@ -32,15 +32,10 @@ static void aplic_msi_irq_unmask(struct irq_data *d)
>         aplic_irq_unmask(d);
>  }
>
> -static void aplic_msi_irq_eoi(struct irq_data *d)
> +static void aplic_msi_irq_retrigger_level(struct irq_data *d)
>  {
>         struct aplic_priv *priv = irq_data_get_irq_chip_data(d);
>
> -       /*
> -        * EOI handling is required only for level-triggered interrupts
> -        * when APLIC is in MSI mode.
> -        */
> -
>         switch (irqd_get_trigger_type(d)) {
>         case IRQ_TYPE_LEVEL_LOW:
>         case IRQ_TYPE_LEVEL_HIGH:
> @@ -59,6 +54,32 @@ static void aplic_msi_irq_eoi(struct irq_data *d)
>         }
>  }
>
> +static void aplic_msi_irq_eoi(struct irq_data *d)
> +{
> +       /*
> +        * EOI handling is required only for level-triggered interrupts
> +        * when APLIC is in MSI mode.
> +        */
> +
> +       aplic_msi_irq_retrigger_level(d);
> +}
> +
> +static int aplic_msi_irq_set_type(struct irq_data *d, unsigned int type)
> +{
> +       int rc;
> +
> +       rc = aplic_irq_set_type(d, type);
> +       if (rc)
> +               return rc;
> +
> +       /*
> +        * Updating sourcecfg register for level-triggered interrupts
> +        * requires interrupt retriggering when APLIC is in MSI mode.
> +        */
> +       aplic_msi_irq_retrigger_level(d);
> +       return 0;
> +}
> +
>  static void aplic_msi_write_msg(struct irq_data *d, struct msi_msg *msg)
>  {
>         unsigned int group_index, hart_index, guest_index, val;
> @@ -130,7 +151,7 @@ static const struct msi_domain_template aplic_msi_template = {
>                 .name                   = "APLIC-MSI",
>                 .irq_mask               = aplic_msi_irq_mask,
>                 .irq_unmask             = aplic_msi_irq_unmask,
> -               .irq_set_type           = aplic_irq_set_type,
> +               .irq_set_type           = aplic_msi_irq_set_type,
>                 .irq_eoi                = aplic_msi_irq_eoi,
>  #ifdef CONFIG_SMP
>                 .irq_set_affinity       = irq_chip_set_affinity_parent,
> --
> 2.17.1
>