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

Lukasz Majewski l.majewski at samsung.com
Tue Feb 2 14:08:59 CET 2016


Hi Heiko,

> Hello Lukasz,
> 
> Am 02.02.2016 um 12:51 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, long long
> >> *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 last
> >> 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.
> >>
> >> - nand/mtd dfu modules tested only, please verify mmc, sf
> >>
> >>
> >> Changes in v2:
> >> - add comments from Luaksz:
> >>    use long long instead u64 (also recommended from Joe)
> >>    change b_left also into long long ->
> >>    read_medium and write_medium needs also adaptions
> >> - add comments from Joe
> >>    check if size is not NULL, before dereferencing it
> >>
> >>   drivers/dfu/dfu.c      | 12 ++++++------
> >>   drivers/dfu/dfu_mmc.c  | 25 ++++++++++++++++---------
> >>   drivers/dfu/dfu_nand.c | 18 +++++++++++-------
> >>   drivers/dfu/dfu_ram.c  | 14 +++++++++-----
> >>   drivers/dfu/dfu_sf.c   | 19 +++++++++++++------
> >>   include/dfu.h          | 10 +++++-----
> >>   6 files changed, 60 insertions(+), 38 deletions(-)
> >>
> >> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> >> index 8f5915e..873dad5 100644
> >> --- a/drivers/dfu/dfu.c
> >> +++ b/drivers/dfu/dfu.c
> >> @@ -134,7 +134,7 @@ static char *dfu_get_hash_algo(void)
> >>
> >>   static int dfu_write_buffer_drain(struct dfu_entity *dfu)
> >>   {
> >> -	long w_size;
> >> +	long long w_size;
> >>   	int ret;
> >>
> >>   	/* flush size? */
> >> @@ -279,7 +279,7 @@ static int dfu_read_buffer_fill(struct
> >> dfu_entity *dfu, void *buf, int size) readn = 0;
> >>   	while (size > 0) {
> >>   		/* get chunk that can be read */
> >> -		chunk = min((long)size, dfu->b_left);
> >> +		chunk = min((long long)size, dfu->b_left);
> >>   		/* consume */
> >>   		if (chunk > 0) {
> >>   			memcpy(buf, dfu->i_buf, chunk);
> >> @@ -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..e256664 100644
> >> --- a/drivers/dfu/dfu_mmc.c
> >> +++ b/drivers/dfu/dfu_mmc.c
> >> @@ -169,17 +169,18 @@ static int mmc_file_op(enum dfu_op op, struct
> >> dfu_entity *dfu, }
> >>
> >>   int dfu_write_medium_mmc(struct dfu_entity *dfu,
> >> -		u64 offset, void *buf, long *len)
> >> +		u64 offset, void *buf, long long *len)
> >>   {
> >>   	int ret = -1;
> >> +	long len_long = *len;
> >>
> >>   	switch (dfu->layout) {
> >>   	case DFU_RAW_ADDR:
> >> -		ret = mmc_block_op(DFU_OP_WRITE, dfu, offset, buf,
> >> len);
> >> +		ret = mmc_block_op(DFU_OP_WRITE, dfu, offset, buf,
> >> &len_long); break;
> >>   	case DFU_FS_FAT:
> >>   	case DFU_FS_EXT4:
> >> -		ret = mmc_file_buffer(dfu, buf, len);
> >> +		ret = mmc_file_buffer(dfu, buf, &len_long);
> >>   		break;
> >>   	default:
> >>   		printf("%s: Layout (%s) not (yet) supported!\n",
> >> __func__, @@ -205,14 +206,17 @@ 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, long long
> >> *size) {
> >>   	int ret;
> >>   	long len;
> >>
> >> +	if (!size)
> >> +		return -EFAULT;
> >> +
> >>   	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; case DFU_FS_FAT:
> >>   	case DFU_FS_EXT4:
> >>   		dfu_file_buf_filled = -1;
> >> @@ -221,12 +225,14 @@ 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;
> >>   	default:
> >>   		printf("%s: Layout (%s) not (yet) supported!\n",
> >> __func__, dfu_get_layout(dfu->layout));
> >>   		return -1;
> >>   	}
> >> +
> >> +	return 0;
> >>   }
> >>
> >>   static int mmc_file_unbuffer(struct dfu_entity *dfu, u64 offset,
> >> void *buf, @@ -251,17 +257,18 @@ static int
> >> mmc_file_unbuffer(struct dfu_entity *dfu, u64 offset, void *buf, }
> >>
> >>   int dfu_read_medium_mmc(struct dfu_entity *dfu, u64 offset, void
> >> *buf,
> >> -		long *len)
> >> +		long long *len)
> >>   {
> >>   	int ret = -1;
> >> +	long len_long = *len;
> >>
> >>   	switch (dfu->layout) {
> >>   	case DFU_RAW_ADDR:
> >> -		ret = mmc_block_op(DFU_OP_READ, dfu, offset, buf,
> >> len);
> >> +		ret = mmc_block_op(DFU_OP_READ, dfu, offset, buf,
> >> &len_long); break;
> >>   	case DFU_FS_FAT:
> >>   	case DFU_FS_EXT4:
> >> -		ret = mmc_file_unbuffer(dfu, offset, buf, len);
> >> +		ret = mmc_file_unbuffer(dfu, offset, buf,
> >> &len_long); break;
> >>   	default:
> >>   		printf("%s: Layout (%s) not (yet) supported!\n",
> >> __func__, diff --git a/drivers/dfu/dfu_nand.c
> >> b/drivers/dfu/dfu_nand.c index a975492..da2278e 100644
> >> --- a/drivers/dfu/dfu_nand.c
> >> +++ b/drivers/dfu/dfu_nand.c
> >> @@ -20,7 +20,7 @@
> >>   #include <nand.h>
> >>
> >>   static int nand_block_op(enum dfu_op op, struct dfu_entity *dfu,
> >> -			u64 offset, void *buf, long *len)
> >> +			u64 offset, void *buf, long long *len)
> >>   {
> >>   	loff_t start, lim;
> >>   	size_t count, actual;
> >> @@ -86,19 +86,19 @@ static int nand_block_op(enum dfu_op op, struct
> >> dfu_entity *dfu, }
> >>
> >>   static inline int nand_block_write(struct dfu_entity *dfu,
> >> -		u64 offset, void *buf, long *len)
> >> +		u64 offset, void *buf, long long *len)
> >>   {
> >>   	return nand_block_op(DFU_OP_WRITE, dfu, offset, buf,
> >> len); }
> >>
> >>   static inline int nand_block_read(struct dfu_entity *dfu,
> >> -		u64 offset, void *buf, long *len)
> >> +		u64 offset, void *buf, long long *len)
> >>   {
> >>   	return nand_block_op(DFU_OP_READ, dfu, offset, buf, len);
> >>   }
> >>
> >>   static int dfu_write_medium_nand(struct dfu_entity *dfu,
> >> -		u64 offset, void *buf, long *len)
> >> +		u64 offset, void *buf, long long *len)
> >>   {
> >>   	int ret = -1;
> >>
> >> @@ -114,13 +114,17 @@ 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, long long
> >> *size) {
> >> -	return dfu->data.nand.size;
> >> +	if (!size)
> >> +		return -EFAULT;
> >> +
> >> +	*size = dfu->data.nand.size;
> >> +	return 0;
> >>   }
> >>
> >>   static int dfu_read_medium_nand(struct dfu_entity *dfu, u64
> >> offset, void *buf,
> >> -		long *len)
> >> +		long long *len)
> >>   {
> >>   	int ret = -1;
> >>
> >> diff --git a/drivers/dfu/dfu_ram.c b/drivers/dfu/dfu_ram.c
> >> index e094a94..e63c682 100644
> >> --- a/drivers/dfu/dfu_ram.c
> >> +++ b/drivers/dfu/dfu_ram.c
> >> @@ -15,7 +15,7 @@
> >>   #include <dfu.h>
> >>
> >>   static int dfu_transfer_medium_ram(enum dfu_op op, struct
> >> dfu_entity *dfu,
> >> -				   u64 offset, void *buf, long
> >> *len)
> >> +				   u64 offset, void *buf, long
> >> long *len) {
> >>   	if (dfu->layout != DFU_RAM_ADDR) {
> >>   		error("unsupported layout: %s\n",
> >> dfu_get_layout(dfu->layout)); @@ -36,18 +36,22 @@ static int
> >> dfu_transfer_medium_ram(enum dfu_op op, struct dfu_entity *dfu, }
> >>
> >>   static int dfu_write_medium_ram(struct dfu_entity *dfu, u64
> >> offset,
> >> -				void *buf, long *len)
> >> +				void *buf, long long *len)
> >>   {
> >>   	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, long long
> >> *size) {
> >> -	return dfu->data.ram.size;
> >> +	if (!size)
> >> +		return -EFAULT;
> >> +
> >> +	*size = dfu->data.ram.size;
> >> +	return 0;
> >>   }
> >>
> >>   static int dfu_read_medium_ram(struct dfu_entity *dfu, u64
> >> offset,
> >> -			       void *buf, long *len)
> >> +			       void *buf, long long *len)
> >>   {
> >>   	return dfu_transfer_medium_ram(DFU_OP_READ, dfu, offset,
> >> buf, len); }
> >> diff --git a/drivers/dfu/dfu_sf.c b/drivers/dfu/dfu_sf.c
> >> index 9702eee..8ed565d 100644
> >> --- a/drivers/dfu/dfu_sf.c
> >> +++ b/drivers/dfu/dfu_sf.c
> >> @@ -12,15 +12,21 @@
> >>   #include <spi.h>
> >>   #include <spi_flash.h>
> >>
> >> -static long dfu_get_medium_size_sf(struct dfu_entity *dfu)
> >> +static int dfu_get_medium_size_sf(struct dfu_entity *dfu, long
> >> long *size) {
> >> -	return dfu->data.sf.size;
> >> +	if (!size)
> >> +		return -EFAULT;
> >> +
> >> +	*size = dfu->data.sf.size;
> >> +	return 0;
> >>   }
> >>
> >>   static int dfu_read_medium_sf(struct dfu_entity *dfu, u64 offset,
> >> void *buf,
> >> -		long *len)
> >> +		long long *len)
> >>   {
> >> -	return spi_flash_read(dfu->data.sf.dev, offset, *len,
> >> buf);
> >> +	size_t size = *len;
> >> +
> >> +	return spi_flash_read(dfu->data.sf.dev, offset, size,
> >> buf); }
> >>
> >>   static u64 find_sector(struct dfu_entity *dfu, u64 start, u64
> >> offset) @@ -30,9 +36,10 @@ static u64 find_sector(struct
> >> dfu_entity *dfu, u64 start, u64 offset) }
> >>
> >>   static int dfu_write_medium_sf(struct dfu_entity *dfu,
> >> -		u64 offset, void *buf, long *len)
> >> +		u64 offset, void *buf, long long *len)
> >>   {
> >>   	int ret;
> >> +	size_t size = *len;
> >>
> >>   	ret = spi_flash_erase(dfu->data.sf.dev,
> >>   			      find_sector(dfu,
> >> dfu->data.sf.start, offset), @@ -41,7 +48,7 @@ static int
> >> dfu_write_medium_sf(struct dfu_entity *dfu, return ret;
> >>
> >>   	ret = spi_flash_write(dfu->data.sf.dev,
> >> dfu->data.sf.start + offset,
> >> -			      *len, buf);
> >> +			      size, buf);
> >>   	if (ret)
> >>   		return ret;
> >>
> >> diff --git a/include/dfu.h b/include/dfu.h
> >> index 6118dc2..c327bb5 100644
> >> --- a/include/dfu.h
> >> +++ b/include/dfu.h
> >> @@ -110,13 +110,13 @@ 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, long long
> >> *size);
> >>   	int (*read_medium)(struct dfu_entity *dfu,
> >> -			u64 offset, void *buf, long *len);
> >> +			u64 offset, void *buf, long long *len);
> >>
> >>   	int (*write_medium)(struct dfu_entity *dfu,
> >> -			u64 offset, void *buf, long *len);
> >> +			u64 offset, void *buf, long long *len);
> >>
> >>   	int (*flush_medium)(struct dfu_entity *dfu);
> >>   	unsigned int (*poll_timeout)(struct dfu_entity *dfu);
> >> @@ -132,8 +132,8 @@ struct dfu_entity {
> >>   	u8 *i_buf;
> >>   	u8 *i_buf_start;
> >>   	u8 *i_buf_end;
> >> -	long r_left;
> >> -	long b_left;
> >> +	long long r_left;
> >> +	long long b_left;
> >>
> >>   	u32 bad_skip;	/* for nand use */
> >>
> >
> > This patch seems correct, but unfortunately it introduces DFU
> > regression on Trats (Exynos 4210) and Odroid XU3 (Exynos 5433)
> > boards.
> >
> > Those boards are equipped with eMMC.
> > I've used ./tests/dfu/dfu_gadget_test.sh script.
> >
> > Could you check if your patch works on device equipped with eMMC?
> 
> Thats, why I posted it as an RFC ... I can test it only on NAND.

I will try to find some time by the end of the week and investigate this
issue on my boards.

> 
> bye,
> Heiko
> >
> >
> > Please find output from the test system:
> >
> >
> > ODROID-XU3 #
> >
> > ODROID-XU3 # setenv dfu_alt_system 'dfu_test.bin fat 0
> > 5;dfudummy.bin fat 0 5'
> >
> > ODROID-XU3 # dfu 0 mmc 0
> >
> > DFU alt info setting: done
> >
> > Init script for generating data necessary for DFU test script OK
> > =========================================================================================
> > DFU EP0 transmission test program
> > Trouble shoot -> disable DBG (even the KERN_DEBUG) in the UDC driver
> > @ -> TRATS2 # dfu 0 mmc 0
> >   =========================================================================================
> > File: ./dat_1048575.img
> > TX: md5sum:51ed36ddc1231ab675e647daa6b629bb
> >      FAILED
> > + ret=1
> > + power_down_board_gpio.sh
> >
> >
> >
> 



-- 
Best regards,

Lukasz Majewski

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


More information about the U-Boot mailing list