Hi Hanjun,

Thanks for your comments.

quoted

quoted

quoted

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 <redacted>
Signed-off-by: Kazuhiro Abe <redacted>
---
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?

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/