linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: linux-usb@vger.kernel.org
Cc: balbi@kernel.org, Joel Stanley <joel@jms.id.au>,
	Alan Stern <stern@rowland.harvard.edu>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>
Subject: [PATCH 03/10] usb: gadget: aspeed: Fix EP0 stall handling
Date: Sat,  6 Jul 2019 10:53:38 +1000	[thread overview]
Message-ID: <20190706005345.18131-4-benh@kernel.crashing.org> (raw)
In-Reply-To: <20190706005345.18131-1-benh@kernel.crashing.org>

When stalling EP0, we need to wait for an ACK interrupt,
otherwise we may get out of sync on the next setup packet
data phase. Also we need to ignore the direction when
processing that interrupt as the HW reports a potential
mismatch.

Implement this by adding a stall state to EP0. This fixes
some reported issues with mass storage and some hosts.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/usb/gadget/udc/aspeed-vhub/ep0.c  | 48 +++++++++++++++--------
 drivers/usb/gadget/udc/aspeed-vhub/vhub.h |  1 +
 2 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/gadget/udc/aspeed-vhub/ep0.c b/drivers/usb/gadget/udc/aspeed-vhub/ep0.c
index 5054c6343ead..d27d97b97b5c 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/ep0.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/ep0.c
@@ -105,18 +105,20 @@ void ast_vhub_ep0_handle_setup(struct ast_vhub_ep *ep)
 	       (crq.bRequestType & USB_DIR_IN) ? "in" : "out",
 	       ep->ep0.state);
 
-	/* Check our state, cancel pending requests if needed */
-	if (ep->ep0.state != ep0_state_token) {
+	/*
+	 * Check our state, cancel pending requests if needed
+	 *
+	 * Note: Under some circumstances, we can get a new setup
+	 * packet while waiting for the stall ack, just accept it.
+	 *
+	 * In any case, a SETUP packet in wrong state should have
+	 * reset the HW state machine, so let's just log, nuke
+	 * requests, move on.
+	 */
+	if (ep->ep0.state != ep0_state_token &&
+	    ep->ep0.state != ep0_state_stall) {
 		EPDBG(ep, "wrong state\n");
 		ast_vhub_nuke(ep, -EIO);
-
-		/*
-		 * Accept the packet regardless, this seems to happen
-		 * when stalling a SETUP packet that has an OUT data
-		 * phase.
-		 */
-		ast_vhub_nuke(ep, 0);
-		goto stall;
 	}
 
 	/* Calculate next state for EP0 */
@@ -165,7 +167,7 @@ void ast_vhub_ep0_handle_setup(struct ast_vhub_ep *ep)
  stall:
 	EPDBG(ep, "stalling\n");
 	writel(VHUB_EP0_CTRL_STALL, ep->ep0.ctlstat);
-	ep->ep0.state = ep0_state_status;
+	ep->ep0.state = ep0_state_stall;
 	ep->ep0.dir_in = false;
 	return;
 
@@ -299,8 +301,8 @@ void ast_vhub_ep0_handle_ack(struct ast_vhub_ep *ep, bool in_ack)
 		if ((ep->ep0.dir_in && (stat & VHUB_EP0_TX_BUFF_RDY)) ||
 		    (!ep->ep0.dir_in && (stat & VHUB_EP0_RX_BUFF_RDY)) ||
 		    (ep->ep0.dir_in != in_ack)) {
+			/* In that case, ignore interrupt */
 			dev_warn(dev, "irq state mismatch");
-			stall = true;
 			break;
 		}
 		/*
@@ -335,12 +337,22 @@ void ast_vhub_ep0_handle_ack(struct ast_vhub_ep *ep, bool in_ack)
 			dev_warn(dev, "status direction mismatch\n");
 			stall = true;
 		}
+		break;
+	case ep0_state_stall:
+		/*
+		 * There shouldn't be any request left, but nuke just in case
+		 * otherwise the stale request will block subsequent ones
+		 */
+		ast_vhub_nuke(ep, -EIO);
+		break;
 	}
 
-	/* Reset to token state */
-	ep->ep0.state = ep0_state_token;
-	if (stall)
+	/* Reset to token state or stall */
+	if (stall) {
 		writel(VHUB_EP0_CTRL_STALL, ep->ep0.ctlstat);
+		ep->ep0.state = ep0_state_stall;
+	} else
+		ep->ep0.state = ep0_state_token;
 }
 
 static int ast_vhub_ep0_queue(struct usb_ep* u_ep, struct usb_request *u_req,
@@ -390,8 +402,12 @@ static int ast_vhub_ep0_queue(struct usb_ep* u_ep, struct usb_request *u_req,
 	spin_lock_irqsave(&vhub->lock, flags);
 
 	/* EP0 can only support a single request at a time */
-	if (!list_empty(&ep->queue) || ep->ep0.state == ep0_state_token) {
+	if (!list_empty(&ep->queue) ||
+	    ep->ep0.state == ep0_state_token ||
+	    ep->ep0.state == ep0_state_stall) {
 		dev_warn(dev, "EP0: Request in wrong state\n");
+	        EPVDBG(ep, "EP0: list_empty=%d state=%d\n",
+		       list_empty(&ep->queue), ep->ep0.state);
 		spin_unlock_irqrestore(&vhub->lock, flags);
 		return -EBUSY;
 	}
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/vhub.h b/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
index 2e7ef387f4f0..00f922604a1c 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
+++ b/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
@@ -257,6 +257,7 @@ enum ep0_state {
 	ep0_state_token,
 	ep0_state_data,
 	ep0_state_status,
+	ep0_state_stall,
 };
 
 /*
-- 
2.17.1


  parent reply	other threads:[~2019-07-06  0:54 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-06  0:53 [PATCH 00/10] usb: gadget: aspeed: Bug fixes Benjamin Herrenschmidt
2019-07-06  0:53 ` [PATCH 01/10] usb: gadget: aspeed: Don't set port enable change bit on reset Benjamin Herrenschmidt
2019-07-06  0:53 ` [PATCH 02/10] usb: gadget: aspeed: Cleanup EP0 state on port reset Benjamin Herrenschmidt
2019-07-06  8:11   ` Sergei Shtylyov
2019-07-06 12:34     ` Benjamin Herrenschmidt
2019-07-06  0:53 ` Benjamin Herrenschmidt [this message]
2019-07-06  0:53 ` [PATCH 04/10] usb: gadget: aspeed: Improve debugging when nuking Benjamin Herrenschmidt
2019-07-06  0:53 ` [PATCH 05/10] usb: gadget: aspeed: Don't reject requests on suspended devices Benjamin Herrenschmidt
2019-07-06  0:53 ` [PATCH 06/10] usb: gadget: aspeed: Check suspend/resume callback existence Benjamin Herrenschmidt
2019-07-06  0:53 ` [PATCH 07/10] usb: gadget: aspeed: Rework the reset logic Benjamin Herrenschmidt
2019-07-06  0:53 ` [PATCH 08/10] usb: gadget: aspeed: Remove unused "suspended" flag Benjamin Herrenschmidt
2019-07-06  0:53 ` [PATCH 09/10] usb: Add definitions for the USB2.0 hub TT requests Benjamin Herrenschmidt
2019-07-06  0:53 ` [PATCH 10/10] usb: gadget: aspeed: Implement dummy " Benjamin Herrenschmidt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190706005345.18131-4-benh@kernel.crashing.org \
    --to=benh@kernel.crashing.org \
    --cc=balbi@kernel.org \
    --cc=joel@jms.id.au \
    --cc=linux-usb@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).