diff mbox series

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

Message ID 20240809071049.2454-1-yongxuan.wang@sifive.com (mailing list archive)
State Handled Elsewhere
Headers show
Series [v2,1/1] irqchip/riscv-aplic: Retrigger MSI interrupt on source configuration | expand

Checks

Context Check Description
conchuod/vmtest-for-next-PR success PR summary
conchuod/patch-1-test-1 success .github/scripts/patches/tests/build_rv32_defconfig.sh
conchuod/patch-1-test-2 success .github/scripts/patches/tests/build_rv64_clang_allmodconfig.sh
conchuod/patch-1-test-3 success .github/scripts/patches/tests/build_rv64_gcc_allmodconfig.sh
conchuod/patch-1-test-4 success .github/scripts/patches/tests/build_rv64_nommu_k210_defconfig.sh
conchuod/patch-1-test-5 success .github/scripts/patches/tests/build_rv64_nommu_virt_defconfig.sh
conchuod/patch-1-test-6 success .github/scripts/patches/tests/checkpatch.sh
conchuod/patch-1-test-7 success .github/scripts/patches/tests/dtb_warn_rv64.sh
conchuod/patch-1-test-8 success .github/scripts/patches/tests/header_inline.sh
conchuod/patch-1-test-9 success .github/scripts/patches/tests/kdoc.sh
conchuod/patch-1-test-10 success .github/scripts/patches/tests/module_param.sh
conchuod/patch-1-test-11 success .github/scripts/patches/tests/verify_fixes.sh
conchuod/patch-1-test-12 success .github/scripts/patches/tests/verify_signedoff.sh

Commit Message

Yong-Xuan Wang Aug. 9, 2024, 7:10 a.m. UTC 
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@sifive.com>
Reviewed-by: Vincent Chen <vincent.chen@sifive.com>
---
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(-)

Comments

Anup Patel Aug. 9, 2024, 7:40 a.m. UTC | #1 
On Fri, Aug 9, 2024 at 12:40 PM Yong-Xuan Wang <yongxuan.wang@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@sifive.com>
> Reviewed-by: Vincent Chen <vincent.chen@sifive.com>

LGTM.

Reviewed-by: Anup Patel <anup@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
>
diff mbox series

Patch

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,