* [PATCH] ACPI: AGDI: Add interrupt signaling mode support
@ 2025-08-18 6:54 Kazuhiro Abe
2025-08-19 14:47 ` Hanjun Guo
2025-08-21 3:51 ` Ilkka Koskinen
0 siblings, 2 replies; 6+ messages in thread
From: Kazuhiro Abe @ 2025-08-18 6:54 UTC (permalink / raw)
To: Lorenzo Pieralisi, Hanjun Guo, Sudeep Holla, Rafael J. Wysocki,
Len Brown, linux-acpi, linux-arm-kernel, linux-kernel,
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.
SDEI & Interrupt mode is not supported.
Signed-off-by: Kazuhiro Abe <fj1078ii@aa.jp.fujitsu.com>
---
drivers/acpi/arm64/agdi.c | 114 +++++++++++++++++++++++++++++++++++---
include/acpi/actbl2.h | 4 +-
2 files changed, 110 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/arm64/agdi.c b/drivers/acpi/arm64/agdi.c
index e0df3daa4abf0..c514bb874c5d3 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,20 @@ static int agdi_probe(struct platform_device *pdev)
if (!adata)
return -EINVAL;
- return agdi_sdei_probe(pdev, adata);
+ switch (adata->flags & ACPI_AGDI_SIGNALING_MODE_MASK) {
+ case ACPI_AGDI_SIGNALING_MODE_SDEI:
+ return agdi_sdei_probe(pdev, adata);
+
+ case ACPI_AGDI_SIGNALING_MODE_INTERRUPT:
+ return agdi_interrupt_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 +144,34 @@ 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);
+
+ switch (adata->flags & ACPI_AGDI_SIGNALING_MODE_MASK) {
+ case ACPI_AGDI_SIGNALING_MODE_SDEI:
+ agdi_sdei_remove(pdev, adata);
+ break;
+
+ case ACPI_AGDI_SIGNALING_MODE_INTERRUPT:
+ agdi_interrupt_remove(pdev, adata);
+ break;
+ }
+}
+
static struct platform_driver agdi_driver = {
.driver = {
.name = "agdi",
@@ -94,7 +183,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 +192,23 @@ 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");
+ switch (agdi_table->flags & ACPI_AGDI_SIGNALING_MODE_MASK) {
+ case ACPI_AGDI_SIGNALING_MODE_SDEI:
+ pdata.sdei_event = agdi_table->sdei_event;
+ break;
+
+ case ACPI_AGDI_SIGNALING_MODE_INTERRUPT:
+ pdata.gsiv = agdi_table->gsiv;
+ break;
+
+ default:
+ pr_warn("Signaling mode(%d) is not supported",
+ agdi_table->flags & ACPI_AGDI_SIGNALING_MODE_MASK);
goto err_put_table;
}
- 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))
diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index 048f5f47f8b88..9ddbdd772f139 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -339,7 +339,9 @@ struct acpi_table_agdi {
/* Mask for Flags field above */
-#define ACPI_AGDI_SIGNALING_MODE (1)
+#define ACPI_AGDI_SIGNALING_MODE_MASK (3)
+#define ACPI_AGDI_SIGNALING_MODE_SDEI (0)
+#define ACPI_AGDI_SIGNALING_MODE_INTERRUPT (1)
/*******************************************************************************
*
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI: AGDI: Add interrupt signaling mode support
2025-08-18 6:54 [PATCH] ACPI: AGDI: Add interrupt signaling mode support Kazuhiro Abe
@ 2025-08-19 14:47 ` Hanjun Guo
2025-08-25 8:22 ` Kazuhiro Abe (Fujitsu)
2025-08-21 3:51 ` Ilkka Koskinen
1 sibling, 1 reply; 6+ messages in thread
From: Hanjun Guo @ 2025-08-19 14:47 UTC (permalink / raw)
To: Kazuhiro Abe, Lorenzo Pieralisi, Sudeep Holla, Rafael J. Wysocki,
Len Brown, linux-acpi, linux-arm-kernel, linux-kernel
Hi Kazuhiro,
On 2025/8/18 14:54, 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.
> SDEI & Interrupt mode is not supported.
I think this can be removed, it's not allowed naturely.
>
> Signed-off-by: Kazuhiro Abe <fj1078ii@aa.jp.fujitsu.com>
> ---
> drivers/acpi/arm64/agdi.c | 114 +++++++++++++++++++++++++++++++++++---
> include/acpi/actbl2.h | 4 +-
> 2 files changed, 110 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/acpi/arm64/agdi.c b/drivers/acpi/arm64/agdi.c
> index e0df3daa4abf0..c514bb874c5d3 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;
will we use normal interrupt as the signaling?
In the spec, it says:
Some use-cases, such as system management, require the ability to
generate a non-maskable event to the OS to request the OS kernel to
perform a diagnostic dump and reset the system.
Seems only non-maskable event is allowed, Sudeep, any idea about this?
> + 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,20 @@ static int agdi_probe(struct platform_device *pdev)
> if (!adata)
> return -EINVAL;
>
> - return agdi_sdei_probe(pdev, adata);
> + switch (adata->flags & ACPI_AGDI_SIGNALING_MODE_MASK) {
> + case ACPI_AGDI_SIGNALING_MODE_SDEI:
> + return agdi_sdei_probe(pdev, adata);
> +
> + case ACPI_AGDI_SIGNALING_MODE_INTERRUPT:
> + return agdi_interrupt_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 +144,34 @@ 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);
> +
> + switch (adata->flags & ACPI_AGDI_SIGNALING_MODE_MASK) {
> + case ACPI_AGDI_SIGNALING_MODE_SDEI:
> + agdi_sdei_remove(pdev, adata);
> + break;
> +
> + case ACPI_AGDI_SIGNALING_MODE_INTERRUPT:
> + agdi_interrupt_remove(pdev, adata);
> + break;
> + }
> +}
> +
> static struct platform_driver agdi_driver = {
> .driver = {
> .name = "agdi",
> @@ -94,7 +183,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 +192,23 @@ 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");
> + switch (agdi_table->flags & ACPI_AGDI_SIGNALING_MODE_MASK) {
> + case ACPI_AGDI_SIGNALING_MODE_SDEI:
> + pdata.sdei_event = agdi_table->sdei_event;
> + break;
> +
> + case ACPI_AGDI_SIGNALING_MODE_INTERRUPT:
> + pdata.gsiv = agdi_table->gsiv;
> + break;
> +
> + default:
> + pr_warn("Signaling mode(%d) is not supported",
> + agdi_table->flags & ACPI_AGDI_SIGNALING_MODE_MASK);
> goto err_put_table;
> }
>
> - pdata.sdei_event = agdi_table->sdei_event;
> + pdata.irq = -1;
> + pdata.flags = agdi_table->flags;
My major concern is about the normal interrrupt as the event, not sure
if it is ok, let's figure it out first.
>
> pdev = platform_device_register_data(NULL, "agdi", 0, &pdata, sizeof(pdata));
> if (IS_ERR(pdev))
> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> index 048f5f47f8b88..9ddbdd772f139 100644
> --- a/include/acpi/actbl2.h
> +++ b/include/acpi/actbl2.h
> @@ -339,7 +339,9 @@ struct acpi_table_agdi {
>
> /* Mask for Flags field above */
>
> -#define ACPI_AGDI_SIGNALING_MODE (1)
> +#define ACPI_AGDI_SIGNALING_MODE_MASK (3)
> +#define ACPI_AGDI_SIGNALING_MODE_SDEI (0)
> +#define ACPI_AGDI_SIGNALING_MODE_INTERRUPT (1)
You need to send a patch to ACPICA first to add interrupt support,
this file belongs acpica.
Thanks
Hanjun
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI: AGDI: Add interrupt signaling mode support
2025-08-18 6:54 [PATCH] ACPI: AGDI: Add interrupt signaling mode support Kazuhiro Abe
2025-08-19 14:47 ` Hanjun Guo
@ 2025-08-21 3:51 ` Ilkka Koskinen
2025-08-25 8:23 ` Kazuhiro Abe (Fujitsu)
1 sibling, 1 reply; 6+ messages in thread
From: Ilkka Koskinen @ 2025-08-21 3:51 UTC (permalink / raw)
To: Kazuhiro Abe
Cc: Lorenzo Pieralisi, Hanjun Guo, Sudeep Holla, Rafael J. Wysocki,
Len Brown, linux-acpi, linux-arm-kernel, linux-kernel
Hi Kazuhiro
On Mon, 18 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.
> SDEI & Interrupt mode is not supported.
>
> Signed-off-by: Kazuhiro Abe <fj1078ii@aa.jp.fujitsu.com>
> ---
> drivers/acpi/arm64/agdi.c | 114 +++++++++++++++++++++++++++++++++++---
> include/acpi/actbl2.h | 4 +-
> 2 files changed, 110 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/acpi/arm64/agdi.c b/drivers/acpi/arm64/agdi.c
> index e0df3daa4abf0..c514bb874c5d3 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,20 @@ static int agdi_probe(struct platform_device *pdev)
> if (!adata)
> return -EINVAL;
>
> - return agdi_sdei_probe(pdev, adata);
> + switch (adata->flags & ACPI_AGDI_SIGNALING_MODE_MASK) {
> + case ACPI_AGDI_SIGNALING_MODE_SDEI:
> + return agdi_sdei_probe(pdev, adata);
> +
> + case ACPI_AGDI_SIGNALING_MODE_INTERRUPT:
> + return agdi_interrupt_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 +144,34 @@ 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);
> +
> + switch (adata->flags & ACPI_AGDI_SIGNALING_MODE_MASK) {
> + case ACPI_AGDI_SIGNALING_MODE_SDEI:
> + agdi_sdei_remove(pdev, adata);
> + break;
> +
> + case ACPI_AGDI_SIGNALING_MODE_INTERRUPT:
> + agdi_interrupt_remove(pdev, adata);
> + break;
> + }
> +}
> +
> static struct platform_driver agdi_driver = {
> .driver = {
> .name = "agdi",
> @@ -94,7 +183,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 +192,23 @@ 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");
> + switch (agdi_table->flags & ACPI_AGDI_SIGNALING_MODE_MASK) {
> + case ACPI_AGDI_SIGNALING_MODE_SDEI:
> + pdata.sdei_event = agdi_table->sdei_event;
> + break;
> +
> + case ACPI_AGDI_SIGNALING_MODE_INTERRUPT:
> + pdata.gsiv = agdi_table->gsiv;
> + break;
> +
> + default:
> + pr_warn("Signaling mode(%d) is not supported",
> + agdi_table->flags & ACPI_AGDI_SIGNALING_MODE_MASK);
> goto err_put_table;
> }
>
> - 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))
> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> index 048f5f47f8b88..9ddbdd772f139 100644
> --- a/include/acpi/actbl2.h
> +++ b/include/acpi/actbl2.h
> @@ -339,7 +339,9 @@ struct acpi_table_agdi {
>
> /* Mask for Flags field above */
>
> -#define ACPI_AGDI_SIGNALING_MODE (1)
> +#define ACPI_AGDI_SIGNALING_MODE_MASK (3)
> +#define ACPI_AGDI_SIGNALING_MODE_SDEI (0)
> +#define ACPI_AGDI_SIGNALING_MODE_INTERRUPT (1)
>
> /*******************************************************************************
> *
Like Hanjun mentioned, these should go through ACPICA project. When you do
that could you also add the SDEI flag to the other places, which are
needed for iasl to be able to decode AGDI tables. You can find them if you
take a look at my patch in ACPICA project.
Cheers, Ilkka
> --
> 2.43.0
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] ACPI: AGDI: Add interrupt signaling mode support
2025-08-19 14:47 ` Hanjun Guo
@ 2025-08-25 8:22 ` Kazuhiro Abe (Fujitsu)
0 siblings, 0 replies; 6+ messages in thread
From: Kazuhiro Abe (Fujitsu) @ 2025-08-25 8:22 UTC (permalink / raw)
To: 'Hanjun Guo', Lorenzo Pieralisi, Sudeep Holla,
Rafael J. Wysocki, Len Brown, linux-acpi@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Hi, Hanjun
Thank you for your comment.
> Hi Kazuhiro,
>
> On 2025/8/18 14:54, 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.
>
>
> > SDEI & Interrupt mode is not supported.
>
> I think this can be removed, it's not allowed naturely.
I mentioned this because it was added in "ACPI for Arm Components 1.2 Platform Design Document"
I think I'll remove it from the cover letter.
>
> >
> > Signed-off-by: Kazuhiro Abe <fj1078ii@aa.jp.fujitsu.com>
> > ---
> > drivers/acpi/arm64/agdi.c | 114
> +++++++++++++++++++++++++++++++++++---
> > include/acpi/actbl2.h | 4 +-
> > 2 files changed, 110 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/acpi/arm64/agdi.c b/drivers/acpi/arm64/agdi.c
> > index e0df3daa4abf0..c514bb874c5d3 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;
>
> will we use normal interrupt as the signaling?
>
> In the spec, it says:
>
> Some use-cases, such as system management, require the ability to generate a
> non-maskable event to the OS to request the OS kernel to perform a diagnostic
> dump and reset the system.
I also included request_irq as a fallback, as NMI might not be enabled.
However, if this is not allowed by the specifications, I'd like to remove it.
>
> Seems only non-maskable event is allowed, Sudeep, any idea about this?
>
> > + 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,20 @@ static int agdi_probe(struct platform_device *pdev)
> > if (!adata)
> > return -EINVAL;
> >
> > - return agdi_sdei_probe(pdev, adata);
> > + switch (adata->flags & ACPI_AGDI_SIGNALING_MODE_MASK) {
> > + case ACPI_AGDI_SIGNALING_MODE_SDEI:
> > + return agdi_sdei_probe(pdev, adata);
> > +
> > + case ACPI_AGDI_SIGNALING_MODE_INTERRUPT:
> > + return agdi_interrupt_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 +144,34 @@ 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);
> > +
> > + switch (adata->flags & ACPI_AGDI_SIGNALING_MODE_MASK) {
> > + case ACPI_AGDI_SIGNALING_MODE_SDEI:
> > + agdi_sdei_remove(pdev, adata);
> > + break;
> > +
> > + case ACPI_AGDI_SIGNALING_MODE_INTERRUPT:
> > + agdi_interrupt_remove(pdev, adata);
> > + break;
> > + }
> > +}
> > +
> > static struct platform_driver agdi_driver = {
> > .driver = {
> > .name = "agdi",
> > @@ -94,7 +183,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 +192,23 @@ 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");
> > + switch (agdi_table->flags & ACPI_AGDI_SIGNALING_MODE_MASK)
> {
> > + case ACPI_AGDI_SIGNALING_MODE_SDEI:
> > + pdata.sdei_event = agdi_table->sdei_event;
> > + break;
> > +
> > + case ACPI_AGDI_SIGNALING_MODE_INTERRUPT:
> > + pdata.gsiv = agdi_table->gsiv;
> > + break;
> > +
> > + default:
> > + pr_warn("Signaling mode(%d) is not supported",
> > + agdi_table->flags &
> ACPI_AGDI_SIGNALING_MODE_MASK);
> > goto err_put_table;
> > }
> >
> > - pdata.sdei_event = agdi_table->sdei_event;
> > + pdata.irq = -1;
> > + pdata.flags = agdi_table->flags;
>
> My major concern is about the normal interrrupt as the event, not sure if it is ok,
> let's figure it out first.
As mentioned above, I'd like to remove normal interrupt handling if this is not allowed by the specifications.
>
> >
> > pdev = platform_device_register_data(NULL, "agdi", 0, &pdata,
> sizeof(pdata));
> > if (IS_ERR(pdev))
> > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h index
> > 048f5f47f8b88..9ddbdd772f139 100644
> > --- a/include/acpi/actbl2.h
> > +++ b/include/acpi/actbl2.h
> > @@ -339,7 +339,9 @@ struct acpi_table_agdi {
> >
> > /* Mask for Flags field above */
> >
> > -#define ACPI_AGDI_SIGNALING_MODE (1)
> > +#define ACPI_AGDI_SIGNALING_MODE_MASK (3) #define
> > +ACPI_AGDI_SIGNALING_MODE_SDEI (0) #define
> > +ACPI_AGDI_SIGNALING_MODE_INTERRUPT (1)
>
> You need to send a patch to ACPICA first to add interrupt support, this file
> belongs acpica.
Understood. I will send a patch to ACPICA first. Once it is merged, I will send v2.
Best Regards,
Kazuhiro Abe
>
> Thanks
> Hanjun
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] ACPI: AGDI: Add interrupt signaling mode support
2025-08-21 3:51 ` Ilkka Koskinen
@ 2025-08-25 8:23 ` Kazuhiro Abe (Fujitsu)
2025-08-29 10:21 ` Kazuhiro Abe (Fujitsu)
0 siblings, 1 reply; 6+ messages in thread
From: Kazuhiro Abe (Fujitsu) @ 2025-08-25 8:23 UTC (permalink / raw)
To: 'Ilkka Koskinen'
Cc: Lorenzo Pieralisi, Hanjun Guo, Sudeep Holla, Rafael J. Wysocki,
Len Brown, linux-acpi@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Hi, Ilkka
> Hi Kazuhiro
>
> On Mon, 18 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.
> > SDEI & Interrupt mode is not supported.
> >
> > Signed-off-by: Kazuhiro Abe <fj1078ii@aa.jp.fujitsu.com>
> > ---
> > drivers/acpi/arm64/agdi.c | 114
> +++++++++++++++++++++++++++++++++++---
> > include/acpi/actbl2.h | 4 +-
> > 2 files changed, 110 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/acpi/arm64/agdi.c b/drivers/acpi/arm64/agdi.c
> > index e0df3daa4abf0..c514bb874c5d3 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,20 @@ static int agdi_probe(struct platform_device *pdev)
> > if (!adata)
> > return -EINVAL;
> >
> > - return agdi_sdei_probe(pdev, adata);
> > + switch (adata->flags & ACPI_AGDI_SIGNALING_MODE_MASK) {
> > + case ACPI_AGDI_SIGNALING_MODE_SDEI:
> > + return agdi_sdei_probe(pdev, adata);
> > +
> > + case ACPI_AGDI_SIGNALING_MODE_INTERRUPT:
> > + return agdi_interrupt_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 +144,34 @@ 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);
> > +
> > + switch (adata->flags & ACPI_AGDI_SIGNALING_MODE_MASK) {
> > + case ACPI_AGDI_SIGNALING_MODE_SDEI:
> > + agdi_sdei_remove(pdev, adata);
> > + break;
> > +
> > + case ACPI_AGDI_SIGNALING_MODE_INTERRUPT:
> > + agdi_interrupt_remove(pdev, adata);
> > + break;
> > + }
> > +}
> > +
> > static struct platform_driver agdi_driver = {
> > .driver = {
> > .name = "agdi",
> > @@ -94,7 +183,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 +192,23 @@ 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");
> > + switch (agdi_table->flags & ACPI_AGDI_SIGNALING_MODE_MASK) {
> > + case ACPI_AGDI_SIGNALING_MODE_SDEI:
> > + pdata.sdei_event = agdi_table->sdei_event;
> > + break;
> > +
> > + case ACPI_AGDI_SIGNALING_MODE_INTERRUPT:
> > + pdata.gsiv = agdi_table->gsiv;
> > + break;
> > +
> > + default:
> > + pr_warn("Signaling mode(%d) is not supported",
> > + agdi_table->flags &
> ACPI_AGDI_SIGNALING_MODE_MASK);
> > goto err_put_table;
> > }
> >
> > - 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))
> > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h index
> > 048f5f47f8b88..9ddbdd772f139 100644
> > --- a/include/acpi/actbl2.h
> > +++ b/include/acpi/actbl2.h
> > @@ -339,7 +339,9 @@ struct acpi_table_agdi {
> >
> > /* Mask for Flags field above */
> >
> > -#define ACPI_AGDI_SIGNALING_MODE (1)
> > +#define ACPI_AGDI_SIGNALING_MODE_MASK (3) #define
> > +ACPI_AGDI_SIGNALING_MODE_SDEI (0) #define
> > +ACPI_AGDI_SIGNALING_MODE_INTERRUPT (1)
> >
> > /*********************************************************************
> > **********
> > *
>
> Like Hanjun mentioned, these should go through ACPICA project. When you do
> that could you also add the SDEI flag to the other places, which are needed for iasl
> to be able to decode AGDI tables. You can find them if you take a look at my patch
> in ACPICA project.
Thanks for your comment. I found your patch.
I will send a patch to ACPICA first. Once it is merged, I will send v2.
Best Regards,
Kazuhiro Abe
>
> Cheers, Ilkka
>
> > --
> > 2.43.0
> >
> >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] ACPI: AGDI: Add interrupt signaling mode support
2025-08-25 8:23 ` Kazuhiro Abe (Fujitsu)
@ 2025-08-29 10:21 ` Kazuhiro Abe (Fujitsu)
0 siblings, 0 replies; 6+ messages in thread
From: Kazuhiro Abe (Fujitsu) @ 2025-08-29 10:21 UTC (permalink / raw)
To: 'Ilkka Koskinen'
Cc: 'Lorenzo Pieralisi', 'Hanjun Guo',
'Sudeep Holla', 'Rafael J. Wysocki',
'Len Brown', 'linux-acpi@vger.kernel.org',
'linux-arm-kernel@lists.infradead.org',
'linux-kernel@vger.kernel.org'
Hi, Ilkka
> Hi, Ilkka
>
> > Hi Kazuhiro
> >
> > On Mon, 18 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.
> > > SDEI & Interrupt mode is not supported.
> > >
> > > Signed-off-by: Kazuhiro Abe <fj1078ii@aa.jp.fujitsu.com>
> > > ---
> > > drivers/acpi/arm64/agdi.c | 114
> > +++++++++++++++++++++++++++++++++++---
> > > include/acpi/actbl2.h | 4 +-
> > > 2 files changed, 110 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/acpi/arm64/agdi.c b/drivers/acpi/arm64/agdi.c
> > > index e0df3daa4abf0..c514bb874c5d3 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,20 @@ static int agdi_probe(struct platform_device *pdev)
> > > if (!adata)
> > > return -EINVAL;
> > >
> > > - return agdi_sdei_probe(pdev, adata);
> > > + switch (adata->flags & ACPI_AGDI_SIGNALING_MODE_MASK) {
> > > + case ACPI_AGDI_SIGNALING_MODE_SDEI:
> > > + return agdi_sdei_probe(pdev, adata);
> > > +
> > > + case ACPI_AGDI_SIGNALING_MODE_INTERRUPT:
> > > + return agdi_interrupt_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 +144,34 @@ 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);
> > > +
> > > + switch (adata->flags & ACPI_AGDI_SIGNALING_MODE_MASK) {
> > > + case ACPI_AGDI_SIGNALING_MODE_SDEI:
> > > + agdi_sdei_remove(pdev, adata);
> > > + break;
> > > +
> > > + case ACPI_AGDI_SIGNALING_MODE_INTERRUPT:
> > > + agdi_interrupt_remove(pdev, adata);
> > > + break;
> > > + }
> > > +}
> > > +
> > > static struct platform_driver agdi_driver = {
> > > .driver = {
> > > .name = "agdi",
> > > @@ -94,7 +183,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 +192,23 @@ 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");
> > > + switch (agdi_table->flags & ACPI_AGDI_SIGNALING_MODE_MASK) {
> > > + case ACPI_AGDI_SIGNALING_MODE_SDEI:
> > > + pdata.sdei_event = agdi_table->sdei_event;
> > > + break;
> > > +
> > > + case ACPI_AGDI_SIGNALING_MODE_INTERRUPT:
> > > + pdata.gsiv = agdi_table->gsiv;
> > > + break;
> > > +
> > > + default:
> > > + pr_warn("Signaling mode(%d) is not supported",
> > > + agdi_table->flags &
> > ACPI_AGDI_SIGNALING_MODE_MASK);
> > > goto err_put_table;
> > > }
> > >
> > > - 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))
> > > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h index
> > > 048f5f47f8b88..9ddbdd772f139 100644
> > > --- a/include/acpi/actbl2.h
> > > +++ b/include/acpi/actbl2.h
> > > @@ -339,7 +339,9 @@ struct acpi_table_agdi {
> > >
> > > /* Mask for Flags field above */
> > >
> > > -#define ACPI_AGDI_SIGNALING_MODE (1)
> > > +#define ACPI_AGDI_SIGNALING_MODE_MASK (3) #define
> > > +ACPI_AGDI_SIGNALING_MODE_SDEI (0) #define
> > > +ACPI_AGDI_SIGNALING_MODE_INTERRUPT (1)
> > >
> > > /*******************************************************************
> > > **
> > > **********
> > > *
> >
> > Like Hanjun mentioned, these should go through ACPICA project. When
> > you do that could you also add the SDEI flag to the other places,
> > which are needed for iasl to be able to decode AGDI tables. You can
> > find them if you take a look at my patch in ACPICA project.
>
> Thanks for your comment. I found your patch.
> I will send a patch to ACPICA first. Once it is merged, I will send v2.
After reconsidering, I decided that the acpica code fix was unnecessary,
so I posted v2 patch without the acpica fix.
Best Regards,
Kazuhiro Abe
>
> Best Regards,
> Kazuhiro Abe
>
> >
> > Cheers, Ilkka
> >
> > > --
> > > 2.43.0
> > >
> > >
> > >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-08-29 10:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-18 6:54 [PATCH] ACPI: AGDI: Add interrupt signaling mode support Kazuhiro Abe
2025-08-19 14:47 ` Hanjun Guo
2025-08-25 8:22 ` Kazuhiro Abe (Fujitsu)
2025-08-21 3:51 ` Ilkka Koskinen
2025-08-25 8:23 ` Kazuhiro Abe (Fujitsu)
2025-08-29 10:21 ` Kazuhiro Abe (Fujitsu)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).