From: Scott Wood <scottwood@freescale.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] Fix fsl_elbc_nand driver
Date: Wed, 20 May 2015 21:27:01 -0500 [thread overview]
Message-ID: <1432175221.27761.176.camel@freescale.com> (raw)
In-Reply-To: <1432173354.14341.258.camel@andreilinux>
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
next prev parent reply other threads:[~2015-05-21 2:27 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-19 2:16 [U-Boot] Fix fsl_elbc_nand driver Andrei Yakimov
2015-05-19 21:40 ` Scott Wood
[not found] ` <1432074568.14341.149.camel@andreilinux>
[not found] ` <1432075134.27761.82.camel@freescale.com>
2015-05-19 23:42 ` Andrei Yakimov
2015-05-20 22:25 ` Scott Wood
2015-05-21 1:27 ` Andrei Yakimov
2015-05-21 1:37 ` Scott Wood
2015-05-21 1:55 ` Andrei Yakimov
2015-05-21 2:27 ` Scott Wood [this message]
2015-05-21 2:42 ` Andrei Yakimov
2015-05-21 2:46 ` Scott Wood
2015-05-21 3:03 ` Andrei Yakimov
2015-05-21 3:11 ` Scott Wood
2015-05-21 3:54 ` Andrei Yakimov
2015-05-21 3:57 ` Scott Wood
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1432175221.27761.176.camel@freescale.com \
--to=scottwood@freescale.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.