Hi Alberto,
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 une 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 request to complete.
Serialization by printf.....
This patch add a way to postpone sending csw after first rockusb_tx_write is completed (indeed inside rockusb_complete) fixing execution of: $ rkdeveloptool rfi when DEBUG is not defined.
Signed-off-by: Alberto Panizzo alberto@amarulasolutions.com
arch/arm/include/asm/arch-rockchip/f_rockusb.h | 1 + drivers/usb/gadget/f_rockusb.c | 37 ++++++++++++++++++++++---- 2 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/arch/arm/include/asm/arch-rockchip/f_rockusb.h b/arch/arm/include/asm/arch-rockchip/f_rockusb.h index 0b62771..f5cad8e 100644 --- a/arch/arm/include/asm/arch-rockchip/f_rockusb.h +++ b/arch/arm/include/asm/arch-rockchip/f_rockusb.h @@ -124,6 +124,7 @@ struct f_rockusb { int reboot_flag; void *buf; void *buf_head;
- struct bulk_cs_wrap *next_csw;
};
/* init rockusb device, tell rockusb which device you want to read/write*/ diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c index b8833d0..a39ad51 100644 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -97,6 +97,7 @@ static struct usb_gadget_strings *rkusb_strings[] = { static struct f_rockusb *rockusb_func; static void rx_handler_command(struct usb_ep *ep, struct usb_request *req); +static int rockusb_tx_write(const char *buffer, unsigned int buffer_size); static int rockusb_tx_write_csw(u32 tag, int residue, u8 status, int size); struct f_rockusb *get_rkusb(void) @@ -136,11 +137,22 @@ struct usb_endpoint_descriptor *hs)
static void rockusb_complete(struct usb_ep *ep, struct usb_request *req) {
- struct f_rockusb *f_rkusb = get_rkusb(); int status = req->status;
- if (!status)
return;- debug("status: %d ep '%s' trans: %d\n", status, ep->name,
req->actual);
- if (status)
debug("status: %d ep '%s' trans: %d\n",status, ep->name, req->actual);- /* Send Command Status on previous transfer complete */
- if (f_rkusb->next_csw) {
^^^^^^^^ - isn't this a bit misleading? We send the status for the previous transfer.
+#ifdef DEBUG
printcsw((char *)f_rkusb->next_csw);+#endif
rockusb_tx_write((char *)f_rkusb->next_csw,USB_BULK_CS_WRAP_LEN);- }
- f_rkusb->next_csw = NULL;
}
/* config the rockusb device*/ @@ -388,6 +400,21 @@ static int rockusb_tx_write_csw(u32 tag, int residue, u8 status, int size) return rockusb_tx_write((char *)csw, size); }
+struct bulk_cs_wrap g_next_csw;
You have added the pointer to struct bulk_cs_wrap_g to struct f_rockusb, and here we do have global definition.
Two issues with cache; alignment and padding.
Maybe it would be better to allocate it and store pointer int struct f_rockusb ?
+static void rockusb_tx_write_csw_on_complete(u32 tag, int residue, u8 status) +{
- struct f_rockusb *f_rkusb = get_rkusb();
- g_next_csw.signature = cpu_to_le32(USB_BULK_CS_SIG);
- g_next_csw.tag = tag;
- g_next_csw.residue = cpu_to_be32(residue);
- g_next_csw.status = status;
+#ifdef DEBUG
- printcsw((char *)&g_next_csw);
+#endif
- f_rkusb->next_csw = &g_next_csw;
+}
static unsigned int rx_bytes_expected(struct usb_ep *ep) { struct f_rockusb *f_rkusb = get_rkusb(); @@ -501,8 +528,8 @@ static void cb_read_storage_id(struct usb_ep *ep, struct usb_request *req) printf("read storage id\n"); memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN); rockusb_tx_write_str(emmc_id);
- rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length,
CSW_GOOD,
USB_BULK_CS_WRAP_LEN);
- rockusb_tx_write_csw_on_complete(cbw->tag,
cbw->data_transfer_length,
CSW_GOOD);
It seems like you are preparing the content of the csw structure to be ready when the completion is called. Am I right?
What I'm concerned about - with your patch we do have two functions with almost the same code - namely rockusb_tx_write_csw() and rockusb_tx_write_csw_on_complete().
Would it be possible to unify (reuse) the code?
One more remark - shouldn't we set content of g_next_csw in the rockusb_complete() ?
}
static void cb_write_lba(struct usb_ep *ep, struct usb_request *req)
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de