[PATCHv3 4/5] mtd: mxc_nand fixups
From: john.ogness@linutronix.de (John Ogness)
Date: 2010-06-24 10:17:10
Also in:
lkml
Possibly related (same subject, not in this thread)
- 2010-07-01 · Re: [PATCHv3 4/5] mtd: mxc_nand fixups · Ivo Clarysse <hidden>
- 2010-06-26 · Re: [PATCHv3 4/5] mtd: mxc_nand fixups · John Ogness <john.ogness@linutronix.de>
- 2010-06-25 · Re: [PATCHv3 4/5] mtd: mxc_nand fixups · Ivo Clarysse <hidden>
- 2010-06-24 · Re: [PATCHv3 4/5] mtd: mxc_nand fixups · Sascha Hauer <s.hauer@pengutronix.de>
- 2010-06-23 · Re: [PATCHv3 4/5] mtd: mxc_nand fixups · John Ogness <john.ogness@linutronix.de>
On 2010-06-24, Sascha Hauer [off-list ref] wrote:
Ok, if it's the only way out to have 5 cpu_is_* blocks, then lets go for it.
Here is a new patch that puts the behavior behind a "nfc_avoid_masking" macro. The macro is only used 3 times. I also changed the code to use a completion, since that's exactly what we are doing there anyway. I also added some other macros to simplify reading of the code and reduce possible copy/paste errors relating to masking and interrupt handling.
BTW I observed that at least on i.MX27 the latencies introduced by waiting for an interrupt cause a significant performance drop. The driver gets much faster when we just poll all the time. I don't know how this affects system performance otherwise, but it may be a possibility to drop interrupt support at least for i.MX21. I have no idea how long the longest possible time we'd have to poll is though.
I don't recommend moving to pure polling. Although it may be faster for NAND-only performance tests, I expect it would have a dramatic affect on the rest of the system during many NAND reads/writes. This patch is based on linux-next 20100618. Signed-off-by: John Ogness <john.ogness@linutronix.de> --- drivers/mtd/nand/mxc_nand.c | 81 +++++++++++++++++++++++++--------- 1 file changed, 61 insertions(+), 20 deletions(-) Index: linux-next-20100618/drivers/mtd/nand/mxc_nand.c ===================================================================
--- linux-next-20100618.orig/drivers/mtd/nand/mxc_nand.c
+++ linux-next-20100618/drivers/mtd/nand/mxc_nand.c@@ -30,6 +30,8 @@ #include <linux/clk.h> #include <linux/err.h> #include <linux/io.h> +#include <linux/irq.h> +#include <linux/completion.h> #include <asm/mach/flash.h> #include <mach/mxc_nand.h>
@@ -40,6 +42,13 @@ #define nfc_is_v21() (cpu_is_mx25() || cpu_is_mx35()) #define nfc_is_v1() (cpu_is_mx31() || cpu_is_mx27() || cpu_is_mx21()) +/* It has been observeed that the i.MX21 cannot read the CONFIG2:INT bit + * if interrupts are masked (CONFIG1:INT_MSK is set). To handle this, the + * driver can enable/disable the irq line rather than simply masking the + * interrupts. The nfc_avoid_masking() macro identifies the systems that + * should use this workaround. */ +#define nfc_avoid_masking() (cpu_is_mx21()) + /* Addresses for NFC registers */ #define NFC_BUF_SIZE 0xE00 #define NFC_BUF_ADDR 0xE04
@@ -100,6 +109,18 @@ #define NFC_RSLTSPARE_AREA_MASK 0xff +#define nfc_interrupt_set(_regs) \ + (readw(_regs + NFC_CONFIG2) & NFC_INT) +#define nfc_clear_interrupt(_regs) \ + writew(readw(_regs + NFC_CONFIG2) & ~NFC_INT, \ + _regs + NFC_CONFIG2) +#define nfc_mask_irq(_regs) \ + writew(readw(_regs + NFC_CONFIG1) | NFC_INT_MSK, \ + _regs + NFC_CONFIG1) +#define nfc_unmask_irq(_regs) \ + writew(readw(_regs + NFC_CONFIG1) & ~NFC_INT_MSK, \ + _regs + NFC_CONFIG1) + struct mxc_nand_host { struct mtd_info mtd; struct nand_chip nand;
@@ -117,7 +138,7 @@ struct mxc_nand_host { int clk_act; int irq; - wait_queue_head_t irq_waitq; + struct completion op_completion; uint8_t *data_buf; unsigned int buf_start;
@@ -174,9 +195,18 @@ static irqreturn_t mxc_nfc_irq(int irq, { struct mxc_nand_host *host = dev_id; - disable_irq_nosync(irq); - - wake_up(&host->irq_waitq); + /* If NFC_INT is not set, we have a spurious interrupt! */ + if (!nfc_interrupt_set(host->regs)) + return IRQ_NONE; + + /* Even with nfc_avoid_masking() we mask the interrupt + * here to avoid an interrupt flood until the irq line + * is disabled by the driver thread. */ + nfc_mask_irq(host->regs); + + /* Notify the driver thread that an interrupt has occurred. + * The driver thread will clear the interrupt. */ + complete(&host->op_completion); return IRQ_HANDLED; }
@@ -186,27 +216,32 @@ static irqreturn_t mxc_nfc_irq(int irq, */ static void wait_op_done(struct mxc_nand_host *host, int useirq) { - uint16_t tmp; int max_retries = 8000; if (useirq) { - if ((readw(host->regs + NFC_CONFIG2) & NFC_INT) == 0) { + if (!nfc_interrupt_set(host->regs)) { + + INIT_COMPLETION(host->op_completion); - enable_irq(host->irq); + if (nfc_avoid_masking()) + enable_irq(host->irq); + else + nfc_unmask_irq(host->regs); - wait_event(host->irq_waitq, - readw(host->regs + NFC_CONFIG2) & NFC_INT); + /* wait for the interrupt */ + wait_for_completion(&host->op_completion); - tmp = readw(host->regs + NFC_CONFIG2); - tmp &= ~NFC_INT; - writew(tmp, host->regs + NFC_CONFIG2); + if (nfc_avoid_masking()) { + disable_irq(host->irq); + nfc_unmask_irq(host->regs); + } } + + nfc_clear_interrupt(host->regs); } else { while (max_retries-- > 0) { - if (readw(host->regs + NFC_CONFIG2) & NFC_INT) { - tmp = readw(host->regs + NFC_CONFIG2); - tmp &= ~NFC_INT; - writew(tmp, host->regs + NFC_CONFIG2); + if (nfc_interrupt_set(host->regs)) { + nfc_clear_interrupt(host->regs); break; } udelay(1);
@@ -582,9 +617,8 @@ static void preset(struct mtd_info *mtd) struct mxc_nand_host *host = nand_chip->priv; uint16_t tmp; - /* enable interrupt, disable spare enable, setup ECC */ + /* disable spare-only, setup ECC */ tmp = readw(host->regs + NFC_CONFIG1); - tmp &= ~NFC_INT_MSK; tmp &= ~NFC_SP_EN; if (nand_chip->ecc.mode == NAND_ECC_HW) { tmp |= NFC_ECC_EN;
@@ -842,11 +876,18 @@ static int __init mxcnd_probe(struct pla this->options |= NAND_USE_FLASH_BBT; } - init_waitqueue_head(&host->irq_waitq); + init_completion(&host->op_completion); host->irq = platform_get_irq(pdev, 0); - err = request_irq(host->irq, mxc_nfc_irq, IRQF_DISABLED, DRIVER_NAME, host); + if (nfc_avoid_masking()) { + set_irq_flags(host->irq, IRQF_VALID | IRQF_NOAUTOEN); + nfc_unmask_irq(host->regs); + } else { + nfc_mask_irq(host->regs); + } + + err = request_irq(host->irq, mxc_nfc_irq, 0, DRIVER_NAME, host); if (err) goto eirq;