From: Matt Redfearn <matt.redfearn@imgtec.com> To: Paul Burton <paul.burton@imgtec.com> Cc: <linux-mips@linux-mips.org>, Ralf Baechle <ralf@linux-mips.org>, James Hogan <james.hogan@imgtec.com>, Marc Zyngier <marc.zyngier@arm.com>, Jason Cooper <jason@lakedaemon.net>, Thomas Gleixner <tglx@linutronix.de>, <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 2/2] irqchip/mips-gic: Fix Local compare interrupt Date: Tue, 11 Apr 2017 09:20:35 +0100 [thread overview] Message-ID: <a71e58d4-b223-7bc7-803a-937e3b6837bb@imgtec.com> (raw) In-Reply-To: <9298882.YeGiCV4jUY@np-p-burton> Hi Paul, On 10/04/17 23:06, Paul Burton wrote: > Hi Matt, > > On Friday, 31 March 2017 04:05:32 PDT Matt Redfearn wrote: >> Commit 4cfffcfa5106 ("irqchip/mips-gic: Fix local interrupts") added >> mapping of several local interrupts during initialisation of the gic >> driver. This associates virq numbers with these interrupts. >> Unfortunately, as not all of the interrupts are mapped in hardware >> order, when drivers subsequently request these interrupts they conflict >> with the mappings that have already been set up. For example, this >> manifests itself in the gic clocksource driver, which fails to probe >> with the message: >> >> clocksource: GIC: mask: 0xffffffffffffffff max_cycles: 0x7350c9738, >> max_idle_ns: 440795203769 ns >> GIC timer IRQ 25 setup failed: -22 >> >> This is because virq 25 (the correct IRQ number specified via device >> tree) was allocated to the PERFCTR interrupt (and 24 to the timer, 26 to >> the FDC). > I'm confused by this - the DT doesn't specify VIRQs, it specifies hardware IRQ > numbers. Which VIRQ is used should be irrelevant. Is this on a system using > gic_clocksource_init() from platform code? (Malta?) and therefore relying on > MIPS_GIC_IRQ_BASE? Yes, this is on Malta, which as you say, uses MIPS_GIC_IRQ_BASE. On Malta that ends up, through the definition of I8259A_IRQ_BASE and MIPS_CPU_IRQ_BASE, to be 24. Therefore hardware interrupt 1 of the GIC ends up expecting to be allocated at virq 25. But since 4cfffcfa5106, that virq number was allocated to the PERFCTR interrupt. Everything about the order-dependent and hardcoded bases of Maltas IRQs seems bad and needs looking at but this was the easiest fix for this cycle. > > If so I think this would be much more cleanly fixed by moving to probe the > clocksource using DT Not sure that would help if Maltas expected virq for this source had already been allocated? Thanks, Matt > than by adding more fragile order-dependent mappings in > the GIC driver. Perhaps we have to live with it for this cycle though... > > Thanks, > Paul > >> To fix this, map all of these local interrupts in the hardware >> order so as to associate their virq numbers with the correct hw >> interrupts. >> >> Fixes: 4cfffcfa5106 ("irqchip/mips-gic: Fix local interrupts") >> Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com> >> --- >> >> drivers/irqchip/irq-mips-gic.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c >> index 11d12bccc4e7..cd20df12d63d 100644 >> --- a/drivers/irqchip/irq-mips-gic.c >> +++ b/drivers/irqchip/irq-mips-gic.c >> @@ -991,8 +991,12 @@ static void __init gic_map_single_int(struct >> device_node *node, >> >> static void __init gic_map_interrupts(struct device_node *node) >> { >> + gic_map_single_int(node, GIC_LOCAL_INT_WD); >> + gic_map_single_int(node, GIC_LOCAL_INT_COMPARE); >> gic_map_single_int(node, GIC_LOCAL_INT_TIMER); >> gic_map_single_int(node, GIC_LOCAL_INT_PERFCTR); >> + gic_map_single_int(node, GIC_LOCAL_INT_SWINT0); >> + gic_map_single_int(node, GIC_LOCAL_INT_SWINT1); >> gic_map_single_int(node, GIC_LOCAL_INT_FDC); >> }
WARNING: multiple messages have this Message-ID (diff)
From: Matt Redfearn <matt.redfearn@imgtec.com> To: Paul Burton <paul.burton@imgtec.com> Cc: linux-mips@linux-mips.org, Ralf Baechle <ralf@linux-mips.org>, James Hogan <james.hogan@imgtec.com>, Marc Zyngier <marc.zyngier@arm.com>, Jason Cooper <jason@lakedaemon.net>, Thomas Gleixner <tglx@linutronix.de>, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] irqchip/mips-gic: Fix Local compare interrupt Date: Tue, 11 Apr 2017 09:20:35 +0100 [thread overview] Message-ID: <a71e58d4-b223-7bc7-803a-937e3b6837bb@imgtec.com> (raw) Message-ID: <20170411082035.04OklIKMdplEwHssBoPQE3rA4OlOCzHBXFcbszBrTwU@z> (raw) In-Reply-To: <9298882.YeGiCV4jUY@np-p-burton> Hi Paul, On 10/04/17 23:06, Paul Burton wrote: > Hi Matt, > > On Friday, 31 March 2017 04:05:32 PDT Matt Redfearn wrote: >> Commit 4cfffcfa5106 ("irqchip/mips-gic: Fix local interrupts") added >> mapping of several local interrupts during initialisation of the gic >> driver. This associates virq numbers with these interrupts. >> Unfortunately, as not all of the interrupts are mapped in hardware >> order, when drivers subsequently request these interrupts they conflict >> with the mappings that have already been set up. For example, this >> manifests itself in the gic clocksource driver, which fails to probe >> with the message: >> >> clocksource: GIC: mask: 0xffffffffffffffff max_cycles: 0x7350c9738, >> max_idle_ns: 440795203769 ns >> GIC timer IRQ 25 setup failed: -22 >> >> This is because virq 25 (the correct IRQ number specified via device >> tree) was allocated to the PERFCTR interrupt (and 24 to the timer, 26 to >> the FDC). > I'm confused by this - the DT doesn't specify VIRQs, it specifies hardware IRQ > numbers. Which VIRQ is used should be irrelevant. Is this on a system using > gic_clocksource_init() from platform code? (Malta?) and therefore relying on > MIPS_GIC_IRQ_BASE? Yes, this is on Malta, which as you say, uses MIPS_GIC_IRQ_BASE. On Malta that ends up, through the definition of I8259A_IRQ_BASE and MIPS_CPU_IRQ_BASE, to be 24. Therefore hardware interrupt 1 of the GIC ends up expecting to be allocated at virq 25. But since 4cfffcfa5106, that virq number was allocated to the PERFCTR interrupt. Everything about the order-dependent and hardcoded bases of Maltas IRQs seems bad and needs looking at but this was the easiest fix for this cycle. > > If so I think this would be much more cleanly fixed by moving to probe the > clocksource using DT Not sure that would help if Maltas expected virq for this source had already been allocated? Thanks, Matt > than by adding more fragile order-dependent mappings in > the GIC driver. Perhaps we have to live with it for this cycle though... > > Thanks, > Paul > >> To fix this, map all of these local interrupts in the hardware >> order so as to associate their virq numbers with the correct hw >> interrupts. >> >> Fixes: 4cfffcfa5106 ("irqchip/mips-gic: Fix local interrupts") >> Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com> >> --- >> >> drivers/irqchip/irq-mips-gic.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c >> index 11d12bccc4e7..cd20df12d63d 100644 >> --- a/drivers/irqchip/irq-mips-gic.c >> +++ b/drivers/irqchip/irq-mips-gic.c >> @@ -991,8 +991,12 @@ static void __init gic_map_single_int(struct >> device_node *node, >> >> static void __init gic_map_interrupts(struct device_node *node) >> { >> + gic_map_single_int(node, GIC_LOCAL_INT_WD); >> + gic_map_single_int(node, GIC_LOCAL_INT_COMPARE); >> gic_map_single_int(node, GIC_LOCAL_INT_TIMER); >> gic_map_single_int(node, GIC_LOCAL_INT_PERFCTR); >> + gic_map_single_int(node, GIC_LOCAL_INT_SWINT0); >> + gic_map_single_int(node, GIC_LOCAL_INT_SWINT1); >> gic_map_single_int(node, GIC_LOCAL_INT_FDC); >> }
next prev parent reply other threads:[~2017-04-11 8:20 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-03-31 11:05 [PATCH 0/2] Fix v4.11 malta_defconfig regressions Matt Redfearn 2017-03-31 11:05 ` Matt Redfearn 2017-03-31 11:05 ` [PATCH 1/2] MIPS: Malta: Fix i8259 irqchip setup Matt Redfearn 2017-03-31 11:05 ` Matt Redfearn 2017-03-31 12:49 ` Ralf Baechle 2017-03-31 12:53 ` Matt Redfearn 2017-03-31 12:53 ` Matt Redfearn 2017-03-31 11:05 ` [PATCH 2/2] irqchip/mips-gic: Fix Local compare interrupt Matt Redfearn 2017-03-31 11:05 ` Matt Redfearn 2017-03-31 12:46 ` Ralf Baechle 2017-04-10 22:06 ` Paul Burton 2017-04-10 22:06 ` Paul Burton 2017-04-11 8:20 ` Matt Redfearn [this message] 2017-04-11 8:20 ` Matt Redfearn 2017-04-11 17:56 ` Paul Burton 2017-04-11 17:56 ` Paul Burton 2017-03-31 12:04 ` [PATCH 0/2] Fix v4.11 malta_defconfig regressions Marc Zyngier 2017-03-31 12:55 ` Matt Redfearn 2017-03-31 12:55 ` Matt Redfearn 2017-03-31 13:28 ` Marc Zyngier
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=a71e58d4-b223-7bc7-803a-937e3b6837bb@imgtec.com \ --to=matt.redfearn@imgtec.com \ --cc=james.hogan@imgtec.com \ --cc=jason@lakedaemon.net \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mips@linux-mips.org \ --cc=marc.zyngier@arm.com \ --cc=paul.burton@imgtec.com \ --cc=ralf@linux-mips.org \ --cc=tglx@linutronix.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).