1. Patchew
  2. linux
  3. View series

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

Kazuhiro Abe posted 1 patch 1 month, 4 weeks ago 
drivers/acpi/arm64/agdi.c | 101 ++++++++++++++++++++++++++++++++++----
1 file changed, 92 insertions(+), 9 deletions(-)
[PATCH v4] ACPI: AGDI: Add interrupt signaling mode support
Posted by Kazuhiro Abe 1 month, 4 weeks ago 
AGDI has two types of signaling modes: SDEI and interrupt.
Currently, the AGDI driver only supports SDEI.
Therefore, add support for interrupt signaling mode
The interrupt vector is retrieved from the AGDI table, and call panic
function when an interrupt occurs.

Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Signed-off-by: Kazuhiro Abe <fj1078ii@aa.jp.fujitsu.com>
---
Hanjun, I have addressed all your comments.
Please review them.

v3->v4
 - Add a comment to the flags member.
 - Fix agdi_interrupt_probe.
 - Fix agdi_interrupt_remove.
 - Add space in struct initializsation.
 - Delete curly braces.

v3: https://lore.kernel.org/all/20250905042751.945616-1-fj1078ii@aa.jp.fujitsu.com/
v2->v3
 - Fix bug in the return value of agdi_probe function.
 - Remove unnecessary curly braces in the agdi_remove function.

v2: https://lore.kernel.org/all/20250829101154.2377800-1-fj1078ii@aa.jp.fujitsu.com/
v1->v2
 - Remove acpica update since there is no need to update define value
   for this patch.
---
 drivers/acpi/arm64/agdi.c | 101 ++++++++++++++++++++++++++++++++++----
 1 file changed, 92 insertions(+), 9 deletions(-)

diff --git a/drivers/acpi/arm64/agdi.c b/drivers/acpi/arm64/agdi.c
index e0df3daa4abf..feb4b2cb4618 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; /* AGDI Signaling Mode */
 	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,57 @@ 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) {
+		enable_nmi(irq);
+		adata->irq = irq;
+		adata->use_nmi = true;
+		return 0;
+	}
+
+	/* Then try normal interrupt */
+	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;
+
+	return 0;
+}
+
 static int agdi_probe(struct platform_device *pdev)
 {
 	struct agdi_data *adata = dev_get_platdata(&pdev->dev);
@@ -55,12 +110,15 @@ 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)
+		return agdi_interrupt_probe(pdev, adata);
+	else
+		return agdi_sdei_probe(pdev, adata);
 }
 
-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)
+		return;
+
+	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;
 
@@ -103,12 +185,13 @@ void __init acpi_agdi_init(void)
 	if (ACPI_FAILURE(status))
 		return;
 
-	if (agdi_table->flags & ACPI_AGDI_SIGNALING_MODE) {
-		pr_warn("Interrupt signaling is not supported");
-		goto err_put_table;
-	}
+	if (agdi_table->flags & ACPI_AGDI_SIGNALING_MODE)
+		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 v4] ACPI: AGDI: Add interrupt signaling mode support
Posted by Hanjun Guo 1 month, 3 weeks ago 
On 2025/10/17 15:39, 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 signaling mode
> The interrupt vector is retrieved from the AGDI table, and call panic
> function when an interrupt occurs.
> 
> Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
> Signed-off-by: Kazuhiro Abe <fj1078ii@aa.jp.fujitsu.com>
> ---
> Hanjun, I have addressed all your comments.
> Please review them.
> 
> v3->v4
>   - Add a comment to the flags member.
>   - Fix agdi_interrupt_probe.
>   - Fix agdi_interrupt_remove.
>   - Add space in struct initializsation.
>   - Delete curly braces.

Looks good to me,

Acked-by: Hanjun Guo <guohanjun@huawei.com>

Thanks
Hanjun
Re: [PATCH v4] ACPI: AGDI: Add interrupt signaling mode support
Posted by Will Deacon 1 month, 1 week ago 
On Mon, Oct 20, 2025 at 09:23:05PM +0800, Hanjun Guo wrote:
> On 2025/10/17 15:39, 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 signaling mode
> > The interrupt vector is retrieved from the AGDI table, and call panic
> > function when an interrupt occurs.
> > 
> > Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
> > Signed-off-by: Kazuhiro Abe <fj1078ii@aa.jp.fujitsu.com>
> > ---
> > Hanjun, I have addressed all your comments.
> > Please review them.
> > 
> > v3->v4
> >   - Add a comment to the flags member.
> >   - Fix agdi_interrupt_probe.
> >   - Fix agdi_interrupt_remove.
> >   - Add space in struct initializsation.
> >   - Delete curly braces.
> 
> Looks good to me,
> 
> Acked-by: Hanjun Guo <guohanjun@huawei.com>

I wasn't cc'd on the original patch but I couldn't figure out why it
uses IRQF_NO_AUTOEN when requesting the irq given that the first thing
it does is enable it.

Will
Re: [PATCH v4] ACPI: AGDI: Add interrupt signaling mode support
Posted by Hanjun Guo 3 weeks, 5 days ago 
On 2025/11/5 0:06, Will Deacon wrote:
> On Mon, Oct 20, 2025 at 09:23:05PM +0800, Hanjun Guo wrote:
>> On 2025/10/17 15:39, 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 signaling mode
>>> The interrupt vector is retrieved from the AGDI table, and call panic
>>> function when an interrupt occurs.
>>>
>>> Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
>>> Signed-off-by: Kazuhiro Abe <fj1078ii@aa.jp.fujitsu.com>
>>> ---
>>> Hanjun, I have addressed all your comments.
>>> Please review them.
>>>
>>> v3->v4
>>>    - Add a comment to the flags member.
>>>    - Fix agdi_interrupt_probe.
>>>    - Fix agdi_interrupt_remove.
>>>    - Add space in struct initializsation.
>>>    - Delete curly braces.
>>
>> Looks good to me,
>>
>> Acked-by: Hanjun Guo <guohanjun@huawei.com>
> 
> I wasn't cc'd on the original patch but I couldn't figure out why it
> uses IRQF_NO_AUTOEN when requesting the irq given that the first thing
> it does is enable it.

Sorry for the late reply, seems this comment was addressed.

But when I go back to read the latest AGDI spec [0], I see
the signaling mode can be SDEI *and* interrupt:

Flags node structures 1 36 Bits [1:0]: Signaling mode:

2: Both SDEI and Interrupt-based signaling is
supported. While an SDEI event handler is
registered, the platform is allowed to not
generate the wired interrupt

So which means we need to support both SDEI and interrupt when the
signaling mode is 2, but for now, the code is

+	if (agdi_table->flags & ACPI_AGDI_SIGNALING_MODE)
+		pdata.gsiv = agdi_table->gsiv;
+	else
+		pdata.sdei_event = agdi_table->sdei_event;

Kazuhiro, could you take a look again?

Thanks
Hanjun

[0]: https://developer.arm.com/documentation/den0093/latest/

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.