diff mbox

[3/4] usb: renesas_usbhs: fix the timing of dcp_control_transfer_done

Message ID 5410291D.4050703@renesas.com (mailing list archive)
State Superseded
Headers show

Commit Message

Yoshihiro Shimoda Sept. 10, 2014, 10:34 a.m. UTC
According to the datasheet, this driver should clear the INTSTS0.CTRT
bit before this controller detects the next stage transition. Otherwise,
the driver may not be able to clear the bit after the controller went to
the next stage transition. After that, the driver will not be able to
clear the INTSTS0.VALID, and a usb control transfer will not finish
finally.

If we use the testusb tool, it is easy to reproduce this issue:

 # testusb -a -t 10

Since the previous code handled a data stage and a status stage in
the usbhsf_pio_try_push(), it may not clear the INTSTS0.CTRT at the
right timing.
So, this patch changes the timing of usbhs_dcp_control_transfer_done()
to the usbhsg_irq_ctrl_stage().

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/usb/renesas_usbhs/fifo.c       |   16 ----------------
 drivers/usb/renesas_usbhs/mod_gadget.c |    3 +++
 2 files changed, 3 insertions(+), 16 deletions(-)

Comments

Felipe Balbi Sept. 10, 2014, 1:50 p.m. UTC | #1
On Wed, Sep 10, 2014 at 07:34:05PM +0900, Yoshihiro Shimoda wrote:
> According to the datasheet, this driver should clear the INTSTS0.CTRT
> bit before this controller detects the next stage transition. Otherwise,
> the driver may not be able to clear the bit after the controller went to
> the next stage transition. After that, the driver will not be able to
> clear the INTSTS0.VALID, and a usb control transfer will not finish
> finally.
> 
> If we use the testusb tool, it is easy to reproduce this issue:
> 
>  # testusb -a -t 10
> 
> Since the previous code handled a data stage and a status stage in
> the usbhsf_pio_try_push(), it may not clear the INTSTS0.CTRT at the
> right timing.
> So, this patch changes the timing of usbhs_dcp_control_transfer_done()
> to the usbhsg_irq_ctrl_stage().
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

which commit does this fix ? Do we need to Cc stable ?
diff mbox

Patch

diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
index b0c97a3..0e07925 100644
--- a/drivers/usb/renesas_usbhs/fifo.c
+++ b/drivers/usb/renesas_usbhs/fifo.c
@@ -577,14 +577,6 @@  static int usbhsf_pio_try_push(struct usbhs_pkt *pkt, int *is_done)
 		usbhs_pipe_number(pipe),
 		pkt->length, pkt->actual, *is_done, pkt->zero);

-	/*
-	 * Transmission end
-	 */
-	if (*is_done) {
-		if (usbhs_pipe_is_dcp(pipe))
-			usbhs_dcp_control_transfer_done(pipe);
-	}
-
 	usbhsf_fifo_unselect(pipe, fifo);

 	return 0;
@@ -722,14 +714,6 @@  usbhs_fifo_read_end:
 		usbhs_pipe_number(pipe),
 		pkt->length, pkt->actual, *is_done, pkt->zero);

-	/*
-	 * Transmission end
-	 */
-	if (*is_done) {
-		if (usbhs_pipe_is_dcp(pipe))
-			usbhs_dcp_control_transfer_done(pipe);
-	}
-
 usbhs_fifo_read_busy:
 	usbhsf_fifo_unselect(pipe, fifo);

diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c b/drivers/usb/renesas_usbhs/mod_gadget.c
index b80c802..ba890c1 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -486,6 +486,9 @@  static int usbhsg_irq_ctrl_stage(struct usbhs_priv *priv,
 	case NODATA_STATUS_STAGE:
 		pipe->handler = &usbhs_ctrl_stage_end_handler;
 		break;
+	case READ_STATUS_STAGE:
+	case WRITE_STATUS_STAGE:
+		usbhs_dcp_control_transfer_done(pipe);
 	default:
 		return ret;
 	}