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

Thomas Gleixner
2006-03-12T16:43:54Z

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