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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox