* Re: [PATCH] usb: host: xhci-plat: fix suspend/resume on xhci-rcar
2014-10-09 4:45 [PATCH] usb: host: xhci-plat: fix suspend/resume on xhci-rcar Yoshihiro Shimoda
@ 2014-10-09 14:14 ` Felipe Balbi
2014-10-10 0:29 ` Yoshihiro Shimoda
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Felipe Balbi @ 2014-10-09 14:14 UTC (permalink / raw)
To: linux-sh
[-- Attachment #1: Type: text/plain, Size: 2279 bytes --]
On Thu, Oct 09, 2014 at 01:45:47PM +0900, Yoshihiro Shimoda wrote:
> 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.
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.
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
> 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.
> /*
> * 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 ?
> + 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] usb: host: xhci-plat: fix suspend/resume on xhci-rcar
2014-10-09 4:45 [PATCH] usb: host: xhci-plat: fix suspend/resume on xhci-rcar Yoshihiro Shimoda
2014-10-09 14:14 ` Felipe Balbi
@ 2014-10-10 0:29 ` Yoshihiro Shimoda
2014-10-10 6:48 ` Geert Uytterhoeven
2014-10-14 14:30 ` Felipe Balbi
3 siblings, 0 replies; 5+ messages in thread
From: Yoshihiro Shimoda @ 2014-10-10 0:29 UTC (permalink / raw)
To: linux-sh
Hi,
(2014/10/09 23:14), Felipe Balbi wrote:
> On Thu, Oct 09, 2014 at 01:45:47PM +0900, Yoshihiro Shimoda wrote:
>> 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.
>
> 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.
> 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.
>> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>> ---
>> 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.
Thank you for the point. I will fix it.
>> /*
>> * 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 ?
As I said above, Just a result on my environment.
So, I will ask platform team.
Best regards,
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
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] usb: host: xhci-plat: fix suspend/resume on xhci-rcar
2014-10-09 4:45 [PATCH] usb: host: xhci-plat: fix suspend/resume on xhci-rcar Yoshihiro Shimoda
2014-10-09 14:14 ` Felipe Balbi
2014-10-10 0:29 ` Yoshihiro Shimoda
@ 2014-10-10 6:48 ` Geert Uytterhoeven
2014-10-14 14:30 ` Felipe Balbi
3 siblings, 0 replies; 5+ messages in thread
From: Geert Uytterhoeven @ 2014-10-10 6:48 UTC (permalink / raw)
To: linux-sh
On Thu, Oct 9, 2014 at 4:14 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Thu, Oct 09, 2014 at 01:45:47PM +0900, Yoshihiro Shimoda wrote:
>> 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.
>
> 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.
FWIW, xhci_handshake() runs a tight loop around udelay(1) for up to 16 ms
(160 ms when the quirk is enabled --- not counting loop overhead),
which looks like an area for improvement.
As the individual udelay() calls wait for only 1 us, the __bad_udelay()
sanity check "Use only for very small delays ( < 2 msec)" doesn't kick in.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] usb: host: xhci-plat: fix suspend/resume on xhci-rcar
2014-10-09 4:45 [PATCH] usb: host: xhci-plat: fix suspend/resume on xhci-rcar Yoshihiro Shimoda
` (2 preceding siblings ...)
2014-10-10 6:48 ` Geert Uytterhoeven
@ 2014-10-14 14:30 ` Felipe Balbi
3 siblings, 0 replies; 5+ messages in thread
From: Felipe Balbi @ 2014-10-14 14:30 UTC (permalink / raw)
To: linux-sh
[-- Attachment #1: Type: text/plain, Size: 835 bytes --]
On Fri, Oct 10, 2014 at 09:29:18AM +0900, Yoshihiro Shimoda wrote:
> Hi,
>
> (2014/10/09 23:14), Felipe Balbi wrote:
> > On Thu, Oct 09, 2014 at 01:45:47PM +0900, Yoshihiro Shimoda wrote:
> >> 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.
> >
> > 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
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread