mtd: nandbiterrs: Support for NAND biterrors test on platforms without raw write
PATCH 1/1 mtd: nandbiterrs: Support for NAND biterrors test on platforms without raw write
Hi all,
I'm vaguely aware that there is an ongoing effort to move this stuff to mtd-utils, but I was unable to find a source tree with the work so far.
Below is a single-line patch for the kernel tests, feel free to apply to the userspace source as well.
Best regards,
Iwo
Support for NAND biterrors test on platforms without raw write
While the default test mode relies on raw write (mtd_write_oob) to introduce bit errors into a page, the rewrite test mode doesn't need it.
Some drivers use eldritch data/ECC arrangements in a NAND page and reshuffle things on-the-fly, to present a normal page view to the kernel. Typically, raw write / read is unsupported on such platforms. Examples are Freescale MXS and Qualcomm MDM9 and probably many others.
Changed the overwrite test to use normal writes.
--- drivers/mtd/tests/nandbiterrs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mtd/tests/nandbiterrs.c +1 -1
@@ -290,7 +290,7 @@ static int overwrite_test(void)while (opno < max_overwrite) { - err =rewrite_page (0 ); + err =write_page (0 );if (err)break ;
Signature · 1 line
2.7.3
Hi Iwo,
Quoted 18 lines · On Fri, 6 May 2016 10:00:12 +1000 Iwo Mergler…
Hi all,
I'm vaguely aware that there is an ongoing effort to move this stuff to mtd-utils, but I was unable to find a source tree with the work so far.
Below is a single-line patch for the kernel tests, feel free to apply to the userspace source as well.
Best regards,
Iwo
Support for NAND biterrors test on platforms without raw write
Quoted 7 lines
While the default test mode relies on raw write (
mtd_write_oob) to introduce bit errors into a page, the rewrite test mode doesn't need it.Some drivers use eldritch data/ECC arrangements in a NAND page and reshuffle things on-the-fly, to present a normal page view to the kernel. Typically, raw write / read is unsupported on such platforms. Examples are Freescale MXS and Qualcomm MDM9 and probably many others.
Sorry, but I think such platforms should unshuffle the data/ECC sections to expose a standard in-band/out-of-band view to the upper layer.
This is completely doable since ->read_page_raw()/->write_page_raw() can be overloaded (see the GPMI implementation or the default ECC_HW_SYNDROME raw implementation if you need examples).
So, it's a NACK on my side.
Best Regards,
Boris
Quoted 22 lines
Changed the overwrite test to use normal writes.
Signed-off-by Iwo Mergler <[email protected]>--- drivers/mtd/tests/nandbiterrs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/mtd/tests/nandbiterrs.c +1
-1@@ -290,7 +290,7 @@ static int overwrite_test(void)while (opno < max_overwrite) { - err =rewrite_page (0 ); + err =write_page (0 );if (err)break ;
Signature · 3 lines
Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Quoted 30 lines · On Sun, 8 May 2016 18:44:51 +0200 Boris Brezillon…
Hi Iwo,
Quoted 25 lines · On Fri, 6 May 2016 10:00:12 +1000 Iwo Mergler…
Hi all,
I'm vaguely aware that there is an ongoing effort to move this stuff to mtd-utils, but I was unable to find a source tree with the work so far.
Below is a single-line patch for the kernel tests, feel free to apply to the userspace source as well.
Best regards,
Iwo
Support for NAND biterrors test on platforms without raw write
While the default test mode relies on raw write (
mtd_write_oob) to introduce bit errors into a page, the rewrite test mode doesn't need it.Some drivers use eldritch data/ECC arrangements in a NAND page and reshuffle things on-the-fly, to present a normal page view to the kernel. Typically, raw write / read is unsupported on such platforms. Examples are Freescale MXS and Qualcomm MDM9 and probably many others.
Sorry, I didn't realize you were talking about freescale (i.e. GPMI controller). I'm pretty sure raw access is properly supported on this platform, and if it's not, you should fix the ->{read,write}_page_raw() implementations.
Quoted 41 lines
Sorry, but I think such platforms should unshuffle the data/ECC sections to expose a standard in-band/out-of-band view to the upper layer.
This is completely doable since ->read_page_raw()/->write_page_raw() can be overloaded (see the GPMI implementation or the default
ECC_HW_SYNDROMEraw implementation if you need examples).So, it's a NACK on my side.
Best Regards,
Boris
Quoted 22 lines
Changed the overwrite test to use normal writes.
Signed-off-by Iwo Mergler <[email protected]>--- drivers/mtd/tests/nandbiterrs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/mtd/tests/nandbiterrs.c +1
-1@@ -290,7 +290,7 @@ static int overwrite_test(void)while (opno < max_overwrite) { - err =rewrite_page (0 ); + err =write_page (0 );if (err)break ;
Signature · 3 lines
Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Hi Boris,
I have to admit that your NACK surprised me.
My patch removes an unnecessary use of raw write from the test. It was only there because of my original implementation, which I now consider mistaken.
I fully agree with you that raw write should be implemented, despite the impediments. Although I have seen at least one NAND controller that always computed and wrote ECC, with no way for software to circumvent it.
Could you please elaborate a little why you don't want a test module to work with incomplete MTD drivers? Is that supposed to be motivating driver writers for better implementations? ;-)
Would you accept the patch if I remove the comment about data reshuffling drivers? It's not required for the patch and, as you correctly pointed out, now inaccurate.
Best regards,
Iwo
Quoted 68 lines · Boris Brezillon
Hi Iwo,
Quoted 18 lines · On Fri, 6 May 2016 10:00:12 +1000 Iwo Mergler…
Hi all,
I'm vaguely aware that there is an ongoing effort to move this stuff to mtd-utils, but I was unable to find a source tree with the work so far.
Below is a single-line patch for the kernel tests, feel free to apply to the userspace source as well.
Best regards,
Iwo
Support for NAND biterrors test on platforms without raw write
Quoted 7 lines
While the default test mode relies on raw write (
mtd_write_oob) to introduce bit errors into a page, the rewrite test mode doesn't need it.Some drivers use eldritch data/ECC arrangements in a NAND page and reshuffle things on-the-fly, to present a normal page view to the kernel. Typically, raw write / read is unsupported on such platforms. Examples are Freescale MXS and Qualcomm MDM9 and probably many others.
Sorry, but I think such platforms should unshuffle the data/ECC sections to expose a standard in-band/out-of-band view to the upper layer.
This is completely doable since ->read_page_raw()/->write_page_raw() can be overloaded (see the GPMI implementation or the default
ECC_HW_SYNDROMEraw implementation if you need examples).So, it's a NACK on my side.
Best Regards,
Boris
Quoted 21 lines
Changed the overwrite test to use normal writes.
Signed-off-by Iwo Mergler <[email protected]>--- drivers/mtd/tests/nandbiterrs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/mtd/tests/nandbiterrs.c +1
-1@@ -290,7 +290,7 @@ static int overwrite_test(void)while (opno < max_overwrite) { - err =rewrite_page (0 ); + err =write_page (0 );if (err)break ;
Hi Iwo,
Quoted 9 lines · On Mon, 9 May 2016 14:09:46 +1000 Iwo Mergler…
Hi Boris,
I have to admit that your NACK surprised me.
My patch removes an unnecessary use of raw write from the test. It was only there because of my original implementation, which I now consider mistaken.
Sorry, I didn't look at the diff itself, and focused on the commit message :-/. Indeed, using normal write in the overwrite test should be harmless, but I still think that all controller should properly implement raw access functions, otherwise the "incremental errors" test is irrelevant (you'll overwrite ECC bytes along with in-band data, and will end up with more bitflips than you expected).
Quoted 6 lines
I fully agree with you that raw write should be implemented, despite the impediments. Although I have seen at least one NAND controller that always computed and wrote ECC, with no way for software to circumvent it.
Hm, I was told that so many times and each time I had a closer look it appeared to be untrue, so I tend to be skeptical on these kind of statement now. Could you tell me more about this controller?
Quoted 4 lines
Could you please elaborate a little why you don't want a test module to work with incomplete MTD drivers?
As I said, this test module will only work in overwrite mode when the controller does not support raw accesses.
Quoted 2 lines
Is that supposed to be motivating
driver writers for better implementations? ;-)
Yes, partly, and also because it's really helpful when you need to debug NAND stuff.
Honestly, I'd rather see NAND implementations return -ENOTSUPP when they do not support raw accesses than pretending they are.
Quoted 5 lines
Would you accept the patch if I remove the comment about data reshuffling drivers? It's not required for the patch and, as you correctly pointed out, now inaccurate.
At least rework it to mention that you're only modifying the overwrite test, and that writing in normal mode in this case is harmless.
Thanks,
Boris
Signature · 3 lines
Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Quoted 14 lines · Boris Brezillon
Hi Iwo,
Quoted 9 lines · On Mon, 9 May 2016 14:09:46 +1000 Iwo Mergler…
Hi Boris,
I have to admit that your NACK surprised me.
My patch removes an unnecessary use of raw write from the test. It was only there because of my original implementation, which I now consider mistaken.
Quoted 6 lines
Sorry, I didn't look at the diff itself, and focused on the commit message :-/. Indeed, using normal write in the overwrite test should be harmless, but I still think that all controller should properly implement raw access functions, otherwise the "incremental errors" test is irrelevant (you'll overwrite ECC bytes along with in-band data, and will end up with more bitflips than you expected).
Indeed. Fully agree.
Quoted 8 lines
Quoted 5 lines
I fully agree with you that raw write should be implemented, despite the impediments. Although I have seen at least one NAND controller that always computed and wrote ECC, with no way for software to circumvent it.
Hm, I was told that so many times and each time I had a closer look it appeared to be untrue, so I tend to be skeptical on these kind of statement now. Could you tell me more about this controller?
It's been a while. Probably Philips/NXP mobile phone chip set, PNX8000 or similar. Don't think it ever made it into the market as such, but the earliest ARM9 'microcontrollers' (LPC3xxx) have a high likelihood of being the same silicon.
DMA with no provision of software I/O was fashionable in the early '00s. Patent US8060688 is a reflection of that pain, despite the attempts at obfuscation by the attorneys. ;-)
Quoted 5 lines
Quoted 3 lines
Could you please elaborate a little why you don't want a test module to work with incomplete MTD drivers?
As I said, this test module will only work in overwrite mode when the controller does not support raw accesses.
Quoted 7 lines
Quoted 2 lines
Is that supposed to be motivating
driver writers for better implementations? ;-)Yes, partly, and also because it's really helpful when you need to debug NAND stuff.
Honestly, I'd rather see NAND implementations return
-ENOTSUPPwhen they do not support raw accesses than pretending they are.
I think the Qualcomm driver does that, the original form of the test fails with an error code on the raw write.
Quoted 6 lines
Quoted 4 lines
Would you accept the patch if I remove the comment about data reshuffling drivers? It's not required for the patch and, as you correctly pointed out, now inaccurate.
At least rework it to mention that you're only modifying the overwrite test, and that writing in normal mode in this case is harmless.
I'll do that. Patch follows.
Best regards,
Iwo
Support for NAND biterrors test on platforms without raw write
While the default test mode relies on raw write (mtd_write_oob) to introduce bit errors into a page, the rewrite test mode doesn't need it.
Changed the overwrite test to use normal writes. The default test mode is unaffected and still requires raw write as before.
--- drivers/mtd/tests/nandbiterrs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mtd/tests/nandbiterrs.c +1 -1
@@ -290,7 +290,7 @@ static int overwrite_test(void)while (opno < max_overwrite) { - err =rewrite_page (0 ); + err =write_page (0 );if (err)break ;
Signature · 1 line
2.7.3
Boris Brezillon
AppliedQuoted 7 lines · On Wed, 11 May 2016 16:54:57 +1000 Iwo Mergler…
Support for NAND biterrors test on platforms without raw write
While the default test mode relies on raw write (
mtd_write_oob) to introduce bit errors into a page, the rewrite test mode doesn't need it.Changed the overwrite test to use normal writes. The default test mode is unaffected and still requires raw write as before.
Applied.
Thanks,
Boris
Quoted 20 lines
Signed-off-by Iwo Mergler <[email protected]>--- drivers/mtd/tests/nandbiterrs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/mtd/tests/nandbiterrs.c +1
-1@@ -290,7 +290,7 @@ static int overwrite_test(void)while (opno < max_overwrite) { - err =rewrite_page (0 ); + err =write_page (0 );if (err)break ;