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

Lukasz Majewski
2016-01-28T14:56:52Z

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