Re: [PATCH] Fixup in NAND bad block management + fix ofmisspring.(nand_base.c)

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, 2006-03-02 at 20:29 +0300, Alexey, Korolev wrote:
> > When the ST chips have the bad block pos at offset 0 in general then we
> > want a generic solution which fixes up the nand_scan_bbt code as well
> > instead of requiring a seperate board driver supplied badblock_pattern.
> >
> Generic solution would be great. 

Yes. And no other solution will make it into the kernel. Board level
fixes for chip level problems are not acceptable.

> But this solution will require 
> inforamtion about BB positioning for all chips.
> At present time I don't have info about BB positioning for all chips. I 
> wonder if BB positioning is somehow standardized?

Yes it is. And according to the ST datasheet it is simply at offset 0x05
for 8 bit devices and offset 0x00 for 16 bit devices.

> I'm not sure that it will be possible to avoid board driver supplied 
> badblock_patterns at all.

Fixup the offsets in nand_scan and tweak nand_bbt_default. Again this is
a chip problem not a board problem. Chips are handled by the generic
nand_base and nand_bbt code to avoid code duplication all over the
place.

>  > And you believe that your patch is the only way to solve that trivial
>  > problem ?
> 
> Imo the latest nand_base code has some inconsistence.
> If mem based BBT is used, this case Bad block scanning is based on data 
> from bad block pattern, but writing is based on data of badblockpos 
> variable.

Well, thats true, but this can be fixed with two lines of code.

> I guess it's not good to have two variables responsible for one thing if 
> they are used at the same time. Moreover this code doesn't work if 
> somebody specified BB pattern different from default.
> My patch resolves this inconsistence, logically it works rather clear 
> a) if pattern is specified we will use pattern for both read and write.
> b) if not we will use the "badblockpos" variable.
> I'd like to know do you have any objections to the patch? IMO it should 
> not break anything. But it resolves the described inconsistence.
> 
> > Your patch is bogus anyway, as the else path will _NEVER_ be executed.
> > this->badblock_pattern is never NULL.
> >
> That is incorrect. This->badblock_pattern can be NULL.  According to the 
> nand_base code if NAND_SKIP_BBTSCAN option is given and pattern is not 
> defined in low level driver the nand_scan_bbt function will not be 
> executed and no code will define badblock_pattern.

Did not think about this weird scenario as I never imagined a usecase.
It still does not need all that hacks. A simple fixup of badblockpos is
enough.

	tglx