[U-Boot] [RFC PATCH] dfu: allow get_medium_size() to return bigger medium sizes than 2GiB
Heiko Schocher
hs at denx.de
Fri Jan 29 06:00:18 CET 2016
Hello Lukasz,
Am 28.01.2016 um 15:56 schrieb Lukasz Majewski:
> 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.
Thats, why I posted it as an RFC, I was unsure too...
> 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.
Ok.
>> {
>> - 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.
Ok, I change it to long long ... but, I have detected another problem:
drivers/dfu/dfu_nand.c:
static int nand_block_op(enum dfu_op op, struct dfu_entity *dfu,
u64 offset, void *buf, long *len)
{
loff_t start, lim;
size_t count, actual;
int ret;
nand_info_t *nand;
/* if buf == NULL return total size of the area */
if (buf == NULL) {
*len = dfu->data.nand.size;
return 0;
}
returns also a long only. nand_block_op gets called in the end from
static int dfu_write_medium_nand(struct dfu_entity *dfu,
u64 offset, void *buf, long *len)
and
static int dfu_read_medium_nand(struct dfu_entity *dfu, u64 offset, void *buf,
long *len)
{
So I have to change also this functions ... Why returns the read/write
operation the length of the complete area, if buf is NULL ? Is this
a DFU feature?
bye,
Heiko
>
>> long b_left;
>>
>> u32 bad_skip; /* for nand use */
>
>
>
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
More information about the U-Boot
mailing list