[PATCH] usb: host: xhci-plat: fix suspend/resume on xhci-rcar

Discussion:

Yoshihiro Shimoda

2014-10-09 04:45:47 UTC

This patch fixes an issue that suspend/resume cannot work correctly
on xhci-rcar because the xhci driver output the following log:

xhci-hcd ee000000.usb: WARN: xHC CMD_RUN timeout

So, this patch adds to set the XHCI_SLOW_SUSPEND quirk if xhci-rcar.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh-***@public.gmane.org>
---
This patch is based on the Greg's usb.git / usb-next branch.
(commit id : 4ed9a3d455558406cad83d38764ee659de25851c)

drivers/usb/host/xhci-plat.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 3d78b0c..a266883 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -27,12 +27,18 @@ static struct hc_driver __read_mostly xhci_plat_hc_driver;

static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
{
+ struct device_node *of_node = dev->of_node;
/*
* As of now platform drivers don't provide MSI support so we ensure
* here that the generic code does not try to make a pci_dev from our
* dev struct in order to setup MSI
*/
xhci->quirks |= XHCI_PLAT;
+
+ /* QUIRK: R-Car xHCI must be suspended extra slowly */
+ if (of_device_is_compatible(of_node, "renesas,xhci-r8a7790") ||
+ of_device_is_compatible(of_node, "renesas,xhci-r8a7791"))
+ xhci->quirks |= XHCI_SLOW_SUSPEND;
}

/* called during probe() after chip reset completes */

--
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Felipe Balbi

2014-10-09 14:14:26 UTC

Permalink

Post by Yoshihiro Shimoda
This patch fixes an issue that suspend/resume cannot work correctly
xhci-hcd ee000000.usb: WARN: xHC CMD_RUN timeout
So, this patch adds to set the XHCI_SLOW_SUSPEND quirk if xhci-rcar.

do you have erratum number confirming this is needed for your platform ?
We really don't want to enable quirks just because they help, we need to
be sure HW is quirky. For example, we triggered the same thing on DRA7xx
(which uses dwc3) but after digging a little, it seems like we're having
memory access latency issues, not a problem with XHCI.

Post by Yoshihiro Shimoda
---
This patch is based on the Greg's usb.git / usb-next branch.
(commit id : 4ed9a3d455558406cad83d38764ee659de25851c)
drivers/usb/host/xhci-plat.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 3d78b0c..a266883 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -27,12 +27,18 @@ static struct hc_driver __read_mostly xhci_plat_hc_driver;
static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
{
+ struct device_node *of_node = dev->of_node;

add also a blank line here.

Post by Yoshihiro Shimoda
/*
* As of now platform drivers don't provide MSI support so we ensure
* here that the generic code does not try to make a pci_dev from our
* dev struct in order to setup MSI
*/
xhci->quirks |= XHCI_PLAT;
+
+ /* QUIRK: R-Car xHCI must be suspended extra slowly */

why ? Who says that ? Where's the errata document ?

Post by Yoshihiro Shimoda
+ if (of_device_is_compatible(of_node, "renesas,xhci-r8a7790") ||
+ of_device_is_compatible(of_node, "renesas,xhci-r8a7791"))
+ xhci->quirks |= XHCI_SLOW_SUSPEND;
}
/* called during probe() after chip reset completes */
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
balbi

Yoshihiro Shimoda

2014-10-10 00:29:18 UTC

Permalink

Hi,

Post by Felipe Balbi

do you have erratum number confirming this is needed for your platform ?

At the moment, I don't have such number... I just tested it on my platform.
And, the handshake in xhci_suspend needs about 80 ms...
Anyway, I will ask platform team about this behavior.

Post by Felipe Balbi
We really don't want to enable quirks just because they help, we need to
be sure HW is quirky. For example, we triggered the same thing on DRA7xx
(which uses dwc3) but after digging a little, it seems like we're having
memory access latency issues, not a problem with XHCI.

Thank for the information.
I got it. So, I will dig about this issue on my platform.

Post by Felipe Balbi

add also a blank line here.

Thank you for the point. I will fix it.

Post by Felipe Balbi

why ? Who says that ? Where's the errata document ?

As I said above, Just a result on my environment.
So, I will ask platform team.

Best regards,
Yoshihiro Shimoda

Post by Felipe Balbi

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Felipe Balbi

2014-10-14 14:30:07 UTC

Permalink

Post by Yoshihiro Shimoda
Hi,

Post by Felipe Balbi

do you have erratum number confirming this is needed for your platform ?

At the moment, I don't have such number... I just tested it on my platform.
And, the handshake in xhci_suspend needs about 80 ms...
Anyway, I will ask platform team about this behavior.

alright, let's wait for your update here, then.

--
balbi

Geert Uytterhoeven

2014-10-10 06:48:46 UTC

Permalink

This post might be inappropriate. Click to display it.