[U-Boot] [PATCH v2] dfu, nand: before write a buffer to nand, erase the nand sectors

[Heiko Schocher]

Hello Scott,

Am 18.06.2013 02:51, schrieb Scott Wood:
> On 06/17/2013 12:01:01 AM, Heiko Schocher wrote:
>> before writing the received buffer to nand, erase the nand
>> sectors. If not doing this, nand write fails. See for
>> more info here:
>>
>> http://lists.denx.de/pipermail/u-boot/2013-June/156361.html
>>
>> Signed-off-by: Heiko Schocher <hs at denx.de>
>> Cc: Scott Wood <scottwood at freescale.com>
>> Cc: Pantelis Antoniou <panto at antoniou-consulting.com>
>> Cc: Lukasz Majewski <l.majewski at samsung.com>
>> Cc: Kyungmin Park <kyungmin.park at samsung.com>
>> Cc: Marek Vasut <marex at denx.de>
>> Cc: Tom Rini <trini at ti.com>
>>
>> ---
>> - changes for v2:
>>   - use opts.spread as Scott Wood suggested
>>
>>  drivers/dfu/dfu_nand.c | 17 +++++++++++++++--
>>  1 Datei geändert, 15 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)
>>
>> diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
>> index 7dc89b2..93db9bd 100644
>> --- a/drivers/dfu/dfu_nand.c
>> +++ b/drivers/dfu/dfu_nand.c
>> @@ -63,12 +63,25 @@ static int nand_block_op(enum dfu_nand_op op,  
>> struct dfu_entity *dfu,
>>
>>  	nand = &nand_info[nand_curr_device];
>>
>> -	if (op == DFU_OP_READ)
>> +	if (op == DFU_OP_READ) {
>>  		ret = nand_read_skip_bad(nand, start, &count, &actual,
>>  				lim, buf);
>> -	else
>> +	} else {
>> +		nand_erase_options_t opts;
>> +
>> +		memset(&opts, 0, sizeof(opts));
>> +		opts.offset = start;
>> +		opts.length = count;
>> +		opts.spread = 1;
>> +		opts.quiet = 1;
>> +		/* first erase */
>> +		ret = nand_erase_opts(nand, &opts);
>> +		if (ret)
>> +			return ret;
>> +		/* then write */
>>  		ret = nand_write_skip_bad(nand, start, &count, &actual,
>>  				lim, buf, 0);
> 
> BTW, I notice you are currently using the limit functionality of  
> nand_read/write_skip_bad...  opts.spread currently does not have this  
> support (as I noted before), which means that if there's an error you'd  
> erase too much and then refuse to write.
> 
> Maybe we need an opts.limit?

Yes, I think so ... whats with the following proposal:

diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c
index d81972c..b877c7d 100644
--- a/drivers/mtd/nand/nand_util.c
+++ b/drivers/mtd/nand/nand_util.c
@@ -120,6 +120,10 @@ int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts)

                WATCHDOG_RESET();

+               if ((opts->limit) && (erase.addr > opts->limit)) {
+                       puts("Size of write exceeds partition or device limit\n");
+                       return -EFBIG;
+               }
                if (!opts->scrub && bbtest) {
                        int ret = mtd_block_isbad(meminfo, erase.addr);
                        if (ret > 0) {
diff --git a/include/nand.h b/include/nand.h
index 26190e4..d799df3 100644
--- a/include/nand.h
+++ b/include/nand.h
@@ -125,6 +125,8 @@ struct nand_erase_options {

        /* Don't include skipped bad blocks in size to be erased */
        int spread;
+       /* maximum size that actual may be in order to not exceed the buf */
+       loff_t limit;
 };

 typedef struct nand_erase_options nand_erase_options_t;

I checked for all calls from nand_erase_opts, that the nand_erase_options_t
parameters are initialized with 0 ... so this patch should not change
current behaviour.

Should I do this in a seperate patch, or add it to the "dfu, nand:
before write a buffer to nand, erase the nand sectors" patch, so it adds
no dead code ...

> adjust_size_for_badblocks, OTOH, is probably the opposite of what you  
> wanted -- it subtracts from the size in order to get the number of good  
> blocks within an interval, rather than adding the number of bad blocks  
> to turn a data size into an interval.  It's meant to produce an input  
> to be used with skipping/spreading operations.

Yes, thats not what I wanted ...

> Which makes me think we have a bug in cmd_nand.c -- we should be  
> setting .spread in erase cases where we call adjust_size_for_badblocks.

Yes, seems so ...

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany