[U-Boot] [PATCH 0/9] USB: Gadget & DFU related fixes (v3)

[U-Boot] [PATCH 0/9] USB: Gadget & DFU related fixes (v3)

[Pantelis Antoniou]

Third take of USB + DFU updates.

Pantelis Antoniou (9):
  usb: Fix bug when both DFU & ETHER are defined
  g_dnl: Issue connect/disconnect as appropriate
  g_dnl: Properly terminate string list.
  dfu: Only perform DFU board_usb_init() for TRATS
  dfu: Fix crash when wrong number of arguments given
  dfu: Send correct DFU response from composite_setup
  dfu: Properly zero out timeout value
  dfu: Add a partition type target
  dfu: Support larger than memory transfers.

 common/cmd_dfu.c                    |   5 +-
 drivers/dfu/dfu.c                   | 244 ++++++++++++++++++++++++++++--------
 drivers/dfu/dfu_mmc.c               | 109 ++++++++++++----
 drivers/usb/gadget/Makefile         |   8 +-
 drivers/usb/gadget/composite.c      |  27 ++++
 drivers/usb/gadget/ep0.c            |   1 +
 drivers/usb/gadget/f_dfu.c          |   9 +-
 drivers/usb/gadget/g_dnl.c          |  12 +-
 include/configs/am335x_evm.h        |   1 +
 include/configs/am3517_evm.h        |   1 +
 include/configs/h2200.h             |   1 +
 include/configs/omap3_beagle.h      |   1 +
 include/configs/s5p_goni.h          |   1 +
 include/configs/s5pc210_universal.h |   1 +
 include/configs/trats.h             |   1 +
 include/dfu.h                       |  21 +++-
 16 files changed, 356 insertions(+), 87 deletions(-)

-- 
1.7.12

[U-Boot] [PATCH 1/9] usb: Fix bug when both DFU & ETHER are defined

[Pantelis Antoniou]

When both CONFIG_USB_GADGET & CONFIG_USB_ETHER are defined
the makefile links objects twice.

The cleanest way to fix is to use a new define, CONFIG_USB_UTIL
which must be defined when either CONFIG_USB_ETHER or
CONFIG_USB_GADGET are defined.

All affected boards have been modified as well.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/usb/gadget/Makefile         | 8 ++++++--
 include/configs/am335x_evm.h        | 1 +
 include/configs/am3517_evm.h        | 1 +
 include/configs/h2200.h             | 1 +
 include/configs/omap3_beagle.h      | 1 +
 include/configs/s5p_goni.h          | 1 +
 include/configs/s5pc210_universal.h | 1 +
 include/configs/trats.h             | 1 +
 8 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index 040eaba..167f24f 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -25,15 +25,19 @@ include $(TOPDIR)/config.mk
 
 LIB	:= $(obj)libusb_gadget.o
 
+# required for both USB_GADGET & USB_ETHER
+ifdef CONFIG_USB_UTIL
+COBJS-y += epautoconf.o config.o usbstring.o
+endif
+
 # new USB gadget layer dependencies
 ifdef CONFIG_USB_GADGET
-COBJS-y += epautoconf.o config.o usbstring.o
 COBJS-$(CONFIG_USB_GADGET_S3C_UDC_OTG) += s3c_udc_otg.o
 COBJS-$(CONFIG_USBDOWNLOAD_GADGET) += g_dnl.o
 COBJS-$(CONFIG_DFU_FUNCTION) += f_dfu.o
 endif
 ifdef CONFIG_USB_ETHER
-COBJS-y += ether.o epautoconf.o config.o usbstring.o
+COBJS-y += ether.o
 COBJS-$(CONFIG_USB_ETH_RNDIS) += rndis.o
 COBJS-$(CONFIG_MV_UDC)	+= mv_udc.o
 COBJS-$(CONFIG_CPU_PXA25X) += pxa25x_udc.o
diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
index ab9549b..ee19e54 100644
--- a/include/configs/am335x_evm.h
+++ b/include/configs/am335x_evm.h
@@ -278,6 +278,7 @@
 
 #ifdef CONFIG_MUSB_GADGET
 #define CONFIG_USB_ETHER
+#define CONFIG_USB_UTIL
 #define CONFIG_USB_ETH_RNDIS
 #endif /* CONFIG_MUSB_GADGET */
 
diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h
index ba15325..3aedff3 100644
--- a/include/configs/am3517_evm.h
+++ b/include/configs/am3517_evm.h
@@ -123,6 +123,7 @@
 #ifdef CONFIG_MUSB_GADGET
 #define CONFIG_USB_GADGET_DUALSPEED
 #define CONFIG_USB_ETHER
+#define CONFIG_USB_UTIL
 #define CONFIG_USB_ETH_RNDIS
 #endif /* CONFIG_MUSB_GADGET */
 
diff --git a/include/configs/h2200.h b/include/configs/h2200.h
index 516a26e..fc27bf0 100644
--- a/include/configs/h2200.h
+++ b/include/configs/h2200.h
@@ -170,6 +170,7 @@
 
 #define CONFIG_USB_GADGET_PXA2XX
 #define CONFIG_USB_ETHER
+#define CONFIG_USB_UTIL
 #define CONFIG_USB_ETH_SUBSET
 
 #define CONFIG_USBNET_DEV_ADDR		"de:ad:be:ef:00:01"
diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
index 12d65f2..04fbb5d 100644
--- a/include/configs/omap3_beagle.h
+++ b/include/configs/omap3_beagle.h
@@ -123,6 +123,7 @@
 #define CONFIG_USB_GADGET_DUALSPEED
 #define CONFIG_TWL4030_USB		1
 #define CONFIG_USB_ETHER
+#define CONFIG_USB_UTIL
 #define CONFIG_USB_ETHER_RNDIS
 
 /* USB EHCI */
diff --git a/include/configs/s5p_goni.h b/include/configs/s5p_goni.h
index 56e8347..1e180ade 100644
--- a/include/configs/s5p_goni.h
+++ b/include/configs/s5p_goni.h
@@ -231,6 +231,7 @@
 #define CONFIG_I2C_MULTI_BUS
 #define CONFIG_SYS_MAX_I2C_BUS	7
 #define CONFIG_USB_GADGET
+#define CONFIG_USB_UTIL
 #define CONFIG_USB_GADGET_S3C_UDC_OTG
 #define CONFIG_USB_GADGET_DUALSPEED
 
diff --git a/include/configs/s5pc210_universal.h b/include/configs/s5pc210_universal.h
index 894f38b..07ab884 100644
--- a/include/configs/s5pc210_universal.h
+++ b/include/configs/s5pc210_universal.h
@@ -258,6 +258,7 @@
 #define CONFIG_POWER_MAX8998
 
 #define CONFIG_USB_GADGET
+#define CONFIG_USB_UTIL
 #define CONFIG_USB_GADGET_S3C_UDC_OTG
 #define CONFIG_USB_GADGET_DUALSPEED
 
diff --git a/include/configs/trats.h b/include/configs/trats.h
index 355029e..7c2c875 100644
--- a/include/configs/trats.h
+++ b/include/configs/trats.h
@@ -245,6 +245,7 @@
 #define CONFIG_POWER_BATTERY
 #define CONFIG_POWER_BATTERY_TRATS
 #define CONFIG_USB_GADGET
+#define CONFIG_USB_UTIL
 #define CONFIG_USB_GADGET_S3C_UDC_OTG
 #define CONFIG_USB_GADGET_DUALSPEED
 #define CONFIG_USB_GADGET_VBUS_DRAW	2
-- 
1.7.12

[U-Boot] [PATCH 2/9] g_dnl: Issue connect/disconnect as appropriate

[Pantelis Antoniou]

Call usb_gadget_connect/usb_gadget_disconnect in g_dnl_bind/g_dnl_unbind.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/usb/gadget/g_dnl.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c
index 7d87050..25da733 100644
--- a/drivers/usb/gadget/g_dnl.c
+++ b/drivers/usb/gadget/g_dnl.c
@@ -83,7 +83,12 @@ static struct usb_gadget_strings *g_dnl_composite_strings[] = {
 
 static int g_dnl_unbind(struct usb_composite_dev *cdev)
 {
-	debug("%s\n", __func__);
+	struct usb_gadget *gadget = cdev->gadget;
+
+	debug("%s: calling usb_gadget_disconnect for "
+			"controller '%s'\n", shortname, gadget->name);
+	usb_gadget_disconnect(gadget);
+
 	return 0;
 }
 
@@ -153,6 +158,10 @@ static int g_dnl_bind(struct usb_composite_dev *cdev)
 		device_desc.bcdDevice = __constant_cpu_to_le16(0x9999);
 	}
 
+	debug("%s: calling usb_gadget_connect for "
+			"controller '%s'\n", shortname, gadget->name);
+	usb_gadget_connect(gadget);
+
 	return 0;
 
  error:
-- 
1.7.12

[U-Boot] [PATCH 3/9] g_dnl: Properly terminate string list.

[Pantelis Antoniou]

Well, not terminating the list causes very interesting crashes.
As in changing the vendor & product ID crashes. Fun.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/usb/gadget/g_dnl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c
index 25da733..a5a4c1f 100644
--- a/drivers/usb/gadget/g_dnl.c
+++ b/drivers/usb/gadget/g_dnl.c
@@ -69,6 +69,7 @@ static struct usb_device_descriptor device_desc = {
 static struct usb_string g_dnl_string_defs[] = {
 	{ 0, manufacturer, },
 	{ 1, product, },
+	{  }		/* end of list */
 };
 
 static struct usb_gadget_strings g_dnl_string_tab = {
-- 
1.7.12

[U-Boot] [PATCH 4/9] dfu: Only perform DFU board_usb_init() for TRATS

[Pantelis Antoniou]

USB initialization shouldn't happen for all the boards.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 common/cmd_dfu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
index 01d6b3a..327c738 100644
--- a/common/cmd_dfu.c
+++ b/common/cmd_dfu.c
@@ -55,7 +55,10 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		goto done;
 	}
 
+#ifdef CONFIG_TRATS
 	board_usb_init();
+#endif
+
 	g_dnl_register(s);
 	while (1) {
 		if (ctrlc())
-- 
1.7.12

[U-Boot] [PATCH 4/9] dfu: Only perform DFU board_usb_init() for TRATS

[Lukasz Majewski]

Hi Pantelis,


One request:

Please stick to following guidelines:
http://www.denx.de/wiki/U-Boot/Patches


> USB initialization shouldn't happen for all the boards.
> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>  common/cmd_dfu.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
> index 01d6b3a..327c738 100644
> --- a/common/cmd_dfu.c
> +++ b/common/cmd_dfu.c
> @@ -55,7 +55,10 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int
> argc, char * const argv[]) goto done;
>  	}
>  
> +#ifdef CONFIG_TRATS
>  	board_usb_init();
> +#endif
> +

This is under discussion with Tom.

>  	g_dnl_register(s);
>  	while (1) {
>  		if (ctrlc())



-- 
Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group

[U-Boot] [PATCH 5/9] dfu: Fix crash when wrong number of arguments given

[Pantelis Antoniou]

Fix obvious crash when not enough arguments are given to the dfu
command.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 common/cmd_dfu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
index 327c738..83ef324 100644
--- a/common/cmd_dfu.c
+++ b/common/cmd_dfu.c
@@ -50,7 +50,7 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	if (ret)
 		return CMD_RET_FAILURE;
 
-	if (strcmp(argv[3], "list") == 0) {
+	if (argc > 3 && strcmp(argv[3], "list") == 0) {
 		dfu_show_entities();
 		goto done;
 	}
-- 
1.7.12

[U-Boot] [PATCH 5/9] dfu: Fix crash when wrong number of arguments given

[Lukasz Majewski]

Hi Pantelis,

> Fix obvious crash when not enough arguments are given to the dfu
> command.
> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>  common/cmd_dfu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
> index 327c738..83ef324 100644
> --- a/common/cmd_dfu.c
> +++ b/common/cmd_dfu.c
> @@ -50,7 +50,7 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int
> argc, char * const argv[]) if (ret)
>  		return CMD_RET_FAILURE;
>  
> -	if (strcmp(argv[3], "list") == 0) {
> +	if (argc > 3 && strcmp(argv[3], "list") == 0) {
>  		dfu_show_entities();
>  		goto done;
>  	}

Why do you don't include changelog for your patches?

This is already v3, and I had comment to this patch in v2.
Unfortunately with no feedback from your side.

-- 
Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group

[U-Boot] [PATCH 6/9] dfu: Send correct DFU response from composite_setup

[Pantelis Antoniou]

DFU is a bit peculiar. It needs to hook to composite setup and
return it's function descriptor.

So when get-descriptor request comes with a type of DFU_DT_FUNC
we iterate over the configs, and functions, and when we find
the DFU function we call the setup method which is prepared
to return the appropriate function descriptor.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/usb/gadget/composite.c | 27 +++++++++++++++++++++++++++
 drivers/usb/gadget/ep0.c       |  1 +
 drivers/usb/gadget/f_dfu.c     |  6 ++++--
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index ebb5131..6496436 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -773,6 +773,33 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 			if (value >= 0)
 				value = min(w_length, (u16) value);
 			break;
+
+#ifdef CONFIG_DFU_FUNCTION
+		/* DFU is mighty weird */
+		case DFU_DT_FUNC:
+			w_value &= 0xff;
+			list_for_each_entry(c, &cdev->configs, list) {
+				if (w_value != 0) {
+					w_value--;
+					continue;
+				}
+
+				list_for_each_entry(f, &c->functions, list) {
+
+					/* DFU function only */
+					if (strcmp(f->name, "dfu") != 0)
+						continue;
+
+					value = f->setup(f, ctrl);
+					goto dfu_func_done;
+				}
+			}
+dfu_func_done:
+			if (value >= 0)
+				value = min(w_length, (u16) value);
+			break;
+#endif
+
 		default:
 			goto unknown;
 		}
diff --git a/drivers/usb/gadget/ep0.c b/drivers/usb/gadget/ep0.c
index aa8f916..971d846 100644
--- a/drivers/usb/gadget/ep0.c
+++ b/drivers/usb/gadget/ep0.c
@@ -221,6 +221,7 @@ static int ep0_get_descriptor (struct usb_device_instance *device,
 		break;
 
 	case USB_DESCRIPTOR_TYPE_CONFIGURATION:
+	case USB_DESCRIPTOR_TYPE_OTHER_SPEED_CONFIGURATION:
 		{
 			struct usb_configuration_descriptor
 				*configuration_descriptor;
diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
index 10547e3..6494f5e 100644
--- a/drivers/usb/gadget/f_dfu.c
+++ b/drivers/usb/gadget/f_dfu.c
@@ -534,8 +534,10 @@ dfu_handle(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 			value = min(len, (u16) sizeof(dfu_func));
 			memcpy(req->buf, &dfu_func, value);
 		}
-	} else /* DFU specific request */
-		value = dfu_state[f_dfu->dfu_state] (f_dfu, ctrl, gadget, req);
+		return value;
+	}
+
+	value = dfu_state[f_dfu->dfu_state] (f_dfu, ctrl, gadget, req);
 
 	if (value >= 0) {
 		req->length = value;
-- 
1.7.12

[U-Boot] [PATCH 7/9] dfu: Properly zero out timeout value

[Pantelis Antoniou]

Zero out timeout value; letting it filled with undefined values
ends up with the dfu host hanging.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/usb/gadget/f_dfu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
index 6494f5e..c15585c 100644
--- a/drivers/usb/gadget/f_dfu.c
+++ b/drivers/usb/gadget/f_dfu.c
@@ -164,6 +164,9 @@ static void handle_getstatus(struct usb_request *req)
 
 	/* send status response */
 	dstat->bStatus = f_dfu->dfu_status;
+	dstat->bwPollTimeout[0] = 0;
+	dstat->bwPollTimeout[1] = 0;
+	dstat->bwPollTimeout[2] = 0;
 	dstat->bState = f_dfu->dfu_state;
 	dstat->iString = 0;
 }
-- 
1.7.12

[U-Boot] [PATCH 8/9] dfu: Add a partition type target

[Pantelis Antoniou]

Dealing with raw block numbers with the dfu is very annoying.
Introduce a partition method.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/dfu/dfu_mmc.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
index 5d504df..083d745 100644
--- a/drivers/dfu/dfu_mmc.c
+++ b/drivers/dfu/dfu_mmc.c
@@ -21,6 +21,7 @@
 
 #include <common.h>
 #include <malloc.h>
+#include <errno.h>
 #include <dfu.h>
 
 enum dfu_mmc_op {
@@ -153,6 +154,10 @@ int dfu_read_medium_mmc(struct dfu_entity *dfu, void *buf, long *len)
 
 int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s)
 {
+	int dev, part;
+	struct mmc *mmc;
+	block_dev_desc_t *blk_dev;
+	disk_partition_t partinfo;
 	char *st;
 
 	dfu->dev_type = DFU_DEV_MMC;
@@ -166,8 +171,34 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s)
 		dfu->layout = DFU_FS_FAT;
 	} else if (!strcmp(st, "ext4")) {
 		dfu->layout = DFU_FS_EXT4;
+	} else if (!strcmp(st, "part")) {
+
+		dfu->layout = DFU_RAW_ADDR;
+
+		dev = simple_strtoul(s, &s, 10);
+		s++;
+		part = simple_strtoul(s, &s, 10);
+
+		mmc = find_mmc_device(dev);
+		if (mmc == NULL || mmc_init(mmc)) {
+			printf("%s: could not find mmc device #%d!\n", __func__, dev);
+			return -ENODEV;
+		}
+
+		blk_dev = &mmc->block_dev;
+		if (get_partition_info(blk_dev, part, &partinfo) != 0) {
+			printf("%s: could not find partition #%d on mmc device #%d!\n",
+					__func__, part, dev);
+			return -ENODEV;
+		}
+
+		dfu->data.mmc.lba_start = partinfo.start;
+		dfu->data.mmc.lba_size = partinfo.size;
+		dfu->data.mmc.lba_blk_size = partinfo.blksz;
+
 	} else {
 		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
+		return -ENODEV;
 	}
 
 	if (dfu->layout == DFU_FS_EXT4 || dfu->layout == DFU_FS_FAT) {
-- 
1.7.12

[U-Boot] [PATCH 9/9] dfu: Support larger than memory transfers.

[Pantelis Antoniou]

We didn't support upload/download larger than available memory.
This is pretty bad when you have to update your root filesystem for
example.

This patch removes the limitation (and the crashes when you transfered
any file larger than 4MB).
On top of that reduces the huge dfu buffer from 4MB to just 64K, which
was over the top.

The sequence number is a 16 bit counter; make sure we
handle rollover correctly. This fixes the wrong transfers for
large (> 256MB) images.

Also utilize a variable to handle initialization, so that we
don't rely on just the counter sent by the host.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/dfu/dfu.c     | 244 +++++++++++++++++++++++++++++++++++++++-----------
 drivers/dfu/dfu_mmc.c |  82 +++++++++++------
 include/dfu.h         |  21 ++++-
 3 files changed, 264 insertions(+), 83 deletions(-)

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index e8477fb..fb9b417 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -44,90 +44,228 @@ static int dfu_find_alt_num(const char *s)
 static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE)
 				     dfu_buf[DFU_DATA_BUF_SIZE];
 
+static int dfu_write_buffer_flush(struct dfu_entity *dfu)
+{
+	long w_size;
+	int ret;
+
+	/* flush size? */
+	w_size = dfu->i_buf - dfu->i_buf_start;
+	if (w_size == 0)
+		return 0;
+
+	/* update CRC32 */
+	dfu->crc = crc32(dfu->crc, dfu->i_buf_start, w_size);
+
+	ret = dfu->write_medium(dfu, dfu->offset, dfu->i_buf_start, &w_size);
+	if (ret)
+		debug("%s: Write error!\n", __func__);
+
+	/* point back */
+	dfu->i_buf = dfu->i_buf_start;
+
+	/* update offset */
+	dfu->offset += w_size;
+
+	puts("#");
+
+	return ret;
+}
+
 int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
 {
-	static unsigned char *i_buf;
-	static int i_blk_seq_num;
-	long w_size = 0;
 	int ret = 0;
+	int tret;
+
+	debug("%s: name: %s buf: 0x%p size: 0x%x p_num: 0x%x "
+			"offset: 0x%llx bufoffset: 0x%x\n",
+	       __func__, dfu->name, buf, size, blk_seq_num, dfu->offset,
+	       dfu->i_buf - dfu->i_buf_start);
+
+	if (!dfu->inited) {
+		/* initial state */
+		dfu->crc = 0;
+		dfu->offset = 0;
+		dfu->i_blk_seq_num = 0;
+		dfu->i_buf_start = dfu_buf;
+		dfu->i_buf_end = dfu_buf + sizeof(dfu_buf);
+		dfu->i_buf = dfu->i_buf_start;
+
+		dfu->inited = 1;
+	}
 
-	debug("%s: name: %s buf: 0x%p size: 0x%x p_num: 0x%x i_buf: 0x%p\n",
-	       __func__, dfu->name, buf, size, blk_seq_num, i_buf);
+	if (dfu->i_blk_seq_num != blk_seq_num) {
+		printf("%s: Wrong sequence number! [%d] [%d]\n",
+		       __func__, dfu->i_blk_seq_num, blk_seq_num);
+		return -1;
+	}
 
-	if (blk_seq_num == 0) {
-		i_buf = dfu_buf;
-		i_blk_seq_num = 0;
+	/* DFU 1.1 standard says:
+	 * The wBlockNum field is a block sequence number. It increments each
+	 * time a block is transferred, wrapping to zero from 65,535. It is used
+	 * to provide useful context to the DFU loader in the device."
+	 *
+	 * This means that it's a 16 bit counter that roll-overs at
+	 * 0xffff -> 0x0000. By having a typical 4K transfer block
+	 * we roll-over at exactly 256MB. Not very fun to debug.
+	 *
+	 * Handling rollover, and having an inited variable,
+	 * makes things work.
+	 */
+
+	/* handle rollover */
+	dfu->i_blk_seq_num = (dfu->i_blk_seq_num + 1) & 0xffff;
+
+	/* flush buffer if overflow */
+	if ((dfu->i_buf + size) > dfu->i_buf_end) {
+		tret = dfu_write_buffer_flush(dfu);
+		if (ret == 0)
+			ret = tret;
 	}
 
-	if (i_blk_seq_num++ != blk_seq_num) {
-		printf("%s: Wrong sequence number! [%d] [%d]\n",
-		       __func__, i_blk_seq_num, blk_seq_num);
+	/* we should be in buffer now (if not then size too large) */
+	if ((dfu->i_buf + size) > dfu->i_buf_end) {
+		printf("%s: Wrong size! [%d] [%d] - %d\n",
+		       __func__, dfu->i_blk_seq_num, blk_seq_num, size);
 		return -1;
 	}
 
-	memcpy(i_buf, buf, size);
-	i_buf += size;
+	memcpy(dfu->i_buf, buf, size);
+	dfu->i_buf += size;
 
+	/* if end or if buffer full flush */
+	if (size == 0 || (dfu->i_buf + size) > dfu->i_buf_end) {
+		tret = dfu_write_buffer_flush(dfu);
+		if (ret == 0)
+			ret = tret;
+	}
+
+	/* end? */
 	if (size == 0) {
-		/* Integrity check (if needed) */
-		debug("%s: %s %d [B] CRC32: 0x%x\n", __func__, dfu->name,
-		       i_buf - dfu_buf, crc32(0, dfu_buf, i_buf - dfu_buf));
+		debug("%s: DFU complete CRC32: 0x%x\n", __func__, dfu->crc);
 
-		w_size = i_buf - dfu_buf;
-		ret = dfu->write_medium(dfu, dfu_buf, &w_size);
-		if (ret)
-			debug("%s: Write error!\n", __func__);
+		printf("\nDownload complete (CRC32 0x%04x)\n", dfu->crc);
+
+		/* clear everything */
+		dfu->crc = 0;
+		dfu->offset = 0;
+		dfu->i_blk_seq_num = 0;
+		dfu->i_buf_start = dfu_buf;
+		dfu->i_buf_end = dfu_buf + sizeof(dfu_buf);
+		dfu->i_buf = dfu->i_buf_start;
+
+		dfu->inited = 0;
 
-		i_blk_seq_num = 0;
-		i_buf = NULL;
-		return ret;
 	}
 
-	return ret;
+	return ret = 0 ? size : ret;
+}
+
+static int dfu_read_buffer_fill(struct dfu_entity *dfu, void *buf, int size)
+{
+	long chunk;
+	int ret, readn;
+
+	readn = 0;
+	while (size > 0) {
+
+		/* get chunk that can be read */
+		chunk = min(size, dfu->b_left);
+		/* consume */
+		if (chunk > 0) {
+			memcpy(buf, dfu->i_buf, chunk);
+			dfu->crc = crc32(dfu->crc, buf, chunk);
+			dfu->i_buf += chunk;
+			dfu->b_left -= chunk;
+			size -= chunk;
+			buf += chunk;
+			readn += chunk;
+		}
+
+		/* all done */
+		if (size > 0) {
+			/* no more to read */
+			if (dfu->r_left == 0)
+				break;
+
+			dfu->i_buf = dfu->i_buf_start;
+			dfu->b_left = dfu->i_buf_end - dfu->i_buf_start;
+
+			/* got to read, but buffer is empty */
+			if (dfu->b_left > dfu->r_left)
+				dfu->b_left = dfu->r_left;
+			ret = dfu->read_medium(dfu, dfu->offset, dfu->i_buf,
+					&dfu->b_left);
+			if (ret != 0) {
+				debug("%s: Read error!\n", __func__);
+				return ret;
+			}
+			dfu->offset += dfu->b_left;
+			dfu->r_left -= dfu->b_left;
+
+			puts("#");
+		}
+	}
+
+	return readn;
 }
 
 int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
 {
-	static unsigned char *i_buf;
-	static int i_blk_seq_num;
-	static long r_size;
-	static u32 crc;
 	int ret = 0;
 
 	debug("%s: name: %s buf: 0x%p size: 0x%x p_num: 0x%x i_buf: 0x%p\n",
-	       __func__, dfu->name, buf, size, blk_seq_num, i_buf);
-
-	if (blk_seq_num == 0) {
-		i_buf = dfu_buf;
-		ret = dfu->read_medium(dfu, i_buf, &r_size);
-		debug("%s: %s %ld [B]\n", __func__, dfu->name, r_size);
-		i_blk_seq_num = 0;
-		/* Integrity check (if needed) */
-		crc = crc32(0, dfu_buf, r_size);
+	       __func__, dfu->name, buf, size, blk_seq_num, dfu->i_buf);
+
+	if (!dfu->inited) {
+		ret = dfu->read_medium(dfu, 0, NULL, &dfu->r_left);
+		if (ret != 0) {
+			debug("%s: failed to get r_left\n", __func__);
+			return ret;
+		}
+
+		debug("%s: %s %ld [B]\n", __func__, dfu->name, dfu->r_left);
+
+		dfu->i_blk_seq_num = 0;
+		dfu->crc = 0;
+		dfu->offset = 0;
+		dfu->i_buf_start = dfu_buf;
+		dfu->i_buf_end = dfu_buf + sizeof(dfu_buf);
+		dfu->i_buf = dfu->i_buf_start;
+		dfu->b_left = 0;
+
+		dfu->inited = 1;
 	}
 
-	if (i_blk_seq_num++ != blk_seq_num) {
+	if (dfu->i_blk_seq_num != blk_seq_num) {
 		printf("%s: Wrong sequence number! [%d] [%d]\n",
-		       __func__, i_blk_seq_num, blk_seq_num);
+		       __func__, dfu->i_blk_seq_num, blk_seq_num);
 		return -1;
 	}
+	/* handle rollover */
+	dfu->i_blk_seq_num = (dfu->i_blk_seq_num + 1) & 0xffff;
 
-	if (r_size >= size) {
-		memcpy(buf, i_buf, size);
-		i_buf += size;
-		r_size -= size;
-		return size;
-	} else {
-		memcpy(buf, i_buf, r_size);
-		i_buf += r_size;
-		debug("%s: %s CRC32: 0x%x\n", __func__, dfu->name, crc);
-		puts("UPLOAD ... done\nCtrl+C to exit ...\n");
+	ret = dfu_read_buffer_fill(dfu, buf, size);
+	if (ret < 0) {
+		printf("%s: Failed to fill buffer\n", __func__);
+		return -1;
+	}
+
+	if (ret < size) {
+		debug("%s: %s CRC32: 0x%x\n", __func__, dfu->name, dfu->crc);
+		puts("\nUPLOAD ... done\nCtrl+C to exit ...\n");
 
-		i_buf = NULL;
-		i_blk_seq_num = 0;
-		crc = 0;
-		return r_size;
+		dfu->i_blk_seq_num = 0;
+		dfu->crc = 0;
+		dfu->offset = 0;
+		dfu->i_buf_start = dfu_buf;
+		dfu->i_buf_end = dfu_buf + sizeof(dfu_buf);
+		dfu->i_buf = dfu->i_buf_start;
+		dfu->b_left = 0;
+
+		dfu->inited = 0;
 	}
+
 	return ret;
 }
 
diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
index 083d745..29a2c2e 100644
--- a/drivers/dfu/dfu_mmc.c
+++ b/drivers/dfu/dfu_mmc.c
@@ -22,6 +22,7 @@
 #include <common.h>
 #include <malloc.h>
 #include <errno.h>
+#include <div64.h>
 #include <dfu.h>
 
 enum dfu_mmc_op {
@@ -30,35 +31,48 @@ enum dfu_mmc_op {
 };
 
 static int mmc_block_op(enum dfu_mmc_op op, struct dfu_entity *dfu,
-			void *buf, long *len)
+			u64 offset, void *buf, long *len)
 {
 	char cmd_buf[DFU_CMD_BUF_SIZE];
+	u32 blk_start, blk_count;
 
-	sprintf(cmd_buf, "mmc %s 0x%x %x %x",
-		op == DFU_OP_READ ? "read" : "write",
-		(unsigned int) buf,
-		dfu->data.mmc.lba_start,
-		dfu->data.mmc.lba_size);
-
-	if (op == DFU_OP_READ)
+	/* if buf == NULL return total size of the area */
+	if (buf == NULL) {
 		*len = dfu->data.mmc.lba_blk_size * dfu->data.mmc.lba_size;
+		return 0;
+	}
+
+	blk_start = dfu->data.mmc.lba_start +
+			(u32)lldiv(offset, dfu->data.mmc.lba_blk_size);
+	blk_count = *len / dfu->data.mmc.lba_blk_size;
+	if (blk_start + blk_count >
+			dfu->data.mmc.lba_start + dfu->data.mmc.lba_size) {
+		debug("%s: block_op out of bounds\n", __func__);
+		return -1;
+	}
+
+	sprintf(cmd_buf, "mmc %s %p %x %x",
+		op == DFU_OP_READ ? "read" : "write",
+		 buf, blk_start, blk_count);
 
 	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
 	return run_command(cmd_buf, 0);
 }
 
-static inline int mmc_block_write(struct dfu_entity *dfu, void *buf, long *len)
+static inline int mmc_block_write(struct dfu_entity *dfu,
+		u64 offset, void *buf, long *len)
 {
-	return mmc_block_op(DFU_OP_WRITE, dfu, buf, len);
+	return mmc_block_op(DFU_OP_WRITE, dfu, offset, buf, len);
 }
 
-static inline int mmc_block_read(struct dfu_entity *dfu, void *buf, long *len)
+static inline int mmc_block_read(struct dfu_entity *dfu,
+		u64 offset, void *buf, long *len)
 {
-	return mmc_block_op(DFU_OP_READ, dfu, buf, len);
+	return mmc_block_op(DFU_OP_READ, dfu, offset, buf, len);
 }
 
 static int mmc_file_op(enum dfu_mmc_op op, struct dfu_entity *dfu,
-			void *buf, long *len)
+			u64 offset, void *buf, long *len)
 {
 	char cmd_buf[DFU_CMD_BUF_SIZE];
 	char *str_env;
@@ -66,12 +80,17 @@ static int mmc_file_op(enum dfu_mmc_op op, struct dfu_entity *dfu,
 
 	switch (dfu->layout) {
 	case DFU_FS_FAT:
-		sprintf(cmd_buf, "fat%s mmc %d:%d 0x%x %s %lx",
+		sprintf(cmd_buf, "fat%s mmc %d:%d 0x%x %s %lx %llx",
 			op == DFU_OP_READ ? "load" : "write",
 			dfu->data.mmc.dev, dfu->data.mmc.part,
-			(unsigned int) buf, dfu->name, *len);
+			(unsigned int) buf, dfu->name, *len, offset);
 		break;
 	case DFU_FS_EXT4:
+		if (offset != 0) {
+			debug("%s: Offset value %llx != 0 not supported!\n",
+					__func__, offset);
+			return -1;
+		}
 		sprintf(cmd_buf, "ext4%s mmc %d:%d /%s 0x%x %ld",
 			op == DFU_OP_READ ? "load" : "write",
 			dfu->data.mmc.dev, dfu->data.mmc.part,
@@ -80,6 +99,7 @@ static int mmc_file_op(enum dfu_mmc_op op, struct dfu_entity *dfu,
 	default:
 		printf("%s: Layout (%s) not (yet) supported!\n", __func__,
 		       dfu_get_layout(dfu->layout));
+		return -1;
 	}
 
 	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
@@ -102,27 +122,30 @@ static int mmc_file_op(enum dfu_mmc_op op, struct dfu_entity *dfu,
 	return ret;
 }
 
-static inline int mmc_file_write(struct dfu_entity *dfu, void *buf, long *len)
+static inline int mmc_file_write(struct dfu_entity *dfu,
+		u64 offset, void *buf, long *len)
 {
-	return mmc_file_op(DFU_OP_WRITE, dfu, buf, len);
+	return mmc_file_op(DFU_OP_WRITE, dfu, offset, buf, len);
 }
 
-static inline int mmc_file_read(struct dfu_entity *dfu, void *buf, long *len)
+static inline int mmc_file_read(struct dfu_entity *dfu,
+		u64 offset, void *buf, long *len)
 {
-	return mmc_file_op(DFU_OP_READ, dfu, buf, len);
+	return mmc_file_op(DFU_OP_READ, dfu, offset, buf, len);
 }
 
-int dfu_write_medium_mmc(struct dfu_entity *dfu, void *buf, long *len)
+int dfu_write_medium_mmc(struct dfu_entity *dfu,
+		u64 offset, void *buf, long *len)
 {
 	int ret = -1;
 
 	switch (dfu->layout) {
 	case DFU_RAW_ADDR:
-		ret = mmc_block_write(dfu, buf, len);
+		ret = mmc_block_write(dfu, offset, buf, len);
 		break;
 	case DFU_FS_FAT:
 	case DFU_FS_EXT4:
-		ret = mmc_file_write(dfu, buf, len);
+		ret = mmc_file_write(dfu, offset, buf, len);
 		break;
 	default:
 		printf("%s: Layout (%s) not (yet) supported!\n", __func__,
@@ -132,17 +155,17 @@ int dfu_write_medium_mmc(struct dfu_entity *dfu, void *buf, long *len)
 	return ret;
 }
 
-int dfu_read_medium_mmc(struct dfu_entity *dfu, void *buf, long *len)
+int dfu_read_medium_mmc(struct dfu_entity *dfu, u64 offset, void *buf, long *len)
 {
 	int ret = -1;
 
 	switch (dfu->layout) {
 	case DFU_RAW_ADDR:
-		ret = mmc_block_read(dfu, buf, len);
+		ret = mmc_block_read(dfu, offset, buf, len);
 		break;
 	case DFU_FS_FAT:
 	case DFU_FS_EXT4:
-		ret = mmc_file_read(dfu, buf, len);
+		ret = mmc_file_read(dfu, offset, buf, len);
 		break;
 	default:
 		printf("%s: Layout (%s) not (yet) supported!\n", __func__,
@@ -181,13 +204,15 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s)
 
 		mmc = find_mmc_device(dev);
 		if (mmc == NULL || mmc_init(mmc)) {
-			printf("%s: could not find mmc device #%d!\n", __func__, dev);
+			printf("%s: could not find mmc device #%d!\n",
+					__func__, dev);
 			return -ENODEV;
 		}
 
 		blk_dev = &mmc->block_dev;
 		if (get_partition_info(blk_dev, part, &partinfo) != 0) {
-			printf("%s: could not find partition #%d on mmc device #%d!\n",
+			printf("%s: could not find partition #%d "
+					"on mmc device #%d!\n",
 					__func__, part, dev);
 			return -ENODEV;
 		}
@@ -209,5 +234,8 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s)
 	dfu->read_medium = dfu_read_medium_mmc;
 	dfu->write_medium = dfu_write_medium_mmc;
 
+	/* initial state */
+	dfu->inited = 0;
+
 	return 0;
 }
diff --git a/include/dfu.h b/include/dfu.h
index 5350d79..2847ef8 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -59,7 +59,7 @@ static inline unsigned int get_mmc_blk_size(int dev)
 
 #define DFU_NAME_SIZE 32
 #define DFU_CMD_BUF_SIZE 128
-#define DFU_DATA_BUF_SIZE (1024*1024*4) /* 4 MiB */
+#define DFU_DATA_BUF_SIZE (1024*64) /* 64 KB (the huge buffer is overkill) */
 
 struct dfu_entity {
 	char			name[DFU_NAME_SIZE];
@@ -73,10 +73,25 @@ struct dfu_entity {
 		struct mmc_internal_data mmc;
 	} data;
 
-	int (*read_medium)(struct dfu_entity *dfu, void *buf, long *len);
-	int (*write_medium)(struct dfu_entity *dfu, void *buf, long *len);
+	int (*read_medium)(struct dfu_entity *dfu,
+			u64 offset, void *buf, long *len);
+
+	int (*write_medium)(struct dfu_entity *dfu,
+			u64 offset, void *buf, long *len);
 
 	struct list_head list;
+
+	/* on the fly state */
+	u32 crc;
+	u64 offset;
+	int i_blk_seq_num;
+	u8 *i_buf;
+	u8 *i_buf_start;
+	u8 *i_buf_end;
+	long r_left;
+	long b_left;
+
+	unsigned int inited : 1;
 };
 
 int dfu_config_entities(char *s, char *interface, int num);
-- 
1.7.12

[U-Boot] [PATCH 9/9] dfu: Support larger than memory transfers.

[kasim ling]

Hi, Pantelis,
A little confusion about FAT fs write supporting,


On Sat, Dec 1, 2012 at 12:51 AM, Pantelis Antoniou
<panto@antoniou-consulting.com> wrote:
>
> We didn't support upload/download larger than available memory.
> This is pretty bad when you have to update your root filesystem for
> example.
>
> This patch removes the limitation (and the crashes when you transfered
> any file larger than 4MB).
> On top of that reduces the huge dfu buffer from 4MB to just 64K, which
> was over the top.
>
> The sequence number is a 16 bit counter; make sure we
> handle rollover correctly. This fixes the wrong transfers for
> large (> 256MB) images.
>
> Also utilize a variable to handle initialization, so that we
> don't rely on just the counter sent by the host.
>
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>  drivers/dfu/dfu.c     | 244 +++++++++++++++++++++++++++++++++++++++-----------
>  drivers/dfu/dfu_mmc.c |  82 +++++++++++------
>  include/dfu.h         |  21 ++++-
>  3 files changed, 264 insertions(+), 83 deletions(-)

> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
> index 083d745..29a2c2e 100644
> --- a/drivers/dfu/dfu_mmc.c
> +++ b/drivers/dfu/dfu_mmc.c
> @@ -22,6 +22,7 @@
>  #include <common.h>
>  #include <malloc.h>
>  #include <errno.h>
> +#include <div64.h>
>  #include <dfu.h>
>
>  enum dfu_mmc_op {
> @@ -30,35 +31,48 @@ enum dfu_mmc_op {
>  };
>
>  static int mmc_block_op(enum dfu_mmc_op op, struct dfu_entity *dfu,
> -                       void *buf, long *len)
> +                       u64 offset, void *buf, long *len)
>  {
>         char cmd_buf[DFU_CMD_BUF_SIZE];
> +       u32 blk_start, blk_count;
>
> -       sprintf(cmd_buf, "mmc %s 0x%x %x %x",
> -               op == DFU_OP_READ ? "read" : "write",
> -               (unsigned int) buf,
> -               dfu->data.mmc.lba_start,
> -               dfu->data.mmc.lba_size);
> -
> -       if (op == DFU_OP_READ)
> +       /* if buf == NULL return total size of the area */
> +       if (buf == NULL) {
>                 *len = dfu->data.mmc.lba_blk_size * dfu->data.mmc.lba_size;
> +               return 0;
> +       }
> +
> +       blk_start = dfu->data.mmc.lba_start +
> +                       (u32)lldiv(offset, dfu->data.mmc.lba_blk_size);
> +       blk_count = *len / dfu->data.mmc.lba_blk_size;
> +       if (blk_start + blk_count >
> +                       dfu->data.mmc.lba_start + dfu->data.mmc.lba_size) {
> +               debug("%s: block_op out of bounds\n", __func__);
> +               return -1;
> +       }
> +
> +       sprintf(cmd_buf, "mmc %s %p %x %x",
> +               op == DFU_OP_READ ? "read" : "write",
> +                buf, blk_start, blk_count);
>
>         debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
>         return run_command(cmd_buf, 0);
>  }
>
> -static inline int mmc_block_write(struct dfu_entity *dfu, void *buf, long *len)
> +static inline int mmc_block_write(struct dfu_entity *dfu,
> +               u64 offset, void *buf, long *len)
>  {
> -       return mmc_block_op(DFU_OP_WRITE, dfu, buf, len);
> +       return mmc_block_op(DFU_OP_WRITE, dfu, offset, buf, len);
>  }
>
> -static inline int mmc_block_read(struct dfu_entity *dfu, void *buf, long *len)
> +static inline int mmc_block_read(struct dfu_entity *dfu,
> +               u64 offset, void *buf, long *len)
>  {
> -       return mmc_block_op(DFU_OP_READ, dfu, buf, len);
> +       return mmc_block_op(DFU_OP_READ, dfu, offset, buf, len);
>  }
>
>  static int mmc_file_op(enum dfu_mmc_op op, struct dfu_entity *dfu,
> -                       void *buf, long *len)
> +                       u64 offset, void *buf, long *len)
>  {
>         char cmd_buf[DFU_CMD_BUF_SIZE];
>         char *str_env;
> @@ -66,12 +80,17 @@ static int mmc_file_op(enum dfu_mmc_op op, struct dfu_entity *dfu,
>
>         switch (dfu->layout) {
>         case DFU_FS_FAT:
> -               sprintf(cmd_buf, "fat%s mmc %d:%d 0x%x %s %lx",
> +               sprintf(cmd_buf, "fat%s mmc %d:%d 0x%x %s %lx %llx",
>                         op == DFU_OP_READ ? "load" : "write",
>                         dfu->data.mmc.dev, dfu->data.mmc.part,
> -                       (unsigned int) buf, dfu->name, *len);
> +                       (unsigned int) buf, dfu->name, *len, offset);
Did you tested it on FAT partitions? According to do_fat_fswrite()
defined in common/cmd_fat.c, the "fatwrite" command does not support
"offset" argument.


Thanks,
Alex

[U-Boot] [PATCH 9/9] dfu: Support larger than memory transfers.

[Pantelis Antoniou]

Hi Kasim,

On Jan 3, 2013, at 10:30 AM, kasim ling wrote:

> Hi, Pantelis,
> A little confusion about FAT fs write supporting,
> 
> 
> On Sat, Dec 1, 2012 at 12:51 AM, Pantelis Antoniou
> <panto@antoniou-consulting.com> wrote:
>> 
>> We didn't support upload/download larger than available memory.
>> This is pretty bad when you have to update your root filesystem for
>> example.
>> 
>> This patch removes the limitation (and the crashes when you transfered
>> any file larger than 4MB).
>> On top of that reduces the huge dfu buffer from 4MB to just 64K, which
>> was over the top.
>> 
>> The sequence number is a 16 bit counter; make sure we
>> handle rollover correctly. This fixes the wrong transfers for
>> large (> 256MB) images.
>> 
>> Also utilize a variable to handle initialization, so that we
>> don't rely on just the counter sent by the host.
>> 
>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>> ---
>> drivers/dfu/dfu.c     | 244 +++++++++++++++++++++++++++++++++++++++-----------
>> drivers/dfu/dfu_mmc.c |  82 +++++++++++------
>> include/dfu.h         |  21 ++++-
>> 3 files changed, 264 insertions(+), 83 deletions(-)
> 
>> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
>> index 083d745..29a2c2e 100644
>> --- a/drivers/dfu/dfu_mmc.c
>> +++ b/drivers/dfu/dfu_mmc.c
>> @@ -22,6 +22,7 @@
>> #include <common.h>
>> #include <malloc.h>
>> #include <errno.h>
>> +#include <div64.h>
>> #include <dfu.h>
>> 
>> enum dfu_mmc_op {
>> @@ -30,35 +31,48 @@ enum dfu_mmc_op {
>> };
>> 
>> static int mmc_block_op(enum dfu_mmc_op op, struct dfu_entity *dfu,
>> -                       void *buf, long *len)
>> +                       u64 offset, void *buf, long *len)
>> {
>>        char cmd_buf[DFU_CMD_BUF_SIZE];
>> +       u32 blk_start, blk_count;
>> 
>> -       sprintf(cmd_buf, "mmc %s 0x%x %x %x",
>> -               op == DFU_OP_READ ? "read" : "write",
>> -               (unsigned int) buf,
>> -               dfu->data.mmc.lba_start,
>> -               dfu->data.mmc.lba_size);
>> -
>> -       if (op == DFU_OP_READ)
>> +       /* if buf == NULL return total size of the area */
>> +       if (buf == NULL) {
>>                *len = dfu->data.mmc.lba_blk_size * dfu->data.mmc.lba_size;
>> +               return 0;
>> +       }
>> +
>> +       blk_start = dfu->data.mmc.lba_start +
>> +                       (u32)lldiv(offset, dfu->data.mmc.lba_blk_size);
>> +       blk_count = *len / dfu->data.mmc.lba_blk_size;
>> +       if (blk_start + blk_count >
>> +                       dfu->data.mmc.lba_start + dfu->data.mmc.lba_size) {
>> +               debug("%s: block_op out of bounds\n", __func__);
>> +               return -1;
>> +       }
>> +
>> +       sprintf(cmd_buf, "mmc %s %p %x %x",
>> +               op == DFU_OP_READ ? "read" : "write",
>> +                buf, blk_start, blk_count);
>> 
>>        debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
>>        return run_command(cmd_buf, 0);
>> }
>> 
>> -static inline int mmc_block_write(struct dfu_entity *dfu, void *buf, long *len)
>> +static inline int mmc_block_write(struct dfu_entity *dfu,
>> +               u64 offset, void *buf, long *len)
>> {
>> -       return mmc_block_op(DFU_OP_WRITE, dfu, buf, len);
>> +       return mmc_block_op(DFU_OP_WRITE, dfu, offset, buf, len);
>> }
>> 
>> -static inline int mmc_block_read(struct dfu_entity *dfu, void *buf, long *len)
>> +static inline int mmc_block_read(struct dfu_entity *dfu,
>> +               u64 offset, void *buf, long *len)
>> {
>> -       return mmc_block_op(DFU_OP_READ, dfu, buf, len);
>> +       return mmc_block_op(DFU_OP_READ, dfu, offset, buf, len);
>> }
>> 
>> static int mmc_file_op(enum dfu_mmc_op op, struct dfu_entity *dfu,
>> -                       void *buf, long *len)
>> +                       u64 offset, void *buf, long *len)
>> {
>>        char cmd_buf[DFU_CMD_BUF_SIZE];
>>        char *str_env;
>> @@ -66,12 +80,17 @@ static int mmc_file_op(enum dfu_mmc_op op, struct dfu_entity *dfu,
>> 
>>        switch (dfu->layout) {
>>        case DFU_FS_FAT:
>> -               sprintf(cmd_buf, "fat%s mmc %d:%d 0x%x %s %lx",
>> +               sprintf(cmd_buf, "fat%s mmc %d:%d 0x%x %s %lx %llx",
>>                        op == DFU_OP_READ ? "load" : "write",
>>                        dfu->data.mmc.dev, dfu->data.mmc.part,
>> -                       (unsigned int) buf, dfu->name, *len);
>> +                       (unsigned int) buf, dfu->name, *len, offset);
> Did you tested it on FAT partitions? According to do_fat_fswrite()
> defined in common/cmd_fat.c, the "fatwrite" command does not support
> "offset" argument.
> 

No I haven't had a use case either for fat or ext2/3/4.

So I guess it is as broken as it was before.
If you want to write to a file, make sure it's smaller than the
DFU buffer.

One of these days the file access functions must be fixed.
> 
> Thanks,
> Alex

Regards

-- Pantelis