[U-Boot] [RFC PATCH] dfu: allow get_medium_size() to return bigger medium sizes than 2GiB

Lukasz Majewski l.majewski at samsung.com
Thu Jan 28 15:56:52 CET 2016


Hi Heiko,

> change the get_medium_size() function from
> -       long (*get_medium_size)(struct dfu_entity *dfu);
> +       int (*get_medium_size)(struct dfu_entity *dfu, u64 *size);
> 
> so it can return bigger medium sizes than 2GiB, and the return
> value is seperate from the size.
> 
> Signed-off-by: Heiko Schocher <hs at denx.de>
> 
> ---
> I just have a DDP nand with a size of 4GiB, and the
> mtd partition layout is:
> device nand2 <omap2-nand.0>, # parts = 9
>  #: name                size            offset          mask_flags
>  0: spl                 0x00080000      0x00000000      0
>  1: spl.backup1         0x00080000      0x00080000      0
>  2: spl.backup2         0x00080000      0x00100000      0
>  3: spl.backup3         0x00080000      0x00180000      0
>  4: u-boot              0x00780000      0x00200000      0
>  5: u-boot.env0         0x00200000      0x00980000      0
>  6: u-boot.env1         0x00200000      0x00b80000      0
>  7: mtdoops             0x00200000      0x00d80000      0
>  8: rootfs              0xff080000      0x00f80000      0
> 
> so the last partition is to big for returning the size in a long.
> 
>  drivers/dfu/dfu.c      | 8 ++++----
>  drivers/dfu/dfu_mmc.c  | 8 +++++---
>  drivers/dfu/dfu_nand.c | 5 +++--
>  drivers/dfu/dfu_ram.c  | 5 +++--
>  drivers/dfu/dfu_sf.c   | 5 +++--
>  include/dfu.h          | 4 ++--
>  6 files changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index 8f5915e..daa2eb9 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -335,11 +335,11 @@ int dfu_read(struct dfu_entity *dfu, void *buf,
> int size, int blk_seq_num) if (dfu->i_buf_start == NULL)
>  			return -ENOMEM;
>  
> -		dfu->r_left = dfu->get_medium_size(dfu);
> -		if (dfu->r_left < 0)
> -			return dfu->r_left;
> +		ret = dfu->get_medium_size(dfu, &dfu->r_left);
> +		if (ret < 0)
> +			return ret;
>  
> -		debug("%s: %s %ld [B]\n", __func__, dfu->name,
> dfu->r_left);
> +		debug("%s: %s %lld [B]\n", __func__, dfu->name,
> dfu->r_left); 
>  		dfu->i_blk_seq_num = 0;
>  		dfu->crc = 0;
> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
> index 395d472..5c1c1d1 100644
> --- a/drivers/dfu/dfu_mmc.c
> +++ b/drivers/dfu/dfu_mmc.c
> @@ -205,14 +205,15 @@ int dfu_flush_medium_mmc(struct dfu_entity *dfu)
>  	return ret;
>  }
>  
> -long dfu_get_medium_size_mmc(struct dfu_entity *dfu)
> +int dfu_get_medium_size_mmc(struct dfu_entity *dfu, u64 *size)

The idea to use the return value to get error code and separate pointer
for passing the size is the way to go in my opinion.

The problem is in details. Please find my comments below.

>  {
>  	int ret;
>  	long len;
>  
>  	switch (dfu->layout) {
>  	case DFU_RAW_ADDR:
> -		return dfu->data.mmc.lba_size *
> dfu->data.mmc.lba_blk_size;
> +		*size = dfu->data.mmc.lba_size *
> dfu->data.mmc.lba_blk_size;
> +		return 0;
>  	case DFU_FS_FAT:
>  	case DFU_FS_EXT4:
>  		dfu_file_buf_filled = -1;
> @@ -221,7 +222,8 @@ long dfu_get_medium_size_mmc(struct dfu_entity
> *dfu) return ret;
>  		if (len > CONFIG_SYS_DFU_MAX_FILE_SIZE)
>  			return -1;
> -		return len;
> +		*size = len;
> +		return 0;
>  	default:
>  		printf("%s: Layout (%s) not (yet) supported!\n",
> __func__, dfu_get_layout(dfu->layout));
> diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
> index a975492..4612e09 100644
> --- a/drivers/dfu/dfu_nand.c
> +++ b/drivers/dfu/dfu_nand.c
> @@ -114,9 +114,10 @@ static int dfu_write_medium_nand(struct
> dfu_entity *dfu, return ret;
>  }
>  
> -long dfu_get_medium_size_nand(struct dfu_entity *dfu)
> +int dfu_get_medium_size_nand(struct dfu_entity *dfu, u64 *size)
>  {
> -	return dfu->data.nand.size;
> +	*size = dfu->data.nand.size;
> +	return 0;
>  }
>  
>  static int dfu_read_medium_nand(struct dfu_entity *dfu, u64 offset,
> void *buf, diff --git a/drivers/dfu/dfu_ram.c b/drivers/dfu/dfu_ram.c
> index e094a94..466759d 100644
> --- a/drivers/dfu/dfu_ram.c
> +++ b/drivers/dfu/dfu_ram.c
> @@ -41,9 +41,10 @@ static int dfu_write_medium_ram(struct dfu_entity
> *dfu, u64 offset, return dfu_transfer_medium_ram(DFU_OP_WRITE, dfu,
> offset, buf, len); }
>  
> -long dfu_get_medium_size_ram(struct dfu_entity *dfu)
> +int dfu_get_medium_size_ram(struct dfu_entity *dfu, u64 *size)
>  {
> -	return dfu->data.ram.size;
> +	*size = dfu->data.ram.size;
> +	return 0;
>  }
>  
>  static int dfu_read_medium_ram(struct dfu_entity *dfu, u64 offset,
> diff --git a/drivers/dfu/dfu_sf.c b/drivers/dfu/dfu_sf.c
> index 9702eee..35c5fa1 100644
> --- a/drivers/dfu/dfu_sf.c
> +++ b/drivers/dfu/dfu_sf.c
> @@ -12,9 +12,10 @@
>  #include <spi.h>
>  #include <spi_flash.h>
>  
> -static long dfu_get_medium_size_sf(struct dfu_entity *dfu)
> +int dfu_get_medium_size_sf(struct dfu_entity *dfu, u64 *size)

Originally the dfu_get_medium_size returns int. It is not suitable for
sizes > 2GiB.

I don't like the u64 for *size, since we use that size to perform some
arithmetic operation and comparison (>) in the dfu.c file. I would
prefer to have long long here.

>  {
> -	return dfu->data.sf.size;
> +	*size = dfu->data.sf.size;
> +	return 0;
>  }
>  
>  static int dfu_read_medium_sf(struct dfu_entity *dfu, u64 offset,
> void *buf, diff --git a/include/dfu.h b/include/dfu.h
> index 6118dc2..323b032 100644
> --- a/include/dfu.h
> +++ b/include/dfu.h
> @@ -110,7 +110,7 @@ struct dfu_entity {
>  		struct sf_internal_data sf;
>  	} data;
>  
> -	long (*get_medium_size)(struct dfu_entity *dfu);
> +	int (*get_medium_size)(struct dfu_entity *dfu, u64 *size);
>  
>  	int (*read_medium)(struct dfu_entity *dfu,
>  			u64 offset, void *buf, long *len);
> @@ -132,7 +132,7 @@ struct dfu_entity {
>  	u8 *i_buf;
>  	u8 *i_buf_start;
>  	u8 *i_buf_end;
> -	long r_left;
> +	u64 r_left;

This patch changes r_left to be unsigned, but leaves b_left as signed
(long). 

I think that both b_left and r_left should be promoted to long long.

>  	long b_left;
>  
>  	u32 bad_skip;	/* for nand use */



-- 
Best regards,

Lukasz Majewski

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


More information about the U-Boot mailing list