From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Fri, 29 Jan 2016 06:00:18 +0100 Subject: [U-Boot] [RFC PATCH] dfu: allow get_medium_size() to return bigger medium sizes than 2GiB In-Reply-To: <20160128155652.3279ad8a@amdc2363> References: <1453991070-29369-1-git-send-email-hs@denx.de> <20160128155652.3279ad8a@amdc2363> Message-ID: <56AAF1E2.2000505@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >> >> --- >> I just have a DDP nand with a size of 4GiB, and the >> mtd partition layout is: >> device nand2 , # 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 >> #include >> >> -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