[PATCHv3 4/5] mtd: mxc_nand fixups

[John Ogness]

On 2010-06-24, Sascha Hauer <s.hauer at pengutronix.de> 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 at 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;