[2/4] usb: host: xhci: Introduce one new 'usb3_slow_suspend' member for xhci private data

Message ID a1dd24f8779c3f65a07622275abebdf4729ddd56.1468571634.git.baolin.wang@linaro.org
State New
Headers show

Commit Message

Baolin Wang July 15, 2016, 9:13 a.m.
Now some usb controllers (such as dwc3 controller) need 'XHCI_SLOW_SUSPEND'
quirk when suspending the xhci, thus we need to add 'usb3_slow_suspend' member
in xhci platform data to support this.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>

---
 drivers/usb/host/xhci-plat.c     |    3 +++
 include/linux/usb/xhci_pdriver.h |    3 +++
 2 files changed, 6 insertions(+)

-- 
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

Comments

Baolin Wang Aug. 18, 2016, 12:47 p.m. | #1
Hi Felipe,

On 18 August 2016 at 20:25, Felipe Balbi <balbi@kernel.org> wrote:
>

> Hi,

>

> Baolin Wang <baolin.wang@linaro.org> writes:

>> Hi Felipe,

>>

>> On 18 August 2016 at 15:18, Felipe Balbi <balbi@kernel.org> wrote:

>>>

>>> Hi,

>>>

>>> Baolin Wang <baolin.wang@linaro.org> writes:

>>>> Now some usb controllers (such as dwc3 controller) need 'XHCI_SLOW_SUSPEND'

>>>> quirk when suspending the xhci, thus we need to add 'usb3_slow_suspend' member

>>>> in xhci platform data to support this.

>>>>

>>>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>

>>>> ---

>>>>  drivers/usb/host/xhci-plat.c     |    3 +++

>>>>  include/linux/usb/xhci_pdriver.h |    3 +++

>>>>  2 files changed, 6 insertions(+)

>>>>

>>>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c

>>>> index e2e2487..162f17c 100644

>>>> --- a/drivers/usb/host/xhci-plat.c

>>>> +++ b/drivers/usb/host/xhci-plat.c

>>>> @@ -250,6 +250,9 @@ static int xhci_plat_probe(struct platform_device *pdev)

>>>>                       (pdata && pdata->usb3_lpm_capable))

>>>>               xhci->quirks |= XHCI_LPM_SUPPORT;

>>>>

>>>> +     if (pdata && pdata->usb3_slow_suspend)

>>>> +             xhci->quirks |= XHCI_SLOW_SUSPEND;

>>>

>>> I remember having a discussion about this with Paul Z and it turned out

>>> that we really didn't need SLOW_SUSPEND. Can you describe further in

>>> what situation you need this quirk?

>>

>> On my dwc3 platform, xhci suspend will be failed if we have not

>> enabled XHCI_SLOW_SUSPEND quirk.

>

> fail how? What error do you see? Do you have some traces of what's

> happening? Did you try figuring out if this is, perhaps, caused by some

> call ordering which is wrong? Perhaps disabling PHYs too early or

> something like that?


It shows the warning "WARN: xHC CMD_RUN timeout" when running
xhci_suspend(). If I enbale XHCI_SLOW_SUSPEND quirk, then it can work
well. I did not try to figure out other things, due to I think the
dwc3 need XHCI_SLOW_SUSPEND quirk. But I can re-try to figure out if
there are other issues if you still believe that dwc3 does not need
XHCI_SLOW_SUSPEND quirk. Thanks.

-- 
Baolin.wang
Best Regards
--
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
Baolin Wang Aug. 19, 2016, 7:52 a.m. | #2
Hi Felipe,

On 18 August 2016 at 21:42, Felipe Balbi <balbi@kernel.org> wrote:
>

> Hi Baolin,

>

> Baolin Wang <baolin.wang@linaro.org> writes:

>>>>>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c

>>>>>> index e2e2487..162f17c 100644

>>>>>> --- a/drivers/usb/host/xhci-plat.c

>>>>>> +++ b/drivers/usb/host/xhci-plat.c

>>>>>> @@ -250,6 +250,9 @@ static int xhci_plat_probe(struct platform_device *pdev)

>>>>>>                       (pdata && pdata->usb3_lpm_capable))

>>>>>>               xhci->quirks |= XHCI_LPM_SUPPORT;

>>>>>>

>>>>>> +     if (pdata && pdata->usb3_slow_suspend)

>>>>>> +             xhci->quirks |= XHCI_SLOW_SUSPEND;

>>>>>

>>>>> I remember having a discussion about this with Paul Z and it turned out

>>>>> that we really didn't need SLOW_SUSPEND. Can you describe further in

>>>>> what situation you need this quirk?

>>>>

>>>> On my dwc3 platform, xhci suspend will be failed if we have not

>>>> enabled XHCI_SLOW_SUSPEND quirk.

>>>

>>> fail how? What error do you see? Do you have some traces of what's

>>> happening? Did you try figuring out if this is, perhaps, caused by some

>>> call ordering which is wrong? Perhaps disabling PHYs too early or

>>> something like that?

>>

>> It shows the warning "WARN: xHC CMD_RUN timeout" when running

>> xhci_suspend(). If I enbale XHCI_SLOW_SUSPEND quirk, then it can work

>> well. I did not try to figure out other things, due to I think the

>> dwc3 need XHCI_SLOW_SUSPEND quirk. But I can re-try to figure out if

>> there are other issues if you still believe that dwc3 does not need

>> XHCI_SLOW_SUSPEND quirk. Thanks.

>

> When I discussed this with Paul Z, he told me there was no known

> DWC3 SoC that really needed SLOW_SUSPEND, so it's likely to be a bug in

> either xhci or dwc3.

>

> Please try to track this down. I would start looking at ordering with

> PHY poweroff or something like that.


OK. Thanks.

-- 
Baolin.wang
Best Regards
--
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

Patch hide | download patch | download mbox

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index e2e2487..162f17c 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -250,6 +250,9 @@  static int xhci_plat_probe(struct platform_device *pdev)
 			(pdata && pdata->usb3_lpm_capable))
 		xhci->quirks |= XHCI_LPM_SUPPORT;
 
+	if (pdata && pdata->usb3_slow_suspend)
+		xhci->quirks |= XHCI_SLOW_SUSPEND;
+
 	return 0;
 
 
diff --git a/include/linux/usb/xhci_pdriver.h b/include/linux/usb/xhci_pdriver.h
index 376654b..1276de1 100644
--- a/include/linux/usb/xhci_pdriver.h
+++ b/include/linux/usb/xhci_pdriver.h
@@ -18,10 +18,13 @@ 
  *
  * @usb3_lpm_capable:	determines if this xhci platform supports USB3
  *			LPM capability
+ * @usb3_slow_suspend:	determines if it need an extraordinary delay when
+ *			suspending xhci.
  *
  */
 struct usb_xhci_pdata {
 	unsigned	usb3_lpm_capable:1;
+	unsigned	usb3_slow_suspend:1;
 };
 
 #endif /* __USB_CORE_XHCI_PDRIVER_H */