Thanks Rajmohan. Comments inline.
On Thu, Jul 9, 2015 at 6:17 PM, <rajmohan.mani@xxxxxxxxx> wrote:
> From: Rajmohan Mani <rajmohan.mani@xxxxxxxxx>
>
> The xHCI in Intel CherryView / Braswell Platform requires
> a driver workaround to get xHCI D3 working. Without this
> workaround, xHCI might not enter D3.
>
> Workaround is to configure SSIC PORT as "unused" before D3
> entry and "used" after D3 exit. This is done through a
> vendor specific register (PORT2_SSIC_CONFIG_REG2 at offset
> 0x883c), in xhci suspend / resume callbacks.
>
> Verified xHCI D3 works fine in CherryView / Braswell platform.
>
> Signed-off-by: Rajmohan Mani <rajmohan.mani@xxxxxxxxx>
> ---
> drivers/usb/host/xhci-pci.c | 40 +++++++++++++++++++++++++++++++++++++---
> 1 file changed, 37 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 91260b3..7a69a12 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -27,6 +27,10 @@
> #include "xhci.h"
> #include "xhci-trace.h"
>
> +#define PORT2_SSIC_CONFIG_REG2 0x883c
> +#define PROG_DONE (1 << 30)
> +#define SSIC_PORT_UNUSED (1 << 31)
> +
> /* Device for a quirk */
> #define PCI_VENDOR_ID_FRESCO_LOGIC 0x1b73
> #define PCI_DEVICE_ID_FRESCO_LOGIC_PDK 0x1000
> @@ -167,14 +171,44 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
> }
>
> /*
> + * In some Intel xHCI controllers, in order to get D3 working,
> + * through a vendor specific SSIC CONFIG register at offset 0x883c,
> + * SSIC PORT need to be marked as "unused" before putting xHCI
> + * into D3. After D3 exit, the SSIC port need to be marked as "used".
> + * Without this change, xHCI might not enter D3 state.
> * Make sure PME works on some Intel xHCI controllers by writing 1 to clear
> * the Internal PME flag bit in vendor specific PMCTRL register at offset 0x80a4
> */
> -static void xhci_pme_quirk(struct xhci_hcd *xhci)
> +static void xhci_pme_quirk(struct usb_hcd *hcd, bool suspend)
Is the function name here still descriptive of the quirk in question?
It seems like the SSIC change is a separate workaround entirely from
the PME quirk that existed here previously, but you're just reusing
this function because it's conveniently called at suspend/resume.
> {
> + struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> + struct pci_dev *pdev = to_pci_dev(hcd->self.controller);
> u32 val;
> void __iomem *reg;
>
> + if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
> + pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI) {
I would prefer you didn't do this here. Instead, we should keep all of
the vendor/device matching in one place, namely xhci_pci_quirks().
You should create a new quirk name for this ssic D3 workaround, and
match it only to PCI_VENDOR_ID_INTEL &&
PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI in xhci_pci_quirks, for now. Also,
this block should be in a separate function from xhci_pme_quirk.
> +
> + reg = (void __iomem *) xhci->cap_regs + PORT2_SSIC_CONFIG_REG2;
> +
> + /* Notify SSIC that SSIC profile programming is not done */
> + val = readl(reg) & ~PROG_DONE;
> + writel(val, reg);
> +
> + /* Mark SSIC port as unused(suspend) or used(resume) */
> + val = readl(reg);
> + if (suspend)
> + val |= SSIC_PORT_UNUSED;
> + else
> + val &= ~SSIC_PORT_UNUSED;
> + writel(val, reg);
> +
> + /* Notify SSIC that SSIC profile programming is done */
> + val = readl(reg) | PROG_DONE;
> + writel(val, reg);
> + readl(reg);
> + }
> +
> reg = (void __iomem *) xhci->cap_regs + 0x80a4;
> val = readl(reg);
> writel(val | BIT(28), reg);
> @@ -301,7 +335,7 @@ static int xhci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
> pdev->no_d3cold = true;
>
> if (xhci->quirks & XHCI_PME_STUCK_QUIRK)
> - xhci_pme_quirk(xhci);
> + xhci_pme_quirk(hcd, true);
>
> return xhci_suspend(xhci, do_wakeup);
> }
> @@ -334,7 +368,7 @@ static int xhci_pci_resume(struct usb_hcd *hcd, bool hibernated)
> usb_enable_intel_xhci_ports(pdev);
>
> if (xhci->quirks & XHCI_PME_STUCK_QUIRK)
> - xhci_pme_quirk(xhci);
> + xhci_pme_quirk(hcd, false);
>
> retval = xhci_resume(xhci, hibernated);
> return retval;
> --
> 1.9.1
>
--
Benson Leung
Software Engineer, Chrom* OS
bleung@xxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
