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

[Marc Zyngier <marc.zyngier@xxxxxxx>]

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?

> 
>> I appreciate there is a dependency between the two, but that's something
>> we should be able to solve (probe deferral?).
>>
> 
> Perhaps it would be better to just clone the irqchip part into the
> GPIO driver? Everything else needs to be modified anyway, including
> the domain alloc/translate methods.
> 
> That still leaves the question how to deal with the first four signal
> lines, which are not shared between the EXIU and the GPIO controller.
> but personally, I'm happy to just categorize that as "don't do that"
> territory, and just ignore it for the time being.
> 

Maybe. But I'd really like to understand why the above doesn't work in
your case (as you can tell, my ACPI-foo is pretty basic...).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...