* [PATCH] usb: dwc3: core: Disable USB2.0 phy suspend when dwc3 acts as host role
@ 2016-11-15 9:41 Baolin Wang
2016-11-15 10:49 ` Felipe Balbi
0 siblings, 1 reply; 10+ messages in thread
From: Baolin Wang @ 2016-11-15 9:41 UTC (permalink / raw)
To: balbi; +Cc: gregkh, broonie, linux-usb, linux-kernel, baolin.wang
When dwc3 controller acts as host role with attaching slow speed device
(like mouse or keypad). Then if we plugged out the slow speed device,
it will timeout to run the deconfiguration endpoint command to drop the
endpoint's resources. Some xHCI command timeout log as below when
disconnecting one slow device:
[ 99.807739] c0 xhci-hcd.0.auto: Port Status Change Event for port 1
[ 99.814699] c0 xhci-hcd.0.auto: resume root hub
[ 99.819992] c0 xhci-hcd.0.auto: handle_port_status: starting port
polling.
[ 99.827808] c0 xhci-hcd.0.auto: get port status, actual port 0 status
= 0x202a0
[ 99.835903] c0 xhci-hcd.0.auto: Get port status returned 0x10100
[ 99.850052] c0 xhci-hcd.0.auto: clear port connect change, actual
port 0 status = 0x2a0
[ 99.859313] c0 xhci-hcd.0.auto: Cancel URB ffffffc01ed6cd00, dev 1,
ep 0x81, starting at offset 0xc406d210
[ 99.869645] c0 xhci-hcd.0.auto: // Ding dong!
[ 99.874776] c0 xhci-hcd.0.auto: Stopped on Transfer TRB
[ 99.880713] c0 xhci-hcd.0.auto: Removing canceled TD starting at
0xc406d210 (dma).
[ 99.889012] c0 xhci-hcd.0.auto: Finding endpoint context
[ 99.895069] c0 xhci-hcd.0.auto: Cycle state = 0x1
[ 99.900519] c0 xhci-hcd.0.auto: New dequeue segment =
ffffffc1112f0880 (virtual)
[ 99.908655] c0 xhci-hcd.0.auto: New dequeue pointer = 0xc406d220 (DMA)
[ 99.915927] c0 xhci-hcd.0.auto: Set TR Deq Ptr cmd, new deq seg =
ffffffc1112f0880 (0xc406d000 dma),
new deq ptr = ffffff8002175220
(0xc406d220 dma), new cycle = 1
[ 99.931242] c0 xhci-hcd.0.auto: // Ding dong!
[ 99.936360] c0 xhci-hcd.0.auto: Successful Set TR Deq Ptr cmd,
deq = @c406d220
[ 99.944458] c0 xhci-hcd.0.auto: xhci_hub_status_data: stopping port
polling.
[ 100.047619] c0 xhci-hcd.0.auto: xhci_drop_endpoint called for udev
ffffffc01ae08800
[ 100.057002] c0 xhci-hcd.0.auto: drop ep 0x81, slot id 1, new drop
flags = 0x8, new add flags = 0x0
[ 100.067878] c0 xhci-hcd.0.auto: xhci_check_bandwidth called for udev
ffffffc01ae08800
[ 100.076868] c0 xhci-hcd.0.auto: New Input Control Context:
......
[ 100.427252] c0 xhci-hcd.0.auto: // Ding dong!
[ 105.430728] c0 xhci-hcd.0.auto: Command timeout
[ 105.436029] c0 xhci-hcd.0.auto: Abort command ring
[ 113.558223] c0 xhci-hcd.0.auto: Command completion event does not match
command
[ 113.569778] c0 xhci-hcd.0.auto: Timeout while waiting for configure
endpoint command
The reason is it will suspend USB phy to disable phy clock when
disconnecting the slow USB decice, that will hang on the xHCI commands
executing which depends on the phy clock.
Thus we should disable USB2.0 phy suspend feature when dwc3 acts as host
role.
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/usb/dwc3/core.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 9a4a5e4..0b646cf 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -565,6 +565,20 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
if (dwc->revision > DWC3_REVISION_194A)
reg |= DWC3_GUSB2PHYCFG_SUSPHY;
+ /*
+ * When dwc3 controller acts as host role with attaching one slow speed
+ * device (like mouse or keypad). Then if we plugged out the slow speed
+ * device, it will timeout to run the deconfiguration endpoint command.
+ * The reason is it will suspend USB phy to disable phy clock when
+ * disconnecting slow speed decice, which will affect the xHCI commands
+ * executing.
+ *
+ * Thus we should disable USB 2.0 phy suspend feature when dwc3 acts as
+ * host role.
+ */
+ if (dwc->dr_mode == USB_DR_MODE_HOST || dwc->dr_mode == USB_DR_MODE_OTG)
+ reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
+
if (dwc->dis_u2_susphy_quirk)
reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] usb: dwc3: core: Disable USB2.0 phy suspend when dwc3 acts as host role
2016-11-15 9:41 [PATCH] usb: dwc3: core: Disable USB2.0 phy suspend when dwc3 acts as host role Baolin Wang
@ 2016-11-15 10:49 ` Felipe Balbi
2016-11-16 2:55 ` Baolin Wang
0 siblings, 1 reply; 10+ messages in thread
From: Felipe Balbi @ 2016-11-15 10:49 UTC (permalink / raw)
To: Baolin Wang; +Cc: gregkh, broonie, linux-usb, linux-kernel, baolin.wang
[-- Attachment #1: Type: text/plain, Size: 4294 bytes --]
Hi,
Baolin Wang <baolin.wang@linaro.org> writes:
> When dwc3 controller acts as host role with attaching slow speed device
> (like mouse or keypad). Then if we plugged out the slow speed device,
> it will timeout to run the deconfiguration endpoint command to drop the
> endpoint's resources. Some xHCI command timeout log as below when
> disconnecting one slow device:
>
> [ 99.807739] c0 xhci-hcd.0.auto: Port Status Change Event for port 1
> [ 99.814699] c0 xhci-hcd.0.auto: resume root hub
> [ 99.819992] c0 xhci-hcd.0.auto: handle_port_status: starting port
> polling.
> [ 99.827808] c0 xhci-hcd.0.auto: get port status, actual port 0 status
> = 0x202a0
> [ 99.835903] c0 xhci-hcd.0.auto: Get port status returned 0x10100
> [ 99.850052] c0 xhci-hcd.0.auto: clear port connect change, actual
> port 0 status = 0x2a0
> [ 99.859313] c0 xhci-hcd.0.auto: Cancel URB ffffffc01ed6cd00, dev 1,
> ep 0x81, starting at offset 0xc406d210
> [ 99.869645] c0 xhci-hcd.0.auto: // Ding dong!
> [ 99.874776] c0 xhci-hcd.0.auto: Stopped on Transfer TRB
> [ 99.880713] c0 xhci-hcd.0.auto: Removing canceled TD starting at
> 0xc406d210 (dma).
> [ 99.889012] c0 xhci-hcd.0.auto: Finding endpoint context
> [ 99.895069] c0 xhci-hcd.0.auto: Cycle state = 0x1
> [ 99.900519] c0 xhci-hcd.0.auto: New dequeue segment =
> ffffffc1112f0880 (virtual)
> [ 99.908655] c0 xhci-hcd.0.auto: New dequeue pointer = 0xc406d220 (DMA)
> [ 99.915927] c0 xhci-hcd.0.auto: Set TR Deq Ptr cmd, new deq seg =
> ffffffc1112f0880 (0xc406d000 dma),
> new deq ptr = ffffff8002175220
> (0xc406d220 dma), new cycle = 1
> [ 99.931242] c0 xhci-hcd.0.auto: // Ding dong!
> [ 99.936360] c0 xhci-hcd.0.auto: Successful Set TR Deq Ptr cmd,
> deq = @c406d220
> [ 99.944458] c0 xhci-hcd.0.auto: xhci_hub_status_data: stopping port
> polling.
> [ 100.047619] c0 xhci-hcd.0.auto: xhci_drop_endpoint called for udev
> ffffffc01ae08800
> [ 100.057002] c0 xhci-hcd.0.auto: drop ep 0x81, slot id 1, new drop
> flags = 0x8, new add flags = 0x0
> [ 100.067878] c0 xhci-hcd.0.auto: xhci_check_bandwidth called for udev
> ffffffc01ae08800
> [ 100.076868] c0 xhci-hcd.0.auto: New Input Control Context:
>
> ......
>
> [ 100.427252] c0 xhci-hcd.0.auto: // Ding dong!
> [ 105.430728] c0 xhci-hcd.0.auto: Command timeout
> [ 105.436029] c0 xhci-hcd.0.auto: Abort command ring
> [ 113.558223] c0 xhci-hcd.0.auto: Command completion event does not match
> command
> [ 113.569778] c0 xhci-hcd.0.auto: Timeout while waiting for configure
> endpoint command
>
> The reason is it will suspend USB phy to disable phy clock when
> disconnecting the slow USB decice, that will hang on the xHCI commands
> executing which depends on the phy clock.
>
> Thus we should disable USB2.0 phy suspend feature when dwc3 acts as host
> role.
>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
> drivers/usb/dwc3/core.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 9a4a5e4..0b646cf 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -565,6 +565,20 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
> if (dwc->revision > DWC3_REVISION_194A)
> reg |= DWC3_GUSB2PHYCFG_SUSPHY;
>
> + /*
> + * When dwc3 controller acts as host role with attaching one slow speed
> + * device (like mouse or keypad). Then if we plugged out the slow speed
> + * device, it will timeout to run the deconfiguration endpoint command.
> + * The reason is it will suspend USB phy to disable phy clock when
> + * disconnecting slow speed decice, which will affect the xHCI commands
> + * executing.
> + *
> + * Thus we should disable USB 2.0 phy suspend feature when dwc3 acts as
> + * host role.
> + */
> + if (dwc->dr_mode == USB_DR_MODE_HOST || dwc->dr_mode == USB_DR_MODE_OTG)
> + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
which version of the core you're using? Recent version (since 1.94A,
IIRC) can manage core suspend automatically. Also, this patch of yours
will cause a power consumption regression.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] usb: dwc3: core: Disable USB2.0 phy suspend when dwc3 acts as host role
2016-11-15 10:49 ` Felipe Balbi
@ 2016-11-16 2:55 ` Baolin Wang
2017-01-12 7:49 ` Felipe Balbi
0 siblings, 1 reply; 10+ messages in thread
From: Baolin Wang @ 2016-11-16 2:55 UTC (permalink / raw)
To: Felipe Balbi; +Cc: Greg KH, Mark Brown, USB, LKML
Hi,
On 15 November 2016 at 18:49, Felipe Balbi <balbi@kernel.org> wrote:
>
> Hi,
>
> Baolin Wang <baolin.wang@linaro.org> writes:
>> When dwc3 controller acts as host role with attaching slow speed device
>> (like mouse or keypad). Then if we plugged out the slow speed device,
>> it will timeout to run the deconfiguration endpoint command to drop the
>> endpoint's resources. Some xHCI command timeout log as below when
>> disconnecting one slow device:
>>
>> [ 99.807739] c0 xhci-hcd.0.auto: Port Status Change Event for port 1
>> [ 99.814699] c0 xhci-hcd.0.auto: resume root hub
>> [ 99.819992] c0 xhci-hcd.0.auto: handle_port_status: starting port
>> polling.
>> [ 99.827808] c0 xhci-hcd.0.auto: get port status, actual port 0 status
>> = 0x202a0
>> [ 99.835903] c0 xhci-hcd.0.auto: Get port status returned 0x10100
>> [ 99.850052] c0 xhci-hcd.0.auto: clear port connect change, actual
>> port 0 status = 0x2a0
>> [ 99.859313] c0 xhci-hcd.0.auto: Cancel URB ffffffc01ed6cd00, dev 1,
>> ep 0x81, starting at offset 0xc406d210
>> [ 99.869645] c0 xhci-hcd.0.auto: // Ding dong!
>> [ 99.874776] c0 xhci-hcd.0.auto: Stopped on Transfer TRB
>> [ 99.880713] c0 xhci-hcd.0.auto: Removing canceled TD starting at
>> 0xc406d210 (dma).
>> [ 99.889012] c0 xhci-hcd.0.auto: Finding endpoint context
>> [ 99.895069] c0 xhci-hcd.0.auto: Cycle state = 0x1
>> [ 99.900519] c0 xhci-hcd.0.auto: New dequeue segment =
>> ffffffc1112f0880 (virtual)
>> [ 99.908655] c0 xhci-hcd.0.auto: New dequeue pointer = 0xc406d220 (DMA)
>> [ 99.915927] c0 xhci-hcd.0.auto: Set TR Deq Ptr cmd, new deq seg =
>> ffffffc1112f0880 (0xc406d000 dma),
>> new deq ptr = ffffff8002175220
>> (0xc406d220 dma), new cycle = 1
>> [ 99.931242] c0 xhci-hcd.0.auto: // Ding dong!
>> [ 99.936360] c0 xhci-hcd.0.auto: Successful Set TR Deq Ptr cmd,
>> deq = @c406d220
>> [ 99.944458] c0 xhci-hcd.0.auto: xhci_hub_status_data: stopping port
>> polling.
>> [ 100.047619] c0 xhci-hcd.0.auto: xhci_drop_endpoint called for udev
>> ffffffc01ae08800
>> [ 100.057002] c0 xhci-hcd.0.auto: drop ep 0x81, slot id 1, new drop
>> flags = 0x8, new add flags = 0x0
>> [ 100.067878] c0 xhci-hcd.0.auto: xhci_check_bandwidth called for udev
>> ffffffc01ae08800
>> [ 100.076868] c0 xhci-hcd.0.auto: New Input Control Context:
>>
>> ......
>>
>> [ 100.427252] c0 xhci-hcd.0.auto: // Ding dong!
>> [ 105.430728] c0 xhci-hcd.0.auto: Command timeout
>> [ 105.436029] c0 xhci-hcd.0.auto: Abort command ring
>> [ 113.558223] c0 xhci-hcd.0.auto: Command completion event does not match
>> command
>> [ 113.569778] c0 xhci-hcd.0.auto: Timeout while waiting for configure
>> endpoint command
>>
>> The reason is it will suspend USB phy to disable phy clock when
>> disconnecting the slow USB decice, that will hang on the xHCI commands
>> executing which depends on the phy clock.
>>
>> Thus we should disable USB2.0 phy suspend feature when dwc3 acts as host
>> role.
>>
>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>> ---
>> drivers/usb/dwc3/core.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 9a4a5e4..0b646cf 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -565,6 +565,20 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>> if (dwc->revision > DWC3_REVISION_194A)
>> reg |= DWC3_GUSB2PHYCFG_SUSPHY;
>>
>> + /*
>> + * When dwc3 controller acts as host role with attaching one slow speed
>> + * device (like mouse or keypad). Then if we plugged out the slow speed
>> + * device, it will timeout to run the deconfiguration endpoint command.
>> + * The reason is it will suspend USB phy to disable phy clock when
>> + * disconnecting slow speed decice, which will affect the xHCI commands
>> + * executing.
>> + *
>> + * Thus we should disable USB 2.0 phy suspend feature when dwc3 acts as
>> + * host role.
>> + */
>> + if (dwc->dr_mode == USB_DR_MODE_HOST || dwc->dr_mode == USB_DR_MODE_OTG)
>> + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>
> which version of the core you're using? Recent version (since 1.94A,
My version is 2.80a.
> IIRC) can manage core suspend automatically. Also, this patch of yours
> will cause a power consumption regression.
Yes, it can manage core suspend automatically, that is the problem.
When plugging out one mouse or keypad device, the phy will suspend
automatically to disable the phy clock. But now the disconnecting
process is not finished, and some xHCI commands (like deconfiguration
endpoint command to drop endpoint resources) need depend on the phy
clock, which will hang on the system to timeout the command or abort
command ring to halt the xHCI.
I agree with you it will cause a power consumption regression, but it
will cause serious problem if not. Do you have some suggestion?
Thanks.
--
Baolin.wang
Best Regards
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] usb: dwc3: core: Disable USB2.0 phy suspend when dwc3 acts as host role
2016-11-16 2:55 ` Baolin Wang
@ 2017-01-12 7:49 ` Felipe Balbi
2017-01-12 21:47 ` John Youn
0 siblings, 1 reply; 10+ messages in thread
From: Felipe Balbi @ 2017-01-12 7:49 UTC (permalink / raw)
To: Baolin Wang, John Youn; +Cc: Greg KH, Mark Brown, USB, LKML
[-- Attachment #1: Type: text/plain, Size: 5907 bytes --]
Hi,
Baolin Wang <baolin.wang@linaro.org> writes:
>> Baolin Wang <baolin.wang@linaro.org> writes:
>>> When dwc3 controller acts as host role with attaching slow speed device
>>> (like mouse or keypad). Then if we plugged out the slow speed device,
>>> it will timeout to run the deconfiguration endpoint command to drop the
>>> endpoint's resources. Some xHCI command timeout log as below when
>>> disconnecting one slow device:
>>>
>>> [ 99.807739] c0 xhci-hcd.0.auto: Port Status Change Event for port 1
>>> [ 99.814699] c0 xhci-hcd.0.auto: resume root hub
>>> [ 99.819992] c0 xhci-hcd.0.auto: handle_port_status: starting port
>>> polling.
>>> [ 99.827808] c0 xhci-hcd.0.auto: get port status, actual port 0 status
>>> = 0x202a0
>>> [ 99.835903] c0 xhci-hcd.0.auto: Get port status returned 0x10100
>>> [ 99.850052] c0 xhci-hcd.0.auto: clear port connect change, actual
>>> port 0 status = 0x2a0
>>> [ 99.859313] c0 xhci-hcd.0.auto: Cancel URB ffffffc01ed6cd00, dev 1,
>>> ep 0x81, starting at offset 0xc406d210
>>> [ 99.869645] c0 xhci-hcd.0.auto: // Ding dong!
>>> [ 99.874776] c0 xhci-hcd.0.auto: Stopped on Transfer TRB
>>> [ 99.880713] c0 xhci-hcd.0.auto: Removing canceled TD starting at
>>> 0xc406d210 (dma).
>>> [ 99.889012] c0 xhci-hcd.0.auto: Finding endpoint context
>>> [ 99.895069] c0 xhci-hcd.0.auto: Cycle state = 0x1
>>> [ 99.900519] c0 xhci-hcd.0.auto: New dequeue segment =
>>> ffffffc1112f0880 (virtual)
>>> [ 99.908655] c0 xhci-hcd.0.auto: New dequeue pointer = 0xc406d220 (DMA)
>>> [ 99.915927] c0 xhci-hcd.0.auto: Set TR Deq Ptr cmd, new deq seg =
>>> ffffffc1112f0880 (0xc406d000 dma),
>>> new deq ptr = ffffff8002175220
>>> (0xc406d220 dma), new cycle = 1
>>> [ 99.931242] c0 xhci-hcd.0.auto: // Ding dong!
>>> [ 99.936360] c0 xhci-hcd.0.auto: Successful Set TR Deq Ptr cmd,
>>> deq = @c406d220
>>> [ 99.944458] c0 xhci-hcd.0.auto: xhci_hub_status_data: stopping port
>>> polling.
>>> [ 100.047619] c0 xhci-hcd.0.auto: xhci_drop_endpoint called for udev
>>> ffffffc01ae08800
>>> [ 100.057002] c0 xhci-hcd.0.auto: drop ep 0x81, slot id 1, new drop
>>> flags = 0x8, new add flags = 0x0
>>> [ 100.067878] c0 xhci-hcd.0.auto: xhci_check_bandwidth called for udev
>>> ffffffc01ae08800
>>> [ 100.076868] c0 xhci-hcd.0.auto: New Input Control Context:
>>>
>>> ......
>>>
>>> [ 100.427252] c0 xhci-hcd.0.auto: // Ding dong!
>>> [ 105.430728] c0 xhci-hcd.0.auto: Command timeout
>>> [ 105.436029] c0 xhci-hcd.0.auto: Abort command ring
>>> [ 113.558223] c0 xhci-hcd.0.auto: Command completion event does not match
>>> command
>>> [ 113.569778] c0 xhci-hcd.0.auto: Timeout while waiting for configure
>>> endpoint command
>>>
>>> The reason is it will suspend USB phy to disable phy clock when
>>> disconnecting the slow USB decice, that will hang on the xHCI commands
>>> executing which depends on the phy clock.
>>>
>>> Thus we should disable USB2.0 phy suspend feature when dwc3 acts as host
>>> role.
>>>
>>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>>> ---
>>> drivers/usb/dwc3/core.c | 14 ++++++++++++++
>>> 1 file changed, 14 insertions(+)
>>>
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index 9a4a5e4..0b646cf 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -565,6 +565,20 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>>> if (dwc->revision > DWC3_REVISION_194A)
>>> reg |= DWC3_GUSB2PHYCFG_SUSPHY;
>>>
>>> + /*
>>> + * When dwc3 controller acts as host role with attaching one slow speed
>>> + * device (like mouse or keypad). Then if we plugged out the slow speed
>>> + * device, it will timeout to run the deconfiguration endpoint command.
>>> + * The reason is it will suspend USB phy to disable phy clock when
>>> + * disconnecting slow speed decice, which will affect the xHCI commands
>>> + * executing.
>>> + *
>>> + * Thus we should disable USB 2.0 phy suspend feature when dwc3 acts as
>>> + * host role.
>>> + */
>>> + if (dwc->dr_mode == USB_DR_MODE_HOST || dwc->dr_mode == USB_DR_MODE_OTG)
>>> + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>
>> which version of the core you're using? Recent version (since 1.94A,
>
> My version is 2.80a.
>
>> IIRC) can manage core suspend automatically. Also, this patch of yours
>> will cause a power consumption regression.
>
> Yes, it can manage core suspend automatically, that is the problem.
> When plugging out one mouse or keypad device, the phy will suspend
> automatically to disable the phy clock. But now the disconnecting
> process is not finished, and some xHCI commands (like deconfiguration
> endpoint command to drop endpoint resources) need depend on the phy
> clock, which will hang on the system to timeout the command or abort
> command ring to halt the xHCI.
>
> I agree with you it will cause a power consumption regression, but it
> will cause serious problem if not. Do you have some suggestion?
sorry for the long delay. This was lost in my inbox.
I'm not sure this patch is the best solution. There's no mention in
Databook that we should avoid PHY suspend when acting as host. Adding
John here to see if John has any idea of how to fix this.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] usb: dwc3: core: Disable USB2.0 phy suspend when dwc3 acts as host role
2017-01-12 7:49 ` Felipe Balbi
@ 2017-01-12 21:47 ` John Youn
2017-01-16 10:36 ` Felipe Balbi
0 siblings, 1 reply; 10+ messages in thread
From: John Youn @ 2017-01-12 21:47 UTC (permalink / raw)
To: Felipe Balbi, Baolin Wang, John Youn; +Cc: Greg KH, Mark Brown, USB, LKML
On 1/11/2017 11:51 PM, Felipe Balbi wrote:
>
> Hi,
>
> Baolin Wang <baolin.wang@linaro.org> writes:
>>> Baolin Wang <baolin.wang@linaro.org> writes:
>>>> When dwc3 controller acts as host role with attaching slow speed device
>>>> (like mouse or keypad). Then if we plugged out the slow speed device,
>>>> it will timeout to run the deconfiguration endpoint command to drop the
>>>> endpoint's resources. Some xHCI command timeout log as below when
>>>> disconnecting one slow device:
>>>>
>>>> [ 99.807739] c0 xhci-hcd.0.auto: Port Status Change Event for port 1
>>>> [ 99.814699] c0 xhci-hcd.0.auto: resume root hub
>>>> [ 99.819992] c0 xhci-hcd.0.auto: handle_port_status: starting port
>>>> polling.
>>>> [ 99.827808] c0 xhci-hcd.0.auto: get port status, actual port 0 status
>>>> = 0x202a0
>>>> [ 99.835903] c0 xhci-hcd.0.auto: Get port status returned 0x10100
>>>> [ 99.850052] c0 xhci-hcd.0.auto: clear port connect change, actual
>>>> port 0 status = 0x2a0
>>>> [ 99.859313] c0 xhci-hcd.0.auto: Cancel URB ffffffc01ed6cd00, dev 1,
>>>> ep 0x81, starting at offset 0xc406d210
>>>> [ 99.869645] c0 xhci-hcd.0.auto: // Ding dong!
>>>> [ 99.874776] c0 xhci-hcd.0.auto: Stopped on Transfer TRB
>>>> [ 99.880713] c0 xhci-hcd.0.auto: Removing canceled TD starting at
>>>> 0xc406d210 (dma).
>>>> [ 99.889012] c0 xhci-hcd.0.auto: Finding endpoint context
>>>> [ 99.895069] c0 xhci-hcd.0.auto: Cycle state = 0x1
>>>> [ 99.900519] c0 xhci-hcd.0.auto: New dequeue segment =
>>>> ffffffc1112f0880 (virtual)
>>>> [ 99.908655] c0 xhci-hcd.0.auto: New dequeue pointer = 0xc406d220 (DMA)
>>>> [ 99.915927] c0 xhci-hcd.0.auto: Set TR Deq Ptr cmd, new deq seg =
>>>> ffffffc1112f0880 (0xc406d000 dma),
>>>> new deq ptr = ffffff8002175220
>>>> (0xc406d220 dma), new cycle = 1
>>>> [ 99.931242] c0 xhci-hcd.0.auto: // Ding dong!
>>>> [ 99.936360] c0 xhci-hcd.0.auto: Successful Set TR Deq Ptr cmd,
>>>> deq = @c406d220
>>>> [ 99.944458] c0 xhci-hcd.0.auto: xhci_hub_status_data: stopping port
>>>> polling.
>>>> [ 100.047619] c0 xhci-hcd.0.auto: xhci_drop_endpoint called for udev
>>>> ffffffc01ae08800
>>>> [ 100.057002] c0 xhci-hcd.0.auto: drop ep 0x81, slot id 1, new drop
>>>> flags = 0x8, new add flags = 0x0
>>>> [ 100.067878] c0 xhci-hcd.0.auto: xhci_check_bandwidth called for udev
>>>> ffffffc01ae08800
>>>> [ 100.076868] c0 xhci-hcd.0.auto: New Input Control Context:
>>>>
>>>> ......
>>>>
>>>> [ 100.427252] c0 xhci-hcd.0.auto: // Ding dong!
>>>> [ 105.430728] c0 xhci-hcd.0.auto: Command timeout
>>>> [ 105.436029] c0 xhci-hcd.0.auto: Abort command ring
>>>> [ 113.558223] c0 xhci-hcd.0.auto: Command completion event does not match
>>>> command
>>>> [ 113.569778] c0 xhci-hcd.0.auto: Timeout while waiting for configure
>>>> endpoint command
>>>>
>>>> The reason is it will suspend USB phy to disable phy clock when
>>>> disconnecting the slow USB decice, that will hang on the xHCI commands
>>>> executing which depends on the phy clock.
>>>>
>>>> Thus we should disable USB2.0 phy suspend feature when dwc3 acts as host
>>>> role.
>>>>
>>>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>>>> ---
>>>> drivers/usb/dwc3/core.c | 14 ++++++++++++++
>>>> 1 file changed, 14 insertions(+)
>>>>
>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>> index 9a4a5e4..0b646cf 100644
>>>> --- a/drivers/usb/dwc3/core.c
>>>> +++ b/drivers/usb/dwc3/core.c
>>>> @@ -565,6 +565,20 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>>>> if (dwc->revision > DWC3_REVISION_194A)
>>>> reg |= DWC3_GUSB2PHYCFG_SUSPHY;
>>>>
>>>> + /*
>>>> + * When dwc3 controller acts as host role with attaching one slow speed
>>>> + * device (like mouse or keypad). Then if we plugged out the slow speed
>>>> + * device, it will timeout to run the deconfiguration endpoint command.
>>>> + * The reason is it will suspend USB phy to disable phy clock when
>>>> + * disconnecting slow speed decice, which will affect the xHCI commands
>>>> + * executing.
>>>> + *
>>>> + * Thus we should disable USB 2.0 phy suspend feature when dwc3 acts as
>>>> + * host role.
>>>> + */
>>>> + if (dwc->dr_mode == USB_DR_MODE_HOST || dwc->dr_mode == USB_DR_MODE_OTG)
>>>> + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>>
>>> which version of the core you're using? Recent version (since 1.94A,
>>
>> My version is 2.80a.
>>
>>> IIRC) can manage core suspend automatically. Also, this patch of yours
>>> will cause a power consumption regression.
>>
>> Yes, it can manage core suspend automatically, that is the problem.
>> When plugging out one mouse or keypad device, the phy will suspend
>> automatically to disable the phy clock. But now the disconnecting
>> process is not finished, and some xHCI commands (like deconfiguration
>> endpoint command to drop endpoint resources) need depend on the phy
>> clock, which will hang on the system to timeout the command or abort
>> command ring to halt the xHCI.
>>
>> I agree with you it will cause a power consumption regression, but it
>> will cause serious problem if not. Do you have some suggestion?
>
> sorry for the long delay. This was lost in my inbox.
>
> I'm not sure this patch is the best solution. There's no mention in
> Databook that we should avoid PHY suspend when acting as host. Adding
> John here to see if John has any idea of how to fix this.
>
I'm not familiar enough with XHCI side of things to say.
I'll ask around to see if anyone has an idea.
Regards,
John
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] usb: dwc3: core: Disable USB2.0 phy suspend when dwc3 acts as host role
2017-01-12 21:47 ` John Youn
@ 2017-01-16 10:36 ` Felipe Balbi
2017-01-19 1:33 ` John Youn
0 siblings, 1 reply; 10+ messages in thread
From: Felipe Balbi @ 2017-01-16 10:36 UTC (permalink / raw)
To: John Youn, Baolin Wang, John Youn; +Cc: Greg KH, Mark Brown, USB, LKML
[-- Attachment #1: Type: text/plain, Size: 6315 bytes --]
Hi,
John Youn <John.Youn@synopsys.com> writes:
>> Baolin Wang <baolin.wang@linaro.org> writes:
>>>> Baolin Wang <baolin.wang@linaro.org> writes:
>>>>> When dwc3 controller acts as host role with attaching slow speed device
>>>>> (like mouse or keypad). Then if we plugged out the slow speed device,
>>>>> it will timeout to run the deconfiguration endpoint command to drop the
>>>>> endpoint's resources. Some xHCI command timeout log as below when
>>>>> disconnecting one slow device:
>>>>>
>>>>> [ 99.807739] c0 xhci-hcd.0.auto: Port Status Change Event for port 1
>>>>> [ 99.814699] c0 xhci-hcd.0.auto: resume root hub
>>>>> [ 99.819992] c0 xhci-hcd.0.auto: handle_port_status: starting port
>>>>> polling.
>>>>> [ 99.827808] c0 xhci-hcd.0.auto: get port status, actual port 0 status
>>>>> = 0x202a0
>>>>> [ 99.835903] c0 xhci-hcd.0.auto: Get port status returned 0x10100
>>>>> [ 99.850052] c0 xhci-hcd.0.auto: clear port connect change, actual
>>>>> port 0 status = 0x2a0
>>>>> [ 99.859313] c0 xhci-hcd.0.auto: Cancel URB ffffffc01ed6cd00, dev 1,
>>>>> ep 0x81, starting at offset 0xc406d210
>>>>> [ 99.869645] c0 xhci-hcd.0.auto: // Ding dong!
>>>>> [ 99.874776] c0 xhci-hcd.0.auto: Stopped on Transfer TRB
>>>>> [ 99.880713] c0 xhci-hcd.0.auto: Removing canceled TD starting at
>>>>> 0xc406d210 (dma).
>>>>> [ 99.889012] c0 xhci-hcd.0.auto: Finding endpoint context
>>>>> [ 99.895069] c0 xhci-hcd.0.auto: Cycle state = 0x1
>>>>> [ 99.900519] c0 xhci-hcd.0.auto: New dequeue segment =
>>>>> ffffffc1112f0880 (virtual)
>>>>> [ 99.908655] c0 xhci-hcd.0.auto: New dequeue pointer = 0xc406d220 (DMA)
>>>>> [ 99.915927] c0 xhci-hcd.0.auto: Set TR Deq Ptr cmd, new deq seg =
>>>>> ffffffc1112f0880 (0xc406d000 dma),
>>>>> new deq ptr = ffffff8002175220
>>>>> (0xc406d220 dma), new cycle = 1
>>>>> [ 99.931242] c0 xhci-hcd.0.auto: // Ding dong!
>>>>> [ 99.936360] c0 xhci-hcd.0.auto: Successful Set TR Deq Ptr cmd,
>>>>> deq = @c406d220
>>>>> [ 99.944458] c0 xhci-hcd.0.auto: xhci_hub_status_data: stopping port
>>>>> polling.
>>>>> [ 100.047619] c0 xhci-hcd.0.auto: xhci_drop_endpoint called for udev
>>>>> ffffffc01ae08800
>>>>> [ 100.057002] c0 xhci-hcd.0.auto: drop ep 0x81, slot id 1, new drop
>>>>> flags = 0x8, new add flags = 0x0
>>>>> [ 100.067878] c0 xhci-hcd.0.auto: xhci_check_bandwidth called for udev
>>>>> ffffffc01ae08800
>>>>> [ 100.076868] c0 xhci-hcd.0.auto: New Input Control Context:
>>>>>
>>>>> ......
>>>>>
>>>>> [ 100.427252] c0 xhci-hcd.0.auto: // Ding dong!
>>>>> [ 105.430728] c0 xhci-hcd.0.auto: Command timeout
>>>>> [ 105.436029] c0 xhci-hcd.0.auto: Abort command ring
>>>>> [ 113.558223] c0 xhci-hcd.0.auto: Command completion event does not match
>>>>> command
>>>>> [ 113.569778] c0 xhci-hcd.0.auto: Timeout while waiting for configure
>>>>> endpoint command
>>>>>
>>>>> The reason is it will suspend USB phy to disable phy clock when
>>>>> disconnecting the slow USB decice, that will hang on the xHCI commands
>>>>> executing which depends on the phy clock.
>>>>>
>>>>> Thus we should disable USB2.0 phy suspend feature when dwc3 acts as host
>>>>> role.
>>>>>
>>>>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>>>>> ---
>>>>> drivers/usb/dwc3/core.c | 14 ++++++++++++++
>>>>> 1 file changed, 14 insertions(+)
>>>>>
>>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>>> index 9a4a5e4..0b646cf 100644
>>>>> --- a/drivers/usb/dwc3/core.c
>>>>> +++ b/drivers/usb/dwc3/core.c
>>>>> @@ -565,6 +565,20 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>>>>> if (dwc->revision > DWC3_REVISION_194A)
>>>>> reg |= DWC3_GUSB2PHYCFG_SUSPHY;
>>>>>
>>>>> + /*
>>>>> + * When dwc3 controller acts as host role with attaching one slow speed
>>>>> + * device (like mouse or keypad). Then if we plugged out the slow speed
>>>>> + * device, it will timeout to run the deconfiguration endpoint command.
>>>>> + * The reason is it will suspend USB phy to disable phy clock when
>>>>> + * disconnecting slow speed decice, which will affect the xHCI commands
>>>>> + * executing.
>>>>> + *
>>>>> + * Thus we should disable USB 2.0 phy suspend feature when dwc3 acts as
>>>>> + * host role.
>>>>> + */
>>>>> + if (dwc->dr_mode == USB_DR_MODE_HOST || dwc->dr_mode == USB_DR_MODE_OTG)
>>>>> + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>>>
>>>> which version of the core you're using? Recent version (since 1.94A,
>>>
>>> My version is 2.80a.
>>>
>>>> IIRC) can manage core suspend automatically. Also, this patch of yours
>>>> will cause a power consumption regression.
>>>
>>> Yes, it can manage core suspend automatically, that is the problem.
>>> When plugging out one mouse or keypad device, the phy will suspend
>>> automatically to disable the phy clock. But now the disconnecting
>>> process is not finished, and some xHCI commands (like deconfiguration
>>> endpoint command to drop endpoint resources) need depend on the phy
>>> clock, which will hang on the system to timeout the command or abort
>>> command ring to halt the xHCI.
>>>
>>> I agree with you it will cause a power consumption regression, but it
>>> will cause serious problem if not. Do you have some suggestion?
>>
>> sorry for the long delay. This was lost in my inbox.
>>
>> I'm not sure this patch is the best solution. There's no mention in
>> Databook that we should avoid PHY suspend when acting as host. Adding
>> John here to see if John has any idea of how to fix this.
>>
>
> I'm not familiar enough with XHCI side of things to say.
>
> I'll ask around to see if anyone has an idea.
Thank you
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] usb: dwc3: core: Disable USB2.0 phy suspend when dwc3 acts as host role
2017-01-16 10:36 ` Felipe Balbi
@ 2017-01-19 1:33 ` John Youn
2017-01-19 3:12 ` Baolin Wang
0 siblings, 1 reply; 10+ messages in thread
From: John Youn @ 2017-01-19 1:33 UTC (permalink / raw)
To: Felipe Balbi, John Youn, Baolin Wang; +Cc: Greg KH, Mark Brown, USB, LKML
On 1/16/2017 2:38 AM, Felipe Balbi wrote:
>
> Hi,
>
> John Youn <John.Youn@synopsys.com> writes:
>>> Baolin Wang <baolin.wang@linaro.org> writes:
>>>>> Baolin Wang <baolin.wang@linaro.org> writes:
>>>>>> When dwc3 controller acts as host role with attaching slow speed device
>>>>>> (like mouse or keypad). Then if we plugged out the slow speed device,
>>>>>> it will timeout to run the deconfiguration endpoint command to drop the
>>>>>> endpoint's resources. Some xHCI command timeout log as below when
>>>>>> disconnecting one slow device:
>>>>>>
>>>>>> [ 99.807739] c0 xhci-hcd.0.auto: Port Status Change Event for port 1
>>>>>> [ 99.814699] c0 xhci-hcd.0.auto: resume root hub
>>>>>> [ 99.819992] c0 xhci-hcd.0.auto: handle_port_status: starting port
>>>>>> polling.
>>>>>> [ 99.827808] c0 xhci-hcd.0.auto: get port status, actual port 0 status
>>>>>> = 0x202a0
>>>>>> [ 99.835903] c0 xhci-hcd.0.auto: Get port status returned 0x10100
>>>>>> [ 99.850052] c0 xhci-hcd.0.auto: clear port connect change, actual
>>>>>> port 0 status = 0x2a0
>>>>>> [ 99.859313] c0 xhci-hcd.0.auto: Cancel URB ffffffc01ed6cd00, dev 1,
>>>>>> ep 0x81, starting at offset 0xc406d210
>>>>>> [ 99.869645] c0 xhci-hcd.0.auto: // Ding dong!
>>>>>> [ 99.874776] c0 xhci-hcd.0.auto: Stopped on Transfer TRB
>>>>>> [ 99.880713] c0 xhci-hcd.0.auto: Removing canceled TD starting at
>>>>>> 0xc406d210 (dma).
>>>>>> [ 99.889012] c0 xhci-hcd.0.auto: Finding endpoint context
>>>>>> [ 99.895069] c0 xhci-hcd.0.auto: Cycle state = 0x1
>>>>>> [ 99.900519] c0 xhci-hcd.0.auto: New dequeue segment =
>>>>>> ffffffc1112f0880 (virtual)
>>>>>> [ 99.908655] c0 xhci-hcd.0.auto: New dequeue pointer = 0xc406d220 (DMA)
>>>>>> [ 99.915927] c0 xhci-hcd.0.auto: Set TR Deq Ptr cmd, new deq seg =
>>>>>> ffffffc1112f0880 (0xc406d000 dma),
>>>>>> new deq ptr = ffffff8002175220
>>>>>> (0xc406d220 dma), new cycle = 1
>>>>>> [ 99.931242] c0 xhci-hcd.0.auto: // Ding dong!
>>>>>> [ 99.936360] c0 xhci-hcd.0.auto: Successful Set TR Deq Ptr cmd,
>>>>>> deq = @c406d220
>>>>>> [ 99.944458] c0 xhci-hcd.0.auto: xhci_hub_status_data: stopping port
>>>>>> polling.
>>>>>> [ 100.047619] c0 xhci-hcd.0.auto: xhci_drop_endpoint called for udev
>>>>>> ffffffc01ae08800
>>>>>> [ 100.057002] c0 xhci-hcd.0.auto: drop ep 0x81, slot id 1, new drop
>>>>>> flags = 0x8, new add flags = 0x0
>>>>>> [ 100.067878] c0 xhci-hcd.0.auto: xhci_check_bandwidth called for udev
>>>>>> ffffffc01ae08800
>>>>>> [ 100.076868] c0 xhci-hcd.0.auto: New Input Control Context:
>>>>>>
>>>>>> ......
>>>>>>
>>>>>> [ 100.427252] c0 xhci-hcd.0.auto: // Ding dong!
>>>>>> [ 105.430728] c0 xhci-hcd.0.auto: Command timeout
>>>>>> [ 105.436029] c0 xhci-hcd.0.auto: Abort command ring
>>>>>> [ 113.558223] c0 xhci-hcd.0.auto: Command completion event does not match
>>>>>> command
>>>>>> [ 113.569778] c0 xhci-hcd.0.auto: Timeout while waiting for configure
>>>>>> endpoint command
>>>>>>
>>>>>> The reason is it will suspend USB phy to disable phy clock when
>>>>>> disconnecting the slow USB decice, that will hang on the xHCI commands
>>>>>> executing which depends on the phy clock.
>>>>>>
>>>>>> Thus we should disable USB2.0 phy suspend feature when dwc3 acts as host
>>>>>> role.
>>>>>>
>>>>>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>>>>>> ---
>>>>>> drivers/usb/dwc3/core.c | 14 ++++++++++++++
>>>>>> 1 file changed, 14 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>>>> index 9a4a5e4..0b646cf 100644
>>>>>> --- a/drivers/usb/dwc3/core.c
>>>>>> +++ b/drivers/usb/dwc3/core.c
>>>>>> @@ -565,6 +565,20 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>>>>>> if (dwc->revision > DWC3_REVISION_194A)
>>>>>> reg |= DWC3_GUSB2PHYCFG_SUSPHY;
>>>>>>
>>>>>> + /*
>>>>>> + * When dwc3 controller acts as host role with attaching one slow speed
>>>>>> + * device (like mouse or keypad). Then if we plugged out the slow speed
>>>>>> + * device, it will timeout to run the deconfiguration endpoint command.
>>>>>> + * The reason is it will suspend USB phy to disable phy clock when
>>>>>> + * disconnecting slow speed decice, which will affect the xHCI commands
>>>>>> + * executing.
>>>>>> + *
>>>>>> + * Thus we should disable USB 2.0 phy suspend feature when dwc3 acts as
>>>>>> + * host role.
>>>>>> + */
>>>>>> + if (dwc->dr_mode == USB_DR_MODE_HOST || dwc->dr_mode == USB_DR_MODE_OTG)
>>>>>> + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>>>>
>>>>> which version of the core you're using? Recent version (since 1.94A,
>>>>
>>>> My version is 2.80a.
>>>>
>>>>> IIRC) can manage core suspend automatically. Also, this patch of yours
>>>>> will cause a power consumption regression.
>>>>
>>>> Yes, it can manage core suspend automatically, that is the problem.
>>>> When plugging out one mouse or keypad device, the phy will suspend
>>>> automatically to disable the phy clock. But now the disconnecting
>>>> process is not finished, and some xHCI commands (like deconfiguration
>>>> endpoint command to drop endpoint resources) need depend on the phy
>>>> clock, which will hang on the system to timeout the command or abort
>>>> command ring to halt the xHCI.
>>>>
>>>> I agree with you it will cause a power consumption regression, but it
>>>> will cause serious problem if not. Do you have some suggestion?
>>>
>>> sorry for the long delay. This was lost in my inbox.
>>>
>>> I'm not sure this patch is the best solution. There's no mention in
>>> Databook that we should avoid PHY suspend when acting as host. Adding
>>> John here to see if John has any idea of how to fix this.
>>>
>>
>> I'm not familiar enough with XHCI side of things to say.
>>
>> I'll ask around to see if anyone has an idea.
>
Hi Felipe, Baolin,
I talked with a couple engineers here and the behavior is not
something that's expected in host mode.
Can you check that the value of the GCTL.RAMCLKSEL is set
appropriately? This affects where the core gets the clock signal
from. If it is getting it from the phy clock then you will likely have
this problem and will need to adjust it. Otherwise you should probably
use the existing quirk instead.
Regards,
John
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] usb: dwc3: core: Disable USB2.0 phy suspend when dwc3 acts as host role
2017-01-19 1:33 ` John Youn
@ 2017-01-19 3:12 ` Baolin Wang
2017-01-19 3:31 ` John Youn
0 siblings, 1 reply; 10+ messages in thread
From: Baolin Wang @ 2017-01-19 3:12 UTC (permalink / raw)
To: John Youn; +Cc: Felipe Balbi, Greg KH, Mark Brown, USB, LKML
Hi John,
On 19 January 2017 at 09:33, John Youn <John.Youn@synopsys.com> wrote:
> On 1/16/2017 2:38 AM, Felipe Balbi wrote:
>>
>> Hi,
>>
>> John Youn <John.Youn@synopsys.com> writes:
>>>> Baolin Wang <baolin.wang@linaro.org> writes:
>>>>>> Baolin Wang <baolin.wang@linaro.org> writes:
>>>>>>> When dwc3 controller acts as host role with attaching slow speed device
>>>>>>> (like mouse or keypad). Then if we plugged out the slow speed device,
>>>>>>> it will timeout to run the deconfiguration endpoint command to drop the
>>>>>>> endpoint's resources. Some xHCI command timeout log as below when
>>>>>>> disconnecting one slow device:
>>>>>>>
>>>>>>> [ 99.807739] c0 xhci-hcd.0.auto: Port Status Change Event for port 1
>>>>>>> [ 99.814699] c0 xhci-hcd.0.auto: resume root hub
>>>>>>> [ 99.819992] c0 xhci-hcd.0.auto: handle_port_status: starting port
>>>>>>> polling.
>>>>>>> [ 99.827808] c0 xhci-hcd.0.auto: get port status, actual port 0 status
>>>>>>> = 0x202a0
>>>>>>> [ 99.835903] c0 xhci-hcd.0.auto: Get port status returned 0x10100
>>>>>>> [ 99.850052] c0 xhci-hcd.0.auto: clear port connect change, actual
>>>>>>> port 0 status = 0x2a0
>>>>>>> [ 99.859313] c0 xhci-hcd.0.auto: Cancel URB ffffffc01ed6cd00, dev 1,
>>>>>>> ep 0x81, starting at offset 0xc406d210
>>>>>>> [ 99.869645] c0 xhci-hcd.0.auto: // Ding dong!
>>>>>>> [ 99.874776] c0 xhci-hcd.0.auto: Stopped on Transfer TRB
>>>>>>> [ 99.880713] c0 xhci-hcd.0.auto: Removing canceled TD starting at
>>>>>>> 0xc406d210 (dma).
>>>>>>> [ 99.889012] c0 xhci-hcd.0.auto: Finding endpoint context
>>>>>>> [ 99.895069] c0 xhci-hcd.0.auto: Cycle state = 0x1
>>>>>>> [ 99.900519] c0 xhci-hcd.0.auto: New dequeue segment =
>>>>>>> ffffffc1112f0880 (virtual)
>>>>>>> [ 99.908655] c0 xhci-hcd.0.auto: New dequeue pointer = 0xc406d220 (DMA)
>>>>>>> [ 99.915927] c0 xhci-hcd.0.auto: Set TR Deq Ptr cmd, new deq seg =
>>>>>>> ffffffc1112f0880 (0xc406d000 dma),
>>>>>>> new deq ptr = ffffff8002175220
>>>>>>> (0xc406d220 dma), new cycle = 1
>>>>>>> [ 99.931242] c0 xhci-hcd.0.auto: // Ding dong!
>>>>>>> [ 99.936360] c0 xhci-hcd.0.auto: Successful Set TR Deq Ptr cmd,
>>>>>>> deq = @c406d220
>>>>>>> [ 99.944458] c0 xhci-hcd.0.auto: xhci_hub_status_data: stopping port
>>>>>>> polling.
>>>>>>> [ 100.047619] c0 xhci-hcd.0.auto: xhci_drop_endpoint called for udev
>>>>>>> ffffffc01ae08800
>>>>>>> [ 100.057002] c0 xhci-hcd.0.auto: drop ep 0x81, slot id 1, new drop
>>>>>>> flags = 0x8, new add flags = 0x0
>>>>>>> [ 100.067878] c0 xhci-hcd.0.auto: xhci_check_bandwidth called for udev
>>>>>>> ffffffc01ae08800
>>>>>>> [ 100.076868] c0 xhci-hcd.0.auto: New Input Control Context:
>>>>>>>
>>>>>>> ......
>>>>>>>
>>>>>>> [ 100.427252] c0 xhci-hcd.0.auto: // Ding dong!
>>>>>>> [ 105.430728] c0 xhci-hcd.0.auto: Command timeout
>>>>>>> [ 105.436029] c0 xhci-hcd.0.auto: Abort command ring
>>>>>>> [ 113.558223] c0 xhci-hcd.0.auto: Command completion event does not match
>>>>>>> command
>>>>>>> [ 113.569778] c0 xhci-hcd.0.auto: Timeout while waiting for configure
>>>>>>> endpoint command
>>>>>>>
>>>>>>> The reason is it will suspend USB phy to disable phy clock when
>>>>>>> disconnecting the slow USB decice, that will hang on the xHCI commands
>>>>>>> executing which depends on the phy clock.
>>>>>>>
>>>>>>> Thus we should disable USB2.0 phy suspend feature when dwc3 acts as host
>>>>>>> role.
>>>>>>>
>>>>>>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>>>>>>> ---
>>>>>>> drivers/usb/dwc3/core.c | 14 ++++++++++++++
>>>>>>> 1 file changed, 14 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>>>>> index 9a4a5e4..0b646cf 100644
>>>>>>> --- a/drivers/usb/dwc3/core.c
>>>>>>> +++ b/drivers/usb/dwc3/core.c
>>>>>>> @@ -565,6 +565,20 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>>>>>>> if (dwc->revision > DWC3_REVISION_194A)
>>>>>>> reg |= DWC3_GUSB2PHYCFG_SUSPHY;
>>>>>>>
>>>>>>> + /*
>>>>>>> + * When dwc3 controller acts as host role with attaching one slow speed
>>>>>>> + * device (like mouse or keypad). Then if we plugged out the slow speed
>>>>>>> + * device, it will timeout to run the deconfiguration endpoint command.
>>>>>>> + * The reason is it will suspend USB phy to disable phy clock when
>>>>>>> + * disconnecting slow speed decice, which will affect the xHCI commands
>>>>>>> + * executing.
>>>>>>> + *
>>>>>>> + * Thus we should disable USB 2.0 phy suspend feature when dwc3 acts as
>>>>>>> + * host role.
>>>>>>> + */
>>>>>>> + if (dwc->dr_mode == USB_DR_MODE_HOST || dwc->dr_mode == USB_DR_MODE_OTG)
>>>>>>> + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>>>>>
>>>>>> which version of the core you're using? Recent version (since 1.94A,
>>>>>
>>>>> My version is 2.80a.
>>>>>
>>>>>> IIRC) can manage core suspend automatically. Also, this patch of yours
>>>>>> will cause a power consumption regression.
>>>>>
>>>>> Yes, it can manage core suspend automatically, that is the problem.
>>>>> When plugging out one mouse or keypad device, the phy will suspend
>>>>> automatically to disable the phy clock. But now the disconnecting
>>>>> process is not finished, and some xHCI commands (like deconfiguration
>>>>> endpoint command to drop endpoint resources) need depend on the phy
>>>>> clock, which will hang on the system to timeout the command or abort
>>>>> command ring to halt the xHCI.
>>>>>
>>>>> I agree with you it will cause a power consumption regression, but it
>>>>> will cause serious problem if not. Do you have some suggestion?
>>>>
>>>> sorry for the long delay. This was lost in my inbox.
>>>>
>>>> I'm not sure this patch is the best solution. There's no mention in
>>>> Databook that we should avoid PHY suspend when acting as host. Adding
>>>> John here to see if John has any idea of how to fix this.
>>>>
>>>
>>> I'm not familiar enough with XHCI side of things to say.
>>>
>>> I'll ask around to see if anyone has an idea.
>>
>
> Hi Felipe, Baolin,
>
> I talked with a couple engineers here and the behavior is not
> something that's expected in host mode.
>
> Can you check that the value of the GCTL.RAMCLKSEL is set
> appropriately? This affects where the core gets the clock signal
In host mode, the bit[6:7] for RAMCLKSEL is default value 0, which
means it selects bus clock.
> from. If it is getting it from the phy clock then you will likely have
> this problem and will need to adjust it. Otherwise you should probably
> use the existing quirk instead.
So the bus clock is from the phy clock, then it will have this problem
when suspending phy? Yes, I can use the existing quirk, but I am
afraid it is one common problem if we use the mainline kernel. Or we
can add some documentation for enabling the phy suspend feature to
remind other people.
--
Baolin.wang
Best Regards
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] usb: dwc3: core: Disable USB2.0 phy suspend when dwc3 acts as host role
2017-01-19 3:12 ` Baolin Wang
@ 2017-01-19 3:31 ` John Youn
2017-01-19 5:37 ` Baolin Wang
0 siblings, 1 reply; 10+ messages in thread
From: John Youn @ 2017-01-19 3:31 UTC (permalink / raw)
To: Baolin Wang, John Youn; +Cc: Felipe Balbi, Greg KH, Mark Brown, USB, LKML
On 1/18/2017 7:12 PM, Baolin Wang wrote:
> Hi John,
>
> On 19 January 2017 at 09:33, John Youn <John.Youn@synopsys.com> wrote:
>> On 1/16/2017 2:38 AM, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> John Youn <John.Youn@synopsys.com> writes:
>>>>> Baolin Wang <baolin.wang@linaro.org> writes:
>>>>>>> Baolin Wang <baolin.wang@linaro.org> writes:
>>>>>>>> When dwc3 controller acts as host role with attaching slow speed device
>>>>>>>> (like mouse or keypad). Then if we plugged out the slow speed device,
>>>>>>>> it will timeout to run the deconfiguration endpoint command to drop the
>>>>>>>> endpoint's resources. Some xHCI command timeout log as below when
>>>>>>>> disconnecting one slow device:
>>>>>>>>
>>>>>>>> [ 99.807739] c0 xhci-hcd.0.auto: Port Status Change Event for port 1
>>>>>>>> [ 99.814699] c0 xhci-hcd.0.auto: resume root hub
>>>>>>>> [ 99.819992] c0 xhci-hcd.0.auto: handle_port_status: starting port
>>>>>>>> polling.
>>>>>>>> [ 99.827808] c0 xhci-hcd.0.auto: get port status, actual port 0 status
>>>>>>>> = 0x202a0
>>>>>>>> [ 99.835903] c0 xhci-hcd.0.auto: Get port status returned 0x10100
>>>>>>>> [ 99.850052] c0 xhci-hcd.0.auto: clear port connect change, actual
>>>>>>>> port 0 status = 0x2a0
>>>>>>>> [ 99.859313] c0 xhci-hcd.0.auto: Cancel URB ffffffc01ed6cd00, dev 1,
>>>>>>>> ep 0x81, starting at offset 0xc406d210
>>>>>>>> [ 99.869645] c0 xhci-hcd.0.auto: // Ding dong!
>>>>>>>> [ 99.874776] c0 xhci-hcd.0.auto: Stopped on Transfer TRB
>>>>>>>> [ 99.880713] c0 xhci-hcd.0.auto: Removing canceled TD starting at
>>>>>>>> 0xc406d210 (dma).
>>>>>>>> [ 99.889012] c0 xhci-hcd.0.auto: Finding endpoint context
>>>>>>>> [ 99.895069] c0 xhci-hcd.0.auto: Cycle state = 0x1
>>>>>>>> [ 99.900519] c0 xhci-hcd.0.auto: New dequeue segment =
>>>>>>>> ffffffc1112f0880 (virtual)
>>>>>>>> [ 99.908655] c0 xhci-hcd.0.auto: New dequeue pointer = 0xc406d220 (DMA)
>>>>>>>> [ 99.915927] c0 xhci-hcd.0.auto: Set TR Deq Ptr cmd, new deq seg =
>>>>>>>> ffffffc1112f0880 (0xc406d000 dma),
>>>>>>>> new deq ptr = ffffff8002175220
>>>>>>>> (0xc406d220 dma), new cycle = 1
>>>>>>>> [ 99.931242] c0 xhci-hcd.0.auto: // Ding dong!
>>>>>>>> [ 99.936360] c0 xhci-hcd.0.auto: Successful Set TR Deq Ptr cmd,
>>>>>>>> deq = @c406d220
>>>>>>>> [ 99.944458] c0 xhci-hcd.0.auto: xhci_hub_status_data: stopping port
>>>>>>>> polling.
>>>>>>>> [ 100.047619] c0 xhci-hcd.0.auto: xhci_drop_endpoint called for udev
>>>>>>>> ffffffc01ae08800
>>>>>>>> [ 100.057002] c0 xhci-hcd.0.auto: drop ep 0x81, slot id 1, new drop
>>>>>>>> flags = 0x8, new add flags = 0x0
>>>>>>>> [ 100.067878] c0 xhci-hcd.0.auto: xhci_check_bandwidth called for udev
>>>>>>>> ffffffc01ae08800
>>>>>>>> [ 100.076868] c0 xhci-hcd.0.auto: New Input Control Context:
>>>>>>>>
>>>>>>>> ......
>>>>>>>>
>>>>>>>> [ 100.427252] c0 xhci-hcd.0.auto: // Ding dong!
>>>>>>>> [ 105.430728] c0 xhci-hcd.0.auto: Command timeout
>>>>>>>> [ 105.436029] c0 xhci-hcd.0.auto: Abort command ring
>>>>>>>> [ 113.558223] c0 xhci-hcd.0.auto: Command completion event does not match
>>>>>>>> command
>>>>>>>> [ 113.569778] c0 xhci-hcd.0.auto: Timeout while waiting for configure
>>>>>>>> endpoint command
>>>>>>>>
>>>>>>>> The reason is it will suspend USB phy to disable phy clock when
>>>>>>>> disconnecting the slow USB decice, that will hang on the xHCI commands
>>>>>>>> executing which depends on the phy clock.
>>>>>>>>
>>>>>>>> Thus we should disable USB2.0 phy suspend feature when dwc3 acts as host
>>>>>>>> role.
>>>>>>>>
>>>>>>>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>>>>>>>> ---
>>>>>>>> drivers/usb/dwc3/core.c | 14 ++++++++++++++
>>>>>>>> 1 file changed, 14 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>>>>>> index 9a4a5e4..0b646cf 100644
>>>>>>>> --- a/drivers/usb/dwc3/core.c
>>>>>>>> +++ b/drivers/usb/dwc3/core.c
>>>>>>>> @@ -565,6 +565,20 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>>>>>>>> if (dwc->revision > DWC3_REVISION_194A)
>>>>>>>> reg |= DWC3_GUSB2PHYCFG_SUSPHY;
>>>>>>>>
>>>>>>>> + /*
>>>>>>>> + * When dwc3 controller acts as host role with attaching one slow speed
>>>>>>>> + * device (like mouse or keypad). Then if we plugged out the slow speed
>>>>>>>> + * device, it will timeout to run the deconfiguration endpoint command.
>>>>>>>> + * The reason is it will suspend USB phy to disable phy clock when
>>>>>>>> + * disconnecting slow speed decice, which will affect the xHCI commands
>>>>>>>> + * executing.
>>>>>>>> + *
>>>>>>>> + * Thus we should disable USB 2.0 phy suspend feature when dwc3 acts as
>>>>>>>> + * host role.
>>>>>>>> + */
>>>>>>>> + if (dwc->dr_mode == USB_DR_MODE_HOST || dwc->dr_mode == USB_DR_MODE_OTG)
>>>>>>>> + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>>>>>>
>>>>>>> which version of the core you're using? Recent version (since 1.94A,
>>>>>>
>>>>>> My version is 2.80a.
>>>>>>
>>>>>>> IIRC) can manage core suspend automatically. Also, this patch of yours
>>>>>>> will cause a power consumption regression.
>>>>>>
>>>>>> Yes, it can manage core suspend automatically, that is the problem.
>>>>>> When plugging out one mouse or keypad device, the phy will suspend
>>>>>> automatically to disable the phy clock. But now the disconnecting
>>>>>> process is not finished, and some xHCI commands (like deconfiguration
>>>>>> endpoint command to drop endpoint resources) need depend on the phy
>>>>>> clock, which will hang on the system to timeout the command or abort
>>>>>> command ring to halt the xHCI.
>>>>>>
>>>>>> I agree with you it will cause a power consumption regression, but it
>>>>>> will cause serious problem if not. Do you have some suggestion?
>>>>>
>>>>> sorry for the long delay. This was lost in my inbox.
>>>>>
>>>>> I'm not sure this patch is the best solution. There's no mention in
>>>>> Databook that we should avoid PHY suspend when acting as host. Adding
>>>>> John here to see if John has any idea of how to fix this.
>>>>>
>>>>
>>>> I'm not familiar enough with XHCI side of things to say.
>>>>
>>>> I'll ask around to see if anyone has an idea.
>>>
>>
>> Hi Felipe, Baolin,
>>
>> I talked with a couple engineers here and the behavior is not
>> something that's expected in host mode.
>>
>> Can you check that the value of the GCTL.RAMCLKSEL is set
>> appropriately? This affects where the core gets the clock signal
>
> In host mode, the bit[6:7] for RAMCLKSEL is default value 0, which
> means it selects bus clock.
>
>> from. If it is getting it from the phy clock then you will likely have
>> this problem and will need to adjust it. Otherwise you should probably
>> use the existing quirk instead.
>
> So the bus clock is from the phy clock, then it will have this problem
> when suspending phy? Yes, I can use the existing quirk, but I am
> afraid it is one common problem if we use the mainline kernel. Or we
> can add some documentation for enabling the phy suspend feature to
> remind other people.
>
Hi Baolin,
It's expected the clocks to the PHY and core are different if you
suspend only the PHY. That's not particular to just the host, for
example handling LPM in device mode you could have the same problem.
You would have to confirm with your own platform how it is.
Regards,
John
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] usb: dwc3: core: Disable USB2.0 phy suspend when dwc3 acts as host role
2017-01-19 3:31 ` John Youn
@ 2017-01-19 5:37 ` Baolin Wang
0 siblings, 0 replies; 10+ messages in thread
From: Baolin Wang @ 2017-01-19 5:37 UTC (permalink / raw)
To: John Youn; +Cc: Felipe Balbi, Greg KH, Mark Brown, USB, LKML
Hi John,
On 19 January 2017 at 11:31, John Youn <John.Youn@synopsys.com> wrote:
> On 1/18/2017 7:12 PM, Baolin Wang wrote:
>> Hi John,
>>
>> On 19 January 2017 at 09:33, John Youn <John.Youn@synopsys.com> wrote:
>>> On 1/16/2017 2:38 AM, Felipe Balbi wrote:
>>>>
>>>> Hi,
>>>>
>>>> John Youn <John.Youn@synopsys.com> writes:
>>>>>> Baolin Wang <baolin.wang@linaro.org> writes:
>>>>>>>> Baolin Wang <baolin.wang@linaro.org> writes:
>>>>>>>>> When dwc3 controller acts as host role with attaching slow speed device
>>>>>>>>> (like mouse or keypad). Then if we plugged out the slow speed device,
>>>>>>>>> it will timeout to run the deconfiguration endpoint command to drop the
>>>>>>>>> endpoint's resources. Some xHCI command timeout log as below when
>>>>>>>>> disconnecting one slow device:
>>>>>>>>>
>>>>>>>>> [ 99.807739] c0 xhci-hcd.0.auto: Port Status Change Event for port 1
>>>>>>>>> [ 99.814699] c0 xhci-hcd.0.auto: resume root hub
>>>>>>>>> [ 99.819992] c0 xhci-hcd.0.auto: handle_port_status: starting port
>>>>>>>>> polling.
>>>>>>>>> [ 99.827808] c0 xhci-hcd.0.auto: get port status, actual port 0 status
>>>>>>>>> = 0x202a0
>>>>>>>>> [ 99.835903] c0 xhci-hcd.0.auto: Get port status returned 0x10100
>>>>>>>>> [ 99.850052] c0 xhci-hcd.0.auto: clear port connect change, actual
>>>>>>>>> port 0 status = 0x2a0
>>>>>>>>> [ 99.859313] c0 xhci-hcd.0.auto: Cancel URB ffffffc01ed6cd00, dev 1,
>>>>>>>>> ep 0x81, starting at offset 0xc406d210
>>>>>>>>> [ 99.869645] c0 xhci-hcd.0.auto: // Ding dong!
>>>>>>>>> [ 99.874776] c0 xhci-hcd.0.auto: Stopped on Transfer TRB
>>>>>>>>> [ 99.880713] c0 xhci-hcd.0.auto: Removing canceled TD starting at
>>>>>>>>> 0xc406d210 (dma).
>>>>>>>>> [ 99.889012] c0 xhci-hcd.0.auto: Finding endpoint context
>>>>>>>>> [ 99.895069] c0 xhci-hcd.0.auto: Cycle state = 0x1
>>>>>>>>> [ 99.900519] c0 xhci-hcd.0.auto: New dequeue segment =
>>>>>>>>> ffffffc1112f0880 (virtual)
>>>>>>>>> [ 99.908655] c0 xhci-hcd.0.auto: New dequeue pointer = 0xc406d220 (DMA)
>>>>>>>>> [ 99.915927] c0 xhci-hcd.0.auto: Set TR Deq Ptr cmd, new deq seg =
>>>>>>>>> ffffffc1112f0880 (0xc406d000 dma),
>>>>>>>>> new deq ptr = ffffff8002175220
>>>>>>>>> (0xc406d220 dma), new cycle = 1
>>>>>>>>> [ 99.931242] c0 xhci-hcd.0.auto: // Ding dong!
>>>>>>>>> [ 99.936360] c0 xhci-hcd.0.auto: Successful Set TR Deq Ptr cmd,
>>>>>>>>> deq = @c406d220
>>>>>>>>> [ 99.944458] c0 xhci-hcd.0.auto: xhci_hub_status_data: stopping port
>>>>>>>>> polling.
>>>>>>>>> [ 100.047619] c0 xhci-hcd.0.auto: xhci_drop_endpoint called for udev
>>>>>>>>> ffffffc01ae08800
>>>>>>>>> [ 100.057002] c0 xhci-hcd.0.auto: drop ep 0x81, slot id 1, new drop
>>>>>>>>> flags = 0x8, new add flags = 0x0
>>>>>>>>> [ 100.067878] c0 xhci-hcd.0.auto: xhci_check_bandwidth called for udev
>>>>>>>>> ffffffc01ae08800
>>>>>>>>> [ 100.076868] c0 xhci-hcd.0.auto: New Input Control Context:
>>>>>>>>>
>>>>>>>>> ......
>>>>>>>>>
>>>>>>>>> [ 100.427252] c0 xhci-hcd.0.auto: // Ding dong!
>>>>>>>>> [ 105.430728] c0 xhci-hcd.0.auto: Command timeout
>>>>>>>>> [ 105.436029] c0 xhci-hcd.0.auto: Abort command ring
>>>>>>>>> [ 113.558223] c0 xhci-hcd.0.auto: Command completion event does not match
>>>>>>>>> command
>>>>>>>>> [ 113.569778] c0 xhci-hcd.0.auto: Timeout while waiting for configure
>>>>>>>>> endpoint command
>>>>>>>>>
>>>>>>>>> The reason is it will suspend USB phy to disable phy clock when
>>>>>>>>> disconnecting the slow USB decice, that will hang on the xHCI commands
>>>>>>>>> executing which depends on the phy clock.
>>>>>>>>>
>>>>>>>>> Thus we should disable USB2.0 phy suspend feature when dwc3 acts as host
>>>>>>>>> role.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>>>>>>>>> ---
>>>>>>>>> drivers/usb/dwc3/core.c | 14 ++++++++++++++
>>>>>>>>> 1 file changed, 14 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>>>>>>> index 9a4a5e4..0b646cf 100644
>>>>>>>>> --- a/drivers/usb/dwc3/core.c
>>>>>>>>> +++ b/drivers/usb/dwc3/core.c
>>>>>>>>> @@ -565,6 +565,20 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>>>>>>>>> if (dwc->revision > DWC3_REVISION_194A)
>>>>>>>>> reg |= DWC3_GUSB2PHYCFG_SUSPHY;
>>>>>>>>>
>>>>>>>>> + /*
>>>>>>>>> + * When dwc3 controller acts as host role with attaching one slow speed
>>>>>>>>> + * device (like mouse or keypad). Then if we plugged out the slow speed
>>>>>>>>> + * device, it will timeout to run the deconfiguration endpoint command.
>>>>>>>>> + * The reason is it will suspend USB phy to disable phy clock when
>>>>>>>>> + * disconnecting slow speed decice, which will affect the xHCI commands
>>>>>>>>> + * executing.
>>>>>>>>> + *
>>>>>>>>> + * Thus we should disable USB 2.0 phy suspend feature when dwc3 acts as
>>>>>>>>> + * host role.
>>>>>>>>> + */
>>>>>>>>> + if (dwc->dr_mode == USB_DR_MODE_HOST || dwc->dr_mode == USB_DR_MODE_OTG)
>>>>>>>>> + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>>>>>>>
>>>>>>>> which version of the core you're using? Recent version (since 1.94A,
>>>>>>>
>>>>>>> My version is 2.80a.
>>>>>>>
>>>>>>>> IIRC) can manage core suspend automatically. Also, this patch of yours
>>>>>>>> will cause a power consumption regression.
>>>>>>>
>>>>>>> Yes, it can manage core suspend automatically, that is the problem.
>>>>>>> When plugging out one mouse or keypad device, the phy will suspend
>>>>>>> automatically to disable the phy clock. But now the disconnecting
>>>>>>> process is not finished, and some xHCI commands (like deconfiguration
>>>>>>> endpoint command to drop endpoint resources) need depend on the phy
>>>>>>> clock, which will hang on the system to timeout the command or abort
>>>>>>> command ring to halt the xHCI.
>>>>>>>
>>>>>>> I agree with you it will cause a power consumption regression, but it
>>>>>>> will cause serious problem if not. Do you have some suggestion?
>>>>>>
>>>>>> sorry for the long delay. This was lost in my inbox.
>>>>>>
>>>>>> I'm not sure this patch is the best solution. There's no mention in
>>>>>> Databook that we should avoid PHY suspend when acting as host. Adding
>>>>>> John here to see if John has any idea of how to fix this.
>>>>>>
>>>>>
>>>>> I'm not familiar enough with XHCI side of things to say.
>>>>>
>>>>> I'll ask around to see if anyone has an idea.
>>>>
>>>
>>> Hi Felipe, Baolin,
>>>
>>> I talked with a couple engineers here and the behavior is not
>>> something that's expected in host mode.
>>>
>>> Can you check that the value of the GCTL.RAMCLKSEL is set
>>> appropriately? This affects where the core gets the clock signal
>>
>> In host mode, the bit[6:7] for RAMCLKSEL is default value 0, which
>> means it selects bus clock.
>>
>>> from. If it is getting it from the phy clock then you will likely have
>>> this problem and will need to adjust it. Otherwise you should probably
>>> use the existing quirk instead.
>>
>> So the bus clock is from the phy clock, then it will have this problem
>> when suspending phy? Yes, I can use the existing quirk, but I am
>> afraid it is one common problem if we use the mainline kernel. Or we
>> can add some documentation for enabling the phy suspend feature to
>> remind other people.
>>
>
> Hi Baolin,
>
> It's expected the clocks to the PHY and core are different if you
> suspend only the PHY. That's not particular to just the host, for
> example handling LPM in device mode you could have the same problem.
>From Synopsys FAE engineer, she told me it will affect xHCI commands
executing when phy has been suspended. But in device mode, I can not
reproduce this problem, just in host mode with attaching slow devices.
>
> You would have to confirm with your own platform how it is.
Okay, I will.
--
Baolin.wang
Best Regards
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-01-19 5:38 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-15 9:41 [PATCH] usb: dwc3: core: Disable USB2.0 phy suspend when dwc3 acts as host role Baolin Wang
2016-11-15 10:49 ` Felipe Balbi
2016-11-16 2:55 ` Baolin Wang
2017-01-12 7:49 ` Felipe Balbi
2017-01-12 21:47 ` John Youn
2017-01-16 10:36 ` Felipe Balbi
2017-01-19 1:33 ` John Youn
2017-01-19 3:12 ` Baolin Wang
2017-01-19 3:31 ` John Youn
2017-01-19 5:37 ` Baolin Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).