[PATCH v2] ACPI: AGDI: Add interrupt signaling mode support

[PATCH v2] ACPI: AGDI: Add interrupt signaling mode support

[Kazuhiro Abe]

AGDI has two types of signaling modes: SDEI and interrupt.
Currently, the AGDI driver only supports SDEI.
Therefore, add support for interrupt singaling mode
The interrupt vector is retrieved from the AGDI table, and call panic
function when an interrupt occurs.

---
I keep normal IRQ code when NMI cannot be used.
If there is any concern, please let me know.

v1->v2
 - Remove acpica update since there is no need to update define value
   for this patch.

Signed-off-by: Kazuhiro Abe <fj1078ii@aa.jp.fujitsu.com>
---
 drivers/acpi/arm64/agdi.c | 98 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 91 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/arm64/agdi.c b/drivers/acpi/arm64/agdi.c
index e0df3daa4abf..e887aab6b448 100644
--- a/drivers/acpi/arm64/agdi.c
+++ b/drivers/acpi/arm64/agdi.c
@@ -16,7 +16,11 @@
 #include "init.h"
 
 struct agdi_data {
+	unsigned char flags;
 	int sdei_event;
+	unsigned int gsiv;
+	bool use_nmi;
+	int irq;
 };
 
 static int agdi_sdei_handler(u32 sdei_event, struct pt_regs *regs, void *arg)
@@ -48,6 +52,55 @@ static int agdi_sdei_probe(struct platform_device *pdev,
 	return 0;
 }
 
+static irqreturn_t agdi_interrupt_handler_nmi(int irq, void *dev_id)
+{
+	nmi_panic(NULL, "Arm Generic Diagnostic Dump and Reset NMI Interrupt event issued\n");
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t agdi_interrupt_handler_irq(int irq, void *dev_id)
+{
+	panic("Arm Generic Diagnostic Dump and Reset Interrupt event issued\n");
+	return IRQ_HANDLED;
+}
+
+static int agdi_interrupt_probe(struct platform_device *pdev,
+				struct agdi_data *adata)
+{
+	unsigned long irq_flags;
+	int ret;
+	int irq;
+
+	irq = acpi_register_gsi(NULL, adata->gsiv, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_HIGH);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "cannot register GSI#%d (%d)\n", adata->gsiv, irq);
+		return irq;
+	}
+
+	irq_flags = IRQF_PERCPU | IRQF_NOBALANCING | IRQF_NO_AUTOEN |
+		    IRQF_NO_THREAD;
+	/* try NMI first */
+	ret = request_nmi(irq, &agdi_interrupt_handler_nmi, irq_flags,
+			  "agdi_interrupt_nmi", NULL);
+	if (ret) {
+		ret = request_irq(irq, &agdi_interrupt_handler_irq,
+				  irq_flags, "agdi_interrupt_irq", NULL);
+		if (ret) {
+			dev_err(&pdev->dev, "cannot register IRQ %d\n", ret);
+			acpi_unregister_gsi(adata->gsiv);
+			return ret;
+		}
+		enable_irq(irq);
+		adata->irq = irq;
+	} else {
+		enable_nmi(irq);
+		adata->irq = irq;
+		adata->use_nmi = true;
+	}
+
+	return 0;
+}
+
 static int agdi_probe(struct platform_device *pdev)
 {
 	struct agdi_data *adata = dev_get_platdata(&pdev->dev);
@@ -55,12 +108,17 @@ static int agdi_probe(struct platform_device *pdev)
 	if (!adata)
 		return -EINVAL;
 
-	return agdi_sdei_probe(pdev, adata);
+	if (adata->flags & ACPI_AGDI_SIGNALING_MODE)
+		agdi_interrupt_probe(pdev, adata);
+	else
+		agdi_sdei_probe(pdev, adata);
+
+	return 0;
 }
 
-static void agdi_remove(struct platform_device *pdev)
+static void agdi_sdei_remove(struct platform_device *pdev,
+			     struct agdi_data *adata)
 {
-	struct agdi_data *adata = dev_get_platdata(&pdev->dev);
 	int err, i;
 
 	err = sdei_event_disable(adata->sdei_event);
@@ -83,6 +141,30 @@ static void agdi_remove(struct platform_device *pdev)
 			adata->sdei_event, ERR_PTR(err));
 }
 
+static void agdi_interrupt_remove(struct platform_device *pdev,
+				  struct agdi_data *adata)
+{
+	if (adata->irq != -1) {
+		if (adata->use_nmi)
+			free_nmi(adata->irq, NULL);
+		else
+			free_irq(adata->irq, NULL);
+
+		acpi_unregister_gsi(adata->gsiv);
+	}
+}
+
+static void agdi_remove(struct platform_device *pdev)
+{
+	struct agdi_data *adata = dev_get_platdata(&pdev->dev);
+
+	if (adata->flags & ACPI_AGDI_SIGNALING_MODE) {
+		agdi_interrupt_remove(pdev, adata);
+	} else {
+		agdi_sdei_remove(pdev, adata);
+	}
+}
+
 static struct platform_driver agdi_driver = {
 	.driver = {
 		.name = "agdi",
@@ -94,7 +176,7 @@ static struct platform_driver agdi_driver = {
 void __init acpi_agdi_init(void)
 {
 	struct acpi_table_agdi *agdi_table;
-	struct agdi_data pdata;
+	struct agdi_data pdata = {0};
 	struct platform_device *pdev;
 	acpi_status status;
 
@@ -104,11 +186,13 @@ void __init acpi_agdi_init(void)
 		return;
 
 	if (agdi_table->flags & ACPI_AGDI_SIGNALING_MODE) {
-		pr_warn("Interrupt signaling is not supported");
-		goto err_put_table;
+		pdata.gsiv = agdi_table->gsiv;
+	} else {
+		pdata.sdei_event = agdi_table->sdei_event;
 	}
 
-	pdata.sdei_event = agdi_table->sdei_event;
+	pdata.irq = -1;
+	pdata.flags = agdi_table->flags;
 
 	pdev = platform_device_register_data(NULL, "agdi", 0, &pdata, sizeof(pdata));
 	if (IS_ERR(pdev))
-- 
2.43.0

Re: [PATCH v2] ACPI: AGDI: Add interrupt signaling mode support

[Ilkka Koskinen]

Hi Kazuhiro,

I have a couple of minor comments below

On Fri, 29 Aug 2025, Kazuhiro Abe wrote:
> AGDI has two types of signaling modes: SDEI and interrupt.
> Currently, the AGDI driver only supports SDEI.
> Therefore, add support for interrupt singaling mode
> The interrupt vector is retrieved from the AGDI table, and call panic
> function when an interrupt occurs.
>
> ---
> I keep normal IRQ code when NMI cannot be used.
> If there is any concern, please let me know.
>
> v1->v2
> - Remove acpica update since there is no need to update define value
>   for this patch.
>
> Signed-off-by: Kazuhiro Abe <fj1078ii@aa.jp.fujitsu.com>
> ---
> drivers/acpi/arm64/agdi.c | 98 ++++++++++++++++++++++++++++++++++++---
> 1 file changed, 91 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/acpi/arm64/agdi.c b/drivers/acpi/arm64/agdi.c
> index e0df3daa4abf..e887aab6b448 100644
> --- a/drivers/acpi/arm64/agdi.c
> +++ b/drivers/acpi/arm64/agdi.c

...

> static int agdi_probe(struct platform_device *pdev)
> {
> 	struct agdi_data *adata = dev_get_platdata(&pdev->dev);
> @@ -55,12 +108,17 @@ static int agdi_probe(struct platform_device *pdev)
> 	if (!adata)
> 		return -EINVAL;
>
> -	return agdi_sdei_probe(pdev, adata);
> +	if (adata->flags & ACPI_AGDI_SIGNALING_MODE)
> +		agdi_interrupt_probe(pdev, adata);
> +	else
> +		agdi_sdei_probe(pdev, adata);
> +
> +	return 0;

Is there a reason why you always return zero instead of a possible
error code from either of the probe functions?

> }
>

...

> +static void agdi_remove(struct platform_device *pdev)
> +{
> +	struct agdi_data *adata = dev_get_platdata(&pdev->dev);
> +
> +	if (adata->flags & ACPI_AGDI_SIGNALING_MODE) {
> +		agdi_interrupt_remove(pdev, adata);
> +	} else {
> +		agdi_sdei_remove(pdev, adata);
> +	}

You don't need curly braces in this if/else block.

Cheers, Ilkka

RE: [PATCH v2] ACPI: AGDI: Add interrupt signaling mode support

[Kazuhiro Abe (Fujitsu)]

Hi Ilkka
Thank you for your comment.

> Hi Kazuhiro,
> 
> I have a couple of minor comments below
> 
> On Fri, 29 Aug 2025, Kazuhiro Abe wrote:
> > AGDI has two types of signaling modes: SDEI and interrupt.
> > Currently, the AGDI driver only supports SDEI.
> > Therefore, add support for interrupt singaling mode The interrupt
> > vector is retrieved from the AGDI table, and call panic function when
> > an interrupt occurs.
> >
> > ---
> > I keep normal IRQ code when NMI cannot be used.
> > If there is any concern, please let me know.
> >
> > v1->v2
> > - Remove acpica update since there is no need to update define value
> >   for this patch.
> >
> > Signed-off-by: Kazuhiro Abe <fj1078ii@aa.jp.fujitsu.com>
> > ---
> > drivers/acpi/arm64/agdi.c | 98
> ++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 91 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/acpi/arm64/agdi.c b/drivers/acpi/arm64/agdi.c
> > index e0df3daa4abf..e887aab6b448 100644
> > --- a/drivers/acpi/arm64/agdi.c
> > +++ b/drivers/acpi/arm64/agdi.c
> 
> ...
> 
> > static int agdi_probe(struct platform_device *pdev) {
> > 	struct agdi_data *adata = dev_get_platdata(&pdev->dev); @@ -55,12
> > +108,17 @@ static int agdi_probe(struct platform_device *pdev)
> > 	if (!adata)
> > 		return -EINVAL;
> >
> > -	return agdi_sdei_probe(pdev, adata);
> > +	if (adata->flags & ACPI_AGDI_SIGNALING_MODE)
> > +		agdi_interrupt_probe(pdev, adata);
> > +	else
> > +		agdi_sdei_probe(pdev, adata);
> > +
> > +	return 0;
> 
> Is there a reason why you always return zero instead of a possible error code from
> either of the probe functions?

There was no particular reason; it was a bug.
I will fix it.

> 
> > }
> >
> 
> ...
> 
> > +static void agdi_remove(struct platform_device *pdev) {
> > +	struct agdi_data *adata = dev_get_platdata(&pdev->dev);
> > +
> > +	if (adata->flags & ACPI_AGDI_SIGNALING_MODE) {
> > +		agdi_interrupt_remove(pdev, adata);
> > +	} else {
> > +		agdi_sdei_remove(pdev, adata);
> > +	}
> 
> You don't need curly braces in this if/else block.

Understood, I will do that.

Best Regards,
Kazuhiro Abe

> 
> Cheers, Ilkka