[U-Boot-Users] [PATCH 2/7 v3] NAND_CMD_READOOB is not supported by all chips, read OOB with the page instead

[Scott Wood]

On Tue, Aug 05, 2008 at 03:08:04PM +0200, Guennadi Liakhovetski wrote:
> It's a large-page device. And, as far as I understand the datasheet, to 
> read data at arbitrary offset in a page, you first have to issue a READ 
> PAGE (READ0) for _the_ _whole_ page, then you can use RANDOM DATA READ to 
> read arbitrary data within this page. Whereas, the driver attempts to use 
> READ0 to read the bad-block marker directly, which doesn't work with this 
> chip. At least this is my understanding.

Are you saying that your NAND chip can't read the OOB by issuing READ0
with the appropriate column address?  Which chip is this, and where can I
find a manual?

> > > @@ -150,8 +135,6 @@ static int nand_read_page(struct mtd_info *mtd, int block, int page, uchar *dst)
> > >  	u_char *ecc_code;
> > >  	u_char *oob_data;
> > >  	int i;
> > > -	int eccsize = CFG_NAND_ECCSIZE;
> > > -	int eccbytes = CFG_NAND_ECCBYTES;
> > 
> > Any particular reason for this change?  It's more readable as is, IMHO.
> 
> Acually, it was to improve readability:-) First, this way you can easier 
> grep.

Grep will find the initialization.

> Secondly, when I see an assignment to a _variable_, I expect, that this
> variable's value can indeed _vary_. So, it makes extra work looking
> through the code and verifying what other values this variable takes. 
> Thus, at the very least I would add "const" to the definition. And, I
> do think using constants directly makes it clearer...

It replaces a short lower-case name with a longer all-caps name that
forces line breaks.  I'm fine with adding "const".

-Scott