usb: dwc2: gadget: manage ep0 state in software

Manage ep0 state in software to add handling of status OUT stage.
Just toggling hsotg->setup in s3c_hsotg_handle_outdone leaves it in
wrong state in 2-stage control transfers.
Moreover, ensure that for setup-packet s3c_hsotg_handle_outdone is
called either from SetupDone or OutDone but not both. Dwc2 ip v3.00a
generates both SetupDone and OutDone on setup packets.

Tested-by: Robert Baldyga <r.baldyga@samsung.com>
Acked-by: Paul Zimmerman <paulz@synopsys.com>
Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@intel.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>

drivers/usb/dwc2/core.h

@@ -196,6 +196,15 @@ enum dwc2_lx_state {
 #define DWC2_G_P_LEGACY_TX_FIFO_SIZE {256, 256, 256, 256, 768, 768, 768, \
 					   768, 0, 0, 0, 0, 0, 0, 0}
 
+/* Gadget ep0 states */
+enum dwc2_ep0_state {
+	DWC2_EP0_SETUP,
+	DWC2_EP0_DATA_IN,
+	DWC2_EP0_DATA_OUT,
+	DWC2_EP0_STATUS_IN,
+	DWC2_EP0_STATUS_OUT,
+};
+
 /**
  * struct dwc2_core_params - Parameters for configuring the core
  *
@@ -563,7 +572,7 @@ struct dwc2_hw_params {
  * @ep0_buff:           Buffer for EP0 reply data, if needed.
  * @ctrl_buff:          Buffer for EP0 control requests.
  * @ctrl_req:           Request for EP0 control packets.
- * @setup:              NAK management for EP0 SETUP
+ * @ep0_state:          EP0 control transfers state
  * @last_rst:           Time of last reset
  * @eps:                The endpoints being supplied to the gadget framework
  * @g_using_dma:          Indicate if dma usage is enabled
@@ -696,11 +705,11 @@ struct dwc2_hsotg {
 	struct usb_request *ctrl_req;
 	void *ep0_buff;
 	void *ctrl_buff;
+	enum dwc2_ep0_state ep0_state;
 
 	struct usb_gadget gadget;
 	unsigned int enabled:1;
 	unsigned int connected:1;
-	unsigned int setup:1;
 	unsigned long last_rst;
 	struct s3c_hsotg_ep *eps_in[MAX_EPS_CHANNELS];
 	struct s3c_hsotg_ep *eps_out[MAX_EPS_CHANNELS];

drivers/usb/dwc2/gadget.c

@@ -638,15 +638,12 @@ static void s3c_hsotg_start_req(struct dwc2_hsotg *hsotg,
 	ctrl |= DXEPCTL_EPENA;	/* ensure ep enabled */
 	ctrl |= DXEPCTL_USBACTEP;
 
-	dev_dbg(hsotg->dev, "setup req:%d\n", hsotg->setup);
+	dev_dbg(hsotg->dev, "ep0 state:%d\n", hsotg->ep0_state);
 
 	/* For Setup request do not clear NAK */
-	if (hsotg->setup && index == 0)
-		hsotg->setup = 0;
-	else
+	if (!(index == 0 && hsotg->ep0_state == DWC2_EP0_SETUP))
 		ctrl |= DXEPCTL_CNAK;	/* clear NAK set by core */
 
-
 	dev_dbg(hsotg->dev, "%s: DxEPCTL=0x%08x\n", __func__, ctrl);
 	writel(ctrl, hsotg->regs + epctrl_reg);
 
@@ -865,8 +862,6 @@ static int s3c_hsotg_send_reply(struct dwc2_hsotg *hsotg,
 
 	if (length)
 		memcpy(req->buf, buff, length);
-	else
-		ep->sent_zlp = 1;
 
 	ret = s3c_hsotg_ep_queue(&ep->ep, req, GFP_ATOMIC);
 	if (ret) {
@@ -1080,26 +1075,20 @@ static void s3c_hsotg_process_control(struct dwc2_hsotg *hsotg,
 	int ret = 0;
 	u32 dcfg;
 
-	ep0->sent_zlp = 0;
-
 	dev_dbg(hsotg->dev, "ctrl Req=%02x, Type=%02x, V=%04x, L=%04x\n",
 		 ctrl->bRequest, ctrl->bRequestType,
 		 ctrl->wValue, ctrl->wLength);
 
-	/*
-	 * record the direction of the request, for later use when enquing
-	 * packets onto EP0.
-	 */
-
-	ep0->dir_in = (ctrl->bRequestType & USB_DIR_IN) ? 1 : 0;
-	dev_dbg(hsotg->dev, "ctrl: dir_in=%d\n", ep0->dir_in);
-
-	/*
-	 * if we've no data with this request, then the last part of the
-	 * transaction is going to implicitly be IN.
-	 */
-	if (ctrl->wLength == 0)
+	if (ctrl->wLength == 0) {
+		ep0->dir_in = 1;
+		hsotg->ep0_state = DWC2_EP0_STATUS_IN;
+	} else if (ctrl->bRequestType & USB_DIR_IN) {
 		ep0->dir_in = 1;
+		hsotg->ep0_state = DWC2_EP0_DATA_IN;
+	} else {
+		ep0->dir_in = 0;
+		hsotg->ep0_state = DWC2_EP0_DATA_OUT;
+	}
 
 	if ((ctrl->bRequestType & USB_TYPE_MASK) == USB_TYPE_STANDARD) {
 		switch (ctrl->bRequest) {
@@ -1198,6 +1187,8 @@ static void s3c_hsotg_enqueue_setup(struct dwc2_hsotg *hsotg)
 	}
 
 	hsotg->eps_out[0]->dir_in = 0;
+	hsotg->eps_out[0]->sent_zlp = 0;
+	hsotg->ep0_state = DWC2_EP0_SETUP;
 
 	ret = s3c_hsotg_ep_queue(&hsotg->eps_out[0]->ep, req, GFP_ATOMIC);
 	if (ret < 0) {
@@ -1209,6 +1200,27 @@ static void s3c_hsotg_enqueue_setup(struct dwc2_hsotg *hsotg)
 	}
 }
 
+static void s3c_hsotg_program_zlp(struct dwc2_hsotg *hsotg,
+					struct s3c_hsotg_ep *hs_ep)
+{
+	u32 ctrl;
+	u8 index = hs_ep->index;
+	u32 epctl_reg = hs_ep->dir_in ? DIEPCTL(index) : DOEPCTL(index);
+	u32 epsiz_reg = hs_ep->dir_in ? DIEPTSIZ(index) : DOEPTSIZ(index);
+
+	dev_dbg(hsotg->dev, "Sending zero-length packet on ep%d\n", index);
+
+	writel(DXEPTSIZ_MC(1) | DXEPTSIZ_PKTCNT(1) |
+			DXEPTSIZ_XFERSIZE(0), hsotg->regs +
+			epsiz_reg);
+
+	ctrl = readl(hsotg->regs + epctl_reg);
+	ctrl |= DXEPCTL_CNAK;  /* clear NAK set by core */
+	ctrl |= DXEPCTL_EPENA; /* ensure ep enabled */
+	ctrl |= DXEPCTL_USBACTEP;
+	writel(ctrl, hsotg->regs + epctl_reg);
+}
+
 /**
  * s3c_hsotg_complete_request - complete a request given to us
  * @hsotg: The device state.
@@ -1341,9 +1353,9 @@ static void s3c_hsotg_rx_data(struct dwc2_hsotg *hsotg, int ep_idx, int size)
 }
 
 /**
- * s3c_hsotg_send_zlp - send zero-length packet on control endpoint
+ * s3c_hsotg_ep0_zlp - send/receive zero-length packet on control endpoint
  * @hsotg: The device instance
- * @req: The request currently on this endpoint
+ * @dir_in: If IN zlp
  *
  * Generate a zero-length IN packet request for terminating a SETUP
  * transaction.
@@ -1352,51 +1364,25 @@ static void s3c_hsotg_rx_data(struct dwc2_hsotg *hsotg, int ep_idx, int size)
  * currently believed that we do not need to wait for any space in
  * the TxFIFO.
  */
-static void s3c_hsotg_send_zlp(struct dwc2_hsotg *hsotg,
-			       struct s3c_hsotg_req *req)
+static void s3c_hsotg_ep0_zlp(struct dwc2_hsotg *hsotg, bool dir_in)
 {
-	u32 ctrl;
-
-	if (!req) {
-		dev_warn(hsotg->dev, "%s: no request?\n", __func__);
-		return;
-	}
-
-	if (req->req.length == 0) {
-		hsotg->eps_out[0]->sent_zlp = 1;
-		s3c_hsotg_enqueue_setup(hsotg);
-		return;
-	}
-
 	/* eps_out[0] is used in both directions */
-	hsotg->eps_out[0]->dir_in = 1;
-	hsotg->eps_out[0]->sent_zlp = 1;
-
-	dev_dbg(hsotg->dev, "sending zero-length packet\n");
-
-	/* issue a zero-sized packet to terminate this */
-	writel(DXEPTSIZ_MC(1) | DXEPTSIZ_PKTCNT(1) |
-	       DXEPTSIZ_XFERSIZE(0), hsotg->regs + DIEPTSIZ(0));
+	hsotg->eps_out[0]->dir_in = dir_in;
+	hsotg->ep0_state = dir_in ? DWC2_EP0_STATUS_IN : DWC2_EP0_STATUS_OUT;
 
-	ctrl = readl(hsotg->regs + DIEPCTL0);
-	ctrl |= DXEPCTL_CNAK;  /* clear NAK set by core */
-	ctrl |= DXEPCTL_EPENA; /* ensure ep enabled */
-	ctrl |= DXEPCTL_USBACTEP;
-	writel(ctrl, hsotg->regs + DIEPCTL0);
+	s3c_hsotg_program_zlp(hsotg, hsotg->eps_out[0]);
 }
 
 /**
  * s3c_hsotg_handle_outdone - handle receiving OutDone/SetupDone from RXFIFO
  * @hsotg: The device instance
  * @epnum: The endpoint received from
- * @was_setup: Set if processing a SetupDone event.
  *
  * The RXFIFO has delivered an OutDone event, which means that the data
  * transfer for an OUT endpoint has been completed, either by a short
  * packet or by the finish of a transfer.
  */
-static void s3c_hsotg_handle_outdone(struct dwc2_hsotg *hsotg,
-				     int epnum, bool was_setup)
+static void s3c_hsotg_handle_outdone(struct dwc2_hsotg *hsotg, int epnum)
 {
 	u32 epsize = readl(hsotg->regs + DOEPTSIZ(epnum));
 	struct s3c_hsotg_ep *hs_ep = hsotg->eps_out[epnum];
@@ -1410,6 +1396,13 @@ static void s3c_hsotg_handle_outdone(struct dwc2_hsotg *hsotg,
 		return;
 	}
 
+	if (epnum == 0 && hsotg->ep0_state == DWC2_EP0_STATUS_OUT) {
+		dev_dbg(hsotg->dev, "zlp packet received\n");
+		s3c_hsotg_complete_request(hsotg, hs_ep, hs_req, 0);
+		s3c_hsotg_enqueue_setup(hsotg);
+		return;
+	}
+
 	if (using_dma(hsotg)) {
 		unsigned size_done;
 
@@ -1432,12 +1425,6 @@ static void s3c_hsotg_handle_outdone(struct dwc2_hsotg *hsotg,
 	if (req->actual < req->length && size_left == 0) {
 		s3c_hsotg_start_req(hsotg, hs_ep, hs_req, true);
 		return;
-	} else if (epnum == 0) {
-		/*
-		 * After was_setup = 1 =>
-		 * set CNAK for non Setup requests
-		 */
-		hsotg->setup = was_setup ? 0 : 1;
 	}
 
 	if (req->actual < req->length && req->short_not_ok) {
@@ -1450,13 +1437,10 @@ static void s3c_hsotg_handle_outdone(struct dwc2_hsotg *hsotg,
 		 */
 	}
 
-	if (epnum == 0) {
-		/*
-		 * Condition req->complete != s3c_hsotg_complete_setup says:
-		 * send ZLP when we have an asynchronous request from gadget
-		 */
-		if (!was_setup && req->complete != s3c_hsotg_complete_setup)
-			s3c_hsotg_send_zlp(hsotg, hs_req);
+	if (epnum == 0 && hsotg->ep0_state == DWC2_EP0_DATA_OUT) {
+		/* Move to STATUS IN */
+		s3c_hsotg_ep0_zlp(hsotg, true);
+		return;
 	}
 
 	s3c_hsotg_complete_request(hsotg, hs_ep, hs_req, result);
@@ -1522,7 +1506,7 @@ static void s3c_hsotg_handle_rx(struct dwc2_hsotg *hsotg)
 			s3c_hsotg_read_frameno(hsotg));
 
 		if (!using_dma(hsotg))
-			s3c_hsotg_handle_outdone(hsotg, epnum, false);
+			s3c_hsotg_handle_outdone(hsotg, epnum);
 		break;
 
 	case GRXSTS_PKTSTS_SETUPDONE:
@@ -1530,8 +1514,13 @@ static void s3c_hsotg_handle_rx(struct dwc2_hsotg *hsotg)
 			"SetupDone (Frame=0x%08x, DOPEPCTL=0x%08x)\n",
 			s3c_hsotg_read_frameno(hsotg),
 			readl(hsotg->regs + DOEPCTL(0)));
-
-		s3c_hsotg_handle_outdone(hsotg, epnum, true);
+		/*
+		 * Call s3c_hsotg_handle_outdone here if it was not called from
+		 * GRXSTS_PKTSTS_OUTDONE. That is, if the core didn't
+		 * generate GRXSTS_PKTSTS_OUTDONE for setup packet.
+		 */
+		if (hsotg->ep0_state == DWC2_EP0_SETUP)
+			s3c_hsotg_handle_outdone(hsotg, epnum);
 		break;
 
 	case GRXSTS_PKTSTS_OUTRX:
@@ -1544,6 +1533,8 @@ static void s3c_hsotg_handle_rx(struct dwc2_hsotg *hsotg)
 			s3c_hsotg_read_frameno(hsotg),
 			readl(hsotg->regs + DOEPCTL(0)));
 
+		WARN_ON(hsotg->ep0_state != DWC2_EP0_SETUP);
+
 		s3c_hsotg_rx_data(hsotg, epnum, size);
 		break;
 
@@ -1723,9 +1714,10 @@ static void s3c_hsotg_complete_in(struct dwc2_hsotg *hsotg,
 	}
 
 	/* Finish ZLP handling for IN EP0 transactions */
-	if (hsotg->eps_out[0]->sent_zlp) {
-		dev_dbg(hsotg->dev, "zlp packet received\n");
+	if (hs_ep->index == 0 && hsotg->ep0_state == DWC2_EP0_STATUS_IN) {
+		dev_dbg(hsotg->dev, "zlp packet sent\n");
 		s3c_hsotg_complete_request(hsotg, hs_ep, hs_req, 0);
+		s3c_hsotg_enqueue_setup(hsotg);
 		return;
 	}
 
@@ -1767,7 +1759,7 @@ static void s3c_hsotg_complete_in(struct dwc2_hsotg *hsotg,
 	    !(hs_req->req.length % hs_ep->ep.maxpacket)) {
 
 		dev_dbg(hsotg->dev, "ep0 zlp IN packet sent\n");
-		s3c_hsotg_send_zlp(hsotg, hs_req);
+		s3c_hsotg_program_zlp(hsotg, hs_ep);
 
 		return;
 	}
@@ -1775,8 +1767,16 @@ static void s3c_hsotg_complete_in(struct dwc2_hsotg *hsotg,
 	if (!size_left && hs_req->req.actual < hs_req->req.length) {
 		dev_dbg(hsotg->dev, "%s trying more for req...\n", __func__);
 		s3c_hsotg_start_req(hsotg, hs_ep, hs_req, true);
-	} else
-		s3c_hsotg_complete_request(hsotg, hs_ep, hs_req, 0);
+		return;
+	}
+
+	if (hs_ep->index == 0 && hsotg->ep0_state == DWC2_EP0_DATA_IN) {
+		/* Move to STATUS OUT */
+		s3c_hsotg_ep0_zlp(hsotg, false);
+		return;
+	}
+
+	s3c_hsotg_complete_request(hsotg, hs_ep, hs_req, 0);
 }
 
 /**
@@ -1845,7 +1845,7 @@ static void s3c_hsotg_epint(struct dwc2_hsotg *hsotg, unsigned int idx,
 			 * as we ignore the RXFIFO.
 			 */
 
-			s3c_hsotg_handle_outdone(hsotg, idx, false);
+			s3c_hsotg_handle_outdone(hsotg, idx);
 		}
 	}
 
@@ -1884,7 +1884,7 @@ static void s3c_hsotg_epint(struct dwc2_hsotg *hsotg, unsigned int idx,
 			if (dir_in)
 				WARN_ON_ONCE(1);
 			else
-				s3c_hsotg_handle_outdone(hsotg, 0, true);
+				s3c_hsotg_handle_outdone(hsotg, 0);
 		}
 	}