[U-Boot] [PATCH v1 0/6] mtd: nand: omap: clean up of omap_elm and omap_gpmc driver
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(-)
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>
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
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);
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
Hi Stefan,
From: Stefan Roese [mailto:sr@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
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,
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
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;
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
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 */
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
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;
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);@@ -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
participants (3)
-
Gupta, Pekon -
Pekon Gupta
-
Stefan Roese