| Message ID | 20230420172130.375819-3-WeitaoWang-oc@zhaoxin.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix some issues of xHCI for zhaoxin | expand |
On 4/20/23 8:21 PM, Weitao Wang wrote: > Add U1/U2 feature support of xHCI for zhaoxin. > > Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com> [...] > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index 6307bae9cddf..730c0f68518d 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c [...] > @@ -4938,6 +4938,27 @@ static int xhci_update_timeout_for_interface(struct xhci_hcd *xhci, > return 0; > } > > +static int xhci_check_zhaoxin_tier_policy(struct usb_device *udev, > + enum usb3_link_state state) > +{ > + struct usb_device *parent; > + unsigned int num_hubs; > + > + /* Don't enable U1/U2 if the device is on an external hub. */ > + for (parent = udev->parent, num_hubs = 0; parent->parent; > + parent = parent->parent) > + num_hubs++; > + > + if (num_hubs < 1) > + return 0; > + > + dev_dbg(&udev->dev, "Disabling U1/U2 link state for device" > + " below external hub.\n"); > + dev_dbg(&udev->dev, "Plug device into root hub " > + "to decrease power consumption.\n"); Please don't break up the message strings. [...] > @@ -4965,6 +4986,8 @@ static int xhci_check_tier_policy(struct xhci_hcd *xhci, > { > if (xhci->quirks & XHCI_INTEL_HOST) > return xhci_check_intel_tier_policy(udev, state); > + else if (xhci->quirks & XHCI_ZHAOXIN_HOST) *else* not needed after *return*. > + return xhci_check_zhaoxin_tier_policy(udev, state); > else > return 0; > } MBR, Sergey
On 20.4.2023 20.21, Weitao Wang wrote: > Add U1/U2 feature support of xHCI for zhaoxin. > > Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com> > --- > drivers/usb/host/xhci-pci.c | 5 +++++ > drivers/usb/host/xhci.c | 27 +++++++++++++++++++++++++-- > 2 files changed, 30 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c > index 6db07ca419c3..a235effe8e5c 100644 > --- a/drivers/usb/host/xhci-pci.c > +++ b/drivers/usb/host/xhci-pci.c > @@ -334,6 +334,11 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) > pdev->device == PCI_DEVICE_ID_AMD_PROMONTORYA_4)) > xhci->quirks |= XHCI_NO_SOFT_RETRY; > > + if (pdev->vendor == PCI_VENDOR_ID_ZHAOXIN) { > + xhci->quirks |= XHCI_LPM_SUPPORT; > + xhci->quirks |= XHCI_ZHAOXIN_HOST; > + } > + > /* xHC spec requires PCI devices to support D3hot and D3cold */ > if (xhci->hci_version >= 0x120) > xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW; > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index 6307bae9cddf..730c0f68518d 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); Looks odd to tie Zhaoxin vendor to Intel specific values but ok, if they diverge in the future we anyway need to modify this. > 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); same. > else > timeout_ns = udev->u2_params.sel; > @@ -4938,6 +4938,27 @@ static int xhci_update_timeout_for_interface(struct xhci_hcd *xhci, > return 0; > } > > +static int xhci_check_zhaoxin_tier_policy(struct usb_device *udev, > + enum usb3_link_state state) > +{ > + struct usb_device *parent; > + unsigned int num_hubs; > + > + /* Don't enable U1/U2 if the device is on an external hub. */ > + for (parent = udev->parent, num_hubs = 0; parent->parent; > + parent = parent->parent) > + num_hubs++; > + > + if (num_hubs < 1) > + return 0; > + > + dev_dbg(&udev->dev, "Disabling U1/U2 link state for device" > + " below external hub.\n"); > + dev_dbg(&udev->dev, "Plug device into root hub " > + "to decrease power consumption.\n"); > + return -E2BIG; > +} > + I don't think we should add more vendor specific functions, this is almost an exact copy of xhci_check_intel_tier_policy(). How about getting rid of both of those and use something like this instead (untested): diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 2b280beb0011..e9a25e4d99cf 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -4926,10 +4926,24 @@ 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; + struct usb_device *parent = udev->parent; + int tier = 1; /* roothub is tier1 */ + + while (parent) { + parent = parent->parent; + tier++; + } + + if (xhci->quirks & XHCI_INTEL_HOST && tier > 3) + goto fail; + if (xhci->quirks & XHCI_ZHAOXIN_HOST && tier > 2) + goto fail; + + return 0; +fail: + dev_dbg(&udev->dev, "Tier policy prevents U1/U2 LPM states for devices at tier %d\n", + tier); + return -E2BIG; } Or possibly even add a xhci->max_tier_for_lpm that can be set during probe based on vendor or from device property. Thanks -Mathias
On 4/20/23 11:21 PM, WeitaoWang-oc@zhaoxin.com wrote: >>> Add U1/U2 feature support of xHCI for zhaoxin. >>> >>> Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com> >> [...] >> >>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c >>> index 6307bae9cddf..730c0f68518d 100644 >>> --- a/drivers/usb/host/xhci.c >>> +++ b/drivers/usb/host/xhci.c >> [...] >>> @@ -4938,6 +4938,27 @@ static int xhci_update_timeout_for_interface(struct xhci_hcd *xhci, >>> return 0; >>> } >>> +static int xhci_check_zhaoxin_tier_policy(struct usb_device *udev, >>> + enum usb3_link_state state) >>> +{ >>> + struct usb_device *parent; >>> + unsigned int num_hubs; >>> + >>> + /* Don't enable U1/U2 if the device is on an external hub. */ >>> + for (parent = udev->parent, num_hubs = 0; parent->parent; >>> + parent = parent->parent) >>> + num_hubs++; >>> + >>> + if (num_hubs < 1) >>> + return 0; >>> + >>> + dev_dbg(&udev->dev, "Disabling U1/U2 link state for device" >>> + " below external hub.\n"); >>> + dev_dbg(&udev->dev, "Plug device into root hub " >>> + "to decrease power consumption.\n"); >> >> Please don't break up the message strings. > > Thanks for your advice, and I will merge this message in next patch version. >> [...] >>> @@ -4965,6 +4986,8 @@ static int xhci_check_tier_policy(struct xhci_hcd *xhci, >>> { >>> if (xhci->quirks & XHCI_INTEL_HOST) >>> return xhci_check_intel_tier_policy(udev, state); >>> + else if (xhci->quirks & XHCI_ZHAOXIN_HOST) >> >> *else* not needed after *return*. > This function need a "int" type return value. If remove "else" branch, > vendor other than intel and zhaoxin will not get a return value. I didn't tell you to remove the whole branch, just the *else* keyword. [...] > Best Regards, > Weitao MBR, Sergey
On 2023/4/20 17:39, Sergei Shtylyov wrote: > On 4/20/23 8:21 PM, Weitao Wang wrote: > >> Add U1/U2 feature support of xHCI for zhaoxin. >> >> Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com> > [...] > >> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c >> index 6307bae9cddf..730c0f68518d 100644 >> --- a/drivers/usb/host/xhci.c >> +++ b/drivers/usb/host/xhci.c > [...] >> @@ -4938,6 +4938,27 @@ static int xhci_update_timeout_for_interface(struct xhci_hcd *xhci, >> return 0; >> } >> >> +static int xhci_check_zhaoxin_tier_policy(struct usb_device *udev, >> + enum usb3_link_state state) >> +{ >> + struct usb_device *parent; >> + unsigned int num_hubs; >> + >> + /* Don't enable U1/U2 if the device is on an external hub. */ >> + for (parent = udev->parent, num_hubs = 0; parent->parent; >> + parent = parent->parent) >> + num_hubs++; >> + >> + if (num_hubs < 1) >> + return 0; >> + >> + dev_dbg(&udev->dev, "Disabling U1/U2 link state for device" >> + " below external hub.\n"); >> + dev_dbg(&udev->dev, "Plug device into root hub " >> + "to decrease power consumption.\n"); > > Please don't break up the message strings. Thanks for your advice, and I will merge this message in next patch version. > [...] >> @@ -4965,6 +4986,8 @@ static int xhci_check_tier_policy(struct xhci_hcd *xhci, >> { >> if (xhci->quirks & XHCI_INTEL_HOST) >> return xhci_check_intel_tier_policy(udev, state); >> + else if (xhci->quirks & XHCI_ZHAOXIN_HOST) > > *else* not needed after *return*. This function need a "int" type return value. If remove "else" branch, vendor other than intel and zhaoxin will not get a return value. Best Regards, Weitao >> + return xhci_check_zhaoxin_tier_policy(udev, state); >> else >> return 0; >> } > > MBR, Sergey > .
On 21.4.2023 13.56, WeitaoWang-oc@zhaoxin.com wrote: > On 2023/4/20 22:07, Mathias Nyman wrote: >> On 20.4.2023 20.21, Weitao Wang wrote: >>> Add U1/U2 feature support of xHCI for zhaoxin. >>> >>> Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com> >>> --- >>> drivers/usb/host/xhci-pci.c | 5 +++++ >>> drivers/usb/host/xhci.c | 27 +++++++++++++++++++++++++-- >>> 2 files changed, 30 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c >>> index 6db07ca419c3..a235effe8e5c 100644 >>> --- a/drivers/usb/host/xhci-pci.c >>> +++ b/drivers/usb/host/xhci-pci.c >>> @@ -334,6 +334,11 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) >>> pdev->device == PCI_DEVICE_ID_AMD_PROMONTORYA_4)) >>> xhci->quirks |= XHCI_NO_SOFT_RETRY; >>> + if (pdev->vendor == PCI_VENDOR_ID_ZHAOXIN) { >>> + xhci->quirks |= XHCI_LPM_SUPPORT; >>> + xhci->quirks |= XHCI_ZHAOXIN_HOST; >>> + } >>> + >>> /* xHC spec requires PCI devices to support D3hot and D3cold */ >>> if (xhci->hci_version >= 0x120) >>> xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW; >>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c >>> index 6307bae9cddf..730c0f68518d 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); >> >> Looks odd to tie Zhaoxin vendor to Intel specific values but ok, >> if they diverge in the future we anyway need to modify this. > > These Intel specific values look good for zhaoxin xHCI with test. > Reused this piece of code for simplicity. If there are any difference > for these value, update will be submitted in the future. > >>> 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); >> >> same. >> >>> else >>> timeout_ns = udev->u2_params.sel; >>> @@ -4938,6 +4938,27 @@ static int xhci_update_timeout_for_interface(struct xhci_hcd *xhci, >>> return 0; >>> } >>> +static int xhci_check_zhaoxin_tier_policy(struct usb_device *udev, >>> + enum usb3_link_state state) >>> +{ >>> + struct usb_device *parent; >>> + unsigned int num_hubs; >>> + >>> + /* Don't enable U1/U2 if the device is on an external hub. */ >>> + for (parent = udev->parent, num_hubs = 0; parent->parent; >>> + parent = parent->parent) >>> + num_hubs++; >>> + >>> + if (num_hubs < 1) >>> + return 0; >>> + >>> + dev_dbg(&udev->dev, "Disabling U1/U2 link state for device" >>> + " below external hub.\n"); >>> + dev_dbg(&udev->dev, "Plug device into root hub " >>> + "to decrease power consumption.\n"); >>> + return -E2BIG; >>> +} >>> + >> >> I don't think we should add more vendor specific functions, this is almost >> an exact copy of xhci_check_intel_tier_policy(). > > Adding duplicate vendor related code is indeed a bit redundant. > >> How about getting rid of both of those and use something like this instead (untested): >> >> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c >> index 2b280beb0011..e9a25e4d99cf 100644 >> --- a/drivers/usb/host/xhci.c >> +++ b/drivers/usb/host/xhci.c >> @@ -4926,10 +4926,24 @@ 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; >> + struct usb_device *parent = udev->parent; >> + int tier = 1; /* roothub is tier1 */ >> + >> + while (parent) { >> + parent = parent->parent; >> + tier++; >> + } >> + >> + if (xhci->quirks & XHCI_INTEL_HOST && tier > 3) >> + goto fail; >> + if (xhci->quirks & XHCI_ZHAOXIN_HOST && tier > 2) >> + goto fail; >> + >> + return 0; >> +fail: >> + dev_dbg(&udev->dev, "Tier policy prevents U1/U2 LPM states for devices at tier %d\n", >> + tier); >> + return -E2BIG; >> } > > These code looks very elegant. Could I resubmit patch using your code > after testing pass on Intel and Zhaoxin platform. > Yes, you can add Suggested-by: Mathias Nyman <mathias.nyman@linux.intel.com> Thanks Mathias
On 2023/4/20 22:22, Sergei Shtylyov wrote: > On 4/20/23 11:21 PM, WeitaoWang-oc@zhaoxin.com wrote: > >>>> Add U1/U2 feature support of xHCI for zhaoxin. >>>> >>>> Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com> >>> [...] >>> >>>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c >>>> index 6307bae9cddf..730c0f68518d 100644 >>>> --- a/drivers/usb/host/xhci.c >>>> +++ b/drivers/usb/host/xhci.c >>> [...] >>>> @@ -4938,6 +4938,27 @@ static int xhci_update_timeout_for_interface(struct xhci_hcd *xhci, >>>> return 0; >>>> } >>>> +static int xhci_check_zhaoxin_tier_policy(struct usb_device *udev, >>>> + enum usb3_link_state state) >>>> +{ >>>> + struct usb_device *parent; >>>> + unsigned int num_hubs; >>>> + >>>> + /* Don't enable U1/U2 if the device is on an external hub. */ >>>> + for (parent = udev->parent, num_hubs = 0; parent->parent; >>>> + parent = parent->parent) >>>> + num_hubs++; >>>> + >>>> + if (num_hubs < 1) >>>> + return 0; >>>> + >>>> + dev_dbg(&udev->dev, "Disabling U1/U2 link state for device" >>>> + " below external hub.\n"); >>>> + dev_dbg(&udev->dev, "Plug device into root hub " >>>> + "to decrease power consumption.\n"); >>> >>> Please don't break up the message strings. >> >> Thanks for your advice, and I will merge this message in next patch version. >>> [...] >>>> @@ -4965,6 +4986,8 @@ static int xhci_check_tier_policy(struct xhci_hcd *xhci, >>>> { >>>> if (xhci->quirks & XHCI_INTEL_HOST) >>>> return xhci_check_intel_tier_policy(udev, state); >>>> + else if (xhci->quirks & XHCI_ZHAOXIN_HOST) >>> >>> *else* not needed after *return*. >> This function need a "int" type return value. If remove "else" branch, >> vendor other than intel and zhaoxin will not get a return value. > > I didn't tell you to remove the whole branch, just the *else* keyword. I understand what you mean this time. Here's code needs more concise. I'll try to do more think and test.Thanks again! weitao > [...] >> Best Regards, >> Weitao > > MBR, Sergey > .
On 2023/4/20 22:07, Mathias Nyman wrote: > On 20.4.2023 20.21, Weitao Wang wrote: >> Add U1/U2 feature support of xHCI for zhaoxin. >> >> Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com> >> --- >> drivers/usb/host/xhci-pci.c | 5 +++++ >> drivers/usb/host/xhci.c | 27 +++++++++++++++++++++++++-- >> 2 files changed, 30 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c >> index 6db07ca419c3..a235effe8e5c 100644 >> --- a/drivers/usb/host/xhci-pci.c >> +++ b/drivers/usb/host/xhci-pci.c >> @@ -334,6 +334,11 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) >> pdev->device == PCI_DEVICE_ID_AMD_PROMONTORYA_4)) >> xhci->quirks |= XHCI_NO_SOFT_RETRY; >> + if (pdev->vendor == PCI_VENDOR_ID_ZHAOXIN) { >> + xhci->quirks |= XHCI_LPM_SUPPORT; >> + xhci->quirks |= XHCI_ZHAOXIN_HOST; >> + } >> + >> /* xHC spec requires PCI devices to support D3hot and D3cold */ >> if (xhci->hci_version >= 0x120) >> xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW; >> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c >> index 6307bae9cddf..730c0f68518d 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); > > Looks odd to tie Zhaoxin vendor to Intel specific values but ok, > if they diverge in the future we anyway need to modify this. These Intel specific values look good for zhaoxin xHCI with test. Reused this piece of code for simplicity. If there are any difference for these value, update will be submitted in the future. >> 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); > > same. > >> else >> timeout_ns = udev->u2_params.sel; >> @@ -4938,6 +4938,27 @@ static int xhci_update_timeout_for_interface(struct xhci_hcd *xhci, >> return 0; >> } >> +static int xhci_check_zhaoxin_tier_policy(struct usb_device *udev, >> + enum usb3_link_state state) >> +{ >> + struct usb_device *parent; >> + unsigned int num_hubs; >> + >> + /* Don't enable U1/U2 if the device is on an external hub. */ >> + for (parent = udev->parent, num_hubs = 0; parent->parent; >> + parent = parent->parent) >> + num_hubs++; >> + >> + if (num_hubs < 1) >> + return 0; >> + >> + dev_dbg(&udev->dev, "Disabling U1/U2 link state for device" >> + " below external hub.\n"); >> + dev_dbg(&udev->dev, "Plug device into root hub " >> + "to decrease power consumption.\n"); >> + return -E2BIG; >> +} >> + > > I don't think we should add more vendor specific functions, this is almost > an exact copy of xhci_check_intel_tier_policy(). Adding duplicate vendor related code is indeed a bit redundant. > How about getting rid of both of those and use something like this instead (untested): > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index 2b280beb0011..e9a25e4d99cf 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -4926,10 +4926,24 @@ 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; > + struct usb_device *parent = udev->parent; > + int tier = 1; /* roothub is tier1 */ > + > + while (parent) { > + parent = parent->parent; > + tier++; > + } > + > + if (xhci->quirks & XHCI_INTEL_HOST && tier > 3) > + goto fail; > + if (xhci->quirks & XHCI_ZHAOXIN_HOST && tier > 2) > + goto fail; > + > + return 0; > +fail: > + dev_dbg(&udev->dev, "Tier policy prevents U1/U2 LPM states for devices at tier %d\n", > + tier); > + return -E2BIG; > } These code looks very elegant. Could I resubmit patch using your code after testing pass on Intel and Zhaoxin platform. Thanks, weitao > Or possibly even add a xhci->max_tier_for_lpm that can be set during probe based on > vendor or from device property. > > Thanks > -Mathias > > .
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 6db07ca419c3..a235effe8e5c 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -334,6 +334,11 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) pdev->device == PCI_DEVICE_ID_AMD_PROMONTORYA_4)) xhci->quirks |= XHCI_NO_SOFT_RETRY; + if (pdev->vendor == PCI_VENDOR_ID_ZHAOXIN) { + xhci->quirks |= XHCI_LPM_SUPPORT; + xhci->quirks |= XHCI_ZHAOXIN_HOST; + } + /* xHC spec requires PCI devices to support D3hot and D3cold */ if (xhci->hci_version >= 0x120) xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW; diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 6307bae9cddf..730c0f68518d 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,6 +4938,27 @@ static int xhci_update_timeout_for_interface(struct xhci_hcd *xhci, return 0; } +static int xhci_check_zhaoxin_tier_policy(struct usb_device *udev, + enum usb3_link_state state) +{ + struct usb_device *parent; + unsigned int num_hubs; + + /* Don't enable U1/U2 if the device is on an external hub. */ + for (parent = udev->parent, num_hubs = 0; parent->parent; + parent = parent->parent) + num_hubs++; + + if (num_hubs < 1) + return 0; + + dev_dbg(&udev->dev, "Disabling U1/U2 link state for device" + " below external hub.\n"); + dev_dbg(&udev->dev, "Plug device into root hub " + "to decrease power consumption.\n"); + return -E2BIG; +} + static int xhci_check_intel_tier_policy(struct usb_device *udev, enum usb3_link_state state) { @@ -4965,6 +4986,8 @@ static int xhci_check_tier_policy(struct xhci_hcd *xhci, { if (xhci->quirks & XHCI_INTEL_HOST) return xhci_check_intel_tier_policy(udev, state); + else if (xhci->quirks & XHCI_ZHAOXIN_HOST) + return xhci_check_zhaoxin_tier_policy(udev, state); else return 0; }
Add U1/U2 feature support of xHCI for zhaoxin. Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com> --- drivers/usb/host/xhci-pci.c | 5 +++++ drivers/usb/host/xhci.c | 27 +++++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-)