[PATCH 0/1] Bad block markers here, there and everywhere

[Brian Norris]

On Thu, Nov 14, 2013 at 03:58:46PM -0300, Ezequiel Garcia wrote:
> On Tue, Nov 05, 2013 at 08:15:22PM -0300, Ezequiel Garcia wrote:
> > On Tue, Nov 05, 2013 at 08:01:26PM -0300, Ezequiel Garcia wrote:
> > > On Tue, Nov 05, 2013 at 10:07:43AM -0800, Brian Norris wrote:
> > > > On Tue, Nov 05, 2013 at 09:00:20AM -0300, Ezequiel Garcia wrote:
> > > > > This commit adds a new option called NAND_BBT_DATA_BBM. The change itself
> > > > > is pretty small and simple to understand: when the badblock_pattern sets the
> > > > > NAND_BBT_DATA_BBM option, scan_block_fast() reads the data region instead
> > > > > of the OOB region.
> > > > 
> > > > I think this type of scanning method is better suited to a different
> > > > type of solution: implement a custom nand_chip.bad_block() call-back.
> > > 
> > > Fully agreed (I guess you mean block_bad() right?)
> > > 
> > > > Unfortunately, nand_base/nand_bbt are kind of inconsistent, so that some
> > > > code paths use nand_chip.bad_block() and some use nand_bbt.c's scanning
> > > > code to check for bad block markers, so this is not currently a good
> > > > solution.
> > > > 
> > > 
> > > Which is why I couldn't implement a custom block_bad(). My particular
> > > use case (which could match others) needs this customization in the
> > > first scan. After that, once the bad block table is built, the in-flash
> > > bad block markers are never touched.
> > > 
> > > > I've been meaning to follow through with an improved version of this
> > > > patch for a while:
> > > > 
> > > >   http://lists.infradead.org/pipermail/linux-mtd/2012-June/042571.html
> > > > 
> > > > Such a patch provides several benefits, one of them being that drivers
> > > > like yours can easily provide a custom BBM location. What do you think?
> > > > 
> > > 
> > > Of course, that sounds much more flexible. Let me pick it where Matthieu left,
> > > I'll read the patch and prepare something to discuss.
> > > 
> > > On the other side, I'm seeing the above patch was a bit argued :/ I'm
> > > not sure why it got never merged, maybe you can give me some heads up
> > > about potential drawbacks?
> > 
> > After reading the patch and reading the code, now I'm even more confused :)
> > 
> > The first thing that seems odd is the fact that the scan_block_fast()
> > path matches a pattern (which can be several bytes long), whereas the
> > default nand_block_bad() seem to check for just one byte.
> > 
> > This may or may not be an issue, after some thought, but it's not
> > a trivial change, IMHO.
> > 
> > The second thing, which was already discussed back in June-2012 is the
> > fact that scan_block_fast() uses mtd_read_oob() to read, but
> > nand_block_bad() just issues a READOOB command.
> > 
> > So, what do you propose? If you can give me some guidelines I've no problem
> > in preparing a (first/draft) patch to trigger the discussion.
> 
> Ping?
> 
> This is an important issue for the pxa3xx driver and I'd like to move
> forward with it.
> 
> However, as I previously said I'd like to discuss some more about your
> proposal: currently scan_block_fast() uses mtd_read_oob() to read, and
> nand_block_bad() issues a READOOB command, so it's not trivial to
> replace the former with the latter.
> 
> Ideas?

I believe my plan was to totally kill of the nand_chip.badblock_pattern
field first, since that's the source of unnecessary complexity (and a
perverted mixture of "BBT" (table) and "BBM" (marker)).

Most drivers already just use the default markers from
nand_base/nand_bbt, and any remaining ones can either just tweak the
nand_chip.badblockpos/nand_chip.bbt_options or (like pxa3xx_nand) can
define their own nand_chip.block_bad() callback, I think. Once there are
no users with a custom badblock_pattern, I think the nand_base
nand_block_bad() implementation should be equivalent to the nand_bbt
scan_block_fast() checks.

I think I started to do all of this a while back (and I have a few
changes queued somewhere), but I stalled on some problem. Or maybe I
just didn't make the time to finish testing it properly. But now that we
have a good reason to do so, I can get back to this issue--either
submitting or reviewing patches, and testing on my setups.

Brian