[U-Boot] [PATCH] flash: Add optional verify-after-write feature
Sometimes it might make sense to verify the written data to NOR flash. This patch adds this feature. To enable this verify-after-write, you need to define CONFIG_SYS_FLASH_VERIFY_AFTER_WRITE in your board config header.
Signed-off-by: Stefan Roese sr@denx.de --- common/flash.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/common/flash.c b/common/flash.c index 8244ba2..3ae3c9a 100644 --- a/common/flash.c +++ b/common/flash.c @@ -149,6 +149,9 @@ flash_write (char *src, ulong addr, ulong cnt) flash_info_t *info_first = addr2info (addr); flash_info_t *info_last = addr2info (end ); flash_info_t *info; + __maybe_unused char *src_orig = src; + __maybe_unused char *addr_orig = (char *)addr; + __maybe_unused ulong cnt_orig = cnt;
if (cnt == 0) { return (ERR_OK); @@ -185,6 +188,14 @@ flash_write (char *src, ulong addr, ulong cnt) addr += len; src += len; } + +#if defined(CONFIG_SYS_FLASH_VERIFY_AFTER_WRITE) + if (memcmp(src_orig, addr_orig, cnt_orig)) { + printf("\nVerify-after-write failed!\n"); + return ERR_PROG_ERROR; + } +#endif /* CONFIG_SYS_FLASH_VERIFY_AFTER_WRITE */ + return (ERR_OK); #endif /* CONFIG_SPD823TS */ }
Dear Stefan Roese,
In message 1365059554-10662-1-git-send-email-sr@denx.de you wrote:
Sometimes it might make sense to verify the written data to NOR flash. This patch adds this feature. To enable this verify-after-write, you need to define CONFIG_SYS_FLASH_VERIFY_AFTER_WRITE in your board config header.
Signed-off-by: Stefan Roese sr@denx.de
common/flash.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
This is a user selectable feature, thus the option should be CONFIG_FLASH_VERIFY_AFTER_WRITE. Thinking about it - obviously you cannot verify _before_ the write, so all the "after write" is redundant. Better call it just "CONFIG_FLASH_VERIFY". Please also change the error message:
Instead
printf("\nVerify-after-write failed!\n");
just:
printf("\nVerify failed!\n");
Finally - you are introducing a new CONFIG_ option; this must be documented in the README.
And it might make sense to add a comment that this option is totally useless in almost all cases, and should only be enabled if you know EXACTLY what you are doing - and that it does not really work even then.
Best regards,
Wolfgang Denk
Hi Wolfgang,
On 04.04.2013 14:46, Wolfgang Denk wrote:
In message 1365059554-10662-1-git-send-email-sr@denx.de you wrote:
Sometimes it might make sense to verify the written data to NOR flash. This patch adds this feature. To enable this verify-after-write, you need to define CONFIG_SYS_FLASH_VERIFY_AFTER_WRITE in your board config header.
Signed-off-by: Stefan Roese sr@denx.de
common/flash.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
This is a user selectable feature, thus the option should be CONFIG_FLASH_VERIFY_AFTER_WRITE. Thinking about it - obviously you cannot verify _before_ the write, so all the "after write" is redundant. Better call it just "CONFIG_FLASH_VERIFY". Please also change the error message:
Instead
printf("\nVerify-after-write failed!\n");
just:
printf("\nVerify failed!\n");
Finally - you are introducing a new CONFIG_ option; this must be documented in the README.
And it might make sense to add a comment that this option is totally useless in almost all cases, and should only be enabled if you know EXACTLY what you are doing - and that it does not really work even then.
Thanks for your review. Everything you mentioned makes perfect sense. I'll address those issues in v2 of the patch.
Thanks, Stefan
Sometimes it might make sense to verify the written data to NOR flash. This patch adds this feature. To enable this verify-after-write, you need to define CONFIG_FLASH_VERIFY in your board config header.
Please note that this option is useless in nearly all cases, since such flash programming errors usually are detected earlier while unprotecting/erasing/programming. Please only enable this option if you really know what you are doing.
Signed-off-by: Stefan Roese sr@denx.de --- v2: - Change CONFIG_SYS_FLASH_VERIFY_AFTER_WRITE to CONFIG_FLASH_VERIFY - Change error message - Add documentation for this option to README
README | 9 +++++++++ common/flash.c | 11 +++++++++++ 2 files changed, 20 insertions(+)
diff --git a/README b/README index 5701016..1ce9a98 100644 --- a/README +++ b/README @@ -3190,6 +3190,15 @@ Configuration Settings: digits and dots. Recommended value: 45 (9..1) for 80 column displays, 15 (3..1) for 40 column displays.
+- CONFIG_FLASH_VERIFY + If defined, the content of the flash (destination) is compared + against the source after the write operation. An error message + will be printed when the contents are not identical. + Please note that this option is useless in nearly all cases, + since such flash programming errors usually are detected earlier + while unprotecting/erasing/programming. Please only enable + this option if you really know what you are doing. + - CONFIG_SYS_RX_ETH_BUFFER: Defines the number of Ethernet receive buffers. On some Ethernet controllers it is recommended to set this value diff --git a/common/flash.c b/common/flash.c index 8244ba2..0c57a3f 100644 --- a/common/flash.c +++ b/common/flash.c @@ -149,6 +149,9 @@ flash_write (char *src, ulong addr, ulong cnt) flash_info_t *info_first = addr2info (addr); flash_info_t *info_last = addr2info (end ); flash_info_t *info; + __maybe_unused char *src_orig = src; + __maybe_unused char *addr_orig = (char *)addr; + __maybe_unused ulong cnt_orig = cnt;
if (cnt == 0) { return (ERR_OK); @@ -185,6 +188,14 @@ flash_write (char *src, ulong addr, ulong cnt) addr += len; src += len; } + +#if defined(CONFIG_FLASH_VERIFY) + if (memcmp(src_orig, addr_orig, cnt_orig)) { + printf("\nVerify failed!\n"); + return ERR_PROG_ERROR; + } +#endif /* CONFIG_SYS_FLASH_VERIFY_AFTER_WRITE */ + return (ERR_OK); #endif /* CONFIG_SPD823TS */ }
On 04.04.2013 15:53, Stefan Roese wrote:
Sometimes it might make sense to verify the written data to NOR flash. This patch adds this feature. To enable this verify-after-write, you need to define CONFIG_FLASH_VERIFY in your board config header.
Please note that this option is useless in nearly all cases, since such flash programming errors usually are detected earlier while unprotecting/erasing/programming. Please only enable this option if you really know what you are doing.
Signed-off-by: Stefan Roese sr@denx.de
Applied to u-boot-cfi-flash/master.
Thanks, Stefan
participants (2)
-
Stefan Roese -
Wolfgang Denk