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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 26 Aug 2019 10:15:26 -0600
Lina Iyer <ilina@xxxxxxxxxxxxxx> wrote:

> 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@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?  
> >
> >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?

[desperately trying to switch to my korg address...]

I'm not sure I fully get the question: if this is whether it is worth
implementing it, it usually is a good idea if the HW supports is. But I
have the feeling that this isn't your question, so maybe explaining
your use-case would help.

Do you mean using irq_set_irqchip_state from within an irqchip driver?
That'd be quite odd, as this function is usually used when something
*outside* of the irqchip driver needs to poke at some internal state
because "it knows best" (see for example what KVM does with the active
state).

But again, stating your problem would help.

Thanks,

	M.
-- 
Without deviation from the norm, progress is not possible.



[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux