RE: [RFC PATCH 02/10] mtd: nand: omap: Always use chip-ecc.steps for BCH sector count
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 rog...@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-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 02/10] mtd: nand: omap: Always use chip-ecc.steps for BCH sector count
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 rog...@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
RE: [RFC PATCH 02/10] mtd: nand: omap: Always use chip-ecc.steps for BCH sector count
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-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 02/10] mtd: nand: omap: Always use chip-ecc.steps for BCH sector count
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 majord...@vger.kernel.org
More
