Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1464957 > unrolled thread

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

Started byFelipe Balbi <balbi@kernel.org>
First post2016-08-18 09:20 +0200
Last post2016-08-19 10:10 +0200
Articles 6 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH 2/4] usb: host: xhci: Introduce one new 'usb3_slow_suspend' member for xhci private data Felipe Balbi <balbi@kernel.org> - 2016-08-18 09:20 +0200
    Re: [PATCH 2/4] usb: host: xhci: Introduce one new  'usb3_slow_suspend' member for xhci private data Baolin Wang <baolin.wang@linaro.org> - 2016-08-18 11:00 +0200
      Re: [PATCH 2/4] usb: host: xhci: Introduce one new 'usb3_slow_suspend' member for xhci private data Felipe Balbi <balbi@kernel.org> - 2016-08-18 14:30 +0200
        Re: [PATCH 2/4] usb: host: xhci: Introduce one new  'usb3_slow_suspend' member for xhci private data Baolin Wang <baolin.wang@linaro.org> - 2016-08-18 14:50 +0200
          Re: [PATCH 2/4] usb: host: xhci: Introduce one new 'usb3_slow_suspend' member for xhci private data Felipe Balbi <balbi@kernel.org> - 2016-08-18 15:50 +0200
            Re: [PATCH 2/4] usb: host: xhci: Introduce one new  'usb3_slow_suspend' member for xhci private data Baolin Wang <baolin.wang@linaro.org> - 2016-08-19 10:10 +0200

#1464957 — Re: [PATCH 2/4] usb: host: xhci: Introduce one new 'usb3_slow_suspend' member for xhci private data

FromFelipe Balbi <balbi@kernel.org>
Date2016-08-18 09:20 +0200
SubjectRe: [PATCH 2/4] usb: host: xhci: Introduce one new 'usb3_slow_suspend' member for xhci private data
Message-ID<s7pMl-4SU-11@gated-at.bofh.it>

[Multipart message — attachments visible in raw view] — view raw

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?

-- 
balbi

[toc] | [next] | [standalone]


#1465004 — Re: [PATCH 2/4] usb: host: xhci: Introduce one new 'usb3_slow_suspend' member for xhci private data

FromBaolin Wang <baolin.wang@linaro.org>
Date2016-08-18 11:00 +0200
SubjectRe: [PATCH 2/4] usb: host: xhci: Introduce one new 'usb3_slow_suspend' member for xhci private data
Message-ID<s7rl7-5No-1@gated-at.bofh.it>
In reply to#1464957
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.


-- 
Baolin.wang
Best Regards

[toc] | [prev] | [next] | [standalone]


#1465220

FromFelipe Balbi <balbi@kernel.org>
Date2016-08-18 14:30 +0200
Message-ID<s7uCm-8aC-17@gated-at.bofh.it>
In reply to#1465004

[Multipart message — attachments visible in raw view] — view raw

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?

-- 
balbi

[toc] | [prev] | [next] | [standalone]


#1465232 — Re: [PATCH 2/4] usb: host: xhci: Introduce one new 'usb3_slow_suspend' member for xhci private data

FromBaolin Wang <baolin.wang@linaro.org>
Date2016-08-18 14:50 +0200
SubjectRe: [PATCH 2/4] usb: host: xhci: Introduce one new 'usb3_slow_suspend' member for xhci private data
Message-ID<s7uVI-8hT-21@gated-at.bofh.it>
In reply to#1465220
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

[toc] | [prev] | [next] | [standalone]


#1465355

FromFelipe Balbi <balbi@kernel.org>
Date2016-08-18 15:50 +0200
Message-ID<s7vRL-sf-1@gated-at.bofh.it>
In reply to#1465232

[Multipart message — attachments visible in raw view] — view raw

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.

-- 
balbi

[toc] | [prev] | [next] | [standalone]


#1466210 — Re: [PATCH 2/4] usb: host: xhci: Introduce one new 'usb3_slow_suspend' member for xhci private data

FromBaolin Wang <baolin.wang@linaro.org>
Date2016-08-19 10:10 +0200
SubjectRe: [PATCH 2/4] usb: host: xhci: Introduce one new 'usb3_slow_suspend' member for xhci private data
Message-ID<s7N2i-360-3@gated-at.bofh.it>
In reply to#1465355
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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web