[RFC,02/10] mtd: nand: omap: Always use chip->ecc.steps for BCH sector count

Message ID 1404909450-11970-3-git-send-email-rogerq@ti.com
State New
Headers show

Commit Message

Roger Quadros July 9, 2014, 12:37 p.m.
Instead of hardcoding use the pre-calculated chip->ecc.steps for
configuring number of sectors to process with the BCH algorithm.

This also avoids unnecessary access to the ECC_CONFIG register in
omap_calculate_ecc_bch().

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/mtd/nand/omap2.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

Comments

Pekon Gupta July 11, 2014, 7:43 a.m. | #1
>From: Quadros, Roger
>
>Instead of hardcoding use the pre-calculated chip->ecc.steps for
>configuring number of sectors to process with the BCH algorithm.
>
>This also avoids unnecessary access to the ECC_CONFIG register in
>omap_calculate_ecc_bch().
>
>Signed-off-by: Roger Quadros <rogerq@ti.com>
>---
> drivers/mtd/nand/omap2.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
>index 5b8739c..6f3d7cd 100644
>--- a/drivers/mtd/nand/omap2.c
>+++ b/drivers/mtd/nand/omap2.c
>@@ -1066,10 +1066,10 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>*mtd, int mode)
> 	unsigned int ecc_size1, ecc_size0;
>
> 	/* GPMC configurations for calculating ECC */
>+	nsectors = chip->ecc.steps;
> 	switch (ecc_opt) {
> 	case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
> 		bch_type = 0;
>-		nsectors = 1;
> 		if (mode == NAND_ECC_READ) {
> 			wr_mode	  = BCH_WRAPMODE_6;
> 			ecc_size0 = BCH_ECC_SIZE0;
>@@ -1082,7 +1082,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>*mtd, int mode)
> 		break;
> 	case OMAP_ECC_BCH4_CODE_HW:
> 		bch_type = 0;
>-		nsectors = chip->ecc.steps;
> 		if (mode == NAND_ECC_READ) {
> 			wr_mode	  = BCH_WRAPMODE_1;
> 			ecc_size0 = BCH4R_ECC_SIZE0;
>@@ -1095,7 +1094,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>*mtd, int mode)
> 		break;
> 	case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
> 		bch_type = 1;
>-		nsectors = 1;
> 		if (mode == NAND_ECC_READ) {
> 			wr_mode	  = BCH_WRAPMODE_6;
> 			ecc_size0 = BCH_ECC_SIZE0;
>@@ -1108,7 +1106,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>*mtd, int mode)
> 		break;
> 	case OMAP_ECC_BCH8_CODE_HW:
> 		bch_type = 1;
>-		nsectors = chip->ecc.steps;
> 		if (mode == NAND_ECC_READ) {
> 			wr_mode	  = BCH_WRAPMODE_1;
> 			ecc_size0 = BCH8R_ECC_SIZE0;
>@@ -1121,7 +1118,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>*mtd, int mode)
> 		break;
> 	case OMAP_ECC_BCH16_CODE_HW:
> 		bch_type = 0x2;
>-		nsectors = chip->ecc.steps;
> 		if (mode == NAND_ECC_READ) {
> 			wr_mode	  = 0x01;
> 			ecc_size0 = 52; /* ECC bits in nibbles per sector */
>@@ -1176,6 +1172,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
> {
> 	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
> 						   mtd);
>+	struct nand_chip *chip = mtd->priv;
> 	int eccbytes	= info->nand.ecc.bytes;
> 	struct gpmc_nand_regs	*gpmc_regs = &info->reg;
> 	u8 *ecc_code;
>@@ -1183,7 +1180,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
> 	u32 val;
> 	int i, j;
>
>-	nsectors = ((readl(info->reg.gpmc_ecc_config) >> 4) & 0x7) + 1;
>+	nsectors = chip->ecc.steps;

Sorry NAK.. I'm sure you are breaking something here :-)

NAND driver supports multiple ECC schemes like;
OMAP_ECC_CODE_HAM1_HW (support for legacy reasons)
OMAP_ECC_CODE_BCH4_HW_DETECTION_SW (needed for OMAP3 and AM35xx)
OMAP_ECC_CODE_BCH4_HW
OMAP_ECC_CODE_BCH8_HW
OMAP_ECC_CODE_BCH8_HW_DETECTION_SW  (needed for OMAP3 and AM35xx)
OMAP_ECC_CODE_BCH16_HW

IIRC ..
- software based ecc-schemes OMAP_ECC_CODE_BCHx_HW_DETECTION_SW
  Reads/Write in per-sector granularity. (here nsector != chip->ecc.steps)

- hardware based ecc-schemes OMAP_ECC_CODE_BCHx_HW, perform
  Reads/Write in per-page granularity (or something like this).
  Therefore you have custom implementation of 
  chip->ecc.read_page = omap_read_page_bch()
 Also if you change the configurations here, it will break the compatibility with
 u-boot, so images flashed via u-boot will stop to boot in kernel and vice-versa.

I suggest, please refrain from these tweaks for now..
All these optimizations have been tested on multiple scenario so please test
all ecc-schemes before doing anything, otherwise you will end-up in
a bad loop of breaking and fixing NAND driver :-).


with regards, pekon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Roger Quadros July 11, 2014, 10:35 a.m. | #2
On 07/11/2014 10:43 AM, Gupta, Pekon wrote:
>> From: Quadros, Roger
>>
>> Instead of hardcoding use the pre-calculated chip->ecc.steps for
>> configuring number of sectors to process with the BCH algorithm.
>>
>> This also avoids unnecessary access to the ECC_CONFIG register in
>> omap_calculate_ecc_bch().
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>> drivers/mtd/nand/omap2.c | 9 +++------
>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
>> index 5b8739c..6f3d7cd 100644
>> --- a/drivers/mtd/nand/omap2.c
>> +++ b/drivers/mtd/nand/omap2.c
>> @@ -1066,10 +1066,10 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>> *mtd, int mode)
>> 	unsigned int ecc_size1, ecc_size0;
>>
>> 	/* GPMC configurations for calculating ECC */
>> +	nsectors = chip->ecc.steps;
>> 	switch (ecc_opt) {
>> 	case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
>> 		bch_type = 0;
>> -		nsectors = 1;
>> 		if (mode == NAND_ECC_READ) {
>> 			wr_mode	  = BCH_WRAPMODE_6;
>> 			ecc_size0 = BCH_ECC_SIZE0;
>> @@ -1082,7 +1082,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>> *mtd, int mode)
>> 		break;
>> 	case OMAP_ECC_BCH4_CODE_HW:
>> 		bch_type = 0;
>> -		nsectors = chip->ecc.steps;
>> 		if (mode == NAND_ECC_READ) {
>> 			wr_mode	  = BCH_WRAPMODE_1;
>> 			ecc_size0 = BCH4R_ECC_SIZE0;
>> @@ -1095,7 +1094,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>> *mtd, int mode)
>> 		break;
>> 	case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
>> 		bch_type = 1;
>> -		nsectors = 1;
>> 		if (mode == NAND_ECC_READ) {
>> 			wr_mode	  = BCH_WRAPMODE_6;
>> 			ecc_size0 = BCH_ECC_SIZE0;
>> @@ -1108,7 +1106,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>> *mtd, int mode)
>> 		break;
>> 	case OMAP_ECC_BCH8_CODE_HW:
>> 		bch_type = 1;
>> -		nsectors = chip->ecc.steps;
>> 		if (mode == NAND_ECC_READ) {
>> 			wr_mode	  = BCH_WRAPMODE_1;
>> 			ecc_size0 = BCH8R_ECC_SIZE0;
>> @@ -1121,7 +1118,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>> *mtd, int mode)
>> 		break;
>> 	case OMAP_ECC_BCH16_CODE_HW:
>> 		bch_type = 0x2;
>> -		nsectors = chip->ecc.steps;
>> 		if (mode == NAND_ECC_READ) {
>> 			wr_mode	  = 0x01;
>> 			ecc_size0 = 52; /* ECC bits in nibbles per sector */
>> @@ -1176,6 +1172,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
>> {
>> 	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
>> 						   mtd);
>> +	struct nand_chip *chip = mtd->priv;
>> 	int eccbytes	= info->nand.ecc.bytes;
>> 	struct gpmc_nand_regs	*gpmc_regs = &info->reg;
>> 	u8 *ecc_code;
>> @@ -1183,7 +1180,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
>> 	u32 val;
>> 	int i, j;
>>
>> -	nsectors = ((readl(info->reg.gpmc_ecc_config) >> 4) & 0x7) + 1;
>> +	nsectors = chip->ecc.steps;
> 
> Sorry NAK.. I'm sure you are breaking something here :-)
> 
> NAND driver supports multiple ECC schemes like;
> OMAP_ECC_CODE_HAM1_HW (support for legacy reasons)
> OMAP_ECC_CODE_BCH4_HW_DETECTION_SW (needed for OMAP3 and AM35xx)
> OMAP_ECC_CODE_BCH4_HW
> OMAP_ECC_CODE_BCH8_HW
> OMAP_ECC_CODE_BCH8_HW_DETECTION_SW  (needed for OMAP3 and AM35xx)
> OMAP_ECC_CODE_BCH16_HW
> 
> IIRC ..
> - software based ecc-schemes OMAP_ECC_CODE_BCHx_HW_DETECTION_SW
>   Reads/Write in per-sector granularity. (here nsector != chip->ecc.steps)

OK. I still don't have a full understanding about the ECC schemes so to ensure we
don't break anything I can just leave nsectors as it is and probably just store a
copy of it in omap_nand_info to avoid reading it back from gpmc_ecc_config.

I still have a few questions to clarify my understanding.

The only difference between OMAP_ECC_CODE_BCHx_HW_DETECTION_SW and
OMAP_ECC_CODE_BCHx_HW is that in the former the _correction_ is done by software
and in the latter the _correction_ is done by hardware (i.e. ELM module).
In both cases the _detection_ is done by the same hardware IP via ecc.calculate(),
i.e. omap_calculate_ecc_bch().

so why should nsector be different for both in the _detection_ stage?

An I right that ecc_steps is nothing but number of sub-blocks ECC calculation and correction
needs to be done for larger pages. This is a function of ECC hw capability (chip->ecc.size)
and NAND flash capability (mtd->writesize). i.e. ecc_steps = mtd->writesize / chip->ecc.size

We hardcode chip->ecc.size to 512 for all the ECC schemes in omap_nand_probe() so
calculate and correct will always be called for 512 byte sized blocks. So when does
the usecase for nsector > 1 come in?

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Pekon Gupta July 11, 2014, 11:27 a.m. | #3
>From: Quadros, Roger
>>On 07/11/2014 10:43 AM, Gupta, Pekon wrote:
>>> From: Quadros, Roger
[...]

>>> @@ -1176,6 +1172,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info
>*mtd,
>>> {
>>> 	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
>>> 						   mtd);
>>> +	struct nand_chip *chip = mtd->priv;
>>> 	int eccbytes	= info->nand.ecc.bytes;
>>> 	struct gpmc_nand_regs	*gpmc_regs = &info->reg;
>>> 	u8 *ecc_code;
>>> @@ -1183,7 +1180,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info
>*mtd,
>>> 	u32 val;
>>> 	int i, j;
>>>
>>> -	nsectors = ((readl(info->reg.gpmc_ecc_config) >> 4) & 0x7) + 1;
>>> +	nsectors = chip->ecc.steps;
>>
>> Sorry NAK.. I'm sure you are breaking something here :-)
>>
>> NAND driver supports multiple ECC schemes like;
>> OMAP_ECC_CODE_HAM1_HW (support for legacy reasons)
>> OMAP_ECC_CODE_BCH4_HW_DETECTION_SW (needed for OMAP3 and AM35xx)
>> OMAP_ECC_CODE_BCH4_HW
>> OMAP_ECC_CODE_BCH8_HW
>> OMAP_ECC_CODE_BCH8_HW_DETECTION_SW  (needed for OMAP3 and AM35xx)
>> OMAP_ECC_CODE_BCH16_HW
>>
>> IIRC ..
>> - software based ecc-schemes OMAP_ECC_CODE_BCHx_HW_DETECTION_SW
>>   Reads/Write in per-sector granularity. (here nsector != chip->ecc.steps)
>
>OK. I still don't have a full understanding about the ECC schemes so to ensure we
>don't break anything I can just leave nsectors as it is and probably just store a
>copy of it in omap_nand_info to avoid reading it back from gpmc_ecc_config.
>
>I still have a few questions to clarify my understanding.
>
>The only difference between OMAP_ECC_CODE_BCHx_HW_DETECTION_SW and
>OMAP_ECC_CODE_BCHx_HW is that in the former the _correction_ is done by software
>and in the latter the _correction_ is done by hardware (i.e. ELM module).
>In both cases the _detection_ is done by the same hardware IP via ecc.calculate(),
>i.e. omap_calculate_ecc_bch().
>
>so why should nsector be different for both in the _detection_ stage?
>
Again IIRC, That is because of ELM driver.
ELM hardware engine has 8 channels with each of them having 512Bytes capacity.
Now, there can be two approaches:
(1) SECTOR_MODE:  Process only one sector of 512 bytes at a time, and iterate
      chip->ecc.steps times.
(2) PAGE_MODE: Process complete page at a time, enabling multiple channels
    simultaneously. This improves some throughput, especially for large-page
   NAND devices and MLC NAND where bit-flips are common.

As ELM uses (2)nd approach, so the GPMC also has to calculate and store
ECC for complete page at a time. That is why trace NAND driver you will find
-  OMAP_ECC_CODE_BCHx_HW_DETECTION_SW use generic implementation
         chip->ecc.read_page= nand_read_page_hwecc() defined in nand_base.c
whereas,
-  OMAP_ECC_CODE_BCHx_HW use custom implementation
         chip->ecc.read_page= omap_read_page_bch() defined in omap.c


>An I right that ecc_steps is nothing but number of sub-blocks ECC calculation and correction
>needs to be done for larger pages. This is a function of ECC hw capability (chip->ecc.size)
>and NAND flash capability (mtd->writesize). i.e. ecc_steps = mtd->writesize / chip->ecc.size
>
>We hardcode chip->ecc.size to 512 for all the ECC schemes in omap_nand_probe() so
>calculate and correct will always be called for 512 byte sized blocks. So when does
>the usecase for nsector > 1 come in?
>
Ok.. I'll try to explain above details again in probably simplified version
- OMAP_ECC_CODE_BCHx_HW_DETECTION_SW
  uses lib/bch.c (via nand_bch.c) to correct ECC. And is generic implementation
  so here each sector  (data chunk of ecc_size) is corrected independently.
  So nsector = 1;
  
- OMAP_ECC_CODE_BCHx_HW
  Uses ELM to correct ECC. But the way ELM driver is written today. It corrects
  the whole page in single go. Not individual sectors (ecc_size chunks).
  So, its doable to make it same like OMAP_ECC_CODE_BCHx_HW_DETECTION_SW
  But then you _may_ have performance penalty for new technology NAND and MLC
  NAND on which bit-flips are very common.
  So to keep ELM driver as it is (performance tweaked), we use different
  configurations in GPMC to read complete page in single go. This is where
   nsector = chip->ecc.steps;

Hope that clarifies the implementation..

Good to document this detail somewhere for OMAP drivers both (u-boot and kernel).
Any thoughts ?

with regards, pekon


>cheers,
>-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Roger Quadros July 11, 2014, 11:51 a.m. | #4
On 07/11/2014 02:27 PM, Gupta, Pekon wrote:
>> From: Quadros, Roger
>>> On 07/11/2014 10:43 AM, Gupta, Pekon wrote:
>>>> From: Quadros, Roger
> [...]
> 
>>>> @@ -1176,6 +1172,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info
>> *mtd,
>>>> {
>>>> 	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
>>>> 						   mtd);
>>>> +	struct nand_chip *chip = mtd->priv;
>>>> 	int eccbytes	= info->nand.ecc.bytes;
>>>> 	struct gpmc_nand_regs	*gpmc_regs = &info->reg;
>>>> 	u8 *ecc_code;
>>>> @@ -1183,7 +1180,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info
>> *mtd,
>>>> 	u32 val;
>>>> 	int i, j;
>>>>
>>>> -	nsectors = ((readl(info->reg.gpmc_ecc_config) >> 4) & 0x7) + 1;
>>>> +	nsectors = chip->ecc.steps;
>>>
>>> Sorry NAK.. I'm sure you are breaking something here :-)
>>>
>>> NAND driver supports multiple ECC schemes like;
>>> OMAP_ECC_CODE_HAM1_HW (support for legacy reasons)
>>> OMAP_ECC_CODE_BCH4_HW_DETECTION_SW (needed for OMAP3 and AM35xx)
>>> OMAP_ECC_CODE_BCH4_HW
>>> OMAP_ECC_CODE_BCH8_HW
>>> OMAP_ECC_CODE_BCH8_HW_DETECTION_SW  (needed for OMAP3 and AM35xx)
>>> OMAP_ECC_CODE_BCH16_HW
>>>
>>> IIRC ..
>>> - software based ecc-schemes OMAP_ECC_CODE_BCHx_HW_DETECTION_SW
>>>   Reads/Write in per-sector granularity. (here nsector != chip->ecc.steps)
>>
>> OK. I still don't have a full understanding about the ECC schemes so to ensure we
>> don't break anything I can just leave nsectors as it is and probably just store a
>> copy of it in omap_nand_info to avoid reading it back from gpmc_ecc_config.
>>
>> I still have a few questions to clarify my understanding.
>>
>> The only difference between OMAP_ECC_CODE_BCHx_HW_DETECTION_SW and
>> OMAP_ECC_CODE_BCHx_HW is that in the former the _correction_ is done by software
>> and in the latter the _correction_ is done by hardware (i.e. ELM module).
>> In both cases the _detection_ is done by the same hardware IP via ecc.calculate(),
>> i.e. omap_calculate_ecc_bch().
>>
>> so why should nsector be different for both in the _detection_ stage?
>>
> Again IIRC, That is because of ELM driver.
> ELM hardware engine has 8 channels with each of them having 512Bytes capacity.
> Now, there can be two approaches:
> (1) SECTOR_MODE:  Process only one sector of 512 bytes at a time, and iterate
>       chip->ecc.steps times.
> (2) PAGE_MODE: Process complete page at a time, enabling multiple channels
>     simultaneously. This improves some throughput, especially for large-page
>    NAND devices and MLC NAND where bit-flips are common.
> 
> As ELM uses (2)nd approach, so the GPMC also has to calculate and store
> ECC for complete page at a time. That is why trace NAND driver you will find
> -  OMAP_ECC_CODE_BCHx_HW_DETECTION_SW use generic implementation
>          chip->ecc.read_page= nand_read_page_hwecc() defined in nand_base.c
> whereas,
> -  OMAP_ECC_CODE_BCHx_HW use custom implementation
>          chip->ecc.read_page= omap_read_page_bch() defined in omap.c
> 
> 
>> An I right that ecc_steps is nothing but number of sub-blocks ECC calculation and correction
>> needs to be done for larger pages. This is a function of ECC hw capability (chip->ecc.size)
>> and NAND flash capability (mtd->writesize). i.e. ecc_steps = mtd->writesize / chip->ecc.size
>>
>> We hardcode chip->ecc.size to 512 for all the ECC schemes in omap_nand_probe() so
>> calculate and correct will always be called for 512 byte sized blocks. So when does
>> the usecase for nsector > 1 come in?
>>
> Ok.. I'll try to explain above details again in probably simplified version
> - OMAP_ECC_CODE_BCHx_HW_DETECTION_SW
>   uses lib/bch.c (via nand_bch.c) to correct ECC. And is generic implementation
>   so here each sector  (data chunk of ecc_size) is corrected independently.
>   So nsector = 1;
>   
> - OMAP_ECC_CODE_BCHx_HW
>   Uses ELM to correct ECC. But the way ELM driver is written today. It corrects
>   the whole page in single go. Not individual sectors (ecc_size chunks).

Then shouldn't chip->ecc.size be equal page size and chip->ecc.steps be equal to 1 for upto 4KB pages?
For larger pages it can be a multiple of 4KB page size. i.e. 2 for 8KB, 4 for 16KB and so on.

So nsectors is not necessarily equal to ecc.steps but equal to how many 512 byte blocks are there in
one step. i.e. [min(4096, page_size) / 512]. And it must be local to omap NAND driver.


>   So, its doable to make it same like OMAP_ECC_CODE_BCHx_HW_DETECTION_SW
>   But then you _may_ have performance penalty for new technology NAND and MLC
>   NAND on which bit-flips are very common.
>   So to keep ELM driver as it is (performance tweaked), we use different
>   configurations in GPMC to read complete page in single go. This is where
>    nsector = chip->ecc.steps;
> 
> Hope that clarifies the implementation..
> 
> Good to document this detail somewhere for OMAP drivers both (u-boot and kernel).
> Any thoughts ?

Sure. we have the processors wiki. That should be a good place.

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch hide | download patch | download mbox

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 5b8739c..6f3d7cd 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1066,10 +1066,10 @@  static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, int mode)
 	unsigned int ecc_size1, ecc_size0;
 
 	/* GPMC configurations for calculating ECC */
+	nsectors = chip->ecc.steps;
 	switch (ecc_opt) {
 	case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
 		bch_type = 0;
-		nsectors = 1;
 		if (mode == NAND_ECC_READ) {
 			wr_mode	  = BCH_WRAPMODE_6;
 			ecc_size0 = BCH_ECC_SIZE0;
@@ -1082,7 +1082,6 @@  static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, int mode)
 		break;
 	case OMAP_ECC_BCH4_CODE_HW:
 		bch_type = 0;
-		nsectors = chip->ecc.steps;
 		if (mode == NAND_ECC_READ) {
 			wr_mode	  = BCH_WRAPMODE_1;
 			ecc_size0 = BCH4R_ECC_SIZE0;
@@ -1095,7 +1094,6 @@  static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, int mode)
 		break;
 	case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
 		bch_type = 1;
-		nsectors = 1;
 		if (mode == NAND_ECC_READ) {
 			wr_mode	  = BCH_WRAPMODE_6;
 			ecc_size0 = BCH_ECC_SIZE0;
@@ -1108,7 +1106,6 @@  static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, int mode)
 		break;
 	case OMAP_ECC_BCH8_CODE_HW:
 		bch_type = 1;
-		nsectors = chip->ecc.steps;
 		if (mode == NAND_ECC_READ) {
 			wr_mode	  = BCH_WRAPMODE_1;
 			ecc_size0 = BCH8R_ECC_SIZE0;
@@ -1121,7 +1118,6 @@  static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, int mode)
 		break;
 	case OMAP_ECC_BCH16_CODE_HW:
 		bch_type = 0x2;
-		nsectors = chip->ecc.steps;
 		if (mode == NAND_ECC_READ) {
 			wr_mode	  = 0x01;
 			ecc_size0 = 52; /* ECC bits in nibbles per sector */
@@ -1176,6 +1172,7 @@  static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
 {
 	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
 						   mtd);
+	struct nand_chip *chip = mtd->priv;
 	int eccbytes	= info->nand.ecc.bytes;
 	struct gpmc_nand_regs	*gpmc_regs = &info->reg;
 	u8 *ecc_code;
@@ -1183,7 +1180,7 @@  static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
 	u32 val;
 	int i, j;
 
-	nsectors = ((readl(info->reg.gpmc_ecc_config) >> 4) & 0x7) + 1;
+	nsectors = chip->ecc.steps;
 	for (i = 0; i < nsectors; i++) {
 		ecc_code = ecc_calc;
 		switch (info->ecc_opt) {