* [PATCH 0/4] 34xx spurious interrupts unravelling
@ 2008-10-31 19:21 Tony Lindgren
2008-10-31 19:21 ` [PATCH 1/4] Revert "Add MT_MEMORY_SO, mark L3 and L4 to use it" Tony Lindgren
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Tony Lindgren @ 2008-10-31 19:21 UTC (permalink / raw)
To: linux-omap; +Cc: Tony Lindgren
Hi all,
So it's now starting to look like we need to ensure write posting in
interrupt handlers for 34xx instead of marking the IO areas as strongly
ordered.
The first patch in the series reverts marking the IO areas as strongly
ordered.
The second debug patch also prints out the previous interrupt before
the spurious interrupt, which should help identify the interrupt
handler that needs a read back of some register.
So if you see errors like this with dspbridge:
FIXME: spurious irq 95: 0xffffffdf previous irq 26
FIXME: spurious irq 95: 0xffffffdf previous irq 26
FIXME: spurious irq 95: 0xffffffdf previous irq 26
...
You'll know that the interrupt handler for irq 26 needs a read back
of some register in the interrupt handler. Most likely you need to
read back the interrupt ack register right after writing to it in
the handler, see a later patch in this series for dspbridge.
If you get errors like this during boot:
FIXME: spurious irq 95: 0xffffffdf previous irq 25
FIXME: spurious irq 95: 0xffffffdf previous irq 25
FIXME: spurious irq 95: 0xffffffdf previous irq 37
...
Most likely we need to do a read back when initializing some devices
like dispc and gptimer. I don't have patches for these yet..
The other patches fix few interrupt handlers that I'm aware of needing
this, like i2c-omap and dspbridge. The dspbridge patch only applies
if you have it installed.
I'd appreciate if people who have been working on this issue would
torture test this series and provide feedback!
Cheers,
Tony
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/4] Revert "Add MT_MEMORY_SO, mark L3 and L4 to use it" 2008-10-31 19:21 [PATCH 0/4] 34xx spurious interrupts unravelling Tony Lindgren @ 2008-10-31 19:21 ` Tony Lindgren 2008-10-31 19:21 ` [PATCH 2/4] ARM: OMAP3: Print debug info on spurious interrupts Tony Lindgren 2008-10-31 20:07 ` [PATCH 0/4] 34xx spurious interrupts unravelling David Brownell 2008-11-01 21:14 ` Felipe Contreras 2 siblings, 1 reply; 18+ messages in thread From: Tony Lindgren @ 2008-10-31 19:21 UTC (permalink / raw) To: linux-omap; +Cc: Tony Lindgren This reverts commit acb7f883f1ec61fd4dcb840a66ddca051ad8f2ef. Marking the IO areas as strongly ordered causes performance penalties. Ensuring write posting should only be needed in few selected places. Conflicts: arch/arm/include/asm/mach/map.h Signed-off-by: Tony Lindren <tony@atomide.com> --- arch/arm/include/asm/mach/map.h | 18 ++++++++++-------- arch/arm/mach-omap2/io.c | 12 ++++++------ arch/arm/mm/mmu.c | 4 ---- 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/arch/arm/include/asm/mach/map.h b/arch/arm/include/asm/mach/map.h index fa0e695..9eb936e 100644 --- a/arch/arm/include/asm/mach/map.h +++ b/arch/arm/include/asm/mach/map.h @@ -18,14 +18,16 @@ struct map_desc { unsigned int type; }; -/* types 0-3 are defined in asm/io.h */ -#define MT_CACHECLEAN 4 -#define MT_MINICLEAN 5 -#define MT_LOW_VECTORS 6 -#define MT_HIGH_VECTORS 7 -#define MT_MEMORY 8 -#define MT_ROM 9 -#define MT_MEMORY_SO 10 +/* types 0-4 are defined in asm/io.h */ +#define MT_CACHECLEAN 5 +#define MT_MINICLEAN 6 +#define MT_LOW_VECTORS 7 +#define MT_HIGH_VECTORS 8 +#define MT_MEMORY 9 +#define MT_ROM 10 + +#define MT_NONSHARED_DEVICE MT_DEVICE_NONSHARED +#define MT_IXP2000_DEVICE MT_DEVICE_IXP2000 #ifdef CONFIG_MMU extern void iotable_init(struct map_desc *, int); diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c index c11c0e8..adbe21f 100644 --- a/arch/arm/mach-omap2/io.c +++ b/arch/arm/mach-omap2/io.c @@ -119,13 +119,13 @@ static struct map_desc omap34xx_io_desc[] __initdata = { .virtual = L3_34XX_VIRT, .pfn = __phys_to_pfn(L3_34XX_PHYS), .length = L3_34XX_SIZE, - .type = MT_MEMORY_SO + .type = MT_DEVICE }, { .virtual = L4_34XX_VIRT, .pfn = __phys_to_pfn(L4_34XX_PHYS), .length = L4_34XX_SIZE, - .type = MT_MEMORY_SO + .type = MT_DEVICE }, { .virtual = L4_WK_34XX_VIRT, @@ -137,19 +137,19 @@ static struct map_desc omap34xx_io_desc[] __initdata = { .virtual = OMAP34XX_GPMC_VIRT, .pfn = __phys_to_pfn(OMAP34XX_GPMC_PHYS), .length = OMAP34XX_GPMC_SIZE, - .type = MT_MEMORY_SO + .type = MT_DEVICE }, { .virtual = OMAP343X_SMS_VIRT, .pfn = __phys_to_pfn(OMAP343X_SMS_PHYS), .length = OMAP343X_SMS_SIZE, - .type = MT_MEMORY_SO + .type = MT_DEVICE }, { .virtual = OMAP343X_SDRC_VIRT, .pfn = __phys_to_pfn(OMAP343X_SDRC_PHYS), .length = OMAP343X_SDRC_SIZE, - .type = MT_MEMORY_SO + .type = MT_DEVICE }, { .virtual = L4_PER_34XX_VIRT, @@ -161,7 +161,7 @@ static struct map_desc omap34xx_io_desc[] __initdata = { .virtual = L4_EMU_34XX_VIRT, .pfn = __phys_to_pfn(L4_EMU_34XX_PHYS), .length = L4_EMU_34XX_SIZE, - .type = MT_MEMORY_SO + .type = MT_DEVICE }, }; #endif diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c index 64c5451..8ba7540 100644 --- a/arch/arm/mm/mmu.c +++ b/arch/arm/mm/mmu.c @@ -236,10 +236,6 @@ static struct mem_type mem_types[] = { .prot_sect = PMD_TYPE_SECT, .domain = DOMAIN_KERNEL, }, - [MT_MEMORY_SO] = { - .prot_sect = PMD_TYPE_SECT | PMD_SECT_AP_WRITE | PMD_SECT_UNCACHED, - .domain = DOMAIN_KERNEL, - }, }; const struct mem_type *get_mem_type(unsigned int type) -- 1.5.6.rc3.21.g8c6b5 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/4] ARM: OMAP3: Print debug info on spurious interrupts 2008-10-31 19:21 ` [PATCH 1/4] Revert "Add MT_MEMORY_SO, mark L3 and L4 to use it" Tony Lindgren @ 2008-10-31 19:21 ` Tony Lindgren 2008-10-31 19:21 ` [PATCH 3/4] I2C: Ensure write posting for critical i2c-omap writes Tony Lindgren 2008-10-31 20:55 ` [PATCH 2/4] ARM: OMAP3: Print debug info on spurious interrupts Tony Lindgren 0 siblings, 2 replies; 18+ messages in thread From: Tony Lindgren @ 2008-10-31 19:21 UTC (permalink / raw) To: linux-omap; +Cc: Tony Lindgren In the case of spurious interrupt, the handler for previous irq most likely needs a read back of some register to ensure write posting. Signed-off-by: Tony Lindgren <tony@atomide.com> --- arch/arm/mach-omap2/irq.c | 22 ++++++++++++++++++++++ 1 files changed, 22 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c index 68aff9a..5a3b1f1 100644 --- a/arch/arm/mach-omap2/irq.c +++ b/arch/arm/mach-omap2/irq.c @@ -23,6 +23,7 @@ #define INTC_REVISION 0x0000 #define INTC_SYSCONFIG 0x0010 #define INTC_SYSSTATUS 0x0014 +#define INTC_SIR 0x0040 #define INTC_CONTROL 0x0048 #define INTC_MIR_CLEAR0 0x0088 #define INTC_MIR_SET0 0x008c @@ -60,6 +61,24 @@ static u32 intc_bank_read_reg(struct omap_irq_bank *bank, u16 reg) return __raw_readl(bank->base_reg + reg); } +static int previous_irq; + +static int omap_check_spurious(unsigned int irq) +{ + u32 sir, spurious; + + sir = intc_bank_read_reg(&irq_banks[0], INTC_SIR); + spurious = sir >> 6; + + if (spurious > 1) { + printk("FIXME: spurious irq %i: 0x%08x previous irq %i\n", + irq, sir, previous_irq); + return spurious; + } + + return 0; +} + /* XXX: FIQ and additional INTC support (only MPU at the moment) */ static void omap_ack_irq(unsigned int irq) { @@ -70,6 +89,9 @@ static void omap_mask_irq(unsigned int irq) { int offset = irq & (~(IRQ_BITS_PER_REG - 1)); + if (cpu_is_omap34xx() && !omap_check_spurious(irq)) + previous_irq = irq; + irq &= (IRQ_BITS_PER_REG - 1); intc_bank_write_reg(1 << irq, &irq_banks[0], INTC_MIR_SET0 + offset); -- 1.5.6.rc3.21.g8c6b5 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/4] I2C: Ensure write posting for critical i2c-omap writes 2008-10-31 19:21 ` [PATCH 2/4] ARM: OMAP3: Print debug info on spurious interrupts Tony Lindgren @ 2008-10-31 19:21 ` Tony Lindgren 2008-10-31 19:21 ` [PATCH 4/4] DSPBRIDGE: Ensure write posting when acking mailbox irq Tony Lindgren 2008-10-31 21:03 ` [PATCH 3/4] I2C: Ensure write posting for critical i2c-omap writes Tony Lindgren 2008-10-31 20:55 ` [PATCH 2/4] ARM: OMAP3: Print debug info on spurious interrupts Tony Lindgren 1 sibling, 2 replies; 18+ messages in thread From: Tony Lindgren @ 2008-10-31 19:21 UTC (permalink / raw) To: linux-omap; +Cc: Tony Lindgren Otherwise we may race, or will get spurious I2C interrupts: i2c_omap.1: XDR IRQ while no data to send ... Signed-off-by: Tony Lindgren <tony@atomide.com> --- drivers/i2c/busses/i2c-omap.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index a999606..8468280 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -689,6 +689,9 @@ omap_i2c_isr(int this_irq, void *dev_id) break; } omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w); + + /* Ensure write posting of the write */ + omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG); } omap_i2c_ack_stat(dev, stat & (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR)); -- 1.5.6.rc3.21.g8c6b5 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/4] DSPBRIDGE: Ensure write posting when acking mailbox irq 2008-10-31 19:21 ` [PATCH 3/4] I2C: Ensure write posting for critical i2c-omap writes Tony Lindgren @ 2008-10-31 19:21 ` Tony Lindgren 2008-10-31 21:06 ` Tony Lindgren 2008-11-01 3:43 ` Woodruff, Richard 2008-10-31 21:03 ` [PATCH 3/4] I2C: Ensure write posting for critical i2c-omap writes Tony Lindgren 1 sibling, 2 replies; 18+ messages in thread From: Tony Lindgren @ 2008-10-31 19:21 UTC (permalink / raw) To: linux-omap; +Cc: Tony Lindgren The only way to ensure write posting to L4 bus is to do a read back of the same register right after the write. This seems to be mostly needed in interrupt handlers to avoid causing spurious interrupts. The earlier fix has been to mark the L4 bus as strongly ordered memory, which solves the problem, but causes performance penalties. Similar fixes may be needed in other interrupt handlers too. Signed-off-by: Tony Lindgren <tony@atomide.com> diff --git a/drivers/dsp/bridge/hw/hw_mbox.c b/drivers/dsp/bridge/hw/hw_mbox.c index 7055259..96b5680 100644 --- a/drivers/dsp/bridge/hw/hw_mbox.c +++ b/drivers/dsp/bridge/hw/hw_mbox.c @@ -251,5 +251,20 @@ HW_STATUS HW_MBOX_EventAck(const u32 baseAddress, const HW_MBOX_Id_t mailBoxId, MLBMAILBOX_IRQSTATUS___0_3WriteRegister32(baseAddress, (u32)userId, (u32)irqStatusReg); + /* + * FIXME: Replace all this custom register access with standard + * __raw_read/write(). + * + * FIXME: Replace all interrupt handlers with standard linux style + * interrupt handlers. + * + * FIXME: Replace direct access to PRCM registers with omap standard + * PRCM register access. + * + * Do a read back for the irq status to ensure the write above gets + * posted to avoid spurious interrupts. + */ + MLBMAILBOX_IRQSTATUS___0_3ReadRegister32(baseAddress, (u32)userId); + return status; } ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 4/4] DSPBRIDGE: Ensure write posting when acking mailbox irq 2008-10-31 19:21 ` [PATCH 4/4] DSPBRIDGE: Ensure write posting when acking mailbox irq Tony Lindgren @ 2008-10-31 21:06 ` Tony Lindgren 2008-11-01 3:43 ` Woodruff, Richard 1 sibling, 0 replies; 18+ messages in thread From: Tony Lindgren @ 2008-10-31 21:06 UTC (permalink / raw) To: linux-omap [-- Attachment #1: Type: text/plain, Size: 548 bytes --] * Tony Lindgren <tony@atomide.com> [081031 12:21]: > The only way to ensure write posting to L4 bus is to do a read back > of the same register right after the write. > > This seems to be mostly needed in interrupt handlers to avoid > causing spurious interrupts. > > The earlier fix has been to mark the L4 bus as strongly ordered > memory, which solves the problem, but causes performance penalties. > > Similar fixes may be needed in other interrupt handlers too. Here's this one updated to say "flush posted write" for easy grepping. Tony [-- Attachment #2: flush-dspbridge-v2.patch --] [-- Type: text/x-diff, Size: 1531 bytes --] >From 0124b6a0f24e3d8a305e61d897a9866903b12485 Mon Sep 17 00:00:00 2001 From: Tony Lindgren <tony@atomide.com> Date: Fri, 31 Oct 2008 12:02:57 -0700 Subject: [PATCH] DSPBRIDGE: Flush posted write when acking mailbox irq The only way to flush posted write to L4 bus is to do a read back of the same register right after the write. This seems to be mostly needed in interrupt handlers to avoid causing spurious interrupts. The earlier fix has been to mark the L4 bus as strongly ordered memory, which solves the problem, but causes performance penalties. Similar fixes may be needed in other interrupt handlers too. Signed-off-by: Tony Lindgren <tony@atomide.com> diff --git a/drivers/dsp/bridge/hw/hw_mbox.c b/drivers/dsp/bridge/hw/hw_mbox.c index 7055259..e07e5a7 100644 --- a/drivers/dsp/bridge/hw/hw_mbox.c +++ b/drivers/dsp/bridge/hw/hw_mbox.c @@ -251,5 +251,19 @@ HW_STATUS HW_MBOX_EventAck(const u32 baseAddress, const HW_MBOX_Id_t mailBoxId, MLBMAILBOX_IRQSTATUS___0_3WriteRegister32(baseAddress, (u32)userId, (u32)irqStatusReg); + /* + * FIXME: Replace all this custom register access with standard + * __raw_read/write(). + * + * FIXME: Replace all interrupt handlers with standard linux style + * interrupt handlers. + * + * FIXME: Replace direct access to PRCM registers with omap standard + * PRCM register access. + * + * Flush posted write for the irq status to avoid spurious interrupts. + */ + MLBMAILBOX_IRQSTATUS___0_3ReadRegister32(baseAddress, (u32)userId); + return status; } ^ permalink raw reply related [flat|nested] 18+ messages in thread
* RE: [PATCH 4/4] DSPBRIDGE: Ensure write posting when acking mailbox irq 2008-10-31 19:21 ` [PATCH 4/4] DSPBRIDGE: Ensure write posting when acking mailbox irq Tony Lindgren 2008-10-31 21:06 ` Tony Lindgren @ 2008-11-01 3:43 ` Woodruff, Richard 2008-11-02 22:10 ` Tony Lindgren 1 sibling, 1 reply; 18+ messages in thread From: Woodruff, Richard @ 2008-11-01 3:43 UTC (permalink / raw) To: Tony Lindgren, linux-omap@vger.kernel.org > owner@vger.kernel.org] On Behalf Of Tony Lindgren > Sent: Friday, October 31, 2008 2:21 PM > The only way to ensure write posting to L4 bus is to do a read back > of the same register right after the write. > > This seems to be mostly needed in interrupt handlers to avoid > causing spurious interrupts. > > The earlier fix has been to mark the L4 bus as strongly ordered > memory, which solves the problem, but causes performance penalties. What penalties have you observed? Can you quantify? >From the L4 perspectives DEVICE and SO are similar. Long back I was told one difference is DEVICE is allowed to do burst transactions of element size where SO was not. This behavior is only really wanted to a FIFO. Really performance sensitive devices will be using DMA to FIFOs. SO/DEVICE only applies to the ARM's view of things. DMA is not affected by ARM memory types. Some kind of barrier or read back is needed for sure when dealing with the main interrupt controller. Regards, Richard W. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/4] DSPBRIDGE: Ensure write posting when acking mailbox irq 2008-11-01 3:43 ` Woodruff, Richard @ 2008-11-02 22:10 ` Tony Lindgren 2008-11-03 18:54 ` Tony Lindgren 0 siblings, 1 reply; 18+ messages in thread From: Tony Lindgren @ 2008-11-02 22:10 UTC (permalink / raw) To: Woodruff, Richard; +Cc: linux-omap@vger.kernel.org * Woodruff, Richard <r-woodruff2@ti.com> [081031 20:44]: > > owner@vger.kernel.org] On Behalf Of Tony Lindgren > > Sent: Friday, October 31, 2008 2:21 PM > > > The only way to ensure write posting to L4 bus is to do a read back > > of the same register right after the write. > > > > This seems to be mostly needed in interrupt handlers to avoid > > causing spurious interrupts. > > > > The earlier fix has been to mark the L4 bus as strongly ordered > > memory, which solves the problem, but causes performance penalties. > > What penalties have you observed? Can you quantify? Not yet, I guess we can run some benchmarks though. > From the L4 perspectives DEVICE and SO are similar. Long back I was told one difference is DEVICE is allowed to do burst transactions of element size where SO was not. This behavior is only really wanted to a FIFO. > > Really performance sensitive devices will be using DMA to FIFOs. SO/DEVICE only applies to the ARM's view of things. DMA is not affected by ARM memory types. You may be right, and if that's the only difference, then SO might be even faster as it avoids the extra readbacks. > Some kind of barrier or read back is needed for sure when dealing with the main interrupt controller. Yeah. I'm worried that these issues could happen with SO too.. Regards, Tony ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/4] DSPBRIDGE: Ensure write posting when acking mailbox irq 2008-11-02 22:10 ` Tony Lindgren @ 2008-11-03 18:54 ` Tony Lindgren 0 siblings, 0 replies; 18+ messages in thread From: Tony Lindgren @ 2008-11-03 18:54 UTC (permalink / raw) To: Woodruff, Richard; +Cc: linux-omap@vger.kernel.org [-- Attachment #1: Type: text/plain, Size: 1488 bytes --] * Tony Lindgren <tony@atomide.com> [081102 14:11]: > * Woodruff, Richard <r-woodruff2@ti.com> [081031 20:44]: > > > owner@vger.kernel.org] On Behalf Of Tony Lindgren > > > Sent: Friday, October 31, 2008 2:21 PM > > > > > The only way to ensure write posting to L4 bus is to do a read back > > > of the same register right after the write. > > > > > > This seems to be mostly needed in interrupt handlers to avoid > > > causing spurious interrupts. > > > > > > The earlier fix has been to mark the L4 bus as strongly ordered > > > memory, which solves the problem, but causes performance penalties. > > > > What penalties have you observed? Can you quantify? > > Not yet, I guess we can run some benchmarks though. > > > From the L4 perspectives DEVICE and SO are similar. Long back I was told one difference is DEVICE is allowed to do burst transactions of element size where SO was not. This behavior is only really wanted to a FIFO. > > > > Really performance sensitive devices will be using DMA to FIFOs. SO/DEVICE only applies to the ARM's view of things. DMA is not affected by ARM memory types. > > You may be right, and if that's the only difference, then SO might be > even faster as it avoids the extra readbacks. > > > Some kind of barrier or read back is needed for sure when dealing with the main interrupt controller. > > Yeah. I'm worried that these issues could happen with SO too.. And here's the fix copied from the LAK mailing list. > Regards, > > Tony [-- Attachment #2: apply --] [-- Type: text/plain, Size: 12010 bytes --] Return-Path: <linux+tony=atomide.com@arm.linux.org.uk> X-Original-To: tony@atomide.com Delivered-To: tmlind@muru.com Received: from localhost (localhost [127.0.0.1]) by muru.com (Postfix) with ESMTP id 501906B81 for <tony@atomide.com>; Mon, 3 Nov 2008 18:20:48 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at muru.com Received: from muru.com ([127.0.0.1]) by localhost (muru.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id rnXeC0yRM67G for <tony@atomide.com>; Mon, 3 Nov 2008 18:20:37 +0000 (UTC) Received: from caramon.arm.linux.org.uk (caramon.arm.linux.org.uk [78.32.30.218]) by muru.com (Postfix) with ESMTP id 8E3D26B7F for <tony@atomide.com>; Mon, 3 Nov 2008 18:20:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=arm.linux.org.uk; s=caramon; h=Date:From:To:Cc:Subject: Message-ID:References:Mime-Version:Content-Type:In-Reply-To: Sender; bh=E7T+sNNhAMVrmIeJvqIeD2sTnYDVvwn66HW3FXwY+tE=; b=LRToU WyC/1QDPYX1Kc53XPddYuWrtGX2Hc4JTuxnsGQhtQJ5BgcTYu+O+rSO+LFOXBpMA 6NGqRODq5XguA9DAhRmAA3cGyeOZ0S2KgmQa/0cnx0+/O8n/5H4+CYg8dchHe3s4 Bm0F2mMf4hf/7Z3557TwH+zfnsu57ppmTNWMYg= Received: from flint.arm.linux.org.uk ([2002:4e20:1eda:1:201:2ff:fe14:8fad]) by caramon.arm.linux.org.uk with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.69) (envelope-from <linux@arm.linux.org.uk>) id 1Kx42M-0005Zg-RF; Mon, 03 Nov 2008 18:20:27 +0000 Received: from linux by flint.arm.linux.org.uk with local (Exim 4.69) (envelope-from <linux@flint.arm.linux.org.uk>) id 1Kx42H-0001pA-No; Mon, 03 Nov 2008 18:20:21 +0000 Date: Mon, 3 Nov 2008 18:20:20 +0000 From: Russell King - ARM Linux <linux@arm.linux.org.uk> To: Tony Lindgren <tony@atomide.com>, linux-arm-kernel@lists.arm.linux.org.uk Cc: Catalin Marinas <catalin.marinas@arm.com>, Andrew Morton <akpm@linux-foundation.org> Subject: Re: [CFT] ALL ARM PLATFORMS AND ARM CPUS: Fix ARMv7 memory typing (was: Current omap hsmmc patch pile) Message-ID: <20081103182019.GC16696@flint.arm.linux.org.uk> References: <20081031163102.GE13227@atomide.com> <20081102205506.GL28924@atomide.com> <20081103113643.GA12544@flint.arm.linux.org.uk> <20081103114922.GA12622@flint.arm.linux.org.uk> <20081103133031.GA26993@flint.arm.linux.org.uk> <1225720473.18781.38.camel@pc1117.cambridge.arm.com> <20081103135955.GC12544@flint.arm.linux.org.uk> <1225723480.18781.65.camel@pc1117.cambridge.arm.com> <20081103150810.GD12544@flint.arm.linux.org.uk> <20081103164628.GU28924@atomide.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20081103164628.GU28924@atomide.com> User-Agent: Mutt/1.4.2.1i Sender: Russell King - ARM Linux <linux@arm.linux.org.uk> X-Label: [-TML-] [LAK added] * Russell King - ARM Linux <linux@arm.linux.org.uk> [081103 07:09]: > Solving ARMv7 seems to be fairly simple, at the expense of making > build_mem_types_table() slightly more complex. If that was the only > problem, then I wouldn't be mentioning the idea of dropping the > patchset. Well, this is the fix for ARMv7 (and a few others). In making these changes, I went back to DDI0100I (ARMv6 ARM), DDI0406A (ARMv7 ARM) and the Marvell Xscale3 documentation. I rather wish that this patch was smaller, but that would mean making build_mem_types_table() even harder to read, which would be a mistake given its complexity. I noticed that coherent Xscale3 was setting the shared PTE bit for kernel memory mappings - we never map kernel memory using PTEs, so that's been killed. This solves the issue Tony reported with the UART for me on the OMAP3 LDP platform. I haven't yet tested this on anything else - and it does need testing on other CPUs. The most important thing to do is to manually check the bit combinations - which is why this patch will dump them out. That dumping will of course be removed in the final version. I do not expect the issues which mkp has reported to be affected by this patch. Nevertheless, can as many people as possible test this please. diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h index 7aad784..568020b 100644 --- a/arch/arm/include/asm/system.h +++ b/arch/arm/include/asm/system.h @@ -42,6 +42,10 @@ #define CR_U (1 << 22) /* Unaligned access operation */ #define CR_XP (1 << 23) /* Extended page tables */ #define CR_VE (1 << 24) /* Vectored interrupts */ +#define CR_EE (1 << 25) /* Exception (Big) Endian */ +#define CR_TRE (1 << 28) /* TEX remap enable */ +#define CR_AFE (1 << 29) /* Access flag enable */ +#define CR_TE (1 << 30) /* Thumb exception enable */ /* * This is used to ensure the compiler did actually allocate the register we diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c index 8ba7540..96b9531 100644 --- a/arch/arm/mm/mmu.c +++ b/arch/arm/mm/mmu.c @@ -180,20 +180,20 @@ void adjust_cr(unsigned long mask, unsigned long set) #endif #define PROT_PTE_DEVICE L_PTE_PRESENT|L_PTE_YOUNG|L_PTE_DIRTY|L_PTE_WRITE -#define PROT_SECT_DEVICE PMD_TYPE_SECT|PMD_SECT_XN|PMD_SECT_AP_WRITE +#define PROT_SECT_DEVICE PMD_TYPE_SECT|PMD_SECT_AP_WRITE static struct mem_type mem_types[] = { [MT_DEVICE] = { /* Strongly ordered / ARMv6 shared device */ .prot_pte = PROT_PTE_DEVICE | L_PTE_MT_DEV_SHARED | L_PTE_SHARED, .prot_l1 = PMD_TYPE_TABLE, - .prot_sect = PROT_SECT_DEVICE | PMD_SECT_UNCACHED, + .prot_sect = PROT_SECT_DEVICE | PMD_SECT_S, .domain = DOMAIN_IO, }, [MT_DEVICE_NONSHARED] = { /* ARMv6 non-shared device */ .prot_pte = PROT_PTE_DEVICE | L_PTE_MT_DEV_NONSHARED, .prot_l1 = PMD_TYPE_TABLE, - .prot_sect = PROT_SECT_DEVICE | PMD_SECT_TEX(2), + .prot_sect = PROT_SECT_DEVICE, .domain = DOMAIN_IO, }, [MT_DEVICE_CACHED] = { /* ioremap_cached */ @@ -205,7 +205,7 @@ static struct mem_type mem_types[] = { [MT_DEVICE_WC] = { /* ioremap_wc */ .prot_pte = PROT_PTE_DEVICE | L_PTE_MT_DEV_WC, .prot_l1 = PMD_TYPE_TABLE, - .prot_sect = PROT_SECT_DEVICE | PMD_SECT_BUFFERABLE, + .prot_sect = PROT_SECT_DEVICE, .domain = DOMAIN_IO, }, [MT_CACHECLEAN] = { @@ -273,22 +273,23 @@ static void __init build_mem_type_table(void) #endif /* - * On non-Xscale3 ARMv5-and-older systems, use CB=01 - * (Uncached/Buffered) for ioremap_wc() mappings. On XScale3 - * and ARMv6+, use TEXCB=00100 mappings (Inner/Outer Uncacheable - * in xsc3 parlance, Uncached Normal in ARMv6 parlance). + * Strip out features not present on earlier architectures. + * Pre-ARMv5 CPUs don't have TEX bits. Pre-ARMv6 CPUs or those + * without extended page tables don't have the 'Shared' bit. */ - if (cpu_is_xsc3() || cpu_arch >= CPU_ARCH_ARMv6) { - mem_types[MT_DEVICE_WC].prot_sect |= PMD_SECT_TEX(1); - mem_types[MT_DEVICE_WC].prot_sect &= ~PMD_SECT_BUFFERABLE; - } + if (cpu_arch < CPU_ARCH_ARMv5) + for (i = 0; i < ARRAY_SIZE(mem_types); i++) + mem_types[i].prot_sect &= ~PMD_SECT_TEX(7); + if (cpu_arch < CPU_ARCH_ARMv6 || !(cr & CR_XP)) + for (i = 0; i < ARRAY_SIZE(mem_types); i++) + mem_types[i].prot_sect &= ~PMD_SECT_S; /* - * ARMv5 and lower, bit 4 must be set for page tables. - * (was: cache "update-able on write" bit on ARM610) - * However, Xscale cores require this bit to be cleared. + * ARMv5 and lower, bit 4 must be set for page tables (was: cache + * "update-able on write" bit on ARM610). However, Xscale and + * Xscale3 require this bit to be cleared. */ - if (cpu_is_xscale()) { + if (cpu_is_xscale() || cpu_is_xsc3()) { for (i = 0; i < ARRAY_SIZE(mem_types); i++) { mem_types[i].prot_sect &= ~PMD_BIT4; mem_types[i].prot_l1 &= ~PMD_BIT4; @@ -302,6 +303,54 @@ static void __init build_mem_type_table(void) } } + /* + * Mark the device areas according to the CPU/architecture. + */ + if (cpu_is_xsc3() || (cpu_arch >= CPU_ARCH_ARMv6 && (cr & CR_XP))) { + if (!cpu_is_xsc3()) { + /* + * Mark device regions on ARMv6+ as execute-never + * to prevent speculative instruction fetches. + */ + mem_types[MT_DEVICE].prot_sect |= PMD_SECT_XN; + mem_types[MT_DEVICE_NONSHARED].prot_sect |= PMD_SECT_XN; + mem_types[MT_DEVICE_CACHED].prot_sect |= PMD_SECT_XN; + mem_types[MT_DEVICE_WC].prot_sect |= PMD_SECT_XN; + } + if (cpu_arch >= CPU_ARCH_ARMv7 && (cr & CR_TRE)) { + /* + * For ARMv7 with TEX remapping, + * - shared device is SXCB=1100 + * - nonshared device is SXCB=0100 + * - write combine device mem is SXCB=0001 + * (Uncached Normal memory) + */ + mem_types[MT_DEVICE].prot_sect |= PMD_SECT_TEX(1); + mem_types[MT_DEVICE_NONSHARED].prot_sect |= PMD_SECT_TEX(1); + mem_types[MT_DEVICE_WC].prot_sect |= PMD_SECT_BUFFERABLE; + } else { + /* + * For Xscale3, ARMv6 and ARMv7 without TEX remapping, + * - shared device is TEXCB=00001 + * - nonshared device is TEXCB=01000 + * - write combine device mem is TEXCB=00100 + * (Inner/Outer Uncacheable in xsc3 parlance, Uncached + * Normal in ARMv6 parlance). + */ + mem_types[MT_DEVICE].prot_sect |= PMD_SECT_BUFFERED; + mem_types[MT_DEVICE_NONSHARED].prot_sect |= PMD_SECT_TEX(2); + mem_types[MT_DEVICE_WC].prot_sect |= PMD_SECT_TEX(1); + } + } else { + /* + * On others, write combining is "Uncached/Buffered" + */ + mem_types[MT_DEVICE_WC].prot_sect |= PMD_SECT_BUFFERABLE; + } + + /* + * Now deal with the memory-type mappings + */ cp = &cache_policies[cachepolicy]; vecs_pgprot = kern_pgprot = user_pgprot = cp->pte; @@ -317,12 +366,8 @@ static void __init build_mem_type_table(void) * Enable CPU-specific coherency if supported. * (Only available on XSC3 at the moment.) */ - if (arch_is_coherent()) { - if (cpu_is_xsc3()) { - mem_types[MT_MEMORY].prot_sect |= PMD_SECT_S; - mem_types[MT_MEMORY].prot_pte |= L_PTE_SHARED; - } - } + if (arch_is_coherent() && cpu_is_xsc3()) + mem_types[MT_MEMORY].prot_sect |= PMD_SECT_S; /* * ARMv6 and above have extended page tables. @@ -336,11 +381,6 @@ static void __init build_mem_type_table(void) mem_types[MT_MINICLEAN].prot_sect |= PMD_SECT_APX|PMD_SECT_AP_WRITE; mem_types[MT_CACHECLEAN].prot_sect |= PMD_SECT_APX|PMD_SECT_AP_WRITE; - /* - * Mark the device area as "shared device" - */ - mem_types[MT_DEVICE].prot_sect |= PMD_SECT_BUFFERED; - #ifdef CONFIG_SMP /* * Mark memory with the "shared" attribute for SMP systems @@ -360,9 +400,6 @@ static void __init build_mem_type_table(void) mem_types[MT_LOW_VECTORS].prot_pte |= vecs_pgprot; mem_types[MT_HIGH_VECTORS].prot_pte |= vecs_pgprot; - if (cpu_arch < CPU_ARCH_ARMv5) - mem_types[MT_MINICLEAN].prot_sect &= ~PMD_SECT_TEX(1); - pgprot_user = __pgprot(L_PTE_PRESENT | L_PTE_YOUNG | user_pgprot); pgprot_kernel = __pgprot(L_PTE_PRESENT | L_PTE_YOUNG | L_PTE_DIRTY | L_PTE_WRITE | @@ -387,6 +424,22 @@ static void __init build_mem_type_table(void) for (i = 0; i < ARRAY_SIZE(mem_types); i++) { struct mem_type *t = &mem_types[i]; + const char *s; +#define T(n) if (i == (n)) s = #n; + s = "???"; + T(MT_DEVICE); + T(MT_DEVICE_NONSHARED); + T(MT_DEVICE_CACHED); + T(MT_DEVICE_WC); + T(MT_CACHECLEAN); + T(MT_MINICLEAN); + T(MT_LOW_VECTORS); + T(MT_HIGH_VECTORS); + T(MT_MEMORY); + T(MT_ROM); + printk(KERN_INFO "%-19s: DOM=%#3x S=%#010x L1=%#010x P=%#010x\n", + s, t->domain, t->prot_sect, t->prot_l1, t->prot_pte); + if (t->prot_l1) t->prot_l1 |= PMD_DOMAIN(t->domain); if (t->prot_sect) diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S index 07f82db..f1d158f 100644 --- a/arch/arm/mm/proc-v7.S +++ b/arch/arm/mm/proc-v7.S @@ -192,11 +192,11 @@ __v7_setup: mov pc, lr @ return to head.S:__ret ENDPROC(__v7_setup) - /* - * V X F I D LR - * .... ...E PUI. .T.T 4RVI ZFRS BLDP WCAM - * rrrr rrrx xxx0 0101 xxxx xxxx x111 xxxx < forced - * 0 110 0011 1.00 .111 1101 < we want + /* AT + * TFR EV X F I D LR + * .EEE ..EE PUI. .T.T 4RVI ZFRS BLDP WCAM + * rxxx rrxx xxx0 0101 xxxx xxxx x111 xxxx < forced + * 1 0 110 0011 1.00 .111 1101 < we want */ .type v7_crval, #object v7_crval: ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] I2C: Ensure write posting for critical i2c-omap writes 2008-10-31 19:21 ` [PATCH 3/4] I2C: Ensure write posting for critical i2c-omap writes Tony Lindgren 2008-10-31 19:21 ` [PATCH 4/4] DSPBRIDGE: Ensure write posting when acking mailbox irq Tony Lindgren @ 2008-10-31 21:03 ` Tony Lindgren 1 sibling, 0 replies; 18+ messages in thread From: Tony Lindgren @ 2008-10-31 21:03 UTC (permalink / raw) To: linux-omap [-- Attachment #1: Type: text/plain, Size: 291 bytes --] * Tony Lindgren <tony@atomide.com> [081031 12:21]: > Otherwise we may race, or will get spurious I2C interrupts: > > i2c_omap.1: XDR IRQ while no data to send > ... Here's an updated version of this to say "flush posted write" for easy grepping. Also change previous wmb() to flush. Tony [-- Attachment #2: flush-i2c.patch --] [-- Type: text/x-diff, Size: 1622 bytes --] >From df5cf408d3a59a892ba8ff25feed402fdeb53a0d Mon Sep 17 00:00:00 2001 From: Tony Lindgren <tony@atomide.com> Date: Fri, 31 Oct 2008 13:56:16 -0700 Subject: [PATCH] I2C: Flush posted write for i2c-omap Otherwise we may race, or will get spurious I2C interrupts: i2c_omap.1: XDR IRQ while no data to send ... Also replace previous wmb() with a readback to flush posted write there too. Signed-off-by: Tony Lindgren <tony@atomide.com> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index a999606..6d40781 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -220,16 +220,14 @@ static void omap_i2c_idle(struct omap_i2c_dev *dev) dev->iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG); omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, 0); - if (dev->rev1) + if (dev->rev1) { iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG); /* Read clears */ - else + } else { omap_i2c_write_reg(dev, OMAP_I2C_STAT_REG, dev->iestate); - /* - * The wmb() is to ensure that the I2C interrupt mask write - * reaches the I2C controller before the dev->idle store - * occurs. - */ - wmb(); + + /* Flush posted write for interrupt mask before dev->idle = 1 */ + omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG); + } dev->idle = 1; clk_disable(dev->fclk); if (dev->iclk != NULL) @@ -689,6 +687,9 @@ omap_i2c_isr(int this_irq, void *dev_id) break; } omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w); + + /* Flush posted write */ + omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG); } omap_i2c_ack_stat(dev, stat & (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR)); ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] ARM: OMAP3: Print debug info on spurious interrupts 2008-10-31 19:21 ` [PATCH 2/4] ARM: OMAP3: Print debug info on spurious interrupts Tony Lindgren 2008-10-31 19:21 ` [PATCH 3/4] I2C: Ensure write posting for critical i2c-omap writes Tony Lindgren @ 2008-10-31 20:55 ` Tony Lindgren 1 sibling, 0 replies; 18+ messages in thread From: Tony Lindgren @ 2008-10-31 20:55 UTC (permalink / raw) To: linux-omap [-- Attachment #1: Type: text/plain, Size: 403 bytes --] * Tony Lindgren <tony@atomide.com> [081031 12:21]: > In the case of spurious interrupt, the handler for previous irq most likely > needs a read back of some register to ensure write posting. Here's a better version to kill the checkpatch.pl warning. Later on we might want to add also a check for "irq == 95" to the test in omap_mask_irq() so we can keep this patch around with minimal overhead. Tony [-- Attachment #2: irq-debug-v2.patch --] [-- Type: text/x-diff, Size: 1745 bytes --] >From 7410bd58cbf8b9d60a7b368a73b34fe03cdaa086 Mon Sep 17 00:00:00 2001 From: Tony Lindgren <tony@atomide.com> Date: Fri, 31 Oct 2008 12:01:25 -0700 Subject: [PATCH] ARM: OMAP3: Print debug info on spurious interrupts In the case of spurious interrupt, the handler for previous irq most likely needs to flush posted writes with a read back of the interrupt ack register. Signed-off-by: Tony Lindgren <tony@atomide.com> diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c index 68aff9a..07aa0f5 100644 --- a/arch/arm/mach-omap2/irq.c +++ b/arch/arm/mach-omap2/irq.c @@ -23,6 +23,7 @@ #define INTC_REVISION 0x0000 #define INTC_SYSCONFIG 0x0010 #define INTC_SYSSTATUS 0x0014 +#define INTC_SIR 0x0040 #define INTC_CONTROL 0x0048 #define INTC_MIR_CLEAR0 0x0088 #define INTC_MIR_SET0 0x008c @@ -60,6 +61,25 @@ static u32 intc_bank_read_reg(struct omap_irq_bank *bank, u16 reg) return __raw_readl(bank->base_reg + reg); } +static int previous_irq; + +static int omap_check_spurious(unsigned int irq) +{ + u32 sir, spurious; + + sir = intc_bank_read_reg(&irq_banks[0], INTC_SIR); + spurious = sir >> 6; + + if (spurious > 1) { + printk(KERN_WARNING "Spurious irq %i: 0x%08x, please flush " + "posted write for irq %i\n", + irq, sir, previous_irq); + return spurious; + } + + return 0; +} + /* XXX: FIQ and additional INTC support (only MPU at the moment) */ static void omap_ack_irq(unsigned int irq) { @@ -70,6 +90,9 @@ static void omap_mask_irq(unsigned int irq) { int offset = irq & (~(IRQ_BITS_PER_REG - 1)); + if (cpu_is_omap34xx() && !omap_check_spurious(irq)) + previous_irq = irq; + irq &= (IRQ_BITS_PER_REG - 1); intc_bank_write_reg(1 << irq, &irq_banks[0], INTC_MIR_SET0 + offset); ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 0/4] 34xx spurious interrupts unravelling 2008-10-31 19:21 [PATCH 0/4] 34xx spurious interrupts unravelling Tony Lindgren 2008-10-31 19:21 ` [PATCH 1/4] Revert "Add MT_MEMORY_SO, mark L3 and L4 to use it" Tony Lindgren @ 2008-10-31 20:07 ` David Brownell 2008-10-31 20:35 ` Tony Lindgren 2008-11-01 21:14 ` Felipe Contreras 2 siblings, 1 reply; 18+ messages in thread From: David Brownell @ 2008-10-31 20:07 UTC (permalink / raw) To: Tony Lindgren; +Cc: linux-omap On Friday 31 October 2008, Tony Lindgren wrote: > So it's now starting to look like we need to ensure write posting in > interrupt handlers for 34xx instead of marking the IO areas as strongly > ordered. I always thought the terminology was to "flush posted writes" by the readback ... a "posted write" being one that's sitting in a hardware queue, like a letter sitting in a mailbox until the postal service picks it up (then, much later, delivers it). Regardless, these patches just go to show that OMAP hardware is starting to act more like bigger hardware ... like stuff with PCI busses, where writes are normally posted to bridge writebuffers and drivers need to defend against the relevant write latencies. :) One thing for folk to remember: the consequent races happen outside of IRQ handling too. Every time you write to an I/O register you should ask when its effect *must* be known to the hardware ... and ensure some read flushes that write before that deadline. When you get it wrong, be prepared to waste time tracking down intermittent race induced failures at very low levels. When in doubt, flush it out! For PIO drivers, IRQ handling is the main place these races will appear. If DMA is in use, there are a boatload of additional complications that can crop up ... sometimes you need to worry about when writes get to memory (DMA mapping ops don't necessarily ensure the CPU write buffer is empty), and unless the peripheral has an integrated DMA engine you need to flush writes to the DMA engine separately from writes to the peripheral. - Dave ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/4] 34xx spurious interrupts unravelling 2008-10-31 20:07 ` [PATCH 0/4] 34xx spurious interrupts unravelling David Brownell @ 2008-10-31 20:35 ` Tony Lindgren 2008-10-31 21:59 ` David Brownell 0 siblings, 1 reply; 18+ messages in thread From: Tony Lindgren @ 2008-10-31 20:35 UTC (permalink / raw) To: David Brownell; +Cc: linux-omap * David Brownell <david-b@pacbell.net> [081031 13:07]: > On Friday 31 October 2008, Tony Lindgren wrote: > > So it's now starting to look like we need to ensure write posting in > > interrupt handlers for 34xx instead of marking the IO areas as strongly > > ordered. > > I always thought the terminology was to "flush posted writes" > by the readback ... a "posted write" being one that's sitting > in a hardware queue, like a letter sitting in a mailbox until > the postal service picks it up (then, much later, delivers it). That sounds more professional. Let's standardize the comments to say "flush posted write" so we can easily grep for them later on. > Regardless, these patches just go to show that OMAP hardware > is starting to act more like bigger hardware ... like stuff > with PCI busses, where writes are normally posted to bridge > writebuffers and drivers need to defend against the relevant > write latencies. :) > > > One thing for folk to remember: the consequent races happen > outside of IRQ handling too. > > Every time you write to an I/O register you should ask when > its effect *must* be known to the hardware ... and ensure some > read flushes that write before that deadline. When you get it > wrong, be prepared to waste time tracking down intermittent > race induced failures at very low levels. > > When in doubt, flush it out! > > > For PIO drivers, IRQ handling is the main place these races > will appear. If DMA is in use, there are a boatload of > additional complications that can crop up ... sometimes you > need to worry about when writes get to memory (DMA mapping > ops don't necessarily ensure the CPU write buffer is empty), > and unless the peripheral has an integrated DMA engine you > need to flush writes to the DMA engine separately from > writes to the peripheral. Yeah, especially when recycling dma buffers.. The call to use there is dma_cache_maint(). Tony ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/4] 34xx spurious interrupts unravelling 2008-10-31 20:35 ` Tony Lindgren @ 2008-10-31 21:59 ` David Brownell 2008-11-01 4:01 ` Woodruff, Richard 0 siblings, 1 reply; 18+ messages in thread From: David Brownell @ 2008-10-31 21:59 UTC (permalink / raw) To: Tony Lindgren; +Cc: linux-omap On Friday 31 October 2008, Tony Lindgren wrote: > > > For PIO drivers, IRQ handling is the main place these races > > will appear. If DMA is in use, there are a boatload of > > additional complications that can crop up ... sometimes you > > need to worry about when writes get to memory (DMA mapping > > ops don't necessarily ensure the CPU write buffer is empty), > > and unless the peripheral has an integrated DMA engine you > > need to flush writes to the DMA engine separately from > > writes to the peripheral. > > Yeah, especially when recycling dma buffers.. The call to use > there is dma_cache_maint(). Well, not directly ... call dma_map_*() and dma_sync_*(), letting those functions call dma_cache_maint() as needed. (See arch/arm/include/asm/dma-mapping.h ...) That said, I have seen platforms where the fact that those calls can leave data in the CPU write buffer made trouble. As in, issue those dma cache updates, then write to the DMA or peripheral controller registers ... and the key data was still in the write buffers when those writes completed! That's quite unusual; possibly related to memory controller bugs in that silicon revision. (And there are converse problems in drivers that collect status from both DMA buffers/descriptors and from peripheral registers. Status can show up in registers before memory because of buffering on those paths, and vice versa. The varying clock rates matter ... maybe the memory writes are synced to a 166 MHz clock, but the peripheral internally uses a 48 MHz clock domain.) - Dave -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH 0/4] 34xx spurious interrupts unravelling 2008-10-31 21:59 ` David Brownell @ 2008-11-01 4:01 ` Woodruff, Richard 2008-11-01 6:08 ` David Brownell 0 siblings, 1 reply; 18+ messages in thread From: Woodruff, Richard @ 2008-11-01 4:01 UTC (permalink / raw) To: David Brownell, Tony Lindgren; +Cc: linux-omap@vger.kernel.org > owner@vger.kernel.org] On Behalf Of David Brownell > Sent: Friday, October 31, 2008 4:59 PM > As in, issue those dma cache updates, then write to the > DMA or peripheral controller registers ... and the key data > was still in the write buffers when those writes completed! > That's quite unusual; possibly related to memory controller > bugs in that silicon revision. For the next series of ARMv7 Cortex's this is all being projected to require much more strictness in access ordering and use of barriers. There have been some cautions about newer write buffer policies which try and keep data instead of flushing it. Today even if you miss some drain in a small amount of time it will happen. That may not be the case always. Regards, Richard W. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/4] 34xx spurious interrupts unravelling 2008-11-01 4:01 ` Woodruff, Richard @ 2008-11-01 6:08 ` David Brownell 2008-11-01 12:57 ` Woodruff, Richard 0 siblings, 1 reply; 18+ messages in thread From: David Brownell @ 2008-11-01 6:08 UTC (permalink / raw) To: Woodruff, Richard; +Cc: Tony Lindgren, linux-omap@vger.kernel.org On Friday 31 October 2008, Woodruff, Richard wrote: > > > owner@vger.kernel.org] On Behalf Of David Brownell > > Sent: Friday, October 31, 2008 4:59 PM > > > As in, issue those dma cache updates, then write to the > > DMA or peripheral controller registers ... and the key data > > was still in the write buffers when those writes completed! > > That's quite unusual; possibly related to memory controller > > bugs in that silicon revision. > > For the next series of ARMv7 Cortex's this is all being projected > to require much more strictness in access ordering and use of barriers. Any particular kind of barriers? barrier() is compiler advice, and gets the most use in Linux ... but <asm/system.h> reminds me that V7 has isb/dsb/dmb instructions, and V6 can do them too. (Now I'll have to go look at what they do again...) That case needed a dsb analogue (on armv5, flush write buffer). > There have been some cautions about newer write buffer policies > which try and keep data instead of flushing it. Today even if > you miss some drain in a small amount of time it will happen. > That may not be the case always. Yeah, I remember back when I had to work with SPARC the issue of memory ordering models was problematic. There's a tradeoff: the more ordering guarantees the easier it is for software, at cost of slowing down data transfers by excess serialization. And at the other extreme, if you configured things for highly unordered access, the hardware wouldn't have many roadblocks but the software could get confused rather easily. What this means for software developers -- especially folk writing drivers and tweaking address spaces -- is needing to pay a bunch more attention to some confusing issues that most ARM folk have managed to avoid over the past many years. - Dave > > Regards, > Richard W. > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH 0/4] 34xx spurious interrupts unravelling 2008-11-01 6:08 ` David Brownell @ 2008-11-01 12:57 ` Woodruff, Richard 0 siblings, 0 replies; 18+ messages in thread From: Woodruff, Richard @ 2008-11-01 12:57 UTC (permalink / raw) To: David Brownell; +Cc: Tony Lindgren, linux-omap@vger.kernel.org > From: David Brownell [mailto:david-b@pacbell.net] > Sent: Saturday, November 01, 2008 1:09 AM > > For the next series of ARMv7 Cortex's this is all being projected > > to require much more strictness in access ordering and use of barriers. > > Any particular kind of barriers? barrier() is compiler advice, > and gets the most use in Linux ... but <asm/system.h> reminds > me that V7 has isb/dsb/dmb instructions, and V6 can do them too. > (Now I'll have to go look at what they do again...) > > That case needed a dsb analogue (on armv5, flush write buffer). In doc's and training material there are examples of sequences which need care. As they are not our doc's I can't publicly share them. They will start showing up over time as hardware becomes more real. Single core is more aggressive and the dual core variants compound issues. >From a given CPU's point of few it is free to do many things to normal memory behind your back as long at the time you access it (from a single CPU point of view) the value is correct. This requires strict care to be 100% safe in coordinating with DMA to normal memories. For A8 today many aspects of this are less relevant as implementation won't do it all yet. Regards, Richard W. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/4] 34xx spurious interrupts unravelling 2008-10-31 19:21 [PATCH 0/4] 34xx spurious interrupts unravelling Tony Lindgren 2008-10-31 19:21 ` [PATCH 1/4] Revert "Add MT_MEMORY_SO, mark L3 and L4 to use it" Tony Lindgren 2008-10-31 20:07 ` [PATCH 0/4] 34xx spurious interrupts unravelling David Brownell @ 2008-11-01 21:14 ` Felipe Contreras 2 siblings, 0 replies; 18+ messages in thread From: Felipe Contreras @ 2008-11-01 21:14 UTC (permalink / raw) To: Tony Lindgren; +Cc: linux-omap On Fri, Oct 31, 2008 at 9:21 PM, Tony Lindgren <tony@atomide.com> wrote: > Hi all, > > So it's now starting to look like we need to ensure write posting in > interrupt handlers for 34xx instead of marking the IO areas as strongly > ordered. > > The first patch in the series reverts marking the IO areas as strongly > ordered. > > The second debug patch also prints out the previous interrupt before > the spurious interrupt, which should help identify the interrupt > handler that needs a read back of some register. > > So if you see errors like this with dspbridge: > > FIXME: spurious irq 95: 0xffffffdf previous irq 26 > FIXME: spurious irq 95: 0xffffffdf previous irq 26 > FIXME: spurious irq 95: 0xffffffdf previous irq 26 > ... > > You'll know that the interrupt handler for irq 26 needs a read back > of some register in the interrupt handler. Most likely you need to > read back the interrupt ack register right after writing to it in > the handler, see a later patch in this series for dspbridge. > > If you get errors like this during boot: > > FIXME: spurious irq 95: 0xffffffdf previous irq 25 > FIXME: spurious irq 95: 0xffffffdf previous irq 25 > FIXME: spurious irq 95: 0xffffffdf previous irq 37 > ... > > Most likely we need to do a read back when initializing some devices > like dispc and gptimer. I don't have patches for these yet.. > > The other patches fix few interrupt handlers that I'm aware of needing > this, like i2c-omap and dspbridge. The dspbridge patch only applies > if you have it installed. > > I'd appreciate if people who have been working on this issue would > torture test this series and provide feedback! I tried with various dsp tests, and as you mentioned, I was able to see lots of spurious irq warnings, then I applied patch #4, and those went away. No issues. -- Felipe Contreras ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2008-11-03 18:54 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-10-31 19:21 [PATCH 0/4] 34xx spurious interrupts unravelling Tony Lindgren 2008-10-31 19:21 ` [PATCH 1/4] Revert "Add MT_MEMORY_SO, mark L3 and L4 to use it" Tony Lindgren 2008-10-31 19:21 ` [PATCH 2/4] ARM: OMAP3: Print debug info on spurious interrupts Tony Lindgren 2008-10-31 19:21 ` [PATCH 3/4] I2C: Ensure write posting for critical i2c-omap writes Tony Lindgren 2008-10-31 19:21 ` [PATCH 4/4] DSPBRIDGE: Ensure write posting when acking mailbox irq Tony Lindgren 2008-10-31 21:06 ` Tony Lindgren 2008-11-01 3:43 ` Woodruff, Richard 2008-11-02 22:10 ` Tony Lindgren 2008-11-03 18:54 ` Tony Lindgren 2008-10-31 21:03 ` [PATCH 3/4] I2C: Ensure write posting for critical i2c-omap writes Tony Lindgren 2008-10-31 20:55 ` [PATCH 2/4] ARM: OMAP3: Print debug info on spurious interrupts Tony Lindgren 2008-10-31 20:07 ` [PATCH 0/4] 34xx spurious interrupts unravelling David Brownell 2008-10-31 20:35 ` Tony Lindgren 2008-10-31 21:59 ` David Brownell 2008-11-01 4:01 ` Woodruff, Richard 2008-11-01 6:08 ` David Brownell 2008-11-01 12:57 ` Woodruff, Richard 2008-11-01 21:14 ` Felipe Contreras
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox