[PATCH 0/3] usb: gadget: dfu: Fix timeout on erase when using big partition

[PATCH 0/3] usb: gadget: dfu: Fix timeout on erase when using big partition

[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

---

Jules Maselbas (3):
  usb: gadget: dfu: Add manifestation phase
  usb: gadget: dfu: Progressive erase if file is a mtd
  usb: gadget: dfu: Reset global variables on unbind

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

-- 
2.21.0.196.g041f5ea


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

[PATCH 1/3] usb: gadget: dfu: Add manifestation phase

[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 c2b3d481a..fef6f3e3a 100644
--- a/drivers/usb/gadget/dfu.c
+++ b/drivers/usb/gadget/dfu.c
@@ -342,38 +342,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;
 	}
 
@@ -381,9 +357,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;
 }
@@ -438,23 +457,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;
@@ -479,11 +492,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);
@@ -520,9 +535,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;
@@ -538,6 +561,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;
@@ -553,6 +584,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? */
@@ -564,10 +603,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

[PATCH 2/3] usb: gadget: dfu: Progressive erase if file is a mtd

[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 | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/dfu.c b/drivers/usb/gadget/dfu.c
index fef6f3e3a..592586db1 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 = {
@@ -327,8 +333,16 @@ 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);
+		erase(dfufd, size, dfu_erased);
+		dfu_erased += size;
+	}
+
+	dfu_written += req->length;
 	ret = write(dfufd, req->buf, req->length);
 	if (ret < (int)req->length) {
 		perror("write");
@@ -493,7 +507,12 @@ 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);
+				ret = ioctl(dfufd, MEMGETINFO, &dfu_mtdinfo);
+				if (ret) /* not a mtd */
+					ret = erase(dfufd, ERASE_SIZE_ALL, 0);
+				else
+					prog_erase = 1;
+
 				if (ret && ret != -ENOSYS) {
 					dfu->dfu_status = DFU_STATUS_errERASE;
 					perror("erase");
-- 
2.21.0.196.g041f5ea


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

[PATCH 3/3] usb: gadget: dfu: Reset global variables on unbind

[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 | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/gadget/dfu.c b/drivers/usb/gadget/dfu.c
index 592586db1..5504f4933 100644
--- a/drivers/usb/gadget/dfu.c
+++ b/drivers/usb/gadget/dfu.c
@@ -277,6 +277,15 @@ dfu_unbind(struct usb_configuration *c, struct usb_function *f)
 {
 	struct f_dfu		*dfu = func_to_dfu(f);
 
+	memset(&dfu_mtdinfo, 0, sizeof(dfu_mtdinfo));
+	dfu_files = NULL;
+	dfu_file_entry = NULL;
+	dfufd = -EINVAL;
+	dfudetach = 0;
+	dfu_written = 0;
+	dfu_erased = 0;
+	prog_erase = 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

Re: [PATCH 2/3] usb: gadget: dfu: Progressive erase if file is a mtd

[Sascha Hauer]

Hi Jules,

On Tue, Mar 24, 2020 at 04:46:46PM +0100, Jules Maselbas wrote:
> 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 | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/dfu.c b/drivers/usb/gadget/dfu.c
> index fef6f3e3a..592586db1 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;

These are not initialized, this works for the first file updated via
DFU, but what about the following ones?

>  
>  /* USB DFU functional descriptor */
>  static struct usb_dfu_func_descriptor usb_dfu_func = {
> @@ -327,8 +333,16 @@ 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);
> +		erase(dfufd, size, dfu_erased);

You should check the return value here.

> +		dfu_erased += size;
> +	}
> +
> +	dfu_written += req->length;
>  	ret = write(dfufd, req->buf, req->length);
>  	if (ret < (int)req->length) {
>  		perror("write");
> @@ -493,7 +507,12 @@ 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);
> +				ret = ioctl(dfufd, MEMGETINFO, &dfu_mtdinfo);
> +				if (ret) /* not a mtd */
> +					ret = erase(dfufd, ERASE_SIZE_ALL, 0);
> +				else
> +					prog_erase = 1;

I am not aware of any non mtd devices that need erase. I think you can
drop the full erase here.

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

Re: [PATCH 3/3] usb: gadget: dfu: Reset global variables on unbind

[Sascha Hauer]

On Tue, Mar 24, 2020 at 04:46:47PM +0100, Jules Maselbas wrote:
> 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 | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/usb/gadget/dfu.c b/drivers/usb/gadget/dfu.c
> index 592586db1..5504f4933 100644
> --- a/drivers/usb/gadget/dfu.c
> +++ b/drivers/usb/gadget/dfu.c
> @@ -277,6 +277,15 @@ dfu_unbind(struct usb_configuration *c, struct usb_function *f)
>  {
>  	struct f_dfu		*dfu = func_to_dfu(f);
>  
> +	memset(&dfu_mtdinfo, 0, sizeof(dfu_mtdinfo));
> +	dfu_files = NULL;
> +	dfu_file_entry = NULL;
> +	dfufd = -EINVAL;
> +	dfudetach = 0;
> +	dfu_written = 0;
> +	dfu_erased = 0;
> +	prog_erase = 0;

Ah, ok, here is the missing initialization ;)

Please swap the order of patches 2/3 and 3/3

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

Re: [PATCH 2/3] usb: gadget: dfu: Progressive erase if file is a mtd

[Jules Maselbas]

Hi Sascha,

On Thu, Mar 26, 2020 at 07:03:11AM +0100, Sascha Hauer wrote:
> Hi Jules,
> 
> >  
> > @@ -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;
> 
> These are not initialized, this works for the first file updated via
> DFU, but what about the following ones?
Good point, this initialization is be required between to file.
I don't think cleaning this in unbind will be sufficient, I will
try to update multiple files.

> 
> >  
> >  /* USB DFU functional descriptor */
> >  static struct usb_dfu_func_descriptor usb_dfu_func = {
> > @@ -327,8 +333,16 @@ 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);
> > +		erase(dfufd, size, dfu_erased);
> 
> You should check the return value here.
Right
 
> > +		dfu_erased += size;
> > +	}
> > +
> > +	dfu_written += req->length;
> >  	ret = write(dfufd, req->buf, req->length);
> >  	if (ret < (int)req->length) {
> >  		perror("write");
> > @@ -493,7 +507,12 @@ 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);
> > +				ret = ioctl(dfufd, MEMGETINFO, &dfu_mtdinfo);
> > +				if (ret) /* not a mtd */
> > +					ret = erase(dfufd, ERASE_SIZE_ALL, 0);
> > +				else
> > +					prog_erase = 1;
> 
> I am not aware of any non mtd devices that need erase. I think you can
> drop the full erase here.
OK

Thanks for the review,
Jules

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