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

[Kazuhiro Abe (Fujitsu)]

Hi Hanjun and Will,

> On 2025/11/18 18:09, Kazuhiro Abe (Fujitsu) wrote:
> > Hi Hanjun,
> >
> > Thanks for your comments.
> >
> >>>>> 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 at os.amperecomputing.com>
> >>>>> Signed-off-by: Kazuhiro Abe<fj1078ii at 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 at 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?
> > I don't think enabling both interrupt and SDEI makes sense.
> > Therefore, we need to choose one but we can't know which one is preferred
> for each platform.
> > On the other hand, before this patch the code is:
> >
> >         if (agdi_table->flags & ACPI_AGDI_SIGNALING_MODE) {
> >                 pr_warn("Interrupt signaling is not supported");
> >                 goto err_put_table;
> >         }
> >
> > ...and SDEI is used when mode is 2. This behavior will not be changed by this
> patch.
> > Therefore, It seems for me this patch is reasonable for now.
> > I would appreciate your thoughts on this.
> 
> I had a talk to firmware guy about this, they say even with both supported, the
> platform will use the same interrupt number to trigger, and the spec says:
> 
> While an SDEI event handler is registered, the platform is allowed to not
> generate the wired interrupt
> 
> I think this patch is ready to go.

Hanjun, Thank you for your review.

Will, as all comments have been addressed, please consider applying this patch.

Best Regards,
Kazuhiro Abe

> 
> Thanks
> Hanjun