[PATCH] Revert "irqchip: irq-dove: Add PMU interrupt controller."
Thomas Gleixner
tglx at linutronix.de
Wed Mar 5 09:42:34 EST 2014
On Wed, 5 Mar 2014, Russell King - ARM Linux wrote:
> In some ways, this is good news - it shows that the bits in this register
> latch '1' when an interrupt is pending, and remain '1' while the block
> continues to assert its interrupt signal - but can we say that the other
> interrupt functions in this register have that behaviour?
>
> >From the spec, it looks like this is probably true of DFSDone as well.
> DVSDone - I see no separate status register containing status bits
> indicating what the cause of the DVSDone status is. The thermal bits -
> if it's a transitory excursion, may not hold. Battery fault... we
> can guess.
>
> Now, genirq doesn't have a good way to handle this. I'll also say that
> because of the above, I've always been worried about hardware races when
> trying to clear down interrupts in this register - I'd much prefer not
> to touch it unless absolutely necessary. So... how about this instead?
>
> u32 stat = readl_relaxed(gc->reg_base + DOVE_PMC_IRQ_CAUSE) &
> gc->mask_cache;
> u32 done = ~0;
>
> while (stat) {
> unsigned hwirq = ffs(stat) - 1;
>
> stat &= ~(1 << hwirq);
> done &= ~(1 << hwirq);
>
> generic_handle_irq(irq_find_mapping(domain, hwirq));
> }
>
> irq_gc_lock(gc);
> done &= readl_relaxed(gc->reg_base + DOVE_PMC_IRQ_CAUSE);
> writel_relaxed(done, gc->reg_base + DOVE_PMC_IRQ_CAUSE);
> irq_gc_unlock(gc);
>
> This results in the RTC alarm test receiving exactly one interrupt for
> each alarm expiry, as it should do. Thoughts?
You are worried about clearing an interrupt which is transitory and
not kept active at the device level until you handled it for real,
right?
Is the datasheet for this stuff public available?
> Another question: ffs(stat) - any reason to use ffs() there rather than
> fls(stat) which would result in simpler code? r1 = ffs(r4 = stat) creates:
>
> 198: e2641000 rsb r1, r4, #0
> 19c: e1a00006 mov r0, r6
> 1a0: e0011004 and r1, r1, r4
> 1a4: e16f1f11 clz r1, r1
> 1a8: e261101f rsb r1, r1, #31
>
> whereas fls(stat):
>
> 198: e16f1f14 clz r1, r4
> 19c: e261101f rsb r1, r1, #31
> 1a0: e1a00006 mov r0, r6
>
> Kind of a micro-optimisation, but I see no reason to prefer one over the
> other except for this - and I think the switch to ffs() was made in the
> hope of optimising this code!
I don't think it matters in which order you process multiple pending
interrupts.
Thanks,
tglx
More information about the linux-arm-kernel
mailing list