Skip to contentSkip to conversation

usb: renesas_usbhs: fix the condition of is_done in usbhsf_dma_push_done

PATCH In review Yoshihiro Shimoda 2 participants 3 messages Aug 22, 2014 – Sep 10, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 3/4 usb: renesas_usbhs: fix the condition of is_done in usbhsf_dma_push_done

This patch fixes the condition of is_done in usbhsf_dma_push_done(). This function will be called after a transmission finished by DMAC. So, the function should check if the transmission packet is short packet or not. Also the function should call try_run to send the zero packet by the pio handler if the "*is_done" is not set. Otherwize, the transaction will not finish if a gadget driver sets the "zero" flag in a transmission.

---
 drivers/usb/renesas_usbhs/fifo.c |   17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
drivers/usb/renesas_usbhs/fifo.c +15 -2
index 3efece3..1564829 100644
@@ -889,16 +889,29 @@ usbhsf_pio_prepare_push:
 static int usbhsf_dma_push_done(struct usbhs_pkt *pkt, int *is_done)
 {
 	struct usbhs_pipe *pipe = pkt->pipe;
+	int is_short = pkt->trans % usbhs_pipe_get_maxpacket(pipe);
 
-	pkt->actual = pkt->trans;
+	pkt->actual += pkt->trans;
+
+	if (pkt->actual < pkt->length)
+		*is_done = 0;		/* there are remainder data */
+	else if (is_short)
+		*is_done = 1;		/* short packet */
+	else
+		*is_done = !pkt->zero;	/* send zero packet? */
 
-	*is_done = !pkt->zero;	/* send zero packet ? */
 	usbhs_pipe_running(pipe, !*is_done);
 
 	usbhsf_dma_stop(pipe, pipe->fifo);
 	usbhsf_dma_unmap(pkt);
 	usbhsf_fifo_unselect(pipe, pipe->fifo);
 
+	if (!*is_done) {
+		/* change handler to PIO */
+		pkt->handler = &usbhs_fifo_pio_push_handler;
+		return pkt->handler->try_run(pkt, is_done);
+	}
+
 	return 0;
 }
 
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 3/4 usb: renesas_usbhs: fix the timing of dcp_control_transfer_done

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().

---
 drivers/usb/renesas_usbhs/fifo.c       |   16 ----------------
 drivers/usb/renesas_usbhs/mod_gadget.c |    3 +++
 2 files changed, 3 insertions(+), 16 deletions(-)
drivers/usb/renesas_usbhs/fifo.c +0 -16
index b0c97a3..0e07925 100644
@@ -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);
 
drivers/usb/renesas_usbhs/mod_gadget.c +3 -0
index b80c802..ba890c1 100644
@@ -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;
 	}
Signature · 1 line
1.7.9.5
#2 lore raw

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

In reply to Yoshihiro Shimoda
Quoted 11 lines · Yoshihiro Shimoda

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
Quoted 7 lines

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().

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

Signature · 1 line
balbi