Hi Ilkka > Hi Kazuhiro, > > On Fri, 5 Sep 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 signaling mode The interrupt > > vector is retrieved from the AGDI table, and call panic function when > > an interrupt occurs. > > > > Signed-off-by: Kazuhiro Abe <fj1078ii@xxxxxxxxxxxxxxxxx> > > > Looks good to me. > > Reviewed-by: Ilkka Koskinen <ilkka@xxxxxxxxxxxxxxxxxxxxxx> > Thanks for your review. Best Regards, Kazuhiro Abe > > Hanjun & Sudeep, what's your thought on enabling the use of regular interrupts > here? I do agree the spec talks about non-maskable ones and to my understanding > that's what the idea was indeed. > > Cheers, Ilkka > > > > --- > > I keep normal IRQ code when NMI cannot be used. > > If there is any concern, please let me know. > > > > 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@xxxxxxxx > > jitsu.com/ > > v1->v2 > > - Remove acpica update since there is no need to update define value > > for this patch. > > --- > > drivers/acpi/arm64/agdi.c | 95 > ++++++++++++++++++++++++++++++++++++--- > > 1 file changed, 88 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/acpi/arm64/agdi.c b/drivers/acpi/arm64/agdi.c > > index e0df3daa4abf..2313a46f01cd 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,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 +139,29 @@ 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 +173,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 +183,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 > > > >
