Re: [PATCH 21/24] irqchip: mips-gic: Support local interrupts

From: Qais Yousef
Date: Thu Sep 18 2014 - 02:58:17 EST


On 09/17/2014 06:40 PM, Andrew Bresticker wrote:
On Wed, Sep 17, 2014 at 2:50 AM, Qais Yousef <qais.yousef@xxxxxxxxxx> wrote:
On 09/16/2014 12:51 AM, Andrew Bresticker wrote:
The MIPS GIC supports 7 local interrupts, 2 of which are the GIC
local watchdog and count/compare timer. The remainder are CPU
interrupts which may optionally be re-routed through the GIC.
GIC hardware IRQs 0-6 are now used for local interrupts while
hardware IRQs 7+ are used for external (shared) interrupts.

Note that the 5 CPU interrupts may not be re-routable through
the GIC. In that case mapping will fail and the vectors reported
in C0_IntCtl should be used instead. gic_get_c0_compare_int() and
gic_get_c0_perfcount_int() will return the correct IRQ number to
use for the C0 timer and perfcounter interrupts based on the
routability of those interrupts through the GIC.

Malta, SEAD-3, and the GIC clockevent driver have been updated
to use local interrupts and the R4K clockevent driver has been
updated to poll for C0 timer interrupts through the GIC when
the GIC is present.

Signed-off-by: Andrew Bresticker <abrestic@xxxxxxxxxxxx>
diff --git a/arch/mips/include/asm/mips-boards/maltaint.h
b/arch/mips/include/asm/mips-boards/maltaint.h
index bdd6f39..38b06a0 100644
--- a/arch/mips/include/asm/mips-boards/maltaint.h
+++ b/arch/mips/include/asm/mips-boards/maltaint.h
@@ -10,6 +10,8 @@
#ifndef _MIPS_MALTAINT_H
#define _MIPS_MALTAINT_H
+#include <asm/gic.h>
+

nit: I think gic.h should be split to driver/irqchip/irq-mips-gic.h for
private definitions and include/linux/irqchip/irq-mips-gic.h for
exported/public definitions.
Yup, I was planning on doing this in the next series :). Malta and
the clockevent/clocksource driver need to get cleaned up first though,
so that they don't have to use the private register definitions.

diff --git a/drivers/irqchip/irq-mips-gic.c
b/drivers/irqchip/irq-mips-gic.c
index 6682a4e..3abe310 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -95,12 +96,39 @@ cycle_t gic_read_compare(void)
}
#endif
+static bool gic_local_irq_is_routable(int intr)
+{
+ u32 vpe_ctl;
+
+ /* All local interrupts are routable in EIC mode. */
+ if (cpu_has_veic)
+ return true;
+
+ GICREAD(GIC_REG(VPE_LOCAL, GIC_VPE_CTL), vpe_ctl);
+ switch (intr) {
+ case GIC_LOCAL_INT_TIMER:
+ return vpe_ctl & GIC_VPE_CTL_TIMER_RTBL_MSK;
+ case GIC_LOCAL_INT_PERFCTR:
+ return vpe_ctl & GIC_VPE_CTL_PERFCNT_RTBL_MSK;
+ case GIC_LOCAL_INT_FDC:
+ return vpe_ctl & GIC_VPE_CTL_FDC_RTBL_MSK;
+ case GIC_LOCAL_INT_SWINT0:
+ case GIC_LOCAL_INT_SWINT1:
+ /*
+ * SWINT{0,1} are not routable in non-EIC mode, regardless
+ * of the setting of SWINT_ROUTABLE.
+ */
+ return false;

Hmm AFAIK they are routable. Actually from hard reset they're automatically
routed to vpe0 pin 0 which caught me a number of times when trying to use
software interrupt on hardware that has GIC. When setting software interrupt
I was seeing pin 0 going high too and thought it's a hardware bug for a
while.
Interesting, the interAptiv User's Manual I have, in section 9.4.7.1,
says: "Note that Software Interrupts from the VPE are routed
internally by the CPU in vectored interrupt mode, and are only routed
through the GIC when the GIC is in EIC mode, regardless of the
GIC_VPEi_CTL register." IIRC, there's a similar statement in the
proAptiv manual as well. I didn't play with the SWINT bits myself, so
I wouldn't be surprised if that's wrong :).

I think all local interrupts should be masked at GIC initialisation except
for timer interrupt. I was preparing a set of patches for GIC but you beat
me into it :)
If SWINT gets routed both through the GIC and directly to the CPU,
then that's probably the best thing to do. I suspect we'll also want
to leave the performance counter interrupt unmasked too, since,
unfortunately, both the HW perf event driver and oprofile do not use
the percpu IRQ API.

Yeah probably. I haven't played much with the perf counters to be honest.

@@ -341,12 +342,54 @@ static struct irq_chip gic_edge_irq_controller = {
#endif
};
+static unsigned int gic_get_local_int(void)
+{
+ unsigned long pending, masked;
+
+ GICREAD(GIC_REG(VPE_LOCAL, GIC_VPE_PEND), pending);
+ GICREAD(GIC_REG(VPE_LOCAL, GIC_VPE_MASK), masked);
+
+ bitmap_and(&pending, &pending, &masked, GIC_NUM_LOCAL_INTRS);
+
+ return find_first_bit(&pending, GIC_NUM_LOCAL_INTRS);
+}
+
+static void gic_mask_local_irq(struct irq_data *d)
+{
+ int intr = GIC_HWIRQ_TO_LOCAL(d->hwirq);
+
+ GICWRITE(GIC_REG(VPE_LOCAL, GIC_VPE_RMASK), 1 << intr);
+}
+
+static void gic_unmask_local_irq(struct irq_data *d)
+{
+ int intr = GIC_HWIRQ_TO_LOCAL(d->hwirq);
+
+ GICWRITE(GIC_REG(VPE_LOCAL, GIC_VPE_SMASK), 1 << intr);
+}
+
+static struct irq_chip gic_local_irq_controller = {
+ .name = "MIPS GIC Local",
+ .irq_ack = gic_mask_local_irq,
+ .irq_mask = gic_mask_local_irq,
+ .irq_mask_ack = gic_mask_local_irq,
+ .irq_unmask = gic_unmask_local_irq,
+ .irq_eoi = gic_unmask_local_irq,
+};
+

again I don't think irq_ack, irq_mask_ack and irq_eoi are needed.
Yup, will do.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/