linux-kernel - Re: [PATCHv3 4/5] mtd: mxc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <80vd9ab0d2.fsf@merkur.tec.linutronix.de>
Date:	Wed, 23 Jun 2010 10:48:41 +0200
From:	John Ogness <john.ogness@...utronix.de>
To:	Ivo Clarysse <ivo.clarysse@...il.com>
Cc:	Sascha Hauer <s.hauer@...gutronix.de>,
	Sascha Hauer <kernel@...gutronix.de>,
	linux-arm-kernel@...ts.infradead.org,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv3 4/5] mtd: mxc_nand fixups

On 2010-06-23, Ivo Clarysse <ivo.clarysse@...il.com> wrote:
> Unfortunately, this breaks i.MX21 support.
>
> With this patch, the driver gets stuck at the first send_addr()
> invocation:
>
> [...]
>
> Setting NFC_CONFIG1:NFC_INT_MSK in the interrupt handler causes
> NFC_CONFIG2:NFC_INT to always read out 0 (on i.MX21), causing
>
>     wait_event(host->irq_waitq,
>                readw(host->regs + NFC_CONFIG2) & NFC_INT);
>
> to wait forever.

OK. Now I understand the problem. Here is a new patch that introduces a
flag that is set by the interrupt handler. This way we do not rely on
the i.MX21 being able to read NFC_INT when the interrupt is masked.

I have also added a lot of comments so that it is (hopefully) clear why
we are enabling and disabling the irq line rather than only masking and
unmasking the interrupt.

The patch is against linux-next 20100618.

Signed-off-by: John Ogness <john.ogness@...utronix.de>
---
 drivers/mtd/nand/mxc_nand.c |   49 +++++++++++++++++++++++++++++-----
 1 file changed, 42 insertions(+), 7 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,7 @@
 #include <linux/clk.h>
 #include <linux/err.h>
 #include <linux/io.h>
+#include <linux/irq.h>
 
 #include <asm/mach/flash.h>
 #include <mach/mxc_nand.h>
@@ -117,6 +118,7 @@ struct mxc_nand_host {
 	int			clk_act;
 	int			irq;
 
+	int			nfc_int;
 	wait_queue_head_t	irq_waitq;
 
 	uint8_t			*data_buf;
@@ -173,8 +175,21 @@ static const char *part_probes[] = { "Re
 static irqreturn_t mxc_nfc_irq(int irq, void *dev_id)
 {
 	struct mxc_nand_host *host = dev_id;
+	uint16_t tmp;
+
+	/* If NFC_INT is not set, we have a spurious interrupt! */
+	if ((readw(host->regs + NFC_CONFIG2) & NFC_INT) == 0)
+		return IRQ_NONE;
+
+	/* We use the host->nfc_int flag because the i.MX21 cannot
+	 * read the CONFIG2:NFC_INT bit if interrupts are masked. */
+	host->nfc_int = 1;
 
-	disable_irq_nosync(irq);
+	/* Mask the interrupts so that we avoid an interrupt
+	 * flood until the irq line is disabled. */
+	tmp = readw(host->regs + NFC_CONFIG1);
+	tmp |= NFC_INT_MSK;
+	writew(tmp, host->regs + NFC_CONFIG1);
 
 	wake_up(&host->irq_waitq);
 
@@ -192,15 +207,33 @@ static void wait_op_done(struct mxc_nand
 	if (useirq) {
 		if ((readw(host->regs + NFC_CONFIG2) & NFC_INT) == 0) {
 
+			/* Clear the interrupt flag. It will be set
+			 * by the interrupt handler. */
+			host->nfc_int = 0;
+
+			/* We enable the irq line and wait for an interrupt.
+			 * We must use irq line enable/disable rather than
+			 * simply masking of the interrupt because the i.MX21
+			 * cannot read the CONFIG2:NFC_INT bit if the
+			 * interrupt is masked. This is also the reason we
+			 * use the host->nfc_int flag. */
 			enable_irq(host->irq);
 
-			wait_event(host->irq_waitq,
-				readw(host->regs + NFC_CONFIG2) & NFC_INT);
+			wait_event(host->irq_waitq, host->nfc_int);
+
+			disable_irq(host->irq);
 
-			tmp = readw(host->regs + NFC_CONFIG2);
-			tmp  &= ~NFC_INT;
-			writew(tmp, host->regs + NFC_CONFIG2);
+			/* The irq line has been disabled. We can now
+			 * unmask the interrupts. */
+			tmp = readw(host->regs + NFC_CONFIG1);
+			tmp &= ~NFC_INT_MSK;
+			writew(tmp, host->regs + NFC_CONFIG1);
 		}
+
+		/* clear interrupt flag */
+		tmp = readw(host->regs + NFC_CONFIG2);
+		tmp &= ~NFC_INT;
+		writew(tmp, host->regs + NFC_CONFIG2);
 	} else {
 		while (max_retries-- > 0) {
 			if (readw(host->regs + NFC_CONFIG2) & NFC_INT) {
@@ -846,7 +879,9 @@ static int __init mxcnd_probe(struct pla
 
 	host->irq = platform_get_irq(pdev, 0);
 
-	err = request_irq(host->irq, mxc_nfc_irq, IRQF_DISABLED, DRIVER_NAME, host);
+	/* request irq as disabled */
+	set_irq_flags(host->irq, IRQF_VALID | IRQF_NOAUTOEN);
+	err = request_irq(host->irq, mxc_nfc_irq, 0, DRIVER_NAME, host);
 	if (err)
 		goto eirq;
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/