2023-04-21 12:40:10

by Weitao Wang

[permalink] [raw]
Subject: [PATCH v2 0/4] Fix some issues of xHCI for zhaoxin

Fix some issues of xHCI for zhaoxin.

Weitao Wang (4):
xhci: Add some quirks for zhaoxin xhci to fix issues
xhci: fix issue of cross page boundary in TRB prefetch
xhci: Show zhaoxin xHCI root hub speed correctly
xhci: Add zhaoxin xHCI U1/U2 feature support

drivers/usb/host/xhci-mem.c | 8 +++--
drivers/usb/host/xhci-pci.c | 11 +++++++
drivers/usb/host/xhci.c | 65 +++++++++++++++++++++++--------------
drivers/usb/host/xhci.h | 2 ++
4 files changed, 59 insertions(+), 27 deletions(-)

--
2.32.0


2023-04-21 12:40:16

by Weitao Wang

[permalink] [raw]
Subject: [PATCH v2 2/4] xhci: fix issue of cross page boundary in TRB prefetch

On some Zhaoxin platforms, xHCI will prefetch TRB for performance
improvement. However this TRB prefetch mechanism may cross page boundary,
which may access memory not allocated by xHCI driver. In order to fix
this issue, two pages was allocated for TRB and only the first
page will be used.

Cc: [email protected]
Signed-off-by: Weitao Wang <[email protected]>
---
drivers/usb/host/xhci-mem.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index d0a9467aa5fc..d5517400d874 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2369,8 +2369,12 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
* and our use of dma addresses in the trb_address_map radix tree needs
* TRB_SEGMENT_SIZE alignment, so we pick the greater alignment need.
*/
- xhci->segment_pool = dma_pool_create("xHCI ring segments", dev,
- TRB_SEGMENT_SIZE, TRB_SEGMENT_SIZE, xhci->page_size);
+ if (xhci->quirks & XHCI_ZHAOXIN_TRB_FETCH)
+ xhci->segment_pool = dma_pool_create("xHCI ring segments", dev,
+ TRB_SEGMENT_SIZE * 2, TRB_SEGMENT_SIZE * 2, xhci->page_size * 2);
+ else
+ xhci->segment_pool = dma_pool_create("xHCI ring segments", dev,
+ TRB_SEGMENT_SIZE, TRB_SEGMENT_SIZE, xhci->page_size);

/* See Table 46 and Note on Figure 55 */
xhci->device_pool = dma_pool_create("xHCI input/output contexts", dev,
--
2.32.0

2023-04-21 12:41:03

by Weitao Wang

[permalink] [raw]
Subject: [PATCH v2 4/4] xhci: Add zhaoxin xHCI U1/U2 feature support

Add U1/U2 feature support of xHCI for zhaoxin.
Since both Intel and Zhaoxin need to check the tier where the device is
located to determine whether to enabled U1/U2, remove the previous Intel
U1/U2 tier policy and add common policy in xhci_check_tier_policy.
If vendor has specific U1/U2 enable policy,quirks can be add to declare.

Suggested-by: Mathias Nyman <[email protected]>
Cc: [email protected]
Signed-off-by: Weitao Wang <[email protected]>
---
v1->v2
- Modify the description.
- Adjust U1/U2 tier enable policy.

drivers/usb/host/xhci.c | 43 +++++++++++++++++------------------------
1 file changed, 18 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 31d6ace9cace..b81a69126188 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4802,7 +4802,7 @@ static u16 xhci_calculate_u1_timeout(struct xhci_hcd *xhci,
}
}

- if (xhci->quirks & XHCI_INTEL_HOST)
+ if (xhci->quirks & (XHCI_INTEL_HOST | XHCI_ZHAOXIN_HOST))
timeout_ns = xhci_calculate_intel_u1_timeout(udev, desc);
else
timeout_ns = udev->u1_params.sel;
@@ -4866,7 +4866,7 @@ static u16 xhci_calculate_u2_timeout(struct xhci_hcd *xhci,
}
}

- if (xhci->quirks & XHCI_INTEL_HOST)
+ if (xhci->quirks & (XHCI_INTEL_HOST | XHCI_ZHAOXIN_HOST))
timeout_ns = xhci_calculate_intel_u2_timeout(udev, desc);
else
timeout_ns = udev->u2_params.sel;
@@ -4938,37 +4938,30 @@ static int xhci_update_timeout_for_interface(struct xhci_hcd *xhci,
return 0;
}

-static int xhci_check_intel_tier_policy(struct usb_device *udev,
+static int xhci_check_tier_policy(struct xhci_hcd *xhci,
+ struct usb_device *udev,
enum usb3_link_state state)
{
- struct usb_device *parent;
- unsigned int num_hubs;
+ struct usb_device *parent = udev->parent;
+ int tier = 1; /* roothub is tier1 */

- /* Don't enable U1 if the device is on a 2nd tier hub or lower. */
- for (parent = udev->parent, num_hubs = 0; parent->parent;
- parent = parent->parent)
- num_hubs++;
+ while (parent) {
+ parent = parent->parent;
+ tier++;
+ }

- if (num_hubs < 2)
- return 0;
+ if (xhci->quirks & XHCI_INTEL_HOST && tier > 3)
+ goto fail;
+ if (xhci->quirks & XHCI_ZHAOXIN_HOST && tier > 2)
+ goto fail;

- dev_dbg(&udev->dev, "Disabling U1/U2 link state for device"
- " below second-tier hub.\n");
- dev_dbg(&udev->dev, "Plug device into first-tier hub "
- "to decrease power consumption.\n");
+ return 0;
+fail:
+ dev_dbg(&udev->dev, "Tier policy prevents U1/U2 LPM states for devices at tier %d\n",
+ tier);
return -E2BIG;
}

-static int xhci_check_tier_policy(struct xhci_hcd *xhci,
- struct usb_device *udev,
- enum usb3_link_state state)
-{
- if (xhci->quirks & XHCI_INTEL_HOST)
- return xhci_check_intel_tier_policy(udev, state);
- else
- return 0;
-}
-
/* Returns the U1 or U2 timeout that should be enabled.
* If the tier check or timeout setting functions return with a non-zero exit
* code, that means the timeout value has been finalized and we shouldn't look
--
2.32.0

2023-04-21 12:41:04

by Weitao Wang

[permalink] [raw]
Subject: [PATCH v2 3/4] xhci: Show zhaoxin xHCI root hub speed correctly

Some zhaoxin xHCI controllers follow usb3.1 spec,
but only support gen1 speed 5G. While in Linux kernel,
if xHCI suspport usb3.1,root hub speed will show on 10G.
To fix this issue of zhaoxin xHCI platforms, read usb speed ID
supported by xHCI to determine root hub speed.

Signed-off-by: Weitao Wang <[email protected]>
---
drivers/usb/host/xhci.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 6307bae9cddf..31d6ace9cace 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -5294,6 +5294,7 @@ static void xhci_hcd_init_usb2_data(struct xhci_hcd *xhci, struct usb_hcd *hcd)
static void xhci_hcd_init_usb3_data(struct xhci_hcd *xhci, struct usb_hcd *hcd)
{
unsigned int minor_rev;
+ unsigned int i, j;

/*
* Early xHCI 1.1 spec did not mention USB 3.1 capable hosts
@@ -5323,6 +5324,27 @@ static void xhci_hcd_init_usb3_data(struct xhci_hcd *xhci, struct usb_hcd *hcd)
hcd->self.root_hub->ssp_rate = USB_SSP_GEN_2x1;
break;
}
+
+ /* Usb3.1 has gen1 and gen2, Some zhaoxin's xHCI controller
+ * that follow usb3.1 spec but only support gen1.
+ */
+ if (xhci->quirks & XHCI_ZHAOXIN_HOST) {
+ minor_rev = 0;
+ for (j = 0; j < xhci->num_port_caps; j++) {
+ for (i = 0; i < xhci->port_caps[j].psi_count; i++) {
+ if (XHCI_EXT_PORT_PSIV(xhci->port_caps[j].psi[i]) >= 5) {
+ minor_rev = 1;
+ break;
+ }
+ }
+ if (minor_rev)
+ break;
+ }
+ if (minor_rev != 1) {
+ hcd->speed = HCD_USB3;
+ hcd->self.root_hub->speed = USB_SPEED_SUPER;
+ }
+ }
xhci_info(xhci, "Host supports USB 3.%x %sSuperSpeed\n",
minor_rev, minor_rev ? "Enhanced " : "");

--
2.32.0

2023-05-05 11:17:33

by Mathias Nyman

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] xhci: Show zhaoxin xHCI root hub speed correctly

On 21.4.2023 23.38, Weitao Wang wrote:
> Some zhaoxin xHCI controllers follow usb3.1 spec,
> but only support gen1 speed 5G. While in Linux kernel,
> if xHCI suspport usb3.1,root hub speed will show on 10G.
> To fix this issue of zhaoxin xHCI platforms, read usb speed ID
> supported by xHCI to determine root hub speed.
>
> Signed-off-by: Weitao Wang <[email protected]>
> ---
> drivers/usb/host/xhci.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 6307bae9cddf..31d6ace9cace 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -5294,6 +5294,7 @@ static void xhci_hcd_init_usb2_data(struct xhci_hcd *xhci, struct usb_hcd *hcd)
> static void xhci_hcd_init_usb3_data(struct xhci_hcd *xhci, struct usb_hcd *hcd)
> {
> unsigned int minor_rev;
> + unsigned int i, j;
>
> /*
> * Early xHCI 1.1 spec did not mention USB 3.1 capable hosts
> @@ -5323,6 +5324,27 @@ static void xhci_hcd_init_usb3_data(struct xhci_hcd *xhci, struct usb_hcd *hcd)
> hcd->self.root_hub->ssp_rate = USB_SSP_GEN_2x1;
> break;
> }
> +
> + /* Usb3.1 has gen1 and gen2, Some zhaoxin's xHCI controller
> + * that follow usb3.1 spec but only support gen1.
> + */
> + if (xhci->quirks & XHCI_ZHAOXIN_HOST) {
> + minor_rev = 0;
> + for (j = 0; j < xhci->num_port_caps; j++) {
> + for (i = 0; i < xhci->port_caps[j].psi_count; i++) {
> + if (XHCI_EXT_PORT_PSIV(xhci->port_caps[j].psi[i]) >= 5) {
> + minor_rev = 1;
> + break;
> + }
> + }
> + if (minor_rev)
> + break;
> + }
> + if (minor_rev != 1) {
> + hcd->speed = HCD_USB3;
> + hcd->self.root_hub->speed = USB_SPEED_SUPER;
> + }
> + }
> xhci_info(xhci, "Host supports USB 3.%x %sSuperSpeed\n",
> minor_rev, minor_rev ? "Enhanced " : "");
>

How about checking if port support over 5Gbps (psiv >= 5) when we parse the protocol speed ID
entries the first time? This way we could avoid looping through all the port_cap psiv values.

Something like:

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index c4170421bc9c..2e4c80eb4972 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1961,7 +1961,7 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int num_ports,
{
u32 temp, port_offset, port_count;
int i;
- u8 major_revision, minor_revision;
+ u8 major_revision, minor_revision, tmp_minor_revision;
struct xhci_hub *rhub;
struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
struct xhci_port_cap *port_cap;
@@ -1981,6 +1981,11 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int num_ports,
*/
if (minor_revision > 0x00 && minor_revision < 0x10)
minor_revision <<= 4;
+ if (xhci->quirks & XHCI_ZHAOXIN_HOST) {
+ tmp_minor_revision = minor_revision;
+ minor_revision = 0;
+ }
+
} else if (major_revision <= 0x02) {
rhub = &xhci->usb2_rhub;
} else {
@@ -1989,10 +1994,6 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int num_ports,
/* Ignoring port protocol we can't understand. FIXME */
return;
}
- rhub->maj_rev = XHCI_EXT_PORT_MAJOR(temp);
-
- if (rhub->min_rev < minor_revision)
- rhub->min_rev = minor_revision;

/* Port offset and count in the third dword, see section 7.2 */
temp = readl(addr + 2);
@@ -2010,8 +2011,6 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int num_ports,
if (xhci->num_port_caps > max_caps)
return;

- port_cap->maj_rev = major_revision;
- port_cap->min_rev = minor_revision;
port_cap->psi_count = XHCI_EXT_PORT_PSIC(temp);

if (port_cap->psi_count) {
@@ -2032,6 +2031,10 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int num_ports,
XHCI_EXT_PORT_PSIV(port_cap->psi[i - 1])))
port_cap->psi_uid_count++;

+ if (xhci->quirks & XHCI_ZHAOXIN_HOST &&
+ XHCI_EXT_PORT_PSIV(port_cap->psi[i]) >= 5)
+ minor_revision = tmp_minor_revision;
+
xhci_dbg(xhci, "PSIV:%d PSIE:%d PLT:%d PFD:%d LP:%d PSIM:%d\n",
XHCI_EXT_PORT_PSIV(port_cap->psi[i]),
XHCI_EXT_PORT_PSIE(port_cap->psi[i]),
@@ -2041,6 +2044,15 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int num_ports,
XHCI_EXT_PORT_PSIM(port_cap->psi[i]));
}
}
+
+ rhub->maj_rev = major_revision;
+
+ if (rhub->min_rev < minor_revision)
+ rhub->min_rev = minor_revision;
+
+ port_cap->maj_rev = major_revision;
+ port_cap->min_rev = minor_revision;
+
/* cache usb2 port capabilities */
if (major_revision < 0x03 && xhci->num_ext_caps < max_caps)
xhci->ext_caps[xhci->num_ext_caps++] = temp;

Thanks
Mathias

2023-05-06 03:50:42

by Weitao Wang

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] xhci: Show zhaoxin xHCI root hub speed correctly

On 2023/5/5 18:52, Mathias Nyman wrote:
> On 21.4.2023 23.38, Weitao Wang wrote:
>> Some zhaoxin xHCI controllers follow usb3.1 spec,
>> but only support gen1 speed 5G. While in Linux kernel,
>> if xHCI suspport usb3.1,root hub speed will show on 10G.
>> To fix this issue of zhaoxin xHCI platforms, read usb speed ID
>> supported by xHCI to determine root hub speed.
>>
>> Signed-off-by: Weitao Wang <[email protected]>
>> ---
>>   drivers/usb/host/xhci.c | 22 ++++++++++++++++++++++
>>   1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index 6307bae9cddf..31d6ace9cace 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -5294,6 +5294,7 @@ static void xhci_hcd_init_usb2_data(struct xhci_hcd *xhci, struct
>> usb_hcd *hcd)
>>   static void xhci_hcd_init_usb3_data(struct xhci_hcd *xhci, struct usb_hcd *hcd)
>>   {
>>       unsigned int minor_rev;
>> +    unsigned int i, j;
>>       /*
>>        * Early xHCI 1.1 spec did not mention USB 3.1 capable hosts
>> @@ -5323,6 +5324,27 @@ static void xhci_hcd_init_usb3_data(struct xhci_hcd *xhci, struct
>> usb_hcd *hcd)
>>           hcd->self.root_hub->ssp_rate = USB_SSP_GEN_2x1;
>>           break;
>>       }
>> +
>> +    /* Usb3.1 has gen1 and gen2, Some zhaoxin's xHCI controller
>> +     * that follow usb3.1 spec but only support gen1.
>> +     */
>> +    if (xhci->quirks & XHCI_ZHAOXIN_HOST) {
>> +        minor_rev = 0;
>> +        for (j = 0; j < xhci->num_port_caps; j++) {
>> +            for (i = 0; i < xhci->port_caps[j].psi_count; i++) {
>> +                if (XHCI_EXT_PORT_PSIV(xhci->port_caps[j].psi[i]) >= 5) {
>> +                    minor_rev = 1;
>> +                    break;
>> +                }
>> +            }
>> +            if (minor_rev)
>> +                break;
>> +        }
>> +        if (minor_rev != 1) {
>> +            hcd->speed = HCD_USB3;
>> +            hcd->self.root_hub->speed = USB_SPEED_SUPER;
>> +        }
>> +    }
>>       xhci_info(xhci, "Host supports USB 3.%x %sSuperSpeed\n",
>>             minor_rev, minor_rev ? "Enhanced " : "");
>
> How about checking if port support over 5Gbps (psiv >= 5) when we parse the protocol speed ID
> entries the first time? This way we could avoid looping through all the port_cap psiv values.
>
> Something like:
>
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index c4170421bc9c..2e4c80eb4972 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -1961,7 +1961,7 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int
> num_ports,
>  {
>         u32 temp, port_offset, port_count;
>         int i;
> -       u8 major_revision, minor_revision;
> +       u8 major_revision, minor_revision, tmp_minor_revision;
>         struct xhci_hub *rhub;
>         struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
>         struct xhci_port_cap *port_cap;
> @@ -1981,6 +1981,11 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int
> num_ports,
>                  */
>                 if (minor_revision > 0x00 && minor_revision < 0x10)
>                         minor_revision <<= 4;
> +               if (xhci->quirks & XHCI_ZHAOXIN_HOST) {
> +                       tmp_minor_revision = minor_revision;
> +                       minor_revision = 0;
> +               }
> +
>         } else if (major_revision <= 0x02) {
>                 rhub = &xhci->usb2_rhub;
>         } else {
> @@ -1989,10 +1994,6 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int
> num_ports,
>                 /* Ignoring port protocol we can't understand. FIXME */
>                 return;
>         }
> -       rhub->maj_rev = XHCI_EXT_PORT_MAJOR(temp);
> -
> -       if (rhub->min_rev < minor_revision)
> -               rhub->min_rev = minor_revision;
>
>         /* Port offset and count in the third dword, see section 7.2 */
>         temp = readl(addr + 2);
> @@ -2010,8 +2011,6 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int
> num_ports,
>         if (xhci->num_port_caps > max_caps)
>                 return;
>
> -       port_cap->maj_rev = major_revision;
> -       port_cap->min_rev = minor_revision;
>         port_cap->psi_count = XHCI_EXT_PORT_PSIC(temp);
>
>         if (port_cap->psi_count) {
> @@ -2032,6 +2031,10 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int
> num_ports,
>                                   XHCI_EXT_PORT_PSIV(port_cap->psi[i - 1])))
>                                 port_cap->psi_uid_count++;
>
> +                       if (xhci->quirks & XHCI_ZHAOXIN_HOST &&
> +                           XHCI_EXT_PORT_PSIV(port_cap->psi[i]) >= 5)
> +                               minor_revision = tmp_minor_revision;
> +
>                         xhci_dbg(xhci, "PSIV:%d PSIE:%d PLT:%d PFD:%d LP:%d PSIM:%d\n",
>                                   XHCI_EXT_PORT_PSIV(port_cap->psi[i]),
>                                   XHCI_EXT_PORT_PSIE(port_cap->psi[i]),
> @@ -2041,6 +2044,15 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int
> num_ports,
>                                   XHCI_EXT_PORT_PSIM(port_cap->psi[i]));
>                 }
>         }
> +
> +       rhub->maj_rev = major_revision;
> +
> +       if (rhub->min_rev < minor_revision)
> +               rhub->min_rev = minor_revision;
> +
> +       port_cap->maj_rev = major_revision;
> +       port_cap->min_rev = minor_revision;
> +

This patch solution is effective and concise. Thanks for your suggestion.
I'll adopt it in next patch version after other patches of this patch set
are reviewed or suggested.

Best Regards,
Weitao
>         /* cache usb2 port capabilities */
>         if (major_revision < 0x03 && xhci->num_ext_caps < max_caps)
>                 xhci->ext_caps[xhci->num_ext_caps++] = temp;
>
> Thanks
> Mathias
> .