Re: unexpected side effect of "gpiolib: override irq_enable/disable"

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Thu, Sep 13, 2018 at 11:07 AM Hans Verkuil <hverkuil@xxxxxxxxx> wrote:

> > I am not sure which solution is the best. Of course I prefer this one as
> > I don't have to modify the pinctrl-at91 driver but if I have to modify
> > it to not share the same irq_chip structure, I'll handle it. Just let me
> > know.
(...)
> One concern I have with the approach taken in this driver is that while placing
> the hooks works fine (with this patch), but when the gpiochip that contains
> the old callbacks for this irqchip is removed, the hooks are also removed for
> all gpiochips using this irqchip. I don't think there is anything I can do
> about that without hacking struct irq_chip.
>
> So from that perspective it's better to have separate irq_chip structs. But
> if there are (a lot?) more drivers doing this, than I need to look for a
> better solution.

I think it's something like that before it was an OK practice to have
the same irqchip for multiple GPIO chips, but after this patch it
becomes a bad practice.

I do not think it is a big problem. Very few systems would even
consider randomly unloading their gpiochips, they are mostly
vital infrastructure, and we don't go building complex solutions
around theoretical problems.

I think what we should do is apply this fix, then annotate the
drivers that use the same irqchip with multiple gpiochips with
some
FIXME: this is bad practice
comments.

Yours,
Linus Walleij