Re: [RESEND PATCH v2] usb: dwc2: disable power_down on rockchip for regression
On 10/29/18 4:02 AM, Minas Harutyunyan wrote: Hi Hal, On 10/26/2018 6:38 PM, Hal Emmerich wrote: From 04fbf78e4e569bf872f1ffcb0a6f9b89569dc913 Mon Sep 17 00:00:00 2001 From: Hal Emmerich Date: Thu, 19 Jul 2018 21:48:08 -0500 Subject: [PATCH] usb: dwc2: disable power_down on rockchip devices The bug would let the usb controller enter partial power down, which was formally known as hibernate, upon boot if nothing was plugged in to the port. Partial power down couldn't be exited properly, so any usb devices plugged in after boot would not be usable. Before the name change, params.hibernation was false by default, so _dwc2_hcd_suspend() would skip entering hibernation. With the rename, _dwc2_hcd_suspend() was changed to use params.power_down to decide whether or not to enter partial power down. Since params.power_down is non-zero by default, it needs to be set to 0 for rockchip devices to restore functionality. This bug was reported in the linux-usb thread: REGRESSION: usb: dwc2: USB device not seen after boot The commit that caused this regression is: 6d23ee9caa6790aea047f9aca7f3c03cb8d96eb6 Signed-off-by: Hal Emmerich Acked-by: Minas Harutyunyan --- 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 bf7052e037d6..09292dc977e4 100644 --- a/drivers/usb/dwc2/params.c +++ b/drivers/usb/dwc2/params.c @@ -81,6 +81,7 @@ static void dwc2_set_rk_params(struct dwc2_hsotg *hsotg) p->host_perio_tx_fifo_size = 256; p->ahbcfg = GAHBCFG_HBSTLEN_INCR16 << GAHBCFG_HBSTLEN_SHIFT; + p->power_down = 0; } static void dwc2_set_ltq_params(struct dwc2_hsotg *hsotg) -- 2.11.0 Could you please elaborate. In subject mentioned that it's "v2" patch. But looks like its fully same as v1 patch. If not, then where new version patch updates description. Thanks, Minas Hey Minas, felipe.ba...@linux.intel.com mentioned that he was unable to apply the original patch so I remade the patch, and confirmed I could apply it cleanly to the current tree and marked it v2. He also mentioned that I should "collect" your ack, which I'm not familiar with. I added: >> Acked-by: Minas Harutyunyan which may or may not be the correct way of doing things. Let me know if I am doing that incorrectly. Sorry if I shouldn't have marked this v2. Also, just to let you know, my mailserver says that your mailserver refuses to collect my email. My servers not on any blocklists and seems to send mail elsewhere fine so I'm guessing your mailserver has mistakenly categorized me as spam on its own. Seems my mail still gets through to you from the list though, so not a big deal. Sorry if I'm not doing some part of this correctly. Thanks, Evan
[RESEND PATCH v2] usb: dwc2: disable power_down on rockchip for regression
>From 04fbf78e4e569bf872f1ffcb0a6f9b89569dc913 Mon Sep 17 00:00:00 2001 From: Hal Emmerich Date: Thu, 19 Jul 2018 21:48:08 -0500 Subject: [PATCH] usb: dwc2: disable power_down on rockchip devices The bug would let the usb controller enter partial power down, which was formally known as hibernate, upon boot if nothing was plugged in to the port. Partial power down couldn't be exited properly, so any usb devices plugged in after boot would not be usable. Before the name change, params.hibernation was false by default, so _dwc2_hcd_suspend() would skip entering hibernation. With the rename, _dwc2_hcd_suspend() was changed to use params.power_down to decide whether or not to enter partial power down. Since params.power_down is non-zero by default, it needs to be set to 0 for rockchip devices to restore functionality. This bug was reported in the linux-usb thread: REGRESSION: usb: dwc2: USB device not seen after boot The commit that caused this regression is: 6d23ee9caa6790aea047f9aca7f3c03cb8d96eb6 Signed-off-by: Hal Emmerich Acked-by: Minas Harutyunyan --- 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 bf7052e037d6..09292dc977e4 100644 --- a/drivers/usb/dwc2/params.c +++ b/drivers/usb/dwc2/params.c @@ -81,6 +81,7 @@ static void dwc2_set_rk_params(struct dwc2_hsotg *hsotg) p->host_perio_tx_fifo_size = 256; p->ahbcfg = GAHBCFG_HBSTLEN_INCR16 << GAHBCFG_HBSTLEN_SHIFT; + p->power_down = 0; } static void dwc2_set_ltq_params(struct dwc2_hsotg *hsotg) -- 2.11.0
[PATCH] usb: dwc2: disable power_down on rockchip devices
>From 04fbf78e4e569bf872f1ffcb0a6f9b89569dc913 Mon Sep 17 00:00:00 2001 From: Hal Emmerich Date: Thu, 19 Jul 2018 21:48:08 -0500 Subject: [PATCH] usb: dwc2: disable power_down on rockchip devices The bug would let the usb controller enter partial power down, which was formally known as hibernate, upon boot if nothing was plugged in to the port. Partial power down couldn't be exited properly, so any usb devices plugged in after boot would not be usable. Before the name change, params.hibernation was false by default, so _dwc2_hcd_suspend() would skip entering hibernation. With the rename, _dwc2_hcd_suspend() was changed to use params.power_down to decide whether or not to enter partial power down. Since params.power_down is non-zero by default, it needs to be set to 0 for rockchip devices to restore functionality. This bug was reported in the linux-usb thread: REGRESSION: usb: dwc2: USB device not seen after boot The commit that caused this regression is: 6d23ee9caa6790aea047f9aca7f3c03cb8d96eb6 Signed-off-by: Hal Emmerich Acked-by: Minas Harutyunyan --- 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 bf7052e037d6..09292dc977e4 100644 --- a/drivers/usb/dwc2/params.c +++ b/drivers/usb/dwc2/params.c @@ -81,6 +81,7 @@ static void dwc2_set_rk_params(struct dwc2_hsotg *hsotg) p->host_perio_tx_fifo_size = 256; p->ahbcfg = GAHBCFG_HBSTLEN_INCR16 << GAHBCFG_HBSTLEN_SHIFT; + p->power_down = 0; } static void dwc2_set_ltq_params(struct dwc2_hsotg *hsotg) -- 2.11.0
[PATCH] usb: dwc2: disable power_down on rockchip devices
>From 04fbf78e4e569bf872f1ffcb0a6f9b89569dc913 Mon Sep 17 00:00:00 2001 From: Hal Emmerich Date: Thu, 19 Jul 2018 21:48:08 -0500 Subject: [PATCH] usb: dwc2: disable power_down on rockchip devices The bug would let the usb controller enter partial power down, which was formally known as hibernate, upon boot if nothing was plugged in to the port. Partial power down couldn't be exited properly, so any usb devices plugged in after boot would not be usable. Before the name change, params.hibernation was false by default, so _dwc2_hcd_suspend() would skip entering hibernation. With the rename, _dwc2_hcd_suspend() was changed to use params.power_down to decide whether or not to enter partial power down. Since params.power_down is non-zero by default, it needs to be set to 0 for rockchip devices to restore functionality. This bug was reported in the linux-usb thread: REGRESSION: usb: dwc2: USB device not seen after boot The commit that caused this regression is: 6d23ee9caa6790aea047f9aca7f3c03cb8d96eb6 Signed-off-by: Hal Emmerich --- 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 f03e41879224..492607adc506 100644 --- a/drivers/usb/dwc2/params.c +++ b/drivers/usb/dwc2/params.c @@ -82,6 +82,7 @@ static void dwc2_set_rk_params(struct dwc2_hsotg *hsotg) p->host_perio_tx_fifo_size = 256; p->ahbcfg = GAHBCFG_HBSTLEN_INCR16 << GAHBCFG_HBSTLEN_SHIFT; + p->power_down = 0; } static void dwc2_set_ltq_params(struct dwc2_hsotg *hsotg) -- 2.11.0
Re: REGRESSION: usb: dwc2: USB device not seen after boot
I believe I have found the root cause of this issue.
Before commit 6d23ee9caa6790aea047f9aca7f3c03cb8d96eb6
|_dwc2_hcd_suspend() did not end up calling dwc2_enter_hibernation(),
which was renamed to dwc2_enter_partial_power_down() in the same commit.
_dwc2_hcd_suspend() skipped |
||dwc2_enter_partial_power_down() because of these lines:||
|if (!hsotg->params.hibernation){ goto skip_power_saving; } which now reads:
||if (!hsotg->params.power_down){ goto skip_power_saving; } The problem is,
params.power_down does not default to 0 like params.hibernation did so
|||dwc2_enter_partial_power_down() gets called|, the controller puts the port
into partial power down, and doesn't correctly leave when a device is plugged
in causing the port to be unusable. I've submitted a patch to the list to
address this on rockchip devices: [PATCH] usb: dwc2: disable power_down on
rockchip devices |||
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: dwc2: disable power_down on rockchip devices
>From 04fbf78e4e569bf872f1ffcb0a6f9b89569dc913 Mon Sep 17 00:00:00 2001 From: Hal Emmerich Date: Thu, 19 Jul 2018 21:48:08 -0500 Subject: [PATCH] usb: dwc2: disable power_down on rockchip devices The bug would let the usb controller enter partial power down, which was formally known as hibernate, upon boot if nothing was plugged in to the port. Partial power down couldn't be exited properly, so any usb devices plugged in after boot would not be usable. Before the name change, params.hibernation was false by default, so _dwc2_hcd_suspend() would skip entering hibernation. With the rename, _dwc2_hcd_suspend() was changed to use params.power_down to decide whether or not to enter partial power down. Since params.power_down is non-zero by default, it needs to be set to 0 for rockchip devices to restore functionality. This bug was reported in the linux-usb thread: REGRESSION: usb: dwc2: USB device not seen after boot The commit that caused this regression is: 6d23ee9caa6790aea047f9aca7f3c03cb8d96eb6 Signed-off-by: Hal Emmerich --- 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 f03e41879224..492607adc506 100644 --- a/drivers/usb/dwc2/params.c +++ b/drivers/usb/dwc2/params.c @@ -82,6 +82,7 @@ static void dwc2_set_rk_params(struct dwc2_hsotg *hsotg) p->host_perio_tx_fifo_size = 256; p->ahbcfg = GAHBCFG_HBSTLEN_INCR16 << GAHBCFG_HBSTLEN_SHIFT; + p->power_down = 0; } static void dwc2_set_ltq_params(struct dwc2_hsotg *hsotg) -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: REGRESSION: usb: dwc2: USB device not seen after boot
Replying to include I had with someone at synopsys, edited to remove their info as I don't think their email is public. > Hi Hal, > > Could you please apply this patch ([PATCH] usb: dwc2: Fix host exit from > hibernation flow.) from linux-usb mailing list. > > Let me know if this patch doesn't fix your issue. > > Regards, And what I sent back: > Hi, > > The patch did not fix my issue. > I don't think dwc2_host_enter_hibernation() is to blame, as it is never > called in any of my tests. > _dwc2_hcd_suspend() gets called, calling dwc2_enter_partial_power_down() > which calls the register backup functions. > After that, there are no more debug messages that refer to that > hsotg->dev until I plug a flash drive into the corresponding port. > When it is plugged in, I know dwc2_handle_common_inter() is called to > handle an interrupt, which calls dwc2_read_common_intr(), which prints > the > first of these two lines. > >>> dwc2 ff58.usb: gintsts=4001 gintmsk=f000 >>> dwc2 ff58.usb: Session request interrupt - lx_state=2 > Because gintsts=4001 or gintsts=0101 > dwc2_handle_common_inter() calls dwc2_handle_session_req_intr() > > if (gintsts & GINTSTS_SESSREQINT) > dwc2_handle_session_req_intr(hsotg); > > which prints the second line, then returns as the controller is not in > device mode. dwc2_handle_common_inter() leaving the usb device in a > limbo state. > > > without the commit I specified in my original message, this is what > plugging in a flash drive to the same port looks like: > > [ 32.745159] dwc2 ff58.usb: gintsts=0521 gintmsk=f3000806 > [ 32.749192] dwc2 ff58.usb: find > [ 32.856474] dwc2 ff58.usb: SetPortFeature > [ 32.856533] dwc2 ff58.usb: SetPortFeature - USB_PORT_FEAT_RESET > [ 32.856585] dwc2 ff58.usb: In host mode, hprt0=00021501 > [ 32.908063] dwc2 ff58.usb: gintsts=0521 gintmsk=f3000806 > [ 32.959694] dwc2 ff58.usb: DWC OTG HCD HUB STATUS DATA: Root port > status changed > [ 32.959748] dwc2 ff58.usb: port_connect_status_change: 0 > [ 32.959770] dwc2 ff58.usb: port_reset_change: 1 > [ 32.959790] dwc2 ff58.usb: port_enable_change: 0 > [ 32.959808] dwc2 ff58.usb: port_suspend_change: 0 > [ 32.959827] dwc2 ff58.usb: port_over_current_change: 0 > [ 32.970274] dwc2 ff58.usb: ClearPortFeature USB_PORT_FEAT_C_RESET > [ 33.021709] usb 2-1: new high-speed USB device number 2 using dwc2 > [ 33.024098] dwc2 ff58.usb: SetPortFeature > [ 33.024109] dwc2 ff58.usb: SetPortFeature - USB_PORT_FEAT_RESET > [ 33.024119] dwc2 ff58.usb: In host mode, hprt0=1101 > [ 33.024134] dwc2 ff58.usb: gintsts=0529 gintmsk=f3000806 > [ 33.076237] dwc2 ff58.usb: gintsts=0529 gintmsk=f3000806 > [ 33.137721] dwc2 ff58.usb: ClearPortFeature USB_PORT_FEAT_C_RESET > [ 33.207735] dwc2 ff58.usb: DWC OTG HCD HUB STATUS DATA: Root port > status changed > [ 33.207835] dwc2 ff58.usb: port_connect_status_change: 0 > [ 33.207874] dwc2 ff58.usb: port_reset_change: 0 > [ 33.207910] dwc2 ff58.usb: port_enable_change: 1 > [ 33.207945] dwc2 ff58.usb: port_suspend_change: 0 > [ 33.207979] dwc2 ff58.usb: port_over_current_change: 0 > [ 33.328256] dwc2 ff58.usb: DWC OTG HCD EP DISABLE: > bEndpointAddress=0x00, ep->hcpriv=e81e42ce > [ 33.328275] dwc2 ff58.usb: DWC OTG HCD EP DISABLE: > bEndpointAddress=0x00, ep->hcpriv= (null) > [ 33.328287] dwc2 ff58.usb: DWC OTG HCD EP RESET: > bEndpointAddress=0x00 > [ 33.347535] usb 2-1: New USB device found, idVendor=18a5, > idProduct=0302, bcdDevice= 1.00 > [ 33.347629] usb 2-1: New USB device strings: Mfr=1, Product=2, > SerialNumber=3 > [ 33.347720] usb 2-1: Product: STORE N GO > [ 33.347767] usb 2-1: Manufacturer: Verbatim > [ 33.347815] usb 2-1: SerialNumber: 9E457CF1 > [ 33.348067] dwc2 ff58.usb: DWC OTG HCD EP RESET: > bEndpointAddress=0x01 > [ 33.348072] dwc2 ff58.usb: DWC OTG HCD EP RESET: > bEndpointAddress=0x82 > [ 33.348224] usb-storage 2-1:1.0: USB Mass Storage device detected > [ 33.354164] scsi host1: usb-storage 2-1:1.0 > [ 33.354905] dwc2 ff58.usb: ClearPortFeature > USB_PORT_FEAT_C_ENABLE > [ 34.403547] scsi 1:0:0:0: Direct-Access Verbatim STORE N GO > 8.07 PQ: 0 ANSI: 4 > [ 34.414868] sd 1:0:0:0: [sdb] 15564800 512-byte logical blocks: (7.97 > GB/7.42 GiB) > [ 34.418173] sd 1:0:0:0: [sdb] Write Protect is off > [ 34.418270] sd 1:0:0:0: [sdb] Mode Sense: 23 00 00 00 > [ 34.418979] sd 1:0:0:0: [sdb] Write cache: disabled, read cache: > enabled, doesn't support DPO or FUA > [ 34.424606] sdb: sdb1 > [ 34.426850] sd 1:0:0:0: [sdb] Attached SCSI removable disk > > > Which the commit, the two lines I've included above are all that get > printed to dmesg. > All I can conclude is that gintsts and gintmsk are not in the correct >
REGRESSION: usb: dwc2: USB device not seen after boot
[1.] One line summary of the problem: Regression, USB devices not
recognized if plugged in after system boots
[2.] Full description of the problem/report:
If a usb device is plugged in before system begins booting into the
kernel, it will be recognized once boot completes.
However, if a usb device is plugged in once boot completes the
system will not recognized it, no messages are added to dmesg.
If CONFIG_USB_DWC2_DEBUG is set to yes, two messages are added to
dmesg when a usb device is plugged in:
dwc2 ff58.usb: gintsts=4001 gintmsk=f000
dwc2 ff58.usb: Session request interrupt - lx_state=2
##Reverting the merge made at commit
6d23ee9caa6790aea047f9aca7f3c03cb8d96eb6 (The main 4.17 merge) fixes the
issue##
[3.] Keywords (i.e., modules, networking, kernel): usb, dwc2, kernel, arm
[4.] Kernel information:
[4.1.] Kernel version (from /proc/version):
Linux version 4.17.6 (root@debian-build) (gcc version 5.4.1 20160919
(15:5.4.1+svn241155-1)) #1 SMP PREEMPT Thu Jul 12 21:40:35 CDT 2018
[4.2.] Kernel .config file:
[5.] Most recent kernel version which did not have the bug: 4.16.18
[6.] Output of Oops.. message (if applicable) with symbolic information
resolved (see Documentation/admin-guide/bug-hunting.rst)
[7.] A small shell script or example program which triggers the
problem (if possible)
[8.] Environment
[8.1.] Software (add the output of the ver_linux script here)
[8.2.] Processor information (from /proc/cpuinfo):
processor: 0
model name: ARMv7 Processor rev 1 (v7l)
BogoMIPS: 48.00
Features: half thumb fastmult vfp edsp thumbee neon vfpv3 tls vfpv4
idiva idivt vfpd32 lpae evtstrm
CPU implementer: 0x41
CPU architecture: 7
CPU variant: 0x0
CPU part: 0xc0d
CPU revision: 1
processor: 1
model name: ARMv7 Processor rev 1 (v7l)
BogoMIPS: 48.00
Features: half thumb fastmult vfp edsp thumbee neon vfpv3 tls vfpv4
idiva idivt vfpd32 lpae evtstrm
CPU implementer: 0x41
CPU architecture: 7
CPU variant: 0x0
CPU part: 0xc0d
CPU revision: 1
processor: 2
model name: ARMv7 Processor rev 1 (v7l)
BogoMIPS: 48.00
Features: half thumb fastmult vfp edsp thumbee neon vfpv3 tls vfpv4
idiva idivt vfpd32 lpae evtstrm
CPU implementer: 0x41
CPU architecture: 7
CPU variant: 0x0
CPU part: 0xc0d
CPU revision: 1
processor: 3
model name: ARMv7 Processor rev 1 (v7l)
BogoMIPS: 48.00
Features: half thumb fastmult vfp edsp thumbee neon vfpv3 tls vfpv4
idiva idivt vfpd32 lpae evtstrm
CPU implementer: 0x41
CPU architecture: 7
CPU variant: 0x0
CPU part: 0xc0d
CPU revision: 1
Hardware: Rockchip (Device Tree)
Revision:
Serial:
[8.3.] Module information (from /proc/modules):
vfat 20480 1 - Live 0xbf16f000
fat 65536 1 vfat, Live 0xbf158000
ath9k_htc 65536 0 - Live 0xbf14
ath9k_common 16384 1 ath9k_htc, Live 0xbf137000
ath9k_hw 360448 2 ath9k_htc,ath9k_common, Live 0xbf0d1000
ath 24576 3 ath9k_htc,ath9k_common,ath9k_hw, Live 0xbf0c7000
mac80211 561152 1 ath9k_htc, Live 0xbf00f000
autofs4 36864 2 - Live 0xbf00
[8.5.] PCI information ('lspci -vvv' as root) N/A
[8.6.] SCSI information (from /proc/scsi/scsi) N/A
[8.7.] Other information that might be relevant to the problem
(please look in /proc and include all information that you
think to be relevant):
[X.] Other notes, patches, fixes, workarounds:
Reverting the merge made at commit
6d23ee9caa6790aea047f9aca7f3c03cb8d96eb6 fixes the issue
Walking through the dmesg logs with and without that commit I found this
difference at the same point in the boot process:
dwc2 ff54.usb: In host mode, hprt0=1501
With the non-functional commit
dwc2 ff54.usb: In host mode, hprt0=1101
This difference corresponds to bit 10 being set in the hprt0 register,
but I can’t tell what this bits function is in hw.h
disabling power_down and lpm in dwc2_set_his_params in params.c
did not help.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
