[PATCH v3 0/4] Refactor Aspeed USB vhub driver

[PATCH v3 0/4] Refactor Aspeed USB vhub driver

[Neal Liu]

These patch series include 2 parts. One is adding more features
to pass USB30CV compliance test, the other is fixing hw issues.
More detail descriptions are included below patchsets.

Change since v2:
- Add more description in changelog.
- Fix remote wakeup issue patch and make it more configurable.

Change since v1:
- Remove unnecessary configs for SET_CONFIGURATION.
- Separate supporting test mode to new patch.

*** BLURB HERE ***

Neal Liu (4):
  usb: aspeed-vhub: add qualifier descriptor
  usb: aspeed-vhub: fix remote wakeup failure in iKVM use case
  usb: aspeed-vhub: fix ep0 OUT ack received wrong length issue
  usb: aspeed-vhub: support test mode feature

 drivers/usb/gadget/udc/aspeed-vhub/dev.c  | 19 +++++++--
 drivers/usb/gadget/udc/aspeed-vhub/ep0.c  |  7 ++++
 drivers/usb/gadget/udc/aspeed-vhub/epn.c  |  5 +++
 drivers/usb/gadget/udc/aspeed-vhub/hub.c  | 47 ++++++++++++++++++++---
 drivers/usb/gadget/udc/aspeed-vhub/vhub.h |  1 +
 5 files changed, 69 insertions(+), 10 deletions(-)

-- 
2.25.1

[PATCH v3 1/4] usb: aspeed-vhub: add qualifier descriptor

[Neal Liu]

USB3 Command Verifier (USB3CV) is the official tool for
USB3 Hub and Device Framework testing.

A high-speed capable device that has different device information
for full-speed and high-speed must have a Device Qualifier Descriptor.

This patch is to support device qualifier to pass
USB3CV - Chapter 9 Test [USB 2 devices] - Device Qualifier Tests.

Signed-off-by: Neal Liu <neal_liu@aspeedtech.com>
---
 drivers/usb/gadget/udc/aspeed-vhub/hub.c  | 24 +++++++++++++++++++++++
 drivers/usb/gadget/udc/aspeed-vhub/vhub.h |  1 +
 2 files changed, 25 insertions(+)

diff --git a/drivers/usb/gadget/udc/aspeed-vhub/hub.c b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
index b9960fdd8a51..93f27a745760 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
@@ -68,6 +68,18 @@ static const struct usb_device_descriptor ast_vhub_dev_desc = {
 	.bNumConfigurations	= 1,
 };
 
+static const struct usb_qualifier_descriptor ast_vhub_qual_desc = {
+	.bLength = 0xA,
+	.bDescriptorType = USB_DT_DEVICE_QUALIFIER,
+	.bcdUSB = cpu_to_le16(0x0200),
+	.bDeviceClass = USB_CLASS_HUB,
+	.bDeviceSubClass = 0,
+	.bDeviceProtocol = 0,
+	.bMaxPacketSize0 = 64,
+	.bNumConfigurations = 1,
+	.bRESERVED = 0,
+};
+
 /*
  * Configuration descriptor: same comments as above
  * regarding handling USB1 mode.
@@ -271,9 +283,11 @@ static int ast_vhub_rep_desc(struct ast_vhub_ep *ep,
 		BUILD_BUG_ON(dsize > sizeof(vhub->vhub_dev_desc));
 		BUILD_BUG_ON(USB_DT_DEVICE_SIZE >= AST_VHUB_EP0_MAX_PACKET);
 		break;
+	case USB_DT_OTHER_SPEED_CONFIG:
 	case USB_DT_CONFIG:
 		dsize = AST_VHUB_CONF_DESC_SIZE;
 		memcpy(ep->buf, &vhub->vhub_conf_desc, dsize);
+		((u8 *)ep->buf)[1] = desc_type;
 		BUILD_BUG_ON(dsize > sizeof(vhub->vhub_conf_desc));
 		BUILD_BUG_ON(AST_VHUB_CONF_DESC_SIZE >= AST_VHUB_EP0_MAX_PACKET);
 		break;
@@ -283,6 +297,10 @@ static int ast_vhub_rep_desc(struct ast_vhub_ep *ep,
 		BUILD_BUG_ON(dsize > sizeof(vhub->vhub_hub_desc));
 		BUILD_BUG_ON(AST_VHUB_HUB_DESC_SIZE >= AST_VHUB_EP0_MAX_PACKET);
 		break;
+	case USB_DT_DEVICE_QUALIFIER:
+		dsize = sizeof(vhub->vhub_qual_desc);
+		memcpy(ep->buf, &vhub->vhub_qual_desc, dsize);
+		break;
 	default:
 		return std_req_stall;
 	}
@@ -428,6 +446,8 @@ enum std_req_rc ast_vhub_std_hub_request(struct ast_vhub_ep *ep,
 		switch (wValue >> 8) {
 		case USB_DT_DEVICE:
 		case USB_DT_CONFIG:
+		case USB_DT_DEVICE_QUALIFIER:
+		case USB_DT_OTHER_SPEED_CONFIG:
 			return ast_vhub_rep_desc(ep, wValue >> 8,
 						 wLength);
 		case USB_DT_STRING:
@@ -1033,6 +1053,10 @@ static int ast_vhub_init_desc(struct ast_vhub *vhub)
 	else
 		ret = ast_vhub_str_alloc_add(vhub, &ast_vhub_strings);
 
+	/* Initialize vhub Qualifier Descriptor. */
+	memcpy(&vhub->vhub_qual_desc, &ast_vhub_qual_desc,
+		sizeof(vhub->vhub_qual_desc));
+
 	return ret;
 }
 
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/vhub.h b/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
index 87a5dea12d3c..6b9dfa6e10eb 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
+++ b/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
@@ -425,6 +425,7 @@ struct ast_vhub {
 	struct ast_vhub_full_cdesc	vhub_conf_desc;
 	struct usb_hub_descriptor	vhub_hub_desc;
 	struct list_head		vhub_str_desc;
+	struct usb_qualifier_descriptor	vhub_qual_desc;
 };
 
 /* Standard request handlers result codes */
-- 
2.25.1

[PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup failure in iKVM use case

[Neal Liu]

Signaling remote wakeup if an emulated USB device has any activity
if the device is allowed by host.

Signed-off-by: Neal Liu <neal_liu@aspeedtech.com>
---
 drivers/usb/gadget/udc/aspeed-vhub/epn.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/gadget/udc/aspeed-vhub/epn.c b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
index 917892ca8753..ccc239b5cc17 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/epn.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
@@ -381,6 +381,11 @@ static int ast_vhub_epn_queue(struct usb_ep* u_ep, struct usb_request *u_req,
 	} else
 		u_req->dma = 0;
 
+	if (ep->dev->wakeup_en) {
+		EPVDBG(ep, "Wakeup host first\n");
+		ast_vhub_hub_wake_all(vhub);
+	}
+
 	EPVDBG(ep, "enqueue req @%p\n", req);
 	EPVDBG(ep, " l=%d dma=0x%x zero=%d noshort=%d noirq=%d is_in=%d\n",
 	       u_req->length, (u32)u_req->dma, u_req->zero,
-- 
2.25.1

[PATCH v3 3/4] usb: aspeed-vhub: fix ep0 OUT ack received wrong length issue

[Neal Liu]

If multiple devices in vhub are enumerated simultaneously, ep0 OUT
ack might received wrong data length. Using expected data length
instead.

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Neal Liu <neal_liu@aspeedtech.com>
---
 drivers/usb/gadget/udc/aspeed-vhub/ep0.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/gadget/udc/aspeed-vhub/ep0.c b/drivers/usb/gadget/udc/aspeed-vhub/ep0.c
index 74ea36c19b1e..b4cf46249fea 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/ep0.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/ep0.c
@@ -251,6 +251,13 @@ static void ast_vhub_ep0_do_receive(struct ast_vhub_ep *ep, struct ast_vhub_req
 		len = remain;
 		rc = -EOVERFLOW;
 	}
+
+	/* Hardware return wrong data len */
+	if (len < ep->ep.maxpacket && len != remain) {
+		EPDBG(ep, "using expected data len instead\n");
+		len = remain;
+	}
+
 	if (len && req->req.buf)
 		memcpy(req->req.buf + req->req.actual, ep->buf, len);
 	req->req.actual += len;
-- 
2.25.1

[PATCH v3 4/4] usb: aspeed-vhub: support test mode feature

[Neal Liu]

Support aspeed usb vhub set feature to test mode.

Signed-off-by: Neal Liu <neal_liu@aspeedtech.com>
---
 drivers/usb/gadget/udc/aspeed-vhub/dev.c | 19 +++++++++++++++----
 drivers/usb/gadget/udc/aspeed-vhub/hub.c | 23 +++++++++++++++++------
 2 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/udc/aspeed-vhub/dev.c b/drivers/usb/gadget/udc/aspeed-vhub/dev.c
index d918e8b2af3c..b0dfca43fbdc 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/dev.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/dev.c
@@ -110,15 +110,26 @@ static int ast_vhub_dev_feature(struct ast_vhub_dev *d,
 				u16 wIndex, u16 wValue,
 				bool is_set)
 {
+	u32 val;
+
 	DDBG(d, "%s_FEATURE(dev val=%02x)\n",
 	     is_set ? "SET" : "CLEAR", wValue);
 
-	if (wValue != USB_DEVICE_REMOTE_WAKEUP)
-		return std_req_driver;
+	if (wValue == USB_DEVICE_REMOTE_WAKEUP) {
+		d->wakeup_en = is_set;
+		return std_req_complete;
+	}
 
-	d->wakeup_en = is_set;
+	if (wValue == USB_DEVICE_TEST_MODE) {
+		val = readl(d->vhub->regs + AST_VHUB_CTRL);
+		val &= ~GENMASK(10, 8);
+		val |= VHUB_CTRL_SET_TEST_MODE((wIndex >> 8) & 0x7);
+		writel(val, d->vhub->regs + AST_VHUB_CTRL);
 
-	return std_req_complete;
+		return std_req_complete;
+	}
+
+	return std_req_driver;
 }
 
 static int ast_vhub_ep_feature(struct ast_vhub_dev *d,
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/hub.c b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
index 93f27a745760..65cd4e46f031 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
@@ -212,17 +212,28 @@ static int ast_vhub_hub_dev_feature(struct ast_vhub_ep *ep,
 				    u16 wIndex, u16 wValue,
 				    bool is_set)
 {
+	u32 val;
+
 	EPDBG(ep, "%s_FEATURE(dev val=%02x)\n",
 	      is_set ? "SET" : "CLEAR", wValue);
 
-	if (wValue != USB_DEVICE_REMOTE_WAKEUP)
-		return std_req_stall;
+	if (wValue == USB_DEVICE_REMOTE_WAKEUP) {
+		ep->vhub->wakeup_en = is_set;
+		EPDBG(ep, "Hub remote wakeup %s\n",
+		      is_set ? "enabled" : "disabled");
+		return std_req_complete;
+	}
 
-	ep->vhub->wakeup_en = is_set;
-	EPDBG(ep, "Hub remote wakeup %s\n",
-	      is_set ? "enabled" : "disabled");
+	if (wValue == USB_DEVICE_TEST_MODE) {
+		val = readl(ep->vhub->regs + AST_VHUB_CTRL);
+		val &= ~GENMASK(10, 8);
+		val |= VHUB_CTRL_SET_TEST_MODE((wIndex >> 8) & 0x7);
+		writel(val, ep->vhub->regs + AST_VHUB_CTRL);
 
-	return std_req_complete;
+		return std_req_complete;
+	}
+
+	return std_req_stall;
 }
 
 static int ast_vhub_hub_ep_feature(struct ast_vhub_ep *ep,
-- 
2.25.1

Re: [PATCH v3 0/4] Refactor Aspeed USB vhub driver

[Greg Kroah-Hartman]

On Wed, Dec 08, 2021 at 06:05:41PM +0800, Neal Liu wrote:
> These patch series include 2 parts. One is adding more features
> to pass USB30CV compliance test, the other is fixing hw issues.
> More detail descriptions are included below patchsets.
> 
> Change since v2:
> - Add more description in changelog.
> - Fix remote wakeup issue patch and make it more configurable.
> 
> Change since v1:
> - Remove unnecessary configs for SET_CONFIGURATION.
> - Separate supporting test mode to new patch.
> 
> *** BLURB HERE ***

Blurb is missing :(

Re: [PATCH v3 1/4] usb: aspeed-vhub: add qualifier descriptor

[Benjamin Herrenschmidt]

On Wed, 2021-12-08 at 18:05 +0800, Neal Liu wrote:
> USB3 Command Verifier (USB3CV) is the official tool for
> USB3 Hub and Device Framework testing.
> 
> A high-speed capable device that has different device information
> for full-speed and high-speed must have a Device Qualifier
> Descriptor.
> 
> This patch is to support device qualifier to pass
> USB3CV - Chapter 9 Test [USB 2 devices] - Device Qualifier Tests.
> 
> Signed-off-by: Neal Liu <neal_liu@aspeedtech.com>
> ---

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

>  drivers/usb/gadget/udc/aspeed-vhub/hub.c  | 24
> +++++++++++++++++++++++
>  drivers/usb/gadget/udc/aspeed-vhub/vhub.h |  1 +
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> index b9960fdd8a51..93f27a745760 100644
> --- a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> @@ -68,6 +68,18 @@ static const struct usb_device_descriptor
> ast_vhub_dev_desc = {
>  	.bNumConfigurations	= 1,
>  };
>  
> +static const struct usb_qualifier_descriptor ast_vhub_qual_desc = {
> +	.bLength = 0xA,
> +	.bDescriptorType = USB_DT_DEVICE_QUALIFIER,
> +	.bcdUSB = cpu_to_le16(0x0200),
> +	.bDeviceClass = USB_CLASS_HUB,
> +	.bDeviceSubClass = 0,
> +	.bDeviceProtocol = 0,
> +	.bMaxPacketSize0 = 64,
> +	.bNumConfigurations = 1,
> +	.bRESERVED = 0,
> +};
> +
>  /*
>   * Configuration descriptor: same comments as above
>   * regarding handling USB1 mode.
> @@ -271,9 +283,11 @@ static int ast_vhub_rep_desc(struct ast_vhub_ep
> *ep,
>  		BUILD_BUG_ON(dsize > sizeof(vhub->vhub_dev_desc));
>  		BUILD_BUG_ON(USB_DT_DEVICE_SIZE >=
> AST_VHUB_EP0_MAX_PACKET);
>  		break;
> +	case USB_DT_OTHER_SPEED_CONFIG:
>  	case USB_DT_CONFIG:
>  		dsize = AST_VHUB_CONF_DESC_SIZE;
>  		memcpy(ep->buf, &vhub->vhub_conf_desc, dsize);
> +		((u8 *)ep->buf)[1] = desc_type;
>  		BUILD_BUG_ON(dsize > sizeof(vhub->vhub_conf_desc));
>  		BUILD_BUG_ON(AST_VHUB_CONF_DESC_SIZE >=
> AST_VHUB_EP0_MAX_PACKET);
>  		break;
> @@ -283,6 +297,10 @@ static int ast_vhub_rep_desc(struct ast_vhub_ep
> *ep,
>  		BUILD_BUG_ON(dsize > sizeof(vhub->vhub_hub_desc));
>  		BUILD_BUG_ON(AST_VHUB_HUB_DESC_SIZE >=
> AST_VHUB_EP0_MAX_PACKET);
>  		break;
> +	case USB_DT_DEVICE_QUALIFIER:
> +		dsize = sizeof(vhub->vhub_qual_desc);
> +		memcpy(ep->buf, &vhub->vhub_qual_desc, dsize);
> +		break;
>  	default:
>  		return std_req_stall;
>  	}
> @@ -428,6 +446,8 @@ enum std_req_rc ast_vhub_std_hub_request(struct
> ast_vhub_ep *ep,
>  		switch (wValue >> 8) {
>  		case USB_DT_DEVICE:
>  		case USB_DT_CONFIG:
> +		case USB_DT_DEVICE_QUALIFIER:
> +		case USB_DT_OTHER_SPEED_CONFIG:
>  			return ast_vhub_rep_desc(ep, wValue >> 8,
>  						 wLength);
>  		case USB_DT_STRING:
> @@ -1033,6 +1053,10 @@ static int ast_vhub_init_desc(struct ast_vhub
> *vhub)
>  	else
>  		ret = ast_vhub_str_alloc_add(vhub, &ast_vhub_strings);
>  
> +	/* Initialize vhub Qualifier Descriptor. */
> +	memcpy(&vhub->vhub_qual_desc, &ast_vhub_qual_desc,
> +		sizeof(vhub->vhub_qual_desc));
> +
>  	return ret;
>  }
>  
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
> b/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
> index 87a5dea12d3c..6b9dfa6e10eb 100644
> --- a/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
> @@ -425,6 +425,7 @@ struct ast_vhub {
>  	struct ast_vhub_full_cdesc	vhub_conf_desc;
>  	struct usb_hub_descriptor	vhub_hub_desc;
>  	struct list_head		vhub_str_desc;
> +	struct usb_qualifier_descriptor	vhub_qual_desc;
>  };
>  
>  /* Standard request handlers result codes */

Re: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup failure in iKVM use case

[Benjamin Herrenschmidt]

On Wed, 2021-12-08 at 18:05 +0800, Neal Liu wrote:
> Signaling remote wakeup if an emulated USB device has any activity
> if the device is allowed by host.
> 
> Signed-off-by: Neal Liu <neal_liu@aspeedtech.com>

I still think it should fundamentally be the device making that
decision, but since they don't, this is an acceptable workaround, but
please, don't write the MMIO on every EP queue. Either keep track of
the bus being suspended, or turn on the AUTO bit in HW when wakeup_en
is set.

Cheers,
Ben.

> ---
>  drivers/usb/gadget/udc/aspeed-vhub/epn.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/epn.c
> b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
> index 917892ca8753..ccc239b5cc17 100644
> --- a/drivers/usb/gadget/udc/aspeed-vhub/epn.c
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
> @@ -381,6 +381,11 @@ static int ast_vhub_epn_queue(struct usb_ep*
> u_ep, struct usb_request *u_req,
>  	} else
>  		u_req->dma = 0;
>  
> +	if (ep->dev->wakeup_en) {
> +		EPVDBG(ep, "Wakeup host first\n");
> +		ast_vhub_hub_wake_all(vhub);
> +	}
> +
>  	EPVDBG(ep, "enqueue req @%p\n", req);
>  	EPVDBG(ep, " l=%d dma=0x%x zero=%d noshort=%d noirq=%d
> is_in=%d\n",
>  	       u_req->length, (u32)u_req->dma, u_req->zero,

Re: [PATCH v3 4/4] usb: aspeed-vhub: support test mode feature

[Benjamin Herrenschmidt]

On Wed, 2021-12-08 at 18:05 +0800, Neal Liu wrote:
> Support aspeed usb vhub set feature to test mode.
> 
> Signed-off-by: Neal Liu <neal_liu@aspeedtech.com>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> ---
>  drivers/usb/gadget/udc/aspeed-vhub/dev.c | 19 +++++++++++++++----
>  drivers/usb/gadget/udc/aspeed-vhub/hub.c | 23 +++++++++++++++++-----
> -
>  2 files changed, 32 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/dev.c
> b/drivers/usb/gadget/udc/aspeed-vhub/dev.c
> index d918e8b2af3c..b0dfca43fbdc 100644
> --- a/drivers/usb/gadget/udc/aspeed-vhub/dev.c
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/dev.c
> @@ -110,15 +110,26 @@ static int ast_vhub_dev_feature(struct
> ast_vhub_dev *d,
>  				u16 wIndex, u16 wValue,
>  				bool is_set)
>  {
> +	u32 val;
> +
>  	DDBG(d, "%s_FEATURE(dev val=%02x)\n",
>  	     is_set ? "SET" : "CLEAR", wValue);
>  
> -	if (wValue != USB_DEVICE_REMOTE_WAKEUP)
> -		return std_req_driver;
> +	if (wValue == USB_DEVICE_REMOTE_WAKEUP) {
> +		d->wakeup_en = is_set;
> +		return std_req_complete;
> +	}
>  
> -	d->wakeup_en = is_set;
> +	if (wValue == USB_DEVICE_TEST_MODE) {
> +		val = readl(d->vhub->regs + AST_VHUB_CTRL);
> +		val &= ~GENMASK(10, 8);
> +		val |= VHUB_CTRL_SET_TEST_MODE((wIndex >> 8) & 0x7);
> +		writel(val, d->vhub->regs + AST_VHUB_CTRL);
>  
> -	return std_req_complete;
> +		return std_req_complete;
> +	}
> +
> +	return std_req_driver;
>  }
>  
>  static int ast_vhub_ep_feature(struct ast_vhub_dev *d,
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> index 93f27a745760..65cd4e46f031 100644
> --- a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> @@ -212,17 +212,28 @@ static int ast_vhub_hub_dev_feature(struct
> ast_vhub_ep *ep,
>  				    u16 wIndex, u16 wValue,
>  				    bool is_set)
>  {
> +	u32 val;
> +
>  	EPDBG(ep, "%s_FEATURE(dev val=%02x)\n",
>  	      is_set ? "SET" : "CLEAR", wValue);
>  
> -	if (wValue != USB_DEVICE_REMOTE_WAKEUP)
> -		return std_req_stall;
> +	if (wValue == USB_DEVICE_REMOTE_WAKEUP) {
> +		ep->vhub->wakeup_en = is_set;
> +		EPDBG(ep, "Hub remote wakeup %s\n",
> +		      is_set ? "enabled" : "disabled");
> +		return std_req_complete;
> +	}
>  
> -	ep->vhub->wakeup_en = is_set;
> -	EPDBG(ep, "Hub remote wakeup %s\n",
> -	      is_set ? "enabled" : "disabled");
> +	if (wValue == USB_DEVICE_TEST_MODE) {
> +		val = readl(ep->vhub->regs + AST_VHUB_CTRL);
> +		val &= ~GENMASK(10, 8);
> +		val |= VHUB_CTRL_SET_TEST_MODE((wIndex >> 8) & 0x7);
> +		writel(val, ep->vhub->regs + AST_VHUB_CTRL);
>  
> -	return std_req_complete;
> +		return std_req_complete;
> +	}
> +
> +	return std_req_stall;
>  }
>  
>  static int ast_vhub_hub_ep_feature(struct ast_vhub_ep *ep,

RE: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup failure in iKVM use case

[Neal Liu]

> -----Original Message-----
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Sent: Thursday, December 9, 2021 8:05 AM
> To: Neal Liu <neal_liu@aspeedtech.com>; Felipe Balbi <balbi@kernel.org>;
> Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Joel Stanley
> <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Cai Huoqing
> <caihuoqing@baidu.com>; Tao Ren <rentao.bupt@gmail.com>; Julia Lawall
> <julia.lawall@inria.fr>; kernel test robot <lkp@intel.com>; Sasha Levin
> <sashal@kernel.org>; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-aspeed@lists.ozlabs.org
> Cc: BMC-SW <BMC-SW@aspeedtech.com>
> Subject: Re: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup failure in
> iKVM use case
> 
> On Wed, 2021-12-08 at 18:05 +0800, Neal Liu wrote:
> > Signaling remote wakeup if an emulated USB device has any activity if
> > the device is allowed by host.
> >
> > Signed-off-by: Neal Liu <neal_liu@aspeedtech.com>
> 
> I still think it should fundamentally be the device making that decision, but
> since they don't, this is an acceptable workaround, but please, don't write the
> MMIO on every EP queue. Either keep track of the bus being suspended, or
> turn on the AUTO bit in HW when wakeup_en is set.
> 
> Cheers,
> Ben.
> 

I'm confused. Signaling Wakeup when wakeup_en is set if it has any ep activities is not exactly what you said?
wakeup_en is set only if host allows this device have wakeup capability and bus being suspended.
Normal ep activities would not write the MMIO because wakeup_en is not set.
Thanks

-Neal

> > ---
> >  drivers/usb/gadget/udc/aspeed-vhub/epn.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/usb/gadget/udc/aspeed-vhub/epn.c
> > b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
> > index 917892ca8753..ccc239b5cc17 100644
> > --- a/drivers/usb/gadget/udc/aspeed-vhub/epn.c
> > +++ b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
> > @@ -381,6 +381,11 @@ static int ast_vhub_epn_queue(struct usb_ep*
> > u_ep, struct usb_request *u_req,
> >  	} else
> >  		u_req->dma = 0;
> >
> > +	if (ep->dev->wakeup_en) {
> > +		EPVDBG(ep, "Wakeup host first\n");
> > +		ast_vhub_hub_wake_all(vhub);
> > +	}
> > +
> >  	EPVDBG(ep, "enqueue req @%p\n", req);
> >  	EPVDBG(ep, " l=%d dma=0x%x zero=%d noshort=%d noirq=%d
> is_in=%d\n",
> >  	       u_req->length, (u32)u_req->dma, u_req->zero,

RE: [PATCH v3 0/4] Refactor Aspeed USB vhub driver

[Neal Liu]

> -----Original Message-----
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Sent: Wednesday, December 8, 2021 8:22 PM
> To: Neal Liu <neal_liu@aspeedtech.com>
> Cc: Felipe Balbi <balbi@kernel.org>; Joel Stanley <joel@jms.id.au>; Andrew
> Jeffery <andrew@aj.id.au>; Cai Huoqing <caihuoqing@baidu.com>; Tao Ren
> <rentao.bupt@gmail.com>; Julia Lawall <julia.lawall@inria.fr>; kernel test
> robot <lkp@intel.com>; Sasha Levin <sashal@kernel.org>;
> linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-aspeed@lists.ozlabs.org;
> benh@kernel.crashing.org; BMC-SW <BMC-SW@aspeedtech.com>
> Subject: Re: [PATCH v3 0/4] Refactor Aspeed USB vhub driver
> 
> On Wed, Dec 08, 2021 at 06:05:41PM +0800, Neal Liu wrote:
> > These patch series include 2 parts. One is adding more features to
> > pass USB30CV compliance test, the other is fixing hw issues.
> > More detail descriptions are included below patchsets.
> >
> > Change since v2:
> > - Add more description in changelog.
> > - Fix remote wakeup issue patch and make it more configurable.
> >
> > Change since v1:
> > - Remove unnecessary configs for SET_CONFIGURATION.
> > - Separate supporting test mode to new patch.
> >
> > *** BLURB HERE ***
> 
> Blurb is missing :(

I would remove this comment if new patch is necessary.
Thanks

-Neal

Re: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup failure in iKVM use case

[Greg Kroah-Hartman]

On Wed, Dec 08, 2021 at 06:05:43PM +0800, Neal Liu wrote:
> Signaling remote wakeup if an emulated USB device has any activity
> if the device is allowed by host.
> 
> Signed-off-by: Neal Liu <neal_liu@aspeedtech.com>
> ---
>  drivers/usb/gadget/udc/aspeed-vhub/epn.c | 5 +++++
>  1 file changed, 5 insertions(+)

What commit does this fix?

Does it need to go to stable kernels?

Should it be independent of this patch series that adds new features?

thanks,

greg k-h

Re: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup failure in iKVM use case

[Benjamin Herrenschmidt]

On Thu, 2021-12-09 at 02:37 +0000, Neal Liu wrote:
> I'm confused. Signaling Wakeup when wakeup_en is set if it has any ep
> activities is not exactly what you said?
> 
> wakeup_en is set only if host allows this device have wakeup
> capability and bus being suspended.
> 
> Normal ep activities would not write the MMIO because wakeup_en is
> not set.

Hrm... I didn't think wakeup_en was limited to the bus being suspended,
but maybe I misremember, it's been a while.

Cheers,
Ben.

RE: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup failure in iKVM use case

[Neal Liu]

> -----Original Message-----
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Sent: Tuesday, December 14, 2021 10:36 AM
> To: Neal Liu <neal_liu@aspeedtech.com>; Felipe Balbi <balbi@kernel.org>;
> Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Joel Stanley
> <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Cai Huoqing
> <caihuoqing@baidu.com>; Tao Ren <rentao.bupt@gmail.com>; Julia Lawall
> <julia.lawall@inria.fr>; kernel test robot <lkp@intel.com>; Sasha Levin
> <sashal@kernel.org>; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-aspeed@lists.ozlabs.org
> Cc: BMC-SW <BMC-SW@aspeedtech.com>
> Subject: Re: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup failure in
> iKVM use case
> 
> On Thu, 2021-12-09 at 02:37 +0000, Neal Liu wrote:
> > I'm confused. Signaling Wakeup when wakeup_en is set if it has any ep
> > activities is not exactly what you said?
> >
> > wakeup_en is set only if host allows this device have wakeup
> > capability and bus being suspended.
> >
> > Normal ep activities would not write the MMIO because wakeup_en is not
> > set.
> 
> Hrm... I didn't think wakeup_en was limited to the bus being suspended, but
> maybe I misremember, it's been a while.
> 
> Cheers,
> Ben.
> 

wakeup_en is only set in the case of host set USB_DEVICE_REMOTE_WAKEUP feature to vhub devices.
I think this behavior only occurs during host is going to suspend, and set this feature to any device which can wakeup itself before sleep.
Thanks

-Neal

RE: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup failure in iKVM use case

[Neal Liu]

> -----Original Message-----
> From: Neal Liu
> Sent: Monday, December 20, 2021 10:23 AM
> To: Benjamin Herrenschmidt <benh@kernel.crashing.org>; Felipe Balbi
> <balbi@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Joel
> Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Cai Huoqing
> <caihuoqing@baidu.com>; Tao Ren <rentao.bupt@gmail.com>; Julia Lawall
> <julia.lawall@inria.fr>; kernel test robot <lkp@intel.com>; Sasha Levin
> <sashal@kernel.org>; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-aspeed@lists.ozlabs.org
> Cc: BMC-SW <BMC-SW@aspeedtech.com>
> Subject: RE: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup failure in
> iKVM use case
> 
> > -----Original Message-----
> > From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Sent: Tuesday, December 14, 2021 10:36 AM
> > To: Neal Liu <neal_liu@aspeedtech.com>; Felipe Balbi
> > <balbi@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> > Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Cai
> > Huoqing <caihuoqing@baidu.com>; Tao Ren <rentao.bupt@gmail.com>; Julia
> > Lawall <julia.lawall@inria.fr>; kernel test robot <lkp@intel.com>;
> > Sasha Levin <sashal@kernel.org>; linux-usb@vger.kernel.org;
> > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > linux-aspeed@lists.ozlabs.org
> > Cc: BMC-SW <BMC-SW@aspeedtech.com>
> > Subject: Re: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup
> > failure in iKVM use case
> >
> > On Thu, 2021-12-09 at 02:37 +0000, Neal Liu wrote:
> > > I'm confused. Signaling Wakeup when wakeup_en is set if it has any
> > > ep activities is not exactly what you said?
> > >
> > > wakeup_en is set only if host allows this device have wakeup
> > > capability and bus being suspended.
> > >
> > > Normal ep activities would not write the MMIO because wakeup_en is
> > > not set.
> >
> > Hrm... I didn't think wakeup_en was limited to the bus being
> > suspended, but maybe I misremember, it's been a while.
> >
> > Cheers,
> > Ben.
> >
> 
> wakeup_en is only set in the case of host set USB_DEVICE_REMOTE_WAKEUP
> feature to vhub devices.
> I think this behavior only occurs during host is going to suspend, and set this
> feature to any device which can wakeup itself before sleep.
> Thanks
> 
> -Neal
> 
Would you like to test it if you have some free time.
Please feel free for any feedback.

-Neal

RE: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup failure in iKVM use case

[Neal Liu]

> -----Original Message-----
> From: Neal Liu
> Sent: Monday, December 27, 2021 10:07 AM
> To: Benjamin Herrenschmidt <benh@kernel.crashing.org>; Felipe Balbi
> <balbi@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Joel
> Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Cai Huoqing
> <caihuoqing@baidu.com>; Tao Ren <rentao.bupt@gmail.com>; Julia Lawall
> <julia.lawall@inria.fr>; kernel test robot <lkp@intel.com>; Sasha Levin
> <sashal@kernel.org>; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-aspeed@lists.ozlabs.org
> Cc: BMC-SW <BMC-SW@aspeedtech.com>
> Subject: RE: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup failure in
> iKVM use case
> 
> > -----Original Message-----
> > From: Neal Liu
> > Sent: Monday, December 20, 2021 10:23 AM
> > To: Benjamin Herrenschmidt <benh@kernel.crashing.org>; Felipe Balbi
> > <balbi@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> > Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Cai
> > Huoqing <caihuoqing@baidu.com>; Tao Ren <rentao.bupt@gmail.com>; Julia
> > Lawall <julia.lawall@inria.fr>; kernel test robot <lkp@intel.com>;
> > Sasha Levin <sashal@kernel.org>; linux-usb@vger.kernel.org;
> > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > linux-aspeed@lists.ozlabs.org
> > Cc: BMC-SW <BMC-SW@aspeedtech.com>
> > Subject: RE: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup
> > failure in iKVM use case
> >
> > > -----Original Message-----
> > > From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > Sent: Tuesday, December 14, 2021 10:36 AM
> > > To: Neal Liu <neal_liu@aspeedtech.com>; Felipe Balbi
> > > <balbi@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> > > Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Cai
> > > Huoqing <caihuoqing@baidu.com>; Tao Ren <rentao.bupt@gmail.com>;
> > > Julia Lawall <julia.lawall@inria.fr>; kernel test robot
> > > <lkp@intel.com>; Sasha Levin <sashal@kernel.org>;
> > > linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > linux-arm-kernel@lists.infradead.org;
> > > linux-aspeed@lists.ozlabs.org
> > > Cc: BMC-SW <BMC-SW@aspeedtech.com>
> > > Subject: Re: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup
> > > failure in iKVM use case
> > >
> > > On Thu, 2021-12-09 at 02:37 +0000, Neal Liu wrote:
> > > > I'm confused. Signaling Wakeup when wakeup_en is set if it has any
> > > > ep activities is not exactly what you said?
> > > >
> > > > wakeup_en is set only if host allows this device have wakeup
> > > > capability and bus being suspended.
> > > >
> > > > Normal ep activities would not write the MMIO because wakeup_en is
> > > > not set.
> > >
> > > Hrm... I didn't think wakeup_en was limited to the bus being
> > > suspended, but maybe I misremember, it's been a while.
> > >
> > > Cheers,
> > > Ben.
> > >
> >
> > wakeup_en is only set in the case of host set
> USB_DEVICE_REMOTE_WAKEUP
> > feature to vhub devices.
> > I think this behavior only occurs during host is going to suspend, and
> > set this feature to any device which can wakeup itself before sleep.
> > Thanks
> >
> > -Neal
> >
> Would you like to test it if you have some free time.
> Please feel free for any feedback.
> 
> -Neal

Gentle ping for this patch.
Thanks