[U-Boot] [PATCH v1 0/6] mtd: nand: omap: clean up of omap_elm and omap_gpmc driver

[U-Boot] [PATCH v1 0/6] mtd: nand: omap: clean up of omap_elm and omap_gpmc driver

[Pekon Gupta]

This patch series
 - removes un-wanted & redundant code in omap_elm and omap_gpmc drivers
 - refactors some private struct in omap_gpmc to make them generic across ecc-schemes
 - minor code fixes and clean-ups
This series can be as-it-is applied on u-boot 'v2014.04-rc3'.

*Testing Procedure*
Boot tested on am335x_evm (MLO -> U-boot -> kernel with filesystem flashed from U-boot)
using OMAP_ECC_BCH8_CODE_HW and OMAP_ECC_BCH8_CODE_HW_DETECTION_SW ecc-schemes 


Pekon Gupta (6):
  mtd: nand: omap_elm: remove #include omap_gpmc.h
  mtd: nand: omap_elm: use bch_type instead of nibble count to
    differentiate between BCH4/BCH8/BCH16
  mtd: nand: omap_elm: use macros for register definitions
  mtd: nand: omap_gpmc: remove unused members of 'struct nand_bch_priv'
  mtd: nand: omap_gpmc: rename struct nand_bch_priv to struct
    omap_nand_info
  mtd: nand: omap_gpmc: minor cleanup of omap_correct_data_bch

 drivers/mtd/nand/omap_elm.c  | 21 ++++------
 drivers/mtd/nand/omap_gpmc.c | 97 +++++++++++++++++---------------------------
 include/linux/mtd/omap_elm.h | 11 +++--
 3 files changed, 52 insertions(+), 77 deletions(-)

-- 
1.8.5.1.163.gd7aced9

[U-Boot] [PATCH v1 1/6] mtd: nand: omap_elm: remove #include omap_gpmc.h

[Pekon Gupta]

There is no dependency of omap_elm.c on omap_gpmc.h

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 drivers/mtd/nand/omap_elm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mtd/nand/omap_elm.c b/drivers/mtd/nand/omap_elm.c
index 47b1f1b..4c65f3b 100644
--- a/drivers/mtd/nand/omap_elm.c
+++ b/drivers/mtd/nand/omap_elm.c
@@ -16,7 +16,6 @@
 #include <common.h>
 #include <asm/io.h>
 #include <asm/errno.h>
-#include <linux/mtd/omap_gpmc.h>
 #include <linux/mtd/omap_elm.h>
 #include <asm/arch/hardware.h>
 
-- 
1.8.5.1.163.gd7aced9

[U-Boot] [PATCH v1 2/6] mtd: nand: omap_elm: use bch_type instead of nibble count to differentiate between BCH4/BCH8/BCH16

[Pekon Gupta]

ELM hardware engine support ECC error detection for multiple ECC strengths like
 +------+------------------------+
 |Type  | ECC syndrome length    |
 +------+------------------------+
 |BCH4  | 6.5 bytes = 13 nibbles |
 |BCH8  | 13 byte = 26 nibbles   |
 |BCH16 | 26 bytes = 52 nibbles  |
 +------+------------------------+

Current implementation of omap_elm driver uses ECC syndrom length (in 'nibbles')
to differentiate between BCH4/BCH8/BCH16. This patch replaces it with 'bch_type'

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 drivers/mtd/nand/omap_elm.c  | 20 ++++++++------------
 drivers/mtd/nand/omap_gpmc.c | 10 ++--------
 include/linux/mtd/omap_elm.h |  2 +-
 3 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/drivers/mtd/nand/omap_elm.c b/drivers/mtd/nand/omap_elm.c
index 4c65f3b..5b2172a 100644
--- a/drivers/mtd/nand/omap_elm.c
+++ b/drivers/mtd/nand/omap_elm.c
@@ -24,14 +24,12 @@
 struct elm *elm_cfg;
 
 /**
- * elm_load_syndromes - Load BCH syndromes based on nibble selection
+ * elm_load_syndromes - Load BCH syndromes based on bch_type selection
  * @syndrome: BCH syndrome
- * @nibbles:
+ * @bch_type: BCH4/BCH8/BCH16
  * @poly: Syndrome Polynomial set to use
- *
- * Load BCH syndromes based on nibble selection
  */
-static void elm_load_syndromes(u8 *syndrome, u32 nibbles, u8 poly)
+static void elm_load_syndromes(u8 *syndrome, enum bch_level bch_type, u8 poly)
 {
 	u32 *ptr;
 	u32 val;
@@ -47,8 +45,7 @@ static void elm_load_syndromes(u8 *syndrome, u32 nibbles, u8 poly)
 				(syndrome[7] << 24);
 	writel(val, ptr);
 
-	/* BCH 8-bit with 26 nibbles (4*8=32) */
-	if (nibbles > 13) {
+	if (bch_type == BCH_4_BIT || bch_type == BCH_8_BIT) {
 		/* reg 2 */
 		ptr = &elm_cfg->syndrome_fragments[poly].syndrome_fragment_x[2];
 		val = syndrome[8] | (syndrome[9] << 8) | (syndrome[10] << 16) |
@@ -61,8 +58,7 @@ static void elm_load_syndromes(u8 *syndrome, u32 nibbles, u8 poly)
 		writel(val, ptr);
 	}
 
-	/* BCH 16-bit with 52 nibbles (7*8=56) */
-	if (nibbles > 26) {
+	if (bch_type == BCH_16_BIT) {
 		/* reg 4 */
 		ptr = &elm_cfg->syndrome_fragments[poly].syndrome_fragment_x[4];
 		val = syndrome[16] | (syndrome[17] << 8) |
@@ -86,7 +82,7 @@ static void elm_load_syndromes(u8 *syndrome, u32 nibbles, u8 poly)
 /**
  * elm_check_errors - Check for BCH errors and return error locations
  * @syndrome: BCH syndrome
- * @nibbles:
+ * @bch_type: BCH4/BCH8/BCH16
  * @error_count: Returns number of errrors in the syndrome
  * @error_locations: Returns error locations (in decimal) in this array
  *
@@ -94,14 +90,14 @@ static void elm_load_syndromes(u8 *syndrome, u32 nibbles, u8 poly)
  * and locations in the array passed. Returns -1 if error is not correctable,
  * else returns 0
  */
-int elm_check_error(u8 *syndrome, u32 nibbles, u32 *error_count,
+int elm_check_error(u8 *syndrome, enum bch_level bch_type, u32 *error_count,
 		u32 *error_locations)
 {
 	u8 poly = ELM_DEFAULT_POLY;
 	s8 i;
 	u32 location_status;
 
-	elm_load_syndromes(syndrome, nibbles, poly);
+	elm_load_syndromes(syndrome, bch_type, poly);
 
 	/* start processing */
 	writel((readl(&elm_cfg->syndrome_fragments[poly].syndrome_fragment_x[6])
diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c
index 881a636..af38f00 100644
--- a/drivers/mtd/nand/omap_gpmc.c
+++ b/drivers/mtd/nand/omap_gpmc.c
@@ -153,7 +153,6 @@ static int __maybe_unused omap_correct_data(struct mtd_info *mtd, uint8_t *dat,
 struct nand_bch_priv {
 	uint8_t mode;
 	uint8_t type;
-	uint8_t nibbles;
 	struct bch_control *control;
 	enum omap_ecc ecc_scheme;
 };
@@ -163,11 +162,6 @@ struct nand_bch_priv {
 #define ECC_BCH8	1
 #define ECC_BCH16	2
 
-/* BCH nibbles for diff bch levels */
-#define ECC_BCH4_NIBBLES	13
-#define ECC_BCH8_NIBBLES	26
-#define ECC_BCH16_NIBBLES	52
-
 /*
  * This can be a single instance cause all current users have only one NAND
  * with nearly the same setup (BCH8, some with ELM and others with sw BCH
@@ -176,7 +170,6 @@ struct nand_bch_priv {
  */
 static __maybe_unused struct nand_bch_priv bch_priv = {
 	.type = ECC_BCH8,
-	.nibbles = ECC_BCH8_NIBBLES,
 	.control = NULL
 };
 
@@ -383,7 +376,8 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
 	}
 	/* use elm module to check for errors */
 	elm_config((enum bch_level)(bch->type));
-	if (elm_check_error(calc_ecc, bch->nibbles, &error_count, error_loc)) {
+	if (elm_check_error(calc_ecc, (enum bch_level)bch->type,
+					&error_count, error_loc)) {
 		printf("nand: error: uncorrectable ECC errors\n");
 		return -EINVAL;
 	}
diff --git a/include/linux/mtd/omap_elm.h b/include/linux/mtd/omap_elm.h
index 45454ea..a6e9591 100644
--- a/include/linux/mtd/omap_elm.h
+++ b/include/linux/mtd/omap_elm.h
@@ -68,7 +68,7 @@ struct elm {
 	struct location  error_location[8];	/* 0x800 */
 };
 
-int elm_check_error(u8 *syndrome, u32 nibbles, u32 *error_count,
+int elm_check_error(u8 *syndrome, enum bch_level bch_type, u32 *error_count,
 		u32 *error_locations);
 int elm_config(enum bch_level level);
 void elm_reset(void);
-- 
1.8.5.1.163.gd7aced9

[U-Boot] [PATCH v1 3/6] mtd: nand: omap_elm: use macros for register definitions

[Pekon Gupta]

This patch adds macros for following parameters of ELM Hardware engine
 - ELM_MAX_CHANNELS: ELM can process 8 data streams simultaneously
 - ELM_MAX_ERRORS: ELM can detect upto 16 ECC error when using BCH16 scheme

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 include/linux/mtd/omap_elm.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/linux/mtd/omap_elm.h b/include/linux/mtd/omap_elm.h
index a6e9591..b8096b0 100644
--- a/include/linux/mtd/omap_elm.h
+++ b/include/linux/mtd/omap_elm.h
@@ -24,6 +24,9 @@
 #define ELM_LOCATION_STATUS_ECC_CORRECTABLE_MASK	(0x100)
 #define ELM_LOCATION_STATUS_ECC_NB_ERRORS_MASK		(0x1F)
 
+#define ELM_MAX_CHANNELS				8
+#define ELM_MAX_ERROR_COUNT				16
+
 #ifndef __ASSEMBLY__
 
 enum bch_level {
@@ -43,7 +46,7 @@ struct syndrome {
 struct location {
 	u32 location_status;		/* 0x800 */
 	u8 res1[124];			/* 0x804 */
-	u32 error_location_x[16];	/* 0x880.... */
+	u32 error_location_x[ELM_MAX_ERROR_COUNT]; /* 0x880, 0x980, .. */
 	u8 res2[64];			/* 0x8c0 */
 };
 
@@ -63,9 +66,9 @@ struct elm {
 	u8 res2[92];				/* 0x024 */
 	u32 page_ctrl;				/* 0x080 */
 	u8 res3[892];				/* 0x084 */
-	struct  syndrome syndrome_fragments[8]; /* 0x400 */
+	struct  syndrome syndrome_fragments[ELM_MAX_CHANNELS]; /* 0x400,0x420 */
 	u8 res4[512];				/* 0x600 */
-	struct location  error_location[8];	/* 0x800 */
+	struct location  error_location[ELM_MAX_CHANNELS]; /* 0x800,0x900 ... */
 };
 
 int elm_check_error(u8 *syndrome, enum bch_level bch_type, u32 *error_count,
-- 
1.8.5.1.163.gd7aced9

[U-Boot] [PATCH v1 4/6] mtd: nand: omap_gpmc: remove unused members of 'struct nand_bch_priv'

[Pekon Gupta]

This patch prepares to refactor 'struct nand_bch_priv' -> 'struct omap_nand_info'
And thus performs following clean-ups:
 - remove nand_bch_priv.type: use nand_bch_priv.ecc_scheme instead
 - remove nand_bch_priv.mode: <unused>

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 drivers/mtd/nand/omap_gpmc.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c
index af38f00..4bd296e 100644
--- a/drivers/mtd/nand/omap_gpmc.c
+++ b/drivers/mtd/nand/omap_gpmc.c
@@ -151,17 +151,10 @@ static int __maybe_unused omap_correct_data(struct mtd_info *mtd, uint8_t *dat,
  * Generic BCH interface
  */
 struct nand_bch_priv {
-	uint8_t mode;
-	uint8_t type;
 	struct bch_control *control;
 	enum omap_ecc ecc_scheme;
 };
 
-/* bch types */
-#define ECC_BCH4	0
-#define ECC_BCH8	1
-#define ECC_BCH16	2
-
 /*
  * This can be a single instance cause all current users have only one NAND
  * with nearly the same setup (BCH8, some with ELM and others with sw BCH
@@ -169,7 +162,6 @@ struct nand_bch_priv {
  * When some users with other BCH strength will exists this have to change!
  */
 static __maybe_unused struct nand_bch_priv bch_priv = {
-	.type = ECC_BCH8,
 	.control = NULL
 };
 
@@ -342,6 +334,7 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
 	uint32_t eccbytes = chip->ecc.bytes;
 	uint32_t error_count = 0, error_max;
 	uint32_t error_loc[8];
+	enum bch_level bch_type;
 	uint32_t i, ecc_flag = 0;
 	uint8_t count, err = 0;
 	uint32_t byte_pos, bit_pos;
@@ -369,22 +362,22 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
 	 */
 	switch (bch->ecc_scheme) {
 	case OMAP_ECC_BCH8_CODE_HW:
+		bch_type = BCH_8_BIT;
 		omap_reverse_list(calc_ecc, eccbytes - 1);
 		break;
 	default:
 		return -EINVAL;
 	}
 	/* use elm module to check for errors */
-	elm_config((enum bch_level)(bch->type));
-	if (elm_check_error(calc_ecc, (enum bch_level)bch->type,
-					&error_count, error_loc)) {
+	elm_config(bch_type);
+	if (elm_check_error(calc_ecc, bch_type, &error_count, error_loc)) {
 		printf("nand: error: uncorrectable ECC errors\n");
 		return -EINVAL;
 	}
 	/* correct bch error */
 	for (count = 0; count < error_count; count++) {
-		switch (bch->type) {
-		case ECC_BCH8:
+		switch (bch->ecc_scheme) {
+		case OMAP_ECC_BCH8_CODE_HW:
 			/* 14th byte in ECC is reserved to match ROM layout */
 			error_max = SECTOR_BYTES + (eccbytes - 1);
 			break;
@@ -562,7 +555,6 @@ static int omap_select_ecc_scheme(struct nand_chip *nand,
 		/* For this ecc-scheme, ecc.bytes, ecc.layout, ... are
 		 * initialized in nand_scan_tail(), so just set ecc.mode */
 		bch_priv.control	= NULL;
-		bch_priv.type		= 0;
 		nand->ecc.mode		= NAND_ECC_SOFT;
 		nand->ecc.layout	= NULL;
 		nand->ecc.size		= 0;
@@ -578,7 +570,6 @@ static int omap_select_ecc_scheme(struct nand_chip *nand,
 			return -EINVAL;
 		}
 		bch_priv.control	= NULL;
-		bch_priv.type		= 0;
 		/* populate ecc specific fields */
 		memset(&nand->ecc, 0, sizeof(struct nand_ecc_ctrl));
 		nand->ecc.mode		= NAND_ECC_HW;
@@ -617,7 +608,6 @@ static int omap_select_ecc_scheme(struct nand_chip *nand,
 			printf("nand: error: could not init_bch()\n");
 			return -ENODEV;
 		}
-		bch_priv.type = ECC_BCH8;
 		/* populate ecc specific fields */
 		memset(&nand->ecc, 0, sizeof(struct nand_ecc_ctrl));
 		nand->ecc.mode		= NAND_ECC_HW;
@@ -659,7 +649,6 @@ static int omap_select_ecc_scheme(struct nand_chip *nand,
 		}
 		/* intialize ELM for ECC error detection */
 		elm_init();
-		bch_priv.type		= ECC_BCH8;
 		/* populate ecc specific fields */
 		memset(&nand->ecc, 0, sizeof(struct nand_ecc_ctrl));
 		nand->ecc.mode		= NAND_ECC_HW;
-- 
1.8.5.1.163.gd7aced9

[U-Boot] [PATCH v1 5/6] mtd: nand: omap_gpmc: rename struct nand_bch_priv to struct omap_nand_info

[Pekon Gupta]

This patch renames 'struct nand_bch_priv' which currently holds private data only
for BCH ECC schemes, into 'struct omap_nand_info' so that same can be used for
all ECC schemes

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 drivers/mtd/nand/omap_gpmc.c | 58 ++++++++++++++++++++------------------------
 1 file changed, 26 insertions(+), 32 deletions(-)

diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c
index 4bd296e..ae541c2 100644
--- a/drivers/mtd/nand/omap_gpmc.c
+++ b/drivers/mtd/nand/omap_gpmc.c
@@ -148,9 +148,9 @@ static int __maybe_unused omap_correct_data(struct mtd_info *mtd, uint8_t *dat,
 }
 
 /*
- * Generic BCH interface
+ * Driver configurations
  */
-struct nand_bch_priv {
+struct omap_nand_info {
 	struct bch_control *control;
 	enum omap_ecc ecc_scheme;
 };
@@ -161,7 +161,7 @@ struct nand_bch_priv {
  * library).
  * When some users with other BCH strength will exists this have to change!
  */
-static __maybe_unused struct nand_bch_priv bch_priv = {
+static __maybe_unused struct omap_nand_info omap_nand_info = {
 	.control = NULL
 };
 
@@ -191,7 +191,7 @@ __maybe_unused
 static void omap_enable_hwecc(struct mtd_info *mtd, int32_t mode)
 {
 	struct nand_chip	*nand	= mtd->priv;
-	struct nand_bch_priv	*bch	= nand->priv;
+	struct omap_nand_info	*info = nand->priv;
 	unsigned int dev_width = (nand->options & NAND_BUSWIDTH_16) ? 1 : 0;
 	unsigned int ecc_algo = 0;
 	unsigned int bch_type = 0;
@@ -200,7 +200,7 @@ static void omap_enable_hwecc(struct mtd_info *mtd, int32_t mode)
 	u32 ecc_config_val = 0;
 
 	/* configure GPMC for specific ecc-scheme */
-	switch (bch->ecc_scheme) {
+	switch (info->ecc_scheme) {
 	case OMAP_ECC_HAM1_CODE_SW:
 		return;
 	case OMAP_ECC_HAM1_CODE_HW:
@@ -262,11 +262,11 @@ static int omap_calculate_ecc(struct mtd_info *mtd, const uint8_t *dat,
 				uint8_t *ecc_code)
 {
 	struct nand_chip *chip = mtd->priv;
-	struct nand_bch_priv *bch = chip->priv;
+	struct omap_nand_info	*info = chip->priv;
 	uint32_t *ptr, val = 0;
 	int8_t i = 0, j;
 
-	switch (bch->ecc_scheme) {
+	switch (info->ecc_scheme) {
 	case OMAP_ECC_HAM1_CODE_HW:
 		val = readl(&gpmc_cfg->ecc1_result);
 		ecc_code[0] = val & 0xFF;
@@ -294,7 +294,7 @@ static int omap_calculate_ecc(struct mtd_info *mtd, const uint8_t *dat,
 		return -EINVAL;
 	}
 	/* ECC scheme specific syndrome customizations */
-	switch (bch->ecc_scheme) {
+	switch (info->ecc_scheme) {
 	case OMAP_ECC_HAM1_CODE_HW:
 		break;
 #ifdef CONFIG_BCH
@@ -330,7 +330,7 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
 				uint8_t *read_ecc, uint8_t *calc_ecc)
 {
 	struct nand_chip *chip = mtd->priv;
-	struct nand_bch_priv *bch = chip->priv;
+	struct omap_nand_info	*info = chip->priv;
 	uint32_t eccbytes = chip->ecc.bytes;
 	uint32_t error_count = 0, error_max;
 	uint32_t error_loc[8];
@@ -360,7 +360,7 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
 	 * while reading ECC result we read it in big endian.
 	 * Hence while loading to ELM we have rotate to get the right endian.
 	 */
-	switch (bch->ecc_scheme) {
+	switch (info->ecc_scheme) {
 	case OMAP_ECC_BCH8_CODE_HW:
 		bch_type = BCH_8_BIT;
 		omap_reverse_list(calc_ecc, eccbytes - 1);
@@ -376,7 +376,7 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
 	}
 	/* correct bch error */
 	for (count = 0; count < error_count; count++) {
-		switch (bch->ecc_scheme) {
+		switch (info->ecc_scheme) {
 		case OMAP_ECC_BCH8_CODE_HW:
 			/* 14th byte in ECC is reserved to match ROM layout */
 			error_max = SECTOR_BYTES + (eccbytes - 1);
@@ -483,10 +483,10 @@ static int omap_correct_data_bch_sw(struct mtd_info *mtd, u_char *data,
 	/* cannot correct more than 8 errors */
 	unsigned int errloc[8];
 	struct nand_chip *chip = mtd->priv;
-	struct nand_bch_priv *chip_priv = chip->priv;
-	struct bch_control *bch = chip_priv->control;
+	struct omap_nand_info *info = chip->priv;
 
-	count = decode_bch(bch, NULL, 512, read_ecc, calc_ecc, NULL, errloc);
+	count = decode_bch(info->control, NULL, 512, read_ecc, calc_ecc,
+							NULL, errloc);
 	if (count > 0) {
 		/* correct errors */
 		for (i = 0; i < count; i++) {
@@ -522,15 +522,11 @@ static int omap_correct_data_bch_sw(struct mtd_info *mtd, u_char *data,
 static void __maybe_unused omap_free_bch(struct mtd_info *mtd)
 {
 	struct nand_chip *chip = mtd->priv;
-	struct nand_bch_priv *chip_priv = chip->priv;
-	struct bch_control *bch = NULL;
+	struct omap_nand_info *info = chip->priv;
 
-	if (chip_priv)
-		bch = chip_priv->control;
-
-	if (bch) {
-		free_bch(bch);
-		chip_priv->control = NULL;
+	if (info->control) {
+		free_bch(info->control);
+		info->control = NULL;
 	}
 }
 #endif /* CONFIG_BCH */
@@ -544,7 +540,7 @@ static void __maybe_unused omap_free_bch(struct mtd_info *mtd)
  */
 static int omap_select_ecc_scheme(struct nand_chip *nand,
 	enum omap_ecc ecc_scheme, unsigned int pagesize, unsigned int oobsize) {
-	struct nand_bch_priv	*bch		= nand->priv;
+	struct omap_nand_info	*info		= nand->priv;
 	struct nand_ecclayout	*ecclayout	= &omap_ecclayout;
 	int eccsteps = pagesize / SECTOR_BYTES;
 	int i;
@@ -554,11 +550,10 @@ static int omap_select_ecc_scheme(struct nand_chip *nand,
 		debug("nand: selected OMAP_ECC_HAM1_CODE_SW\n");
 		/* For this ecc-scheme, ecc.bytes, ecc.layout, ... are
 		 * initialized in nand_scan_tail(), so just set ecc.mode */
-		bch_priv.control	= NULL;
+		info->control		= NULL;
 		nand->ecc.mode		= NAND_ECC_SOFT;
 		nand->ecc.layout	= NULL;
 		nand->ecc.size		= 0;
-		bch->ecc_scheme		= OMAP_ECC_HAM1_CODE_SW;
 		break;
 
 	case OMAP_ECC_HAM1_CODE_HW:
@@ -569,7 +564,7 @@ static int omap_select_ecc_scheme(struct nand_chip *nand,
 				(3 * eccsteps) + BADBLOCK_MARKER_LENGTH));
 			return -EINVAL;
 		}
-		bch_priv.control	= NULL;
+		info->control		= NULL;
 		/* populate ecc specific fields */
 		memset(&nand->ecc, 0, sizeof(struct nand_ecc_ctrl));
 		nand->ecc.mode		= NAND_ECC_HW;
@@ -590,7 +585,6 @@ static int omap_select_ecc_scheme(struct nand_chip *nand,
 		ecclayout->oobfree[0].offset = i + BADBLOCK_MARKER_LENGTH;
 		ecclayout->oobfree[0].length = oobsize - ecclayout->eccbytes -
 						BADBLOCK_MARKER_LENGTH;
-		bch->ecc_scheme		= OMAP_ECC_HAM1_CODE_HW;
 		break;
 
 	case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
@@ -603,8 +597,8 @@ static int omap_select_ecc_scheme(struct nand_chip *nand,
 			return -EINVAL;
 		}
 		/* check if BCH S/W library can be used for error detection */
-		bch_priv.control = init_bch(13, 8, 0x201b);
-		if (!bch_priv.control) {
+		info->control = init_bch(13, 8, 0x201b);
+		if (!info->control) {
 			printf("nand: error: could not init_bch()\n");
 			return -ENODEV;
 		}
@@ -631,7 +625,6 @@ static int omap_select_ecc_scheme(struct nand_chip *nand,
 		ecclayout->oobfree[0].offset = i + BADBLOCK_MARKER_LENGTH;
 		ecclayout->oobfree[0].length = oobsize - ecclayout->eccbytes -
 						BADBLOCK_MARKER_LENGTH;
-		bch->ecc_scheme		= OMAP_ECC_BCH8_CODE_HW_DETECTION_SW;
 		break;
 #else
 		printf("nand: error: CONFIG_BCH required for ECC\n");
@@ -649,6 +642,7 @@ static int omap_select_ecc_scheme(struct nand_chip *nand,
 		}
 		/* intialize ELM for ECC error detection */
 		elm_init();
+		info->control		= NULL;
 		/* populate ecc specific fields */
 		memset(&nand->ecc, 0, sizeof(struct nand_ecc_ctrl));
 		nand->ecc.mode		= NAND_ECC_HW;
@@ -666,7 +660,6 @@ static int omap_select_ecc_scheme(struct nand_chip *nand,
 		ecclayout->oobfree[0].offset = i + BADBLOCK_MARKER_LENGTH;
 		ecclayout->oobfree[0].length = oobsize - ecclayout->eccbytes -
 						BADBLOCK_MARKER_LENGTH;
-		bch->ecc_scheme		= OMAP_ECC_BCH8_CODE_HW;
 		break;
 #else
 		printf("nand: error: CONFIG_NAND_OMAP_ELM required for ECC\n");
@@ -682,6 +675,7 @@ static int omap_select_ecc_scheme(struct nand_chip *nand,
 	if (ecc_scheme != OMAP_ECC_HAM1_CODE_SW)
 		nand->ecc.layout = ecclayout;
 
+	info->ecc_scheme = ecc_scheme;
 	return 0;
 }
 
@@ -785,7 +779,7 @@ int board_nand_init(struct nand_chip *nand)
 
 	nand->IO_ADDR_R = (void __iomem *)&gpmc_cfg->cs[cs].nand_dat;
 	nand->IO_ADDR_W = (void __iomem *)&gpmc_cfg->cs[cs].nand_cmd;
-	nand->priv	= &bch_priv;
+	nand->priv	= &omap_nand_info;
 	nand->cmd_ctrl	= omap_nand_hwcontrol;
 	nand->options	|= NAND_NO_PADDING | NAND_CACHEPRG;
 	/* If we are 16 bit dev, our gpmc config tells us that */
-- 
1.8.5.1.163.gd7aced9

[U-Boot] [PATCH v1 6/6] mtd: nand: omap_gpmc: minor cleanup of omap_correct_data_bch

[Pekon Gupta]

This patch tries to avoid some local pointer dereferences, by using common
local variables in omap_correct_data_bch()

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 drivers/mtd/nand/omap_gpmc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c
index ae541c2..a96f310 100644
--- a/drivers/mtd/nand/omap_gpmc.c
+++ b/drivers/mtd/nand/omap_gpmc.c
@@ -331,7 +331,7 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
 {
 	struct nand_chip *chip = mtd->priv;
 	struct omap_nand_info	*info = chip->priv;
-	uint32_t eccbytes = chip->ecc.bytes;
+	struct nand_ecc_ctrl	*ecc = &chip->ecc;
 	uint32_t error_count = 0, error_max;
 	uint32_t error_loc[8];
 	enum bch_level bch_type;
@@ -340,7 +340,7 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
 	uint32_t byte_pos, bit_pos;
 
 	/* check calculated ecc */
-	for (i = 0; i < chip->ecc.bytes && !ecc_flag; i++) {
+	for (i = 0; i < ecc->bytes && !ecc_flag; i++) {
 		if (calc_ecc[i] != 0x00)
 			ecc_flag = 1;
 	}
@@ -349,7 +349,7 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
 
 	/* check for whether its a erased-page */
 	ecc_flag = 0;
-	for (i = 0; i < chip->ecc.bytes && !ecc_flag; i++) {
+	for (i = 0; i < ecc->bytes && !ecc_flag; i++) {
 		if (read_ecc[i] != 0xff)
 			ecc_flag = 1;
 	}
@@ -363,7 +363,7 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
 	switch (info->ecc_scheme) {
 	case OMAP_ECC_BCH8_CODE_HW:
 		bch_type = BCH_8_BIT;
-		omap_reverse_list(calc_ecc, eccbytes - 1);
+		omap_reverse_list(calc_ecc, ecc->bytes - 1);
 		break;
 	default:
 		return -EINVAL;
@@ -372,14 +372,14 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
 	elm_config(bch_type);
 	if (elm_check_error(calc_ecc, bch_type, &error_count, error_loc)) {
 		printf("nand: error: uncorrectable ECC errors\n");
-		return -EINVAL;
+		return -EBADMSG;
 	}
 	/* correct bch error */
 	for (count = 0; count < error_count; count++) {
 		switch (info->ecc_scheme) {
 		case OMAP_ECC_BCH8_CODE_HW:
 			/* 14th byte in ECC is reserved to match ROM layout */
-			error_max = SECTOR_BYTES + (eccbytes - 1);
+			error_max = SECTOR_BYTES + (ecc->bytes - 1);
 			break;
 		default:
 			return -EINVAL;
-- 
1.8.5.1.163.gd7aced9

[U-Boot] [PATCH v1 1/6] mtd: nand: omap_elm: remove #include omap_gpmc.h

[Stefan Roese]

On 10.04.2014 11:35, Pekon Gupta wrote:
> There is no dependency of omap_elm.c on omap_gpmc.h
>
> Signed-off-by: Pekon Gupta <pekon@ti.com>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH v1 2/6] mtd: nand: omap_elm: use bch_type instead of nibble count to differentiate between BCH4/BCH8/BCH16

[Stefan Roese]

On 10.04.2014 11:35, Pekon Gupta wrote:
> ELM hardware engine support ECC error detection for multiple ECC strengths like
>   +------+------------------------+
>   |Type  | ECC syndrome length    |
>   +------+------------------------+
>   |BCH4  | 6.5 bytes = 13 nibbles |
>   |BCH8  | 13 byte = 26 nibbles   |
>   |BCH16 | 26 bytes = 52 nibbles  |
>   +------+------------------------+
>
> Current implementation of omap_elm driver uses ECC syndrom length (in 'nibbles')
> to differentiate between BCH4/BCH8/BCH16. This patch replaces it with 'bch_type'
>
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> ---
>   drivers/mtd/nand/omap_elm.c  | 20 ++++++++------------
>   drivers/mtd/nand/omap_gpmc.c | 10 ++--------
>   include/linux/mtd/omap_elm.h |  2 +-
>   3 files changed, 11 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/mtd/nand/omap_elm.c b/drivers/mtd/nand/omap_elm.c
> index 4c65f3b..5b2172a 100644
> --- a/drivers/mtd/nand/omap_elm.c
> +++ b/drivers/mtd/nand/omap_elm.c
> @@ -24,14 +24,12 @@
>   struct elm *elm_cfg;
>
>   /**
> - * elm_load_syndromes - Load BCH syndromes based on nibble selection
> + * elm_load_syndromes - Load BCH syndromes based on bch_type selection
>    * @syndrome: BCH syndrome
> - * @nibbles:
> + * @bch_type: BCH4/BCH8/BCH16
>    * @poly: Syndrome Polynomial set to use
> - *
> - * Load BCH syndromes based on nibble selection
>    */
> -static void elm_load_syndromes(u8 *syndrome, u32 nibbles, u8 poly)
> +static void elm_load_syndromes(u8 *syndrome, enum bch_level bch_type, u8 poly)
>   {
>   	u32 *ptr;
>   	u32 val;
> @@ -47,8 +45,7 @@ static void elm_load_syndromes(u8 *syndrome, u32 nibbles, u8 poly)
>   				(syndrome[7] << 24);
>   	writel(val, ptr);
>
> -	/* BCH 8-bit with 26 nibbles (4*8=32) */
> -	if (nibbles > 13) {
> +	if (bch_type == BCH_4_BIT || bch_type == BCH_8_BIT) {

Is this correct? Shouldn't this be:

+	if (bch_type == BCH_8_BIT || bch_type == BCH_16_BIT) {

instead?

Thanks,
Stefan

[U-Boot] [PATCH v1 3/6] mtd: nand: omap_elm: use macros for register definitions

[Stefan Roese]

On 10.04.2014 11:35, Pekon Gupta wrote:
> This patch adds macros for following parameters of ELM Hardware engine
>   - ELM_MAX_CHANNELS: ELM can process 8 data streams simultaneously
>   - ELM_MAX_ERRORS: ELM can detect upto 16 ECC error when using BCH16 scheme
>
> Signed-off-by: Pekon Gupta <pekon@ti.com>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH v1 4/6] mtd: nand: omap_gpmc: remove unused members of 'struct nand_bch_priv'

[Stefan Roese]

On 10.04.2014 11:35, Pekon Gupta wrote:
> This patch prepares to refactor 'struct nand_bch_priv' -> 'struct omap_nand_info'
> And thus performs following clean-ups:
>   - remove nand_bch_priv.type: use nand_bch_priv.ecc_scheme instead
>   - remove nand_bch_priv.mode: <unused>
>
> Signed-off-by: Pekon Gupta <pekon@ti.com>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH v1 5/6] mtd: nand: omap_gpmc: rename struct nand_bch_priv to struct omap_nand_info

[Stefan Roese]

On 10.04.2014 11:35, Pekon Gupta wrote:
> This patch renames 'struct nand_bch_priv' which currently holds private data only
> for BCH ECC schemes, into 'struct omap_nand_info' so that same can be used for
> all ECC schemes
>
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> ---
>   drivers/mtd/nand/omap_gpmc.c | 58 ++++++++++++++++++++------------------------
>   1 file changed, 26 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c
> index 4bd296e..ae541c2 100644
> --- a/drivers/mtd/nand/omap_gpmc.c
> +++ b/drivers/mtd/nand/omap_gpmc.c
> @@ -148,9 +148,9 @@ static int __maybe_unused omap_correct_data(struct mtd_info *mtd, uint8_t *dat,
>   }
>
>   /*
> - * Generic BCH interface
> + * Driver configurations
>    */
> -struct nand_bch_priv {
> +struct omap_nand_info {
>   	struct bch_control *control;
>   	enum omap_ecc ecc_scheme;
>   };
> @@ -161,7 +161,7 @@ struct nand_bch_priv {
>    * library).
>    * When some users with other BCH strength will exists this have to change!
>    */
> -static __maybe_unused struct nand_bch_priv bch_priv = {
> +static __maybe_unused struct omap_nand_info omap_nand_info = {
>   	.control = NULL
>   };
>
> @@ -191,7 +191,7 @@ __maybe_unused
>   static void omap_enable_hwecc(struct mtd_info *mtd, int32_t mode)
>   {
>   	struct nand_chip	*nand	= mtd->priv;
> -	struct nand_bch_priv	*bch	= nand->priv;
> +	struct omap_nand_info	*info = nand->priv;
>   	unsigned int dev_width = (nand->options & NAND_BUSWIDTH_16) ? 1 : 0;
>   	unsigned int ecc_algo = 0;
>   	unsigned int bch_type = 0;
> @@ -200,7 +200,7 @@ static void omap_enable_hwecc(struct mtd_info *mtd, int32_t mode)
>   	u32 ecc_config_val = 0;
>
>   	/* configure GPMC for specific ecc-scheme */
> -	switch (bch->ecc_scheme) {
> +	switch (info->ecc_scheme) {
>   	case OMAP_ECC_HAM1_CODE_SW:
>   		return;
>   	case OMAP_ECC_HAM1_CODE_HW:
> @@ -262,11 +262,11 @@ static int omap_calculate_ecc(struct mtd_info *mtd, const uint8_t *dat,
>   				uint8_t *ecc_code)
>   {
>   	struct nand_chip *chip = mtd->priv;
> -	struct nand_bch_priv *bch = chip->priv;
> +	struct omap_nand_info	*info = chip->priv;
>   	uint32_t *ptr, val = 0;
>   	int8_t i = 0, j;

Nitpicking:

This seems to be the only variable declaration with a tab for 
indentation. As the others are not aligned via tab's I suggest to use 
one space here as well. There are other places in this source where this 
indentation is inconsistent. Please re-check.

Other than this:

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH v1 2/6] mtd: nand: omap_elm: use bch_type instead of nibble count to differentiate between BCH4/BCH8/BCH16

[Gupta, Pekon]

Hi Stefan,

>From: Stefan Roese [mailto:sr at denx.de]
>>On 10.04.2014 11:35, Pekon Gupta wrote:
>> ELM hardware engine support ECC error detection for multiple ECC strengths like
>>   +------+------------------------+
>>   |Type  | ECC syndrome length    |
>>   +------+------------------------+
>>   |BCH4  | 6.5 bytes = 13 nibbles |
>>   |BCH8  | 13 byte = 26 nibbles   |
>>   |BCH16 | 26 bytes = 52 nibbles  |
>>   +------+------------------------+
>>
>> Current implementation of omap_elm driver uses ECC syndrom length (in 'nibbles')
>> to differentiate between BCH4/BCH8/BCH16. This patch replaces it with 'bch_type'
>>
>> Signed-off-by: Pekon Gupta <pekon@ti.com>
>> ---
>>   drivers/mtd/nand/omap_elm.c  | 20 ++++++++------------
>>   drivers/mtd/nand/omap_gpmc.c | 10 ++--------
>>   include/linux/mtd/omap_elm.h |  2 +-
>>   3 files changed, 11 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/omap_elm.c b/drivers/mtd/nand/omap_elm.c
>> index 4c65f3b..5b2172a 100644
>> --- a/drivers/mtd/nand/omap_elm.c
>> +++ b/drivers/mtd/nand/omap_elm.c
>> @@ -24,14 +24,12 @@
>>   struct elm *elm_cfg;
>>
>>   /**
>> - * elm_load_syndromes - Load BCH syndromes based on nibble selection
>> + * elm_load_syndromes - Load BCH syndromes based on bch_type selection
>>    * @syndrome: BCH syndrome
>> - * @nibbles:
>> + * @bch_type: BCH4/BCH8/BCH16
>>    * @poly: Syndrome Polynomial set to use
>> - *
>> - * Load BCH syndromes based on nibble selection
>>    */
>> -static void elm_load_syndromes(u8 *syndrome, u32 nibbles, u8 poly)
>> +static void elm_load_syndromes(u8 *syndrome, enum bch_level bch_type, u8 poly)
>>   {
>>   	u32 *ptr;
>>   	u32 val;
>> @@ -47,8 +45,7 @@ static void elm_load_syndromes(u8 *syndrome, u32 nibbles, u8 poly)
>>   				(syndrome[7] << 24);
>>   	writel(val, ptr);
>>
>> -	/* BCH 8-bit with 26 nibbles (4*8=32) */
>> -	if (nibbles > 13) {
>> +	if (bch_type == BCH_4_BIT || bch_type == BCH_8_BIT) {
>
>Is this correct? Shouldn't this be:
>
>+	if (bch_type == BCH_8_BIT || bch_type == BCH_16_BIT) {
>
>instead?
>
yes you are correct. Thanks for pointing this out.
Sorry, as BCH_4_BIT and BCH_16_BIT support is still not added to omap_gpmc
driver so this bug wasn't detected. I'll fix this in next version.


with regards, pekon

[U-Boot] [PATCH v1 6/6] mtd: nand: omap_gpmc: minor cleanup of omap_correct_data_bch

[Stefan Roese]

On 10.04.2014 11:35, Pekon Gupta wrote:
> This patch tries to avoid some local pointer dereferences, by using common
> local variables in omap_correct_data_bch()
>
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> ---
>   drivers/mtd/nand/omap_gpmc.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c
> index ae541c2..a96f310 100644
> --- a/drivers/mtd/nand/omap_gpmc.c
> +++ b/drivers/mtd/nand/omap_gpmc.c
> @@ -331,7 +331,7 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
>   {
>   	struct nand_chip *chip = mtd->priv;
>   	struct omap_nand_info	*info = chip->priv;
> -	uint32_t eccbytes = chip->ecc.bytes;
> +	struct nand_ecc_ctrl	*ecc = &chip->ecc;

Again, nitpicking because of inconsistent indentation.

>   	uint32_t error_count = 0, error_max;
>   	uint32_t error_loc[8];
>   	enum bch_level bch_type;
> @@ -340,7 +340,7 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
>   	uint32_t byte_pos, bit_pos;
>
>   	/* check calculated ecc */
> -	for (i = 0; i < chip->ecc.bytes && !ecc_flag; i++) {
> +	for (i = 0; i < ecc->bytes && !ecc_flag; i++) {
>   		if (calc_ecc[i] != 0x00)
>   			ecc_flag = 1;
>   	}
> @@ -349,7 +349,7 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
>
>   	/* check for whether its a erased-page */
>   	ecc_flag = 0;
> -	for (i = 0; i < chip->ecc.bytes && !ecc_flag; i++) {
> +	for (i = 0; i < ecc->bytes && !ecc_flag; i++) {
>   		if (read_ecc[i] != 0xff)
>   			ecc_flag = 1;
>   	}
> @@ -363,7 +363,7 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
>   	switch (info->ecc_scheme) {
>   	case OMAP_ECC_BCH8_CODE_HW:
>   		bch_type = BCH_8_BIT;
> -		omap_reverse_list(calc_ecc, eccbytes - 1);
> +		omap_reverse_list(calc_ecc, ecc->bytes - 1);
>   		break;
>   	default:
>   		return -EINVAL;
> @@ -372,14 +372,14 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
>   	elm_config(bch_type);
>   	if (elm_check_error(calc_ecc, bch_type, &error_count, error_loc)) {
>   		printf("nand: error: uncorrectable ECC errors\n");
> -		return -EINVAL;
> +		return -EBADMSG;

This change is not mentioned in the commit log. Is it intentional? Or 
should it be moved to a separate patch?

Thanks,
Stefan