On Thu, May 14, 2015 at 10:14 PM, Feng Kan [off-list ref] wrote:

On Thu, May 14, 2015 at 3:32 AM, Linus Walleij [off-list ref] wrote:

quoted

But surely the GPIO block has its own status register, so are
you saying that this register is unreliable?

When the GPIO is used as interrupt, the gpio block does not report the
status anymore. Which leaves us stuck with SPISR.

quoted

I can think of a few reasons, like transient IRQs etc but
what is actually causing this?

I won't say the obvious.

Yeah I see your problem now :(

I think it's better to fix the access functions so that you can
cross-call to the GIC driver to get the SPISR flag out though.
Let's see what Marc says.

quoted

Which GPIO driver is this? Is it upstream?

Yes, it is upstream. It is the xgene slimpro gpio driver. I am starting to
think that we ought to switch to use some gpio poll driver rather than
using gpio-key.

There is both gpio_keys_polled and IRQ-driven gpio_keys so yeah
that's possible. But honestly I think it's better to deal with this
problem for real because IRQ is more efficient.

So the way I perceive it this is the real problem:

+static int gic_irq_get_irqchip_state(struct irq_data *d,
+                                     enum irqchip_irq_state which, bool *val)
+{
+       switch (which) {
(...)
+       case IRQCHIP_STATE_ACTIVE:
+               *val = gic_peek_irq(d, GIC_DIST_ACTIVE_SET);
+               break;

Here it reads the status from 0x300 where DIST_ACTIVE_SET
is, so if you change GIC_DIST_ACTIVE_SET to
0xd04 (SPISR), does it start working?

I would *guess* that maybe you want to implement and
ask for IRQCHIP_STATE_LINE_LEVEL instead of
IRQCHIP_STATE_ACTIVE, and have that case: read
from 0xd04 (SPISR) instead, because that makes more
sense to me, or am I wrong at it?

+       case IRQCHIP_STATE_LINE_LEVEL:
+               *val = gic_peek_irq(d, GIC_DIST_SPISR);
+               break;

And then put a define into <linux/irqchip/arm-gic.h> for
GIC_DIST_SPISR.

Yours,
Linus Walleij