[PATCH 2/3] usb: gadget: dfu: Wrap fs operation in workqueue
Jules Maselbas
jmaselbas at kalray.eu
Mon Feb 8 11:34:57 EST 2021
File system operation shouldn't be executed in a poller. Use
a workqueue to delay filesystem operation to command context.
---
change RFC -> v1:
- Rework manifestation phase, copy work is directly triggered
when entering the manifestation state
- Rework cleanup, it is now done when either exiting the ERROR
state or when calling dfu_abort or dfu_disable.
- Rework error handling when reading from file
Signed-off-by: Jules Maselbas <jmaselbas at kalray.eu>
---
drivers/usb/gadget/dfu.c | 375 +++++++++++++++++++++++++--------------
1 file changed, 246 insertions(+), 129 deletions(-)
diff --git a/drivers/usb/gadget/dfu.c b/drivers/usb/gadget/dfu.c
index 9d6a9d252..95e8da82e 100644
--- a/drivers/usb/gadget/dfu.c
+++ b/drivers/usb/gadget/dfu.c
@@ -54,6 +54,7 @@
#include <fs.h>
#include <ioctl.h>
#include <linux/mtd/mtd-abi.h>
+#include <work.h>
#define USB_DT_DFU 0x21
@@ -153,6 +154,7 @@ struct f_dfu {
u8 dfu_state;
u8 dfu_status;
struct usb_request *dnreq;
+ struct work_queue wq;
};
static inline struct f_dfu *func_to_dfu(struct usb_function *f)
@@ -173,6 +175,191 @@ static struct usb_gadget_strings *dfu_strings[] = {
};
static void dn_complete(struct usb_ep *ep, struct usb_request *req);
+static void up_complete(struct usb_ep *ep, struct usb_request *req);
+static void dfu_cleanup(struct f_dfu *dfu);
+
+struct dfu_work {
+ struct work_struct work;
+ struct f_dfu *dfu;
+ void (*task)(struct dfu_work *dw);
+ size_t len;
+ uint8_t *rbuf;
+ uint8_t wbuf[CONFIG_USBD_DFU_XFER_SIZE];
+};
+
+static void dfu_do_work(struct work_struct *w)
+{
+ struct dfu_work *dw = container_of(w, struct dfu_work, work);
+ struct f_dfu *dfu = dw->dfu;
+
+ if (dfu->dfu_state != DFU_STATE_dfuERROR && dfu->dfu_status == DFU_STATUS_OK)
+ dw->task(dw);
+ else
+ debug("skip work\n");
+
+ free(dw);
+}
+
+static void dfu_work_cancel(struct work_struct *w)
+{
+ struct dfu_work *dw = container_of(w, struct dfu_work, work);
+
+ free(dw);
+}
+
+static void dfu_do_write(struct dfu_work *dw)
+{
+ struct f_dfu *dfu = dw->dfu;
+ ssize_t size, wlen = dw->len;
+ ssize_t ret;
+
+ debug("do write\n");
+
+ if (prog_erase && (dfu_written + wlen) > dfu_erased) {
+ size = roundup(wlen, dfu_mtdinfo.erasesize);
+ ret = erase(dfufd, size, dfu_erased);
+ dfu_erased += size;
+ if (ret && ret != -ENOSYS) {
+ perror("erase");
+ dfu->dfu_state = DFU_STATE_dfuERROR;
+ dfu->dfu_status = DFU_STATUS_errERASE;
+ return;
+ }
+ }
+
+ dfu_written += wlen;
+ ret = write(dfufd, dw->wbuf, wlen);
+ if (ret < wlen) {
+ perror("write");
+ dfu->dfu_state = DFU_STATE_dfuERROR;
+ dfu->dfu_status = DFU_STATUS_errWRITE;
+ }
+}
+
+static void dfu_do_read(struct dfu_work *dw)
+{
+ struct f_dfu *dfu = dw->dfu;
+ struct usb_composite_dev *cdev = dfu->func.config->cdev;
+ ssize_t size, rlen = dw->len;
+
+ debug("do read\n");
+
+ size = read(dfufd, dfu->dnreq->buf, rlen);
+ dfu->dnreq->length = size;
+ if (size < 0) {
+ perror("read");
+ dfu->dnreq->length = 0;
+ dfu->dfu_state = DFU_STATE_dfuERROR;
+ dfu->dfu_status = DFU_STATUS_errFILE;
+ } else if (size < rlen) {
+ /* this is the last chunk, go to IDLE and close file */
+ dfu_cleanup(dfu);
+ }
+
+ dfu->dnreq->complete = up_complete;
+ usb_ep_queue(cdev->gadget->ep0, dfu->dnreq);
+}
+
+static void dfu_do_open_dnload(struct dfu_work *dw)
+{
+ struct f_dfu *dfu = dw->dfu;
+ int ret;
+
+ debug("do open dnload\n");
+
+ if (dfu_file_entry->flags & FILE_LIST_FLAG_SAFE) {
+ dfufd = open(DFU_TEMPFILE, O_WRONLY | O_CREAT);
+ } else {
+ unsigned flags = O_WRONLY;
+
+ if (dfu_file_entry->flags & FILE_LIST_FLAG_CREATE)
+ flags |= O_CREAT | O_TRUNC;
+
+ dfufd = open(dfu_file_entry->filename, flags);
+ }
+
+ if (dfufd < 0) {
+ perror("open");
+ dfu->dfu_state = DFU_STATE_dfuERROR;
+ dfu->dfu_status = DFU_STATUS_errFILE;
+ return;
+ }
+
+ if (!(dfu_file_entry->flags & FILE_LIST_FLAG_SAFE)) {
+ ret = ioctl(dfufd, MEMGETINFO, &dfu_mtdinfo);
+ if (!ret) /* file is on a mtd device */
+ prog_erase = 1;
+ }
+}
+
+static void dfu_do_open_upload(struct dfu_work *dw)
+{
+ struct f_dfu *dfu = dw->dfu;
+
+ debug("do open upload\n");
+
+ dfufd = open(dfu_file_entry->filename, O_RDONLY);
+ if (dfufd < 0) {
+ perror("open");
+ dfu->dfu_state = DFU_STATE_dfuERROR;
+ dfu->dfu_status = DFU_STATUS_errFILE;
+ }
+}
+
+static void dfu_do_close(struct dfu_work *dw)
+{
+ struct stat s;
+
+ debug("do close\n");
+
+ if (dfufd > 0) {
+ close(dfufd);
+ dfufd = -EINVAL;
+ }
+
+ if (!stat(DFU_TEMPFILE, &s))
+ unlink(DFU_TEMPFILE);
+}
+
+static void dfu_do_copy(struct dfu_work *dw)
+{
+ struct f_dfu *dfu = dw->dfu;
+ unsigned flags = O_WRONLY;
+ int ret, fd;
+
+ debug("do copy\n");
+
+ 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_state = DFU_STATE_dfuERROR;
+ dfu->dfu_status = DFU_STATUS_errERASE;
+ return;
+ }
+
+ ret = erase(fd, ERASE_SIZE_ALL, 0);
+ close(fd);
+ if (ret && ret != -ENOSYS) {
+ perror("erase");
+ dfu->dfu_state = DFU_STATE_dfuERROR;
+ dfu->dfu_status = DFU_STATUS_errERASE;
+ return;
+ }
+
+ ret = copy_file(DFU_TEMPFILE, dfu_file_entry->filename, 0);
+ if (ret) {
+ printf("copy file failed\n");
+ dfu->dfu_state = DFU_STATE_dfuERROR;
+ dfu->dfu_status = DFU_STATUS_errWRITE;
+ return;
+ }
+
+ dfu->dfu_state = DFU_STATE_dfuIDLE;
+ dfu_do_close(dw);
+}
static int
dfu_bind(struct usb_configuration *c, struct usb_function *f)
@@ -223,6 +410,10 @@ dfu_bind(struct usb_configuration *c, struct usb_function *f)
goto out;
}
+ dfu->wq.fn = dfu_do_work;
+ dfu->wq.cancel = dfu_work_cancel;
+ wq_register(&dfu->wq);
+
/* allocate instance-specific interface IDs, and patch descriptors */
status = usb_interface_id(c, f);
if (status < 0)
@@ -278,6 +469,8 @@ dfu_unbind(struct usb_configuration *c, struct usb_function *f)
dfu_file_entry = NULL;
dfudetach = 0;
+ wq_unregister(&dfu->wq);
+
usb_free_all_descriptors(f);
dma_free(dfu->dnreq->buf);
@@ -320,47 +513,50 @@ static int dfu_status(struct usb_function *f, const struct usb_ctrlrequest *ctrl
static void dfu_cleanup(struct f_dfu *dfu)
{
- struct stat s;
+ struct dfu_work *dw;
+
+ debug("dfu cleanup\n");
memset(&dfu_mtdinfo, 0, sizeof(dfu_mtdinfo));
dfu_written = 0;
dfu_erased = 0;
prog_erase = 0;
- if (dfufd > 0) {
- close(dfufd);
- dfufd = -EINVAL;
- }
+ dfu->dfu_state = DFU_STATE_dfuIDLE;
+ dfu->dfu_status = DFU_STATUS_OK;
- if (!stat(DFU_TEMPFILE, &s))
- unlink(DFU_TEMPFILE);
+ dw = xzalloc(sizeof(*dw));
+ dw->dfu = dfu;
+ dw->task = dfu_do_close;
+ wq_queue_work(&dfu->wq, &dw->work);
}
static void dn_complete(struct usb_ep *ep, struct usb_request *req)
{
struct f_dfu *dfu = req->context;
- loff_t size;
- int ret;
+ struct dfu_work *dw;
+
+ dw = xzalloc(sizeof(*dw));
+ dw->dfu = dfu;
+ dw->task = dfu_do_write;
+ dw->len = min_t(unsigned int, req->length, CONFIG_USBD_DFU_XFER_SIZE);
+ memcpy(dw->wbuf, req->buf, dw->len);
+ wq_queue_work(&dfu->wq, &dw->work);
+}
- 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;
- }
- }
+static int handle_manifest(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
+{
+ struct f_dfu *dfu = func_to_dfu(f);
+ struct dfu_work *dw;
- dfu_written += req->length;
- ret = write(dfufd, req->buf, req->length);
- if (ret < (int)req->length) {
- perror("write");
- dfu->dfu_status = DFU_STATUS_errWRITE;
- dfu_cleanup(dfu);
+ if (dfu_file_entry->flags & FILE_LIST_FLAG_SAFE) {
+ dw = xzalloc(sizeof(*dw));
+ dw->dfu = dfu;
+ dw->task = dfu_do_copy;
+ wq_queue_work(&dfu->wq, &dw->work);
}
+
+ return 0;
}
static int handle_dnload(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
@@ -370,12 +566,8 @@ static int handle_dnload(struct usb_function *f, const struct usb_ctrlrequest *c
u16 w_length = le16_to_cpu(ctrl->wLength);
if (w_length == 0) {
- if (dfu_file_entry->flags & FILE_LIST_FLAG_SAFE) {
- dfu->dfu_state = DFU_STATE_dfuMANIFEST;
- } else {
- dfu->dfu_state = DFU_STATE_dfuIDLE;
- dfu_cleanup(dfu);
- }
+ handle_manifest(f, ctrl);
+ dfu->dfu_state = DFU_STATE_dfuMANIFEST;
return 0;
}
@@ -386,53 +578,6 @@ static int handle_dnload(struct usb_function *f, const struct usb_ctrlrequest *c
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;
-
-out:
- dfu->dfu_status = DFU_STATUS_errWRITE;
- dfu->dfu_state = DFU_STATE_dfuERROR;
- dfu_cleanup(dfu);
- return ret;
-}
-
static void up_complete(struct usb_ep *ep, struct usb_request *req)
{
}
@@ -440,28 +585,22 @@ static void up_complete(struct usb_ep *ep, struct usb_request *req)
static int handle_upload(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
{
struct f_dfu *dfu = func_to_dfu(f);
- struct usb_composite_dev *cdev = f->config->cdev;
+ struct dfu_work *dw;
u16 w_length = le16_to_cpu(ctrl->wLength);
- int len;
-
- len = read(dfufd, dfu->dnreq->buf, w_length);
- dfu->dnreq->length = len;
- if (len < w_length) {
- dfu_cleanup(dfu);
- dfu->dfu_state = DFU_STATE_dfuIDLE;
- }
-
- dfu->dnreq->complete = up_complete;
- usb_ep_queue(cdev->gadget->ep0, dfu->dnreq);
+ dw = xzalloc(sizeof(*dw));
+ dw->dfu = dfu;
+ dw->task = dfu_do_read;
+ dw->len = w_length;
+ dw->rbuf = dfu->dnreq->buf;
+ wq_queue_work(&dfu->wq, &dw->work);
return 0;
}
static void dfu_abort(struct f_dfu *dfu)
{
- dfu->dfu_state = DFU_STATE_dfuIDLE;
- dfu->dfu_status = DFU_STATUS_OK;
+ wq_cancel_work(&dfu->wq);
dfu_cleanup(dfu);
}
@@ -474,7 +613,7 @@ static int dfu_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
int value = -EOPNOTSUPP;
int w_length = le16_to_cpu(ctrl->wLength);
int w_value = le16_to_cpu(ctrl->wValue);
- int ret;
+ struct dfu_work *dw;
if (ctrl->bRequestType == USB_DIR_IN && ctrl->bRequest == USB_REQ_GET_DESCRIPTOR
&& (w_value >> 8) == 0x21) {
@@ -501,28 +640,10 @@ static int dfu_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
goto out;
}
debug("dfu: starting download to %s\n", dfu_file_entry->filename);
- if (dfu_file_entry->flags & FILE_LIST_FLAG_SAFE) {
- dfufd = open(DFU_TEMPFILE, O_WRONLY | O_CREAT);
- } else {
- unsigned flags = O_WRONLY;
-
- if (dfu_file_entry->flags & FILE_LIST_FLAG_CREATE)
- flags |= O_CREAT | O_TRUNC;
-
- dfufd = open(dfu_file_entry->filename, flags);
- }
-
- if (dfufd < 0) {
- dfu->dfu_state = DFU_STATE_dfuERROR;
- perror("open");
- goto out;
- }
-
- if (!(dfu_file_entry->flags & FILE_LIST_FLAG_SAFE)) {
- ret = ioctl(dfufd, MEMGETINFO, &dfu_mtdinfo);
- if (!ret) /* file is on a mtd device */
- prog_erase = 1;
- }
+ dw = xzalloc(sizeof(*dw));
+ dw->dfu = dfu;
+ dw->task = dfu_do_open_dnload;
+ wq_queue_work(&dfu->wq, &dw->work);
value = handle_dnload(f, ctrl);
dfu->dfu_state = DFU_STATE_dfuDNLOAD_IDLE;
@@ -534,12 +655,12 @@ static int dfu_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
dfu->dfu_state = DFU_STATE_dfuERROR;
goto out;
}
- dfufd = open(dfu_file_entry->filename, O_RDONLY);
- if (dfufd < 0) {
- dfu->dfu_state = DFU_STATE_dfuERROR;
- perror("open");
- goto out;
- }
+
+ dw = xzalloc(sizeof(*dw));
+ dw->dfu = dfu;
+ dw->task = dfu_do_open_upload;
+ wq_queue_work(&dfu->wq, &dw->work);
+
handle_upload(f, ctrl);
return 0;
case USB_REQ_DFU_ABORT:
@@ -606,6 +727,7 @@ static int dfu_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
}
break;
case DFU_STATE_dfuERROR:
+ wq_cancel_work(&dfu->wq);
switch (ctrl->bRequest) {
case USB_REQ_DFU_GETSTATUS:
value = dfu_status(f, ctrl);
@@ -647,10 +769,7 @@ static int dfu_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
}
break;
case DFU_STATE_dfuMANIFEST:
- value = handle_manifest(f, ctrl);
- if (dfu->dfu_state != DFU_STATE_dfuIDLE) {
- return 0;
- }
+ dfu->dfu_state = DFU_STATE_dfuMANIFEST_SYNC;
switch (ctrl->bRequest) {
case USB_REQ_DFU_GETSTATUS:
value = dfu_status(f, ctrl);
@@ -692,9 +811,7 @@ static void dfu_disable(struct usb_function *f)
{
struct f_dfu *dfu = func_to_dfu(f);
- dfu->dfu_state = DFU_STATE_dfuIDLE;
-
- dfu_cleanup(dfu);
+ dfu_abort(dfu);
}
#define STRING_MANUFACTURER_IDX 0
--
2.17.1
More information about the barebox
mailing list