mtd: vf610_nfc: use in-band bad block table

PATCH New Stefan Agner 3 participants 13 messages Apr 3, 2015 – Apr 7, 2015

Subsystems (2)

Memory technology devices (MTD) NAND flash

lore

Share & tools

lore

Stefan Agner

author

2015-04-03 18:40 UTC #0 lore raw

PATCH 1/4 mtd: vf610_nfc: use in-band bad block table

Use in-band bad block table (NAND_BBT_NO_OOB) which allows to use the full OOB for hardare ECC purposes. Since there is no ECC correction on the OOB it is also safer to use in-band area to store the bad block table marker.

Signed-off-by Stefan Agner <[email protected]>

---
 drivers/mtd/nand/vf610_nfc.c | 29 ++---------------------------
 1 file changed, 2 insertions(+), 27 deletions(-)

drivers/mtd/nand/vf610_nfc.c +2 ~~-27~~

index 75c2493..8608ac3 100644
@@ -155,29 +155,6 @@ struct vf610_nfc {
 #define mtd_to_nfc(_mtd) \
 	(struct vf610_nfc *)((struct nand_chip *)_mtd->priv)->priv
 
-static u8 bbt_pattern[] = {'B', 'b', 't', '0' };
-static u8 mirror_pattern[] = {'1', 't', 'b', 'B' };
-
-static struct nand_bbt_descr bbt_main_descr = {
-	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE |
-		   NAND_BBT_2BIT | NAND_BBT_VERSION,
-	.offs =	11,
-	.len = 4,
-	.veroffs = 15,
-	.maxblocks = 4,
-	.pattern = bbt_pattern,
-};
-
-static struct nand_bbt_descr bbt_mirror_descr = {
-	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE |
-		   NAND_BBT_2BIT | NAND_BBT_VERSION,
-	.offs =	11,
-	.len = 4,
-	.veroffs = 15,
-	.maxblocks = 4,
-	.pattern = mirror_pattern,
-};
-
 static struct nand_ecclayout vf610_nfc_ecc45 = {
 	.eccbytes = 45,
 	.eccpos = {19, 20, 21, 22, 23,

@@ -622,10 +599,8 @@ static int vf610_nfc_nand_init(int devnum, void __iomem *addr)
 
 	/* Bad block options. */
 	if (cfg.flash_bbt)
-		chip->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_CREATE;
-
-	chip->bbt_td = &bbt_main_descr;
-	chip->bbt_md = &bbt_mirror_descr;
+		chip->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB |
+				    NAND_BBT_CREATE;
 
 	/* Set configuration register. */
 	vf610_nfc_clear(mtd, NFC_FLASH_CONFIG, CONFIG_ADDR_AUTO_INCR_BIT);

Signature · 1 line

2.3.5

Stefan Agner

author

2015-04-03 18:40 UTC #1 lore raw

PATCH 2/4 mtd: vf610_nfc: add Freescale NFC controller configs to Kconfig

This commit allows users to enable/disable the Freescale NFC controller found in systems like Vybrid (VF610), MPC5125, MCF54418 or Kinetis K70 via Kconfig with more detailed help docs.

Signed-off-by Stefan Agner <[email protected]>

---
 configs/vf610twr_defconfig |  2 ++
 drivers/mtd/nand/Kconfig   | 15 +++++++++++++++
 include/configs/vf610twr.h |  3 ---
 3 files changed, 17 insertions(+), 3 deletions(-)

configs/vf610twr_defconfig +2 -0

index 7de374a..5e0ac9f 100644
@@ -1,3 +1,5 @@
 CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/vf610twr/imximage.cfg,ENV_IS_IN_MMC"
 CONFIG_ARM=y
 CONFIG_TARGET_VF610TWR=y
+CONFIG_NAND_VF610_NFC=y
+CONFIG_SYS_NAND_BUSWIDTH_16BIT=y

drivers/mtd/nand/Kconfig +15 -0

index 72825c3..8056c06 100644
@@ -32,6 +32,21 @@ config NAND_DENALI_SPARE_AREA_SKIP_BYTES
 	  of OOB area before last ECC sector data starts.  This is potentially
 	  used to preserve the bad block marker in the OOB area.
 
+config NAND_VF610_NFC
+	bool "Support for Freescale NFC for VF610/MPC5125"
+	select SYS_NAND_SELF_INIT
+	help
+	  Enables support for NAND Flash Controller on some Freescale
+	  processors like the VF610, MPC5125, MCF54418 or Kinetis K70.
+	  The driver supports a maximum 2k page size. The driver
+	  currently does not support hardware ECC.
+
+config SYS_NAND_BUSWIDTH_16BIT
+	bool "Use 16-bit NAND interface"
+	depends on NAND_VF610_NFC
+	help
+	  Use 16-bit wide NAND flash interface.
+
 if SPL
 
 config SPL_NAND_DENALI

include/configs/vf610twr.h +0 -3

index 05bc7d0..621aa13 100644
@@ -50,10 +50,7 @@
 #define CONFIG_CMD_NAND_TRIMFFS
 
 #ifdef CONFIG_CMD_NAND
-#define CONFIG_NAND_VF610_NFC
-#define CONFIG_SYS_NAND_SELF_INIT
 #define CONFIG_USE_ARCH_MEMCPY
-#define CONFIG_SYS_NAND_BUSWIDTH_16BIT
 #define CONFIG_SYS_MAX_NAND_DEVICE	1
 #define CONFIG_SYS_NAND_BASE		NFC_BASE_ADDR

Signature · 1 line

2.3.5

Scott Wood

2015-04-03 20:30 UTC #2 lore raw

PATCH 2/4 mtd: vf610_nfc: add Freescale NFC controller configs to Kconfig

Quoted 33 lines · Stefan Agner

This commit allows users to enable/disable the Freescale NFC controller found in systems like Vybrid (VF610), MPC5125, MCF54418 or Kinetis K70 via Kconfig with more detailed help docs.

Signed-off-by Stefan Agner <[email protected]>

---
 configs/vf610twr_defconfig |  2 ++
 drivers/mtd/nand/Kconfig   | 15 +++++++++++++++
 include/configs/vf610twr.h |  3 ---
 3 files changed, 17 insertions(+), 3 deletions(-)

configs/vf610twr_defconfig +2 -0

index 7de374a..5e0ac9f 100644
@@ -1,3 +1,5 @@
 CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/vf610twr/imximage.cfg,ENV_IS_IN_MMC"
 CONFIG_ARM=y
 CONFIG_TARGET_VF610TWR=y
+CONFIG_NAND_VF610_NFC=y
+CONFIG_SYS_NAND_BUSWIDTH_16BIT=y

drivers/mtd/nand/Kconfig +4 -0

index 72825c3..8056c06 100644
@@ -32,6 +32,21 @@ config NAND_DENALI_SPARE_AREA_SKIP_BYTES
 	  of OOB area before last ECC sector data starts.  This is potentially
 	  used to preserve the bad block marker in the OOB area.
 
+config NAND_VF610_NFC
+	bool "Support for Freescale NFC for VF610/MPC5125"
+	select SYS_NAND_SELF_INIT
+	help

drivers/mtd/nand/Kconfig config NAND_DENALI_SPARE_AREA_SKIP_BYTES · line 48

Quoted 10 lines

drivers/mtd/nand/Kconfig +10 -0

+	  Enables support for NAND Flash Controller on some Freescale
+	  processors like the VF610, MPC5125, MCF54418 or Kinetis K70.
+	  The driver supports a maximum 2k page size. The driver
+	  currently does not support hardware ECC.
+
+config SYS_NAND_BUSWIDTH_16BIT
+	bool "Use 16-bit NAND interface"
+	depends on NAND_VF610_NFC
+	help
+	  Use 16-bit wide NAND flash interface.

Why does a generic-sounding config name depend on VF610? Especially when README already lists three other drivers as using this option...

Also, the help text makes it sound like it's at the user's discretion, rather than a description of hardware. I'd phrase it as something like "NAND has 16-bit interface"

-Scott

Stefan Agner

author

2015-04-03 20:42 UTC #3 lore raw

PATCH 2/4 mtd: vf610_nfc: add Freescale NFC controller configs to Kconfig

Quoted 39 lines · Scott Wood

Quoted 38 lines · Stefan Agner

This commit allows users to enable/disable the Freescale NFC controller found in systems like Vybrid (VF610), MPC5125, MCF54418 or Kinetis K70 via Kconfig with more detailed help docs.

Signed-off-by Stefan Agner <[email protected]>

---
 configs/vf610twr_defconfig |  2 ++
 drivers/mtd/nand/Kconfig   | 15 +++++++++++++++
 include/configs/vf610twr.h |  3 ---
 3 files changed, 17 insertions(+), 3 deletions(-)

configs/vf610twr_defconfig +2 -0

index 7de374a..5e0ac9f 100644
@@ -1,3 +1,5 @@
 CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/vf610twr/imximage.cfg,ENV_IS_IN_MMC"
 CONFIG_ARM=y
 CONFIG_TARGET_VF610TWR=y
+CONFIG_NAND_VF610_NFC=y
+CONFIG_SYS_NAND_BUSWIDTH_16BIT=y

drivers/mtd/nand/Kconfig +9 -0

index 72825c3..8056c06 100644
@@ -32,6 +32,21 @@ config NAND_DENALI_SPARE_AREA_SKIP_BYTES
 	  of OOB area before last ECC sector data starts.  This is potentially
 	  used to preserve the bad block marker in the OOB area.
 
+config NAND_VF610_NFC
+	bool "Support for Freescale NFC for VF610/MPC5125"
+	select SYS_NAND_SELF_INIT
+	help
+	  Enables support for NAND Flash Controller on some Freescale
+	  processors like the VF610, MPC5125, MCF54418 or Kinetis K70.
+	  The driver supports a maximum 2k page size. The driver
+	  currently does not support hardware ECC.
+

Quoted 8 lines

drivers/mtd/nand/Kconfig config NAND_DENALI_SPARE_AREA_SKIP_BYTES · line 48
Quoted 5 lines
drivers/mtd/nand/Kconfig +5 -0
+config SYS_NAND_BUSWIDTH_16BIT
+	bool "Use 16-bit NAND interface"
+	depends on NAND_VF610_NFC
+	help
+	  Use 16-bit wide NAND flash interface.
Why does a generic-sounding config name depend on VF610? Especially when README already lists three other drivers as using this option...

That option is _not_ meant as being VF610 specific.

Since we have the ability to specify dependencies with Kconfig, I think it is nice to have options only available if a driver supports it, hence the depends. So far the VF610 NAND driver is the only one which is in Kconfig and supports it... I would expect that when another driver which supports that option gets migrated, depends will be extended accordingly.

However, I just realized that the option end up between Vybrid specific configs because of Patch 3. I will move the option at the very bottom in next revision.

Quoted 3 lines

Also, the help text makes it sound like it's at the user's discretion, rather than a description of hardware. I'd phrase it as something like "NAND has 16-bit interface"

Agreed, will change that.

--
Stefan

Scott Wood

2015-04-03 20:46 UTC #4 lore raw

PATCH 2/4 mtd: vf610_nfc: add Freescale NFC controller configs to Kconfig

Quoted 58 lines · Stefan Agner

Quoted 39 lines · Scott Wood
Quoted 38 lines · Stefan Agner
This commit allows users to enable/disable the Freescale NFC controller found in systems like Vybrid (VF610), MPC5125, MCF54418 or Kinetis K70 via Kconfig with more detailed help docs.
Signed-off-by Stefan Agner <[email protected]>
---
 configs/vf610twr_defconfig |  2 ++
 drivers/mtd/nand/Kconfig   | 15 +++++++++++++++
 include/configs/vf610twr.h |  3 ---
 3 files changed, 17 insertions(+), 3 deletions(-)
configs/vf610twr_defconfig +2 -0
index 7de374a..5e0ac9f 100644
@@ -1,3 +1,5 @@
 CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/vf610twr/imximage.cfg,ENV_IS_IN_MMC"
 CONFIG_ARM=y
 CONFIG_TARGET_VF610TWR=y
+CONFIG_NAND_VF610_NFC=y
+CONFIG_SYS_NAND_BUSWIDTH_16BIT=y
drivers/mtd/nand/Kconfig +9 -0
index 72825c3..8056c06 100644
@@ -32,6 +32,21 @@ config NAND_DENALI_SPARE_AREA_SKIP_BYTES
 	  of OOB area before last ECC sector data starts.  This is potentially
 	  used to preserve the bad block marker in the OOB area.
 
+config NAND_VF610_NFC
+	bool "Support for Freescale NFC for VF610/MPC5125"
+	select SYS_NAND_SELF_INIT
+	help
+	  Enables support for NAND Flash Controller on some Freescale
+	  processors like the VF610, MPC5125, MCF54418 or Kinetis K70.
+	  The driver supports a maximum 2k page size. The driver
+	  currently does not support hardware ECC.
+
Quoted 8 lines
drivers/mtd/nand/Kconfig config NAND_DENALI_SPARE_AREA_SKIP_BYTES · line 48
Quoted 5 lines
drivers/mtd/nand/Kconfig +5 -0
+config SYS_NAND_BUSWIDTH_16BIT
+	bool "Use 16-bit NAND interface"
+	depends on NAND_VF610_NFC
+	help
+	  Use 16-bit wide NAND flash interface.
Why does a generic-sounding config name depend on VF610? Especially when README already lists three other drivers as using this option...
That option is _not_ meant as being VF610 specific.
Since we have the ability to specify dependencies with Kconfig, I think it is nice to have options only available if a driver supports it, hence the depends. So far the VF610 NAND driver is the only one which is in Kconfig and supports it... I would expect that when another driver which supports that option gets migrated, depends will be extended accordingly.

Quoted 3 lines

However, I just realized that the option end up between Vybrid specific configs because of Patch 3. I will move the option at the very bottom in next revision.

Could you also add a comment mentioning the other drivers that use it, which aren't yet kconfiged? And then remove the old text from the README.

-Scott

Stefan Agner

author

2015-04-03 22:30 UTC #5 lore raw

PATCH 2/4 mtd: vf610_nfc: add Freescale NFC controller configs to Kconfig

Quoted 59 lines · Scott Wood

Quoted 58 lines · Stefan Agner
Quoted 39 lines · Scott Wood
Quoted 38 lines · Stefan Agner
This commit allows users to enable/disable the Freescale NFC controller found in systems like Vybrid (VF610), MPC5125, MCF54418 or Kinetis K70 via Kconfig with more detailed help docs.
Signed-off-by Stefan Agner <[email protected]>
---
 configs/vf610twr_defconfig |  2 ++
 drivers/mtd/nand/Kconfig   | 15 +++++++++++++++
 include/configs/vf610twr.h |  3 ---
 3 files changed, 17 insertions(+), 3 deletions(-)
configs/vf610twr_defconfig +2 -0
index 7de374a..5e0ac9f 100644
@@ -1,3 +1,5 @@
 CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/vf610twr/imximage.cfg,ENV_IS_IN_MMC"
 CONFIG_ARM=y
 CONFIG_TARGET_VF610TWR=y
+CONFIG_NAND_VF610_NFC=y
+CONFIG_SYS_NAND_BUSWIDTH_16BIT=y
drivers/mtd/nand/Kconfig +9 -0
index 72825c3..8056c06 100644
@@ -32,6 +32,21 @@ config NAND_DENALI_SPARE_AREA_SKIP_BYTES
 	  of OOB area before last ECC sector data starts.  This is potentially
 	  used to preserve the bad block marker in the OOB area.
 
+config NAND_VF610_NFC
+	bool "Support for Freescale NFC for VF610/MPC5125"
+	select SYS_NAND_SELF_INIT
+	help
+	  Enables support for NAND Flash Controller on some Freescale
+	  processors like the VF610, MPC5125, MCF54418 or Kinetis K70.
+	  The driver supports a maximum 2k page size. The driver
+	  currently does not support hardware ECC.
+
Quoted 8 lines
drivers/mtd/nand/Kconfig config NAND_DENALI_SPARE_AREA_SKIP_BYTES · line 48
Quoted 5 lines
drivers/mtd/nand/Kconfig +5 -0
+config SYS_NAND_BUSWIDTH_16BIT
+	bool "Use 16-bit NAND interface"
+	depends on NAND_VF610_NFC
+	help
+	  Use 16-bit wide NAND flash interface.
Why does a generic-sounding config name depend on VF610? Especially when README already lists three other drivers as using this option...
That option is _not_ meant as being VF610 specific.
Since we have the ability to specify dependencies with Kconfig, I think it is nice to have options only available if a driver supports it, hence the depends. So far the VF610 NAND driver is the only one which is in Kconfig and supports it... I would expect that when another driver which supports that option gets migrated, depends will be extended accordingly.

Quoted 7 lines

Quoted 3 lines
However, I just realized that the option end up between Vybrid specific configs because of Patch 3. I will move the option at the very bottom in next revision.
Could you also add a comment mentioning the other drivers that use it, which aren't yet kconfiged? And then remove the old text from the README.

By comment, you mean a Kconfig comment at that option, so the next stumbles upon it? So I can keep that single depends NAND_VF610_NFC for now?

Removing CONFIG_SYS_NAND_BUSWIDTH_16BIT from doc/README.nand right? But with that, the options for the other drivers would be "undocumented" for the time being...

--
Stefan

Scott Wood

2015-04-03 22:47 UTC #6 lore raw

PATCH 2/4 mtd: vf610_nfc: add Freescale NFC controller configs to Kconfig

Quoted 64 lines · Stefan Agner

Quoted 63 lines · Scott Wood
Quoted 61 lines · Stefan Agner
Quoted 39 lines · Scott Wood
Quoted 38 lines · Stefan Agner
This commit allows users to enable/disable the Freescale NFC controller found in systems like Vybrid (VF610), MPC5125, MCF54418 or Kinetis K70 via Kconfig with more detailed help docs.
Signed-off-by Stefan Agner <[email protected]>
---
 configs/vf610twr_defconfig |  2 ++
 drivers/mtd/nand/Kconfig   | 15 +++++++++++++++
 include/configs/vf610twr.h |  3 ---
 3 files changed, 17 insertions(+), 3 deletions(-)
configs/vf610twr_defconfig +2 -0
index 7de374a..5e0ac9f 100644
@@ -1,3 +1,5 @@
 CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/vf610twr/imximage.cfg,ENV_IS_IN_MMC"
 CONFIG_ARM=y
 CONFIG_TARGET_VF610TWR=y
+CONFIG_NAND_VF610_NFC=y
+CONFIG_SYS_NAND_BUSWIDTH_16BIT=y
drivers/mtd/nand/Kconfig +9 -0
index 72825c3..8056c06 100644
@@ -32,6 +32,21 @@ config NAND_DENALI_SPARE_AREA_SKIP_BYTES
 	  of OOB area before last ECC sector data starts.  This is potentially
 	  used to preserve the bad block marker in the OOB area.
 
+config NAND_VF610_NFC
+	bool "Support for Freescale NFC for VF610/MPC5125"
+	select SYS_NAND_SELF_INIT
+	help
+	  Enables support for NAND Flash Controller on some Freescale
+	  processors like the VF610, MPC5125, MCF54418 or Kinetis K70.
+	  The driver supports a maximum 2k page size. The driver
+	  currently does not support hardware ECC.
+
Quoted 8 lines
drivers/mtd/nand/Kconfig config NAND_DENALI_SPARE_AREA_SKIP_BYTES · line 48
Quoted 5 lines
drivers/mtd/nand/Kconfig +5 -0
+config SYS_NAND_BUSWIDTH_16BIT
+	bool "Use 16-bit NAND interface"
+	depends on NAND_VF610_NFC
+	help
+	  Use 16-bit wide NAND flash interface.
Why does a generic-sounding config name depend on VF610? Especially when README already lists three other drivers as using this option...
That option is _not_ meant as being VF610 specific.
Since we have the ability to specify dependencies with Kconfig, I think it is nice to have options only available if a driver supports it, hence the depends. So far the VF610 NAND driver is the only one which is in Kconfig and supports it... I would expect that when another driver which supports that option gets migrated, depends will be extended accordingly.
However, I just realized that the option end up between Vybrid specific configs because of Patch 3. I will move the option at the very bottom in next revision.

Quoted 7 lines

Quoted 3 lines
Could you also add a comment mentioning the other drivers that use it, which aren't yet kconfiged? And then remove the old text from the README.
By comment, you mean a Kconfig comment at that option, so the next stumbles upon it? So I can keep that single depends NAND_VF610_NFC for now?

Yes.

Quoted 3 lines

Removing CONFIG_SYS_NAND_BUSWIDTH_16BIT from doc/README.nand right? But with that, the options for the other drivers would be "undocumented" for the time being...

No, it'd be documented in the kconfig file, even if it's not presented via the kconfig tool. It seems bad to have the option description duplicated -- and then potentially getting changed one place and not the other.

-Scott

Stefan Agner

author

2015-04-03 18:40 UTC #7 lore raw

PATCH 3/4 mtd: vf610_nfc: add 32-error correction option for HW ECC

In reply to Stefan Agner

Add option to choose between current 24-error correction and 32-error correction through Kconfig. 32-error correction allow to use NAND chips which require up to 8-bit error correction per 512 byte (when using 2K pages).

Signed-off-by Stefan Agner <[email protected]>

---
 drivers/mtd/nand/Kconfig     | 15 +++++++++++++++
 drivers/mtd/nand/vf610_nfc.c | 38 ++++++++++++++++++++++++++++++++------
 2 files changed, 47 insertions(+), 6 deletions(-)

drivers/mtd/nand/Kconfig +15 -0

index 8056c06..ca14cc2 100644
@@ -47,6 +47,21 @@ config SYS_NAND_BUSWIDTH_16BIT
 	help
 	  Use 16-bit wide NAND flash interface.
 
+choice
+	prompt "Hardware ECC strength"
+	depends on NAND_VF610_NFC
+	default SYS_NAND_VF610_NFC_45_ECC_BYTES
+	help
+	  Select the ECC strength used in the hardware BCH ECC block.
+
+config SYS_NAND_VF610_NFC_45_ECC_BYTES
+	bool "24-error correction (45 ECC bytes)"
+
+config SYS_NAND_VF610_NFC_60_ECC_BYTES
+	bool "32-error correction (60 ECC bytes)"
+
+endchoice
+
 if SPL
 
 config SPL_NAND_DENALI

drivers/mtd/nand/vf610_nfc.c +32 -6

index 8608ac3..a8af00e 100644
@@ -71,6 +71,7 @@
 /* NFC ECC mode define */
 #define ECC_BYPASS			0
 #define ECC_45_BYTE			6
+#define ECC_60_BYTE			7
 
 /*** Register Mask and bit definitions */

@@ -155,7 +156,10 @@ struct vf610_nfc {
 #define mtd_to_nfc(_mtd) \
 	(struct vf610_nfc *)((struct nand_chip *)_mtd->priv)->priv
 
-static struct nand_ecclayout vf610_nfc_ecc45 = {
+#if defined (CONFIG_SYS_NAND_VF610_NFC_45_ECC_BYTES)
+#define ECC_HW_MODE ECC_45_BYTE
+
+static struct nand_ecclayout vf610_nfc_ecc = {
 	.eccbytes = 45,
 	.eccpos = {19, 20, 21, 22, 23,
 		   24, 25, 26, 27, 28, 29, 30, 31,

@@ -167,6 +171,24 @@ static struct nand_ecclayout vf610_nfc_ecc45 = {
 		{.offset = 8,
 		 .length = 11} }
 };
+#elif defined (CONFIG_SYS_NAND_VF610_NFC_60_ECC_BYTES)
+#define ECC_HW_MODE ECC_60_BYTE
+
+static struct nand_ecclayout vf610_nfc_ecc = {
+	.eccbytes = 60,
+	.eccpos = { 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, 32, 33, 34, 35,
+		   36, 37, 38, 39, 40, 41, 42, 43,
+		   44, 45, 46, 47, 48, 49, 50, 51,
+		   52, 53, 54, 55, 56, 57, 58, 59,
+		   60, 61, 62, 63 },
+	.oobfree = {
+		{.offset = 2,
+		 .length = 2} }
+};
+#endif
 
 static inline u32 vf610_nfc_read(struct mtd_info *mtd, uint reg)
 {

@@ -333,7 +355,7 @@ static void vf610_nfc_command(struct mtd_info *mtd, unsigned command,
 		vf610_nfc_send_commands(nfc->regs, NAND_CMD_SEQIN,
 					command, PROGRAM_PAGE_CMD_CODE);
 		vf610_nfc_addr_cycle(mtd, column, page);
-		vf610_nfc_ecc_mode(mtd, ECC_45_BYTE);
+		vf610_nfc_ecc_mode(mtd, ECC_HW_MODE);
 		break;
 
 	case NAND_CMD_RESET:

@@ -362,7 +384,7 @@ static void vf610_nfc_command(struct mtd_info *mtd, unsigned command,
 		vf610_nfc_send_commands(nfc->regs, NAND_CMD_READ0,
 					NAND_CMD_READSTART, READ_PAGE_CMD_CODE);
 		vf610_nfc_addr_cycle(mtd, column, page);
-		vf610_nfc_ecc_mode(mtd, ECC_45_BYTE);
+		vf610_nfc_ecc_mode(mtd, ECC_HW_MODE);
 		break;
 
 	case NAND_CMD_ERASE1:

@@ -643,17 +665,21 @@ static int vf610_nfc_nand_init(int devnum, void __iomem *addr)
 			goto error;
 		}
 
-		chip->ecc.layout = &vf610_nfc_ecc45;
-
 		/* propagate ecc.layout to mtd_info */
 		mtd->ecclayout = chip->ecc.layout;
 		chip->ecc.read_page = vf610_nfc_read_page;
 		chip->ecc.write_page = vf610_nfc_write_page;
 		chip->ecc.mode = NAND_ECC_HW;
 
-		chip->ecc.bytes = 45;
 		chip->ecc.size = PAGE_2K;
+		chip->ecc.layout = &vf610_nfc_ecc;
+#if defined(CONFIG_SYS_NAND_VF610_NFC_45_ECC_BYTES)
 		chip->ecc.strength = 24;
+		chip->ecc.bytes = 45;
+#elif defined(CONFIG_SYS_NAND_VF610_NFC_60_ECC_BYTES)
+		chip->ecc.strength = 32;
+		chip->ecc.bytes = 60;
+#endif
 
 		/* Enable ECC_STATUS */
 		vf610_nfc_set(mtd, NFC_FLASH_CONFIG, CONFIG_ECC_SRAM_REQ_BIT);

Signature · 1 line

2.3.5

Stefan Agner

author

2015-04-03 18:40 UTC #8 lore raw

PATCH 4/4 mtd: vf610_nfc: support subpage write

In reply to Stefan Agner

Support subpage writes using a custom implementation of write_subpage. The driver loads the page into SRAM buffer using NAND_CMD_READ0, when the framework requests the NAND_CMD_SEQIN command. Then, the buffer is updated by the custom write_subpage implementation. Upon write, the controller calculates the hardware ECC across the whole page before programming the page.

This method saves transferring the whole page over the bus to the NFC IP, which would happen when using NAND_NO_SUBPAGE_WRITE.

Signed-off-by Stefan Agner <[email protected]>

---
This implements the procedure as discussed on the ML:
http://lists.denx.de/pipermail/u-boot/2015-March/209567.html
http://lists.denx.de/pipermail/u-boot/2015-March/209671.html

The drivers mxc_nand and mpc5121_nfc implement a similar subpage write.

 drivers/mtd/nand/vf610_nfc.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

drivers/mtd/nand/vf610_nfc.c +14 -3

index a8af00e..d552bed 100644
@@ -566,6 +566,19 @@ static int vf610_nfc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	return 0;
 }
 
+static int vf610_nfc_write_subpage(struct mtd_info *mtd, struct nand_chip *chip,
+				uint32_t offset, uint32_t data_len,
+				const uint8_t *buf, int oob_required)
+{
+	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+	nfc->column = offset;
+	vf610_nfc_write_buf(mtd, buf, data_len);
+	if (oob_required)
+		vf610_nfc_write_buf(mtd, chip->oob_poi, mtd->oobsize);
+
+	return 0;
+}
+
 struct vf610_nfc_config {
 	int hardware_ecc;
 	int width;

@@ -608,9 +621,6 @@ static int vf610_nfc_nand_init(int devnum, void __iomem *addr)
 		vf610_nfc_clear(mtd, NFC_FLASH_CONFIG, CONFIG_16BIT);
 	}
 
-	/* Disable subpage writes as we do not provide ecc->hwctl */
-	chip->options |= NAND_NO_SUBPAGE_WRITE;
-
 	chip->dev_ready = vf610_nfc_dev_ready;
 	chip->cmdfunc = vf610_nfc_command;
 	chip->read_byte = vf610_nfc_read_byte;

@@ -669,6 +679,7 @@ static int vf610_nfc_nand_init(int devnum, void __iomem *addr)
 		mtd->ecclayout = chip->ecc.layout;
 		chip->ecc.read_page = vf610_nfc_read_page;
 		chip->ecc.write_page = vf610_nfc_write_page;
+		chip->ecc.write_subpage = vf610_nfc_write_subpage;
 		chip->ecc.mode = NAND_ECC_HW;
 
 		chip->ecc.size = PAGE_2K;

Signature · 1 line

2.3.5

Scott Wood

2015-04-03 20:36 UTC #9 lore raw

PATCH 4/4 mtd: vf610_nfc: support subpage write

Quoted 11 lines · Stefan Agner

Support subpage writes using a custom implementation of write_subpage. The driver loads the page into SRAM buffer using NAND_CMD_READ0, when the framework requests the NAND_CMD_SEQIN command. Then, the buffer is updated by the custom write_subpage implementation. Upon write, the controller calculates the hardware ECC across the whole page before programming the page.
This method saves transferring the whole page over the bus to the NFC IP, which would happen when using NAND_NO_SUBPAGE_WRITE.
Signed-off-by Stefan Agner <[email protected]>

As previously discussed, please explain why subpage writes make sense with this controller.

-Scott

Stefan Agner

author

2015-04-03 22:24 UTC #10 lore raw

PATCH 4/4 mtd: vf610_nfc: support subpage write

Quoted 8 lines · Scott Wood

Quoted 7 lines · Stefan Agner
Support subpage writes using a custom implementation of write_subpage. The driver loads the page into SRAM buffer using NAND_CMD_READ0, when the framework requests the NAND_CMD_SEQIN command. Then, the buffer is updated by the custom write_subpage implementation. Upon write, the controller calculates the hardware ECC across the whole page before programming the page.

Quoted 7 lines

Quoted 4 lines
This method saves transferring the whole page over the bus to the NFC IP, which would happen when using NAND_NO_SUBPAGE_WRITE.
Signed-off-by Stefan Agner <[email protected]>
As previously discussed, please explain why subpage writes make sense with this controller.

I thought the subpage callback is just about writing an arbitrary amount of data (e.g. 32 bytes) into a page.

I quickly checked, when doing

# nand write ${loadaddr} 0x10000 0x10

vf610_nfc_write_subpage gets actually called with a data_len of 16.

This is how I understand it:
Currently, subpage writes are supported by the MTD subsystem due to the option NAND_NO_SUBPAGE_WRITE. Due to that option, nand_write_page in nand_base.c calls write_page which copies always the whole page into the SRAM buffer over the AHB bus to the NFC IP (vf610_nfc_write_page uses memcpy uses mtd->writesize).

This patch supports it naively, which means that only the updated data (according to offset and data_len parameter of write_subpage) get copied over the AHB bus to the NFC IP (vf610_nfc_write_subpage uses memcpy to only copy data_len). To have reasonable data for the rest of the page, the page gets read back when calling NAND_CMD_SEQIN.

Since the whole page get programmed, this assumes that none of the page has been programmed before (erased page). Hence, we essentially just read 0xff. When I think about it now, reading the page back in SEQIN is then probably just an expensive memset 0xff replacement.

I guess when having real subpages (e.g. 4x512bytes, each subpage with its own ECC), I would have to make sure only the affected subpage gets ECC'ed and written.

Even without having real subpages, if somebody only writes part of a page, memset directly into SRAM & memcpy the subpage data is theoretically faster then memset the whole page in DDR RAM and memcpy the whole page....

Just checked what higher up the stack is actually happening:
nand_write_page anyway receives a whole page buffer as argument, which is memset'ed with 0xff and the data to be written memcpy'ed.

I see, that this patch tries to improve something which anyway is taken care of by the stack.

I will remove the page read on NAND_CMD_SEQIN, since we memcpy the full page anyway. I also just realized that the page read actually happens always and hence slows down even full page writes...

--
Stefan

Bill Pringlemeir

2015-04-07 13:48 UTC #11 lore raw

PATCH 4/4 mtd: vf610_nfc: support subpage write

Quoted 3 lines · stefan at agner.ch

I will remove the page read on NAND_CMD_SEQIN, since we memcpy the full page anyway. I also just realized that the page read actually happens always and hence slows down even full page writes...

Yes, I remove this in Linux (4.0) and it corrupted things when writing. I think your previous conclusion about we never use 'write caching' was wrong.

This one is for writes,

case NAND_CMD_SEQIN: /* Pre-read for partial writes. */

This one is for reads,

case NAND_CMD_READ0:

The interface between 'nand_base' and the MTD driver is hard to decipher. Does Scott (or anyone) know if there is any documentation on this?

Stefan is completely correct that if a full page is being written, then the 'SEQIN' should not read a page. However, I only see 'column' being passed. How is 'SEQIN' and 'PAGEPROG' to detect if a full page is being written or not?

The other way to handle things would to be to investigate the NFC_CFG[PAGE_CNT] and NFC_SECSZ to have the virtual pages support sub-pages. I think the OOB mapping would be non-standard in such cases. The buffer management in the driver is most simple in it's current form. The other versions that I found seemed to be buggy to me. However, the current driver doesn't use all of the NFC SRAM buffer space.

Btw, the READ_OOB is very nice for Linux as well. It is a much faster mount of UBI/UbiFs as well.

Fwiw,
Bill Pringlemeir.

Scott Wood

2015-04-07 16:21 UTC #12 lore raw

PATCH 4/4 mtd: vf610_nfc: support subpage write

Quoted 14 lines · Bill Pringlemeir

Quoted 3 lines · stefan at agner.ch
I will remove the page read on NAND_CMD_SEQIN, since we memcpy the full page anyway. I also just realized that the page read actually happens always and hence slows down even full page writes...
Yes, I remove this in Linux (4.0) and it corrupted things when writing. I think your previous conclusion about we never use 'write caching' was wrong.
This one is for writes,
case NAND_CMD_SEQIN: /* Pre-read for partial writes. */

Quoted 7 lines

This one is for reads,
case NAND_CMD_READ0:
The interface between 'nand_base' and the MTD driver is hard to decipher. Does Scott (or anyone) know if there is any documentation on this?

It's an awkward interface for drivers that expose a higher-level programming model. Basically you have to behave as if nand_base could send commands directly to the chip.

Quoted 4 lines

Stefan is completely correct that if a full page is being written, then the 'SEQIN' should not read a page. However, I only see 'column' being passed. How is 'SEQIN' and 'PAGEPROG' to detect if a full page is being written or not?

At SEQIN time, you can't. You'll know based on how much data is written into the buffer. Or, you can skip this low-level interface and replace nand_write_page (which is what I wish we'd done in the eLBC/IFC drivers).

Quoted 3 lines

The other way to handle things would to be to investigate the NFC_CFG[PAGE_CNT] and NFC_SECSZ to have the virtual pages support sub-pages. I think the OOB mapping would be non-standard in such cases.

Wouldn't that mess up factory bad block markers?

-Scott