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

[Lina Iyer <ilina@codeaurora.org>]

On Fri, Aug 23 2019 at 03:12 -0600, Marc Zyngier wrote:
>On 23/08/2019 09:24, Linus Walleij wrote:
>> 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@onstation.org> 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?
>
>Certainly, I'd like to gradually move over from the IRQ number to an
>irq_desc. In interrupt-heavy contexts, this ends up being quite time
>consuming. I have an old patch series somewhere changing irq domains to
>use irq_descs internally instead of IRQ numbers, which I should maybe
>revive.
>
>As for __irq_get_irqchip_state, the main issue is that it doesn't
>perform any locking on its own, so you'd have to either provide your
>own, or wrap it with something else.
>
Marc, on a related note, What are your thoughts on a
irq_set_irqchip_state? We are running into an issue where chip state
clear is required as well as clearing out that of the parent. Do you see
problems in doing that from an irqchip driver?

--Lina