diff mbox series

usb: dwc2: Fix HiKey regression caused by power_down feature

Message ID 1526690943-8211-1-git-send-email-john.stultz@linaro.org
State Superseded
Headers show
Series usb: dwc2: Fix HiKey regression caused by power_down feature | expand

Commit Message

John Stultz May 19, 2018, 12:49 a.m. UTC
In 4.17-rc, commit 03ea6d6e9e1f ("usb: dwc2: Enable power down")
caused the HiKey board to not correctly handle switching between
usb-gadget and usb-host mode.

Unplugging the OTG port would result in:
[   42.240973] dwc2 f72c0000.usb: dwc2_restore_host_registers: no host registers to restore
[   42.249066] dwc2 f72c0000.usb: dwc2_host_exit_hibernation: failed to restore host registers

And the USB-host ports would not function.

And plugging in the OTG port, we would see:
[   46.046557] WARNING: CPU: 3 PID: 6 at drivers/usb/dwc2/gadget.c:260 dwc2_hsotg_init_fifo+0x194/0x1a0
[   46.055761] CPU: 3 PID: 6 Comm: kworker/u16:0 Not tainted 4.17.0-rc5-00030-ge67da8c #231
[   46.055767] Hardware name: HiKey Development Board (DT)
[   46.055784] Workqueue: dwc2 dwc2_conn_id_status_change
...

Thus, this patch sets the hisi params to disable the power_down
flag by default, and gets thing working again.

Cc: John Youn <johnyoun@synopsys.com>
Cc: Vardan Mikayelyan <mvardan@synopsys.com>
Cc: Artur Petrosyan <arturp@synopsys.com>
Cc: Grigor Tovmasyan <tovmasya@synopsys.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: linux-usb@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>

---
 drivers/usb/dwc2/params.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.7.4

Comments

Minas Harutyunyan May 21, 2018, 8:45 a.m. UTC | #1
Hi John,

On 5/19/2018 4:49 AM, John Stultz wrote:
> In 4.17-rc, commit 03ea6d6e9e1f ("usb: dwc2: Enable power down")
> caused the HiKey board to not correctly handle switching between
> usb-gadget and usb-host mode.
> 
> Unplugging the OTG port would result in:
> [   42.240973] dwc2 f72c0000.usb: dwc2_restore_host_registers: no host registers to restore
> [   42.249066] dwc2 f72c0000.usb: dwc2_host_exit_hibernation: failed to restore host registers
> 
> And the USB-host ports would not function.
> 
> And plugging in the OTG port, we would see:
> [   46.046557] WARNING: CPU: 3 PID: 6 at drivers/usb/dwc2/gadget.c:260 dwc2_hsotg_init_fifo+0x194/0x1a0
> [   46.055761] CPU: 3 PID: 6 Comm: kworker/u16:0 Not tainted 4.17.0-rc5-00030-ge67da8c #231
> [   46.055767] Hardware name: HiKey Development Board (DT)
> [   46.055784] Workqueue: dwc2 dwc2_conn_id_status_change
> ...
> 
Could you please send full log to debug.


> Thus, this patch sets the hisi params to disable the power_down
> flag by default, and gets thing working again.
> 
> Cc: John Youn <johnyoun@synopsys.com>
> Cc: Vardan Mikayelyan <mvardan@synopsys.com>
> Cc: Artur Petrosyan <arturp@synopsys.com>
> Cc: Grigor Tovmasyan <tovmasya@synopsys.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>   drivers/usb/dwc2/params.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> index f03e418..96b1b25 100644
> --- a/drivers/usb/dwc2/params.c
> +++ b/drivers/usb/dwc2/params.c
> @@ -70,6 +70,7 @@ static void dwc2_set_his_params(struct dwc2_hsotg *hsotg)
>   		GAHBCFG_HBSTLEN_SHIFT;
>   	p->uframe_sched = false;
>   	p->change_speed_quirk = true;
> +	p->power_down = false;

power_down declared as int, suggested to update as follow:
	p->power_down = DWC2_POWER_DOWN_PARAM_NONE;

This can be accepted as temporary solution until we will fully debug 
hibernation feature for HiKey platform.

>   }
>   
>   static void dwc2_set_rk_params(struct dwc2_hsotg *hsotg)
>
Minas Harutyunyan May 22, 2018, 2:24 p.m. UTC | #2
Hi John,

Please provide log with debug enabled configuration.

On 5/21/2018 11:41 PM, John Stultz wrote:
> On Mon, May 21, 2018 at 1:45 AM, Minas Harutyunyan
> <Minas.Harutyunyan@synopsys.com> wrote:
>> Hi John,
>>
>> On 5/19/2018 4:49 AM, John Stultz wrote:
>>> In 4.17-rc, commit 03ea6d6e9e1f ("usb: dwc2: Enable power down")
>>> caused the HiKey board to not correctly handle switching between
>>> usb-gadget and usb-host mode.
>>>
>>> Unplugging the OTG port would result in:
OTG port you mean MicroAB, Correct?
dwc2 driver loaded when some device connected to OTG port?
And below message printed after disconnect the device from OTG port?

>>> [   42.240973] dwc2 f72c0000.usb: dwc2_restore_host_registers: no host registers to restore
>>> [   42.249066] dwc2 f72c0000.usb: dwc2_host_exit_hibernation: failed to restore host registers
>>>
>>> And the USB-host ports would not function.
USB-host ports - you mean 2 USB A-ports, connected to TS3USB221 HUB?
Switching ports between OTG and Host ports via TS3USB221 Switch 
performing automatically or by some SW tool?

>>>
>>> And plugging in the OTG port, we would see:
>>> [   46.046557] WARNING: CPU: 3 PID: 6 at drivers/usb/dwc2/gadget.c:260 dwc2_hsotg_init_fifo+0x194/0x1a0
>>> [   46.055761] CPU: 3 PID: 6 Comm: kworker/u16:0 Not tainted 4.17.0-rc5-00030-ge67da8c #231
>>> [   46.055767] Hardware name: HiKey Development Board (DT)
>>> [   46.055784] Workqueue: dwc2 dwc2_conn_id_status_change
>>> ...
>>>
>> Could you please send full log to debug.
> 
> Full dmesg log attached.
> 
> I unplugged the usb-otg port at 136
> and replugged it back in at 141
> 
> 
>>>        p->uframe_sched = false;
>>>        p->change_speed_quirk = true;
>>> +     p->power_down = false;
>>
>> power_down declared as int, suggested to update as follow:
>>          p->power_down = DWC2_POWER_DOWN_PARAM_NONE;
>>
>> This can be accepted as temporary solution until we will fully debug
>> hibernation feature for HiKey platform.
> 
> Ok, will re-send with the suggested change above.
> 
> thanks
> -john
>
John Stultz Sept. 21, 2018, 1:05 a.m. UTC | #3
On Thu, Sep 20, 2018 at 7:17 AM, Artur Petrosyan
<Arthur.Petrosyan@synopsys.com> wrote:
> On 5/23/2018 01:57, John Stultz wrote:

>> Its done automatically, when the OTG cable is detected it the host

>> ports are disabled and when the OTG port is empty the host ports are

>> enabled.

>>

>> Let me know if you need anything else!

>>

>

> Please apply the patch set with this cover letter "[PATCH 0/3] usb:

> dwc2: Fix hibernation for switching between host and device modes."


Sorry, can you send the patches to me, or point me to a git tree? I'm
not seeing that thread in my mailbox or on google.

> Enable the power down on his devices. Let me know if you still see any

> issue. If there is no issue, please provide Tested-by tag.


Would be happy to test it, thought I'm traveling tomorrow, so I may
not be able to validate till monday.

thanks
-john
John Stultz Sept. 24, 2018, 6:51 p.m. UTC | #4
On Sun, Sep 23, 2018 at 10:57 PM, Artur Petrosyan
<arthur.petrosyan@synopsys.com> wrote:
> Hi John,

>

> On 9/21/2018 05:05, John Stultz wrote:

>> On Thu, Sep 20, 2018 at 7:17 AM, Artur Petrosyan

>> <Arthur.Petrosyan@synopsys.com> wrote:

>>> On 5/23/2018 01:57, John Stultz wrote:

>>>> Its done automatically, when the OTG cable is detected it the host

>>>> ports are disabled and when the OTG port is empty the host ports are

>>>> enabled.

>>>>

>>>> Let me know if you need anything else!

>>>>

>>>

>>> Please apply the patch set with this cover letter "[PATCH 0/3] usb:

>>> dwc2: Fix hibernation for switching between host and device modes."

>>

>> Sorry, can you send the patches to me, or point me to a git tree? I'm

>> not seeing that thread in my mailbox or on google.

>>

>>> Enable the power down on his devices. Let me know if you still see any

>>> issue. If there is no issue, please provide Tested-by tag.

>>

>> Would be happy to test it, thought I'm traveling tomorrow, so I may

>> not be able to validate till monday.

>>

>> thanks

>> -john

>>

>

> You can find the patch set following to this link.

>

> https://marc.info/?l=linux-usb&m=153745139408236&w=2


I applied those three patches, and it seems to work ok.

Just to be clear, was there anything else I was needing to do while testing it?

Otherwise,
Tested-by: John Stultz <john.stultz@linaro.org>   #On HiKey


thanks
-john
John Stultz Sept. 28, 2018, 6:32 p.m. UTC | #5
On Thu, Sep 27, 2018 at 5:33 AM, Artur Petrosyan
<arthur.petrosyan@synopsys.com> wrote:
> We would like to buy the HiKey board to perform testes.

> We found this HiKey LeMaker to have USB 2.0 ports

> https://www.ebay.com/itm/HiKey-LeMaker-version-2GB-Kirin-620-SoC-8-core-ARM-Cortex-A53-CPU-ARM-Mali-450/263958047308?hash=item3d75202a4c:g:aGsAAOSwOkxbqqot

> on ebay.

>

> Could you please confirm that it is the right board to test the issues

> you mention.


Yep. That's the one.

thanks
-john
Artur Petrosyan Oct. 1, 2018, 6:56 a.m. UTC | #6
Hi John,

On 9/28/2018 22:30, John Stultz wrote:
> On Tue, Sep 25, 2018 at 11:02 PM, Artur Petrosyan 
> <arthur.petrosyan@synopsys.com <mailto:arthur.petrosyan@synopsys.com>> 
> wrote:
>  > On 9/25/2018 21:59, John Stultz wrote:
>  >> On Tue, Sep 25, 2018 at 3:04 AM, Artur Petrosyan
>  >> <arthur.petrosyan@synopsys.com 
> <mailto:arthur.petrosyan@synopsys.com>> wrote:
>  >>> Just a clarification by this commit "[PATCH] usb: dwc2: Fix HiKey
>  >>> regression caused by power_down feature"
>  >>> 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__marc.info_-3Fl-3Dlinux-2Dusb-26m-3D152669095513248-26w-3D2&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=9hPBFKCJ_nBjJhGVrrlYOeOQjP_HlVzYqrC_D7niMJI&m=0lMkv7adFVwkzyaUzD6-pUG0iwg4fd6b1-aHQgbqvSI&s=m8SZvo3J_Za08sMbo-S9EkhoA06YnzEN-SRm-uTPnbg&e=
>  >>>
>  >>> the power_down is disabled setting "p->power_down = false;" in
>  >>> "dwc2_set_his_params" function.
>  >>>
>  >>> Could you please clarify that the testes done for those 3 patches were
>  >>> done enabling "p->power_down = true;" in "dwc2_set_his_params" 
> function.
>  >>
>  >> So if I remove the "power_down = true" initialization, USB does not
>  >> seem to function.
>  >>
>  >> If I boot w/ the gadget port removed, the USB host ports do work, but
>  >> plugging in the gadget cable results in a bunch of:
>  >>     dwc2 f72c0000.usb: Waiting for Host Mode, Mode=Peripheral
>  >> messages.
>  >>
>  >> If I boot w/ the gadget port plugged in, USB gadget mode doesn't seem
>  >> to function at all, and when I remove the gadget cable nothing
>  >> happens, it doesn't switch to host mode.
>  >>
>  >
>  > Could you please send the dmesg logs for those situations?
> 
> Attached with some annotations in the log.
> 
>  > Also, please specify the version of the kernel that the testes has been
>  > done on.
> 
> This was done utilizing my dev/hikey-mainline-WIP branch here:
> https://git.linaro.org/people/john.stultz/android-dev.git/log/?h=dev/hikey-mainline-WIP 
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__git.linaro.org_people_john.stultz_android-2Ddev.git_log_-3Fh-3Ddev_hikey-2Dmainline-2DWIP&d=DwMFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=9hPBFKCJ_nBjJhGVrrlYOeOQjP_HlVzYqrC_D7niMJI&m=NziBHu0F0eefQE7CehTMAHF9RnbfSzfHeSOy5MUMv_o&s=MzQmTF47Ved3Qe2vhL66-hyxWyLUwcC5kK7BhZjxRvI&e=>
> 
> which is 4.19.0-rc5 based + your 3 patches + removing the "p->power_down 
> = true" line.
> 
> Though the changes in my tree are mainly for getting Android up and 
> running and not related to USB.
> 
> thanks
> -john

Looking through the dmesg log you have provided, It seems that the debug 
is disabled.
Could you please provide log with debug enabled configuration. So that 
we can see more about the described problem.

Also, the register dump could be really helpful for us. Please provide 
it too.

Regards,
Artur
diff mbox series

Patch

diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index f03e418..96b1b25 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -70,6 +70,7 @@  static void dwc2_set_his_params(struct dwc2_hsotg *hsotg)
 		GAHBCFG_HBSTLEN_SHIFT;
 	p->uframe_sched = false;
 	p->change_speed_quirk = true;
+	p->power_down = false;
 }
 
 static void dwc2_set_rk_params(struct dwc2_hsotg *hsotg)