From: Stefan Roese <sr@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] flash: Add optional verify-after-write feature
Date: Thu, 04 Apr 2013 15:29:35 +0200 [thread overview]
Message-ID: <515D803F.9060303@denx.de> (raw)
In-Reply-To: <20130404124623.CC8F82015BC@gemini.denx.de>
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
next prev parent reply other threads:[~2013-04-04 13:29 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-04 7:12 [U-Boot] [PATCH] flash: Add optional verify-after-write feature Stefan Roese
2013-04-04 12:46 ` Wolfgang Denk
2013-04-04 13:29 ` Stefan Roese [this message]
2013-04-04 13:53 ` [U-Boot] [PATCH v2] " Stefan Roese
2013-04-22 9:08 ` Stefan Roese
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=515D803F.9060303@denx.de \
--to=sr@denx.de \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.