On Mon, Aug 26 2019 at 13:02 -0600, Marc Zyngier wrote:
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).
Yeah, I meant to say, when invoked on a parent irqchip from a child irqchip. In my case when GPIO is used as an interrupt. Even though the interrupt is not enabled, the GPIO could cause the GIC_ISPEND to be set at GIC. We were wondering if we could use the irq_set_irqchip_state to clear pending state at GPIO's irqchip parent and all the way to GIC, when the GPIO is enabled as an interrupt. -- Lina
