File system operation shouldn't be executed in a poller. Use a workqueue to delay filesystem operation to command context.
This is an RFC, extra work must be done to properly handle error cases and dfu cleanup. Signed-off-by: Jules Maselbas <[email protected]> --- drivers/usb/gadget/dfu.c | 321 ++++++++++++++++++++++++++------------- 1 file changed, 216 insertions(+), 105 deletions(-) diff --git a/drivers/usb/gadget/dfu.c b/drivers/usb/gadget/dfu.c index 9d6a9d252..75abd1576 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,178 @@ 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); + + /* TODO: find a better way to skip tasks when the dfu gadget + * has encounter an error and dfu_cleanup has been called */ + if (dw->task && dw->dfu->dfu_status == DFU_STATUS_OK) + dw->task(dw); + + 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; + size_t size, wlen = dw->len; + int 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_status = DFU_STATUS_errERASE; + dfu_cleanup(dfu); + return; + } + } + + dfu_written += wlen; + ret = write(dfufd, dw->wbuf, wlen); + if (ret < (int)wlen) { + perror("write"); + dfu->dfu_status = DFU_STATUS_errWRITE; + dfu_cleanup(dfu); + } +} + +static void dfu_do_read(struct dfu_work *dw) +{ + struct f_dfu *dfu = dw->dfu; + struct usb_composite_dev *cdev = dfu->func.config->cdev; + size_t size, rlen = dw->len; + + debug("do read\n"); + + size = read(dfufd, dfu->dnreq->buf, rlen); + dfu->dnreq->length = size; + if (size < (int)rlen) { + perror("read"); + dfu_cleanup(dfu); + dfu->dfu_state = DFU_STATE_dfuIDLE; + } + + 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_status = DFU_STATUS_errFILE; + 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; + } + + return; +out: + dfu->dfu_state = DFU_STATE_dfuERROR; + dfu_cleanup(dfu); +} + +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_status = DFU_STATUS_errFILE; + dfu->dfu_state = DFU_STATE_dfuERROR; + dfu_cleanup(dfu); + } +} + +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_status = DFU_STATUS_errERASE; + goto out; + } + + ret = erase(fd, ERASE_SIZE_ALL, 0); + close(fd); + if (ret && ret != -ENOSYS) { + perror("erase"); + dfu->dfu_status = DFU_STATUS_errERASE; + goto out; + } + + ret = copy_file(DFU_TEMPFILE, dfu_file_entry->filename, 0); + if (ret) { + dfu->dfu_status = DFU_STATUS_errWRITE; + printf("copy file failed\n"); + goto out; + } + + dfu->dfu_state = DFU_STATE_dfuIDLE; + dfu_cleanup(dfu); + + return; +out: + dfu->dfu_state = DFU_STATE_dfuERROR; + dfu_cleanup(dfu); +} static int dfu_bind(struct usb_configuration *c, struct usb_function *f) @@ -223,6 +397,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 +456,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); @@ -327,6 +507,9 @@ static void dfu_cleanup(struct f_dfu *dfu) dfu_erased = 0; prog_erase = 0; + /* TODO: Right now, close and stat operation can be called + * in a poller, in dfu_abort and dfu_disable. */ + if (dfufd > 0) { close(dfufd); dfufd = -EINVAL; @@ -339,28 +522,15 @@ 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; + struct dfu_work *dw; - 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; - } - } + 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); - 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); - } + wq_queue_work(&dfu->wq, &dw->work); } static int handle_dnload(struct usb_function *f, const struct usb_ctrlrequest *ctrl) @@ -370,12 +540,7 @@ 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); - } + dfu->dfu_state = DFU_STATE_dfuMANIFEST_SYNC; return 0; } @@ -389,48 +554,18 @@ static int handle_dnload(struct usb_function *f, const struct usb_ctrlrequest *c static int handle_manifest(struct usb_function *f, const struct usb_ctrlrequest *ctrl) { struct f_dfu *dfu = func_to_dfu(f); - int ret; + struct dfu_work *dw; 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; - } + dw = xzalloc(sizeof(*dw)); + dw->dfu = dfu; + dw->task = dfu_do_copy; + wq_queue_work(&dfu->wq, &dw->work); } 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,20 +575,17 @@ 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); + /* RFC: I didn't found a better way to queue the usb response other + * than making dfu_do_read call usb_ep_queue after reading from file */ + 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; } @@ -474,7 +606,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 +633,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 +648,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: @@ -648,9 +762,6 @@ 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; - } switch (ctrl->bRequest) { case USB_REQ_DFU_GETSTATUS: value = dfu_status(f, ctrl); -- 2.17.1 _______________________________________________ barebox mailing list [email protected] http://lists.infradead.org/mailman/listinfo/barebox
