Re: [RFC] the way to proceed with OOB data

[Vitaly Wool <vwool@ru.mvista.com>]

Charles Manning wrote:

>>1. Change the API for nand_read_ecc and nand_write_ecc, namely add OOB
>>read/write length parameter. This is what Charles proposes; it's nice
>>for MTD users but it will overcomplicate nand_read_ecc/nand_write_ecc,
>>which are complicated enough at the moment.
>>    
>>
>
>I disagree that it needs to make the current code more complicated.
>
>I did submit a patch of sorts that was quite simple, but fractured the logic 
>in do_read_ecc, do_read_ecc is pretty convoluted and effort should be made to 
>not make it any more complicated.
>
>I think the better solution is to just wrap the code better. ie:
>
>static int nand_read_ecc (struct mtd_info *mtd, loff_t from, size_t len,
>			  size_t * retlen, u_char * buf, u_char * oob_buf, struct nand_oobinfo 
>*oobsel)
>{
>
>       if(buf) {    <-------- 
>	/* use userspace supplied oobinfo, if zero */
>	if (oobsel == NULL)
>		oobsel = &mtd->oobinfo;
>	return nand_do_read_ecc(mtd, from, len, retlen, buf, oob_buf, oobsel, 0xff);
>       } else {   <--------------------
>            /* just read the oob according to oobinfo */  
><---------------------
>           call whatever local function <----------------------
>      }  <----------------
>  
>
That doesn't seem to be a comprehensive solution to me. You imply here 
that in case buf is NULL, OOB area of only one page is to be read which 
may be inconvenient in some cases. Basically that means that you need to 
specify two more parameters - the OOB data length to read and (ptr to 
the var for) the OOB data length actually read. On the other hand, this 
means you shouldn't ignore these parameters in nand_do_read_ecc call, 
thus it's either you make everything a lot more complicated or you get 
routime with quite a limited functionality for OOB reading/writing 
(suppose you have a long, say, cleanmarker which is pread across several 
pages of the eraseblock; you willthen have to figure out how many read 
calls to do in order to retrieve it all).
So, lemme a bit disagree with you, I hope that makes sense a bit :)

>>3. Leave everything as is and add a new functionto the MTD interface
>>that will read/write only free OOB bytes as a single chunk of data.
>>    
>>
>
>In some ways this is the best since all the other options have either  
>compatability issues of one sort or another. It is also very clear what is 
>going on. For this reason, this would be my preferred approach.
>  
>
Yeah, to me as well :)
I'm planning to work out something of a kind soon, maybe even before 
Christmas ;)

Best regards,
   Vitaly