Fix OOB_READ bug where hardware FIFO is not drained completely. Fix potential bug for non-ECC operations.
Subsystems (2)
PATCH 1/1 Fix OOB_READ bug where hardware FIFO is not drained completely. Fix potential bug for non-ECC operations.
--- drivers/mtd/nand/pxa3xx_nand.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/mtd/nand/pxa3xx_nand.c +7 -2
@@ -677,7 +677,7 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)static void drain_fifo (struct pxa3xx_nand_info *info,void *data,int len) { -if (info->ecc_bch) { +if (info->use_ecc && info->ecc_bch) {u32 val;int ret;
@@ -1023,7 +1023,12 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,if (info->cur_chunk < info->nfullchunks) { info->step_chunk_size = info->chunk_size; - info->step_spare_size = info->spare_size; +if (info->use_ecc) { + info->step_spare_size = info->spare_size; + }else { + info->step_spare_size = info->spare_size + + info->ecc_size; + } }else { info->step_chunk_size = info->last_chunk_size; info->step_spare_size = info->last_spare_size;
Signature · 1 line
2.9.5
Hi Greg
Quoted 1 line
Signed-off-by Greg Cook <[email protected]>
Quoted 21 lines
--- drivers/mtd/nand/pxa3xx_nand.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)drivers/mtd/nand/pxa3xx_nand.c +1
-1@@ -677,7 +677,7 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)static void drain_fifo (struct pxa3xx_nand_info *info,void *data,int len) { -if (info->ecc_bch) { +if (info->use_ecc && info->ecc_bch) {u32 val;int ret;@@ -1023,7 +1023,12 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,if (info->cur_chunk < info->nfullchunks) { info->step_chunk_size = info->chunk_size;
drivers/mtd/nand/pxa3xx_nand.c static int prepare_set_command(struct pxa3xx_nand_info *info, int command, · line 1034
Quoted 10 lines
drivers/mtd/nand/pxa3xx_nand.c +6
-1- info->step_spare_size = info->spare_size; +if (info->use_ecc) { + info->step_spare_size = info->spare_size; + }else { + info->step_spare_size = info->spare_size + + info->ecc_size; + } }else { info->step_chunk_size = info->last_chunk_size; info->step_spare_size = info->last_spare_size;
Please change the subject to something shorter like:
mtd: nand: pxa3xx: fix READOOB when hardware FIFO is not drained completely Fix potential bug for non-ECC operations.
/Sean
Hi Greg,
Could you at least Cc the NAND maintainer when you submit something related to NAND!
On Tue, 12 Dec 2017 16:38:26 +0800
Greg Cook <[email protected]> wrote:
Please add a commit message explaining what your fixing.
Quoted 19 lines
Signed-off-by Greg Cook <[email protected]>--- drivers/mtd/nand/pxa3xx_nand.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)drivers/mtd/nand/pxa3xx_nand.c +1
-1@@ -677,7 +677,7 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)static void drain_fifo (struct pxa3xx_nand_info *info,void *data,int len) { -if (info->ecc_bch) { +if (info->use_ecc && info->ecc_bch) {u32 val;int ret;
drivers/mtd/nand/pxa3xx_nand.c static int prepare_set_command(struct pxa3xx_nand_info *info, int command, · line 1031
Quoted 10 lines
drivers/mtd/nand/pxa3xx_nand.c +6
-1@@ -1023,7 +1023,12 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,if (info->cur_chunk < info->nfullchunks) { info->step_chunk_size = info->chunk_size; - info->step_spare_size = info->spare_size; +if (info->use_ecc) { + info->step_spare_size = info->spare_size; + }else { + info->step_spare_size = info->spare_size + + info->ecc_size; + }
Curly braces are unneeded, and I'd prefer to have the + operator at the end of the line. Or it could be written like that:
info->step_spare_size = info->spare_size; if (!info->use_ecc) info->step_spare_size += info->ecc_size;
Anyway, I'm still not convince this is the appropriate fix. As mentioned in my previous, I wonder why we're not activating ECC when reading OOB bytes. There's a good reason for having 2 different hooks to read/write OOBs: one is doing it in raw mode (with the ECC engine disabled), the other one is doing it with the ECC engine enabled. This driver not only omit raw accessors (which is already a bad news for everyone that needs to debug the driver) but it also do things differently for OOB and page access: OOB bytes are read in raw mode while pages are always accessed with the ECC engine enabled.
OOB accesses are broken and should be fixed, I think we all agree on that, but we should fix it correctly, don't you think? And by correctly, I mean we should try to make things consistent at least (read OOB bytes with the ECC engine enabled).
drivers/mtd/nand/pxa3xx_nand.c static int prepare_set_command(struct pxa3xx_nand_info *info, int command, · line 1034
Quoted 3 lines
drivers/mtd/nand/pxa3xx_nand.c +0
-0}else { info->step_chunk_size = info->last_chunk_size; info->step_spare_size = info->last_spare_size;
Can you try with the below patch and see if it fixes the timeout issue?
--->8---
drivers/mtd/nand/pxa3xx_nand.c +1 -0
@@ -961,6 +961,7 @@ static void prepare_start_command(struct pxa3xx_nand_info *info, int command)switch (command) {case NAND_CMD_READ0 : +case NAND_CMD_READOOB :case NAND_CMD_PAGEPROG : info->use_ecc =1 ;break ;
Hi Boris
Quoted 72 lines
Hi Greg,
Could you at least Cc the NAND maintainer when you submit something related to NAND!
On Tue, 12 Dec 2017 16:38:26 +0800
Greg Cook <[email protected]> wrote:Please add a commit message explaining what your fixing.
Quoted 19 lines
Signed-off-by Greg Cook <[email protected]>--- drivers/mtd/nand/pxa3xx_nand.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)drivers/mtd/nand/pxa3xx_nand.c +1
-1@@ -677,7 +677,7 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)static void drain_fifo (struct pxa3xx_nand_info *info,void *data,int len) { -if (info->ecc_bch) { +if (info->use_ecc && info->ecc_bch) {u32 val;int ret;drivers/mtd/nand/pxa3xx_nand.c static int prepare_set_command(struct pxa3xx_nand_info *info, int command, · line 1031
Quoted 10 lines
drivers/mtd/nand/pxa3xx_nand.c +6
-1@@ -1023,7 +1023,12 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,if (info->cur_chunk < info->nfullchunks) { info->step_chunk_size = info->chunk_size; - info->step_spare_size = info->spare_size; +if (info->use_ecc) { + info->step_spare_size = info->spare_size; + }else { + info->step_spare_size = info->spare_size + + info->ecc_size; + }Curly braces are unneeded, and I'd prefer to have the + operator at the end of the line. Or it could be written like that:
info->step_spare_size = info->spare_size; if (!info->use_ecc) info->step_spare_size += info->ecc_size;Anyway, I'm still not convince this is the appropriate fix. As mentioned in my previous, I wonder why we're not activating ECC when reading OOB bytes. There's a good reason for having 2 different hooks to read/write OOBs: one is doing it in raw mode (with the ECC engine disabled), the other one is doing it with the ECC engine enabled. This driver not only omit raw accessors (which is already a bad news for everyone that needs to debug the driver) but it also do things differently for OOB and page access: OOB bytes are read in raw mode while pages are always accessed with the ECC engine enabled.
OOB accesses are broken and should be fixed, I think we all agree on that, but we should fix it correctly, don't you think? And by correctly, I mean we should try to make things consistent at least (read OOB bytes with the ECC engine enabled).
drivers/mtd/nand/pxa3xx_nand.c static int prepare_set_command(struct pxa3xx_nand_info *info, int command, · line 1034
Quoted 3 lines
drivers/mtd/nand/pxa3xx_nand.c +0
-0}else { info->step_chunk_size = info->last_chunk_size; info->step_spare_size = info->last_spare_size;Can you try with the below patch and see if it fixes the timeout issue?
--->8---
drivers/mtd/nand/pxa3xx_nand.c +0
-0
drivers/mtd/nand/pxa3xx_nand.c static void prepare_start_command(struct pxa3xx_nand_info *info, int command) · line 967
Quoted 7 lines
drivers/mtd/nand/pxa3xx_nand.c +1
-0@@ -961,6 +961,7 @@ static void prepare_start_command(struct pxa3xx_nand_info *info, int command)switch (command) {case NAND_CMD_READ0 : +case NAND_CMD_READOOB :case NAND_CMD_PAGEPROG : info->use_ecc =1 ;break ;
This fixes the timeout issue :-) Also without bbt...
root@output-module:~# nanddump -oa /dev/mtd1 ECC failed: 0 ECC corrected: 0 Number of bad blocks: 0 Number of bbt blocks: 8 Block size 131072, page size 2048, OOB size 64 Dumping data starting at 0x00000000 and ending at 0x0ff00000... UBIXSUBI
/Sean
Hi Boris
Quoted 18 lines
Quoted 11 lines
Can you try with the below patch and see if it fixes the timeout issue?
--->8---
drivers/mtd/nand/pxa3xx_nand.c +0
-0@@ -961,6 +961,7 @@ static void prepare_start_command(struct pxa3xx_nand_info *info, int command) switch (command) {Quoted 5 lines
case
NAND_CMD_READ0:
+ caseNAND_CMD_READOOB:
caseNAND_CMD_PAGEPROG:
info->use_ecc= 1;
break;This fixes the timeout issue :-) Also without bbt...
Quoted 9 lines
root@output-module:~# nanddump -oa /dev/mtd1 ECC failed: 0 ECC corrected: 0 Number of bad blocks: 0 Number of bbt blocks: 8 Block size 131072, page size 2048, OOB size 64 Dumping data starting at 0x00000000 and ending at 0x0ff00000... UBIXSUBI
Is this considered a valid patch for our 4.14 LTS kernel, pxa3xx driver? Or is it only masking some other problems?
/Sean
Quoted 4 lines · Boris Brezillon
Hi Greg,
Could you at least Cc the NAND maintainer when you submit something related to NAND!
It's my first time submitting a patch and I was following Sean's instructions. I see now the recommendation is to use './scripts/get_maintainer.pl --norolestats 000*', which would have CC'd in a LOT more people.
Quoted 6 lines
On Tue, 12 Dec 2017 16:38:26 +0800
Greg Cook <[email protected]> wrote:Please add a commit message explaining what your fixing.
I have just resent it with a better subject line and a description. No doubt this is also wrong ;)
Quoted 20 lines
Quoted 20 lines
Signed-off-by Greg Cook <[email protected]>--- drivers/mtd/nand/pxa3xx_nand.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)drivers/mtd/nand/pxa3xx_nand.c +1
-1@@ -677,7 +677,7 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)static void drain_fifo (struct pxa3xx_nand_info *info,void *data,int len) { -if (info->ecc_bch) { +if (info->use_ecc && info->ecc_bch) {u32 val;int ret;@@ -1023,7 +1023,12 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
Quoted 11 lines
drivers/mtd/nand/pxa3xx_nand.c static int prepare_set_command(struct pxa3xx_nand_info *info, int command, · line 1031
Quoted 9 lines
drivers/mtd/nand/pxa3xx_nand.c +6
-1if (info->cur_chunk < info->nfullchunks) { info->step_chunk_size = info->chunk_size; - info->step_spare_size = info->spare_size; +if (info->use_ecc) { + info->step_spare_size = info->spare_size; + }else { + info->step_spare_size = info->spare_size + + info->ecc_size; + }Curly braces are unneeded, ...
I disagree strongly because it costs a couple of keystrokes to add them and I think it improves readability. *But* I see it is in the Kernel coding style guidelines, so I grudgingly accept. I'm not going to promise it won't happen again though...
Quoted 6 lines
...and I'd prefer to have the + operator at the end of the line. Or it could be written like that:
info->step_spare_size = info->spare_size; if (!info->use_ecc) info->step_spare_size += info->ecc_size;
Agreed. This is much nicer.
Quoted 11 lines
Anyway, I'm still not convince this is the appropriate fix. As mentioned in my previous, I wonder why we're not activating ECC when reading OOB bytes. There's a good reason for having 2 different hooks to read/write OOBs: one is doing it in raw mode (with the ECC engine disabled), the other one is doing it with the ECC engine enabled. This driver not only omit raw accessors (which is already a bad news for everyone that needs to debug the driver) but it also do things differently for OOB and page access: OOB bytes are read in raw mode while pages are always accessed with the ECC engine enabled.
Quoted 4 lines
OOB accesses are broken and should be fixed, I think we all agree on that, but we should fix it correctly, don't you think? And by correctly, I mean we should try to make things consistent at least (read OOB bytes with the ECC engine enabled).
All fair points. I assumed there was some deliberate reason why READOOB was being done without ECC, so I made a point of not changing that part. For an extra data-point, I should be able to try your suggested fix on our 2nd-round prototypes in the new year (maybe sooner, on one of my 1st round protos if I get time),
Quoted 20 lines
drivers/mtd/nand/pxa3xx_nand.c static int prepare_set_command(struct pxa3xx_nand_info *info, int command, · line 1034
Quoted 4 lines
drivers/mtd/nand/pxa3xx_nand.c +0
-0}else { info->step_chunk_size = info->last_chunk_size; info->step_spare_size = info->last_spare_size;Can you try with the below patch and see if it fixes the timeout issue?
--->8---
drivers/mtd/nand/pxa3xx_nand.c +1
-0@@ -961,6 +961,7 @@ static void prepare_start_command(struct pxa3xx_nand_info *info, int command)switch (command) {case NAND_CMD_READ0 : +case NAND_CMD_READOOB :case NAND_CMD_PAGEPROG : info->use_ecc =1 ;break ;
Quoted 62 lines · Boris Brezillon
Hi Greg,
Could you at least Cc the NAND maintainer when you submit something related to NAND!
On Tue, 12 Dec 2017 16:38:26 +0800
Greg Cook <[email protected]> wrote:Please add a commit message explaining what your fixing.
Quoted 19 lines
Signed-off-by Greg Cook <[email protected]>--- drivers/mtd/nand/pxa3xx_nand.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)drivers/mtd/nand/pxa3xx_nand.c +1
-1@@ -677,7 +677,7 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)static void drain_fifo (struct pxa3xx_nand_info *info,void *data,int len) { -if (info->ecc_bch) { +if (info->use_ecc && info->ecc_bch) {u32 val;int ret;drivers/mtd/nand/pxa3xx_nand.c static int prepare_set_command(struct pxa3xx_nand_info *info, int command, · line 1031
Quoted 10 lines
drivers/mtd/nand/pxa3xx_nand.c +6
-1@@ -1023,7 +1023,12 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,if (info->cur_chunk < info->nfullchunks) { info->step_chunk_size = info->chunk_size; - info->step_spare_size = info->spare_size; +if (info->use_ecc) { + info->step_spare_size = info->spare_size; + }else { + info->step_spare_size = info->spare_size + + info->ecc_size; + }Curly braces are unneeded, and I'd prefer to have the + operator at the end of the line. Or it could be written like that:
info->step_spare_size = info->spare_size; if (!info->use_ecc) info->step_spare_size += info->ecc_size;Anyway, I'm still not convince this is the appropriate fix. As mentioned in my previous, I wonder why we're not activating ECC when reading OOB bytes. There's a good reason for having 2 different hooks to read/write OOBs: one is doing it in raw mode (with the ECC engine disabled), the other one is doing it with the ECC engine enabled. This driver not only omit raw accessors (which is already a bad news for everyone that needs to debug the driver) but it also do things differently for OOB and page access: OOB bytes are read in raw mode while pages are always accessed with the ECC engine enabled.
OOB accesses are broken and should be fixed, I think we all agree on that, but we should fix it correctly, don't you think? And by correctly, I mean we should try to make things consistent at least (read OOB bytes with the ECC engine enabled).
Quoted 6 lines
drivers/mtd/nand/pxa3xx_nand.c static int prepare_set_command(struct pxa3xx_nand_info *info, int command, · line 1034
Quoted 3 lines
drivers/mtd/nand/pxa3xx_nand.c +0
-0}else { info->step_chunk_size = info->last_chunk_size; info->step_spare_size = info->last_spare_size;Can you try with the below patch and see if it fixes the timeout issue?
Well, the patch may very well fix the timeout, but it's not really reading any OOB.
Like I said in my previous mail, accessing the OOB doesn't work when ECC is enabled. This is specified in the specs, copying from the Armada 370 spec:
"""
14.4.1
Error Checking and Correction (ECC)
[..] When ECC is enabled, the number of spare area bytes required for use by ECC are shown in the middle column of Table 30. These bytes are not available for use by the
system software and cannot be written to or read from.
"""
I seriously doubt you are reading valid data with this change.
Signature · 2 lines
Ezequiel García, VanguardiaSur www.vanguardiasur.com.ar
Quoted 87 lines · On Tue, 12 Dec 2017 13:29:11 -0300 Ezequiel Garcia…
Quoted 62 lines · Boris Brezillon
Hi Greg,
Could you at least Cc the NAND maintainer when you submit something related to NAND!
On Tue, 12 Dec 2017 16:38:26 +0800
Greg Cook <[email protected]> wrote:Please add a commit message explaining what your fixing.
Quoted 19 lines
Signed-off-by Greg Cook <[email protected]>--- drivers/mtd/nand/pxa3xx_nand.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)drivers/mtd/nand/pxa3xx_nand.c +1
-1@@ -677,7 +677,7 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)static void drain_fifo (struct pxa3xx_nand_info *info,void *data,int len) { -if (info->ecc_bch) { +if (info->use_ecc && info->ecc_bch) {u32 val;int ret;drivers/mtd/nand/pxa3xx_nand.c static int prepare_set_command(struct pxa3xx_nand_info *info, int command, · line 1031
Quoted 10 lines
drivers/mtd/nand/pxa3xx_nand.c +6
-1@@ -1023,7 +1023,12 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,if (info->cur_chunk < info->nfullchunks) { info->step_chunk_size = info->chunk_size; - info->step_spare_size = info->spare_size; +if (info->use_ecc) { + info->step_spare_size = info->spare_size; + }else { + info->step_spare_size = info->spare_size + + info->ecc_size; + }Curly braces are unneeded, and I'd prefer to have the + operator at the end of the line. Or it could be written like that:
info->step_spare_size = info->spare_size; if (!info->use_ecc) info->step_spare_size += info->ecc_size;Anyway, I'm still not convince this is the appropriate fix. As mentioned in my previous, I wonder why we're not activating ECC when reading OOB bytes. There's a good reason for having 2 different hooks to read/write OOBs: one is doing it in raw mode (with the ECC engine disabled), the other one is doing it with the ECC engine enabled. This driver not only omit raw accessors (which is already a bad news for everyone that needs to debug the driver) but it also do things differently for OOB and page access: OOB bytes are read in raw mode while pages are always accessed with the ECC engine enabled.
OOB accesses are broken and should be fixed, I think we all agree on that, but we should fix it correctly, don't you think? And by correctly, I mean we should try to make things consistent at least (read OOB bytes with the ECC engine enabled).
Quoted 6 lines
drivers/mtd/nand/pxa3xx_nand.c static int prepare_set_command(struct pxa3xx_nand_info *info, int command, · line 1034
Quoted 3 lines
drivers/mtd/nand/pxa3xx_nand.c +0
-0}else { info->step_chunk_size = info->last_chunk_size; info->step_spare_size = info->last_spare_size;Can you try with the below patch and see if it fixes the timeout issue?
Well, the patch may very well fix the timeout, but it's not really reading any OOB.
Like I said in my previous mail, accessing the OOB doesn't work when ECC is enabled. This is specified in the specs, copying from the Armada 370 spec:
"""
14.4.1
Error Checking and Correction (ECC)
[..] When ECC is enabled, the number of spare area bytes required for use by ECC are shown in the middle column of Table 30. These bytes are not available for use by the
system software and cannot be written to or read from.
"""
Quoted 1 line
I seriously doubt you are reading valid data with this change.
That's not true. Reading ECC bytes is impossible when the ECC engine is enable, reading free OOB bytes is possible, and most of the time the caller doesn't care about ECC bytes when asking for a non-raw OOB access.
Actually, it's even better to read the free OOB bytes with ECC enabled because they are ECC protected, so if you have bitflips in the free OOB section they will be fixed or reported as uncorrectable.
Quoted 92 lines · Boris Brezillon
Quoted 87 lines · On Tue, 12 Dec 2017 13:29:11 -0300 Ezequiel Garcia…
Quoted 62 lines · Boris Brezillon
Hi Greg,
Could you at least Cc the NAND maintainer when you submit something related to NAND!
On Tue, 12 Dec 2017 16:38:26 +0800
Greg Cook <[email protected]> wrote:Please add a commit message explaining what your fixing.
Quoted 19 lines
Signed-off-by Greg Cook <[email protected]>--- drivers/mtd/nand/pxa3xx_nand.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)drivers/mtd/nand/pxa3xx_nand.c +1
-1@@ -677,7 +677,7 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)static void drain_fifo (struct pxa3xx_nand_info *info,void *data,int len) { -if (info->ecc_bch) { +if (info->use_ecc && info->ecc_bch) {u32 val;int ret;drivers/mtd/nand/pxa3xx_nand.c static int prepare_set_command(struct pxa3xx_nand_info *info, int command, · line 1031
Quoted 10 lines
drivers/mtd/nand/pxa3xx_nand.c +6
-1@@ -1023,7 +1023,12 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,if (info->cur_chunk < info->nfullchunks) { info->step_chunk_size = info->chunk_size; - info->step_spare_size = info->spare_size; +if (info->use_ecc) { + info->step_spare_size = info->spare_size; + }else { + info->step_spare_size = info->spare_size + + info->ecc_size; + }Curly braces are unneeded, and I'd prefer to have the + operator at the end of the line. Or it could be written like that:
info->step_spare_size = info->spare_size; if (!info->use_ecc) info->step_spare_size += info->ecc_size;Anyway, I'm still not convince this is the appropriate fix. As mentioned in my previous, I wonder why we're not activating ECC when reading OOB bytes. There's a good reason for having 2 different hooks to read/write OOBs: one is doing it in raw mode (with the ECC engine disabled), the other one is doing it with the ECC engine enabled. This driver not only omit raw accessors (which is already a bad news for everyone that needs to debug the driver) but it also do things differently for OOB and page access: OOB bytes are read in raw mode while pages are always accessed with the ECC engine enabled.
OOB accesses are broken and should be fixed, I think we all agree on that, but we should fix it correctly, don't you think? And by correctly, I mean we should try to make things consistent at least (read OOB bytes with the ECC engine enabled).
Quoted 6 lines
drivers/mtd/nand/pxa3xx_nand.c static int prepare_set_command(struct pxa3xx_nand_info *info, int command, · line 1034
Quoted 3 lines
drivers/mtd/nand/pxa3xx_nand.c +0
-0}else { info->step_chunk_size = info->last_chunk_size; info->step_spare_size = info->last_spare_size;Can you try with the below patch and see if it fixes the timeout issue?
Well, the patch may very well fix the timeout, but it's not really reading any OOB.
Like I said in my previous mail, accessing the OOB doesn't work when ECC is enabled. This is specified in the specs, copying from the Armada 370 spec:
"""
14.4.1
Error Checking and Correction (ECC)
[..] When ECC is enabled, the number of spare area bytes required for use by ECC are shown in the middle column of Table 30. These bytes are not available for use by the
system software and cannot be written to or read from.
"""Quoted 1 line
I seriously doubt you are reading valid data with this change.
Quoted 8 lines
That's not true. Reading ECC bytes is impossible when the ECC engine is enable, reading free OOB bytes is possible, and most of the time the caller doesn't care about ECC bytes when asking for a non-raw OOB access.
Actually, it's even better to read the free OOB bytes with ECC enabled because they are ECC protected, so if you have bitflips in the free OOB section they will be fixed or reported as uncorrectable.
OK, then, maybe I'm wrong. We need mark some a couple blocks as bad in the bootloader, and then have the kernel finds them and builds the BBT properly, with this patch.
Signature · 2 lines
Ezequiel García, VanguardiaSur www.vanguardiasur.com.ar