From mboxrd@z Thu Jan  1 00:00:00 1970
From: Scott Wood <scottwood@freescale.com>
Date: Wed, 20 May 2015 21:27:01 -0500
Subject: [U-Boot] Fix fsl_elbc_nand driver
In-Reply-To: <1432173354.14341.258.camel@andreilinux>
References: <1432001789.14341.120.camel@andreilinux>
 <1432071614.27761.77.camel@freescale.com>
 <1432074568.14341.149.camel@andreilinux>
 <1432075134.27761.82.camel@freescale.com>
 <1432078969.14341.178.camel@andreilinux>
 <1432160719.27761.108.camel@freescale.com>
 <1432171628.14341.246.camel@andreilinux>
 <1432172233.27761.147.camel@freescale.com>
 <1432173354.14341.258.camel@andreilinux>
Message-ID: <1432175221.27761.176.camel@freescale.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Wed, 2015-05-20 at 18:55 -0700, Andrei Yakimov wrote:
> On Wed, 2015-05-20 at 20:37 -0500, Scott Wood wrote:
> > On Wed, 2015-05-20 at 18:27 -0700, Andrei Yakimov wrote:
> > > For elbc and imx due to we reading all at once, it can not be stateless,
> > > we need to read more and more each time
> > 
> > Why do we need to?  Why can't we read all three copies at once?
> > 
> > > reissuing command or relay on different way to ID chip - and this make
> > > it pointless if it can not be done universally.
> > 
> > Or, we can reissue the command.  I don't see any big problem either way.
> > This is not performance critical.
> lets say  1 time you read 256 ( or 512) it go bad, next time you read
> 512 (or 1024) next time you read 768 ( or 1536).

I was thinking read_param() would take the offset as a parameter and use
NAND_CMD_RNDOUT to skip ahead -- but that would change the default flow
which I'd rather avoid.  Another option is that read_param() just sets
up the read for the specified number of bytes, but the caller still uses
read_byte() to extract the data.  This way the code could specify
sizeof(struct)*3 as the size up front without needing three separate
buffers.

Note that whatever gets done should first be accepted in Linux, rather
than being a local U-Boot change.  If you want a short-term fix, just
stick 1536 in the eLBC driver.

> Upper layer can maintain it.
> Roughly like this:
> 
> Was:
> 	chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);
> 	for (i = 0; i < 3; i++) {
> 		chip->read_buf(mtd, (uint8_t *)p, sizeof(*p));

You're looking at old code.  It uses read_byte() now.

> 		if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) ==
> 						le16_to_cpu(p->crc)) {
> 			break;
> 		}
> 	}
> 	if (i == 3)
> 		return 0;
> 
> new:
> 	int read_size, offset;
> 	read_size = 256;
> 	offset =0;
> 	if(!chip->read_param)
> 		chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);

I don't want "if (chip->read_param)" all over the place; there should be
a default read_param() that does what the existing code does.

> 	for (i = 0; i < 3; i++) {
> 		if(chip->read_param) chip->read_param( 0, read_size);
> 		chip->read_buf(mtd, (uint8_t *)p + offest, sizeof(*p));

This isn't going to read the second or third copy; it's going to read
the first copy and write beyond the end of your buffer.

-Scott