Re: [RFC] mtd: nand: separate chip options / bbt_options

[Artem Bityutskiy <dedekind1@gmail.com>]

Hi Brian,

On Wed, 2011-04-20 at 00:13 -0700, Brian Norris wrote:
> This RFC begins to handle the conflicts we've been having with using
> conflicting flags from nand.h and bbm.h in the same nand_chip.options
> field. We should try to separate these two spaces a little more
> clearly, and so I have added a bbt_options field to nand_chip.

Sounds good.

> Important notes about nand_chip fields:
> * bbt_options field should contain ONLY flags from bbm.h. They should
>   be able to pass safely to a nand_bbt_descr data structure.
> * options field should contian ONLY flags from nand.h. Ideally, they
>   should not be involved in any BBT related options.

Sounds good. I'd add the following:

* NAND chip option flags start with the: NAND_ prefix
* BBT options start with BBT_ prefix.

You may choose different prefixes, e.g., NAND_BBT_ for BBT options, but
the separation is needed, I think. Also, the renaming of the options
should be a separate patch or set of patches.

I'd also add:
* Every flag should have a nice comment explaining what the flag is.

This is optional, but would be nice :-)

> Other things to consider (not yet implemented):
> * Is it safe to move NAND_CREATE_EMPTY_BBT to bbm.h and require it to be
>   put in bbt_options? It seems not to be used by any in-kernel drivers
>   so it's only likely to mess with independent drivers...

What exactly this option mean and how could it be used?

> 
> * Consider the following three flags:
>   (1) NAND_USE_FLASH_BBT (nand.h)
>   (2) NAND_USE_FLASH_BBT_NO_OOB (nand.h)
>   (3) NAND_BBT_NO_OOB (bbm.h)
> 
>   These flags are all related, yet they are in different headers. Also,
>   flag (2) is simply the combination of (1) and (3) and seemingly can be
>   eliminated. Is it safe to move (1) and (3) to bbm.h and remove (2)
>   altogether? (with appropriate code adjustments of course)

Yes, I think so.

> Regarding Artem's suggestion of bit-fields:
> If we turn all the flags into bit-fields in nand_chip, we still need to
> add these fields to the bbt_descr, right? That seems like too much
> duplication of information and would just be messier.

Let's for get about this for now, then. We can look at this idea at the
end of the clean-up then, again, may be.

Thanks!

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)