Re: [PATCH/RFC] gpiolib: create irq_domain hierarchy bottom if possible

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Wed, Jan 19, 2022 at 5:07 PM Nikita Yushchenko
<nikita.yoush@xxxxxxxxxxxxxxxxxx> wrote:

> Currently, gpiolib creates irq_domain compatible with hierarchical API
> only when interrupts provided by gpiochip lay on top of existing
> hierarchy. Otherwise, legacy API is used.
>
> With this patch, as soon as
> - irq_domain hierarchical API is enabled in the kernel config,
> - chip driver does not request preallocated interrupt numbers,
> - chip driver does not provide it's own irq_domain_ops,
> - chip driver provides fwnode, either explicitly or via it's struct
>   device,
> irq_domain created by gpiolib will use hierarchical API even without
> parent.
>
> This allows other irqchips to lay on top of this irq_domain.

I see what you're trying to do, but we definitely need Marc Z to
provide review for this patch before we merge it.

> @@ -1095,14 +1095,6 @@ static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
(...)
> +       if (d->parent) {

You have just put a big if (d->parent) around 95% of the code, invent
a different
function name and make that do what you want if parent is set, or just
inline the code you need for that case.

>  static bool gpiochip_hierarchy_is_hierarchical(struct gpio_chip *gc)
>  {
> -       return !!gc->irq.parent_domain;
> +       if (gc->irq.parent_domain)
> +               return true;    /* will add to existing hierarchy */

OK the old case...

> +       if (!gc->irq.first && !gc->irq.domain_ops &&
> +           (gc->irq.fwnode || dev_fwnode(&gc->gpiodev->dev)))
> +               return true;    /* will create hierarchy bottom */

This will turn a *lot* of GPIO chips created with this helper
into hierarchical root controllers.

Is this really the right thing to do? (Marc?)

Yours,
Linus Walleij