Re: [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

Hi Brian,

I tried to look into this ssbi-gpio problem...

Paging in Marc Z as well.

On Fri, Aug 16, 2019 at 3:10 AM Brian Masney <masneyb@xxxxxxxxxxxxx> wrote:

> I started to convert ssbi-gpio over to this and pm8xxx_gpio_to_irq() has
> this little snippet that's different from spmi-gpio:
>
>         [ fwspec mapping code ]
>
>         /*
>          * Cache the IRQ since pm8xxx_gpio_get() needs this to get determine the
>          * line level.
>          */
>         pin->irq = ret;
>
> Here's the relevant code in pm8xxx_gpio_get():
>
>         if (pin->mode == PM8XXX_GPIO_MODE_OUTPUT) {
>                 ret = pin->output_value;
>         } else if (pin->irq >= 0) {
>                 ret = irq_get_irqchip_state(pin->irq, IRQCHIP_STATE_LINE_LEVEL, &state);
>                 ...
>         }

It's a bit annoying that this API (irq_get_irqchip_state()) is relying on
the global irq numbers and the internal function using irqdesc
__irq_get_irqchip_state() is not exported.

We should be encouraged to operate on IRQ descriptors rather
than IRQ numbers when doing this kind of deep core work,
right?

> What do you think about using EXPORT_SYMBOL_GPL() for gpiochip_to_irq() so
> that we can call it in pm8xxx_gpio_to_irq()?

I would like to avoid that because people tend to abuse every
API I expose (leasson learnt: APIs shall be narrow and deep).

> Or do you have any other
> suggestions for how we can get rid of that IRQ cache?

So the issue is that moving this to the hierarchical helpers
rids pm8xxx_gpio_to_irq() and we needed that to map this.

I would rather just add a new driver API to wrap the irqchip
API:

ret = gpiochip_get_irq_state(offset, IRQCHIP_STATE_LINE_LEVEL, &state);

Where int gpiochip_get_own_irq_state(int offset, ...) will figure
out the gpio_desc of the offset and then call gpiod_to_irq()
and then use that irq number to call irq_get_irqchip_state()
and the goal is met.

This should work I think, and expose a more precise
API for what this driver wants.

> I don't see any other issues for ssbi-gpio.

That's good news!

Yours,
Linus Walleij