path: root/meta-openbmc-mods/meta-common/recipes-kernel/linux/linux-aspeed/0079-usb-gadget-aspeed-backport-aspeed-vhub-bug-fixes.patch

From 0475ac3698cf3d95d78b0230418ec7ef5fdc62c7 Mon Sep 17 00:00:00 2001
From: Jae Hyun Yoo <jae.hyun.yoo@intel.com>
Date: Tue, 29 Oct 2019 11:42:08 -0700
Subject: [PATCH] usb: gadget: aspeed: backport aspeed vhub bug fixes

usb: gadget: aspeed: Implement dummy hub TT requests

We just accept them instead of stalling and return
zeros on GetTTState.

usb: Add definitions for the USB2.0 hub TT requests

usb: gadget: aspeed: Improve debugging when nuking

When nuking requests, it's useful to display how many were
actually nuked. It has proven handy when debugging issues
where EP0 went in a wrong state.

usb: gadget: aspeed: Remove unused "suspended" flag

The state bit in the hub is sufficient

usb: gadget: aspeed: Rework the reset logic

We had some dodgy code using the speed setting to decide whether a
port reset would reset the device or just enable it.

Instead, if the device is disabled and has a gadget attached, a
reset will enable it. If it's already enabled, a reset will
reset it.

usb: gadget: aspeed: Check suspend/resume callback existence

.. before calling them

usb: gadget: aspeed: Don't reject requests on suspended devices

A disconnect may just suspend the hub in absence of a physical
disconnect detection. If we start rejecting requests, the mass
storage function gets into a spin trying to requeue the same
request for ever and hangs.

usb: gadget: aspeed: Fix EP0 stall handling

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.

usb: gadget: aspeed: Cleanup EP0 state on port reset

Otherwise, we can have a stale state after a disconnect and reconnect
causing errors on the first SETUP packet to the device.

causing errors on the first SETUP packet to the device.

usb: gadget: aspeed: Don't set port enable change bit on reset

This bit should be only set when the port enable goes down, for
example, on errors. Not when it gets set after a port reset. Some
USB stacks seem to be sensitive to this and fails enumeration.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@intel.com>
---
 drivers/usb/gadget/udc/aspeed-vhub/core.c |  7 +--
 drivers/usb/gadget/udc/aspeed-vhub/dev.c  | 80 +++++++++++++++----------------
 drivers/usb/gadget/udc/aspeed-vhub/ep0.c  | 59 ++++++++++++++++-------
 drivers/usb/gadget/udc/aspeed-vhub/epn.c  |  2 +-
 drivers/usb/gadget/udc/aspeed-vhub/hub.c  | 15 +++++-
 drivers/usb/gadget/udc/aspeed-vhub/vhub.h |  3 +-
 include/linux/usb/hcd.h                   |  4 ++
 7 files changed, 107 insertions(+), 63 deletions(-)

diff --git a/drivers/usb/gadget/udc/aspeed-vhub/core.c b/drivers/usb/gadget/udc/aspeed-vhub/core.c
index db3628be38c0..90b134d5dca9 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/core.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/core.c
@@ -65,14 +65,16 @@ void ast_vhub_done(struct ast_vhub_ep *ep, struct ast_vhub_req *req,
 void ast_vhub_nuke(struct ast_vhub_ep *ep, int status)
 {
 	struct ast_vhub_req *req;
-
-	EPDBG(ep, "Nuking\n");
+	int count = 0;
 
 	/* Beware, lock will be dropped & req-acquired by done() */
 	while (!list_empty(&ep->queue)) {
 		req = list_first_entry(&ep->queue, struct ast_vhub_req, queue);
 		ast_vhub_done(ep, req, status);
+		count++;
 	}
+	if (count)
+		EPDBG(ep, "Nuked %d request(s)\n", count);
 }
 
 struct usb_request *ast_vhub_alloc_request(struct usb_ep *u_ep,
@@ -348,7 +350,6 @@ static int ast_vhub_probe(struct platform_device *pdev)
 	/* Find interrupt and install handler */
 	vhub->irq = platform_get_irq(pdev, 0);
 	if (vhub->irq < 0) {
-		dev_err(&pdev->dev, "Failed to get interrupt\n");
 		rc = vhub->irq;
 		goto err;
 	}
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/dev.c b/drivers/usb/gadget/udc/aspeed-vhub/dev.c
index 6b1b16b17d7d..4008e7a51188 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/dev.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/dev.c
@@ -50,11 +50,14 @@ void ast_vhub_dev_irq(struct ast_vhub_dev *d)
 
 static void ast_vhub_dev_enable(struct ast_vhub_dev *d)
 {
-	u32 reg, hmsk;
+	u32 reg, hmsk, i;
 
 	if (d->enabled)
 		return;
 
+	/* Cleanup EP0 state */
+	ast_vhub_reset_ep0(d);
+
 	/* Enable device and its EP0 interrupts */
 	reg = VHUB_DEV_EN_ENABLE_PORT |
 		VHUB_DEV_EN_EP0_IN_ACK_IRQEN |
@@ -73,6 +76,19 @@ static void ast_vhub_dev_enable(struct ast_vhub_dev *d)
 	/* Set EP0 DMA buffer address */
 	writel(d->ep0.buf_dma, d->regs + AST_VHUB_DEV_EP0_DATA);
 
+	/* Clear stall on all EPs */
+	for (i = 0; i < AST_VHUB_NUM_GEN_EPs; i++) {
+		struct ast_vhub_ep *ep = d->epns[i];
+
+		if (ep && (ep->epn.stalled || ep->epn.wedged)) {
+			ep->epn.stalled = false;
+			ep->epn.wedged = false;
+			ast_vhub_update_epn_stall(ep);
+		}
+	}
+
+	/* Additional cleanups */
+	d->wakeup_en = false;
 	d->enabled = true;
 }
 
@@ -93,7 +109,6 @@ static void ast_vhub_dev_disable(struct ast_vhub_dev *d)
 	writel(0, d->regs + AST_VHUB_DEV_EN_CTRL);
 	d->gadget.speed = USB_SPEED_UNKNOWN;
 	d->enabled = false;
-	d->suspended = false;
 }
 
 static int ast_vhub_dev_feature(struct ast_vhub_dev *d,
@@ -201,14 +216,19 @@ int ast_vhub_std_dev_request(struct ast_vhub_ep *ep,
 	u16 wValue, wIndex;
 
 	/* No driver, we shouldn't be enabled ... */
-	if (!d->driver || !d->enabled || d->suspended) {
+	if (!d->driver || !d->enabled) {
 		EPDBG(ep,
-		      "Device is wrong state driver=%p enabled=%d"
-		      " suspended=%d\n",
-		      d->driver, d->enabled, d->suspended);
+		      "Device is wrong state driver=%p enabled=%d\n",
+		      d->driver, d->enabled);
 		return std_req_stall;
 	}
 
+	/*
+	 * Note: we used to reject/stall requests while suspended,
+	 * we don't do that anymore as we seem to have cases of
+	 * mass storage getting very upset.
+	 */
+
 	/* First packet, grab speed */
 	if (d->gadget.speed == USB_SPEED_UNKNOWN) {
 		d->gadget.speed = ep->vhub->speed;
@@ -449,8 +469,7 @@ static const struct usb_gadget_ops ast_vhub_udc_ops = {
 
 void ast_vhub_dev_suspend(struct ast_vhub_dev *d)
 {
-	d->suspended = true;
-	if (d->driver) {
+	if (d->driver && d->driver->suspend) {
 		spin_unlock(&d->vhub->lock);
 		d->driver->suspend(&d->gadget);
 		spin_lock(&d->vhub->lock);
@@ -459,8 +478,7 @@ void ast_vhub_dev_suspend(struct ast_vhub_dev *d)
 
 void ast_vhub_dev_resume(struct ast_vhub_dev *d)
 {
-	d->suspended = false;
-	if (d->driver) {
+	if (d->driver && d->driver->resume) {
 		spin_unlock(&d->vhub->lock);
 		d->driver->resume(&d->gadget);
 		spin_lock(&d->vhub->lock);
@@ -469,46 +487,28 @@ void ast_vhub_dev_resume(struct ast_vhub_dev *d)
 
 void ast_vhub_dev_reset(struct ast_vhub_dev *d)
 {
-	/*
-	 * If speed is not set, we enable the port. If it is,
-	 * send reset to the gadget and reset "speed".
-	 *
-	 * Speed is an indication that we have got the first
-	 * setup packet to the device.
-	 */
-	if (d->gadget.speed == USB_SPEED_UNKNOWN && !d->enabled) {
-		DDBG(d, "Reset at unknown speed of disabled device, enabling...\n");
-		ast_vhub_dev_enable(d);
-		d->suspended = false;
+	/* No driver, just disable the device and return */
+	if (!d->driver) {
+		ast_vhub_dev_disable(d);
+		return;
 	}
-	if (d->gadget.speed != USB_SPEED_UNKNOWN && d->driver) {
-		unsigned int i;
 
-		DDBG(d, "Reset at known speed of bound device, resetting...\n");
+	/* If the port isn't enabled, just enable it */
+	if (!d->enabled) {
+		DDBG(d, "Reset of disabled device, enabling...\n");
+		ast_vhub_dev_enable(d);
+	} else {
+		DDBG(d, "Reset of enabled device, resetting...\n");
 		spin_unlock(&d->vhub->lock);
-		d->driver->reset(&d->gadget);
+		usb_gadget_udc_reset(&d->gadget, d->driver);
 		spin_lock(&d->vhub->lock);
 
 		/*
-		 * Disable/re-enable HW, this will clear the address
+		 * Disable and maybe re-enable HW, this will clear the address
 		 * and speed setting.
 		 */
 		ast_vhub_dev_disable(d);
 		ast_vhub_dev_enable(d);
-
-		/* Clear stall on all EPs */
-		for (i = 0; i < AST_VHUB_NUM_GEN_EPs; i++) {
-			struct ast_vhub_ep *ep = d->epns[i];
-
-			if (ep && ep->epn.stalled) {
-				ep->epn.stalled = false;
-				ast_vhub_update_epn_stall(ep);
-			}
-		}
-
-		/* Additional cleanups */
-		d->wakeup_en = false;
-		d->suspended = false;
 	}
 }
 
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/ep0.c b/drivers/usb/gadget/udc/aspeed-vhub/ep0.c
index e2927fb083cf..022b777b85f8 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,
@@ -367,7 +379,7 @@ static int ast_vhub_ep0_queue(struct usb_ep* u_ep, struct usb_request *u_req,
 		return -EINVAL;
 
 	/* Disabled device */
-	if (ep->dev && (!ep->dev->enabled || ep->dev->suspended))
+	if (ep->dev && !ep->dev->enabled)
 		return -ESHUTDOWN;
 
 	/* Data, no buffer and not internal ? */
@@ -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;
 	}
@@ -459,6 +475,15 @@ static const struct usb_ep_ops ast_vhub_ep0_ops = {
 	.free_request	= ast_vhub_free_request,
 };
 
+void ast_vhub_reset_ep0(struct ast_vhub_dev *dev)
+{
+	struct ast_vhub_ep *ep = &dev->ep0;
+
+	ast_vhub_nuke(ep, -EIO);
+	ep->ep0.state = ep0_state_token;
+}
+
+
 void ast_vhub_init_ep0(struct ast_vhub *vhub, struct ast_vhub_ep *ep,
 		       struct ast_vhub_dev *dev)
 {
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/epn.c b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
index 35941dc125f9..7475c74aa5c5 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/epn.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
@@ -352,7 +352,7 @@ static int ast_vhub_epn_queue(struct usb_ep* u_ep, struct usb_request *u_req,
 
 	/* Endpoint enabled ? */
 	if (!ep->epn.enabled || !u_ep->desc || !ep->dev || !ep->d_idx ||
-	    !ep->dev->enabled || ep->dev->suspended) {
+	    !ep->dev->enabled) {
 		EPDBG(ep, "Enqueuing request on wrong or disabled EP\n");
 		return -ESHUTDOWN;
 	}
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/hub.c b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
index 7c040f56100e..19b3517e04c0 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
@@ -449,8 +449,15 @@ static void ast_vhub_change_port_stat(struct ast_vhub *vhub,
 		       USB_PORT_STAT_C_OVERCURRENT |
 		       USB_PORT_STAT_C_RESET |
 		       USB_PORT_STAT_C_L1;
-		p->change |= chg;
 
+		/*
+		 * We only set USB_PORT_STAT_C_ENABLE if we are disabling
+		 * the port as per USB spec, otherwise MacOS gets upset
+		 */
+		if (p->status & USB_PORT_STAT_ENABLE)
+			chg &= ~USB_PORT_STAT_C_ENABLE;
+
+		p->change = chg;
 		ast_vhub_update_hub_ep1(vhub, port);
 	}
 }
@@ -723,6 +730,12 @@ enum std_req_rc ast_vhub_class_hub_request(struct ast_vhub_ep *ep,
 	case ClearPortFeature:
 		EPDBG(ep, "ClearPortFeature(%d,%d)\n", wIndex & 0xf, wValue);
 		return ast_vhub_clr_port_feature(ep, wIndex & 0xf, wValue);
+	case ClearTTBuffer:
+	case ResetTT:
+	case StopTT:
+		return std_req_complete;
+	case GetTTState:
+		return ast_vhub_simple_reply(ep, 0, 0, 0, 0);
 	default:
 		EPDBG(ep, "Unknown class request\n");
 	}
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/vhub.h b/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
index 4ed03d33a5a9..761919e220d3 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,
 };
 
 /*
@@ -353,7 +354,6 @@ struct ast_vhub_dev {
 	struct usb_gadget_driver	*driver;
 	bool				registered : 1;
 	bool				wakeup_en : 1;
-	bool				suspended : 1;
 	bool				enabled : 1;
 
 	/* Endpoint structures */
@@ -507,6 +507,7 @@ void ast_vhub_init_hw(struct ast_vhub *vhub);
 /* ep0.c */
 void ast_vhub_ep0_handle_ack(struct ast_vhub_ep *ep, bool in_ack);
 void ast_vhub_ep0_handle_setup(struct ast_vhub_ep *ep);
+void ast_vhub_reset_ep0(struct ast_vhub_dev *dev);
 void ast_vhub_init_ep0(struct ast_vhub *vhub, struct ast_vhub_ep *ep,
 		       struct ast_vhub_dev *dev);
 int ast_vhub_reply(struct ast_vhub_ep *ep, char *ptr, int len);
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index a20e7815d814..774a03028da2 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -594,6 +594,10 @@ extern void usb_ep0_reinit(struct usb_device *);
 #define GetPortStatus		HUB_CLASS_REQ(USB_DIR_IN, USB_RT_PORT, USB_REQ_GET_STATUS)
 #define SetHubFeature		HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_HUB, USB_REQ_SET_FEATURE)
 #define SetPortFeature		HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_PORT, USB_REQ_SET_FEATURE)
+#define ClearTTBuffer		HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_PORT, HUB_CLEAR_TT_BUFFER)
+#define ResetTT			HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_PORT, HUB_RESET_TT)
+#define GetTTState		HUB_CLASS_REQ(USB_DIR_IN, USB_RT_PORT, HUB_GET_TT_STATE)
+#define StopTT			HUB_CLASS_REQ(USB_DIR_OUT, USB_RT_PORT, HUB_STOP_TT)
 
 
 /*-------------------------------------------------------------------------*/
-- 
2.7.4