Re: [PATCH] NAND: Fix NAND ECC errors on AMD Au1550

[Leif Lindholm <leif.lindholm@i3micro.com>]

[-- Attachment #1: Type: text/plain, Size: 944 bytes --]

On Mon, 2005-10-31 at 08:00 -0800, Pete Popov wrote:
> > Just a comment (forgot to post this last time around):
> > At least one of the chips we're using - identified as
> > "NAND device: Manufacturer ID: 0xec, Chip ID: 0x76 (Samsung NAND 64MiB 3,3V 8-bit)"
> > seems to also need the CS manually asserted during a READID operation.
> > 
> > This made the if-statements ridicilously long, so I added a case
> > statement and a variable keeping track of wether or not to force-enable
> > CS to the au1550_command function.
> > 
> > If there is any interest, I could post a patch, but am unsure of the
> > correct way. Should I send a new patch against cvs HEAD containing my
> > trivial tweak as well as Sergeis modifications, or...?
> 
> Send an incremental patch on top of Sergey's patch if you can.

Attached is an incremental patch on top of Sergeys latest patch
(yesterday). Is that's ok, or should I do the same for the old one?

/
	Leif


[-- Attachment #2: au1550nd-READID-force-CE.patch --]
[-- Type: text/x-patch, Size: 2264 bytes --]

--- mtd/drivers/mtd/nand/au1550nd.c	2005-10-31 18:37:29.000000000 +0100
+++ mtd-new/drivers/mtd/nand/au1550nd.c	2005-11-01 15:39:43.000000000 +0100
@@ -343,6 +343,16 @@ static void au1550_command(struct mtd_in
 	int ce_override = 0, i;
 	ulong flags;
 
+	/* Decide wether command needs to force-enable CE or not */
+	switch (command) {
+	case NAND_CMD_READ0:
+	case NAND_CMD_READ1:
+	case NAND_CMD_READOOB:
+	case NAND_CMD_READID:
+		ce_override = 1;
+		break;
+	}
+
 	/* Begin command latch cycle */
 	this->hwcontrol(mtd, NAND_CTL_SETCLE);
 	/*
@@ -382,9 +392,7 @@ static void au1550_command(struct mtd_in
 		if (page_addr != -1) {
 			this->write_byte(mtd, (u8)(page_addr & 0xff));
 
-			if (command == NAND_CMD_READ0 ||
-			    command == NAND_CMD_READ1 ||
-			    command == NAND_CMD_READOOB) {
+			if (ce_override) {
 				/*
 				 * NAND controller will release -CE after
 				 * the last address byte is written, so we'll
@@ -393,7 +401,6 @@ static void au1550_command(struct mtd_in
 				 * want the NOR flash or PCMCIA drivers to
 				 * steal our precious bytes of data...
 				 */
-				ce_override = 1;
 				local_irq_save(flags);
 				this->hwcontrol(mtd, NAND_CTL_SETNCE);
 			}
@@ -412,25 +419,7 @@ static void au1550_command(struct mtd_in
 	 * Program and erase have their own busy handlers.
 	 * Status and sequential in need no delay.
 	 */
-	switch (command) {
-
-	case NAND_CMD_PAGEPROG:
-	case NAND_CMD_ERASE1:
-	case NAND_CMD_ERASE2:
-	case NAND_CMD_SEQIN:
-	case NAND_CMD_STATUS:
-		return;
-
-	case NAND_CMD_RESET:
-		break;
-
-	case NAND_CMD_READ0:
-	case NAND_CMD_READ1:
-	case NAND_CMD_READOOB:
-		/* Check if we're really driving -CE low (just in case) */
-		if (unlikely(!ce_override))
-			break;
-
+	if (ce_override) {
 		/* Apply a short delay always to ensure that we do wait tWB. */
 		ndelay(100);
 		/* Wait for a chip to become ready... */
@@ -442,6 +431,19 @@ static void au1550_command(struct mtd_in
 		local_irq_restore(flags);
 		return;
 	}
+
+	switch (command) {
+
+	case NAND_CMD_PAGEPROG:
+	case NAND_CMD_ERASE1:
+	case NAND_CMD_ERASE2:
+	case NAND_CMD_SEQIN:
+	case NAND_CMD_STATUS:
+		return;
+		
+	case NAND_CMD_RESET:
+		break;
+	}
 	/* Apply this short delay always to ensure that we do wait tWB. */
 	ndelay(100);