2021-11-30 10:44:31

by Neal Liu

[permalink] [raw]
Subject: [PATCH v2 0/4] Refactor Aspeed USB vhub driver

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 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: support auto remote wakeup feature
usb: aspeed-vhub: fix ep0 OUT ack received wrong length issue
usb: aspeed-vhub: support test mode feature

drivers/usb/gadget/udc/aspeed-vhub/core.c | 3 ++
drivers/usb/gadget/udc/aspeed-vhub/dev.c | 18 +++++++--
drivers/usb/gadget/udc/aspeed-vhub/ep0.c | 7 ++++
drivers/usb/gadget/udc/aspeed-vhub/hub.c | 46 ++++++++++++++++++++---
drivers/usb/gadget/udc/aspeed-vhub/vhub.h | 1 +
5 files changed, 65 insertions(+), 10 deletions(-)

--
2.25.1



2021-11-30 10:44:36

by Neal Liu

[permalink] [raw]
Subject: [PATCH v2 4/4] usb: aspeed-vhub: support test mode feature

Support aspeed usb vhub set feature to test mode.

Signed-off-by: Neal Liu <[email protected]>
---
drivers/usb/gadget/udc/aspeed-vhub/dev.c | 18 ++++++++++++++----
drivers/usb/gadget/udc/aspeed-vhub/hub.c | 22 ++++++++++++++++------
2 files changed, 30 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..4462f4b73b04 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/dev.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/dev.c
@@ -110,15 +110,25 @@ 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;
+ } else 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..e52805fbdebd 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
@@ -212,17 +212,27 @@ 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");
+ } else 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


2021-11-30 10:44:40

by Neal Liu

[permalink] [raw]
Subject: [PATCH v2 1/4] usb: aspeed-vhub: add qualifier descriptor

Support qualifier descriptor to pass USB30CV compliance test.

Signed-off-by: Neal Liu <[email protected]>
---
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


2021-11-30 10:44:45

by Neal Liu

[permalink] [raw]
Subject: [PATCH v2 2/4] usb: aspeed-vhub: support auto remote wakeup feature

Remote wakeup signaling will be automatically issued
whenever any write commands has been received in suspend
state.

Signed-off-by: Neal Liu <[email protected]>
---
drivers/usb/gadget/udc/aspeed-vhub/core.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/usb/gadget/udc/aspeed-vhub/core.c b/drivers/usb/gadget/udc/aspeed-vhub/core.c
index 7a635c499777..122ee7ef0b03 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/core.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/core.c
@@ -240,6 +240,9 @@ void ast_vhub_init_hw(struct ast_vhub *vhub)
if (vhub->force_usb1)
ctrl |= VHUB_CTRL_FULL_SPEED_ONLY;

+ /* Enable auto remote wakeup */
+ ctrl |= VHUB_CTRL_AUTO_REMOTE_WAKEUP;
+
ctrl |= VHUB_CTRL_UPSTREAM_CONNECT;
writel(ctrl, vhub->regs + AST_VHUB_CTRL);

--
2.25.1


2021-11-30 11:45:20

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] usb: aspeed-vhub: add qualifier descriptor

On Tue, Nov 30, 2021 at 06:42:53PM +0800, Neal Liu wrote:
> Support qualifier descriptor to pass USB30CV compliance test.

Please provide more information here in this description. This does not
explain what is happening here very well.

Also, what is "USB30CV"?

>
> Signed-off-by: Neal Liu <[email protected]>
> ---
> 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,

Fields that are to be set to zero do not need to be set here, the
compiler does it for you.

thanks,

greg k-h

2021-11-30 11:47:44

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] usb: aspeed-vhub: support test mode feature

On Tue, Nov 30, 2021 at 06:42:56PM +0800, Neal Liu wrote:
> Support aspeed usb vhub set feature to test mode.
>
> Signed-off-by: Neal Liu <[email protected]>
> ---
> drivers/usb/gadget/udc/aspeed-vhub/dev.c | 18 ++++++++++++++----
> drivers/usb/gadget/udc/aspeed-vhub/hub.c | 22 ++++++++++++++++------
> 2 files changed, 30 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..4462f4b73b04 100644
> --- a/drivers/usb/gadget/udc/aspeed-vhub/dev.c
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/dev.c
> @@ -110,15 +110,25 @@ 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;
> + } else if (wValue == USB_DEVICE_TEST_MODE) {

If you return, no need for an else statement, right?


> + 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..e52805fbdebd 100644
> --- a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> @@ -212,17 +212,27 @@ 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");
> + } else if (wValue == USB_DEVICE_TEST_MODE) {

Same here, no need for else.

thanks,

greg k-h

2021-12-01 03:37:06

by Neal Liu

[permalink] [raw]
Subject: RE: [PATCH v2 4/4] usb: aspeed-vhub: support test mode feature

> -----Original Message-----
> From: Greg Kroah-Hartman <[email protected]>
> Sent: Tuesday, November 30, 2021 7:47 PM
> To: Neal Liu <[email protected]>
> Cc: Felipe Balbi <[email protected]>; Joel Stanley <[email protected]>; Andrew
> Jeffery <[email protected]>; Cai Huoqing <[email protected]>; Tao Ren
> <[email protected]>; Julia Lawall <[email protected]>; kernel test
> robot <[email protected]>; Sasha Levin <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected]; BMC-SW
> <[email protected]>
> Subject: Re: [PATCH v2 4/4] usb: aspeed-vhub: support test mode feature
>
> On Tue, Nov 30, 2021 at 06:42:56PM +0800, Neal Liu wrote:
> > Support aspeed usb vhub set feature to test mode.
> >
> > Signed-off-by: Neal Liu <[email protected]>
> > ---
> > drivers/usb/gadget/udc/aspeed-vhub/dev.c | 18 ++++++++++++++----
> > drivers/usb/gadget/udc/aspeed-vhub/hub.c | 22 ++++++++++++++++------
> > 2 files changed, 30 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..4462f4b73b04 100644
> > --- a/drivers/usb/gadget/udc/aspeed-vhub/dev.c
> > +++ b/drivers/usb/gadget/udc/aspeed-vhub/dev.c
> > @@ -110,15 +110,25 @@ 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;
> > + } else if (wValue == USB_DEVICE_TEST_MODE) {
>
> If you return, no need for an else statement, right?
>
>
> > + 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..e52805fbdebd 100644
> > --- a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> > +++ b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> > @@ -212,17 +212,27 @@ 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");
> > + } else if (wValue == USB_DEVICE_TEST_MODE) {
>
> Same here, no need for else.
>
> thanks,
>
> greg k-h

Okay, I'll fix it in next patch. Thanks

-Neal

2021-12-01 05:43:37

by Neal Liu

[permalink] [raw]
Subject: RE: [PATCH v2 1/4] usb: aspeed-vhub: add qualifier descriptor

> -----Original Message-----
> From: Greg Kroah-Hartman <[email protected]>
> Sent: Tuesday, November 30, 2021 7:45 PM
> To: Neal Liu <[email protected]>
> Cc: Felipe Balbi <[email protected]>; Joel Stanley <[email protected]>; Andrew
> Jeffery <[email protected]>; Cai Huoqing <[email protected]>; Tao Ren
> <[email protected]>; Julia Lawall <[email protected]>; kernel test
> robot <[email protected]>; Sasha Levin <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected]; BMC-SW
> <[email protected]>
> Subject: Re: [PATCH v2 1/4] usb: aspeed-vhub: add qualifier descriptor
>
> On Tue, Nov 30, 2021 at 06:42:53PM +0800, Neal Liu wrote:
> > Support qualifier descriptor to pass USB30CV compliance test.
>
> Please provide more information here in this description. This does not
> explain what is happening here very well.
>
> Also, what is "USB30CV"?
>

USB 3 Command Verifier (USB3CV) is the official tool for USB 3 Hub and Device Framework testing.
The USB3CV tool includes the xHCI Compliance Drivers for use with the USB3CV.

We would like to pass USB3CV - Chapter 9 Test [USB 2 devices] - Device Qualifier Tests.
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, and the host will retrieve it through Get Descriptor request.

> >
> > Signed-off-by: Neal Liu <[email protected]>
> > ---
> > 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,
>
> Fields that are to be set to zero do not need to be set here, the compiler does it
> for you.
>
> thanks,
>
> greg k-h

This is more clear that we specify each member's value.
Thanks

-Neal

2021-12-01 06:18:22

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] usb: aspeed-vhub: add qualifier descriptor

On Wed, Dec 01, 2021 at 05:43:16AM +0000, Neal Liu wrote:
> > -----Original Message-----
> > From: Greg Kroah-Hartman <[email protected]>
> > Sent: Tuesday, November 30, 2021 7:45 PM
> > To: Neal Liu <[email protected]>
> > Cc: Felipe Balbi <[email protected]>; Joel Stanley <[email protected]>; Andrew
> > Jeffery <[email protected]>; Cai Huoqing <[email protected]>; Tao Ren
> > <[email protected]>; Julia Lawall <[email protected]>; kernel test
> > robot <[email protected]>; Sasha Levin <[email protected]>;
> > [email protected]; [email protected];
> > [email protected]; [email protected]; BMC-SW
> > <[email protected]>
> > Subject: Re: [PATCH v2 1/4] usb: aspeed-vhub: add qualifier descriptor
> >
> > On Tue, Nov 30, 2021 at 06:42:53PM +0800, Neal Liu wrote:
> > > Support qualifier descriptor to pass USB30CV compliance test.
> >
> > Please provide more information here in this description. This does not
> > explain what is happening here very well.
> >
> > Also, what is "USB30CV"?
> >
>
> USB 3 Command Verifier (USB3CV) is the official tool for USB 3 Hub and Device Framework testing.
> The USB3CV tool includes the xHCI Compliance Drivers for use with the USB3CV.

Please put that information in the changelog text.

> We would like to pass USB3CV - Chapter 9 Test [USB 2 devices] - Device Qualifier Tests.
> 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, and the host will retrieve it through Get Descriptor request.

Again, this type of information needs to go into the changelog text.

thanks,

greg k-h

2021-12-01 06:31:26

by Neal Liu

[permalink] [raw]
Subject: RE: [PATCH v2 1/4] usb: aspeed-vhub: add qualifier descriptor

> -----Original Message-----
> From: Greg Kroah-Hartman <[email protected]>
> Sent: Wednesday, December 1, 2021 2:18 PM
> To: Neal Liu <[email protected]>
> Cc: Felipe Balbi <[email protected]>; Joel Stanley <[email protected]>; Andrew
> Jeffery <[email protected]>; Cai Huoqing <[email protected]>; Tao Ren
> <[email protected]>; Julia Lawall <[email protected]>; kernel test
> robot <[email protected]>; Sasha Levin <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected]; BMC-SW
> <[email protected]>
> Subject: Re: [PATCH v2 1/4] usb: aspeed-vhub: add qualifier descriptor
>
> On Wed, Dec 01, 2021 at 05:43:16AM +0000, Neal Liu wrote:
> > > -----Original Message-----
> > > From: Greg Kroah-Hartman <[email protected]>
> > > Sent: Tuesday, November 30, 2021 7:45 PM
> > > To: Neal Liu <[email protected]>
> > > Cc: Felipe Balbi <[email protected]>; Joel Stanley <[email protected]>;
> > > Andrew Jeffery <[email protected]>; Cai Huoqing
> > > <[email protected]>; Tao Ren <[email protected]>; Julia
> > > Lawall <[email protected]>; kernel test robot <[email protected]>;
> > > Sasha Levin <[email protected]>; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; BMC-SW <[email protected]>
> > > Subject: Re: [PATCH v2 1/4] usb: aspeed-vhub: add qualifier
> > > descriptor
> > >
> > > On Tue, Nov 30, 2021 at 06:42:53PM +0800, Neal Liu wrote:
> > > > Support qualifier descriptor to pass USB30CV compliance test.
> > >
> > > Please provide more information here in this description. This does
> > > not explain what is happening here very well.
> > >
> > > Also, what is "USB30CV"?
> > >
> >
> > USB 3 Command Verifier (USB3CV) is the official tool for USB 3 Hub and
> Device Framework testing.
> > The USB3CV tool includes the xHCI Compliance Drivers for use with the
> USB3CV.
>
> Please put that information in the changelog text.
>
> > We would like to pass USB3CV - Chapter 9 Test [USB 2 devices] - Device
> Qualifier Tests.
> > 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, and the host will retrieve it through
> Get Descriptor request.
>
> Again, this type of information needs to go into the changelog text.
>
> thanks,
>
> greg k-h

Okay, I'll put it in next patch. Thanks

-Neal