Re: [PATCH 02/11] mtd: decommission the NAND museum

[Brian Norris <computersforpeace@gmail.com>]

On 03/05/2013 05:17 AM, Artem Bityutskiy wrote:
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>
> The MTD subsystem has its own small museum of ancient NANDs in a form of the
> CONFIG_MTD_NAND_MUSEUM_IDS configuration option. The museum contains stone age
> NANDs with 256 bytes pages, as well as iron age NANDs with 512 bytes per page
> and up to 8MiB page size.
>
> It is with great sorrow that I inform you that the museum is being
> decommissioned. The MTD subsystem is out of budget for Kconfig options and
> already has too many of them, and there is a general kernel trend to simplify
> the configuration menu.
>
> We remove the stone age exhibits along with closing the museum, but the iron
> age ones are transferred to the regular NAND depot, since there is still a
> remote possibility that they may be useful for someone.

Unfortunately, the iron age and modern age exhibits will not survive 
long together. The iron age will conquer the modern, as the exhibits 
that grew up without war have gone soft... OK, I can't draw this analogy 
out much further :)

What I mean to say is: some of the museum IDs that you left will 
conflict with the "current" ones, so that the museum IDs will always win 
out; they will be detected as their museum geometry, not their extended 
ID geometry. See below for examples.

> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> Acked-by: Brian Norris <computersforpeace@gmail.com>

My ack was on the old one (which removed them entirely), not the new one 
:) I'll re-review the other patches quickly and comment if there are any 
other anti-ack's.

> ---
>   drivers/mtd/nand/Kconfig    |    8 --------
>   drivers/mtd/nand/nand_ids.c |   28 ++++++++++------------------
>   2 files changed, 10 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 81bf5e5..0f443ef 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -41,14 +41,6 @@ config MTD_SM_COMMON
>   	tristate
>   	default n
>
> -config MTD_NAND_MUSEUM_IDS
> -	bool "Enable chip ids for obsolete ancient NAND devices"
> -	default n
> -	help
> -	  Enable this option only when your board has first generation
> -	  NAND chips (page size 256 byte, erase size 4-8KiB). The IDs
> -	  of these chips were reused by later, larger chips.

Note the keyword "reused"! Only a few of them were reused though. I 
marked all the repeats below. I don't know what we can do with that, 
though. Can we cut the repeats?

>   config MTD_NAND_DENALI
>           tristate "Support Denali NAND controller"
>           help
> diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
> index f1a799c..44f9db3 100644
> --- a/drivers/mtd/nand/nand_ids.c
> +++ b/drivers/mtd/nand/nand_ids.c
> @@ -22,24 +22,16 @@
>    * extended chip ID.
>    */
>   struct nand_flash_dev nand_flash_ids[] = {
> -
> -#ifdef CONFIG_MTD_NAND_MUSEUM_IDS
> -	{"NAND 1MiB 5V 8-bit",		0x6e, 256, 1, 0x1000, 0},
> -	{"NAND 2MiB 5V 8-bit",		0x64, 256, 2, 0x1000, 0},
> -	{"NAND 4MiB 5V 8-bit",		0x6b, 512, 4, 0x2000, 0},
> -	{"NAND 1MiB 3,3V 8-bit",	0xe8, 256, 1, 0x1000, 0},
> -	{"NAND 1MiB 3,3V 8-bit",	0xec, 256, 1, 0x1000, 0},
> -	{"NAND 2MiB 3,3V 8-bit",	0xea, 256, 2, 0x1000, 0},
> -	{"NAND 4MiB 3,3V 8-bit",	0xd5, 512, 4, 0x2000, 0},
> -	{"NAND 4MiB 3,3V 8-bit",	0xe3, 512, 4, 0x2000, 0},
> -	{"NAND 4MiB 3,3V 8-bit",	0xe5, 512, 4, 0x2000, 0},
> -	{"NAND 8MiB 3,3V 8-bit",	0xd6, 512, 8, 0x2000, 0},
> -
> -	{"NAND 8MiB 1,8V 8-bit",	0x39, 512, 8, 0x2000, 0},
> -	{"NAND 8MiB 3,3V 8-bit",	0xe6, 512, 8, 0x2000, 0},
> -	{"NAND 8MiB 1,8V 16-bit",	0x49, 512, 8, 0x2000, NAND_BUSWIDTH_16},
> -	{"NAND 8MiB 3,3V 16-bit",	0x59, 512, 8, 0x2000, NAND_BUSWIDTH_16},
> -#endif
> +       {"NAND 4MiB 5V 8-bit",          0x6b, 512, 4, 0x2000, 0},
> +       {"NAND 4MiB 3,3V 8-bit",        0xd5, 512, 4, 0x2000, 0},

0xd5 is reused (with a capital 'D'!) as 16 Gbit.

> +       {"NAND 4MiB 3,3V 8-bit",        0xe3, 512, 4, 0x2000, 0},
> +       {"NAND 4MiB 3,3V 8-bit",        0xe5, 512, 4, 0x2000, 0},
> +       {"NAND 8MiB 3,3V 8-bit",        0xd6, 512, 8, 0x2000, 0},
> +
> +       {"NAND 8MiB 1,8V 8-bit",        0x39, 512, 8, 0x2000, 0},

Reused.

> +       {"NAND 8MiB 3,3V 8-bit",        0xe6, 512, 8, 0x2000, 0},
> +       {"NAND 8MiB 1,8V 16-bit",       0x49, 512, 8, 0x2000, NAND_BUSWIDTH_16},

Ditto.

> +       {"NAND 8MiB 3,3V 16-bit",       0x59, 512, 8, 0x2000, NAND_BUSWIDTH_16},

Ditto.

>
>   	{"NAND 16MiB 1,8V 8-bit",	0x33, 512, 16, 0x4000, 0},
>   	{"NAND 16MiB 3,3V 8-bit",	0x73, 512, 16, 0x4000, 0},
>

Brian