[U-Boot,v3,1/9] usb: rockchip: fix command failed on host side due to missing data

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
Related show

Commit Message

Alberto Panizzo July 12, 2018, 11:05 a.m.
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(-)

Comments

Philipp Tomsich July 20, 2018, 4:50 p.m. | #1
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)
>

Patch

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)