| Message ID | 1531393559-28958-2-git-send-email-alberto@amarulasolutions.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Philipp Tomsich |
| Headers | show |
| Series | Improve rockusb support in U-Boot | expand |
On Thu, 12 Jul 2018, Alberto Panizzo wrote: > Two consecutive rockusb_tx_write without waiting for request complete > do results in transfer reset of first request and thus no or incomplete > data transfer. This because rockusb_tx_write do use just one USB request > to keep serialization. > > So calls like: > rockusb_tx_write_str(emmc_id); > rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length, CSW_GOOD); > > was succeeding only when DEBUG was defined because the time spent > printing debug info was enough for transfer to complete. > > This patch fixes the issue adding a simple request complete handler > called rockusb_tx_write_csw to be set as complete handler of in_req > when sending back simple payload + CSW replies to commands. > > This new handler will always send CSW_GOOD replies because in case > of error the command callback itself must send back an error CSW as > unique reply to command. > > This patch fixes execution of: > $ rkdeveloptool rfi > when DEBUG is not defined. > > Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com> > --- > drivers/usb/gadget/f_rockusb.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c > index b8833d0..58e483c 100644 > --- a/drivers/usb/gadget/f_rockusb.c > +++ b/drivers/usb/gadget/f_rockusb.c > @@ -388,6 +388,20 @@ static int rockusb_tx_write_csw(u32 tag, int residue, u8 status, int size) > return rockusb_tx_write((char *)csw, size); > } > > +static void tx_handler_send_csw(struct usb_ep *ep, struct usb_request *req) > +{ > + struct f_rockusb *f_rkusb = get_rkusb(); > + int status = req->status; > + > + if (status) > + debug("status: %d ep '%s' trans: %d\n", > + status, ep->name, req->actual); This looks like an error-message. So shouldn't you always signal the error to the use, even if DEBUG is not defined? > + > + /* Return back to default in_req complete function after sending CSW */ > + req->complete = rockusb_complete; > + rockusb_tx_write_csw(f_rkusb->tag, 0, CSW_GOOD, USB_BULK_CS_WRAP_LEN); > +} > + > static unsigned int rx_bytes_expected(struct usb_ep *ep) > { > struct f_rockusb *f_rkusb = get_rkusb(); > @@ -496,13 +510,17 @@ static void cb_read_storage_id(struct usb_ep *ep, struct usb_request *req) > { > ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw, > sizeof(struct fsg_bulk_cb_wrap)); > + struct f_rockusb *f_rkusb = get_rkusb(); > char emmc_id[] = "EMMC "; > > printf("read storage id\n"); Should this really be a 'printf'? > memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN); > + > + /* Prepare for sending subsequent CSW_GOOD */ > + f_rkusb->tag = cbw->tag; > + f_rkusb->in_req->complete = tx_handler_send_csw; > + > rockusb_tx_write_str(emmc_id); > - rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length, CSW_GOOD, > - USB_BULK_CS_WRAP_LEN); > } > > static void cb_write_lba(struct usb_ep *ep, struct usb_request *req) >
diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c index b8833d0..58e483c 100644 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -388,6 +388,20 @@ static int rockusb_tx_write_csw(u32 tag, int residue, u8 status, int size) return rockusb_tx_write((char *)csw, size); } +static void tx_handler_send_csw(struct usb_ep *ep, struct usb_request *req) +{ + struct f_rockusb *f_rkusb = get_rkusb(); + int status = req->status; + + if (status) + debug("status: %d ep '%s' trans: %d\n", + status, ep->name, req->actual); + + /* Return back to default in_req complete function after sending CSW */ + req->complete = rockusb_complete; + rockusb_tx_write_csw(f_rkusb->tag, 0, CSW_GOOD, USB_BULK_CS_WRAP_LEN); +} + static unsigned int rx_bytes_expected(struct usb_ep *ep) { struct f_rockusb *f_rkusb = get_rkusb(); @@ -496,13 +510,17 @@ static void cb_read_storage_id(struct usb_ep *ep, struct usb_request *req) { ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw, sizeof(struct fsg_bulk_cb_wrap)); + struct f_rockusb *f_rkusb = get_rkusb(); char emmc_id[] = "EMMC "; printf("read storage id\n"); memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN); + + /* Prepare for sending subsequent CSW_GOOD */ + f_rkusb->tag = cbw->tag; + f_rkusb->in_req->complete = tx_handler_send_csw; + rockusb_tx_write_str(emmc_id); - rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length, CSW_GOOD, - USB_BULK_CS_WRAP_LEN); } static void cb_write_lba(struct usb_ep *ep, struct usb_request *req)
Two consecutive rockusb_tx_write without waiting for request complete do results in transfer reset of first request and thus no or incomplete data transfer. This because rockusb_tx_write do use just one USB request to keep serialization. So calls like: rockusb_tx_write_str(emmc_id); rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length, CSW_GOOD); was succeeding only when DEBUG was defined because the time spent printing debug info was enough for transfer to complete. This patch fixes the issue adding a simple request complete handler called rockusb_tx_write_csw to be set as complete handler of in_req when sending back simple payload + CSW replies to commands. This new handler will always send CSW_GOOD replies because in case of error the command callback itself must send back an error CSW as unique reply to command. This patch fixes execution of: $ rkdeveloptool rfi when DEBUG is not defined. Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com> --- drivers/usb/gadget/f_rockusb.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)