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

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

On Sun, 2005-10-30 at 23:58 -0800, Pete Popov wrote:
> It works on the Db1550. I need to run a sanity check on the Db1200. If
> no one has any objections, I'll commit it in the next couple of days
> after I check the Db1200.

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...?

regards
	/
		Leif
> Pete
> 
> On Sun, 2005-10-30 at 00:44 +0400, Sergei Shtylylov wrote:
> > Hello.
> > 
> >           (This is a follow-up to the older thread:
> > http://lists.infradead.org/pipermail/linux-mtd/2005-October/013983.html)
> > 
> >           Reintroducing the problem: the static bus controller fails to keep
> > -CE asserted during chip ready delay on read commands (the NAND chip being
> > used requires this). So, the current driver allows nand_base.c to drive -CE
> > manually (during the whole sector read). When the PCMCIA driver is enabled
> > however, occasionally the ECC errors occur on NAND reads. This happens because
> > PCMCIA driver polls sockets periodically and reads one of the board's
> > control/status regs (BCSRs) which are accessible via the same static bus
> > as the NAND flash using the different chip select (and the NOR flash is
> > accessible the same way), so as the NAND driver forces NAND chip select
> > asserted and the -RE signal is shared, there's a contention on the static
> > bus when BCSR or NOR flash is read while we're reading from NAND.
> >           So, we either can't keep interrupts enabled during the whole NAND
> > sector read (which is hardly acceptable), or have to implement some
> > interlocking scheme between multiple drivers (which is painful, and makes
> > me shudder :-).
> >           There's a third way which proved to work: to force -CE asserted only
> > while we're waiting for NAND chip to become ready after a read command,
> > disabling interrupts for a maximum of 25 microsecs (according to Toshiba
> > TC58DVM92A1FT00 flash chip datasheet). So, here's the initial patch which
> > I'm hoping to have made looking more decently...
> > 
> > WBR, Sergei
> > 
> > 
> > plain text document attachment (Au1550-NAND-chip-select.patch)
> > Signed-off-by: Konstantin Baydarov <kbaidarov@ru.mvista.com>
> > Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> > 
> > Index: drivers/mtd/nand/au1550nd.c
> > ===================================================================
> > RCS file: /home/cvs/mtd/drivers/mtd/nand/au1550nd.c,v
> > retrieving revision 1.12
> > diff -a -u -p -r1.12 au1550nd.c
> > --- drivers/mtd/nand/au1550nd.c	23 Sep 2005 01:44:55 -0000	1.12
> > +++ drivers/mtd/nand/au1550nd.c	29 Oct 2005 20:04:23 -0000
> > @@ -14,6 +14,7 @@
> >  #include <linux/slab.h>
> >  #include <linux/init.h>
> >  #include <linux/module.h>
> > +#include <linux/interrupt.h>
> >  #include <linux/mtd/mtd.h>
> >  #include <linux/mtd/nand.h>
> >  #include <linux/mtd/partitions.h>
> > @@ -313,6 +314,141 @@ int au1550_device_ready(struct mtd_info 
> >  	return ret;
> >  }
> >  
> > +/**
> > + * au1550_select_chip - control -CE line
> > + *	Forbid driving -CE manually permitting the NAND controller to do this.
> > + *	Keeping -CE asserted during the whole sector reads interferes with the
> > + *	NOR flash and PCMCIA drivers as it causes contention on the static bus.
> > + *	We only have to hold -CE low for the NAND read commands since the flash
> > + *	chip needs it to be asserted during chip not ready time but the NAND
> > + *	controller keeps it released.
> > + *
> > + * @mtd:	MTD device structure
> > + * @chip:	chipnumber to select, -1 for deselect
> > + */
> > +static void au1550_select_chip(struct mtd_info *mtd, int chip)
> > +{
> > +}
> > +
> > +/**
> > + * au1550_command - Send command to NAND device
> > + * @mtd:	MTD device structure
> > + * @command:	the command to be sent
> > + * @column:	the column address for this command, -1 if none
> > + * @page_addr:	the page address for this command, -1 if none
> > + */
> > +static void au1550_command(struct mtd_info *mtd, unsigned command, int column, int page_addr)
> > +{
> > +	register struct nand_chip *this = mtd->priv;
> > +	int ce_override = 0, i;
> > +	ulong flags;
> > +
> > +	/* Begin command latch cycle */
> > +	this->hwcontrol(mtd, NAND_CTL_SETCLE);
> > +	/*
> > +	 * Write out the command to the device.
> > +	 */
> > +	if (command == NAND_CMD_SEQIN) {
> > +		int readcmd;
> > +
> > +		if (column >= mtd->oobblock) {
> > +			/* OOB area */
> > +			column -= mtd->oobblock;
> > +			readcmd = NAND_CMD_READOOB;
> > +		} else if (column < 256) {
> > +			/* First 256 bytes --> READ0 */
> > +			readcmd = NAND_CMD_READ0;
> > +		} else {
> > +			column -= 256;
> > +			readcmd = NAND_CMD_READ1;
> > +		}
> > +		this->write_byte(mtd, readcmd);
> > +	}
> > +	this->write_byte(mtd, command);
> > +
> > +	/* Set ALE and clear CLE to start address cycle */
> > +	this->hwcontrol(mtd, NAND_CTL_CLRCLE);
> > +
> > +	if (column != -1 || page_addr != -1) {
> > +		this->hwcontrol(mtd, NAND_CTL_SETALE);
> > +
> > +		/* Serially input address */
> > +		if (column != -1) {
> > +			/* Adjust columns for 16 bit buswidth */
> > +			if (this->options & NAND_BUSWIDTH_16)
> > +				column >>= 1;
> > +			this->write_byte(mtd, column);
> > +		}
> > +		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) {
> > +				/*
> > +				 * NAND controller will release -CE after
> > +				 * the last address byte is written, so we'll
> > +				 * have to forcibly assert it. No interrupts
> > +				 * are allowed while we do this as we don't
> > +				 * 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);
> > +			}
> > +
> > +			this->write_byte(mtd, (u8)(page_addr >> 8));
> > +
> > +			/* One more address cycle for devices > 32MiB */
> > +			if (this->chipsize > (32 << 20))
> > +				this->write_byte(mtd, (u8)((page_addr >> 16) & 0x0f));
> > +		}
> > +		/* Latch in address */
> > +		this->hwcontrol(mtd, NAND_CTL_CLRALE);
> > +	}
> > +
> > +	/*
> > +	 * 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;
> > +
> > +		/* Apply a short delay always to ensure that we do wait tWB. */
> > +		ndelay(100);
> > +		/* Wait for a chip to become ready... */
> > +		for (i = this->chip_delay; !this->dev_ready(mtd) && i > 0; --i)
> > +			udelay(1);
> > +
> > +		/* Release -CE and re-enable interrupts. */
> > +		this->hwcontrol(mtd, NAND_CTL_CLRNCE);
> > +		local_irq_restore(flags);
> > +		return;
> > +	}
> > +	/* Apply this short delay always to ensure that we do wait tWB. */
> > +	ndelay(100);
> > +
> > +	while(!this->dev_ready(mtd));
> > +}
> > +
> > +
> >  /*
> >   * Main initialization routine
> >   */
> > @@ -437,6 +573,9 @@ int __init au1xxx_nand_init (void)
> >  	/* Set address of hardware control function */
> >  	this->hwcontrol = au1550_hwcontrol;
> >  	this->dev_ready = au1550_device_ready;
> > +	this->select_chip = au1550_select_chip;
> > +	this->cmdfunc = au1550_command;
> > +
> >  	/* 30 us command delay time */
> >  	this->chip_delay = 30;		
> >  	this->eccmode = NAND_ECC_SOFT;
> > 
> > 
> > 
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
-- 
Leif Lindholm <leif.lindholm@i3micro.com>
I3 Micro Technology AB