[U-Boot] [U-boot] [Patch v2 0/3] keystone: nand: add additional nand ecclayout

[U-Boot] [U-boot] [Patch v2 0/3] keystone: nand: add additional nand ecclayout

[Ivan Khoronzhuk]

For keystyone k2hk board the default nand layout is different
from davinci. So swich ecc layout at init in board file.

To do this the series adds a nand ecclayout command to davinci nand
that allows the ecclayout of the current nand device to be changed
during run time. This feature is useful when using u-boot to write
something to nand flash that will be read by other applications,
such as ROM bootloader, that expects a different ECC layout.
In that case, change the current nand device ecclayout using the
"nand ecclayout set" command before writing the data to nand flash.

Based on git://git.denx.de/u-boot-ti.git master

v2..v1:
  k2hk: change default nand ecc layout
	- new patch

  mtd: nand: davinci: allow to change ecclayout by ecclayout command
  	- aligned pattern.
	- used ARRAY_SIZE instead of definition of size
	- added some comments
	- rename board_nand_ecclayout_get_layout() to shorter name
	- don't change default ecc layout at init

  common: cmd_nand: add nand ecclayout command
  	- impove format of ecc layout printing
	- improve description of nand ecclayout command
	- add description of CONFIG_CMD_NAND_ECCLAYOUT config
	- move some code stuff to board_nand_ecclayout_set()
	- don't wrap user-visible string
	- remove #ifdef of function prototypes

Ivan Khoronzhuk (1):
  k2hk: change default nand ecc layout

WingMan Kwok (2):
  mtd: nand: davinci: allow to change ecclayout by ecclayout command
  common: cmd_nand: add nand ecclayout command

 arch/arm/include/asm/ti-common/davinci_nand.h |  4 ++
 board/ti/k2hk_evm/board.c                     | 11 ++++
 common/cmd_nand.c                             | 91 ++++++++++++++++++++++++++
 doc/README.nand                               | 12 ++++
 drivers/mtd/nand/davinci_nand.c               | 94 +++++++++++++++++++++++++++
 include/nand.h                                |  7 ++
 6 files changed, 219 insertions(+)

-- 
1.8.3.2

[U-Boot] [U-boot] [Patch v2 1/3] mtd: nand: davinci: allow to change ecclayout by ecclayout command

[Ivan Khoronzhuk]

From: WingMan Kwok <w-kwok2@ti.com>

This patch adds opportunity to change ecclayout of current nand
device during runtime. So we can change the current nand device
ecclayout using the "nand ecclayout set" command before writing
the data to nand flash.

Signed-off-by: Hao Zhang <hzhang@ti.com>
Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 arch/arm/include/asm/ti-common/davinci_nand.h |  4 ++
 drivers/mtd/nand/davinci_nand.c               | 94 +++++++++++++++++++++++++++
 2 files changed, 98 insertions(+)

diff --git a/arch/arm/include/asm/ti-common/davinci_nand.h b/arch/arm/include/asm/ti-common/davinci_nand.h
index 11407be..bb3ed94 100644
--- a/arch/arm/include/asm/ti-common/davinci_nand.h
+++ b/arch/arm/include/asm/ti-common/davinci_nand.h
@@ -93,6 +93,10 @@ struct davinci_emif_regs {
 #define DAVINCI_ABCR_ASIZE_16BIT			1
 #define DAVINCI_ABCR_ASIZE_8BIT				0
 
+/* Layouts */
+#define NAND_DAVINCI_4BIT_LAYOUT			0
+#define NAND_KEYSTONE_RBL_4BIT_LAYOUT			1
+
 void davinci_nand_init(struct nand_chip *nand);
 
 #endif
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index 5d42509..a4bc921 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -305,6 +305,100 @@ static struct nand_ecclayout nand_davinci_4bit_layout_oobfirst = {
 #endif
 };
 
+#if defined(CONFIG_CMD_NAND_ECCLAYOUT)
+#if defined(CONFIG_SYS_NAND_PAGE_2K)
+static struct nand_ecclayout nand_keystone_rbl_4bit_layout_oobfirst = {
+	.eccbytes = 40,
+	.eccpos = {
+		6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
+		22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
+		38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
+		54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
+	},
+	.oobfree = {
+		{.offset = 2, .length = 4, },
+		{.offset = 16, .length = 6, },
+		{.offset = 32, .length = 6, },
+		{.offset = 48, .length = 6, },
+	},
+};
+#elif defined(CONFIG_SYS_NAND_PAGE_4K)
+static struct nand_ecclayout nand_keystone_rbl_4bit_layout_oobfirst = {
+	.eccbytes = 80,
+	.eccpos = {
+		6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
+		22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
+		38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
+		54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
+		70, 71, 72, 73, 74, 75, 76, 77, 78, 79,
+		86, 87, 88, 89, 90, 91, 92, 93, 94, 95,
+		102, 103, 104, 105, 106, 107, 108, 109, 110, 111,
+		118, 119, 120, 121, 122, 123, 124, 125, 126, 127,
+	},
+	.oobfree = {
+		{.offset = 2, .length = 4, },
+		{.offset = 16, .length = 6, },
+		{.offset = 32, .length = 6, },
+		{.offset = 48, .length = 6, },
+		{.offset = 64, .length = 6, },
+		{.offset = 80, .length = 6, },
+		{.offset = 96, .length = 6, },
+		{.offset = 112, .length = 6, },
+	},
+};
+#endif
+
+struct nand_ecclayout *davinci_nand_ecclayouts[] = {
+	&nand_davinci_4bit_layout_oobfirst,
+	&nand_keystone_rbl_4bit_layout_oobfirst,
+};
+
+int board_nand_get_ecclayout_idx(struct nand_chip *nand,
+				 struct nand_ecclayout *layout)
+{
+	int i;
+
+	if (!layout)
+		return -1;
+
+	for (i = 0; i < ARRAY_SIZE(davinci_nand_ecclayouts); i++)
+		if (davinci_nand_ecclayouts[i] == layout)
+			return i;
+
+	return -1;
+}
+
+struct nand_ecclayout *board_nand_get_ecclayout(int idx)
+{
+	if ((idx >= 0) && (idx < ARRAY_SIZE(davinci_nand_ecclayouts)))
+		return davinci_nand_ecclayouts[idx];
+	else
+		return NULL;
+}
+
+int board_nand_set_ecclayout(struct mtd_info *mtd, int idx)
+{
+	int i;
+	struct nand_ecclayout *layout;
+	struct nand_chip *nand = mtd->priv;
+
+	if (idx < 0 || idx >= ARRAY_SIZE(davinci_nand_ecclayouts))
+		return -1;
+
+	layout = nand->ecc.layout;
+	layout = davinci_nand_ecclayouts[idx];
+
+	layout->oobavail = 0;
+	for (i = 0; layout->oobfree[i].length &&
+	     i < ARRAY_SIZE(layout->oobfree); i++)
+		layout->oobavail += layout->oobfree[i].length;
+
+	mtd->oobavail = layout->oobavail;
+
+	return 0;
+}
+#endif /* CONFIG_CMD_NAND_ECCLAYOUT */
+
 static void nand_davinci_4bit_enable_hwecc(struct mtd_info *mtd, int mode)
 {
 	u32 val;
-- 
1.8.3.2

[U-Boot] [U-boot] [Patch v2 2/3] common: cmd_nand: add nand ecclayout command

[Ivan Khoronzhuk]

From: WingMan Kwok <w-kwok2@ti.com>

This commit adds a nand ecclayout command that allows the ecclayout of
the current nand device to be changed during run time. This feature is
useful when using u-boot to write something to nand flash that will be
read by other applications, such as ROM bootloader, that expects a
different ECC layout. In that case, change the current nand device
ecclayout using the "nand ecclayout set" command before writing the
data to nand flash.

Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 common/cmd_nand.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 doc/README.nand   | 12 ++++++++
 include/nand.h    |  7 +++++
 3 files changed, 110 insertions(+)

diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index a84f7dc..61d3fcc 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -462,6 +462,51 @@ static void adjust_size_for_badblocks(loff_t *size, loff_t offset, int dev)
 	}
 }
 
+#ifdef CONFIG_CMD_NAND_ECCLAYOUT
+static void nand_print_ecclayout_info(struct nand_ecclayout *layout)
+{
+	int i;
+	struct nand_oobfree *oobfree;
+
+	if (!layout)
+		return;
+
+	printf("  num ecc bytes: %d\n", layout->eccbytes);
+	puts("  ecc pos:\n    ");
+	for (i = 0; i < layout->eccbytes; i++) {
+		if (i && !(i % 8))
+			printf("\n    ");
+
+		printf("%4d ", layout->eccpos[i]);
+	}
+
+	puts("\n  oobfree:\n");
+	puts("    offset length\n");
+	oobfree = layout->oobfree;
+	for (i = 0; oobfree->length && i < MTD_MAX_OOBFREE_ENTRIES_LARGE; i++) {
+		printf("    %3d   %3d\n", oobfree->offset, oobfree->length);
+		oobfree++;
+	}
+}
+
+static void nand_print_device_ecclayout(int dev)
+{
+	int idx;
+	nand_info_t *nand = &nand_info[dev];
+	struct nand_chip *chip = nand->priv;
+
+	idx = board_nand_get_ecclayout_idx(chip, chip->ecc.layout);
+
+	if (idx < 0) {
+		puts("no ecc layout\n");
+		return;
+	}
+
+	printf("\necc layout %d:\n", idx);
+	nand_print_ecclayout_info(chip->ecc.layout);
+}
+#endif
+
 static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	int i, ret = 0;
@@ -830,6 +875,45 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	}
 #endif
 
+#ifdef CONFIG_CMD_NAND_ECCLAYOUT
+	if (strcmp(cmd, "ecclayout") == 0) {
+		int i;
+		struct nand_ecclayout *lt;
+		nand_info_t *nand = &nand_info[dev];
+
+		if (argc < 3) {
+			puts("Current device ecclayout:\n");
+			nand_print_device_ecclayout(dev);
+			return 0;
+		}
+
+		if (!strcmp(argv[2], "set")) {
+			if (argc < 4)
+				return 1;
+
+			i = (int)simple_strtoul(argv[3], NULL, 10);
+			if (board_nand_set_ecclayout(nand, i))
+				printf("Setting current device to ecc layout %d FAILED!\n", i);
+
+			return 0;
+		}
+
+		if (strcmp(argv[2], "all") != 0)
+			return 1;
+
+		/* show all available ecc layouts */
+		puts("Available ecc layouts:\n");
+
+		for (i = 0; (lt = board_nand_get_ecclayout(i)); i++) {
+
+			printf("\n ecc laytout %d:\n", i);
+			nand_print_ecclayout_info(lt);
+		}
+
+		return 0;
+	}
+#endif
+
 usage:
 	return CMD_RET_USAGE;
 }
@@ -884,6 +968,13 @@ static char nand_help_text[] =
 	"nand env.oob set off|partition - set enviromnent offset\n"
 	"nand env.oob get - get environment offset"
 #endif
+#ifdef CONFIG_CMD_NAND_ECCLAYOUT
+	"\n"
+	"nand ecclayout [all] - show ecclayout, 'all' shows all \n"
+	"    available ecc layouts for setting to current device\n"
+	"nand ecclayout set idx - sets current device ecc layout \n"
+	"    to layout indexed by idx\n"
+#endif
 	"";
 #endif
 
diff --git a/doc/README.nand b/doc/README.nand
index 70cf768..6fdce26 100644
--- a/doc/README.nand
+++ b/doc/README.nand
@@ -87,6 +87,15 @@ Commands:
       a packed sequence of "data, oob, data, oob, ..." -- no alignment of
       individual pages is maintained.
 
+   nand ecclayout [all]
+      Enabled by the CONFIG_CMD_NAND_ECCLAYOUT macro. This command show current
+      ecclayout, "all" shows all available ecc layouts for setting to current
+      device.
+
+   nand ecclayout set idx
+      Enabled by the CONFIG_CMD_NAND_ECCLAYOUT macro. This command set current
+      device ecc layout to layout indexed by idx.
+
 Configuration Options:
 
    CONFIG_CMD_NAND
@@ -208,6 +217,9 @@ Configuration Options:
 	drivers/mtd/nand/ndfc.c
 	drivers/mtd/nand/omap_gpmc.c
 
+   CONFIG_CMD_NAND_ECCLAYOUT
+        Enables support of nand ecclayout commands.
+
 
 Platform specific options
 =========================
diff --git a/include/nand.h b/include/nand.h
index fc735d1..a810626 100644
--- a/include/nand.h
+++ b/include/nand.h
@@ -155,6 +155,13 @@ void nand_deselect(void);
 void board_nand_select_device(struct nand_chip *nand, int chip);
 #endif
 
+int board_nand_get_ecclayout_idx(struct nand_chip *nand,
+				 struct nand_ecclayout *layout);
+
+struct nand_ecclayout *board_nand_get_ecclayout(int idx);
+
+int board_nand_set_ecclayout(struct mtd_info *mtd, int idx);
+
 __attribute__((noreturn)) void nand_boot(void);
 
 #endif
-- 
1.8.3.2

[U-Boot] [U-boot] [Patch v2 3/3] k2hk: change default nand ecc layout

[Ivan Khoronzhuk]

For keystyone k2hk board the default nand layout is different
from davinci. So swich ecc layout at init in board file.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
---
 board/ti/k2hk_evm/board.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/board/ti/k2hk_evm/board.c b/board/ti/k2hk_evm/board.c
index ef90f9d..baa6ab7 100644
--- a/board/ti/k2hk_evm/board.c
+++ b/board/ti/k2hk_evm/board.c
@@ -11,6 +11,7 @@
 #include <exports.h>
 #include <fdt_support.h>
 #include <libfdt.h>
+#include <nand.h>
 
 #include <asm/arch/hardware.h>
 #include <asm/arch/clock.h>
@@ -19,6 +20,7 @@
 #include <asm/arch/emac_defs.h>
 #include <asm/arch/psc_defs.h>
 #include <asm/ti-common/ti-aemif.h>
+#include <asm/ti-common/davinci_nand.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -147,6 +149,15 @@ int cpu_to_bus(u32 *ptr, u32 length)
 	return 0;
 }
 
+int board_nand_init(struct nand_chip *chip)
+{
+	davinci_nand_init(chip);
+	chip->ecc.layout =
+		board_nand_get_ecclayout(NAND_KEYSTONE_RBL_4BIT_LAYOUT);
+
+	return 0;
+}
+
 #if defined(CONFIG_BOARD_EARLY_INIT_F)
 int board_early_init_f(void)
 {
-- 
1.8.3.2

[U-Boot] [U-boot] [Patch v2 3/3] k2hk: change default nand ecc layout

[Scott Wood]

On Sat, 2014-06-21 at 02:28 +0300, Ivan Khoronzhuk wrote:
> For keystyone k2hk board the default nand layout is different
> from davinci. So swich ecc layout at init in board file.
> 
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
>  board/ti/k2hk_evm/board.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/board/ti/k2hk_evm/board.c b/board/ti/k2hk_evm/board.c
> index ef90f9d..baa6ab7 100644
> --- a/board/ti/k2hk_evm/board.c
> +++ b/board/ti/k2hk_evm/board.c
> @@ -11,6 +11,7 @@
>  #include <exports.h>
>  #include <fdt_support.h>
>  #include <libfdt.h>
> +#include <nand.h>
>  
>  #include <asm/arch/hardware.h>
>  #include <asm/arch/clock.h>
> @@ -19,6 +20,7 @@
>  #include <asm/arch/emac_defs.h>
>  #include <asm/arch/psc_defs.h>
>  #include <asm/ti-common/ti-aemif.h>
> +#include <asm/ti-common/davinci_nand.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -147,6 +149,15 @@ int cpu_to_bus(u32 *ptr, u32 length)
>  	return 0;
>  }
>  
> +int board_nand_init(struct nand_chip *chip)
> +{
> +	davinci_nand_init(chip);
> +	chip->ecc.layout =
> +		board_nand_get_ecclayout(NAND_KEYSTONE_RBL_4BIT_LAYOUT);
> +
> +	return 0;
> +}

Shouldn't you be calling board_nand_set_ecclayout()?  How will oobavail
get set?

Is it really OK to start with the wrong ECC layout and switch later?
Are you depending on lazy scanning of the BBT?

For that matter, you also seem to be assuming that markbad will never be
called when the ECC is set to something other than what the BBT uses.

-Scott

[U-Boot] [U-boot] [Patch v2 3/3] k2hk: change default nand ecc layout

[Ivan Khoronzhuk]

On 06/21/2014 02:40 AM, Scott Wood wrote:
> On Sat, 2014-06-21 at 02:28 +0300, Ivan Khoronzhuk wrote:
>> For keystyone k2hk board the default nand layout is different
>> from davinci. So swich ecc layout at init in board file.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> ---
>>   board/ti/k2hk_evm/board.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/board/ti/k2hk_evm/board.c b/board/ti/k2hk_evm/board.c
>> index ef90f9d..baa6ab7 100644
>> --- a/board/ti/k2hk_evm/board.c
>> +++ b/board/ti/k2hk_evm/board.c
>> @@ -11,6 +11,7 @@
>>   #include <exports.h>
>>   #include <fdt_support.h>
>>   #include <libfdt.h>
>> +#include <nand.h>
>>   
>>   #include <asm/arch/hardware.h>
>>   #include <asm/arch/clock.h>
>> @@ -19,6 +20,7 @@
>>   #include <asm/arch/emac_defs.h>
>>   #include <asm/arch/psc_defs.h>
>>   #include <asm/ti-common/ti-aemif.h>
>> +#include <asm/ti-common/davinci_nand.h>
>>   
>>   DECLARE_GLOBAL_DATA_PTR;
>>   
>> @@ -147,6 +149,15 @@ int cpu_to_bus(u32 *ptr, u32 length)
>>   	return 0;
>>   }
>>   
>> +int board_nand_init(struct nand_chip *chip)
>> +{
>> +	davinci_nand_init(chip);
>> +	chip->ecc.layout =
>> +		board_nand_get_ecclayout(NAND_KEYSTONE_RBL_4BIT_LAYOUT);
>> +
>> +	return 0;
>> +}
> Shouldn't you be calling board_nand_set_ecclayout()?  How will oobavail
> get set?

There is no reason to set oobavail here, I suppose that's done while int 
nand_scan_tail().

>
> Is it really OK to start with the wrong ECC layout and switch later?

It can be used to boot from nand.

> Are you depending on lazy scanning of the BBT?

No, define CONFIG_SYS_NAND_USE_FLASH_BBT

>
> For that matter, you also seem to be assuming that markbad will never be
> called when the ECC is set to something other than what the BBT uses.
>
> -Scott
>
>
I suppose that only ECC layout is different.

-- 
Regards,
Ivan Khoronzhuk

[U-Boot] [U-boot] [Patch v2 3/3] k2hk: change default nand ecc layout

[Scott Wood]

On Mon, 2014-06-23 at 18:26 +0300, Ivan Khoronzhuk wrote:
> On 06/21/2014 02:40 AM, Scott Wood wrote:
> > On Sat, 2014-06-21 at 02:28 +0300, Ivan Khoronzhuk wrote:
> >> For keystyone k2hk board the default nand layout is different
> >> from davinci. So swich ecc layout at init in board file.
> >>
> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> >> ---
> >>   board/ti/k2hk_evm/board.c | 11 +++++++++++
> >>   1 file changed, 11 insertions(+)
> >>
> >> diff --git a/board/ti/k2hk_evm/board.c b/board/ti/k2hk_evm/board.c
> >> index ef90f9d..baa6ab7 100644
> >> --- a/board/ti/k2hk_evm/board.c
> >> +++ b/board/ti/k2hk_evm/board.c
> >> @@ -11,6 +11,7 @@
> >>   #include <exports.h>
> >>   #include <fdt_support.h>
> >>   #include <libfdt.h>
> >> +#include <nand.h>
> >>   
> >>   #include <asm/arch/hardware.h>
> >>   #include <asm/arch/clock.h>
> >> @@ -19,6 +20,7 @@
> >>   #include <asm/arch/emac_defs.h>
> >>   #include <asm/arch/psc_defs.h>
> >>   #include <asm/ti-common/ti-aemif.h>
> >> +#include <asm/ti-common/davinci_nand.h>
> >>   
> >>   DECLARE_GLOBAL_DATA_PTR;
> >>   
> >> @@ -147,6 +149,15 @@ int cpu_to_bus(u32 *ptr, u32 length)
> >>   	return 0;
> >>   }
> >>   
> >> +int board_nand_init(struct nand_chip *chip)
> >> +{
> >> +	davinci_nand_init(chip);
> >> +	chip->ecc.layout =
> >> +		board_nand_get_ecclayout(NAND_KEYSTONE_RBL_4BIT_LAYOUT);
> >> +
> >> +	return 0;
> >> +}
> > Shouldn't you be calling board_nand_set_ecclayout()?  How will oobavail
> > get set?
> 
> There is no reason to set oobavail here, I suppose that's done while int 
> nand_scan_tail().

But nand_scan_tail() has already run at this point.  oobavail would be
set based on the wrong layout.

> > Is it really OK to start with the wrong ECC layout and switch later?
> 
> It can be used to boot from nand.

That doesn't answer my question.

> > Are you depending on lazy scanning of the BBT?
> 
> No, define CONFIG_SYS_NAND_USE_FLASH_BBT

My point is, if the scanning of the BBT happens before you set the
layout, it will be read using the wrong ECC layout.

> > For that matter, you also seem to be assuming that markbad will never be
> > called when the ECC is set to something other than what the BBT uses.
> >
> > -Scott
> >
> >
> I suppose that only ECC layout is different.

"only"?  That's a big deal.

-Scott

[U-Boot] [U-boot] [Patch v2 3/3] k2hk: change default nand ecc layout

[Ivan Khoronzhuk]

On 06/25/2014 02:25 AM, Scott Wood wrote:
> On Mon, 2014-06-23 at 18:26 +0300, Ivan Khoronzhuk wrote:
>> On 06/21/2014 02:40 AM, Scott Wood wrote:
>>> On Sat, 2014-06-21 at 02:28 +0300, Ivan Khoronzhuk wrote:
>>>> For keystyone k2hk board the default nand layout is different
>>>> from davinci. So swich ecc layout at init in board file.
>>>>
>>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>>> ---
>>>>    board/ti/k2hk_evm/board.c | 11 +++++++++++
>>>>    1 file changed, 11 insertions(+)
>>>>
>>>> diff --git a/board/ti/k2hk_evm/board.c b/board/ti/k2hk_evm/board.c
>>>> index ef90f9d..baa6ab7 100644
>>>> --- a/board/ti/k2hk_evm/board.c
>>>> +++ b/board/ti/k2hk_evm/board.c
>>>> @@ -11,6 +11,7 @@
>>>>    #include <exports.h>
>>>>    #include <fdt_support.h>
>>>>    #include <libfdt.h>
>>>> +#include <nand.h>
>>>>    
>>>>    #include <asm/arch/hardware.h>
>>>>    #include <asm/arch/clock.h>
>>>> @@ -19,6 +20,7 @@
>>>>    #include <asm/arch/emac_defs.h>
>>>>    #include <asm/arch/psc_defs.h>
>>>>    #include <asm/ti-common/ti-aemif.h>
>>>> +#include <asm/ti-common/davinci_nand.h>
>>>>    
>>>>    DECLARE_GLOBAL_DATA_PTR;
>>>>    
>>>> @@ -147,6 +149,15 @@ int cpu_to_bus(u32 *ptr, u32 length)
>>>>    	return 0;
>>>>    }
>>>>    
>>>> +int board_nand_init(struct nand_chip *chip)
>>>> +{
>>>> +	davinci_nand_init(chip);
>>>> +	chip->ecc.layout =
>>>> +		board_nand_get_ecclayout(NAND_KEYSTONE_RBL_4BIT_LAYOUT);
>>>> +
>>>> +	return 0;
>>>> +}
>>> Shouldn't you be calling board_nand_set_ecclayout()?  How will oobavail
>>> get set?
>> There is no reason to set oobavail here, I suppose that's done while int
>> nand_scan_tail().
> But nand_scan_tail() has already run at this point.  oobavail would be
> set based on the wrong layout.

Where did you get.. I've checked it again. It was called after 
board_nand_init().

nand_init_chip
         -> board_nand_init
                 -> nand_scan
                         -> nand_scan_tail

>
>>> Is it really OK to start with the wrong ECC layout and switch later?
>> It can be used to boot from nand.
> That doesn't answer my question.

Seems It can be started with default ecc layout.
So I'm going to remove subj patch.

>
>>> Are you depending on lazy scanning of the BBT?
>> No, define CONFIG_SYS_NAND_USE_FLASH_BBT
> My point is, if the scanning of the BBT happens before you set the
> layout, it will be read using the wrong ECC layout.
>
>>> For that matter, you also seem to be assuming that markbad will never be
>>> called when the ECC is set to something other than what the BBT uses.
>>>
>>> -Scott
>>>
>>>
>> I suppose that only ECC layout is different.
> "only"?  That's a big deal.
>
> -Scott
>
>

If I correctly understand you, what if while we are writing data a new 
bad block is found?
Then we try to update BBT wich was written using a different layout....

Well probably I should rewrite BBT when layout is switched..
Will see if it can be done correctly.
Thanks.

-- 
Regards,
Ivan Khoronzhuk

[U-Boot] [U-boot] [Patch v2 3/3] k2hk: change default nand ecc layout

[Scott Wood]

On Wed, 2014-06-25 at 18:42 +0300, Ivan Khoronzhuk wrote:
> On 06/25/2014 02:25 AM, Scott Wood wrote:
> > On Mon, 2014-06-23 at 18:26 +0300, Ivan Khoronzhuk wrote:
> >> On 06/21/2014 02:40 AM, Scott Wood wrote:
> >>> On Sat, 2014-06-21 at 02:28 +0300, Ivan Khoronzhuk wrote:
> >>>> For keystyone k2hk board the default nand layout is different
> >>>> from davinci. So swich ecc layout at init in board file.
> >>>>
> >>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> >>>> ---
> >>>>    board/ti/k2hk_evm/board.c | 11 +++++++++++
> >>>>    1 file changed, 11 insertions(+)
> >>>>
> >>>> diff --git a/board/ti/k2hk_evm/board.c b/board/ti/k2hk_evm/board.c
> >>>> index ef90f9d..baa6ab7 100644
> >>>> --- a/board/ti/k2hk_evm/board.c
> >>>> +++ b/board/ti/k2hk_evm/board.c
> >>>> @@ -11,6 +11,7 @@
> >>>>    #include <exports.h>
> >>>>    #include <fdt_support.h>
> >>>>    #include <libfdt.h>
> >>>> +#include <nand.h>
> >>>>    
> >>>>    #include <asm/arch/hardware.h>
> >>>>    #include <asm/arch/clock.h>
> >>>> @@ -19,6 +20,7 @@
> >>>>    #include <asm/arch/emac_defs.h>
> >>>>    #include <asm/arch/psc_defs.h>
> >>>>    #include <asm/ti-common/ti-aemif.h>
> >>>> +#include <asm/ti-common/davinci_nand.h>
> >>>>    
> >>>>    DECLARE_GLOBAL_DATA_PTR;
> >>>>    
> >>>> @@ -147,6 +149,15 @@ int cpu_to_bus(u32 *ptr, u32 length)
> >>>>    	return 0;
> >>>>    }
> >>>>    
> >>>> +int board_nand_init(struct nand_chip *chip)
> >>>> +{
> >>>> +	davinci_nand_init(chip);
> >>>> +	chip->ecc.layout =
> >>>> +		board_nand_get_ecclayout(NAND_KEYSTONE_RBL_4BIT_LAYOUT);
> >>>> +
> >>>> +	return 0;
> >>>> +}
> >>> Shouldn't you be calling board_nand_set_ecclayout()?  How will oobavail
> >>> get set?
> >> There is no reason to set oobavail here, I suppose that's done while int
> >> nand_scan_tail().
> > But nand_scan_tail() has already run at this point.  oobavail would be
> > set based on the wrong layout.
> 
> Where did you get.. I've checked it again. It was called after 
> board_nand_init().
> 
> nand_init_chip
>          -> board_nand_init
>                  -> nand_scan
>                          -> nand_scan_tail

Never mind, I was thinking of the flow when using
CONFIG_SYS_NAND_SELF_INIT.

Still, it's probably not great to introduce a mechanism that is
incompatible with transitioning to self-init.

> If I correctly understand you, what if while we are writing data a new 
> bad block is found?
> Then we try to update BBT wich was written using a different layout....
> 
> Well probably I should rewrite BBT when layout is switched..
> Will see if it can be done correctly.

Could you clarify the use case here?  Are you changing the layout at
runtime so that different portions of the chip use different ECC schemes
(e.g. because booting requires something different)?  If so, then you
don't want to rewrite the BBT based on this, and a better approach than
a command line switcher would be to be able to define different layouts
for different offset ranges.

If this is meant to be a permanent change that affects the entire chip,
then the entire chip (not just the BBT, but excluding factory bad block
markers) needs to be wiped when you change, and you should be recording
the default in the environment (e.g. hwconfig) rather than switching
with a command line option.

-Scott

[U-Boot] [U-boot] [Patch v2 3/3] k2hk: change default nand ecc layout

[Ivan Khoronzhuk]

On 06/25/2014 08:15 PM, Scott Wood wrote:
> On Wed, 2014-06-25 at 18:42 +0300, Ivan Khoronzhuk wrote:
>> On 06/25/2014 02:25 AM, Scott Wood wrote:
>>> On Mon, 2014-06-23 at 18:26 +0300, Ivan Khoronzhuk wrote:
>>>> On 06/21/2014 02:40 AM, Scott Wood wrote:
>>>>> On Sat, 2014-06-21 at 02:28 +0300, Ivan Khoronzhuk wrote:
>>>>>> For keystyone k2hk board the default nand layout is different
>>>>>> from davinci. So swich ecc layout at init in board file.
>>>>>>
>>>>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>>>>>> ---
>>>>>>     board/ti/k2hk_evm/board.c | 11 +++++++++++
>>>>>>     1 file changed, 11 insertions(+)
>>>>>>
>>>>>> diff --git a/board/ti/k2hk_evm/board.c b/board/ti/k2hk_evm/board.c
>>>>>> index ef90f9d..baa6ab7 100644
>>>>>> --- a/board/ti/k2hk_evm/board.c
>>>>>> +++ b/board/ti/k2hk_evm/board.c
>>>>>> @@ -11,6 +11,7 @@
>>>>>>     #include <exports.h>
>>>>>>     #include <fdt_support.h>
>>>>>>     #include <libfdt.h>
>>>>>> +#include <nand.h>
>>>>>>     
>>>>>>     #include <asm/arch/hardware.h>
>>>>>>     #include <asm/arch/clock.h>
>>>>>> @@ -19,6 +20,7 @@
>>>>>>     #include <asm/arch/emac_defs.h>
>>>>>>     #include <asm/arch/psc_defs.h>
>>>>>>     #include <asm/ti-common/ti-aemif.h>
>>>>>> +#include <asm/ti-common/davinci_nand.h>
>>>>>>     
>>>>>>     DECLARE_GLOBAL_DATA_PTR;
>>>>>>     
>>>>>> @@ -147,6 +149,15 @@ int cpu_to_bus(u32 *ptr, u32 length)
>>>>>>     	return 0;
>>>>>>     }
>>>>>>     
>>>>>> +int board_nand_init(struct nand_chip *chip)
>>>>>> +{
>>>>>> +	davinci_nand_init(chip);
>>>>>> +	chip->ecc.layout =
>>>>>> +		board_nand_get_ecclayout(NAND_KEYSTONE_RBL_4BIT_LAYOUT);
>>>>>> +
>>>>>> +	return 0;
>>>>>> +}
>>>>> Shouldn't you be calling board_nand_set_ecclayout()?  How will oobavail
>>>>> get set?
>>>> There is no reason to set oobavail here, I suppose that's done while int
>>>> nand_scan_tail().
>>> But nand_scan_tail() has already run at this point.  oobavail would be
>>> set based on the wrong layout.
>> Where did you get.. I've checked it again. It was called after
>> board_nand_init().
>>
>> nand_init_chip
>>           -> board_nand_init
>>                   -> nand_scan
>>                           -> nand_scan_tail
> Never mind, I was thinking of the flow when using
> CONFIG_SYS_NAND_SELF_INIT.
>
> Still, it's probably not great to introduce a mechanism that is
> incompatible with transitioning to self-init.
>
>> If I correctly understand you, what if while we are writing data a new
>> bad block is found?
>> Then we try to update BBT wich was written using a different layout....
>>
>> Well probably I should rewrite BBT when layout is switched..
>> Will see if it can be done correctly.
> Could you clarify the use case here?  Are you changing the layout at
> runtime so that different portions of the chip use different ECC schemes
> (e.g. because booting requires something different)?

Exactly. It has partitions with different ecc layouts.
ROM bootloader uses own ecclayout ...(and it doesn't touch BBT).

>   If so, then you
> don't want to rewrite the BBT based on this, and a better approach than
> a command line switcher would be to be able to define different layouts
> for different offset ranges.
>
> If this is meant to be a permanent change that affects the entire chip,
> then the entire chip (not just the BBT, but excluding factory bad block
> markers) needs to be wiped when you change, and you should be recording
> the default in the environment (e.g. hwconfig) rather than switching
> with a command line option.
>
> -Scott
>

Thanks Scott,
I'm thinking how to split chip for using different ecc layouts.
If it be OK, I'll remove commands in question.

-- 
Regards,
Ivan Khoronzhuk