All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/2] dfu, nand, ubi: add partubi alt settings for updating ubi partition
@ 2013-07-15  9:49 Heiko Schocher
  2013-07-16  4:54 ` [U-Boot] [PATCH v2] " Heiko Schocher
  0 siblings, 1 reply; 24+ messages in thread
From: Heiko Schocher @ 2013-07-15  9:49 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>

---

- 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?
---
 drivers/dfu/dfu.c      | 33 ++++++++++++++++++++++++++++++++-
 drivers/dfu/dfu_nand.c | 26 ++++++++++++++++++++++++++
 include/dfu.h          |  2 ++
 3 Dateien ge?ndert, 60 Zeilen hinzugef?gt(+), 1 Zeile entfernt(-)

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index 0521752..04059be 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -23,6 +23,7 @@
 #include <errno.h>
 #include <malloc.h>
 #include <mmc.h>
+#include <nand.h>
 #include <fat.h>
 #include <dfu.h>
 #include <linux/list.h>
@@ -176,6 +177,37 @@ int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
 			ret = dfu->flush_medium(dfu);
 		printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc);
 
+		/* in case of ubi partition, erase rest of the partition */
+		if (dfu->ubi == 1) {
+			int ret;
+			nand_info_t *nand;
+			/* erase complete partition */
+			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;
+			opts.lim = opts.length;
+			opts.spread = 1;
+			opts.quiet = 0;
+			ret = nand_erase_opts(nand, &opts);
+			if (ret) {
+				printf("Failure erase: %d\n", ret);
+				return ret;
+			}
+		}
+
 		/* clear everything */
 		dfu_free_buf();
 		dfu->crc = 0;
@@ -186,7 +218,6 @@ int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
 		dfu->i_buf = dfu->i_buf_start;
 
 		dfu->inited = 0;
-
 	}
 
 	return ret = 0 ? size : ret;
diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
index 07dee89..d8afc48 100644
--- a/drivers/dfu/dfu_nand.c
+++ b/drivers/dfu/dfu_nand.c
@@ -153,6 +153,7 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s)
 	char *st;
 	int ret, dev, part;
 
+	dfu->ubi = 0;
 	dfu->dev_type = DFU_DEV_NAND;
 	st = strsep(&s, " ");
 	if (!strcmp(st, "raw")) {
@@ -185,7 +186,32 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s)
 
 		dfu->data.nand.start = pi->offset;
 		dfu->data.nand.size = pi->size;
+	} else if (!strcmp(st, "partubi")) {
+		char mtd_id[32];
+		struct mtd_device *mtd_dev;
+		u8 part_num;
+		struct part_info *pi;
+
+		dfu->layout = DFU_RAW_ADDR;
 
+		dev = simple_strtoul(s, &s, 10);
+		s++;
+		part = simple_strtoul(s, &s, 10);
+
+		sprintf(mtd_id, "%s%d,%d", "nand", dev, part - 1);
+		printf("using id '%s'\n", mtd_id);
+
+		mtdparts_init();
+
+		ret = find_dev_and_part(mtd_id, &mtd_dev, &part_num, &pi);
+		if (ret != 0) {
+			printf("Could not locate '%s'\n", mtd_id);
+			return -1;
+		}
+
+		dfu->data.nand.start = pi->offset;
+		dfu->data.nand.size = pi->size;
+		dfu->ubi = 1;
 	} else {
 		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
 		return -1;
diff --git a/include/dfu.h b/include/dfu.h
index 124653c..7bbe42d 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -111,6 +111,8 @@ struct dfu_entity {
 	u32 bad_skip;	/* for nand use */
 
 	unsigned int inited:1;
+	/* for nand/ubi use */
+	unsigned int ubi:1;
 };
 
 int dfu_config_entities(char *s, char *interface, int num);
-- 
1.7.11.7

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

* [U-Boot] [PATCH v2] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-15  9:49 [U-Boot] [PATCH 1/2] dfu, nand, ubi: add partubi alt settings for updating ubi partition Heiko Schocher
@ 2013-07-16  4:54 ` Heiko Schocher
  2013-07-16  5:00   ` Marek Vasut
                     ` (4 more replies)
  0 siblings, 5 replies; 24+ messages in thread
From: Heiko Schocher @ 2013-07-16  4:54 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>

---

- 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.
---
 drivers/dfu/dfu.c      | 30 +++++++++++++++++++++++++++++-
 drivers/dfu/dfu_nand.c | 26 ++++++++++++++++++++++++++
 include/dfu.h          |  2 ++
 3 Dateien ge?ndert, 57 Zeilen hinzugef?gt(+), 1 Zeile entfernt(-)

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index 0521752..7ba7026 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -23,6 +23,7 @@
 #include <errno.h>
 #include <malloc.h>
 #include <mmc.h>
+#include <nand.h>
 #include <fat.h>
 #include <dfu.h>
 #include <linux/list.h>
@@ -176,6 +177,34 @@ int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
 			ret = dfu->flush_medium(dfu);
 		printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc);
 
+		/* in case of ubi partition, erase rest of the partition */
+		if (dfu->ubi == 1) {
+			int ret;
+			nand_info_t *nand;
+			/* erase complete partition */
+			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;
+			}
+		}
+
 		/* clear everything */
 		dfu_free_buf();
 		dfu->crc = 0;
@@ -186,7 +215,6 @@ int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
 		dfu->i_buf = dfu->i_buf_start;
 
 		dfu->inited = 0;
-
 	}
 
 	return ret = 0 ? size : ret;
diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
index 07dee89..d8afc48 100644
--- a/drivers/dfu/dfu_nand.c
+++ b/drivers/dfu/dfu_nand.c
@@ -153,6 +153,7 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s)
 	char *st;
 	int ret, dev, part;
 
+	dfu->ubi = 0;
 	dfu->dev_type = DFU_DEV_NAND;
 	st = strsep(&s, " ");
 	if (!strcmp(st, "raw")) {
@@ -185,7 +186,32 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s)
 
 		dfu->data.nand.start = pi->offset;
 		dfu->data.nand.size = pi->size;
+	} else if (!strcmp(st, "partubi")) {
+		char mtd_id[32];
+		struct mtd_device *mtd_dev;
+		u8 part_num;
+		struct part_info *pi;
+
+		dfu->layout = DFU_RAW_ADDR;
 
+		dev = simple_strtoul(s, &s, 10);
+		s++;
+		part = simple_strtoul(s, &s, 10);
+
+		sprintf(mtd_id, "%s%d,%d", "nand", dev, part - 1);
+		printf("using id '%s'\n", mtd_id);
+
+		mtdparts_init();
+
+		ret = find_dev_and_part(mtd_id, &mtd_dev, &part_num, &pi);
+		if (ret != 0) {
+			printf("Could not locate '%s'\n", mtd_id);
+			return -1;
+		}
+
+		dfu->data.nand.start = pi->offset;
+		dfu->data.nand.size = pi->size;
+		dfu->ubi = 1;
 	} else {
 		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
 		return -1;
diff --git a/include/dfu.h b/include/dfu.h
index 124653c..7bbe42d 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -111,6 +111,8 @@ struct dfu_entity {
 	u32 bad_skip;	/* for nand use */
 
 	unsigned int inited:1;
+	/* for nand/ubi use */
+	unsigned int ubi:1;
 };
 
 int dfu_config_entities(char *s, char *interface, int num);
-- 
1.7.11.7

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

* [U-Boot] [PATCH v2] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-16  4:54 ` [U-Boot] [PATCH v2] " Heiko Schocher
@ 2013-07-16  5:00   ` Marek Vasut
  2013-07-16  5:11     ` Heiko Schocher
  2013-07-16  7:41   ` Lukasz Majewski
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 24+ messages in thread
From: Marek Vasut @ 2013-07-16  5:00 UTC (permalink / raw)
  To: u-boot

Dear Heiko Schocher,

> 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>
> 
> ---
> 
> - 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.
> ---
>  drivers/dfu/dfu.c      | 30 +++++++++++++++++++++++++++++-
>  drivers/dfu/dfu_nand.c | 26 ++++++++++++++++++++++++++
>  include/dfu.h          |  2 ++
>  3 Dateien ge?ndert, 57 Zeilen hinzugef?gt(+), 1 Zeile entfernt(-)
> 
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index 0521752..7ba7026 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -23,6 +23,7 @@
>  #include <errno.h>
>  #include <malloc.h>
>  #include <mmc.h>
> +#include <nand.h>
>  #include <fat.h>
>  #include <dfu.h>
>  #include <linux/list.h>
> @@ -176,6 +177,34 @@ int dfu_write(struct dfu_entity *dfu, void *buf, int
> size, int blk_seq_num) ret = dfu->flush_medium(dfu);
>  		printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc);
> 
> +		/* in case of ubi partition, erase rest of the partition */
> +		if (dfu->ubi == 1) {
> +			int ret;
> +			nand_info_t *nand;
> +			/* erase complete partition */
> +			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;

Are you not leaking memory here ? I suspect you should call dfu_free_buf() as 
below ?

> +			}
> +		}
> +
>  		/* clear everything */
>  		dfu_free_buf();
>  		dfu->crc = 0;
> @@ -186,7 +215,6 @@ int dfu_write(struct dfu_entity *dfu, void *buf, int
> size, int blk_seq_num) dfu->i_buf = dfu->i_buf_start;
> 
>  		dfu->inited = 0;
> -
>  	}
> 
>  	return ret = 0 ? size : ret;
> diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
> index 07dee89..d8afc48 100644
> --- a/drivers/dfu/dfu_nand.c
> +++ b/drivers/dfu/dfu_nand.c
> @@ -153,6 +153,7 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char
> *s) char *st;
>  	int ret, dev, part;
> 
> +	dfu->ubi = 0;
>  	dfu->dev_type = DFU_DEV_NAND;
>  	st = strsep(&s, " ");
>  	if (!strcmp(st, "raw")) {
> @@ -185,7 +186,32 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char
> *s)
> 
>  		dfu->data.nand.start = pi->offset;
>  		dfu->data.nand.size = pi->size;
> +	} else if (!strcmp(st, "partubi")) {
> +		char mtd_id[32];
> +		struct mtd_device *mtd_dev;
> +		u8 part_num;
> +		struct part_info *pi;
> +
> +		dfu->layout = DFU_RAW_ADDR;
> 
> +		dev = simple_strtoul(s, &s, 10);
> +		s++;
> +		part = simple_strtoul(s, &s, 10);
> +
> +		sprintf(mtd_id, "%s%d,%d", "nand", dev, part - 1);
> +		printf("using id '%s'\n", mtd_id);
> +
> +		mtdparts_init();
> +
> +		ret = find_dev_and_part(mtd_id, &mtd_dev, &part_num, &pi);
> +		if (ret != 0) {
> +			printf("Could not locate '%s'\n", mtd_id);
> +			return -1;
> +		}
> +
> +		dfu->data.nand.start = pi->offset;
> +		dfu->data.nand.size = pi->size;
> +		dfu->ubi = 1;
>  	} else {
>  		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
>  		return -1;
> diff --git a/include/dfu.h b/include/dfu.h
> index 124653c..7bbe42d 100644
> --- a/include/dfu.h
> +++ b/include/dfu.h
> @@ -111,6 +111,8 @@ struct dfu_entity {
>  	u32 bad_skip;	/* for nand use */
> 
>  	unsigned int inited:1;
> +	/* for nand/ubi use */
> +	unsigned int ubi:1;
>  };
> 
>  int dfu_config_entities(char *s, char *interface, int num);

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

* [U-Boot] [PATCH v2] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-16  5:00   ` Marek Vasut
@ 2013-07-16  5:11     ` Heiko Schocher
  0 siblings, 0 replies; 24+ messages in thread
From: Heiko Schocher @ 2013-07-16  5:11 UTC (permalink / raw)
  To: u-boot

Hello Marek,

Am 16.07.2013 07:00, schrieb Marek Vasut:
> Dear Heiko Schocher,
>
>> 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>
>>
>> ---
>>
>> - 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.
>> ---
>>   drivers/dfu/dfu.c      | 30 +++++++++++++++++++++++++++++-
>>   drivers/dfu/dfu_nand.c | 26 ++++++++++++++++++++++++++
>>   include/dfu.h          |  2 ++
>>   3 Dateien ge?ndert, 57 Zeilen hinzugef?gt(+), 1 Zeile entfernt(-)
>>
>> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
>> index 0521752..7ba7026 100644
>> --- a/drivers/dfu/dfu.c
>> +++ b/drivers/dfu/dfu.c
>> @@ -23,6 +23,7 @@
>>   #include<errno.h>
>>   #include<malloc.h>
>>   #include<mmc.h>
>> +#include<nand.h>
>>   #include<fat.h>
>>   #include<dfu.h>
>>   #include<linux/list.h>
>> @@ -176,6 +177,34 @@ int dfu_write(struct dfu_entity *dfu, void *buf, int
>> size, int blk_seq_num) ret = dfu->flush_medium(dfu);
>>   		printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc);
>>
>> +		/* in case of ubi partition, erase rest of the partition */
>> +		if (dfu->ubi == 1) {
>> +			int ret;
>> +			nand_info_t *nand;
>> +			/* erase complete partition */
>> +			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;
>
> Are you not leaking memory here ? I suspect you should call dfu_free_buf() as
> below ?

Yes, fixed. Thanks!

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

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

* [U-Boot] [PATCH v2] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-16  4:54 ` [U-Boot] [PATCH v2] " Heiko Schocher
  2013-07-16  5:00   ` Marek Vasut
@ 2013-07-16  7:41   ` Lukasz Majewski
  2013-07-16  8:01     ` Heiko Schocher
  2013-07-16 15:14   ` Tom Rini
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 24+ messages in thread
From: Lukasz Majewski @ 2013-07-16  7:41 UTC (permalink / raw)
  To: u-boot

Hi Heiko,

> 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>
> 
> ---
> 
> - 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.
> ---
>  drivers/dfu/dfu.c      | 30 +++++++++++++++++++++++++++++-
>  drivers/dfu/dfu_nand.c | 26 ++++++++++++++++++++++++++
>  include/dfu.h          |  2 ++
>  3 Dateien ge?ndert, 57 Zeilen hinzugef?gt(+), 1 Zeile entfernt(-)
> 
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index 0521752..7ba7026 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -23,6 +23,7 @@
>  #include <errno.h>
>  #include <malloc.h>
>  #include <mmc.h>
> +#include <nand.h>
>  #include <fat.h>
>  #include <dfu.h>
>  #include <linux/list.h>
> @@ -176,6 +177,34 @@ int dfu_write(struct dfu_entity *dfu, void *buf,
> int size, int blk_seq_num) ret = dfu->flush_medium(dfu);
>  		printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc);
>  
> +		/* in case of ubi partition, erase rest of the
> partition */
> +		if (dfu->ubi == 1) {
> +			int ret;
> +			nand_info_t *nand;
> +			/* erase complete partition */
> +			nand_erase_options_t opts;
> +
> +			if (nand_curr_device < 0 ||
> +			    nand_curr_device >=
> CONFIG_SYS_MAX_NAND_DEVICE ||
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - this config won't be defined at
  devices, which don't have NAND memory (like Trats).
  Due to that Trats board is not building. 

  Please remove memory type dependent code from the common dfu code.


> +			    !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;
> +			}
> +		}
> +
>  		/* clear everything */
>  		dfu_free_buf();
>  		dfu->crc = 0;
> @@ -186,7 +215,6 @@ int dfu_write(struct dfu_entity *dfu, void *buf,
> int size, int blk_seq_num) dfu->i_buf = dfu->i_buf_start;
>  
>  		dfu->inited = 0;
> -
>  	}
>  
>  	return ret = 0 ? size : ret;
> diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
> index 07dee89..d8afc48 100644
> --- a/drivers/dfu/dfu_nand.c
> +++ b/drivers/dfu/dfu_nand.c
> @@ -153,6 +153,7 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu,
> char *s) char *st;
>  	int ret, dev, part;
>  
> +	dfu->ubi = 0;
>  	dfu->dev_type = DFU_DEV_NAND;
>  	st = strsep(&s, " ");
>  	if (!strcmp(st, "raw")) {
> @@ -185,7 +186,32 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu,
> char *s) 
>  		dfu->data.nand.start = pi->offset;
>  		dfu->data.nand.size = pi->size;
> +	} else if (!strcmp(st, "partubi")) {
> +		char mtd_id[32];
> +		struct mtd_device *mtd_dev;
> +		u8 part_num;
> +		struct part_info *pi;
> +
> +		dfu->layout = DFU_RAW_ADDR;
>  
> +		dev = simple_strtoul(s, &s, 10);
> +		s++;
> +		part = simple_strtoul(s, &s, 10);
> +
> +		sprintf(mtd_id, "%s%d,%d", "nand", dev, part - 1);
> +		printf("using id '%s'\n", mtd_id);
> +
> +		mtdparts_init();
> +
> +		ret = find_dev_and_part(mtd_id, &mtd_dev, &part_num,
> &pi);
> +		if (ret != 0) {
> +			printf("Could not locate '%s'\n", mtd_id);
> +			return -1;
> +		}
> +
> +		dfu->data.nand.start = pi->offset;
> +		dfu->data.nand.size = pi->size;
> +		dfu->ubi = 1;
>  	} else {
>  		printf("%s: Memory layout (%s) not supported!\n",
> __func__, st); return -1;
> diff --git a/include/dfu.h b/include/dfu.h
> index 124653c..7bbe42d 100644
> --- a/include/dfu.h
> +++ b/include/dfu.h
> @@ -111,6 +111,8 @@ struct dfu_entity {
>  	u32 bad_skip;	/* for nand use */
>  
>  	unsigned int inited:1;
> +	/* for nand/ubi use */
> +	unsigned int ubi:1;
>  };
>  
>  int dfu_config_entities(char *s, char *interface, int num);



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

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

* [U-Boot] [PATCH v2] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-16  7:41   ` Lukasz Majewski
@ 2013-07-16  8:01     ` Heiko Schocher
  2013-07-16  8:19       ` Lukasz Majewski
  0 siblings, 1 reply; 24+ messages in thread
From: Heiko Schocher @ 2013-07-16  8:01 UTC (permalink / raw)
  To: u-boot

Hello Lukasz,

Am 16.07.2013 09:41, schrieb Lukasz Majewski:
> Hi Heiko,
>
>> 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>
>>
>> ---
>>
>> - 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.
>> ---
>>   drivers/dfu/dfu.c      | 30 +++++++++++++++++++++++++++++-
>>   drivers/dfu/dfu_nand.c | 26 ++++++++++++++++++++++++++
>>   include/dfu.h          |  2 ++
>>   3 Dateien ge?ndert, 57 Zeilen hinzugef?gt(+), 1 Zeile entfernt(-)
>>
>> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
>> index 0521752..7ba7026 100644
>> --- a/drivers/dfu/dfu.c
>> +++ b/drivers/dfu/dfu.c
>> @@ -23,6 +23,7 @@
>>   #include<errno.h>
>>   #include<malloc.h>
>>   #include<mmc.h>
>> +#include<nand.h>
>>   #include<fat.h>
>>   #include<dfu.h>
>>   #include<linux/list.h>
>> @@ -176,6 +177,34 @@ int dfu_write(struct dfu_entity *dfu, void *buf,
>> int size, int blk_seq_num) ret = dfu->flush_medium(dfu);
>>   		printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc);
>>
>> +		/* in case of ubi partition, erase rest of the
>> partition */
>> +		if (dfu->ubi == 1) {
>> +			int ret;
>> +			nand_info_t *nand;
>> +			/* erase complete partition */
>> +			nand_erase_options_t opts;
>> +
>> +			if (nand_curr_device<  0 ||
>> +			    nand_curr_device>=
>> CONFIG_SYS_MAX_NAND_DEVICE ||
>    ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - this config won't be defined at
>    devices, which don't have NAND memory (like Trats).
>    Due to that Trats board is not building.
>
>    Please remove memory type dependent code from the common dfu code.

Oh... Hmm... maybe I move this code to drivers/dfu/dfu_nand.c
and make there a dfu_nand_flush() ?

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

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

* [U-Boot] [PATCH v2] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-16  8:01     ` Heiko Schocher
@ 2013-07-16  8:19       ` Lukasz Majewski
  0 siblings, 0 replies; 24+ messages in thread
From: Lukasz Majewski @ 2013-07-16  8:19 UTC (permalink / raw)
  To: u-boot

Hi Heiko,

> Hello Lukasz,
> 
> Am 16.07.2013 09:41, schrieb Lukasz Majewski:
> > Hi Heiko,
> >
> >> 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>
> >>
> >> ---
> >>
> >> - 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.
> >> ---
> >>   drivers/dfu/dfu.c      | 30 +++++++++++++++++++++++++++++-
> >>   drivers/dfu/dfu_nand.c | 26 ++++++++++++++++++++++++++
> >>   include/dfu.h          |  2 ++
> >>   3 Dateien ge?ndert, 57 Zeilen hinzugef?gt(+), 1 Zeile entfernt(-)
> >>
> >> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> >> index 0521752..7ba7026 100644
> >> --- a/drivers/dfu/dfu.c
> >> +++ b/drivers/dfu/dfu.c
> >> @@ -23,6 +23,7 @@
> >>   #include<errno.h>
> >>   #include<malloc.h>
> >>   #include<mmc.h>
> >> +#include<nand.h>
> >>   #include<fat.h>
> >>   #include<dfu.h>
> >>   #include<linux/list.h>
> >> @@ -176,6 +177,34 @@ int dfu_write(struct dfu_entity *dfu, void
> >> *buf, int size, int blk_seq_num) ret = dfu->flush_medium(dfu);
> >>   		printf("\nDFU complete CRC32: 0x%08x\n",
> >> dfu->crc);
> >>
> >> +		/* in case of ubi partition, erase rest of the
> >> partition */
> >> +		if (dfu->ubi == 1) {
> >> +			int ret;
> >> +			nand_info_t *nand;
> >> +			/* erase complete partition */
> >> +			nand_erase_options_t opts;
> >> +
> >> +			if (nand_curr_device<  0 ||
> >> +			    nand_curr_device>=
> >> CONFIG_SYS_MAX_NAND_DEVICE ||
> >    ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - this config won't be defined at
> >    devices, which don't have NAND memory (like Trats).
> >    Due to that Trats board is not building.
> >
> >    Please remove memory type dependent code from the common dfu
> > code.
> 
> Oh... Hmm... maybe I move this code to drivers/dfu/dfu_nand.c
> and make there a dfu_nand_flush() ?

Sounds like a good idea :-). I would prefer to encapsulate NAND (MMC)
code at dfu_nand.c (dfu_mmc.c) file.


> 
> bye,
> Heiko



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

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

* [U-Boot] [PATCH v2] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-16  4:54 ` [U-Boot] [PATCH v2] " Heiko Schocher
  2013-07-16  5:00   ` Marek Vasut
  2013-07-16  7:41   ` Lukasz Majewski
@ 2013-07-16 15:14   ` Tom Rini
  2013-07-16 15:43     ` Heiko Schocher
  2013-07-17 22:35   ` Scott Wood
  2013-07-18  6:04   ` [U-Boot] [PATCH v3] " Heiko Schocher
  4 siblings, 1 reply; 24+ messages in thread
From: Tom Rini @ 2013-07-16 15:14 UTC (permalink / raw)
  To: u-boot

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/16/2013 12:54 AM, Heiko Schocher wrote:

[snip]
> - 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

So, lets talk.  Have you, or can you, take a guess at how long it
might take to add code to allow us to do this the right way?

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJR5WNhAAoJENk4IS6UOR1WQboP/jY3dOXBg0dADKHlrKRwUKRu
9E1Esp5n6y5M6jNmjNPihSFI0PuLclLMab2ZoByzZn0RGDNq++/G5LK3XfaMAv4H
yPOqarY2UPvirND0DeYZW471C5hPfsDNm9SGgOU8eL05+b/VaCiYGYYyMn6crFnr
dyMfgmDg16Idr/4VNDSW/kGN+FpP+pD8I2zpSMgrzKM+NcAWRe2cBEWLA6tgJYW+
V5oos+8MdbdsBdOPzFhwY9xsW4S7u5nRVPmKna0/F2y4lI497yi2ctPpDyWFyC9O
rQSTD7R/BpSbzb8r3u4tHlPCneR1me/IxzjAXZeCYS9Dj3JpJFJM5rY8xMIBKCd+
b+hbrCmPtkMqTPBAIrQNp8KmNkVrKZEdLnVs45PB7P+ldzBcwMWMZ+jVNVtwb6gw
D+QcyAJhFXu4kViJcBVdEBoRmTxvJ15Sqxdg9rgL+cgcmpFUUdIgdkcTMngXKAw6
Fk9i01cTCPRaQMDKlR4fuHaRThQIZmdWLLKtJ1AIryS4ArJFlH5e7onZElj6opVc
GvRKfBEm1qe9IeYgVoFP9tQmMyK3sVmeGeivWJ1gtEYHZRO3L/48kgcyv84kxDcF
pT3+bx7sxejgVfoQH41Rze7eStrkR8rTy0KoXA5nJX2prV7Nlhwx2SrYnCJT7pPv
1QSW19Jd1R7MnKbbM9/N
=ZXBp
-----END PGP SIGNATURE-----

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

* [U-Boot] [PATCH v2] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-16 15:14   ` Tom Rini
@ 2013-07-16 15:43     ` Heiko Schocher
  0 siblings, 0 replies; 24+ messages in thread
From: Heiko Schocher @ 2013-07-16 15:43 UTC (permalink / raw)
  To: u-boot

Hello Tom,

Am 16.07.2013 17:14, schrieb Tom Rini:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 07/16/2013 12:54 AM, Heiko Schocher wrote:
>
> [snip]
>> - 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
>
> So, lets talk.  Have you, or can you, take a guess at how long it
> might take to add code to allow us to do this the right way?

Good question! My hope was, that this is a result of a discussion here ;-)

First, we should discuss how to handle an update of an ubi partition
on nand with dfu ...

I propose:

a) add a "partubi" for dfu_alt_info as this patch do, so we can use
    "dfu nand 0" as it is for raw nand partitions *and* nand ubi
     partitions
b) add a "ubi format ..." command
    (or can "ubi write" handle writing ubi images created with
     ubinize ? I think no ...)
    best: "ubi format ..." should handle incremental writes, so we
    can use the dfu_buf as it is ... else we must malloc an buffer
    which can hold the hole image -> the board must have enough ram
    for this ...
c) use "ubi format ..." for burning the ubi image into the nand ubi
    partition

a) done with this patch,
b) no real idea how many time it needs ...
c) should be trivial if we have step b)

What Do you think?

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

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

* [U-Boot] [PATCH v2] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-16  4:54 ` [U-Boot] [PATCH v2] " Heiko Schocher
                     ` (2 preceding siblings ...)
  2013-07-16 15:14   ` Tom Rini
@ 2013-07-17 22:35   ` Scott Wood
  2013-07-18  5:01     ` Heiko Schocher
  2013-07-18  6:04   ` [U-Boot] [PATCH v3] " Heiko Schocher
  4 siblings, 1 reply; 24+ messages in thread
From: Scott Wood @ 2013-07-17 22:35 UTC (permalink / raw)
  To: u-boot

On 07/15/2013 11:54:09 PM, Heiko Schocher wrote:
> 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>
> 
> ---
> 
> - 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?

The lack of erase counter preservation is a problem, but the part about  
ECC on erased pages is dealt with in U-Boot by the WITH_DROP_FFS flag.

> - changes for v2:
>   - do not use spread = 1 for nand_erase_opts, to prevent
>     errormessage if there are bad blocks in the erase range.
> ---
>  drivers/dfu/dfu.c      | 30 +++++++++++++++++++++++++++++-
>  drivers/dfu/dfu_nand.c | 26 ++++++++++++++++++++++++++
>  include/dfu.h          |  2 ++
>  3 Dateien ge?ndert, 57 Zeilen hinzugef?gt(+), 1 Zeile entfernt(-)
> 
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index 0521752..7ba7026 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -23,6 +23,7 @@
>  #include <errno.h>
>  #include <malloc.h>
>  #include <mmc.h>
> +#include <nand.h>
>  #include <fat.h>
>  #include <dfu.h>
>  #include <linux/list.h>
> @@ -176,6 +177,34 @@ int dfu_write(struct dfu_entity *dfu, void *buf,  
> int size, int blk_seq_num)
>  			ret = dfu->flush_medium(dfu);
>  		printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc);
> 
> +		/* in case of ubi partition, erase rest of the  
> partition */
> +		if (dfu->ubi == 1) {
> +			int ret;
> +			nand_info_t *nand;
> +			/* erase complete partition */
> +			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;
> +			}
> +		}

Instead of separately erasing the remainder of the partition, how about  
recognizing up front that it's UBI (or that a full partition erase is  
otherwise desired) and erasing the full partition then?  Besides being  
cleaner, it would be easier to convert to an ubi-aware mechanism.

-Scott

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

* [U-Boot] [PATCH v2] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-17 22:35   ` Scott Wood
@ 2013-07-18  5:01     ` Heiko Schocher
  0 siblings, 0 replies; 24+ messages in thread
From: Heiko Schocher @ 2013-07-18  5:01 UTC (permalink / raw)
  To: u-boot

Hello Scott,

Am 18.07.2013 00:35, schrieb Scott Wood:
> On 07/15/2013 11:54:09 PM, Heiko Schocher wrote:
>> 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>
>>
>> ---
>>
>> - 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?
>
> The lack of erase counter preservation is a problem, but the part about ECC on erased pages is dealt with in U-Boot by the WITH_DROP_FFS flag.

Ah, ok!

>> - changes for v2:
>> - do not use spread = 1 for nand_erase_opts, to prevent
>> errormessage if there are bad blocks in the erase range.
>> ---
>> drivers/dfu/dfu.c | 30 +++++++++++++++++++++++++++++-
>> drivers/dfu/dfu_nand.c | 26 ++++++++++++++++++++++++++
>> include/dfu.h | 2 ++
>> 3 Dateien ge?ndert, 57 Zeilen hinzugef?gt(+), 1 Zeile entfernt(-)
>>
>> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
>> index 0521752..7ba7026 100644
>> --- a/drivers/dfu/dfu.c
>> +++ b/drivers/dfu/dfu.c
>> @@ -23,6 +23,7 @@
>> #include <errno.h>
>> #include <malloc.h>
>> #include <mmc.h>
>> +#include <nand.h>
>> #include <fat.h>
>> #include <dfu.h>
>> #include <linux/list.h>
>> @@ -176,6 +177,34 @@ int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
>> ret = dfu->flush_medium(dfu);
>> printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc);
>>
>> + /* in case of ubi partition, erase rest of the partition */
>> + if (dfu->ubi == 1) {
>> + int ret;
>> + nand_info_t *nand;
>> + /* erase complete partition */
>> + 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;
>> + }
>> + }
>
> Instead of separately erasing the remainder of the partition, how about recognizing up front that it's UBI (or that a full partition erase is otherwise desired) and erasing the full partition then? Besides being cleaner, it would be easier to convert to
> an ubi-aware mechanism.

I tried this too, but if I erase the hole partition first (when
a dfu transfer is started) I got:

GADGET DRIVER: usb_dnl_dfu
dfu_write: Wrong sequence number! [7] [0]
dfu_write: Wrong sequence number! [7] [1]
dfu_write: Wrong sequence number! [7] [2]

So, this version needs more investigation ...

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

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

* [U-Boot] [PATCH v3] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-16  4:54 ` [U-Boot] [PATCH v2] " Heiko Schocher
                     ` (3 preceding siblings ...)
  2013-07-17 22:35   ` Scott Wood
@ 2013-07-18  6:04   ` Heiko Schocher
  2013-07-18  6:57     ` Lukasz Majewski
  2013-07-19  4:32     ` [U-Boot] [PATCH v4] " Heiko Schocher
  4 siblings, 2 replies; 24+ messages in thread
From: Heiko Schocher @ 2013-07-18  6:04 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>

---

- 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()
---
 drivers/dfu/dfu_nand.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/dfu.h          |  2 ++
 2 Dateien ge?ndert, 60 Zeilen hinzugef?gt(+)

diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
index 07dee89..ade5ae7 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->ubi == 1) {
+		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->ubi = 0;
 	dfu->dev_type = DFU_DEV_NAND;
 	st = strsep(&s, " ");
 	if (!strcmp(st, "raw")) {
@@ -185,7 +217,32 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s)
 
 		dfu->data.nand.start = pi->offset;
 		dfu->data.nand.size = pi->size;
+	} else if (!strcmp(st, "partubi")) {
+		char mtd_id[32];
+		struct mtd_device *mtd_dev;
+		u8 part_num;
+		struct part_info *pi;
 
+		dfu->layout = DFU_RAW_ADDR;
+
+		dev = simple_strtoul(s, &s, 10);
+		s++;
+		part = simple_strtoul(s, &s, 10);
+
+		sprintf(mtd_id, "%s%d,%d", "nand", dev, part - 1);
+		printf("using id '%s'\n", mtd_id);
+
+		mtdparts_init();
+
+		ret = find_dev_and_part(mtd_id, &mtd_dev, &part_num, &pi);
+		if (ret != 0) {
+			printf("Could not locate '%s'\n", mtd_id);
+			return -1;
+		}
+
+		dfu->data.nand.start = pi->offset;
+		dfu->data.nand.size = pi->size;
+		dfu->ubi = 1;
 	} else {
 		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
 		return -1;
@@ -193,6 +250,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..7bbe42d 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -111,6 +111,8 @@ struct dfu_entity {
 	u32 bad_skip;	/* for nand use */
 
 	unsigned int inited:1;
+	/* for nand/ubi use */
+	unsigned int ubi:1;
 };
 
 int dfu_config_entities(char *s, char *interface, int num);
-- 
1.7.11.7

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

* [U-Boot] [PATCH v3] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-18  6:04   ` [U-Boot] [PATCH v3] " Heiko Schocher
@ 2013-07-18  6:57     ` Lukasz Majewski
  2013-07-22 21:08       ` Scott Wood
  2013-07-19  4:32     ` [U-Boot] [PATCH v4] " Heiko Schocher
  1 sibling, 1 reply; 24+ messages in thread
From: Lukasz Majewski @ 2013-07-18  6:57 UTC (permalink / raw)
  To: u-boot

On Thu, 18 Jul 2013 08:04:55 +0200 Heiko Schocher hs at denx.de wrote,

Hi Heiko,

> 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>
> 
> ---
> 
> - 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()
> ---
>  drivers/dfu/dfu_nand.c | 58
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> include/dfu.h          |  2 ++ 2 Dateien ge?ndert, 60 Zeilen
> hinzugef?gt(+)
> 
> diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
> index 07dee89..ade5ae7 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->ubi == 1) {
	^^^^^^^^^^^^^^^^^^ if (dfu->ubi) shall be enough here.
	

> +		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->ubi = 0;
>  	dfu->dev_type = DFU_DEV_NAND;
>  	st = strsep(&s, " ");
>  	if (!strcmp(st, "raw")) {
> @@ -185,7 +217,32 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu,
> char *s) 
>  		dfu->data.nand.start = pi->offset;
>  		dfu->data.nand.size = pi->size;
> +	} else if (!strcmp(st, "partubi")) {
> +		char mtd_id[32];
> +		struct mtd_device *mtd_dev;
> +		u8 part_num;
> +		struct part_info *pi;
>  
> +		dfu->layout = DFU_RAW_ADDR;
> +
> +		dev = simple_strtoul(s, &s, 10);
> +		s++;
> +		part = simple_strtoul(s, &s, 10);
> +
> +		sprintf(mtd_id, "%s%d,%d", "nand", dev, part - 1);
> +		printf("using id '%s'\n", mtd_id);
> +
> +		mtdparts_init();
> +
> +		ret = find_dev_and_part(mtd_id, &mtd_dev, &part_num,
> &pi);
> +		if (ret != 0) {
> +			printf("Could not locate '%s'\n", mtd_id);
> +			return -1;
> +		}
> +
> +		dfu->data.nand.start = pi->offset;
> +		dfu->data.nand.size = pi->size;
> +		dfu->ubi = 1;
>  	} else {
>  		printf("%s: Memory layout (%s) not supported!\n",
> __func__, st); return -1;
> @@ -193,6 +250,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..7bbe42d 100644
> --- a/include/dfu.h
> +++ b/include/dfu.h
> @@ -111,6 +111,8 @@ struct dfu_entity {
>  	u32 bad_skip;	/* for nand use */
>  
>  	unsigned int inited:1;
> +	/* for nand/ubi use */
> +	unsigned int ubi:1;

Maybe it would be better to add this flag to struct nand_internal_data?
It seems to me like a nand specific.

As a side note, I'm curious how bool ubi would be implemented by the
compiler and if it is equivalent to explicite bit fields.

>  };
>  
>  int dfu_config_entities(char *s, char *interface, int num);



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

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

* [U-Boot] [PATCH v4] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-18  6:04   ` [U-Boot] [PATCH v3] " Heiko Schocher
  2013-07-18  6:57     ` Lukasz Majewski
@ 2013-07-19  4:32     ` Heiko Schocher
  2013-07-19  6:45       ` Lukasz Majewski
                         ` (2 more replies)
  1 sibling, 3 replies; 24+ messages in thread
From: Heiko Schocher @ 2013-07-19  4:32 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>

---

- 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
---
 drivers/dfu/dfu_nand.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/dfu.h          |  2 ++
 2 Dateien ge?ndert, 60 Zeilen hinzugef?gt(+)

diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
index 07dee89..c0f8adf 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")) {
@@ -185,7 +217,32 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s)
 
 		dfu->data.nand.start = pi->offset;
 		dfu->data.nand.size = pi->size;
+	} else if (!strcmp(st, "partubi")) {
+		char mtd_id[32];
+		struct mtd_device *mtd_dev;
+		u8 part_num;
+		struct part_info *pi;
 
+		dfu->layout = DFU_RAW_ADDR;
+
+		dev = simple_strtoul(s, &s, 10);
+		s++;
+		part = simple_strtoul(s, &s, 10);
+
+		sprintf(mtd_id, "%s%d,%d", "nand", dev, part - 1);
+		printf("using id '%s'\n", mtd_id);
+
+		mtdparts_init();
+
+		ret = find_dev_and_part(mtd_id, &mtd_dev, &part_num, &pi);
+		if (ret != 0) {
+			printf("Could not locate '%s'\n", mtd_id);
+			return -1;
+		}
+
+		dfu->data.nand.start = pi->offset;
+		dfu->data.nand.size = pi->size;
+		dfu->data.nand.ubi = 1;
 	} else {
 		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
 		return -1;
@@ -193,6 +250,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] 24+ messages in thread

* [U-Boot] [PATCH v4] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-19  4:32     ` [U-Boot] [PATCH v4] " Heiko Schocher
@ 2013-07-19  6:45       ` Lukasz Majewski
  2013-07-19 13:54         ` Marek Vasut
  2013-07-22 21:24       ` Scott Wood
  2013-07-23 23:22       ` Scott Wood
  2 siblings, 1 reply; 24+ messages in thread
From: Lukasz Majewski @ 2013-07-19  6:45 UTC (permalink / raw)
  To: u-boot

On Fri, 19 Jul 2013 06:32:14 +0200 Heiko Schocher hs at denx.de wrote,

Hi Heiko,

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

I've tested it on Trats. This patch seems to be orthogonal and doesn't
interfere with eMMC.

Tested-by: Lukasz Majewski <l.majewski@samsung.com>
Ack-by: Lukasz Majewski <l.majewski@samsung.com>



> 
> 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>
> 
> ---
> 
> - 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
> ---
>  drivers/dfu/dfu_nand.c | 58
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> include/dfu.h          |  2 ++ 2 Dateien ge?ndert, 60 Zeilen
> hinzugef?gt(+)
> 
> diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
> index 07dee89..c0f8adf 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")) {
> @@ -185,7 +217,32 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu,
> char *s) 
>  		dfu->data.nand.start = pi->offset;
>  		dfu->data.nand.size = pi->size;
> +	} else if (!strcmp(st, "partubi")) {
> +		char mtd_id[32];
> +		struct mtd_device *mtd_dev;
> +		u8 part_num;
> +		struct part_info *pi;
>  
> +		dfu->layout = DFU_RAW_ADDR;
> +
> +		dev = simple_strtoul(s, &s, 10);
> +		s++;
> +		part = simple_strtoul(s, &s, 10);
> +
> +		sprintf(mtd_id, "%s%d,%d", "nand", dev, part - 1);
> +		printf("using id '%s'\n", mtd_id);
> +
> +		mtdparts_init();
> +
> +		ret = find_dev_and_part(mtd_id, &mtd_dev, &part_num,
> &pi);
> +		if (ret != 0) {
> +			printf("Could not locate '%s'\n", mtd_id);
> +			return -1;
> +		}
> +
> +		dfu->data.nand.start = pi->offset;
> +		dfu->data.nand.size = pi->size;
> +		dfu->data.nand.ubi = 1;
>  	} else {
>  		printf("%s: Memory layout (%s) not supported!\n",
> __func__, st); return -1;
> @@ -193,6 +250,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)



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

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

* [U-Boot] [PATCH v4] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-19  6:45       ` Lukasz Majewski
@ 2013-07-19 13:54         ` Marek Vasut
  0 siblings, 0 replies; 24+ messages in thread
From: Marek Vasut @ 2013-07-19 13:54 UTC (permalink / raw)
  To: u-boot

Dear Lukasz Majewski,

> On Fri, 19 Jul 2013 06:32:14 +0200 Heiko Schocher hs at denx.de wrote,
> 
> Hi Heiko,
> 
> > 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.
> 
> I've tested it on Trats. This patch seems to be orthogonal and doesn't
> interfere with eMMC.
> 
> Tested-by: Lukasz Majewski <l.majewski@samsung.com>
> Ack-by: Lukasz Majewski <l.majewski@samsung.com>

I'll wait for Scott's ACK and then pick this, ok?

Best regards,
Marek Vasut

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

* [U-Boot] [PATCH v3] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-18  6:57     ` Lukasz Majewski
@ 2013-07-22 21:08       ` Scott Wood
  0 siblings, 0 replies; 24+ messages in thread
From: Scott Wood @ 2013-07-22 21:08 UTC (permalink / raw)
  To: u-boot

On 07/18/2013 01:57:23 AM, Lukasz Majewski wrote:
> As a side note, I'm curious how bool ubi would be implemented by the
> compiler and if it is equivalent to explicite bit fields.

I believe the compiler implements bool as an 8-bit data type.

-Scott

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

* [U-Boot] [PATCH v4] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-19  4:32     ` [U-Boot] [PATCH v4] " Heiko Schocher
  2013-07-19  6:45       ` Lukasz Majewski
@ 2013-07-22 21:24       ` Scott Wood
  2013-07-23 13:12         ` Heiko Schocher
  2013-07-23 23:22       ` Scott Wood
  2 siblings, 1 reply; 24+ messages in thread
From: Scott Wood @ 2013-07-22 21:24 UTC (permalink / raw)
  To: u-boot

On 07/18/2013 11:32:14 PM, Heiko Schocher wrote:
> 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>
> 
> ---
> 
> - 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?

In the meantime, should you be using WITH_DROP_FFS when "ubi" is set?   
Though we really should be skipping FFs at the end of each block,  
rather than just at the end of the image.

> - 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
> ---
>  drivers/dfu/dfu_nand.c | 58  
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/dfu.h          |  2 ++
>  2 Dateien ge?ndert, 60 Zeilen hinzugef?gt(+)
> 
> diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
> index 07dee89..c0f8adf 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 */

Did you have a chance to investigate the errors you saw when erasing  
the whole partition up front?  If there's other DFU data in the  
partition that needs to be preserved, then replace "partition" with  
"area reserved for this image".

-Scott

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

* [U-Boot] [PATCH v4] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-22 21:24       ` Scott Wood
@ 2013-07-23 13:12         ` Heiko Schocher
  2013-07-23 23:13           ` Scott Wood
  0 siblings, 1 reply; 24+ messages in thread
From: Heiko Schocher @ 2013-07-23 13:12 UTC (permalink / raw)
  To: u-boot

Hello Scott,

Am 22.07.2013 23:24, schrieb Scott Wood:
> On 07/18/2013 11:32:14 PM, Heiko Schocher wrote:
>> 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>
>>
>> ---
>>
>> - 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?
>
> In the meantime, should you be using WITH_DROP_FFS when "ubi" is set? Though we really should be skipping FFs at the end of each block, rather than just at the end of the image.

Yes, but this would be another patch, as this patch did not
touch the nand write part ...!

>> - 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
>> ---
>> drivers/dfu/dfu_nand.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> include/dfu.h | 2 ++
>> 2 Dateien ge?ndert, 60 Zeilen hinzugef?gt(+)
>>
>> diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
>> index 07dee89..c0f8adf 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 */
>
> Did you have a chance to investigate the errors you saw when erasing the whole partition up front? If there's other DFU data in the partition that needs to be preserved, then replace "partition" with "area reserved for this image".

Currently no :-(

But I hope to get the chance to fix this in the correct way, by
introducing an "ubi format ..." command ...

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

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

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

On 07/23/2013 08:12:03 AM, Heiko Schocher wrote:
> Hello Scott,
> 
> Am 22.07.2013 23:24, schrieb Scott Wood:
>> On 07/18/2013 11:32:14 PM, Heiko Schocher wrote:
>>> - 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?
>> 
>> In the meantime, should you be using WITH_DROP_FFS when "ubi" is  
>> set? Though we really should be skipping FFs at the end of each  
>> block, rather than just at the end of the image.
> 
> Yes, but this would be another patch, as this patch did not
> touch the nand write part ...!

It was a response to the comment, not to the patch.

-Scott

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

* [U-Boot] [PATCH v4] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-19  4:32     ` [U-Boot] [PATCH v4] " Heiko Schocher
  2013-07-19  6:45       ` Lukasz Majewski
  2013-07-22 21:24       ` Scott Wood
@ 2013-07-23 23:22       ` Scott Wood
  2013-07-24  4:30         ` Heiko Schocher
  2 siblings, 1 reply; 24+ messages in thread
From: Scott Wood @ 2013-07-23 23:22 UTC (permalink / raw)
  To: u-boot

On 07/18/2013 11:32:14 PM, Heiko Schocher wrote:
> +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;

opts.length is equivalent to dfu->data.nand.size - dfu->offset -  
dfu->bad_skip.  Is this correct?  dfu->data.nand.size includes  
dfu->offset, but dfu->data.nand.start doesn't?

> @@ -185,7 +217,32 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu,  
> char *s)
> 
>  		dfu->data.nand.start = pi->offset;
> +	} else if (!strcmp(st, "partubi")) {
>  		dfu->data.nand.size = pi->size;
> +		char mtd_id[32];
> +		struct mtd_device *mtd_dev;
> +		u8 part_num;
> +		struct part_info *pi;
> 
> +		dfu->layout = DFU_RAW_ADDR;
> +
> +		dev = simple_strtoul(s, &s, 10);
> +		s++;
> +		part = simple_strtoul(s, &s, 10);
> +
> +		sprintf(mtd_id, "%s%d,%d", "nand", dev, part - 1);
> +		printf("using id '%s'\n", mtd_id);
> +
> +		mtdparts_init();
> +
> +		ret = find_dev_and_part(mtd_id, &mtd_dev, &part_num,  
> &pi);
> +		if (ret != 0) {
> +			printf("Could not locate '%s'\n", mtd_id);
> +			return -1;
> +		}
> +
> +		dfu->data.nand.start = pi->offset;
> +		dfu->data.nand.size = pi->size;
> +		dfu->data.nand.ubi = 1;

Please don't duplicate all of this just to set one flag at the end.

-Scott

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

* [U-Boot] [PATCH v4] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-23 23:22       ` Scott Wood
@ 2013-07-24  4:30         ` Heiko Schocher
  2013-07-24 19:08           ` Scott Wood
  0 siblings, 1 reply; 24+ messages in thread
From: Heiko Schocher @ 2013-07-24  4:30 UTC (permalink / raw)
  To: u-boot

Hello Scott,

Am 24.07.2013 01:22, schrieb Scott Wood:
> On 07/18/2013 11:32:14 PM, Heiko Schocher wrote:
>> +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;
>
> opts.length is equivalent to dfu->data.nand.size - dfu->offset - dfu->bad_skip. Is this correct? dfu->data.nand.size includes dfu->offset, but dfu->data.nand.start doesn't?

Yes, it is correct!

I could not parse "dfu->data.nand.size includes dfu->offset, but
dfu->data.nand.start doesn't" ... What do you mean?

I need to calculate "opts-offset" and "opts.length" for nand_erase_opts():
opts.offset contains the starting sector where the erase should begin
      dfu->data.nand.start is equal to the starting sector of the
      partition, so this is needed here + dfu->offset + dfu->bad_skip
      and we are at our starting point.

opts.length contains the count of sectors to erase.
      I just did here end_sector - start_sector with:
      end_sector = dfu->data.nand.start + dfu->data.nand.size
      start_sector = opts.offset

Maybe my calculation of opts.length is confusing, and I should
use "dfu->data.nand.size - dfu->offset - dfu->bad_skip" ?

>> @@ -185,7 +217,32 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s)
>>
>> dfu->data.nand.start = pi->offset;
>> + } else if (!strcmp(st, "partubi")) {
>> dfu->data.nand.size = pi->size;
>> + char mtd_id[32];
>> + struct mtd_device *mtd_dev;
>> + u8 part_num;
>> + struct part_info *pi;
>>
>> + dfu->layout = DFU_RAW_ADDR;
>> +
>> + dev = simple_strtoul(s, &s, 10);
>> + s++;
>> + part = simple_strtoul(s, &s, 10);
>> +
>> + sprintf(mtd_id, "%s%d,%d", "nand", dev, part - 1);
>> + printf("using id '%s'\n", mtd_id);
>> +
>> + mtdparts_init();
>> +
>> + ret = find_dev_and_part(mtd_id, &mtd_dev, &part_num, &pi);
>> + if (ret != 0) {
>> + printf("Could not locate '%s'\n", mtd_id);
>> + return -1;
>> + }
>> +
>> + dfu->data.nand.start = pi->offset;
>> + dfu->data.nand.size = pi->size;
>> + dfu->data.nand.ubi = 1;
>
> Please don't duplicate all of this just to set one flag at the end.

You are right! Fixed!

Thanks!

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

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

* [U-Boot] [PATCH v4] dfu, nand, ubi: add partubi alt settings for updating ubi partition
  2013-07-24  4:30         ` Heiko Schocher
@ 2013-07-24 19:08           ` Scott Wood
  2013-07-25  4:34             ` Heiko Schocher
  0 siblings, 1 reply; 24+ messages in thread
From: Scott Wood @ 2013-07-24 19:08 UTC (permalink / raw)
  To: u-boot

On 07/23/2013 11:30:55 PM, Heiko Schocher wrote:
> Hello Scott,
> 
> Am 24.07.2013 01:22, schrieb Scott Wood:
>> On 07/18/2013 11:32:14 PM, Heiko Schocher wrote:
>>> +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;
>> 
>> opts.length is equivalent to dfu->data.nand.size - dfu->offset -  
>> dfu->bad_skip. Is this correct? dfu->data.nand.size includes  
>> dfu->offset, but dfu->data.nand.start doesn't?
> 
> Yes, it is correct!

No need to shout...

> I could not parse "dfu->data.nand.size includes dfu->offset, but
> dfu->data.nand.start doesn't" ... What do you mean?

I think my confusion was over what dfu->offset and nand.start  
represent.  If nand.start/nand.size describe the raw partition, and  
dfu->offset is the offset into that partition that the image starts at,  
then this looks OK.

-Scott

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

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

Hello Scott,

Am 24.07.2013 21:08, schrieb Scott Wood:
> On 07/23/2013 11:30:55 PM, Heiko Schocher wrote:
>> Hello Scott,
>>
>> Am 24.07.2013 01:22, schrieb Scott Wood:
>>> On 07/18/2013 11:32:14 PM, Heiko Schocher wrote:
>>>> +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;
>>>
>>> opts.length is equivalent to dfu->data.nand.size - dfu->offset - dfu->bad_skip. Is this correct? dfu->data.nand.size includes dfu->offset, but dfu->data.nand.start doesn't?
>>
>> Yes, it is correct!
>
> No need to shout...

Sorry, I did not intent to shout ...

>> I could not parse "dfu->data.nand.size includes dfu->offset, but
>> dfu->data.nand.start doesn't" ... What do you mean?
>
> I think my confusion was over what dfu->offset and nand.start represent. If nand.start/nand.size describe the raw partition, and dfu->offset is the offset into that partition that the image starts at, then this looks OK.

Ok, I send an update of this patch for your other comment, thanks.

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

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

end of thread, other threads:[~2013-07-25  4:34 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-15  9:49 [U-Boot] [PATCH 1/2] dfu, nand, ubi: add partubi alt settings for updating ubi partition Heiko Schocher
2013-07-16  4:54 ` [U-Boot] [PATCH v2] " Heiko Schocher
2013-07-16  5:00   ` Marek Vasut
2013-07-16  5:11     ` Heiko Schocher
2013-07-16  7:41   ` Lukasz Majewski
2013-07-16  8:01     ` Heiko Schocher
2013-07-16  8:19       ` Lukasz Majewski
2013-07-16 15:14   ` Tom Rini
2013-07-16 15:43     ` Heiko Schocher
2013-07-17 22:35   ` Scott Wood
2013-07-18  5:01     ` Heiko Schocher
2013-07-18  6:04   ` [U-Boot] [PATCH v3] " Heiko Schocher
2013-07-18  6:57     ` Lukasz Majewski
2013-07-22 21:08       ` Scott Wood
2013-07-19  4:32     ` [U-Boot] [PATCH v4] " Heiko Schocher
2013-07-19  6:45       ` Lukasz Majewski
2013-07-19 13:54         ` Marek Vasut
2013-07-22 21:24       ` Scott Wood
2013-07-23 13:12         ` Heiko Schocher
2013-07-23 23:13           ` Scott Wood
2013-07-23 23:22       ` Scott Wood
2013-07-24  4:30         ` Heiko Schocher
2013-07-24 19:08           ` Scott Wood
2013-07-25  4:34             ` Heiko Schocher

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.