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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CACRpkdZ2rBii88APKacg=LyVY-9m9pxM5z08YawkvjuWmjSJrA@mail.gmail.com>
Date:   Sun, 30 Jan 2022 02:08:38 +0100
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Nikita Yushchenko <nikita.yoush@...entembedded.com>,
        Marc Zyngier <maz@...nel.org>
Cc:     Bartosz Golaszewski <brgl@...ev.pl>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Eugeniu Rosca <erosca@...adit-jv.com>,
        linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH/RFC] gpiolib: create irq_domain hierarchy bottom if possible

On Wed, Jan 19, 2022 at 5:07 PM Nikita Yushchenko
<nikita.yoush@...entembedded.com> 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