[U-Boot] [RFC PATCH] dfu: allow get_medium_size() to return bigger medium sizes than 2GiB
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) { 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) { - 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; long b_left;
u32 bad_skip; /* for nand use */
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;@@ -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;__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 */
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@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;@@ -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;__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 */
Hi Heiko,
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@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;@@ -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;__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?
Git blame hints that this code is a reminiscent of first code, which added NAND support to dfu (SHA1: c6631764, year 2013).
It seems like in the past the nand_block_op() has been called with buf = NULL parameter to read the NAND area size.
However, Stephen Warren has added the get_medium_size_nand() function a year later (in 2014) to read the medium size.
Hence it seems to me, that the buf == NULL code in nand_block_op could be safely removed.
As I've check - in contemporary u-boot - there is no call to this function with buf NULL parameter.
bye, Heiko
long b_left;
u32 bad_skip; /* for nand use */
On Thu, Jan 28, 2016 at 8:24 AM, Heiko Schocher hs@denx.de wrote:
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.
to -> too
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)
i64 size? Same for all other places.
{ 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;
You need to check that size is not NULL before dereferencing it. Same for all other places.
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) {
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; long b_left; u32 bad_skip; /* for nand use */-- 2.5.0
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hello Joe,
Am 28.01.2016 um 16:03 schrieb Joe Hershberger:
On Thu, Jan 28, 2016 at 8:24 AM, Heiko Schocher hs@denx.de wrote:
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.
to -> too
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)
i64 size? Same for all other places.
Lukasz mentioned "long long" ...
{ 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;You need to check that size is not NULL before dereferencing it. Same for all other places.
Correct, add it.
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) {
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; long b_left; u32 bad_skip; /* for nand use */-- 2.5.0
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Fri, Jan 29, 2016 at 12:06 AM, Heiko Schocher hs@denx.de wrote:
Hello Joe,
Am 28.01.2016 um 16:03 schrieb Joe Hershberger:
On Thu, Jan 28, 2016 at 8:24 AM, Heiko Schocher hs@denx.de wrote:
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.
to -> too
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)
i64 size? Same for all other places.
Lukasz mentioned "long long" ...
That's OK too.
{ 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;
You need to check that size is not NULL before dereferencing it. Same for all other places.
Correct, add it.
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) {
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; long b_left; u32 bad_skip; /* for nand use */-- 2.5.0
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
-- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
participants (3)
-
Heiko Schocher -
Joe Hershberger -
Lukasz Majewski