[U-Boot] [PATCH 3/3] cmd_ubi: add write.part command, to write a volume in multiple parts

[Stefan Roese <sr@denx.de>]

Hi Paul,

On 06.08.2013 12:13, Paul Burton wrote:
> This allows you to write data to an UBI volume when the amount of memory
> available to write that data from is less than the total size of the
> data. For example, you may split a root filesystem UBIFS image into
> parts, provide the total size of the image to the first write.part
> command and then use multiple write.part commands to write the
> subsequent parts of the volume. This results in a sequence of commands
> akin to:
> 
>   ext4load mmc 0:1 0x80000000 rootfs.ubifs.0
>   ubi write.part 0x80000000 root 0x08000000 0x18000000
>   ext4load mmc 0:1 0x80000000 rootfs.ubifs.1
>   ubi write.part 0x80000000 root 0x08000000
>   ext4load mmc 0:1 0x80000000 rootfs.ubifs.2
>   ubi write.part 0x80000000 root 0x08000000
> 
> This would write 384MiB of data to the UBI volume 'root' whilst only
> requiring 128MiB of said data to be held in memory at a time.

Some coding-style (nitpicking) comments below.

> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> ---
>  common/cmd_ubi.c | 62 ++++++++++++++++++++++++++++++++++++++++++--------------
>  doc/README.ubi   |  3 +++
>  2 files changed, 50 insertions(+), 15 deletions(-)
> 
> diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c
> index 5ba4feb..dadb27b 100644
> --- a/common/cmd_ubi.c
> +++ b/common/cmd_ubi.c
> @@ -266,28 +266,15 @@ out_err:
>  	return err;
>  }
>  
> -int ubi_volume_write(char *volume, void *buf, size_t size)
> +int ubi_volume_continue_write(char *volume, void *buf, size_t size)
>  {
>  	int err = 1;
> -	int rsvd_bytes = 0;
>  	struct ubi_volume *vol;
>  
>  	vol = ubi_find_volume(volume);
>  	if (vol == NULL)
>  		return ENODEV;
>  
> -	rsvd_bytes = vol->reserved_pebs * (ubi->leb_size - vol->data_pad);
> -	if (size < 0 || size > rsvd_bytes) {
> -		printf("size > volume size! Aborting!\n");
> -		return EINVAL;
> -	}
> -
> -	err = ubi_start_update(ubi, vol, size);
> -	if (err < 0) {
> -		printf("Cannot start volume update\n");
> -		return -err;
> -	}
> -
>  	err = ubi_more_update_data(ubi, vol, buf, size);
>  	if (err < 0) {
>  		printf("Couldnt or partially wrote data\n");
> @@ -314,6 +301,37 @@ int ubi_volume_write(char *volume, void *buf, size_t size)
>  	return 0;
>  }
>  
> +int ubi_volume_begin_write(char *volume, void *buf, size_t size,
> +	size_t full_size)
> +{
> +	int err = 1;
> +	int rsvd_bytes = 0;
> +	struct ubi_volume *vol;
> +
> +	vol = ubi_find_volume(volume);
> +	if (vol == NULL)
> +		return ENODEV;
> +
> +	rsvd_bytes = vol->reserved_pebs * (ubi->leb_size - vol->data_pad);
> +	if (size < 0 || size > rsvd_bytes) {
> +		printf("size > volume size! Aborting!\n");
> +		return EINVAL;
> +	}
> +
> +	err = ubi_start_update(ubi, vol, full_size);
> +	if (err < 0) {
> +		printf("Cannot start volume update\n");
> +		return -err;
> +	}
> +
> +	return ubi_volume_continue_write(volume, buf, size);
> +}
> +
> +int ubi_volume_write(char *volume, void *buf, size_t size)
> +{
> +	return ubi_volume_begin_write(volume, buf, size, size);
> +}
> +
>  int ubi_volume_read(char *volume, char *buf, size_t size)
>  {
>  	int err, lnum, off, len, tbuf_size;
> @@ -588,7 +606,19 @@ static int do_ubi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  		addr = simple_strtoul(argv[2], NULL, 16);
>  		size = simple_strtoul(argv[4], NULL, 16);
>  
> -		ret = ubi_volume_write(argv[3], (void *)addr, size);
> +		if (strlen(argv[1]) == 10 &&
> +		    strncmp(argv[1] + 5, ".part", 5) == 0) {
> +			if (argc < 6)
> +				ret = ubi_volume_continue_write(argv[3],
> +						(void *)addr, size);

Please use braces for multi-line statements.

> +			else {
> +				size_t full_size;
> +				full_size = simple_strtoul(argv[5], NULL, 16);
> +				ret = ubi_volume_begin_write(argv[3],
> +						(void *)addr, size, full_size);
> +			}

Especially when the other branch also uses braces.

> +		} else
> +			ret = ubi_volume_write(argv[3], (void *)addr, size);

Here again, please braces since the other branch also uses them.

Thanks,
Stefan