[U-Boot] [PATCH v3] dfu, nand, ubi: add partubi alt settings for updating ubi partition

[Lukasz Majewski]

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 at denx.de>
> Cc: Pantelis Antoniou <panto at antoniou-consulting.com>
> Cc: Tom Rini <trini at ti.com>
> Cc: Lukasz Majewski <l.majewski at samsung.com>
> Cc: Kyungmin Park <kyungmin.park at samsung.com>
> Cc: Marek Vasut <marex at denx.de>
> Cc: Wolfgang Denk <wd at 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