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

[Kazuhiro Abe (Fujitsu)]

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.

Best Regards,
Kazuhiro Abe

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