public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] dfu, nand: before write a buffer to nand, erase the nand sectors
@ 2013-06-14 10:24 Heiko Schocher
  2013-06-14 22:19 ` Scott Wood
  2013-06-17  5:01 ` [U-Boot] [PATCH v2] " Heiko Schocher
  0 siblings, 2 replies; 12+ messages in thread
From: Heiko Schocher @ 2013-06-14 10:24 UTC (permalink / raw)
  To: u-boot

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@denx.de>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Tom Rini <trini@ti.com>
---
 common/cmd_nand.c      |  2 +-
 drivers/dfu/dfu_nand.c | 17 +++++++++++++++--
 include/nand.h         |  1 +
 3 Dateien ge?ndert, 17 Zeilen hinzugef?gt(+), 3 Zeilen entfernt(-)

diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index 8b1e01a..d3fe138 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -428,7 +428,7 @@ static int raw_access(nand_info_t *nand, ulong addr, loff_t off, ulong count,
 /* Adjust a chip/partition size down for bad blocks so we don't
  * read/write/erase past the end of a chip/partition by accident.
  */
-static void adjust_size_for_badblocks(loff_t *size, loff_t offset, int dev)
+void adjust_size_for_badblocks(loff_t *size, loff_t offset, int dev)
 {
 	/* We grab the nand info object here fresh because this is usually
 	 * called after arg_off_size() which can change the value of dev.
diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
index 7dc89b2..6f9a934 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;
+		adjust_size_for_badblocks(&opts.length, offset,
+					  nand_curr_device);
+		/* 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);
+	}
 
 	if (ret != 0) {
 		printf("%s: nand_%s_skip_bad call failed at %llx!\n",
diff --git a/include/nand.h b/include/nand.h
index 26190e4..8495b73 100644
--- a/include/nand.h
+++ b/include/nand.h
@@ -141,6 +141,7 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
 			size_t *actual, loff_t lim, u_char *buffer, int flags);
 int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts);
 int nand_torture(nand_info_t *nand, loff_t offset);
+void adjust_size_for_badblocks(loff_t *size, loff_t offset, int dev);
 
 #define NAND_LOCK_STATUS_TIGHT	0x01
 #define NAND_LOCK_STATUS_UNLOCK 0x04
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] dfu, nand: before write a buffer to nand, erase the nand sectors
  2013-06-14 10:24 [U-Boot] [PATCH] dfu, nand: before write a buffer to nand, erase the nand sectors Heiko Schocher
@ 2013-06-14 22:19 ` Scott Wood
  2013-06-17  5:01 ` [U-Boot] [PATCH v2] " Heiko Schocher
  1 sibling, 0 replies; 12+ messages in thread
From: Scott Wood @ 2013-06-14 22:19 UTC (permalink / raw)
  To: u-boot

On 06/14/2013 05:24:03 AM, Heiko Schocher wrote:
> diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
> index 7dc89b2..6f9a934 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;
> +		adjust_size_for_badblocks(&opts.length, offset,
> +					  nand_curr_device);
> +		/* first erase */
> +		ret = nand_erase_opts(nand, &opts);
> +		if (ret)
> +			return ret;

You could use opts.spread instead of calling  
adjust_size_for_badblocks().  If you do want to use  
adjust_size_for_badblocks() -- say, to do bounds checking on the result  
-- please move it to nand_util.c, and pass in a pointer to nand_info_t  
instead of "int dev".

-Scott

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v2] dfu, nand: before write a buffer to nand, erase the nand sectors
  2013-06-14 10:24 [U-Boot] [PATCH] dfu, nand: before write a buffer to nand, erase the nand sectors Heiko Schocher
  2013-06-14 22:19 ` Scott Wood
@ 2013-06-17  5:01 ` Heiko Schocher
  2013-06-18  0:51   ` Scott Wood
  2013-06-21  5:09   ` [U-Boot] [PATCH v3] " Heiko Schocher
  1 sibling, 2 replies; 12+ messages in thread
From: Heiko Schocher @ 2013-06-17  5:01 UTC (permalink / raw)
  To: u-boot

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@denx.de>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Tom Rini <trini@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);
+	}
 
 	if (ret != 0) {
 		printf("%s: nand_%s_skip_bad call failed at %llx!\n",
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v2] dfu, nand: before write a buffer to nand, erase the nand sectors
  2013-06-17  5:01 ` [U-Boot] [PATCH v2] " Heiko Schocher
@ 2013-06-18  0:51   ` Scott Wood
  2013-06-20  4:36     ` Heiko Schocher
  2013-06-21  5:09   ` [U-Boot] [PATCH v3] " Heiko Schocher
  1 sibling, 1 reply; 12+ messages in thread
From: Scott Wood @ 2013-06-18  0:51 UTC (permalink / raw)
  To: u-boot

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@denx.de>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Tom Rini <trini@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?

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.

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.

-Scott

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v2] dfu, nand: before write a buffer to nand, erase the nand sectors
  2013-06-18  0:51   ` Scott Wood
@ 2013-06-20  4:36     ` Heiko Schocher
  2013-06-20 17:22       ` Scott Wood
  0 siblings, 1 reply; 12+ messages in thread
From: Heiko Schocher @ 2013-06-20  4:36 UTC (permalink / raw)
  To: u-boot

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@denx.de>
>> Cc: Scott Wood <scottwood@freescale.com>
>> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Tom Rini <trini@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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v2] dfu, nand: before write a buffer to nand, erase the nand sectors
  2013-06-20  4:36     ` Heiko Schocher
@ 2013-06-20 17:22       ` Scott Wood
  0 siblings, 0 replies; 12+ messages in thread
From: Scott Wood @ 2013-06-20 17:22 UTC (permalink / raw)
  To: u-boot

On 06/19/2013 11:36:17 PM, Heiko Schocher wrote:
> Hello Scott,
> 
> Am 18.06.2013 02:51, schrieb Scott Wood:
> > 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;
> +               }

This is treating limit as an address rather than a size.

Also, unnecessary parens.

> 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 ...

A separate patch within a patchset should be fine, but I'm also OK with  
combining them since the whole thing would still be small and  
straightforward enough to be easily reviewed.

-Scott

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v3] dfu, nand: before write a buffer to nand, erase the nand sectors
  2013-06-17  5:01 ` [U-Boot] [PATCH v2] " Heiko Schocher
  2013-06-18  0:51   ` Scott Wood
@ 2013-06-21  5:09   ` Heiko Schocher
  2013-06-21 22:50     ` Scott Wood
  2013-06-24 16:50     ` [U-Boot] [PATCH v4] " Heiko Schocher
  1 sibling, 2 replies; 12+ messages in thread
From: Heiko Schocher @ 2013-06-21  5:09 UTC (permalink / raw)
  To: u-boot

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

Using the nand erase option "spread", maybe overwrite
blocks on, for example another mtd partition, if the
erasing range contains bad blocks.
So a limit option is added to nand_erase_opts()

Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Tom Rini <trini@ti.com>

---
- changes for v2:
  - use opts.spread as Scott Wood suggested
- changes for v3:
  - add opts.lim as Scott Wood suggested

 drivers/dfu/dfu_nand.c       | 18 ++++++++++++++++--
 drivers/mtd/nand/nand_util.c |  4 ++++
 include/nand.h               |  2 ++
 3 Dateien ge?ndert, 22 Zeilen hinzugef?gt(+), 2 Zeilen entfernt(-)

diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
index 7dc89b2..07dee89 100644
--- a/drivers/dfu/dfu_nand.c
+++ b/drivers/dfu/dfu_nand.c
@@ -63,12 +63,26 @@ 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;
+		opts.lim = lim;
+		/* 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);
+	}
 
 	if (ret != 0) {
 		printf("%s: nand_%s_skip_bad call failed at %llx!\n",
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c
index d81972c..2778f7f 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->lim && (erase.addr > (opts->offset + opts->lim))) {
+			puts("Size of erase exceeds 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..228d871 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 lim;
 };
 
 typedef struct nand_erase_options nand_erase_options_t;
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v3] dfu, nand: before write a buffer to nand, erase the nand sectors
  2013-06-21  5:09   ` [U-Boot] [PATCH v3] " Heiko Schocher
@ 2013-06-21 22:50     ` Scott Wood
  2013-06-24 16:50     ` [U-Boot] [PATCH v4] " Heiko Schocher
  1 sibling, 0 replies; 12+ messages in thread
From: Scott Wood @ 2013-06-21 22:50 UTC (permalink / raw)
  To: u-boot

On 06/21/2013 12:09:18 AM, Heiko Schocher wrote:
> diff --git a/drivers/mtd/nand/nand_util.c  
> b/drivers/mtd/nand/nand_util.c
> index d81972c..2778f7f 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->lim && (erase.addr > (opts->offset +  
> opts->lim))) {

Should be >=

-Scott

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v4] dfu, nand: before write a buffer to nand, erase the nand sectors
  2013-06-21  5:09   ` [U-Boot] [PATCH v3] " Heiko Schocher
  2013-06-21 22:50     ` Scott Wood
@ 2013-06-24 16:50     ` Heiko Schocher
  2013-06-24 23:26       ` Scott Wood
  2013-07-25  4:43       ` [U-Boot] [PATCH v5] dfu, nand, ubi: add partubi alt settings for updating ubi partition Heiko Schocher
  1 sibling, 2 replies; 12+ messages in thread
From: Heiko Schocher @ 2013-06-24 16:50 UTC (permalink / raw)
  To: u-boot

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

Using the nand erase option "spread", maybe overwrite
blocks on, for example another mtd partition, if the
erasing range contains bad blocks.
So a limit option is added to nand_erase_opts()

Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Tom Rini <trini@ti.com>

---
- changes for v2:
  - use opts.spread as Scott Wood suggested
- changes for v3:
  - add opts.lim as Scott Wood suggested
- changes for v4:
  - fix boundary check as Scott Wood mentioned

 drivers/dfu/dfu_nand.c       | 18 ++++++++++++++++--
 drivers/mtd/nand/nand_util.c |  4 ++++
 include/nand.h               |  2 ++
 3 Dateien ge?ndert, 22 Zeilen hinzugef?gt(+), 2 Zeilen entfernt(-)

diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
index 7dc89b2..07dee89 100644
--- a/drivers/dfu/dfu_nand.c
+++ b/drivers/dfu/dfu_nand.c
@@ -63,12 +63,26 @@ 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;
+		opts.lim = lim;
+		/* 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);
+	}
 
 	if (ret != 0) {
 		printf("%s: nand_%s_skip_bad call failed at %llx!\n",
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c
index d81972c..1d22b52 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->lim && (erase.addr >= (opts->offset + opts->lim))) {
+			puts("Size of erase exceeds 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..228d871 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 lim;
 };
 
 typedef struct nand_erase_options nand_erase_options_t;
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v4] dfu, nand: before write a buffer to nand, erase the nand sectors
  2013-06-24 16:50     ` [U-Boot] [PATCH v4] " Heiko Schocher
@ 2013-06-24 23:26       ` Scott Wood
  2013-07-25  4:43       ` [U-Boot] [PATCH v5] dfu, nand, ubi: add partubi alt settings for updating ubi partition Heiko Schocher
  1 sibling, 0 replies; 12+ messages in thread
From: Scott Wood @ 2013-06-24 23:26 UTC (permalink / raw)
  To: u-boot

On 06/24/2013 11:50:40 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
> 
> Using the nand erase option "spread", maybe overwrite
> blocks on, for example another mtd partition, if the
> erasing range contains bad blocks.
> So a limit option is added to nand_erase_opts()
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Tom Rini <trini@ti.com>
> 
> ---
> - changes for v2:
>   - use opts.spread as Scott Wood suggested
> - changes for v3:
>   - add opts.lim as Scott Wood suggested
> - changes for v4:
>   - fix boundary check as Scott Wood mentioned

Applied to u-boot-nand-flash

-Scott

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v5] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-06-24 16:50     ` [U-Boot] [PATCH v4] " Heiko Schocher
  2013-06-24 23:26       ` Scott Wood
@ 2013-07-25  4:43       ` Heiko Schocher
  2013-07-26 22:05         ` Scott Wood
  1 sibling, 1 reply; 12+ messages in thread
From: Heiko Schocher @ 2013-07-25  4:43 UTC (permalink / raw)
  To: u-boot

updating an ubi partition needs a completely erased mtd partition,
see:
http://lists.infradead.org/pipermail/linux-mtd/2011-May/035416.html

So, add partubi alt setting for the dfu_alt_info environment
variable to mark this partition as an ubi partition. In case we
update an ubi partition, we erase after flashing the image into the
partition, the remaining sektors.

Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
Cc: Tom Rini <trini@ti.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Scott Wood <scottwood@freescale.com>

---

- This patch is also a good starting point to fix up updating ubi, as
  we currently use "nand erase" for erasing the sektors. This is
  not the prefered way for writing an ubi image, see:
  http://www.linux-mtd.infradead.org/faq/ubi.html#L_flash_img

  This must be fixed ... we have no "ubiformat" in u-boot, or?

- changes for v2:
  - do not use spread = 1 for nand_erase_opts, to prevent
    errormessage if there are bad blocks in the erase range.

- changes for v3:
  - add comment from Marek Vasut:
    - prevent losing memory
  - added comment from Lukasz Majewski:
    - move code to dfu_nand.c dfu_flush_medium_nand()

- changes for v4:
  - add comment from Lukasz Majewski:
    - move ubi var to internal struct struct nand_internal_data

- changes for v5:
  - add comment from Scott Wood:
    - don't duplicate code for the "partubi" case

 drivers/dfu/dfu_nand.c | 38 ++++++++++++++++++++++++++++++++++++--
 include/dfu.h          |  2 ++
 2 Dateien ge?ndert, 38 Zeilen hinzugef?gt(+), 2 Zeilen entfernt(-)

diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
index 07dee89..546a4a5 100644
--- a/drivers/dfu/dfu_nand.c
+++ b/drivers/dfu/dfu_nand.c
@@ -148,11 +148,43 @@ static int dfu_read_medium_nand(struct dfu_entity *dfu, u64 offset, void *buf,
 	return ret;
 }
 
+static int dfu_flush_medium_nand(struct dfu_entity *dfu)
+{
+	int ret = 0;
+
+	/* in case of ubi partition, erase rest of the partition */
+	if (dfu->data.nand.ubi) {
+		nand_info_t *nand;
+		nand_erase_options_t opts;
+
+		if (nand_curr_device < 0 ||
+		    nand_curr_device >= CONFIG_SYS_MAX_NAND_DEVICE ||
+		    !nand_info[nand_curr_device].name) {
+			printf("%s: invalid nand device\n", __func__);
+			return -1;
+		}
+
+		nand = &nand_info[nand_curr_device];
+
+		memset(&opts, 0, sizeof(opts));
+		opts.offset = dfu->data.nand.start + dfu->offset +
+				dfu->bad_skip;
+		opts.length = dfu->data.nand.start +
+				dfu->data.nand.size - opts.offset;
+		ret = nand_erase_opts(nand, &opts);
+		if (ret != 0)
+			printf("Failure erase: %d\n", ret);
+	}
+
+	return ret;
+}
+
 int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s)
 {
 	char *st;
 	int ret, dev, part;
 
+	dfu->data.nand.ubi = 0;
 	dfu->dev_type = DFU_DEV_NAND;
 	st = strsep(&s, " ");
 	if (!strcmp(st, "raw")) {
@@ -160,7 +192,7 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s)
 		dfu->data.nand.start = simple_strtoul(s, &s, 16);
 		s++;
 		dfu->data.nand.size = simple_strtoul(s, &s, 16);
-	} else if (!strcmp(st, "part")) {
+	} else if ((!strcmp(st, "part")) || (!strcmp(st, "partubi"))) {
 		char mtd_id[32];
 		struct mtd_device *mtd_dev;
 		u8 part_num;
@@ -185,7 +217,8 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s)
 
 		dfu->data.nand.start = pi->offset;
 		dfu->data.nand.size = pi->size;
-
+		if (!strcmp(st, "partubi"))
+			dfu->data.nand.ubi = 1;
 	} else {
 		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
 		return -1;
@@ -193,6 +226,7 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s)
 
 	dfu->read_medium = dfu_read_medium_nand;
 	dfu->write_medium = dfu_write_medium_nand;
+	dfu->flush_medium = dfu_flush_medium_nand;
 
 	/* initial state */
 	dfu->inited = 0;
diff --git a/include/dfu.h b/include/dfu.h
index 124653c..4de7b34 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -59,6 +59,8 @@ struct nand_internal_data {
 
 	unsigned int dev;
 	unsigned int part;
+	/* for nand/ubi use */
+	unsigned int ubi;
 };
 
 static inline unsigned int get_mmc_blk_size(int dev)
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v5] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-25  4:43       ` [U-Boot] [PATCH v5] dfu, nand, ubi: add partubi alt settings for updating ubi partition Heiko Schocher
@ 2013-07-26 22:05         ` Scott Wood
  0 siblings, 0 replies; 12+ messages in thread
From: Scott Wood @ 2013-07-26 22:05 UTC (permalink / raw)
  To: u-boot

On 07/24/2013 11:43:11 PM, Heiko Schocher wrote:
> diff --git a/include/dfu.h b/include/dfu.h
> index 124653c..4de7b34 100644
> --- a/include/dfu.h
> +++ b/include/dfu.h
> @@ -59,6 +59,8 @@ struct nand_internal_data {
> 
>  	unsigned int dev;
>  	unsigned int part;
> +	/* for nand/ubi use */
> +	unsigned int ubi;
>  };

bool?

In any case, Acked-by: Scott Wood <scottwood@freescale.com>

-Scott

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-07-26 22:05 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-14 10:24 [U-Boot] [PATCH] dfu, nand: before write a buffer to nand, erase the nand sectors Heiko Schocher
2013-06-14 22:19 ` Scott Wood
2013-06-17  5:01 ` [U-Boot] [PATCH v2] " Heiko Schocher
2013-06-18  0:51   ` Scott Wood
2013-06-20  4:36     ` Heiko Schocher
2013-06-20 17:22       ` Scott Wood
2013-06-21  5:09   ` [U-Boot] [PATCH v3] " Heiko Schocher
2013-06-21 22:50     ` Scott Wood
2013-06-24 16:50     ` [U-Boot] [PATCH v4] " Heiko Schocher
2013-06-24 23:26       ` Scott Wood
2013-07-25  4:43       ` [U-Boot] [PATCH v5] dfu, nand, ubi: add partubi alt settings for updating ubi partition Heiko Schocher
2013-07-26 22:05         ` Scott Wood

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox