mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v2 0/4] usb: gadget: dfu: Fix timeout on erase when using big partition
@ 2020-03-26 18:33 Jules Maselbas
  2020-03-26 18:33 ` [PATCH v2 1/4] usb: gadget: dfu: Reset global variables on unbind Jules Maselbas
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Jules Maselbas @ 2020-03-26 18:33 UTC (permalink / raw)
  To: barebox; +Cc: Jules Maselbas

Hi,

The two first patch are focused on two fixes for an issue I encountered
when trying to use the DFU gadget on big flash partition (2MB).

You can find more information in each patch, but here is a short summary
of the issue:

When downloading a firmware into a big flash partition the erase
operation can take a long time to be complete from few seconds to
minutes in extreme cases. During the erase the DFU gadget does not
respond to any USB setup request, the host only see a stalled USB
endpoint and cannot get responses from DFU_GETSTATE nor DFU_GETSTATUS.


The last patch is a small fix that allow to use the dfu gadget after
being unbind, such as:
  barebox:/ usbgadget -D /foo(foo)
  barebox:/ usbgadget -d
  barebox:/ usbgadget -D /bar(bar)


Best regards,
Jules

change in v2:
 - reorder commits
 - reset global variables for progressive erase in dfu_cleanup
   this is to allow multi partition update without restart
 - add missing error check on erase and remove erase on non mtd device
 - add a new commit that fix a bug when exposing multiple partitions at once

---

Jules Maselbas (4):
  usb: gadget: dfu: Reset global variables on unbind
  usb: gadget: dfu: Add manifestation phase
  usb: gadget: dfu: Progressive erase if file is a mtd
  usb: gadget: dfu: Fix DFU mode interface descriptor

 drivers/usb/gadget/dfu.c | 207 +++++++++++++++++++++++++++++----------
 1 file changed, 155 insertions(+), 52 deletions(-)

-- 
2.21.0.196.g041f5ea


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/4] usb: gadget: dfu: Reset global variables on unbind
  2020-03-26 18:33 [PATCH v2 0/4] usb: gadget: dfu: Fix timeout on erase when using big partition Jules Maselbas
@ 2020-03-26 18:33 ` Jules Maselbas
  2020-03-26 18:33 ` [PATCH v2 2/4] usb: gadget: dfu: Add manifestation phase Jules Maselbas
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Jules Maselbas @ 2020-03-26 18:33 UTC (permalink / raw)
  To: barebox; +Cc: Jules Maselbas

Global variables must be reset to their default value before a new
dfu_bind is done. Otherwise things wont work and are likely to cause
a system crash due to a use after free: the global dfu_files was still
pointing deallocated structure after unbind.

Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
---
 drivers/usb/gadget/dfu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/gadget/dfu.c b/drivers/usb/gadget/dfu.c
index c2b3d481a..5bdcb68bf 100644
--- a/drivers/usb/gadget/dfu.c
+++ b/drivers/usb/gadget/dfu.c
@@ -271,6 +271,10 @@ dfu_unbind(struct usb_configuration *c, struct usb_function *f)
 {
 	struct f_dfu		*dfu = func_to_dfu(f);
 
+	dfu_files = NULL;
+	dfu_file_entry = NULL;
+	dfudetach = 0;
+
 	usb_free_all_descriptors(f);
 
 	dma_free(dfu->dnreq->buf);
-- 
2.21.0.196.g041f5ea


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 2/4] usb: gadget: dfu: Add manifestation phase
  2020-03-26 18:33 [PATCH v2 0/4] usb: gadget: dfu: Fix timeout on erase when using big partition Jules Maselbas
  2020-03-26 18:33 ` [PATCH v2 1/4] usb: gadget: dfu: Reset global variables on unbind Jules Maselbas
@ 2020-03-26 18:33 ` Jules Maselbas
  2020-03-26 18:33 ` [PATCH v2 3/4] usb: gadget: dfu: Progressive erase if file is a mtd Jules Maselbas
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Jules Maselbas @ 2020-03-26 18:33 UTC (permalink / raw)
  To: barebox; +Cc: Jules Maselbas

When downloading a firmware into a big flash partition the erase
operation can take a long time to be complete from few seconds to
minutes in extreme cases. During the erase the DFU gadget does not
respond to any USB setup request, the host only see a stalled USB
endpoint and cannot get responses from DFU_GETSTATE nor DFU_GETSTATUS.

After 5 seconds without any updates the host will abort the DFU
transfer and return an error (when using dfu-util).

When using the safe flag on the partition the file is first downloaded
in memory. The partition is only erased when the last chunk is received
if the partition takes more than 5 seconds to be erased then the
transfer fails.

This patch fix this issue by using the manifestation phase for safe
partitions. It's used to inform the host that the transfer is complete
and that the device will now be doing the actual erasing and writing.
Not all devices will be able to respond to DFU_GETSTATUS when in the
dfuMANIFEST state (from DFU-1.1 specification page 22).

This is far from perfect as the utility dfu-util will exit before the
erase/writing is done, but it will not report an error status if a
timeout happen during this phase.

Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
---
 drivers/usb/gadget/dfu.c | 179 ++++++++++++++++++++++++++++-----------
 1 file changed, 128 insertions(+), 51 deletions(-)

diff --git a/drivers/usb/gadget/dfu.c b/drivers/usb/gadget/dfu.c
index 5bdcb68bf..3f457b69d 100644
--- a/drivers/usb/gadget/dfu.c
+++ b/drivers/usb/gadget/dfu.c
@@ -346,38 +346,14 @@ static int handle_dnload(struct usb_function *f, const struct usb_ctrlrequest *c
 	struct f_dfu		*dfu = func_to_dfu(f);
 	struct usb_composite_dev *cdev = f->config->cdev;
 	u16			w_length = le16_to_cpu(ctrl->wLength);
-	int ret;
 
 	if (w_length == 0) {
-		dfu->dfu_state = DFU_STATE_dfuIDLE;
 		if (dfu_file_entry->flags & FILE_LIST_FLAG_SAFE) {
-			int fd;
-			unsigned flags = O_WRONLY;
-
-			if (dfu_file_entry->flags & FILE_LIST_FLAG_CREATE)
-				flags |= O_CREAT | O_TRUNC;
-
-			fd = open(dfu_file_entry->filename, flags);
-			if (fd < 0) {
-				perror("open");
-				ret = -EINVAL;
-				goto err_out;
-			}
-			ret = erase(fd, ERASE_SIZE_ALL, 0);
-			close(fd);
-			if (ret && ret != -ENOSYS) {
-				perror("erase");
-				ret = -EINVAL;
-				goto err_out;
-			}
-			ret = copy_file(DFU_TEMPFILE, dfu_file_entry->filename, 0);
-			if (ret) {
-				printf("copy file failed\n");
-				ret = -EINVAL;
-				goto err_out;
-			}
+			dfu->dfu_state = DFU_STATE_dfuMANIFEST;
+		} else {
+			dfu->dfu_state = DFU_STATE_dfuIDLE;
+			dfu_cleanup(dfu);
 		}
-		dfu_cleanup(dfu);
 		return 0;
 	}
 
@@ -385,9 +361,52 @@ static int handle_dnload(struct usb_function *f, const struct usb_ctrlrequest *c
 	dfu->dnreq->context = dfu;
 	usb_ep_queue(cdev->gadget->ep0, dfu->dnreq);
 
+	return 0;
+}
+
+static int handle_manifest(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
+{
+	struct f_dfu		*dfu = func_to_dfu(f);
+	int ret;
+
+	dfu->dfu_state = DFU_STATE_dfuIDLE;
+
+	if (dfu_file_entry->flags & FILE_LIST_FLAG_SAFE) {
+		int fd;
+		unsigned flags = O_WRONLY;
+
+		if (dfu_file_entry->flags & FILE_LIST_FLAG_CREATE)
+			flags |= O_CREAT | O_TRUNC;
+
+		fd = open(dfu_file_entry->filename, flags);
+		if (fd < 0) {
+			perror("open");
+			dfu->dfu_status = DFU_STATUS_errERASE;
+			ret = -EINVAL;
+			goto out;
+		}
+
+		ret = erase(fd, ERASE_SIZE_ALL, 0);
+		close(fd);
+		if (ret && ret != -ENOSYS) {
+			dfu->dfu_status = DFU_STATUS_errERASE;
+			perror("erase");
+			goto out;
+		}
+
+		ret = copy_file(DFU_TEMPFILE, dfu_file_entry->filename, 0);
+		if (ret) {
+			printf("copy file failed\n");
+			ret = -EINVAL;
+			goto out;
+		}
+	}
+
 	return 0;
 
-err_out:
+out:
+	dfu->dfu_status = DFU_STATUS_errWRITE;
+	dfu->dfu_state = DFU_STATE_dfuERROR;
 	dfu_cleanup(dfu);
 	return ret;
 }
@@ -442,23 +461,17 @@ static int dfu_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 		goto out;
 	}
 
-	/* Allow GETSTATUS in every state */
-	if (ctrl->bRequest == USB_REQ_DFU_GETSTATUS) {
-		value = dfu_status(f, ctrl);
-		value = min(value, w_length);
-		goto out;
-	}
-
-	/* Allow GETSTATE in every state */
-	if (ctrl->bRequest == USB_REQ_DFU_GETSTATE) {
-		*(u8 *)req->buf = dfu->dfu_state;
-		value = sizeof(u8);
-		goto out;
-	}
-
 	switch (dfu->dfu_state) {
 	case DFU_STATE_dfuIDLE:
 		switch (ctrl->bRequest) {
+		case USB_REQ_DFU_GETSTATUS:
+			value = dfu_status(f, ctrl);
+			value = min(value, w_length);
+			break;
+		case USB_REQ_DFU_GETSTATE:
+			*(u8 *)req->buf = dfu->dfu_state;
+			value = sizeof(u8);
+			break;
 		case USB_REQ_DFU_DNLOAD:
 			if (w_length == 0) {
 				dfu->dfu_state = DFU_STATE_dfuERROR;
@@ -483,11 +496,13 @@ static int dfu_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 				goto out;
 			}
 
-			ret = erase(dfufd, ERASE_SIZE_ALL, 0);
-			if (ret && ret != -ENOSYS) {
-				dfu->dfu_status = DFU_STATUS_errERASE;
-				perror("erase");
-				goto out;
+			if (!(dfu_file_entry->flags & FILE_LIST_FLAG_SAFE)) {
+				ret = erase(dfufd, ERASE_SIZE_ALL, 0);
+				if (ret && ret != -ENOSYS) {
+					dfu->dfu_status = DFU_STATUS_errERASE;
+					perror("erase");
+					goto out;
+				}
 			}
 
 			value = handle_dnload(f, ctrl);
@@ -524,9 +539,17 @@ static int dfu_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 		break;
 	case DFU_STATE_dfuDNLOAD_IDLE:
 		switch (ctrl->bRequest) {
+		case USB_REQ_DFU_GETSTATUS:
+			value = dfu_status(f, ctrl);
+			value = min(value, w_length);
+			break;
+		case USB_REQ_DFU_GETSTATE:
+			*(u8 *)req->buf = dfu->dfu_state;
+			value = sizeof(u8);
+			break;
 		case USB_REQ_DFU_DNLOAD:
 			value = handle_dnload(f, ctrl);
-			if (dfu->dfu_state != DFU_STATE_dfuIDLE) {
+			if (dfu->dfu_state == DFU_STATE_dfuDNLOAD_IDLE) {
 				return 0;
 			}
 			break;
@@ -542,6 +565,14 @@ static int dfu_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 		break;
 	case DFU_STATE_dfuUPLOAD_IDLE:
 		switch (ctrl->bRequest) {
+		case USB_REQ_DFU_GETSTATUS:
+			value = dfu_status(f, ctrl);
+			value = min(value, w_length);
+			break;
+		case USB_REQ_DFU_GETSTATE:
+			*(u8 *)req->buf = dfu->dfu_state;
+			value = sizeof(u8);
+			break;
 		case USB_REQ_DFU_UPLOAD:
 			handle_upload(f, ctrl);
 			return 0;
@@ -557,6 +588,14 @@ static int dfu_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 		break;
 	case DFU_STATE_dfuERROR:
 		switch (ctrl->bRequest) {
+		case USB_REQ_DFU_GETSTATUS:
+			value = dfu_status(f, ctrl);
+			value = min(value, w_length);
+			break;
+		case USB_REQ_DFU_GETSTATE:
+			*(u8 *)req->buf = dfu->dfu_state;
+			value = sizeof(u8);
+			break;
 		case USB_REQ_DFU_CLRSTATUS:
 			dfu_abort(dfu);
 			/* no zlp? */
@@ -568,10 +607,48 @@ static int dfu_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 			break;
 		}
 		break;
-	case DFU_STATE_dfuDNLOAD_SYNC:
-	case DFU_STATE_dfuDNBUSY:
 	case DFU_STATE_dfuMANIFEST_SYNC:
+		switch (ctrl->bRequest) {
+		case USB_REQ_DFU_GETSTATUS:
+			value = dfu_status(f, ctrl);
+			if (dfu_file_entry->flags & FILE_LIST_FLAG_SAFE)
+				dfu->dfu_state = DFU_STATE_dfuMANIFEST;
+			else
+				dfu->dfu_state = DFU_STATE_dfuIDLE;
+			value = min(value, w_length);
+			break;
+		case USB_REQ_DFU_GETSTATE:
+			*(u8 *)req->buf = dfu->dfu_state;
+			value = sizeof(u8);
+			break;
+		default:
+			dfu->dfu_state = DFU_STATE_dfuERROR;
+			value = -EINVAL;
+			break;
+		}
+		break;
 	case DFU_STATE_dfuMANIFEST:
+		value = handle_manifest(f, ctrl);
+		if (dfu->dfu_state != DFU_STATE_dfuIDLE) {
+			return 0;
+		}
+		switch (ctrl->bRequest) {
+		case USB_REQ_DFU_GETSTATUS:
+			value = dfu_status(f, ctrl);
+			value = min(value, w_length);
+			break;
+		case USB_REQ_DFU_GETSTATE:
+			*(u8 *)req->buf = dfu->dfu_state;
+			value = sizeof(u8);
+			break;
+		default:
+			dfu->dfu_state = DFU_STATE_dfuERROR;
+			value = -EINVAL;
+			break;
+		}
+		break;
+	case DFU_STATE_dfuDNLOAD_SYNC:
+	case DFU_STATE_dfuDNBUSY:
 		dfu->dfu_state = DFU_STATE_dfuERROR;
 		value = -EINVAL;
 		break;
-- 
2.21.0.196.g041f5ea


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 3/4] usb: gadget: dfu: Progressive erase if file is a mtd
  2020-03-26 18:33 [PATCH v2 0/4] usb: gadget: dfu: Fix timeout on erase when using big partition Jules Maselbas
  2020-03-26 18:33 ` [PATCH v2 1/4] usb: gadget: dfu: Reset global variables on unbind Jules Maselbas
  2020-03-26 18:33 ` [PATCH v2 2/4] usb: gadget: dfu: Add manifestation phase Jules Maselbas
@ 2020-03-26 18:33 ` Jules Maselbas
  2020-03-26 18:33 ` [PATCH v2 4/4] usb: gadget: dfu: Fix DFU mode interface descriptor Jules Maselbas
  2020-03-31  5:13 ` [PATCH v2 0/4] usb: gadget: dfu: Fix timeout on erase when using big partition Sascha Hauer
  4 siblings, 0 replies; 8+ messages in thread
From: Jules Maselbas @ 2020-03-26 18:33 UTC (permalink / raw)
  To: barebox; +Cc: Jules Maselbas

When downloading a firmware into a big flash partition the erase
operation can take a long time to be complete from few seconds to
minutes in extreme cases. During the erase the DFU gadget does not
respond to any USB setup request, the host only see a stalled USB
endpoint and cannot get responses from DFU_GETSTATE nor DFU_GETSTATUS.

After 5 seconds without any updates the host will abort the DFU
transfer and return an error (when using dfu-util).

For instance I have a 2MB partition that takes 25 seconds to erase,
this erase cannot be done in one step as it takes too much time or it
should be done in the manifestation phase, see previous patch.

This patch modify the erase behavior when downloading a new firmware.
If the updated file is a mtd partition then the DFU gadget will do
a progressive erase by erasing the least amount of required blocks
before writing a new chunk into the mtd device.

Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
---
 drivers/usb/gadget/dfu.c | 34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/dfu.c b/drivers/usb/gadget/dfu.c
index 3f457b69d..225c6e3c6 100644
--- a/drivers/usb/gadget/dfu.c
+++ b/drivers/usb/gadget/dfu.c
@@ -55,6 +55,8 @@
 #include <libbb.h>
 #include <init.h>
 #include <fs.h>
+#include <ioctl.h>
+#include <linux/mtd/mtd-abi.h>
 
 #define USB_DT_DFU			0x21
 
@@ -132,6 +134,10 @@ struct file_list_entry *dfu_file_entry;
 static int dfufd = -EINVAL;
 static struct file_list *dfu_files;
 static int dfudetach;
+static struct mtd_info_user dfu_mtdinfo;
+static loff_t dfu_written;
+static loff_t dfu_erased;
+static int prog_erase;
 
 /* USB DFU functional descriptor */
 static struct usb_dfu_func_descriptor usb_dfu_func = {
@@ -319,6 +325,11 @@ static void dfu_cleanup(struct f_dfu *dfu)
 {
 	struct stat s;
 
+	memset(&dfu_mtdinfo, 0, sizeof(dfu_mtdinfo));
+	dfu_written = 0;
+	dfu_erased = 0;
+	prog_erase = 0;
+
 	if (dfufd > 0) {
 		close(dfufd);
 		dfufd = -EINVAL;
@@ -331,8 +342,22 @@ static void dfu_cleanup(struct f_dfu *dfu)
 static void dn_complete(struct usb_ep *ep, struct usb_request *req)
 {
 	struct f_dfu		*dfu = req->context;
+	loff_t size;
 	int ret;
 
+	if (prog_erase && (dfu_written + req->length) > dfu_erased) {
+		size = roundup(req->length, dfu_mtdinfo.erasesize);
+		ret = erase(dfufd, size, dfu_erased);
+		dfu_erased += size;
+		if (ret && ret != -ENOSYS) {
+			perror("erase");
+			dfu->dfu_status = DFU_STATUS_errERASE;
+			dfu_cleanup(dfu);
+			return;
+		}
+	}
+
+	dfu_written += req->length;
 	ret = write(dfufd, req->buf, req->length);
 	if (ret < (int)req->length) {
 		perror("write");
@@ -497,12 +522,9 @@ static int dfu_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 			}
 
 			if (!(dfu_file_entry->flags & FILE_LIST_FLAG_SAFE)) {
-				ret = erase(dfufd, ERASE_SIZE_ALL, 0);
-				if (ret && ret != -ENOSYS) {
-					dfu->dfu_status = DFU_STATUS_errERASE;
-					perror("erase");
-					goto out;
-				}
+				ret = ioctl(dfufd, MEMGETINFO, &dfu_mtdinfo);
+				if (!ret) /* file is on a mtd device */
+					prog_erase = 1;
 			}
 
 			value = handle_dnload(f, ctrl);
-- 
2.21.0.196.g041f5ea


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 4/4] usb: gadget: dfu: Fix DFU mode interface descriptor
  2020-03-26 18:33 [PATCH v2 0/4] usb: gadget: dfu: Fix timeout on erase when using big partition Jules Maselbas
                   ` (2 preceding siblings ...)
  2020-03-26 18:33 ` [PATCH v2 3/4] usb: gadget: dfu: Progressive erase if file is a mtd Jules Maselbas
@ 2020-03-26 18:33 ` Jules Maselbas
  2020-03-30  5:26   ` Sascha Hauer
  2020-03-31  5:13 ` [PATCH v2 0/4] usb: gadget: dfu: Fix timeout on erase when using big partition Sascha Hauer
  4 siblings, 1 reply; 8+ messages in thread
From: Jules Maselbas @ 2020-03-26 18:33 UTC (permalink / raw)
  To: barebox; +Cc: Jules Maselbas

The gadget driver set the bInterfaceProtocol value to 1 in the DFU
interface descriptor. However this value is used to indicate that the
gadget is in run-time mode and not ready for DFU. When ready for DFU
operation the bInterfaceProtocol value must be set to 2 (DFU mode).

From the DFU 1.1 specification, the value of bInterfaceProtocol select:
 - 1: Runtime protocol  (see Table 4.1)
 - 2: DFU mode protocol (see Table 4.4)

This patch change the bInterfaceProtocol value from 1 to 2. As this
DFU gadget driver is always ready for DFU operation (DFU mode) and
doesn't require a USB reset.

Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>

---

I was getting errors when trying to update a partition using
dfu-util when there was more than one partition available.
(the error was "More than one DFU capable USB device found!")

This is the output of dfu-util when listing partitions.
The first is before this patch and second one is after.
Notice the difference between the two: in the first you
can read 'Found Runtime' where in the other 'Found DFU'.

The bInterfaceProtocol was set to 1 (before):

  $ dfu-util -l
  Found Runtime: [1d6b:0104] [...] alt=0, name="/dev/mtd0.p0"
  Found Runtime: [1d6b:0104] [...] alt=1, name="/dev/mtd0.p1"
  ...

When bInterfaceProtocol is set to 2 (after patch):

  $ dfu-util -l
  Found DFU: [1d6b:0104] [...] alt=0, name="/dev/mtd0.p0"
  Found DFU: [1d6b:0104] [...] alt=1, name="/dev/mtd0.p1"
  ...

---
 drivers/usb/gadget/dfu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/dfu.c b/drivers/usb/gadget/dfu.c
index 225c6e3c6..9117a5b06 100644
--- a/drivers/usb/gadget/dfu.c
+++ b/drivers/usb/gadget/dfu.c
@@ -239,7 +239,7 @@ dfu_bind(struct usb_configuration *c, struct usb_function *f)
 		desc[i].bNumEndpoints	=	0;
 		desc[i].bInterfaceClass =	0xfe;
 		desc[i].bInterfaceSubClass =	1;
-		desc[i].bInterfaceProtocol =	1;
+		desc[i].bInterfaceProtocol =	2;
 		desc[i].bAlternateSetting = i;
 		desc[i].iInterface = us[i + 1].id;
 		header[i] = (struct usb_descriptor_header *)&desc[i];
-- 
2.21.0.196.g041f5ea


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 4/4] usb: gadget: dfu: Fix DFU mode interface descriptor
  2020-03-26 18:33 ` [PATCH v2 4/4] usb: gadget: dfu: Fix DFU mode interface descriptor Jules Maselbas
@ 2020-03-30  5:26   ` Sascha Hauer
  2020-03-30  9:13     ` Jules Maselbas
  0 siblings, 1 reply; 8+ messages in thread
From: Sascha Hauer @ 2020-03-30  5:26 UTC (permalink / raw)
  To: Jules Maselbas; +Cc: barebox

Hi Jules,

On Thu, Mar 26, 2020 at 07:33:04PM +0100, Jules Maselbas wrote:
> The gadget driver set the bInterfaceProtocol value to 1 in the DFU
> interface descriptor. However this value is used to indicate that the
> gadget is in run-time mode and not ready for DFU. When ready for DFU
> operation the bInterfaceProtocol value must be set to 2 (DFU mode).
> 
> From the DFU 1.1 specification, the value of bInterfaceProtocol select:
>  - 1: Runtime protocol  (see Table 4.1)
>  - 2: DFU mode protocol (see Table 4.4)
> 
> This patch change the bInterfaceProtocol value from 1 to 2. As this
> DFU gadget driver is always ready for DFU operation (DFU mode) and
> doesn't require a USB reset.
> 
> Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>

DFU stopped working for me quite a while ago and I tracked it down to
this dfu-util commit:

| commit 377f6f136d3369529f44578acaeee82d7c7d7af9
| Author: Paul Fertser <fercerpav@gmail.com>
| Date:   Sun Aug 10 14:26:05 2014 +0400
| 
|     dfu_util: Ignore alt_index/alt_name specification in runtime mode
|     
|     When the device is in runtime mode it needs to be reset first into DFU
|     mode for the list of alternate settings to appear, so unless it is
|     already in the right mode, matching on alt setting number or name
|     should be skipped.
|     
|     Fixes regression on OpenMoko Freerunner.
|     
|     Signed-off-by: Paul Fertser <fercerpav@gmail.com>

Are you using a recent dfu-util version? Is your patch the fix for this
commit?

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 4/4] usb: gadget: dfu: Fix DFU mode interface descriptor
  2020-03-30  5:26   ` Sascha Hauer
@ 2020-03-30  9:13     ` Jules Maselbas
  0 siblings, 0 replies; 8+ messages in thread
From: Jules Maselbas @ 2020-03-30  9:13 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Hi Sascha,

On Mon, Mar 30, 2020 at 07:26:30AM +0200, Sascha Hauer wrote:
> DFU stopped working for me quite a while ago and I tracked it down to
> this dfu-util commit:
> 
> | commit 377f6f136d3369529f44578acaeee82d7c7d7af9
> | Author: Paul Fertser <fercerpav@gmail.com>
> | Date:   Sun Aug 10 14:26:05 2014 +0400
> | 
> |     dfu_util: Ignore alt_index/alt_name specification in runtime mode
> |     
> |     When the device is in runtime mode it needs to be reset first into DFU
> |     mode for the list of alternate settings to appear, so unless it is
> |     already in the right mode, matching on alt setting number or name
> |     should be skipped.
> |     
> |     Fixes regression on OpenMoko Freerunner.
> |     
> |     Signed-off-by: Paul Fertser <fercerpav@gmail.com>
> 
> Are you using a recent dfu-util version? Is your patch the fix for this
> commit?
Yes, I am using dfu-util version 0.9. I didn't search in dfu-util
commit history but yeah my patch seems to be a fix for this commit.

The current DFU gadget do not handle USB reset and thus cannot change
mode, it cannot go from runtime to DFU mode. The solution is to always
be in the DFU ready mode.

Thanks for sharing this commit, it explain well what I was experiencing:
dfu-util was correctly listing partitions but failed to found a device
when trying to update a partition that is still in runtime mode.

Jules

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/4] usb: gadget: dfu: Fix timeout on erase when using big partition
  2020-03-26 18:33 [PATCH v2 0/4] usb: gadget: dfu: Fix timeout on erase when using big partition Jules Maselbas
                   ` (3 preceding siblings ...)
  2020-03-26 18:33 ` [PATCH v2 4/4] usb: gadget: dfu: Fix DFU mode interface descriptor Jules Maselbas
@ 2020-03-31  5:13 ` Sascha Hauer
  4 siblings, 0 replies; 8+ messages in thread
From: Sascha Hauer @ 2020-03-31  5:13 UTC (permalink / raw)
  To: Jules Maselbas; +Cc: barebox

On Thu, Mar 26, 2020 at 07:33:00PM +0100, Jules Maselbas wrote:
> Hi,
> 
> The two first patch are focused on two fixes for an issue I encountered
> when trying to use the DFU gadget on big flash partition (2MB).
> 
> You can find more information in each patch, but here is a short summary
> of the issue:
> 
> When downloading a firmware into a big flash partition the erase
> operation can take a long time to be complete from few seconds to
> minutes in extreme cases. During the erase the DFU gadget does not
> respond to any USB setup request, the host only see a stalled USB
> endpoint and cannot get responses from DFU_GETSTATE nor DFU_GETSTATUS.
> 
> 
> The last patch is a small fix that allow to use the dfu gadget after
> being unbind, such as:
>   barebox:/ usbgadget -D /foo(foo)
>   barebox:/ usbgadget -d
>   barebox:/ usbgadget -D /bar(bar)
> 
> 
> Best regards,
> Jules
> 
> change in v2:
>  - reorder commits
>  - reset global variables for progressive erase in dfu_cleanup
>    this is to allow multi partition update without restart
>  - add missing error check on erase and remove erase on non mtd device
>  - add a new commit that fix a bug when exposing multiple partitions at once

Applied, thanks

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-03-31  5:13 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-26 18:33 [PATCH v2 0/4] usb: gadget: dfu: Fix timeout on erase when using big partition Jules Maselbas
2020-03-26 18:33 ` [PATCH v2 1/4] usb: gadget: dfu: Reset global variables on unbind Jules Maselbas
2020-03-26 18:33 ` [PATCH v2 2/4] usb: gadget: dfu: Add manifestation phase Jules Maselbas
2020-03-26 18:33 ` [PATCH v2 3/4] usb: gadget: dfu: Progressive erase if file is a mtd Jules Maselbas
2020-03-26 18:33 ` [PATCH v2 4/4] usb: gadget: dfu: Fix DFU mode interface descriptor Jules Maselbas
2020-03-30  5:26   ` Sascha Hauer
2020-03-30  9:13     ` Jules Maselbas
2020-03-31  5:13 ` [PATCH v2 0/4] usb: gadget: dfu: Fix timeout on erase when using big partition Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox