[PATCH 00/12] mtd: nand: provision full ID support

[PATCH 00/12] mtd: nand: provision full ID support

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

Hi,

this is an RFC which adds full ID support provision which Huang can then
use to solve his problem described here:

http://thread.gmane.org/gmane.linux.kernel/1428345/focus=45290

But before adding full ID support, I cleaned up things a little and removed
AG-AND support as part of the clean-up.

Thanks,
Artem.

[PATCH 01/12] mtd: nand_ids: minor clean-ups

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

Clean-up the code a little bit:
  * clean-up commentaries.
  * move macro definitions to the top of the file.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 drivers/mtd/nand/nand_ids.c |   26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index e3aa274..b110742 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -10,17 +10,17 @@
  */
 #include <linux/module.h>
 #include <linux/mtd/nand.h>
+
+#define LP_OPTIONS NAND_SAMSUNG_LP_OPTIONS
+#define LP_OPTIONS16 (LP_OPTIONS | NAND_BUSWIDTH_16)
+
 /*
-*	Chip ID list
-*
-*	Name. ID code, pagesize, chipsize in MegaByte, eraseblock size,
-*	options
-*
-*	Pagesize; 0, 256, 512
-*	0	get this information from the extended chip ID
-+	256	256 Byte page size
-*	512	512 Byte page size
-*/
+ * The chip ID list:
+ *    name, device ID, page size, chip size in MiB, eraseblock size, options
+ *
+ * If page size and eraseblock size is 0, the sizes are taken from the extended
+ * chip ID.
+ */
 struct nand_flash_dev nand_flash_ids[] = {
 
 #ifdef CONFIG_MTD_NAND_MUSEUM_IDS
@@ -67,11 +67,9 @@ struct nand_flash_dev nand_flash_ids[] = {
 	{"NAND 256MiB 3,3V 8-bit",	0x71, 512, 256, 0x4000, 0},
 
 	/*
-	 * These are the new chips with large page size. The pagesize and the
-	 * erasesize is determined from the extended id bytes
+	 * These are the new chips with large page size, page size and
+	 * eraseblock size is determined from the extended id bytes.
 	 */
-#define LP_OPTIONS NAND_SAMSUNG_LP_OPTIONS
-#define LP_OPTIONS16 (LP_OPTIONS | NAND_BUSWIDTH_16)
 
 	/* 512 Megabit */
 	{"NAND 64MiB 1,8V 8-bit",	0xA2, 0,  64, 0, LP_OPTIONS},
-- 
1.7.10.4

Re: [PATCH 01/12] mtd: nand_ids: minor clean-ups

[Brian Norris]

On Mon, Mar 4, 2013 at 8:42 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
> index e3aa274..b110742 100644
> --- a/drivers/mtd/nand/nand_ids.c
> +++ b/drivers/mtd/nand/nand_ids.c
> @@ -10,17 +10,17 @@
>   */
>  #include <linux/module.h>
>  #include <linux/mtd/nand.h>
> +
> +#define LP_OPTIONS NAND_SAMSUNG_LP_OPTIONS
> +#define LP_OPTIONS16 (LP_OPTIONS | NAND_BUSWIDTH_16)
> +
>  /*
> -*      Chip ID list
> -*
> -*      Name. ID code, pagesize, chipsize in MegaByte, eraseblock size,
> -*      options
> -*
> -*      Pagesize; 0, 256, 512
> -*      0       get this information from the extended chip ID
> -+      256     256 Byte page size
> -*      512     512 Byte page size
> -*/
> + * The chip ID list:
> + *    name, device ID, page size, chip size in MiB, eraseblock size, options
> + *
> + * If page size and eraseblock size is 0, the sizes are taken from the extended

Grammar correction: "is" should be "are".

> + * chip ID.
> + */
>  struct nand_flash_dev nand_flash_ids[] = {
>
>  #ifdef CONFIG_MTD_NAND_MUSEUM_IDS
> @@ -67,11 +67,9 @@ struct nand_flash_dev nand_flash_ids[] = {
>         {"NAND 256MiB 3,3V 8-bit",      0x71, 512, 256, 0x4000, 0},
>
>         /*
> -        * These are the new chips with large page size. The pagesize and the
> -        * erasesize is determined from the extended id bytes

The original form has incorrect grammar; "is" should be "are".

> +        * These are the new chips with large page size, page size and
> +        * eraseblock size is determined from the extended id bytes.

This is also incorrect grammar. It uses a comma to divide sentences
and also uses "is". I would write something like this:

"These are the new chips with large page size. Their page size and
eraseblock size are determined from the extended ID bytes."

>          */
> -#define LP_OPTIONS NAND_SAMSUNG_LP_OPTIONS
> -#define LP_OPTIONS16 (LP_OPTIONS | NAND_BUSWIDTH_16)
>
>         /* 512 Megabit */
>         {"NAND 64MiB 1,8V 8-bit",       0xA2, 0,  64, 0, LP_OPTIONS},
> --
> 1.7.10.4
>

Brian

Re: [PATCH 01/12] mtd: nand_ids: minor clean-ups

[Artem Bityutskiy]

On Mon, 2013-03-04 at 10:36 -0800, Brian Norris wrote:
> > +        * These are the new chips with large page size, page size and
> > +        * eraseblock size is determined from the extended id bytes.
> 
> This is also incorrect grammar. It uses a comma to divide sentences
> and also uses "is". I would write something like this:
> 
> "These are the new chips with large page size. Their page size and
> eraseblock size are determined from the extended ID bytes."

Thanks Brian, will fix.

-- 
Best Regards,
Artem Bityutskiy

[PATCH 02/12] mtd: remove museum NAND ID's support

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

It is probably OK to remove support for really old NAND chips of 8MiB or
smaller size. We had a separate configuration option for them:
CONFIG_MTD_NAND_MUSEUM_IDS, which we remove along with this patch.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 drivers/mtd/nand/Kconfig    |    8 --------
 drivers/mtd/nand/nand_ids.c |   19 -------------------
 2 files changed, 27 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.
-
 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 b110742..35070a2 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -22,25 +22,6 @@
  * 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 16MiB 1,8V 8-bit",	0x33, 512, 16, 0x4000, 0},
 	{"NAND 16MiB 3,3V 8-bit",	0x73, 512, 16, 0x4000, 0},
 	{"NAND 16MiB 1,8V 16-bit",	0x43, 512, 16, 0x4000, NAND_BUSWIDTH_16},
-- 
1.7.10.4

Re: [PATCH 02/12] mtd: remove museum NAND ID's support

[Brian Norris]

On Mon, Mar 4, 2013 at 8:42 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>
> It is probably OK to remove support for really old NAND chips of 8MiB or
> smaller size. We had a separate configuration option for them:
> CONFIG_MTD_NAND_MUSEUM_IDS, which we remove along with this patch.
>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

I have never seen any of these NAND. So FWIW:

Acked-by: Brian Norris <computersforpeace@gmail.com>

Re[2]: [PATCH 02/12] mtd: remove museum NAND ID's support

[Alexander Shiyan]

> On Mon, Mar 4, 2013 at 8:42 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> >
> > It is probably OK to remove support for really old NAND chips of 8MiB or
> > smaller size. We had a separate configuration option for them:
> > CONFIG_MTD_NAND_MUSEUM_IDS, which we remove along with this patch.
> >
> > Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> 
> I have never seen any of these NAND. So FWIW:
> 
> Acked-by: Brian Norris <computersforpeace@gmail.com>

My previous post on this topic did not go, so I will repeat:
These chips are used in DiskOnChip and removing identifiers will lead
to malfunction of systems using DiskOnChip as extra memory.

---

[PATCH 03/12] arm: defconfigs: lpc32xx_defconfig: remove museum NAND option

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

The MTD_NAND_MUSEUM_IDS configuration options was removed - update the
lpc32xx_defconfig file.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 arch/arm/configs/lpc32xx_defconfig |    1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/configs/lpc32xx_defconfig b/arch/arm/configs/lpc32xx_defconfig
index 92386b2..e77326f 100644
--- a/arch/arm/configs/lpc32xx_defconfig
+++ b/arch/arm/configs/lpc32xx_defconfig
@@ -55,7 +55,6 @@ CONFIG_MTD_CHAR=y
 CONFIG_MTD_BLOCK=y
 CONFIG_MTD_M25P80=y
 CONFIG_MTD_NAND=y
-CONFIG_MTD_NAND_MUSEUM_IDS=y
 CONFIG_MTD_NAND_SLC_LPC32XX=y
 CONFIG_MTD_NAND_MLC_LPC32XX=y
 CONFIG_BLK_DEV_LOOP=y
-- 
1.7.10.4

[PATCH 04/12] mtd: nand: remove the rtc_from4 driver support

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

The AG-AND support is about to be removed from MTD, because this technology is
dead for long time. Thus, remove this the only AG-AND driver we have in the
kernel tree.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 drivers/mtd/nand/Kconfig     |   10 -
 drivers/mtd/nand/rtc_from4.c |  624 ------------------------------------------
 2 files changed, 634 deletions(-)
 delete mode 100644 drivers/mtd/nand/rtc_from4.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 0f443ef..a626629 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -193,16 +193,6 @@ config MTD_NAND_BF5XX_BOOTROM_ECC
 
 	  If unsure, say N.
 
-config MTD_NAND_RTC_FROM4
-	tristate "Renesas Flash ROM 4-slot interface board (FROM_BOARD4)"
-	depends on SH_SOLUTION_ENGINE
-	select REED_SOLOMON
-	select REED_SOLOMON_DEC8
-	select BITREVERSE
-	help
-	  This enables the driver for the Renesas Technology AG-AND
-	  flash interface board (FROM_BOARD4)
-
 config MTD_NAND_PPCHAMELEONEVB
 	tristate "NAND Flash device on PPChameleonEVB board"
 	depends on PPCHAMELEONEVB && BROKEN
diff --git a/drivers/mtd/nand/rtc_from4.c b/drivers/mtd/nand/rtc_from4.c
deleted file mode 100644
index e55b5cf..0000000
--- a/drivers/mtd/nand/rtc_from4.c
+++ /dev/null
@@ -1,624 +0,0 @@
-/*
- *  drivers/mtd/nand/rtc_from4.c
- *
- *  Copyright (C) 2004  Red Hat, Inc.
- *
- *  Derived from drivers/mtd/nand/spia.c
- *       Copyright (C) 2000 Steven J. Hill (sjhill@realitydiluted.com)
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * Overview:
- *   This is a device driver for the AG-AND flash device found on the
- *   Renesas Technology Corp. Flash ROM 4-slot interface board (FROM_BOARD4),
- *   which utilizes the Renesas HN29V1G91T-30 part.
- *   This chip is a 1 GBibit (128MiB x 8 bits) AG-AND flash device.
- */
-
-#include <linux/delay.h>
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/slab.h>
-#include <linux/rslib.h>
-#include <linux/bitrev.h>
-#include <linux/module.h>
-#include <linux/mtd/mtd.h>
-#include <linux/mtd/nand.h>
-#include <linux/mtd/partitions.h>
-#include <asm/io.h>
-
-/*
- * MTD structure for Renesas board
- */
-static struct mtd_info *rtc_from4_mtd = NULL;
-
-#define RTC_FROM4_MAX_CHIPS	2
-
-/* HS77x9 processor register defines */
-#define SH77X9_BCR1	((volatile unsigned short *)(0xFFFFFF60))
-#define SH77X9_BCR2	((volatile unsigned short *)(0xFFFFFF62))
-#define SH77X9_WCR1	((volatile unsigned short *)(0xFFFFFF64))
-#define SH77X9_WCR2	((volatile unsigned short *)(0xFFFFFF66))
-#define SH77X9_MCR	((volatile unsigned short *)(0xFFFFFF68))
-#define SH77X9_PCR	((volatile unsigned short *)(0xFFFFFF6C))
-#define SH77X9_FRQCR	((volatile unsigned short *)(0xFFFFFF80))
-
-/*
- * Values specific to the Renesas Technology Corp. FROM_BOARD4 (used with HS77x9 processor)
- */
-/* Address where flash is mapped */
-#define RTC_FROM4_FIO_BASE	0x14000000
-
-/* CLE and ALE are tied to address lines 5 & 4, respectively */
-#define RTC_FROM4_CLE		(1 << 5)
-#define RTC_FROM4_ALE		(1 << 4)
-
-/* address lines A24-A22 used for chip selection */
-#define RTC_FROM4_NAND_ADDR_SLOT3	(0x00800000)
-#define RTC_FROM4_NAND_ADDR_SLOT4	(0x00C00000)
-#define RTC_FROM4_NAND_ADDR_FPGA	(0x01000000)
-/* mask address lines A24-A22 used for chip selection */
-#define RTC_FROM4_NAND_ADDR_MASK	(RTC_FROM4_NAND_ADDR_SLOT3 | RTC_FROM4_NAND_ADDR_SLOT4 | RTC_FROM4_NAND_ADDR_FPGA)
-
-/* FPGA status register for checking device ready (bit zero) */
-#define RTC_FROM4_FPGA_SR		(RTC_FROM4_NAND_ADDR_FPGA | 0x00000002)
-#define RTC_FROM4_DEVICE_READY		0x0001
-
-/* FPGA Reed-Solomon ECC Control register */
-
-#define RTC_FROM4_RS_ECC_CTL		(RTC_FROM4_NAND_ADDR_FPGA | 0x00000050)
-#define RTC_FROM4_RS_ECC_CTL_CLR	(1 << 7)
-#define RTC_FROM4_RS_ECC_CTL_GEN	(1 << 6)
-#define RTC_FROM4_RS_ECC_CTL_FD_E	(1 << 5)
-
-/* FPGA Reed-Solomon ECC code base */
-#define RTC_FROM4_RS_ECC		(RTC_FROM4_NAND_ADDR_FPGA | 0x00000060)
-#define RTC_FROM4_RS_ECCN		(RTC_FROM4_NAND_ADDR_FPGA | 0x00000080)
-
-/* FPGA Reed-Solomon ECC check register */
-#define RTC_FROM4_RS_ECC_CHK		(RTC_FROM4_NAND_ADDR_FPGA | 0x00000070)
-#define RTC_FROM4_RS_ECC_CHK_ERROR	(1 << 7)
-
-#define ERR_STAT_ECC_AVAILABLE		0x20
-
-/* Undefine for software ECC */
-#define RTC_FROM4_HWECC	1
-
-/* Define as 1 for no virtual erase blocks (in JFFS2) */
-#define RTC_FROM4_NO_VIRTBLOCKS	0
-
-/*
- * Module stuff
- */
-static void __iomem *rtc_from4_fio_base = (void *)P2SEGADDR(RTC_FROM4_FIO_BASE);
-
-static const struct mtd_partition partition_info[] = {
-	{
-	 .name = "Renesas flash partition 1",
-	 .offset = 0,
-	 .size = MTDPART_SIZ_FULL},
-};
-
-#define NUM_PARTITIONS 1
-
-/*
- *	hardware specific flash bbt decriptors
- *	Note: this is to allow debugging by disabling
- *		NAND_BBT_CREATE and/or NAND_BBT_WRITE
- *
- */
-static uint8_t bbt_pattern[] = { 'B', 'b', 't', '0' };
-static uint8_t mirror_pattern[] = { '1', 't', 'b', 'B' };
-
-static struct nand_bbt_descr rtc_from4_bbt_main_descr = {
-	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
-		| NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP,
-	.offs = 40,
-	.len = 4,
-	.veroffs = 44,
-	.maxblocks = 4,
-	.pattern = bbt_pattern
-};
-
-static struct nand_bbt_descr rtc_from4_bbt_mirror_descr = {
-	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
-		| NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP,
-	.offs = 40,
-	.len = 4,
-	.veroffs = 44,
-	.maxblocks = 4,
-	.pattern = mirror_pattern
-};
-
-#ifdef RTC_FROM4_HWECC
-
-/* the Reed Solomon control structure */
-static struct rs_control *rs_decoder;
-
-/*
- *      hardware specific Out Of Band information
- */
-static struct nand_ecclayout rtc_from4_nand_oobinfo = {
-	.eccbytes = 32,
-	.eccpos = {
-		   0, 1, 2, 3, 4, 5, 6, 7,
-		   8, 9, 10, 11, 12, 13, 14, 15,
-		   16, 17, 18, 19, 20, 21, 22, 23,
-		   24, 25, 26, 27, 28, 29, 30, 31},
-	.oobfree = {{32, 32}}
-};
-
-#endif
-
-/*
- * rtc_from4_hwcontrol - hardware specific access to control-lines
- * @mtd:	MTD device structure
- * @cmd:	hardware control command
- *
- * Address lines (A5 and A4) are used to control Command and Address Latch
- * Enable on this board, so set the read/write address appropriately.
- *
- * Chip Enable is also controlled by the Chip Select (CS5) and
- * Address lines (A24-A22), so no action is required here.
- *
- */
-static void rtc_from4_hwcontrol(struct mtd_info *mtd, int cmd,
-				unsigned int ctrl)
-{
-	struct nand_chip *chip = (mtd->priv);
-
-	if (cmd == NAND_CMD_NONE)
-		return;
-
-	if (ctrl & NAND_CLE)
-		writeb(cmd, chip->IO_ADDR_W | RTC_FROM4_CLE);
-	else
-		writeb(cmd, chip->IO_ADDR_W | RTC_FROM4_ALE);
-}
-
-/*
- * rtc_from4_nand_select_chip - hardware specific chip select
- * @mtd:	MTD device structure
- * @chip:	Chip to select (0 == slot 3, 1 == slot 4)
- *
- * The chip select is based on address lines A24-A22.
- * This driver uses flash slots 3 and 4 (A23-A22).
- *
- */
-static void rtc_from4_nand_select_chip(struct mtd_info *mtd, int chip)
-{
-	struct nand_chip *this = mtd->priv;
-
-	this->IO_ADDR_R = (void __iomem *)((unsigned long)this->IO_ADDR_R & ~RTC_FROM4_NAND_ADDR_MASK);
-	this->IO_ADDR_W = (void __iomem *)((unsigned long)this->IO_ADDR_W & ~RTC_FROM4_NAND_ADDR_MASK);
-
-	switch (chip) {
-
-	case 0:		/* select slot 3 chip */
-		this->IO_ADDR_R = (void __iomem *)((unsigned long)this->IO_ADDR_R | RTC_FROM4_NAND_ADDR_SLOT3);
-		this->IO_ADDR_W = (void __iomem *)((unsigned long)this->IO_ADDR_W | RTC_FROM4_NAND_ADDR_SLOT3);
-		break;
-	case 1:		/* select slot 4 chip */
-		this->IO_ADDR_R = (void __iomem *)((unsigned long)this->IO_ADDR_R | RTC_FROM4_NAND_ADDR_SLOT4);
-		this->IO_ADDR_W = (void __iomem *)((unsigned long)this->IO_ADDR_W | RTC_FROM4_NAND_ADDR_SLOT4);
-		break;
-
-	}
-}
-
-/*
- * rtc_from4_nand_device_ready - hardware specific ready/busy check
- * @mtd:	MTD device structure
- *
- * This board provides the Ready/Busy state in the status register
- * of the FPGA.  Bit zero indicates the RDY(1)/BSY(0) signal.
- *
- */
-static int rtc_from4_nand_device_ready(struct mtd_info *mtd)
-{
-	unsigned short status;
-
-	status = *((volatile unsigned short *)(rtc_from4_fio_base + RTC_FROM4_FPGA_SR));
-
-	return (status & RTC_FROM4_DEVICE_READY);
-
-}
-
-/*
- * deplete - code to perform device recovery in case there was a power loss
- * @mtd:	MTD device structure
- * @chip:	Chip to select (0 == slot 3, 1 == slot 4)
- *
- * If there was a sudden loss of power during an erase operation, a
- * "device recovery" operation must be performed when power is restored
- * to ensure correct operation.  This routine performs the required steps
- * for the requested chip.
- *
- * See page 86 of the data sheet for details.
- *
- */
-static void deplete(struct mtd_info *mtd, int chip)
-{
-	struct nand_chip *this = mtd->priv;
-
-	/* wait until device is ready */
-	while (!this->dev_ready(mtd)) ;
-
-	this->select_chip(mtd, chip);
-
-	/* Send the commands for device recovery, phase 1 */
-	this->cmdfunc(mtd, NAND_CMD_DEPLETE1, 0x0000, 0x0000);
-	this->cmdfunc(mtd, NAND_CMD_DEPLETE2, -1, -1);
-
-	/* Send the commands for device recovery, phase 2 */
-	this->cmdfunc(mtd, NAND_CMD_DEPLETE1, 0x0000, 0x0004);
-	this->cmdfunc(mtd, NAND_CMD_DEPLETE2, -1, -1);
-
-}
-
-#ifdef RTC_FROM4_HWECC
-/*
- * rtc_from4_enable_hwecc - hardware specific hardware ECC enable function
- * @mtd:	MTD device structure
- * @mode:	I/O mode; read or write
- *
- * enable hardware ECC for data read or write
- *
- */
-static void rtc_from4_enable_hwecc(struct mtd_info *mtd, int mode)
-{
-	volatile unsigned short *rs_ecc_ctl = (volatile unsigned short *)(rtc_from4_fio_base + RTC_FROM4_RS_ECC_CTL);
-	unsigned short status;
-
-	switch (mode) {
-	case NAND_ECC_READ:
-		status = RTC_FROM4_RS_ECC_CTL_CLR | RTC_FROM4_RS_ECC_CTL_FD_E;
-
-		*rs_ecc_ctl = status;
-		break;
-
-	case NAND_ECC_READSYN:
-		status = 0x00;
-
-		*rs_ecc_ctl = status;
-		break;
-
-	case NAND_ECC_WRITE:
-		status = RTC_FROM4_RS_ECC_CTL_CLR | RTC_FROM4_RS_ECC_CTL_GEN | RTC_FROM4_RS_ECC_CTL_FD_E;
-
-		*rs_ecc_ctl = status;
-		break;
-
-	default:
-		BUG();
-		break;
-	}
-
-}
-
-/*
- * rtc_from4_calculate_ecc - hardware specific code to read ECC code
- * @mtd:	MTD device structure
- * @dat:	buffer containing the data to generate ECC codes
- * @ecc_code	ECC codes calculated
- *
- * The ECC code is calculated by the FPGA.  All we have to do is read the values
- * from the FPGA registers.
- *
- * Note: We read from the inverted registers, since data is inverted before
- * the code is calculated. So all 0xff data (blank page) results in all 0xff rs code
- *
- */
-static void rtc_from4_calculate_ecc(struct mtd_info *mtd, const u_char *dat, u_char *ecc_code)
-{
-	volatile unsigned short *rs_eccn = (volatile unsigned short *)(rtc_from4_fio_base + RTC_FROM4_RS_ECCN);
-	unsigned short value;
-	int i;
-
-	for (i = 0; i < 8; i++) {
-		value = *rs_eccn;
-		ecc_code[i] = (unsigned char)value;
-		rs_eccn++;
-	}
-	ecc_code[7] |= 0x0f;	/* set the last four bits (not used) */
-}
-
-/*
- * rtc_from4_correct_data - hardware specific code to correct data using ECC code
- * @mtd:	MTD device structure
- * @buf:	buffer containing the data to generate ECC codes
- * @ecc1	ECC codes read
- * @ecc2	ECC codes calculated
- *
- * The FPGA tells us fast, if there's an error or not. If no, we go back happy
- * else we read the ecc results from the fpga and call the rs library to decode
- * and hopefully correct the error.
- *
- */
-static int rtc_from4_correct_data(struct mtd_info *mtd, const u_char *buf, u_char *ecc1, u_char *ecc2)
-{
-	int i, j, res;
-	unsigned short status;
-	uint16_t par[6], syn[6];
-	uint8_t ecc[8];
-	volatile unsigned short *rs_ecc;
-
-	status = *((volatile unsigned short *)(rtc_from4_fio_base + RTC_FROM4_RS_ECC_CHK));
-
-	if (!(status & RTC_FROM4_RS_ECC_CHK_ERROR)) {
-		return 0;
-	}
-
-	/* Read the syndrome pattern from the FPGA and correct the bitorder */
-	rs_ecc = (volatile unsigned short *)(rtc_from4_fio_base + RTC_FROM4_RS_ECC);
-	for (i = 0; i < 8; i++) {
-		ecc[i] = bitrev8(*rs_ecc);
-		rs_ecc++;
-	}
-
-	/* convert into 6 10bit syndrome fields */
-	par[5] = rs_decoder->index_of[(((uint16_t) ecc[0] >> 0) & 0x0ff) | (((uint16_t) ecc[1] << 8) & 0x300)];
-	par[4] = rs_decoder->index_of[(((uint16_t) ecc[1] >> 2) & 0x03f) | (((uint16_t) ecc[2] << 6) & 0x3c0)];
-	par[3] = rs_decoder->index_of[(((uint16_t) ecc[2] >> 4) & 0x00f) | (((uint16_t) ecc[3] << 4) & 0x3f0)];
-	par[2] = rs_decoder->index_of[(((uint16_t) ecc[3] >> 6) & 0x003) | (((uint16_t) ecc[4] << 2) & 0x3fc)];
-	par[1] = rs_decoder->index_of[(((uint16_t) ecc[5] >> 0) & 0x0ff) | (((uint16_t) ecc[6] << 8) & 0x300)];
-	par[0] = (((uint16_t) ecc[6] >> 2) & 0x03f) | (((uint16_t) ecc[7] << 6) & 0x3c0);
-
-	/* Convert to computable syndrome */
-	for (i = 0; i < 6; i++) {
-		syn[i] = par[0];
-		for (j = 1; j < 6; j++)
-			if (par[j] != rs_decoder->nn)
-				syn[i] ^= rs_decoder->alpha_to[rs_modnn(rs_decoder, par[j] + i * j)];
-
-		/* Convert to index form */
-		syn[i] = rs_decoder->index_of[syn[i]];
-	}
-
-	/* Let the library code do its magic. */
-	res = decode_rs8(rs_decoder, (uint8_t *) buf, par, 512, syn, 0, NULL, 0xff, NULL);
-	if (res > 0) {
-		pr_debug("rtc_from4_correct_data: " "ECC corrected %d errors on read\n", res);
-	}
-	return res;
-}
-
-/**
- * rtc_from4_errstat - perform additional error status checks
- * @mtd:	MTD device structure
- * @this:	NAND chip structure
- * @state:	state or the operation
- * @status:	status code returned from read status
- * @page:	startpage inside the chip, must be called with (page & this->pagemask)
- *
- * Perform additional error status checks on erase and write failures
- * to determine if errors are correctable.  For this device, correctable
- * 1-bit errors on erase and write are considered acceptable.
- *
- * note: see pages 34..37 of data sheet for details.
- *
- */
-static int rtc_from4_errstat(struct mtd_info *mtd, struct nand_chip *this,
-			     int state, int status, int page)
-{
-	int er_stat = 0;
-	int rtn, retlen;
-	size_t len;
-	uint8_t *buf;
-	int i;
-
-	this->cmdfunc(mtd, NAND_CMD_STATUS_CLEAR, -1, -1);
-
-	if (state == FL_ERASING) {
-
-		for (i = 0; i < 4; i++) {
-			if (!(status & 1 << (i + 1)))
-				continue;
-			this->cmdfunc(mtd, (NAND_CMD_STATUS_ERROR + i + 1),
-				      -1, -1);
-			rtn = this->read_byte(mtd);
-			this->cmdfunc(mtd, NAND_CMD_STATUS_RESET, -1, -1);
-
-			/* err_ecc_not_avail */
-			if (!(rtn & ERR_STAT_ECC_AVAILABLE))
-				er_stat |= 1 << (i + 1);
-		}
-
-	} else if (state == FL_WRITING) {
-
-		unsigned long corrected = mtd->ecc_stats.corrected;
-
-		/* single bank write logic */
-		this->cmdfunc(mtd, NAND_CMD_STATUS_ERROR, -1, -1);
-		rtn = this->read_byte(mtd);
-		this->cmdfunc(mtd, NAND_CMD_STATUS_RESET, -1, -1);
-
-		if (!(rtn & ERR_STAT_ECC_AVAILABLE)) {
-			/* err_ecc_not_avail */
-			er_stat |= 1 << 1;
-			goto out;
-		}
-
-		len = mtd->writesize;
-		buf = kmalloc(len, GFP_KERNEL);
-		if (!buf) {
-			er_stat = 1;
-			goto out;
-		}
-
-		/* recovery read */
-		rtn = nand_do_read(mtd, page, len, &retlen, buf);
-
-		/* if read failed or > 1-bit error corrected */
-		if (rtn || (mtd->ecc_stats.corrected - corrected) > 1)
-			er_stat |= 1 << 1;
-		kfree(buf);
-	}
-out:
-	rtn = status;
-	if (er_stat == 0) {	/* if ECC is available   */
-		rtn = (status & ~NAND_STATUS_FAIL);	/*   clear the error bit */
-	}
-
-	return rtn;
-}
-#endif
-
-/*
- * Main initialization routine
- */
-static int __init rtc_from4_init(void)
-{
-	struct nand_chip *this;
-	unsigned short bcr1, bcr2, wcr2;
-	int i;
-	int ret;
-
-	/* Allocate memory for MTD device structure and private data */
-	rtc_from4_mtd = kmalloc(sizeof(struct mtd_info) + sizeof(struct nand_chip), GFP_KERNEL);
-	if (!rtc_from4_mtd) {
-		printk("Unable to allocate Renesas NAND MTD device structure.\n");
-		return -ENOMEM;
-	}
-
-	/* Get pointer to private data */
-	this = (struct nand_chip *)(&rtc_from4_mtd[1]);
-
-	/* Initialize structures */
-	memset(rtc_from4_mtd, 0, sizeof(struct mtd_info));
-	memset(this, 0, sizeof(struct nand_chip));
-
-	/* Link the private data with the MTD structure */
-	rtc_from4_mtd->priv = this;
-	rtc_from4_mtd->owner = THIS_MODULE;
-
-	/* set area 5 as PCMCIA mode to clear the spec of tDH(Data hold time;9ns min) */
-	bcr1 = *SH77X9_BCR1 & ~0x0002;
-	bcr1 |= 0x0002;
-	*SH77X9_BCR1 = bcr1;
-
-	/* set */
-	bcr2 = *SH77X9_BCR2 & ~0x0c00;
-	bcr2 |= 0x0800;
-	*SH77X9_BCR2 = bcr2;
-
-	/* set area 5 wait states */
-	wcr2 = *SH77X9_WCR2 & ~0x1c00;
-	wcr2 |= 0x1c00;
-	*SH77X9_WCR2 = wcr2;
-
-	/* Set address of NAND IO lines */
-	this->IO_ADDR_R = rtc_from4_fio_base;
-	this->IO_ADDR_W = rtc_from4_fio_base;
-	/* Set address of hardware control function */
-	this->cmd_ctrl = rtc_from4_hwcontrol;
-	/* Set address of chip select function */
-	this->select_chip = rtc_from4_nand_select_chip;
-	/* command delay time (in us) */
-	this->chip_delay = 100;
-	/* return the status of the Ready/Busy line */
-	this->dev_ready = rtc_from4_nand_device_ready;
-
-#ifdef RTC_FROM4_HWECC
-	printk(KERN_INFO "rtc_from4_init: using hardware ECC detection.\n");
-
-	this->ecc.mode = NAND_ECC_HW_SYNDROME;
-	this->ecc.size = 512;
-	this->ecc.bytes = 8;
-	this->ecc.strength = 3;
-	/* return the status of extra status and ECC checks */
-	this->errstat = rtc_from4_errstat;
-	/* set the nand_oobinfo to support FPGA H/W error detection */
-	this->ecc.layout = &rtc_from4_nand_oobinfo;
-	this->ecc.hwctl = rtc_from4_enable_hwecc;
-	this->ecc.calculate = rtc_from4_calculate_ecc;
-	this->ecc.correct = rtc_from4_correct_data;
-
-	/* We could create the decoder on demand, if memory is a concern.
-	 * This way we have it handy, if an error happens
-	 *
-	 * Symbolsize is 10 (bits)
-	 * Primitve polynomial is x^10+x^3+1
-	 * first consecutive root is 0
-	 * primitve element to generate roots = 1
-	 * generator polinomial degree = 6
-	 */
-	rs_decoder = init_rs(10, 0x409, 0, 1, 6);
-	if (!rs_decoder) {
-		printk(KERN_ERR "Could not create a RS decoder\n");
-		ret = -ENOMEM;
-		goto err_1;
-	}
-#else
-	printk(KERN_INFO "rtc_from4_init: using software ECC detection.\n");
-
-	this->ecc.mode = NAND_ECC_SOFT;
-#endif
-
-	/* set the bad block tables to support debugging */
-	this->bbt_td = &rtc_from4_bbt_main_descr;
-	this->bbt_md = &rtc_from4_bbt_mirror_descr;
-
-	/* Scan to find existence of the device */
-	if (nand_scan(rtc_from4_mtd, RTC_FROM4_MAX_CHIPS)) {
-		ret = -ENXIO;
-		goto err_2;
-	}
-
-	/* Perform 'device recovery' for each chip in case there was a power loss. */
-	for (i = 0; i < this->numchips; i++) {
-		deplete(rtc_from4_mtd, i);
-	}
-
-#if RTC_FROM4_NO_VIRTBLOCKS
-	/* use a smaller erase block to minimize wasted space when a block is bad */
-	/* note: this uses eight times as much RAM as using the default and makes */
-	/*       mounts take four times as long. */
-	rtc_from4_mtd->flags |= MTD_NO_VIRTBLOCKS;
-#endif
-
-	/* Register the partitions */
-	ret = mtd_device_register(rtc_from4_mtd, partition_info,
-				  NUM_PARTITIONS);
-	if (ret)
-		goto err_3;
-
-	/* Return happy */
-	return 0;
-err_3:
-	nand_release(rtc_from4_mtd);
-err_2:
-	free_rs(rs_decoder);
-err_1:
-	kfree(rtc_from4_mtd);
-	return ret;
-}
-
-module_init(rtc_from4_init);
-
-/*
- * Clean up routine
- */
-static void __exit rtc_from4_cleanup(void)
-{
-	/* Release resource, unregister partitions */
-	nand_release(rtc_from4_mtd);
-
-	/* Free the MTD device structure */
-	kfree(rtc_from4_mtd);
-
-#ifdef RTC_FROM4_HWECC
-	/* Free the reed solomon resources */
-	if (rs_decoder) {
-		free_rs(rs_decoder);
-	}
-#endif
-}
-
-module_exit(rtc_from4_cleanup);
-
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("d.marlin <dmarlin@redhat.com");
-MODULE_DESCRIPTION("Board-specific glue layer for AG-AND flash on Renesas FROM_BOARD4");
-- 
1.7.10.4

[PATCH 05/12] mtd: nand: remove AG-AND support

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

We have only one AG-AND driver and it was not touched since 2005. It looks
like AG-AND was not really make it to mass-production and can be considered
a dead technology.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 drivers/mtd/nand/nand_base.c |   78 ++----------------------------------------
 drivers/mtd/nand/nand_bbt.c  |   25 --------------
 drivers/mtd/nand/nand_ids.c  |   13 -------
 include/linux/mtd/nand.h     |   16 ---------
 4 files changed, 2 insertions(+), 130 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 4321415..0e28f55 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -4,7 +4,6 @@
  *  Overview:
  *   This is the generic MTD driver for NAND flash devices. It should be
  *   capable of working with almost all NAND chips currently available.
- *   Basic support for AG-AND chips is provided.
  *
  *	Additional technical information is available on
  *	http://www.linux-mtd.infradead.org/doc/nand.html
@@ -22,7 +21,6 @@
  *	Enable cached programming for 2k page size chips
  *	Check, if mtd->ecctype should be set to MTD_ECC_HW
  *	if we have HW ECC support.
- *	The AG-AND chips have nice features for speed improvement,
  *	which are not supported yet. Read / program 4 pages in one go.
  *	BBT table is not serialized, has to be fixed
  *
@@ -836,9 +834,7 @@ static int nand_wait(struct mtd_info *mtd, struct nand_chip *chip)
 	 */
 	ndelay(100);
 
-	if ((state == FL_ERASING) && (chip->options & NAND_IS_AND))
-		chip->cmdfunc(mtd, NAND_CMD_STATUS_MULTI, -1, -1);
-	else
+	if (state == FL_ERASING)
 		chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
 
 	if (in_interrupt() || oops_in_progress)
@@ -2465,24 +2461,6 @@ static void single_erase_cmd(struct mtd_info *mtd, int page)
 }
 
 /**
- * multi_erase_cmd - [GENERIC] AND specific block erase command function
- * @mtd: MTD device structure
- * @page: the page address of the block which will be erased
- *
- * AND multi block erase command function. Erase 4 consecutive blocks.
- */
-static void multi_erase_cmd(struct mtd_info *mtd, int page)
-{
-	struct nand_chip *chip = mtd->priv;
-	/* Send commands to erase a block */
-	chip->cmdfunc(mtd, NAND_CMD_ERASE1, -1, page++);
-	chip->cmdfunc(mtd, NAND_CMD_ERASE1, -1, page++);
-	chip->cmdfunc(mtd, NAND_CMD_ERASE1, -1, page++);
-	chip->cmdfunc(mtd, NAND_CMD_ERASE1, -1, page);
-	chip->cmdfunc(mtd, NAND_CMD_ERASE2, -1, -1);
-}
-
-/**
  * nand_erase - [MTD Interface] erase block(s)
  * @mtd: MTD device structure
  * @instr: erase instruction
@@ -2494,7 +2472,6 @@ static int nand_erase(struct mtd_info *mtd, struct erase_info *instr)
 	return nand_erase_nand(mtd, instr, 0);
 }
 
-#define BBT_PAGE_MASK	0xffffff3f
 /**
  * nand_erase_nand - [INTERN] erase block(s)
  * @mtd: MTD device structure
@@ -2508,8 +2485,6 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
 {
 	int page, status, pages_per_block, ret, chipnr;
 	struct nand_chip *chip = mtd->priv;
-	loff_t rewrite_bbt[NAND_MAX_CHIPS] = {0};
-	unsigned int bbt_masked_page = 0xffffffff;
 	loff_t len;
 
 	pr_debug("%s: start = 0x%012llx, len = %llu\n",
@@ -2540,15 +2515,6 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
 		goto erase_exit;
 	}
 
-	/*
-	 * If BBT requires refresh, set the BBT page mask to see if the BBT
-	 * should be rewritten. Otherwise the mask is set to 0xffffffff which
-	 * can not be matched. This is also done when the bbt is actually
-	 * erased to avoid recursive updates.
-	 */
-	if (chip->options & BBT_AUTO_REFRESH && !allowbbt)
-		bbt_masked_page = chip->bbt_td->pages[chipnr] & BBT_PAGE_MASK;
-
 	/* Loop through the pages */
 	len = instr->len;
 
@@ -2594,15 +2560,6 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
 			goto erase_exit;
 		}
 
-		/*
-		 * If BBT requires refresh, set the BBT rewrite flag to the
-		 * page being erased.
-		 */
-		if (bbt_masked_page != 0xffffffff &&
-		    (page & BBT_PAGE_MASK) == bbt_masked_page)
-			    rewrite_bbt[chipnr] =
-					((loff_t)page << chip->page_shift);
-
 		/* Increment page address and decrement length */
 		len -= (1 << chip->phys_erase_shift);
 		page += pages_per_block;
@@ -2612,15 +2569,6 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
 			chipnr++;
 			chip->select_chip(mtd, -1);
 			chip->select_chip(mtd, chipnr);
-
-			/*
-			 * If BBT requires refresh and BBT-PERCHIP, set the BBT
-			 * page mask to see if this BBT should be rewritten.
-			 */
-			if (bbt_masked_page != 0xffffffff &&
-			    (chip->bbt_td->options & NAND_BBT_PERCHIP))
-				bbt_masked_page = chip->bbt_td->pages[chipnr] &
-					BBT_PAGE_MASK;
 		}
 	}
 	instr->state = MTD_ERASE_DONE;
@@ -2637,23 +2585,6 @@ erase_exit:
 	if (!ret)
 		mtd_erase_callback(instr);
 
-	/*
-	 * If BBT requires refresh and erase was successful, rewrite any
-	 * selected bad block tables.
-	 */
-	if (bbt_masked_page == 0xffffffff || ret)
-		return ret;
-
-	for (chipnr = 0; chipnr < chip->numchips; chipnr++) {
-		if (!rewrite_bbt[chipnr])
-			continue;
-		/* Update the BBT for chip */
-		pr_debug("%s: nand_update_bbt (%d:0x%0llx 0x%0x)\n",
-				__func__, chipnr, rewrite_bbt[chipnr],
-				chip->bbt_td->pages[chipnr]);
-		nand_update_bbt(mtd, rewrite_bbt[chipnr]);
-	}
-
 	/* Return more or less happy */
 	return ret;
 }
@@ -3286,12 +3217,7 @@ ident_done:
 	}
 
 	chip->badblockbits = 8;
-
-	/* Check for AND chips with 4 page planes */
-	if (chip->options & NAND_4PAGE_ARRAY)
-		chip->erase_cmd = multi_erase_cmd;
-	else
-		chip->erase_cmd = single_erase_cmd;
+	chip->erase_cmd = single_erase_cmd;
 
 	/* Do not replace user supplied command function! */
 	if (mtd->writesize > 512 && chip->cmdfunc == nand_command)
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 916d6e9..2672643 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -1240,15 +1240,6 @@ int nand_update_bbt(struct mtd_info *mtd, loff_t offs)
  */
 static uint8_t scan_ff_pattern[] = { 0xff, 0xff };
 
-static uint8_t scan_agand_pattern[] = { 0x1C, 0x71, 0xC7, 0x1C, 0x71, 0xC7 };
-
-static struct nand_bbt_descr agand_flashbased = {
-	.options = NAND_BBT_SCANEMPTY | NAND_BBT_SCANALLPAGES,
-	.offs = 0x20,
-	.len = 6,
-	.pattern = scan_agand_pattern
-};
-
 /* Generic flash bbt descriptors */
 static uint8_t bbt_pattern[] = {'B', 'b', 't', '0' };
 static uint8_t mirror_pattern[] = {'1', 't', 'b', 'B' };
@@ -1333,22 +1324,6 @@ int nand_default_bbt(struct mtd_info *mtd)
 {
 	struct nand_chip *this = mtd->priv;
 
-	/*
-	 * Default for AG-AND. We must use a flash based bad block table as the
-	 * devices have factory marked _good_ blocks. Erasing those blocks
-	 * leads to loss of the good / bad information, so we _must_ store this
-	 * information in a good / bad table during startup.
-	 */
-	if (this->options & NAND_IS_AND) {
-		/* Use the default pattern descriptors */
-		if (!this->bbt_td) {
-			this->bbt_td = &bbt_main_descr;
-			this->bbt_md = &bbt_mirror_descr;
-		}
-		this->bbt_options |= NAND_BBT_USE_FLASH;
-		return nand_scan_bbt(mtd, &agand_flashbased);
-	}
-
 	/* Is a flash based bad block table requested? */
 	if (this->bbt_options & NAND_BBT_USE_FLASH) {
 		/* Use the default pattern descriptors */
diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index 35070a2..7c5dec18 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -125,19 +125,6 @@ struct nand_flash_dev nand_flash_ids[] = {
 	{"NAND 64GiB 1,8V 16-bit",	0x2E, 0, 65536, 0, LP_OPTIONS16},
 	{"NAND 64GiB 3,3V 16-bit",	0x4E, 0, 65536, 0, LP_OPTIONS16},
 
-	/*
-	 * Renesas AND 1 Gigabit. Those chips do not support extended id and
-	 * have a strange page/block layout !  The chosen minimum erasesize is
-	 * 4 * 2 * 2048 = 16384 Byte, as those chips have an array of 4 page
-	 * planes 1 block = 2 pages, but due to plane arrangement the blocks
-	 * 0-3 consists of page 0 + 4,1 + 5, 2 + 6, 3 + 7 Anyway JFFS2 would
-	 * increase the eraseblock size so we chose a combined one which can be
-	 * erased in one go There are more speed improvements for reads and
-	 * writes possible, but not implemented now
-	 */
-	{"AND 128MiB 3,3V 8-bit",	0x01, 2048, 128, 0x4000,
-	 NAND_IS_AND | NAND_4PAGE_ARRAY | BBT_AUTO_REFRESH},
-
 	{NULL,}
 };
 
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 7ccb3c5..2aba049 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -171,22 +171,6 @@ typedef enum {
 #define NAND_CACHEPRG		0x00000008
 /* Chip has copy back function */
 #define NAND_COPYBACK		0x00000010
-/*
- * AND Chip which has 4 banks and a confusing page / block
- * assignment. See Renesas datasheet for further information.
- */
-#define NAND_IS_AND		0x00000020
-/*
- * Chip has a array of 4 pages which can be read without
- * additional ready /busy waits.
- */
-#define NAND_4PAGE_ARRAY	0x00000040
-/*
- * Chip requires that BBT is periodically rewritten to prevent
- * bits from adjacent blocks from 'leaking' in altering data.
- * This happens with the Renesas AG-AND chips, possibly others.
- */
-#define BBT_AUTO_REFRESH	0x00000080
 /* Chip does not allow subpage writes */
 #define NAND_NO_SUBPAGE_WRITE	0x00000200
 
-- 
1.7.10.4

Re: [PATCH 05/12] mtd: nand: remove AG-AND support

[Brian Norris]

On Mon, Mar 4, 2013 at 8:42 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>
> We have only one AG-AND driver and it was not touched since 2005. It looks
> like AG-AND was not really make it to mass-production and can be considered
> a dead technology.

You might want to include in your commit message that BBT_AUTO_REFRESH
was only needed for AG-AND. You drop it here (which is perfectly
reasonable) but it is technically provided as a generic feature which
*could* be used outside of AG-AND.

> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> ---
>  drivers/mtd/nand/nand_base.c |   78 ++----------------------------------------
>  drivers/mtd/nand/nand_bbt.c  |   25 --------------
>  drivers/mtd/nand/nand_ids.c  |   13 -------
>  include/linux/mtd/nand.h     |   16 ---------
>  4 files changed, 2 insertions(+), 130 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 4321415..0e28f55 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c

...

> @@ -22,7 +21,6 @@
>   *     Enable cached programming for 2k page size chips
>   *     Check, if mtd->ecctype should be set to MTD_ECC_HW
>   *     if we have HW ECC support.
> - *     The AG-AND chips have nice features for speed improvement,
>   *     which are not supported yet. Read / program 4 pages in one go.
>   *     BBT table is not serialized, has to be fixed
>   *

You cut this off mid-sentence. Did you mean to cut three lines here,
instead of just one?

> @@ -836,9 +834,7 @@ static int nand_wait(struct mtd_info *mtd, struct nand_chip *chip)
>          */
>         ndelay(100);
>
> -       if ((state == FL_ERASING) && (chip->options & NAND_IS_AND))
> -               chip->cmdfunc(mtd, NAND_CMD_STATUS_MULTI, -1, -1);
> -       else
> +       if (state == FL_ERASING)
>                 chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);

This is not a precise refactor. All non-AND flash would previously
have run the STATUS command, but now you make it only run when
FL_ERASING. Shouldn't it just be an unconditional cmdfunc(STATUS)?

>         if (in_interrupt() || oops_in_progress)

...

The rest looks OK to me.

Brian

Re: [PATCH 05/12] mtd: nand: remove AG-AND support

[Artem Bityutskiy]

On Mon, 2013-03-04 at 10:56 -0800, Brian Norris wrote:
> On Mon, Mar 4, 2013 at 8:42 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> >
> > We have only one AG-AND driver and it was not touched since 2005. It looks
> > like AG-AND was not really make it to mass-production and can be considered
> > a dead technology.
> 
> You might want to include in your commit message that BBT_AUTO_REFRESH
> was only needed for AG-AND. You drop it here (which is perfectly
> reasonable) but it is technically provided as a generic feature which
> *could* be used outside of AG-AND.

OK, will do, thanks!

> > @@ -22,7 +21,6 @@
> >   *     Enable cached programming for 2k page size chips
> >   *     Check, if mtd->ecctype should be set to MTD_ECC_HW
> >   *     if we have HW ECC support.
> > - *     The AG-AND chips have nice features for speed improvement,
> >   *     which are not supported yet. Read / program 4 pages in one go.
> >   *     BBT table is not serialized, has to be fixed
> >   *
> 
> You cut this off mid-sentence. Did you mean to cut three lines here,
> instead of just one?

Oops.

> > -       if ((state == FL_ERASING) && (chip->options & NAND_IS_AND))
> > -               chip->cmdfunc(mtd, NAND_CMD_STATUS_MULTI, -1, -1);
> > -       else
> > +       if (state == FL_ERASING)
> >                 chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
> 
> This is not a precise refactor. All non-AND flash would previously
> have run the STATUS command, but now you make it only run when
> FL_ERASING. Shouldn't it just be an unconditional cmdfunc(STATUS)?
 
OOPS!

-- 
Best Regards,
Artem Bityutskiy

[PATCH 06/12] mtd: nand: remove a bunch of unused commands

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 drivers/mtd/nand/cafe_nand.c   |    6 ------
 drivers/mtd/nand/nand_base.c   |   10 ----------
 drivers/mtd/nand/nandsim.c     |    8 --------
 drivers/mtd/nand/nuc900_nand.c |    9 ---------
 include/linux/mtd/nand.h       |   20 --------------------
 5 files changed, 53 deletions(-)

diff --git a/drivers/mtd/nand/cafe_nand.c b/drivers/mtd/nand/cafe_nand.c
index 010d612..a01ccb9 100644
--- a/drivers/mtd/nand/cafe_nand.c
+++ b/drivers/mtd/nand/cafe_nand.c
@@ -303,13 +303,7 @@ static void cafe_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 	case NAND_CMD_SEQIN:
 	case NAND_CMD_RNDIN:
 	case NAND_CMD_STATUS:
-	case NAND_CMD_DEPLETE1:
 	case NAND_CMD_RNDOUT:
-	case NAND_CMD_STATUS_ERROR:
-	case NAND_CMD_STATUS_ERROR0:
-	case NAND_CMD_STATUS_ERROR1:
-	case NAND_CMD_STATUS_ERROR2:
-	case NAND_CMD_STATUS_ERROR3:
 		cafe_writel(cafe, cafe->ctl2, NAND_CTRL2);
 		return;
 	}
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 0e28f55..760ee49 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -669,16 +669,6 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
 	case NAND_CMD_SEQIN:
 	case NAND_CMD_RNDIN:
 	case NAND_CMD_STATUS:
-	case NAND_CMD_DEPLETE1:
-		return;
-
-	case NAND_CMD_STATUS_ERROR:
-	case NAND_CMD_STATUS_ERROR0:
-	case NAND_CMD_STATUS_ERROR1:
-	case NAND_CMD_STATUS_ERROR2:
-	case NAND_CMD_STATUS_ERROR3:
-		/* Read error status commands require only a short delay */
-		udelay(chip->chip_delay);
 		return;
 
 	case NAND_CMD_RESET:
diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
index 891c52a..7199acc 100644
--- a/drivers/mtd/nand/nandsim.c
+++ b/drivers/mtd/nand/nandsim.c
@@ -218,7 +218,6 @@ MODULE_PARM_DESC(bch,		 "Enable BCH ecc and set how many bits should "
 #define STATE_CMD_READOOB      0x00000005 /* read OOB area */
 #define STATE_CMD_ERASE1       0x00000006 /* sector erase first command */
 #define STATE_CMD_STATUS       0x00000007 /* read status */
-#define STATE_CMD_STATUS_M     0x00000008 /* read multi-plane status (isn't implemented) */
 #define STATE_CMD_SEQIN        0x00000009 /* sequential data input */
 #define STATE_CMD_READID       0x0000000A /* read ID */
 #define STATE_CMD_ERASE2       0x0000000B /* sector erase second command */
@@ -406,8 +405,6 @@ static struct nandsim_operations {
 	{OPT_ANY, {STATE_CMD_ERASE1, STATE_ADDR_SEC, STATE_CMD_ERASE2 | ACTION_SECERASE, STATE_READY}},
 	/* Read status */
 	{OPT_ANY, {STATE_CMD_STATUS, STATE_DATAOUT_STATUS, STATE_READY}},
-	/* Read multi-plane status */
-	{OPT_SMARTMEDIA, {STATE_CMD_STATUS_M, STATE_DATAOUT_STATUS_M, STATE_READY}},
 	/* Read ID */
 	{OPT_ANY, {STATE_CMD_READID, STATE_ADDR_ZERO, STATE_DATAOUT_ID, STATE_READY}},
 	/* Large page devices read page */
@@ -1079,8 +1076,6 @@ static char *get_state_name(uint32_t state)
 			return "STATE_CMD_ERASE1";
 		case STATE_CMD_STATUS:
 			return "STATE_CMD_STATUS";
-		case STATE_CMD_STATUS_M:
-			return "STATE_CMD_STATUS_M";
 		case STATE_CMD_SEQIN:
 			return "STATE_CMD_SEQIN";
 		case STATE_CMD_READID:
@@ -1145,7 +1140,6 @@ static int check_command(int cmd)
 	case NAND_CMD_RNDOUTSTART:
 		return 0;
 
-	case NAND_CMD_STATUS_MULTI:
 	default:
 		return 1;
 	}
@@ -1171,8 +1165,6 @@ static uint32_t get_state_by_command(unsigned command)
 			return STATE_CMD_ERASE1;
 		case NAND_CMD_STATUS:
 			return STATE_CMD_STATUS;
-		case NAND_CMD_STATUS_MULTI:
-			return STATE_CMD_STATUS_M;
 		case NAND_CMD_SEQIN:
 			return STATE_CMD_SEQIN;
 		case NAND_CMD_READID:
diff --git a/drivers/mtd/nand/nuc900_nand.c b/drivers/mtd/nand/nuc900_nand.c
index a619119..cd6be2e 100644
--- a/drivers/mtd/nand/nuc900_nand.c
+++ b/drivers/mtd/nand/nuc900_nand.c
@@ -177,15 +177,6 @@ static void nuc900_nand_command_lp(struct mtd_info *mtd, unsigned int command,
 	case NAND_CMD_SEQIN:
 	case NAND_CMD_RNDIN:
 	case NAND_CMD_STATUS:
-	case NAND_CMD_DEPLETE1:
-		return;
-
-	case NAND_CMD_STATUS_ERROR:
-	case NAND_CMD_STATUS_ERROR0:
-	case NAND_CMD_STATUS_ERROR1:
-	case NAND_CMD_STATUS_ERROR2:
-	case NAND_CMD_STATUS_ERROR3:
-		udelay(chip->chip_delay);
 		return;
 
 	case NAND_CMD_RESET:
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 2aba049..69097e8 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -86,7 +86,6 @@ extern int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
 #define NAND_CMD_READOOB	0x50
 #define NAND_CMD_ERASE1		0x60
 #define NAND_CMD_STATUS		0x70
-#define NAND_CMD_STATUS_MULTI	0x71
 #define NAND_CMD_SEQIN		0x80
 #define NAND_CMD_RNDIN		0x85
 #define NAND_CMD_READID		0x90
@@ -105,25 +104,6 @@ extern int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
 #define NAND_CMD_RNDOUTSTART	0xE0
 #define NAND_CMD_CACHEDPROG	0x15
 
-/* Extended commands for AG-AND device */
-/*
- * Note: the command for NAND_CMD_DEPLETE1 is really 0x00 but
- *       there is no way to distinguish that from NAND_CMD_READ0
- *       until the remaining sequence of commands has been completed
- *       so add a high order bit and mask it off in the command.
- */
-#define NAND_CMD_DEPLETE1	0x100
-#define NAND_CMD_DEPLETE2	0x38
-#define NAND_CMD_STATUS_MULTI	0x71
-#define NAND_CMD_STATUS_ERROR	0x72
-/* multi-bank error status (banks 0-3) */
-#define NAND_CMD_STATUS_ERROR0	0x73
-#define NAND_CMD_STATUS_ERROR1	0x74
-#define NAND_CMD_STATUS_ERROR2	0x75
-#define NAND_CMD_STATUS_ERROR3	0x76
-#define NAND_CMD_STATUS_RESET	0x7f
-#define NAND_CMD_STATUS_CLEAR	0xff
-
 #define NAND_CMD_NONE		-1
 
 /* Status bits */
-- 
1.7.10.4

Re: [PATCH 06/12] mtd: nand: remove a bunch of unused commands

[Brian Norris]

+ Alexander

On Mon, Mar 4, 2013 at 8:42 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> ---
>  drivers/mtd/nand/cafe_nand.c   |    6 ------
>  drivers/mtd/nand/nand_base.c   |   10 ----------
>  drivers/mtd/nand/nandsim.c     |    8 --------
>  drivers/mtd/nand/nuc900_nand.c |    9 ---------
>  include/linux/mtd/nand.h       |   20 --------------------
>  5 files changed, 53 deletions(-)
>

... trimmed ...

> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 2aba049..69097e8 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -86,7 +86,6 @@ extern int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
>  #define NAND_CMD_READOOB       0x50
>  #define NAND_CMD_ERASE1                0x60
>  #define NAND_CMD_STATUS                0x70
> -#define NAND_CMD_STATUS_MULTI  0x71
>  #define NAND_CMD_SEQIN         0x80
>  #define NAND_CMD_RNDIN         0x85
>  #define NAND_CMD_READID                0x90
> @@ -105,25 +104,6 @@ extern int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
>  #define NAND_CMD_RNDOUTSTART   0xE0
>  #define NAND_CMD_CACHEDPROG    0x15
>
> -/* Extended commands for AG-AND device */
> -/*
> - * Note: the command for NAND_CMD_DEPLETE1 is really 0x00 but
> - *       there is no way to distinguish that from NAND_CMD_READ0
> - *       until the remaining sequence of commands has been completed
> - *       so add a high order bit and mask it off in the command.
> - */
> -#define NAND_CMD_DEPLETE1      0x100

Perhaps this is the reason for the "unnecessary command masking" noted
by Alexander? There is one instance of a command function which masks
command & 0xff. Maybe Alexander's patch can be updated to mention this
likely cause for the original masking and can be applied on top of
Artem's cleanup series? Anyway, the mask was likely not used anyway,
since (as Artem mentions in this patch series) AG-AND had very little
general use (or none?).

> -#define NAND_CMD_DEPLETE2      0x38
> -#define NAND_CMD_STATUS_MULTI  0x71
> -#define NAND_CMD_STATUS_ERROR  0x72
> -/* multi-bank error status (banks 0-3) */
> -#define NAND_CMD_STATUS_ERROR0 0x73
> -#define NAND_CMD_STATUS_ERROR1 0x74
> -#define NAND_CMD_STATUS_ERROR2 0x75
> -#define NAND_CMD_STATUS_ERROR3 0x76
> -#define NAND_CMD_STATUS_RESET  0x7f
> -#define NAND_CMD_STATUS_CLEAR  0xff
> -
>  #define NAND_CMD_NONE          -1
>
>  /* Status bits */

Brian

Re[2]: [PATCH 06/12] mtd: nand: remove a bunch of unused commands

[Alexander Shiyan]

> On Mon, Mar 4, 2013 at 8:42 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> >
> > Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> > ---
> >  drivers/mtd/nand/cafe_nand.c   |    6 ------
> >  drivers/mtd/nand/nand_base.c   |   10 ----------
> >  drivers/mtd/nand/nandsim.c     |    8 --------
> >  drivers/mtd/nand/nuc900_nand.c |    9 ---------
> >  include/linux/mtd/nand.h       |   20 --------------------
> >  5 files changed, 53 deletions(-)
> >
> 
> ... trimmed ...
...
> > - * Note: the command for NAND_CMD_DEPLETE1 is really 0x00 but
> > - *       there is no way to distinguish that from NAND_CMD_READ0
> > - *       until the remaining sequence of commands has been completed
> > - *       so add a high order bit and mask it off in the command.
> > - */
> > -#define NAND_CMD_DEPLETE1      0x100
> 
> Perhaps this is the reason for the "unnecessary command masking" noted
> by Alexander? There is one instance of a command function which masks
> command & 0xff. Maybe Alexander's patch can be updated to mention this
> likely cause for the original masking and can be applied on top of
> Artem's cleanup series? Anyway, the mask was likely not used anyway,
> since (as Artem mentions in this patch series) AG-AND had very little
> general use (or none?).

My patch is cleanup only. I think that the mask was originally included to
limit the byte boundaries. However, this is not necessary because callback
functions "cmd_ctrl" use writeb/iowrite8 for send command, ie do not allow
the use of the wrong size. Address also passed to "cmd_ctrl" via "cmd"
parameter, which is not masked but only shifted, i.e. we have a values
above 0xff, so that is not a problem.

---

Re: Re[2]: [PATCH 06/12] mtd: nand: remove a bunch of unused commands

[Brian Norris]

On Mon, Mar 4, 2013 at 11:29 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
>> On Mon, Mar 4, 2013 at 8:42 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
>> > From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>> >
>> > Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>> > ---
>> >  drivers/mtd/nand/cafe_nand.c   |    6 ------
>> >  drivers/mtd/nand/nand_base.c   |   10 ----------
>> >  drivers/mtd/nand/nandsim.c     |    8 --------
>> >  drivers/mtd/nand/nuc900_nand.c |    9 ---------
>> >  include/linux/mtd/nand.h       |   20 --------------------
>> >  5 files changed, 53 deletions(-)
>> >
>>
>> ... trimmed ...
> ...
>> > - * Note: the command for NAND_CMD_DEPLETE1 is really 0x00 but
>> > - *       there is no way to distinguish that from NAND_CMD_READ0
>> > - *       until the remaining sequence of commands has been completed
>> > - *       so add a high order bit and mask it off in the command.
>> > - */
>> > -#define NAND_CMD_DEPLETE1      0x100
>>
>> Perhaps this is the reason for the "unnecessary command masking" noted
>> by Alexander? There is one instance of a command function which masks
>> command & 0xff. Maybe Alexander's patch can be updated to mention this
>> likely cause for the original masking and can be applied on top of
>> Artem's cleanup series? Anyway, the mask was likely not used anyway,
>> since (as Artem mentions in this patch series) AG-AND had very little
>> general use (or none?).
>
> My patch is cleanup only. I think that the mask was originally included to
> limit the byte boundaries. However, this is not necessary because callback
> functions "cmd_ctrl" use writeb/iowrite8 for send command, ie do not allow
> the use of the wrong size. Address also passed to "cmd_ctrl" via "cmd"
> parameter, which is not masked but only shifted, i.e. we have a values
> above 0xff, so that is not a problem.

To be clear, I'm referring to your patch:

  [PATCH 1/3] mtd: nand_base: Removed unnecessary command masking

The function that you are editing (nand_command_lp) can be used for
AG-AND devices. The driver (rtc_from4.c - slated for removal in this
series) can send NAND_CMD_DEPLETE1, which according to the comments in
nand.h that I highlight above, should be "mask[ed] ... off in the
command". So without Artem's change, your patch is actually breaking
rtc_from4.c. Unforunately, no one bothered to actually document this
within nand_command_lp, but such is life.

Brian

Re[4]: [PATCH 06/12] mtd: nand: remove a bunch of unused commands

[Alexander Shiyan]

> On Mon, Mar 4, 2013 at 11:29 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
> >> On Mon, Mar 4, 2013 at 8:42 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> >> > From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> >> >
> >> > Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> >> > ---
> >> >  drivers/mtd/nand/cafe_nand.c   |    6 ------
> >> >  drivers/mtd/nand/nand_base.c   |   10 ----------
> >> >  drivers/mtd/nand/nandsim.c     |    8 --------
> >> >  drivers/mtd/nand/nuc900_nand.c |    9 ---------
> >> >  include/linux/mtd/nand.h       |   20 --------------------
> >> >  5 files changed, 53 deletions(-)
> >> >
> >>
> >> ... trimmed ...
> > ...
> >> > - * Note: the command for NAND_CMD_DEPLETE1 is really 0x00 but
> >> > - *       there is no way to distinguish that from NAND_CMD_READ0
> >> > - *       until the remaining sequence of commands has been completed
> >> > - *       so add a high order bit and mask it off in the command.
> >> > - */
> >> > -#define NAND_CMD_DEPLETE1      0x100
> >>
> >> Perhaps this is the reason for the "unnecessary command masking" noted
> >> by Alexander? There is one instance of a command function which masks
> >> command & 0xff. Maybe Alexander's patch can be updated to mention this
> >> likely cause for the original masking and can be applied on top of
> >> Artem's cleanup series? Anyway, the mask was likely not used anyway,
> >> since (as Artem mentions in this patch series) AG-AND had very little
> >> general use (or none?).
> >
> > My patch is cleanup only. I think that the mask was originally included to
> > limit the byte boundaries. However, this is not necessary because callback
> > functions "cmd_ctrl" use writeb/iowrite8 for send command, ie do not allow
> > the use of the wrong size. Address also passed to "cmd_ctrl" via "cmd"
> > parameter, which is not masked but only shifted, i.e. we have a values
> > above 0xff, so that is not a problem.
> 
> To be clear, I'm referring to your patch:
> 
>   [PATCH 1/3] mtd: nand_base: Removed unnecessary command masking
> 
> The function that you are editing (nand_command_lp) can be used for

OK, for Samsung chips too.

> AG-AND devices. The driver (rtc_from4.c - slated for removal in this
> series) can send NAND_CMD_DEPLETE1, which according to the comments in
> nand.h that I highlight above, should be "mask[ed] ... off in the
> command". So without Artem's change, your patch is actually breaking

Where it breaks? "rtc_from4_hwcontrol" use writeb() for "cmd" parameter,
so its not breaks. Is I think incorrect?

> rtc_from4.c. Unforunately, no one bothered to actually document this
> within nand_command_lp, but such is life.

---

Re: Re[4]: [PATCH 06/12] mtd: nand: remove a bunch of unused commands

[Brian Norris]

On Mon, Mar 4, 2013 at 12:03 PM, Alexander Shiyan <shc_work@mail.ru> wrote:
>> On Mon, Mar 4, 2013 at 11:29 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
>> >> On Mon, Mar 4, 2013 at 8:42 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
>> >> > From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>> >> >
>> >> > Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>> >> > ---
>> >> >  drivers/mtd/nand/cafe_nand.c   |    6 ------
>> >> >  drivers/mtd/nand/nand_base.c   |   10 ----------
>> >> >  drivers/mtd/nand/nandsim.c     |    8 --------
>> >> >  drivers/mtd/nand/nuc900_nand.c |    9 ---------
>> >> >  include/linux/mtd/nand.h       |   20 --------------------
>> >> >  5 files changed, 53 deletions(-)
>> >> >
>> >>
>> >> ... trimmed ...
>> > ...
>> >> > - * Note: the command for NAND_CMD_DEPLETE1 is really 0x00 but
>> >> > - *       there is no way to distinguish that from NAND_CMD_READ0
>> >> > - *       until the remaining sequence of commands has been completed
>> >> > - *       so add a high order bit and mask it off in the command.
>> >> > - */
>> >> > -#define NAND_CMD_DEPLETE1      0x100
>> >>
>> >> Perhaps this is the reason for the "unnecessary command masking" noted
>> >> by Alexander? There is one instance of a command function which masks
>> >> command & 0xff. Maybe Alexander's patch can be updated to mention this
>> >> likely cause for the original masking and can be applied on top of
>> >> Artem's cleanup series? Anyway, the mask was likely not used anyway,
>> >> since (as Artem mentions in this patch series) AG-AND had very little
>> >> general use (or none?).
>> >
>> > My patch is cleanup only. I think that the mask was originally included to
>> > limit the byte boundaries. However, this is not necessary because callback
>> > functions "cmd_ctrl" use writeb/iowrite8 for send command, ie do not allow
>> > the use of the wrong size. Address also passed to "cmd_ctrl" via "cmd"
>> > parameter, which is not masked but only shifted, i.e. we have a values
>> > above 0xff, so that is not a problem.
>>
>> To be clear, I'm referring to your patch:
>>
>>   [PATCH 1/3] mtd: nand_base: Removed unnecessary command masking
>>
>> The function that you are editing (nand_command_lp) can be used for
>
> OK, for Samsung chips too.

Sure, and many others. But the AG-AND are the only case where you
would use a command that is not 8 bits anyway.

>> AG-AND devices. The driver (rtc_from4.c - slated for removal in this
>> series) can send NAND_CMD_DEPLETE1, which according to the comments in
>> nand.h that I highlight above, should be "mask[ed] ... off in the
>> command". So without Artem's change, your patch is actually breaking
>
> Where it breaks? "rtc_from4_hwcontrol" use writeb() for "cmd" parameter,
> so its not breaks. Is I think incorrect?

I see. I guess it is correct, but it still hard to follow the logic
that leads me to believe that this is a 100% correct change. I would
appreciate it if:

(1) We remove all commands that are larger than 8-bit (as with Artem's
current patch set)
(2) Alexander explains his change in more detail.

Anyway, I will move discussion of his changes back to his patch set, not here.

>> rtc_from4.c. Unforunately, no one bothered to actually document this
>> within nand_command_lp, but such is life.
>
> ---

Brian

[PATCH 07/12] mtd: nand: remove NAND_NO_PADDING macro

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

It is not used anywhere.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 include/linux/mtd/nand.h |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 69097e8..822bc48c 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -145,8 +145,6 @@ typedef enum {
  */
 /* Buswidth is 16 bit */
 #define NAND_BUSWIDTH_16	0x00000002
-/* Device supports partial programming without padding */
-#define NAND_NO_PADDING		0x00000004
 /* Chip has cache program function */
 #define NAND_CACHEPRG		0x00000008
 /* Chip has copy back function */
@@ -164,11 +162,9 @@ typedef enum {
 #define NAND_SUBPAGE_READ	0x00001000
 
 /* Options valid for Samsung large page devices */
-#define NAND_SAMSUNG_LP_OPTIONS \
-	(NAND_NO_PADDING | NAND_CACHEPRG | NAND_COPYBACK)
+#define NAND_SAMSUNG_LP_OPTIONS (NAND_CACHEPRG | NAND_COPYBACK)
 
 /* Macros to identify the above */
-#define NAND_MUST_PAD(chip) (!(chip->options & NAND_NO_PADDING))
 #define NAND_HAS_CACHEPROG(chip) ((chip->options & NAND_CACHEPRG))
 #define NAND_HAS_COPYBACK(chip) ((chip->options & NAND_COPYBACK))
 #define NAND_HAS_SUBPAGE_READ(chip) ((chip->options & NAND_SUBPAGE_READ))
-- 
1.7.10.4

[PATCH 08/12] mtd: nand: remove NAND_COPYBACK macro

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

It is unused.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 include/linux/mtd/nand.h |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 822bc48c..9af859f 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -147,8 +147,6 @@ typedef enum {
 #define NAND_BUSWIDTH_16	0x00000002
 /* Chip has cache program function */
 #define NAND_CACHEPRG		0x00000008
-/* Chip has copy back function */
-#define NAND_COPYBACK		0x00000010
 /* Chip does not allow subpage writes */
 #define NAND_NO_SUBPAGE_WRITE	0x00000200
 
@@ -162,11 +160,10 @@ typedef enum {
 #define NAND_SUBPAGE_READ	0x00001000
 
 /* Options valid for Samsung large page devices */
-#define NAND_SAMSUNG_LP_OPTIONS (NAND_CACHEPRG | NAND_COPYBACK)
+#define NAND_SAMSUNG_LP_OPTIONS (NAND_CACHEPRG)
 
 /* Macros to identify the above */
 #define NAND_HAS_CACHEPROG(chip) ((chip->options & NAND_CACHEPRG))
-#define NAND_HAS_COPYBACK(chip) ((chip->options & NAND_COPYBACK))
 #define NAND_HAS_SUBPAGE_READ(chip) ((chip->options & NAND_SUBPAGE_READ))
 
 /* Non chip related options */
-- 
1.7.10.4

[PATCH 09/12] mtd: nand: use NAND_HAS_CACHEPROG

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

We have this unused macro, let's use it and justify its existence.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 drivers/mtd/nand/nand_base.c |    2 +-
 include/linux/mtd/nand.h     |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 760ee49..e607565 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2045,7 +2045,7 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	 */
 	cached = 0;
 
-	if (!cached || !(chip->options & NAND_CACHEPRG)) {
+	if (!cached || !NAND_HAS_CACHEPROG(chip)) {
 
 		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
 		status = chip->waitfunc(mtd, chip);
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 9af859f..59dee20 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -160,7 +160,7 @@ typedef enum {
 #define NAND_SUBPAGE_READ	0x00001000
 
 /* Options valid for Samsung large page devices */
-#define NAND_SAMSUNG_LP_OPTIONS (NAND_CACHEPRG)
+#define NAND_SAMSUNG_LP_OPTIONS NAND_CACHEPRG
 
 /* Macros to identify the above */
 #define NAND_HAS_CACHEPROG(chip) ((chip->options & NAND_CACHEPRG))
-- 
1.7.10.4

[PATCH 10/12] mtd: nand_ids: introduce helper macros

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

Introduce helper macros for defining NAND chips. These macros do not really add
much value in the current code-base. However, we are going to add full ID
support which adds some more complexity to the table, and helper macros become
useful for readability.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 drivers/mtd/nand/nand_ids.c  |  159 +++++++++++++++++++++---------------------
 drivers/mtd/nand/sm_common.c |   64 ++++++++---------
 include/linux/mtd/nand.h     |   17 +++++
 3 files changed, 127 insertions(+), 113 deletions(-)

diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index 7c5dec18..f04942c 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -15,37 +15,34 @@
 #define LP_OPTIONS16 (LP_OPTIONS | NAND_BUSWIDTH_16)
 
 /*
- * The chip ID list:
- *    name, device ID, page size, chip size in MiB, eraseblock size, options
- *
- * If page size and eraseblock size is 0, the sizes are taken from the extended
- * chip ID.
+ * The list of supported chips.  If page size and eraseblock size is 0, the
+ * sizes are taken from the extended chip ID.
  */
 struct nand_flash_dev nand_flash_ids[] = {
-	{"NAND 16MiB 1,8V 8-bit",	0x33, 512, 16, 0x4000, 0},
-	{"NAND 16MiB 3,3V 8-bit",	0x73, 512, 16, 0x4000, 0},
-	{"NAND 16MiB 1,8V 16-bit",	0x43, 512, 16, 0x4000, NAND_BUSWIDTH_16},
-	{"NAND 16MiB 3,3V 16-bit",	0x53, 512, 16, 0x4000, NAND_BUSWIDTH_16},
-
-	{"NAND 32MiB 1,8V 8-bit",	0x35, 512, 32, 0x4000, 0},
-	{"NAND 32MiB 3,3V 8-bit",	0x75, 512, 32, 0x4000, 0},
-	{"NAND 32MiB 1,8V 16-bit",	0x45, 512, 32, 0x4000, NAND_BUSWIDTH_16},
-	{"NAND 32MiB 3,3V 16-bit",	0x55, 512, 32, 0x4000, NAND_BUSWIDTH_16},
-
-	{"NAND 64MiB 1,8V 8-bit",	0x36, 512, 64, 0x4000, 0},
-	{"NAND 64MiB 3,3V 8-bit",	0x76, 512, 64, 0x4000, 0},
-	{"NAND 64MiB 1,8V 16-bit",	0x46, 512, 64, 0x4000, NAND_BUSWIDTH_16},
-	{"NAND 64MiB 3,3V 16-bit",	0x56, 512, 64, 0x4000, NAND_BUSWIDTH_16},
-
-	{"NAND 128MiB 1,8V 8-bit",	0x78, 512, 128, 0x4000, 0},
-	{"NAND 128MiB 1,8V 8-bit",	0x39, 512, 128, 0x4000, 0},
-	{"NAND 128MiB 3,3V 8-bit",	0x79, 512, 128, 0x4000, 0},
-	{"NAND 128MiB 1,8V 16-bit",	0x72, 512, 128, 0x4000, NAND_BUSWIDTH_16},
-	{"NAND 128MiB 1,8V 16-bit",	0x49, 512, 128, 0x4000, NAND_BUSWIDTH_16},
-	{"NAND 128MiB 3,3V 16-bit",	0x74, 512, 128, 0x4000, NAND_BUSWIDTH_16},
-	{"NAND 128MiB 3,3V 16-bit",	0x59, 512, 128, 0x4000, NAND_BUSWIDTH_16},
-
-	{"NAND 256MiB 3,3V 8-bit",	0x71, 512, 256, 0x4000, 0},
+	TABLE_NAND("NAND 16MiB 1,8V 8-bit",  0x33, 512, 16, 0x4000, 0),
+	TABLE_NAND("NAND 16MiB 3,3V 8-bit",  0x73, 512, 16, 0x4000, 0),
+	TABLE_NAND("NAND 16MiB 1,8V 16-bit", 0x43, 512, 16, 0x4000, NAND_BUSWIDTH_16),
+	TABLE_NAND("NAND 16MiB 3,3V 16-bit", 0x53, 512, 16, 0x4000, NAND_BUSWIDTH_16),
+
+	TABLE_NAND("NAND 32MiB 1,8V 8-bit",  0x35, 512, 32, 0x4000, 0),
+	TABLE_NAND("NAND 32MiB 3,3V 8-bit",  0x75, 512, 32, 0x4000, 0),
+	TABLE_NAND("NAND 32MiB 1,8V 16-bit", 0x45, 512, 32, 0x4000, NAND_BUSWIDTH_16),
+	TABLE_NAND("NAND 32MiB 3,3V 16-bit", 0x55, 512, 32, 0x4000, NAND_BUSWIDTH_16),
+
+	TABLE_NAND("NAND 64MiB 1,8V 8-bit",  0x36, 512, 64, 0x4000, 0),
+	TABLE_NAND("NAND 64MiB 3,3V 8-bit",  0x76, 512, 64, 0x4000, 0),
+	TABLE_NAND("NAND 64MiB 1,8V 16-bit", 0x46, 512, 64, 0x4000, NAND_BUSWIDTH_16),
+	TABLE_NAND("NAND 64MiB 3,3V 16-bit", 0x56, 512, 64, 0x4000, NAND_BUSWIDTH_16),
+
+	TABLE_NAND("NAND 128MiB 1,8V 8-bit",  0x78, 512, 128, 0x4000, 0),
+	TABLE_NAND("NAND 128MiB 1,8V 8-bit",  0x39, 512, 128, 0x4000, 0),
+	TABLE_NAND("NAND 128MiB 3,3V 8-bit",  0x79, 512, 128, 0x4000, 0),
+	TABLE_NAND("NAND 128MiB 1,8V 16-bit", 0x72, 512, 128, 0x4000, NAND_BUSWIDTH_16),
+	TABLE_NAND("NAND 128MiB 1,8V 16-bit", 0x49, 512, 128, 0x4000, NAND_BUSWIDTH_16),
+	TABLE_NAND("NAND 128MiB 3,3V 16-bit", 0x74, 512, 128, 0x4000, NAND_BUSWIDTH_16),
+	TABLE_NAND("NAND 128MiB 3,3V 16-bit", 0x59, 512, 128, 0x4000, NAND_BUSWIDTH_16),
+
+	TABLE_NAND("NAND 256MiB 3,3V 8-bit",  0x71, 512, 256, 0x4000, 0),
 
 	/*
 	 * These are the new chips with large page size, page size and
@@ -53,79 +50,79 @@ struct nand_flash_dev nand_flash_ids[] = {
 	 */
 
 	/* 512 Megabit */
-	{"NAND 64MiB 1,8V 8-bit",	0xA2, 0,  64, 0, LP_OPTIONS},
-	{"NAND 64MiB 1,8V 8-bit",	0xA0, 0,  64, 0, LP_OPTIONS},
-	{"NAND 64MiB 3,3V 8-bit",	0xF2, 0,  64, 0, LP_OPTIONS},
-	{"NAND 64MiB 3,3V 8-bit",	0xD0, 0,  64, 0, LP_OPTIONS},
-	{"NAND 64MiB 3,3V 8-bit",	0xF0, 0,  64, 0, LP_OPTIONS},
-	{"NAND 64MiB 1,8V 16-bit",	0xB2, 0,  64, 0, LP_OPTIONS16},
-	{"NAND 64MiB 1,8V 16-bit",	0xB0, 0,  64, 0, LP_OPTIONS16},
-	{"NAND 64MiB 3,3V 16-bit",	0xC2, 0,  64, 0, LP_OPTIONS16},
-	{"NAND 64MiB 3,3V 16-bit",	0xC0, 0,  64, 0, LP_OPTIONS16},
+	EXTENDED_ID_NAND("NAND 64MiB 1,8V 8-bit",  0xA2,  64, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 64MiB 1,8V 8-bit",  0xA0,  64, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 64MiB 3,3V 8-bit",  0xF2,  64, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 64MiB 3,3V 8-bit",  0xD0,  64, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 64MiB 3,3V 8-bit",  0xF0,  64, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 64MiB 1,8V 16-bit", 0xB2,  64, LP_OPTIONS16),
+	EXTENDED_ID_NAND("NAND 64MiB 1,8V 16-bit", 0xB0,  64, LP_OPTIONS16),
+	EXTENDED_ID_NAND("NAND 64MiB 3,3V 16-bit", 0xC2,  64, LP_OPTIONS16),
+	EXTENDED_ID_NAND("NAND 64MiB 3,3V 16-bit", 0xC0,  64, LP_OPTIONS16),
 
 	/* 1 Gigabit */
-	{"NAND 128MiB 1,8V 8-bit",	0xA1, 0, 128, 0, LP_OPTIONS},
-	{"NAND 128MiB 3,3V 8-bit",	0xF1, 0, 128, 0, LP_OPTIONS},
-	{"NAND 128MiB 3,3V 8-bit",	0xD1, 0, 128, 0, LP_OPTIONS},
-	{"NAND 128MiB 1,8V 16-bit",	0xB1, 0, 128, 0, LP_OPTIONS16},
-	{"NAND 128MiB 3,3V 16-bit",	0xC1, 0, 128, 0, LP_OPTIONS16},
-	{"NAND 128MiB 1,8V 16-bit",     0xAD, 0, 128, 0, LP_OPTIONS16},
+	EXTENDED_ID_NAND("NAND 128MiB 1,8V 8-bit",  0xA1, 128, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 128MiB 3,3V 8-bit",  0xF1, 128, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 128MiB 3,3V 8-bit",  0xD1, 128, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 128MiB 1,8V 16-bit", 0xB1, 128, LP_OPTIONS16),
+	EXTENDED_ID_NAND("NAND 128MiB 3,3V 16-bit", 0xC1, 128, LP_OPTIONS16),
+	EXTENDED_ID_NAND("NAND 128MiB 1,8V 16-bit", 0xAD, 128, LP_OPTIONS16),
 
 	/* 2 Gigabit */
-	{"NAND 256MiB 1,8V 8-bit",	0xAA, 0, 256, 0, LP_OPTIONS},
-	{"NAND 256MiB 3,3V 8-bit",	0xDA, 0, 256, 0, LP_OPTIONS},
-	{"NAND 256MiB 1,8V 16-bit",	0xBA, 0, 256, 0, LP_OPTIONS16},
-	{"NAND 256MiB 3,3V 16-bit",	0xCA, 0, 256, 0, LP_OPTIONS16},
+	EXTENDED_ID_NAND("NAND 256MiB 1,8V 8-bit",  0xAA, 256, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 256MiB 3,3V 8-bit",  0xDA, 256, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 256MiB 1,8V 16-bit", 0xBA, 256, LP_OPTIONS16),
+	EXTENDED_ID_NAND("NAND 256MiB 3,3V 16-bit", 0xCA, 256, LP_OPTIONS16),
 
 	/* 4 Gigabit */
-	{"NAND 512MiB 1,8V 8-bit",	0xAC, 0, 512, 0, LP_OPTIONS},
-	{"NAND 512MiB 3,3V 8-bit",	0xDC, 0, 512, 0, LP_OPTIONS},
-	{"NAND 512MiB 1,8V 16-bit",	0xBC, 0, 512, 0, LP_OPTIONS16},
-	{"NAND 512MiB 3,3V 16-bit",	0xCC, 0, 512, 0, LP_OPTIONS16},
+	EXTENDED_ID_NAND("NAND 512MiB 1,8V 8-bit",  0xAC, 512, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 512MiB 3,3V 8-bit",  0xDC, 512, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 512MiB 1,8V 16-bit", 0xBC, 512, LP_OPTIONS16),
+	EXTENDED_ID_NAND("NAND 512MiB 3,3V 16-bit", 0xCC, 512, LP_OPTIONS16),
 
 	/* 8 Gigabit */
-	{"NAND 1GiB 1,8V 8-bit",	0xA3, 0, 1024, 0, LP_OPTIONS},
-	{"NAND 1GiB 3,3V 8-bit",	0xD3, 0, 1024, 0, LP_OPTIONS},
-	{"NAND 1GiB 1,8V 16-bit",	0xB3, 0, 1024, 0, LP_OPTIONS16},
-	{"NAND 1GiB 3,3V 16-bit",	0xC3, 0, 1024, 0, LP_OPTIONS16},
+	EXTENDED_ID_NAND("NAND 1GiB 1,8V 8-bit",  0xA3, 1024, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 1GiB 3,3V 8-bit",  0xD3, 1024, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 1GiB 1,8V 16-bit", 0xB3, 1024, LP_OPTIONS16),
+	EXTENDED_ID_NAND("NAND 1GiB 3,3V 16-bit", 0xC3, 1024, LP_OPTIONS16),
 
 	/* 16 Gigabit */
-	{"NAND 2GiB 1,8V 8-bit",	0xA5, 0, 2048, 0, LP_OPTIONS},
-	{"NAND 2GiB 3,3V 8-bit",	0xD5, 0, 2048, 0, LP_OPTIONS},
-	{"NAND 2GiB 1,8V 16-bit",	0xB5, 0, 2048, 0, LP_OPTIONS16},
-	{"NAND 2GiB 3,3V 16-bit",	0xC5, 0, 2048, 0, LP_OPTIONS16},
+	EXTENDED_ID_NAND("NAND 2GiB 1,8V 8-bit",  0xA5, 2048, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 2GiB 3,3V 8-bit",  0xD5, 2048, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 2GiB 1,8V 16-bit", 0xB5, 2048, LP_OPTIONS16),
+	EXTENDED_ID_NAND("NAND 2GiB 3,3V 16-bit", 0xC5, 2048, LP_OPTIONS16),
 
 	/* 32 Gigabit */
-	{"NAND 4GiB 1,8V 8-bit",	0xA7, 0, 4096, 0, LP_OPTIONS},
-	{"NAND 4GiB 3,3V 8-bit",	0xD7, 0, 4096, 0, LP_OPTIONS},
-	{"NAND 4GiB 1,8V 16-bit",	0xB7, 0, 4096, 0, LP_OPTIONS16},
-	{"NAND 4GiB 3,3V 16-bit",	0xC7, 0, 4096, 0, LP_OPTIONS16},
+	EXTENDED_ID_NAND("NAND 4GiB 1,8V 8-bit",  0xA7, 4096, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 4GiB 3,3V 8-bit",  0xD7, 4096, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 4GiB 1,8V 16-bit", 0xB7, 4096, LP_OPTIONS16),
+	EXTENDED_ID_NAND("NAND 4GiB 3,3V 16-bit", 0xC7, 4096, LP_OPTIONS16),
 
 	/* 64 Gigabit */
-	{"NAND 8GiB 1,8V 8-bit",	0xAE, 0, 8192, 0, LP_OPTIONS},
-	{"NAND 8GiB 3,3V 8-bit",	0xDE, 0, 8192, 0, LP_OPTIONS},
-	{"NAND 8GiB 1,8V 16-bit",	0xBE, 0, 8192, 0, LP_OPTIONS16},
-	{"NAND 8GiB 3,3V 16-bit",	0xCE, 0, 8192, 0, LP_OPTIONS16},
+	EXTENDED_ID_NAND("NAND 8GiB 1,8V 8-bit",  0xAE, 8192, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 8GiB 3,3V 8-bit",  0xDE, 8192, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 8GiB 1,8V 16-bit", 0xBE, 8192, LP_OPTIONS16),
+	EXTENDED_ID_NAND("NAND 8GiB 3,3V 16-bit", 0xCE, 8192, LP_OPTIONS16),
 
 	/* 128 Gigabit */
-	{"NAND 16GiB 1,8V 8-bit",	0x1A, 0, 16384, 0, LP_OPTIONS},
-	{"NAND 16GiB 3,3V 8-bit",	0x3A, 0, 16384, 0, LP_OPTIONS},
-	{"NAND 16GiB 1,8V 16-bit",	0x2A, 0, 16384, 0, LP_OPTIONS16},
-	{"NAND 16GiB 3,3V 16-bit",	0x4A, 0, 16384, 0, LP_OPTIONS16},
+	EXTENDED_ID_NAND("NAND 16GiB 1,8V 8-bit",  0x1A, 16384, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 16GiB 3,3V 8-bit",  0x3A, 16384, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 16GiB 1,8V 16-bit", 0x2A, 16384, LP_OPTIONS16),
+	EXTENDED_ID_NAND("NAND 16GiB 3,3V 16-bit", 0x4A, 16384, LP_OPTIONS16),
 
 	/* 256 Gigabit */
-	{"NAND 32GiB 1,8V 8-bit",	0x1C, 0, 32768, 0, LP_OPTIONS},
-	{"NAND 32GiB 3,3V 8-bit",	0x3C, 0, 32768, 0, LP_OPTIONS},
-	{"NAND 32GiB 1,8V 16-bit",	0x2C, 0, 32768, 0, LP_OPTIONS16},
-	{"NAND 32GiB 3,3V 16-bit",	0x4C, 0, 32768, 0, LP_OPTIONS16},
+	EXTENDED_ID_NAND("NAND 32GiB 1,8V 8-bit",  0x1C, 32768, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 32GiB 3,3V 8-bit",  0x3C, 32768, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 32GiB 1,8V 16-bit", 0x2C, 32768, LP_OPTIONS16),
+	EXTENDED_ID_NAND("NAND 32GiB 3,3V 16-bit", 0x4C, 32768, LP_OPTIONS16),
 
 	/* 512 Gigabit */
-	{"NAND 64GiB 1,8V 8-bit",	0x1E, 0, 65536, 0, LP_OPTIONS},
-	{"NAND 64GiB 3,3V 8-bit",	0x3E, 0, 65536, 0, LP_OPTIONS},
-	{"NAND 64GiB 1,8V 16-bit",	0x2E, 0, 65536, 0, LP_OPTIONS16},
-	{"NAND 64GiB 3,3V 16-bit",	0x4E, 0, 65536, 0, LP_OPTIONS16},
+	EXTENDED_ID_NAND("NAND 64GiB 1,8V 8-bit",  0x1E, 65536, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 64GiB 3,3V 8-bit",  0x3E, 65536, LP_OPTIONS),
+	EXTENDED_ID_NAND("NAND 64GiB 1,8V 16-bit", 0x2E, 65536, LP_OPTIONS16),
+	EXTENDED_ID_NAND("NAND 64GiB 3,3V 16-bit", 0x4E, 65536, LP_OPTIONS16),
 
-	{NULL,}
+	{NULL}
 };
 
 /*
diff --git a/drivers/mtd/nand/sm_common.c b/drivers/mtd/nand/sm_common.c
index 082bcdc..85fb95a 100644
--- a/drivers/mtd/nand/sm_common.c
+++ b/drivers/mtd/nand/sm_common.c
@@ -69,42 +69,42 @@ static int sm_block_markbad(struct mtd_info *mtd, loff_t ofs)
 
 
 static struct nand_flash_dev nand_smartmedia_flash_ids[] = {
-	{"SmartMedia 1MiB 5V",          0x6e, 256, 1, 0x1000, 0},
-	{"SmartMedia 1MiB 3,3V",        0xe8, 256, 1, 0x1000, 0},
-	{"SmartMedia 1MiB 3,3V",        0xec, 256, 1, 0x1000, 0},
-	{"SmartMedia 2MiB 3,3V",        0xea, 256, 2, 0x1000, 0},
-	{"SmartMedia 2MiB 5V",          0x64, 256, 2, 0x1000, 0},
-	{"SmartMedia 2MiB 3,3V ROM",    0x5d, 512, 2, 0x2000, NAND_ROM},
-	{"SmartMedia 4MiB 3,3V",        0xe3, 512, 4, 0x2000, 0},
-	{"SmartMedia 4MiB 3,3/5V",      0xe5, 512, 4, 0x2000, 0},
-	{"SmartMedia 4MiB 5V",          0x6b, 512, 4, 0x2000, 0},
-	{"SmartMedia 4MiB 3,3V ROM",    0xd5, 512, 4, 0x2000, NAND_ROM},
-	{"SmartMedia 8MiB 3,3V",        0xe6, 512, 8, 0x2000, 0},
-	{"SmartMedia 8MiB 3,3V ROM",    0xd6, 512, 8, 0x2000, NAND_ROM},
-	{"SmartMedia 16MiB 3,3V",       0x73, 512, 16, 0x4000, 0},
-	{"SmartMedia 16MiB 3,3V ROM",   0x57, 512, 16, 0x4000, NAND_ROM},
-	{"SmartMedia 32MiB 3,3V",       0x75, 512, 32, 0x4000, 0},
-	{"SmartMedia 32MiB 3,3V ROM",   0x58, 512, 32, 0x4000, NAND_ROM},
-	{"SmartMedia 64MiB 3,3V",       0x76, 512, 64, 0x4000, 0},
-	{"SmartMedia 64MiB 3,3V ROM",   0xd9, 512, 64, 0x4000, NAND_ROM},
-	{"SmartMedia 128MiB 3,3V",      0x79, 512, 128, 0x4000, 0},
-	{"SmartMedia 128MiB 3,3V ROM",  0xda, 512, 128, 0x4000, NAND_ROM},
-	{"SmartMedia 256MiB 3,3V",      0x71, 512, 256, 0x4000 },
-	{"SmartMedia 256MiB 3,3V ROM",  0x5b, 512, 256, 0x4000, NAND_ROM},
-	{NULL,}
+	TABLE_NAND("SmartMedia 1MiB 5V",         0x6e, 256, 1, 0x1000, 0),
+	TABLE_NAND("SmartMedia 1MiB 3,3V",       0xe8, 256, 1, 0x1000, 0),
+	TABLE_NAND("SmartMedia 1MiB 3,3V",       0xec, 256, 1, 0x1000, 0),
+	TABLE_NAND("SmartMedia 2MiB 3,3V",       0xea, 256, 2, 0x1000, 0),
+	TABLE_NAND("SmartMedia 2MiB 5V",         0x64, 256, 2, 0x1000, 0),
+	TABLE_NAND("SmartMedia 2MiB 3,3V ROM",   0x5d, 512, 2, 0x2000, NAND_ROM),
+	TABLE_NAND("SmartMedia 4MiB 3,3V",       0xe3, 512, 4, 0x2000, 0),
+	TABLE_NAND("SmartMedia 4MiB 3,3/5V",     0xe5, 512, 4, 0x2000, 0),
+	TABLE_NAND("SmartMedia 4MiB 5V",         0x6b, 512, 4, 0x2000, 0),
+	TABLE_NAND("SmartMedia 4MiB 3,3V ROM",   0xd5, 512, 4, 0x2000, NAND_ROM),
+	TABLE_NAND("SmartMedia 8MiB 3,3V",       0xe6, 512, 8, 0x2000, 0),
+	TABLE_NAND("SmartMedia 8MiB 3,3V ROM",   0xd6, 512, 8, 0x2000, NAND_ROM),
+	TABLE_NAND("SmartMedia 16MiB 3,3V",      0x73, 512, 16, 0x4000, 0),
+	TABLE_NAND("SmartMedia 16MiB 3,3V ROM",  0x57, 512, 16, 0x4000, NAND_ROM),
+	TABLE_NAND("SmartMedia 32MiB 3,3V",      0x75, 512, 32, 0x4000, 0),
+	TABLE_NAND("SmartMedia 32MiB 3,3V ROM",  0x58, 512, 32, 0x4000, NAND_ROM),
+	TABLE_NAND("SmartMedia 64MiB 3,3V",      0x76, 512, 64, 0x4000, 0),
+	TABLE_NAND("SmartMedia 64MiB 3,3V ROM",  0xd9, 512, 64, 0x4000, NAND_ROM),
+	TABLE_NAND("SmartMedia 128MiB 3,3V",     0x79, 512, 128, 0x4000, 0),
+	TABLE_NAND("SmartMedia 128MiB 3,3V ROM", 0xda, 512, 128, 0x4000, NAND_ROM),
+	TABLE_NAND("SmartMedia 256MiB 3, 3V",    0x71, 512, 256, 0x4000, 0),
+	TABLE_NAND("SmartMedia 256MiB 3,3V ROM", 0x5b, 512, 256, 0x4000, NAND_ROM),
+	{NULL}
 };
 
 static struct nand_flash_dev nand_xd_flash_ids[] = {
 
-	{"xD 16MiB 3,3V",    0x73, 512, 16, 0x4000, 0},
-	{"xD 32MiB 3,3V",    0x75, 512, 32, 0x4000, 0},
-	{"xD 64MiB 3,3V",    0x76, 512, 64, 0x4000, 0},
-	{"xD 128MiB 3,3V",   0x79, 512, 128, 0x4000, 0},
-	{"xD 256MiB 3,3V",   0x71, 512, 256, 0x4000, NAND_BROKEN_XD},
-	{"xD 512MiB 3,3V",   0xdc, 512, 512, 0x4000, NAND_BROKEN_XD},
-	{"xD 1GiB 3,3V",     0xd3, 512, 1024, 0x4000, NAND_BROKEN_XD},
-	{"xD 2GiB 3,3V",     0xd5, 512, 2048, 0x4000, NAND_BROKEN_XD},
-	{NULL,}
+	TABLE_NAND("xD 16MiB 3,3V",  0x73, 512, 16, 0x4000, 0),
+	TABLE_NAND("xD 32MiB 3,3V",  0x75, 512, 32, 0x4000, 0),
+	TABLE_NAND("xD 64MiB 3,3V",  0x76, 512, 64, 0x4000, 0),
+	TABLE_NAND("xD 128MiB 3,3V", 0x79, 512, 128, 0x4000, 0),
+	TABLE_NAND("xD 256MiB 3,3V", 0x71, 512, 256, 0x4000, NAND_BROKEN_XD),
+	TABLE_NAND("xD 512MiB 3,3V", 0xdc, 512, 512, 0x4000, NAND_BROKEN_XD),
+	TABLE_NAND("xD 1GiB 3,3V",   0xd3, 512, 1024, 0x4000, NAND_BROKEN_XD),
+	TABLE_NAND("xD 2GiB 3,3V",   0xd5, 512, 2048, 0x4000, NAND_BROKEN_XD),
+	{NULL}
 };
 
 int sm_register_device(struct mtd_info *mtd, int smartmedia)
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 59dee20..f7f198e 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -539,6 +539,23 @@ struct nand_chip {
 #define NAND_MFR_MACRONIX	0xc2
 #define NAND_MFR_EON		0x92
 
+/*
+ * A helper for defining older chips which did not have page size and
+ * eraseblock size encoded in the chip ID bytes.
+ */
+#define TABLE_NAND(nm, devid, pagesz, chipsz, erasesz, opts) \
+	{ .name = (nm), .id = (devid), .pagesize = (pagesz), \
+	  .chipsize = (chipsz), .erasesize = (erasesz),      \
+	  .options = (opts) }
+
+/*
+ * A helper for defining newer chips which report their page size and
+ * eraseblock size via the extended ID bytes.
+ */
+#define EXTENDED_ID_NAND(nm, devid, chipsz, opts)            \
+	{ .name = (nm), .id = (devid), .chipsize = (chipsz), \
+	  .options = (opts) }
+
 /**
  * struct nand_flash_dev - NAND Flash Device ID Structure
  * @name:	Identify the device type
-- 
1.7.10.4

Re: [PATCH 10/12] mtd: nand_ids: introduce helper macros

[Brian Norris]

On Mon, Mar 4, 2013 at 8:42 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>
> Introduce helper macros for defining NAND chips. These macros do not really add
> much value in the current code-base. However, we are going to add full ID
> support which adds some more complexity to the table, and helper macros become
> useful for readability.
>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> ---
>  drivers/mtd/nand/nand_ids.c  |  159 +++++++++++++++++++++---------------------
>  drivers/mtd/nand/sm_common.c |   64 ++++++++---------
>  include/linux/mtd/nand.h     |   17 +++++
>  3 files changed, 127 insertions(+), 113 deletions(-)
>

...

> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 59dee20..f7f198e 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -539,6 +539,23 @@ struct nand_chip {
>  #define NAND_MFR_MACRONIX      0xc2
>  #define NAND_MFR_EON           0x92
>
> +/*
> + * A helper for defining older chips which did not have page size and
> + * eraseblock size encoded in the chip ID bytes.
> + */

Do the "TABLE_NAND" flash really "not have page size and eraseblock
size encoded in the chip ID bytes"? I would say they have them
"encoded" in a way: the encoding is just a 1-to-1 constant mapping!

The real difference between TABLE_NAND and EXTENDED_ID_NAND is that
with EXTENDED_ID_NAND, manufacturers overloaded the same device ID so
that the device ID now only represented a particular total chip size
(and voltage, buswidth), and the page size, eraseblock size, and OOB
size could vary while using the same device ID.

I don't know if I'm just being too picky here or if this actually is
worth clarifying/improving these comments. One candidate change:

"A helper for defining older chips whose device ID determines their
page size and eraseblock size."

> +#define TABLE_NAND(nm, devid, pagesz, chipsz, erasesz, opts) \
> +       { .name = (nm), .id = (devid), .pagesize = (pagesz), \
> +         .chipsize = (chipsz), .erasesize = (erasesz),      \
> +         .options = (opts) }
> +
> +/*
> + * A helper for defining newer chips which report their page size and
> + * eraseblock size via the extended ID bytes.
> + */
> +#define EXTENDED_ID_NAND(nm, devid, chipsz, opts)            \
> +       { .name = (nm), .id = (devid), .chipsize = (chipsz), \
> +         .options = (opts) }
> +
>  /**
>   * struct nand_flash_dev - NAND Flash Device ID Structure
>   * @name:      Identify the device type
> --
> 1.7.10.4
>

Brian

Re: [PATCH 10/12] mtd: nand_ids: introduce helper macros

[Artem Bityutskiy]

On Mon, 2013-03-04 at 11:23 -0800, Brian Norris wrote:
> > +/*
> > + * A helper for defining older chips which did not have page size and
> > + * eraseblock size encoded in the chip ID bytes.
> > + */
> 
> Do the "TABLE_NAND" flash really "not have page size and eraseblock
> size encoded in the chip ID bytes"? I would say they have them
> "encoded" in a way: the encoding is just a 1-to-1 constant mapping!
> 
> The real difference between TABLE_NAND and EXTENDED_ID_NAND is that
> with EXTENDED_ID_NAND, manufacturers overloaded the same device ID so
> that the device ID now only represented a particular total chip size
> (and voltage, buswidth), and the page size, eraseblock size, and OOB
> size could vary while using the same device ID.
> 
> I don't know if I'm just being too picky here or if this actually is
> worth clarifying/improving these comments. One candidate change:
> 
> "A helper for defining older chips whose device ID determines their
> page size and eraseblock size."

No, you are not picky, you are pointing to a serious flaw in my
commentaries. Thank you. I'll update the comments accordingly and send
out the new version for your review.

-- 
Best Regards,
Artem Bityutskiy

[PATCH 11/12] mtd: nand: rename the id filed of 'struct nand_flash_dev'

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

The 'id' is a bit confusing name because NAND IDs are multi-byte. Re-name
it to 'dev_id' to make it clear that this is the "device ID" part (the second
byte).

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 drivers/mtd/devices/doc2000.c     |    2 +-
 drivers/mtd/devices/doc2001.c     |    2 +-
 drivers/mtd/devices/doc2001plus.c |    2 +-
 drivers/mtd/nand/nand_base.c      |    2 +-
 drivers/mtd/nand/nandsim.c        |    6 +++---
 drivers/mtd/nand/pxa3xx_nand.c    |    2 +-
 include/linux/mtd/nand.h          |   14 +++++++-------
 7 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/mtd/devices/doc2000.c b/drivers/mtd/devices/doc2000.c
index a4eb8b5..363ec3c 100644
--- a/drivers/mtd/devices/doc2000.c
+++ b/drivers/mtd/devices/doc2000.c
@@ -379,7 +379,7 @@ static int DoC_IdentChip(struct DiskOnChip *doc, int floor, int chip)
 
 	/* Print and store the manufacturer and ID codes. */
 	for (i = 0; nand_flash_ids[i].name != NULL; i++) {
-		if (id == nand_flash_ids[i].id) {
+		if (id == nand_flash_ids[i].dev_id) {
 			/* Try to identify manufacturer */
 			for (j = 0; nand_manuf_ids[j].id != 0x0; j++) {
 				if (nand_manuf_ids[j].id == mfr)
diff --git a/drivers/mtd/devices/doc2001.c b/drivers/mtd/devices/doc2001.c
index f692795..00644bb 100644
--- a/drivers/mtd/devices/doc2001.c
+++ b/drivers/mtd/devices/doc2001.c
@@ -206,7 +206,7 @@ static int DoC_IdentChip(struct DiskOnChip *doc, int floor, int chip)
 
 	/* FIXME: to deal with multi-flash on multi-Millennium case more carefully */
 	for (i = 0; nand_flash_ids[i].name != NULL; i++) {
-		if ( id == nand_flash_ids[i].id) {
+		if ( id == nand_flash_ids[i].dev_id) {
 			/* Try to identify manufacturer */
 			for (j = 0; nand_manuf_ids[j].id != 0x0; j++) {
 				if (nand_manuf_ids[j].id == mfr)
diff --git a/drivers/mtd/devices/doc2001plus.c b/drivers/mtd/devices/doc2001plus.c
index 4f2220a..1b0c12f 100644
--- a/drivers/mtd/devices/doc2001plus.c
+++ b/drivers/mtd/devices/doc2001plus.c
@@ -314,7 +314,7 @@ static int DoC_IdentChip(struct DiskOnChip *doc, int floor, int chip)
 		return 0;
 
 	for (i = 0; nand_flash_ids[i].name != NULL; i++) {
-		if (id == nand_flash_ids[i].id) {
+		if (id == nand_flash_ids[i].dev_id) {
 			/* Try to identify manufacturer */
 			for (j = 0; nand_manuf_ids[j].id != 0x0; j++) {
 				if (nand_manuf_ids[j].id == mfr)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index e607565..8dacd1e 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3128,7 +3128,7 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 		type = nand_flash_ids;
 
 	for (; type->name != NULL; type++)
-		if (*dev_id == type->id)
+		if (*dev_id == type->dev_id)
 			break;
 
 	chip->onfi_version = 0;
diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
index 7199acc..eb1238f 100644
--- a/drivers/mtd/nand/nandsim.c
+++ b/drivers/mtd/nand/nandsim.c
@@ -766,9 +766,9 @@ static int init_nandsim(struct mtd_info *mtd)
 	}
 
 	/* Detect how many ID bytes the NAND chip outputs */
-        for (i = 0; nand_flash_ids[i].name != NULL; i++) {
-                if (second_id_byte != nand_flash_ids[i].id)
-                        continue;
+	for (i = 0; nand_flash_ids[i].name != NULL; i++) {
+		if (second_id_byte != nand_flash_ids[i].dev_id)
+			continue;
 	}
 
 	if (ns->busw == 16)
diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 37ee75c..dec80ca 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -989,7 +989,7 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 	}
 
 	pxa3xx_flash_ids[0].name = f->name;
-	pxa3xx_flash_ids[0].id = (f->chip_id >> 8) & 0xffff;
+	pxa3xx_flash_ids[0].dev_id = (f->chip_id >> 8) & 0xffff;
 	pxa3xx_flash_ids[0].pagesize = f->page_size;
 	chipsize = (uint64_t)f->num_blocks * f->page_per_block * f->page_size;
 	pxa3xx_flash_ids[0].chipsize = chipsize >> 20;
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index f7f198e..7af6600 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -543,23 +543,23 @@ struct nand_chip {
  * A helper for defining older chips which did not have page size and
  * eraseblock size encoded in the chip ID bytes.
  */
-#define TABLE_NAND(nm, devid, pagesz, chipsz, erasesz, opts) \
-	{ .name = (nm), .id = (devid), .pagesize = (pagesz), \
-	  .chipsize = (chipsz), .erasesize = (erasesz),      \
+#define TABLE_NAND(nm, devid, pagesz, chipsz, erasesz, opts)     \
+	{ .name = (nm), .dev_id = (devid), .pagesize = (pagesz), \
+	  .chipsize = (chipsz), .erasesize = (erasesz),          \
 	  .options = (opts) }
 
 /*
  * A helper for defining newer chips which report their page size and
  * eraseblock size via the extended ID bytes.
  */
-#define EXTENDED_ID_NAND(nm, devid, chipsz, opts)            \
-	{ .name = (nm), .id = (devid), .chipsize = (chipsz), \
+#define EXTENDED_ID_NAND(nm, devid, chipsz, opts)                \
+	{ .name = (nm), .dev_id = (devid), .chipsize = (chipsz), \
 	  .options = (opts) }
 
 /**
  * struct nand_flash_dev - NAND Flash Device ID Structure
  * @name:	Identify the device type
- * @id:		device ID code
+ * @dev_id:	device ID code
  * @pagesize:	Pagesize in bytes. Either 256 or 512 or 0
  *		If the pagesize is 0, then the real pagesize
  *		and the eraseize are determined from the
@@ -570,7 +570,7 @@ struct nand_chip {
  */
 struct nand_flash_dev {
 	char *name;
-	int id;
+	int dev_id;
 	unsigned long pagesize;
 	unsigned long chipsize;
 	unsigned long erasesize;
-- 
1.7.10.4

Re: [PATCH 11/12] mtd: nand: rename the id filed of 'struct nand_flash_dev'

[Brian Norris]

On Mon, Mar 4, 2013 at 8:42 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>
> The 'id' is a bit confusing name because NAND IDs are multi-byte. Re-name
> it to 'dev_id' to make it clear that this is the "device ID" part (the second
> byte).
>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

While we're going for typos (a la Jan's comment on another part of
this series) the subject of this patch/email says "filed" where it
should be "field".

Still reviewing the rest of this.

Brian

[PATCH 12/12] mtd: nand: provision full ID support

[Artem Bityutskiy]

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 include/linux/mtd/nand.h |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 7af6600..951ea0d 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -559,7 +559,9 @@ struct nand_chip {
 /**
  * struct nand_flash_dev - NAND Flash Device ID Structure
  * @name:	Identify the device type
- * @dev_id:	device ID code
+ * @mfr_id:     manufecturer ID part of the full chip ID array (byte 0)
+ * @dev_id:     device ID part of the full chip ID array (byte 1)
+ * @id:         full device ID array
  * @pagesize:	Pagesize in bytes. Either 256 or 512 or 0
  *		If the pagesize is 0, then the real pagesize
  *		and the eraseize are determined from the
@@ -570,7 +572,13 @@ struct nand_chip {
  */
 struct nand_flash_dev {
 	char *name;
-	int dev_id;
+	union {
+		struct {
+			uint8_t mfr_id;
+			uint8_t dev_id;
+		};
+		uint8_t id[8];
+	};
 	unsigned long pagesize;
 	unsigned long chipsize;
 	unsigned long erasesize;
-- 
1.7.10.4

Re: [PATCH 12/12] mtd: nand: provision full ID support

[Jan Lübbe]

On Mon, 2013-03-04 at 18:42 +0200, Artem Bityutskiy wrote:
>  /**
>   * struct nand_flash_dev - NAND Flash Device ID Structure
>   * @name:	Identify the device type
> - * @dev_id:	device ID code
> + * @mfr_id:     manufecturer ID part of the full chip ID array (byte 0)

This should be 'manufacturer'.

> + * @dev_id:     device ID part of the full chip ID array (byte 1)

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 12/12] mtd: nand: provision full ID support

[Brian Norris]

On Mon, Mar 4, 2013 at 8:42 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> ---
>  include/linux/mtd/nand.h |   12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 7af6600..951ea0d 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -559,7 +559,9 @@ struct nand_chip {
>  /**
>   * struct nand_flash_dev - NAND Flash Device ID Structure
>   * @name:      Identify the device type
> - * @dev_id:    device ID code
> + * @mfr_id:     manufecturer ID part of the full chip ID array (byte 0)
> + * @dev_id:     device ID part of the full chip ID array (byte 1)
> + * @id:         full device ID array
>   * @pagesize:  Pagesize in bytes. Either 256 or 512 or 0
>   *             If the pagesize is 0, then the real pagesize
>   *             and the eraseize are determined from the
> @@ -570,7 +572,13 @@ struct nand_chip {
>   */
>  struct nand_flash_dev {
>         char *name;
> -       int dev_id;
> +       union {
> +               struct {
> +                       uint8_t mfr_id;
> +                       uint8_t dev_id;
> +               };
> +               uint8_t id[8];
> +       };

I'm sorry that I was unable to fully review Huang's code while it was
being discussed earlier. But from what I read, it seemed there was
some disagreement on some of the format of the tables (e.g., Artem
wondered why the extra zeroes). Now, it seems like instead, we're
including duplicate fields. The "mfr_id" and "dev_id" are always the
first two bytes of the "id[8]" field. Does it make sense to repeat
them? And I actually don't ever see where "mfr_id" would be used.

There's also the (yet unanswered?) question of whether we reasonably
would handle in this table both those chips which have their full
information listed in conjunction with their 8-byte ID string and
those chips with minimal information in the table, and the rest must
be decoded from extended ID.

Perhaps the answer to this last question was already assumed to be
"yes." But how would we differentiate the two types of table entries?
Simply by seeing whether they filled out "dev_id" vs. "id[8]"? This
patch doesn't clearly show how the new fields could be used.

Hopefully my comments make some sense. If they don't, then that
probably means we need some more discussion before including this last
patch. If they do make sense, then that probably still means this last
patch needs to be rewritten and/or included only along with a series
that can reasonably make use of these fields (like Huang's series).

>         unsigned long pagesize;
>         unsigned long chipsize;
>         unsigned long erasesize;

Thanks,
Brian

Re: [PATCH 12/12] mtd: nand: provision full ID support

[Artem Bityutskiy]

On Mon, 2013-03-04 at 11:45 -0800, Brian Norris wrote:
> >  struct nand_flash_dev {
> >         char *name;
> > -       int dev_id;
> > +       union {
> > +               struct {
> > +                       uint8_t mfr_id;
> > +                       uint8_t dev_id;
> > +               };
> > +               uint8_t id[8];
> > +       };
> 
> I'm sorry that I was unable to fully review Huang's code while it was
> being discussed earlier. But from what I read, it seemed there was
> some disagreement on some of the format of the tables (e.g., Artem
> wondered why the extra zeroes). Now, it seems like instead, we're
> including duplicate fields. The "mfr_id" and "dev_id" are always the
> first two bytes of the "id[8]" field. Does it make sense to repeat
> them? And I actually don't ever see where "mfr_id" would be used.

This is not repeating. Because of the union {}, mfr_id's memory address
is identical to id[0]'s address, and dev_id's memory address is
identical to id[1]'s address. You can consider 'mfr_id' and 'dev_id' to
be alias names for id[0] and id[1]. If one needs to have an alias name
for other ID bytes, they can be added.

My idea was that type->mfr_id and type->dev_id is more readable than
type->id[0] and type->id[1].

> There's also the (yet unanswered?) question of whether we reasonably
> would handle in this table both those chips which have their full
> information listed in conjunction with their 8-byte ID string and
> those chips with minimal information in the table, and the rest must
> be decoded from extended ID.

I was thinking that we have a single table which contains both, and we
make sure that all full ID records go first, and device ID-only records
go last. This would mean that we first match by full ID, and then by
device IDs.

> Perhaps the answer to this last question was already assumed to be
> "yes." But how would we differentiate the two types of table entries?
> Simply by seeing whether they filled out "dev_id" vs. "id[8]"? This
> patch doesn't clearly show how the new fields could be used.

I was thinking about

if (type->mfr_id == 0)
	/* this is device-only ID */
else
	/* this is full ID */

or

static inline bool is_full_nand_id(type) {
	return type->mfr_id;
}

> Hopefully my comments make some sense. If they don't, then that
> probably means we need some more discussion before including this last
> patch. If they do make sense, then that probably still means this last
> patch needs to be rewritten and/or included only along with a series
> that can reasonably make use of these fields (like Huang's series).

Sure, this is why you was in "To:" - you are the main NAND janitor
nowadays, because others mostly care about their little drivers, while
you demonstrated skills and will to make the general layer sane.

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH 12/12] mtd: nand: provision full ID support

[Huang Shijie]

于 2013年03月05日 18:37, Artem Bityutskiy 写道:
> I was thinking about
>
> if (type->mfr_id == 0)
> 	/* this is device-only ID */
> else
> 	/* this is full ID */
>
> or
>
> static inline bool is_full_nand_id(type) {
> 	return type->mfr_id;
> }
okay. If you think we should not add another table for the full-id nand.
I will follow your advice.

thanks
Huang Shijie

Re: [PATCH 12/12] mtd: nand: provision full ID support

[Huang Shijie]

On Tue, Mar 5, 2013 at 12:42 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> ---
>  include/linux/mtd/nand.h |   12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 7af6600..951ea0d 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -559,7 +559,9 @@ struct nand_chip {
>  /**
>   * struct nand_flash_dev - NAND Flash Device ID Structure
>   * @name:      Identify the device type
> - * @dev_id:    device ID code
> + * @mfr_id:     manufecturer ID part of the full chip ID array (byte 0)
> + * @dev_id:     device ID part of the full chip ID array (byte 1)
> + * @id:         full device ID array
>   * @pagesize:  Pagesize in bytes. Either 256 or 512 or 0
>   *             If the pagesize is 0, then the real pagesize
>   *             and the eraseize are determined from the
> @@ -570,7 +572,13 @@ struct nand_chip {
>   */
>  struct nand_flash_dev {
>         char *name;
> -       int dev_id;
> +       union {
> +               struct {
> +                       uint8_t mfr_id;
> +                       uint8_t dev_id;
> +               };
> +               uint8_t id[8];
> +       };
>
We can add a new helper like compare_nand_with_devid() to remove the
mfr_id and dev_id, only left the id[8].

The compare_nand_with_devid() can be used in places where your patch
11 does. For example:
   ..............................................................
        if (id == nand_flash_ids[i].dev_id) {

   changed to:

        if (compare_nand_with_devid(id)){

  ...............................................................

By this way, we can remove the mfr_id and dev_id.


thanks
Huang Shijie

Re: [PATCH 12/12] mtd: nand: provision full ID support

[Artem Bityutskiy]

On Tue, 2013-03-05 at 14:08 +0800, Huang Shijie wrote:
> >  struct nand_flash_dev {
> >         char *name;
> > -       int dev_id;
> > +       union {
> > +               struct {
> > +                       uint8_t mfr_id;
> > +                       uint8_t dev_id;
> > +               };
> > +               uint8_t id[8];
> > +       };
> >
> We can add a new helper like compare_nand_with_devid() to remove the
> mfr_id and dev_id, only left the id[8].

Huang, did you miss the "union { ... }" part or you think using 'id[N]'
directly is readable enough?

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH 12/12] mtd: nand: provision full ID support

[Huang Shijie]

On Tue, Mar 5, 2013 at 6:42 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Tue, 2013-03-05 at 14:08 +0800, Huang Shijie wrote:
>> >  struct nand_flash_dev {
>> >         char *name;
>> > -       int dev_id;
>> > +       union {
>> > +               struct {
>> > +                       uint8_t mfr_id;
>> > +                       uint8_t dev_id;
>> > +               };
>> > +               uint8_t id[8];
>> > +       };
>> >
>> We can add a new helper like compare_nand_with_devid() to remove the
>> mfr_id and dev_id, only left the id[8].
>
> Huang, did you miss the "union { ... }" part or you think using 'id[N]'
> directly is readable enough?
ｉ　ｔｈｉｎｋ　ｕｓｉｎｇ　ｔｈｅ　ｉｄ［］　ｉｓ　ｅｎｏｕｇｈ．
ｔｈｅ　ｕｎｉｏｎ　ｉｓ　ｒｅｄｕｎｄａｎｔ．

ｔｈａｎｋｓ
Ｈｕａｎｇ　Ｓｈｉｊｉｅ

Re: [PATCH 12/12] mtd: nand: provision full ID support

[Artem Bityutskiy]

On Tue, 2013-03-05 at 22:36 +0800, Huang Shijie wrote:
> >> We can add a new helper like compare_nand_with_devid() to remove the
> >> mfr_id and dev_id, only left the id[8].
> >
> > Huang, did you miss the "union { ... }" part or you think using 'id[N]'
> > directly is readable enough?
> ｉ　ｔｈｉｎｋ　ｕｓｉｎｇ　ｔｈｅ　ｉｄ［］　ｉｓ　ｅｎｏｕｇｈ．
> ｔｈｅ　ｕｎｉｏｎ　ｉｓ　ｒｅｄｕｎｄａｎｔ．

I just find 'type->dev_id' more readable than 'type->id[1]'...

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH 12/12] mtd: nand: provision full ID support

[Huang Shijie]

于 2013年03月05日 22:55, Artem Bityutskiy 写道:
> On Tue, 2013-03-05 at 22:36 +0800, Huang Shijie wrote:
>>>> We can add a new helper like compare_nand_with_devid() to remove the
>>>> mfr_id and dev_id, only left the id[8].
>>> Huang, did you miss the "union { ... }" part or you think using 'id[N]'
>>> directly is readable enough?
>> ｉ　ｔｈｉｎｋ　ｕｓｉｎｇ　ｔｈｅ　ｉｄ［］　ｉｓ　ｅｎｏｕｇｈ．
>> ｔｈｅ　ｕｎｉｏｎ　ｉｓ　ｒｅｄｕｎｄａｎｔ．
> I just find 'type->dev_id' more readable than 'type->id[1]'...
Just a suggestion, add more comments to the code will also make it readable.

But it's ok to me if you keep the union.

thanks
Huang Shijie

Re: [PATCH 00/12] mtd: nand: provision full ID support

[Brian Norris]

Hi Artem,

On Mon, Mar 4, 2013 at 8:42 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>
> Hi,
>
> this is an RFC which adds full ID support provision which Huang can then
> use to solve his problem described here:
>
> http://thread.gmane.org/gmane.linux.kernel/1428345/focus=45290
>
> But before adding full ID support, I cleaned up things a little and removed
> AG-AND support as part of the clean-up.
>
> Thanks,
> Artem.

Thanks for the cleanup work (and thanks Huang, for the initiative on
expanding the ID table). Aside from my minor comments, I'll ack
patches 1 through 11:

Acked-by: Brian Norris <computersforpeace@gmail.com>

As noted in my review, the last patch seems a little underspecified.

Brian

Re: [PATCH 00/12] mtd: nand: provision full ID support

[Artem Bityutskiy]

On Mon, 2013-03-04 at 11:48 -0800, Brian Norris wrote:
> Thanks for the cleanup work (and thanks Huang, for the initiative on
> expanding the ID table). Aside from my minor comments, I'll ack
> patches 1 through 11:
> 
> Acked-by: Brian Norris <computersforpeace@gmail.com>

Added this tag to 1-11, thanks!

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH 00/12] mtd: nand: provision full ID support

[Huang Shijie]

于 2013年03月06日 15:19, Artem Bityutskiy 写道:
> On Mon, 2013-03-04 at 11:48 -0800, Brian Norris wrote:
>> Thanks for the cleanup work (and thanks Huang, for the initiative on
>> expanding the ID table). Aside from my minor comments, I'll ack
>> patches 1 through 11:
>>
>> Acked-by: Brian Norris<computersforpeace@gmail.com>
> Added this tag to 1-11, thanks!
>
could you do a compiler test for this patch set?

I tested it, and a compiler error occurred.

thanks
Huang Shijie