[U-Boot] [PATCH] flash: Add optional verify-after-write feature

[U-Boot] [PATCH] flash: Add optional verify-after-write feature

[Stefan Roese]

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 */
 }
-- 
1.8.2

[U-Boot] [PATCH] flash: Add optional verify-after-write feature

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"One planet is all you get."

[U-Boot] [PATCH] flash: Add optional verify-after-write feature

[Stefan Roese]

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

[U-Boot] [PATCH v2] flash: Add optional verify-after-write feature

[Stefan Roese]

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 */
 }
-- 
1.8.2

[U-Boot] [PATCH v2] flash: Add optional verify-after-write feature

[Stefan Roese]

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