Re: [RFC PATCH 2/3] irqchip/exiu: implement ACPI gpiolib/irqchip support

[Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>]

On Fri, 26 Apr 2019 at 13:45, Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote:
>
> On Fri, 26 Apr 2019 at 10:44, Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
> >
> > On 26/04/2019 09:24, Ard Biesheuvel wrote:
> > > On Thu, 25 Apr 2019 at 17:33, Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
> > >>
> > >> Hi Ard,
> > >>
> > >> On 25/04/2019 11:20, Ard Biesheuvel wrote:
> > >>> Expose the existing EXIU hierarchical irqchip domain code to permit
> > >>> the interrupt controller to be used as the irqchip component of a
> > >>> GPIO controller on ACPI systems.
> > >>>
> > >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
> > >>> ---
> > >>>  drivers/irqchip/irq-sni-exiu.c | 82 +++++++++++++++++---
> > >>>  1 file changed, 73 insertions(+), 9 deletions(-)
> > >>>
> > >>
> > >> [...]
> > >>
> > >>> +int exiu_acpi_init(struct platform_device *pdev, struct gpio_chip *gc)
> > >>> +{
> > >>> +     struct irq_domain *parent_domain = NULL, *domain;
> > >>> +     struct resource *res;
> > >>> +     int irq;
> > >>> +
> > >>> +     irq = platform_get_irq(pdev, 0);
> > >>> +     if (irq > 0)
> > >>> +             parent_domain = irq_get_irq_data(irq)->domain;
> > >>> +
> > >>> +     if (!parent_domain) {
> > >>> +             dev_err(&pdev->dev, "unable to obtain parent domain\n");
> > >>> +             return -ENODEV;
> > >>> +     }
> > >>> +
> > >>> +     res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > >>> +     if (!res) {
> > >>> +             dev_err(&pdev->dev, "failed to parse memory resource\n");
> > >>> +             return -ENXIO;
> > >>> +     }
> > >>> +
> > >>> +     domain = exiu_init(parent_domain, dev_fwnode(&pdev->dev), res);
> > >>> +     if (IS_ERR(domain)) {
> > >>> +             dev_err(&pdev->dev, "failed to create IRQ domain (%ld)\n",
> > >>> +                     PTR_ERR(domain));
> > >>> +             return PTR_ERR(domain);
> > >>> +     }
> > >>> +
> > >>> +     gc->irq.domain = domain;
> > >>> +     gc->to_irq = exiu_acpi_gpio_to_irq;
> > >>> +
> > >>> +     dev_info(&pdev->dev, "%d interrupts forwarded\n", NUM_IRQS);
> > >>> +
> > >>> +     return 0;
> > >>> +}
> > >>> +EXPORT_SYMBOL(exiu_acpi_init);
> > >>> +#endif
> > >>>
> > >>
> > >> I must say I'm not overly keen on this function. Why can't this be
> > >> probed as an ACPI device, creating the corresponding domain, and leaving
> > >> to the GPIO driver the task of doing the GPIO stuff?
> > >>
> > >
> > > Because ACPI only permits 'system' interrupts or GPIO interrupts, it
> > > does not allow arbitrary device objects in the ACPI namespace to be
> > > targeted as interrupt controllers.
> >
> > Hmmm. We already have at least one driver (irq-mbigen.c) that does
> > exactly that. It uses interrupts from the GIC (it is notionally behind
> > the ITS), and offers a bunch of interrupt lines itself. Why is it different?
> >
>
> You are right, it isn't. It turns out that there is another way to
> signal ACPI events based on interrupts, and it involves the ACPI0013
> GED device. I will try to model it that way instead.
>

Unfortunately, this doesn't work either. The GED device can respond to
GSIVs only, and so going via the EXIU doesn't work.

I have attempted to hack it up via AML, but the GED driver uses a
threaded interrupt, and so the interrupt is re-enabled at the GIC
before the AML handler is executed (which clears it in the EXIU)

For the RAS case, I guess we could let the firmware pick an arbitrary
unused SPI and signal that directly into the GIC, but for the power
button (which is physically wired to the EXIU), we have to model the
EXIU either has a separate pseudo-GPIO controller, or as part of the
real GPIO block.