Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757877AbcDGWMj (ORCPT ); Thu, 7 Apr 2016 18:12:39 -0400 Received: from smtprelay4.synopsys.com ([198.182.47.9]:33746 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751934AbcDGWMh (ORCPT ); Thu, 7 Apr 2016 18:12:37 -0400 Subject: Re: [RFT PATCH 0/4] usb: dwc2: Fix core reset and force mode delay problems To: Michael Niewoehner , John Youn References: <01515703-AA78-4E38-9F73-B5D491C2DF7F@mniewoehner.de> <5706A9F4.2000800@synopsys.com> <0D083742-37A2-4239-B318-66EFD3B50F65@mniewoehner.de> CC: "linux-usb@vger.kernel.org" , Doug Anderson , Stefan Wahren , Tao Huang , Julius Werner , "Greg Kroah-Hartman" , "linux-kernel@vger.kernel.org" , Caesar Wang , Heiko Stuebner , Felipe Balbi , "Remi Pommarel" , Kever Yang , "Przemek Rudy" From: John Youn Message-ID: <5706DB4F.9040903@synopsys.com> Date: Thu, 7 Apr 2016 15:12:31 -0700 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <0D083742-37A2-4239-B318-66EFD3B50F65@mniewoehner.de> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.9.139.55] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 11656 Lines: 236 On 4/7/2016 1:36 PM, Michael Niewoehner wrote: > > Am 07.04.2016 um 20:41 schrieb John Youn : > >> On 3/31/2016 2:44 PM, Michael Niewoehner wrote: >>> Hi John, >>> >>> Am 29.03.2016 um 04:36 schrieb John Youn : >>> >>>> Hi, >>>> >>>> The following patch series addresses the core reset and force mode >>>> delay problems we have been seeing on dwc2 for some platforms. >>>> >>>> I think I have identified the source of the inconsistencies between >>>> platforms and this series attempts to address them. >>>> >>>> Basically everything stems from the IDDIG debounce filter delay, which >>>> is a function of the PHY clock speed and can range from 5-50 ms if >>>> enabled. This delay must be taken into account on core reset and force >>>> modes. A full explanation is provided in the patch commit log and code >>>> comments. >>>> >>>> The first two patches are prerequisites to the force mode fixes, >>>> including one patch that was sent separately by Przemek Rudy. I have >>>> resubmitted it with this series for convenience. >>>> >>>> Please help by reviewing and testing on your platforms. >>>> >>>> Patches were tested on: >>>> * Synopsys HAPS platform IP 3.20a OTG, dr_mode=OTG,HOST,PERIPHERAL >>>> >>>> Regards, >>>> John >>>> >>>> John Youn (3): >>>> usb: dwc2: gadget: Only initialize device if in device mode >>>> usb: dwc2: Add delay to core soft reset >>>> usb: dwc2: Properly account for the force mode delays >>>> >>>> Przemek Rudy (1): >>>> usb: dwc2: do not override forced dr_mode in gadget setup >>>> >>>> drivers/usb/dwc2/core.c | 195 ++++++++++++++++++++++++++++---------------- >>>> drivers/usb/dwc2/core.h | 2 +- >>>> drivers/usb/dwc2/gadget.c | 30 +++++-- >>>> drivers/usb/dwc2/hcd.c | 6 +- >>>> drivers/usb/dwc2/hw.h | 1 + >>>> drivers/usb/dwc2/platform.c | 9 +- >>>> 6 files changed, 161 insertions(+), 82 deletions(-) >>>> >>>> -- >>>> 2.7.4 >>>> >>> >>> after applying your patch series on v4.6-rc1 usb keeps being broken on rk3188. >>> Besides that I get "dwc2 10180000.usb: dwc2_wait_for_mode: Couldn't set host mode“ repeatedly. >>> >>> Currently this works for me: >>> - Revert "usb: dwc2: Fix probe problem on bcm2835“ >>> - Apply "usb: dwc2: Add a 10 ms delay to dwc2_core_reset()" >>> >>> >>> Best regards >>> Michael >>> >> >> Thanks Michael. >> >> I won't be able to look at this again until next week. In the meantime >> could you provide a driver log? In particular I want to see the values >> of your GHWCFG registers, and where you are seeing the >> dwc2_wait_for_mode() failure. >> >> Regards, >> John > > Looks like the problem is gone on -rc2… on -rc1 the errors came up shortly after "dwc2 10180000.usb“ messages. > USB keeps being broken, though. The USB hub is detected but nothing that is attached to it. > > Here are the logs and register values for each test with Doug’s and your patches. > > Michael > > > good usb, Doug's patches > > [ 0.420125] usbcore: registered new interface driver usbfs > [ 0.426246] usbcore: registered new interface driver hub > [ 0.432296] usbcore: registered new device driver usb > [...] > [ 0.853769] 10180000.usb supply vusb_d not found, using dummy regulator > [ 0.860560] 10180000.usb supply vusb_a not found, using dummy regulator > [ 0.867365] dwc2 10180000.usb: Configuration mismatch. dr_mode forced to host > [ 0.977737] dwc2 10180000.usb: 128 invalid for host_nperio_tx_fifo_size. Check H. > [ 0.986562] dwc2 10180000.usb: 256 invalid for host_perio_tx_fifo_size. Check HW. > [ 1.047959] dwc2 10180000.usb: DWC OTG Controller > [ 1.052732] dwc2 10180000.usb: new USB bus registered, assigned bus number 1 > [ 1.059868] dwc2 10180000.usb: irq 24, io mem 0x00000000 > [ 1.065586] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002 > [ 1.072430] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1 > [ 1.079706] usb usb1: Product: DWC OTG Controller > [ 1.084432] usb usb1: Manufacturer: Linux 4.6.0-rc2+ dwc2_hsotg > [ 1.090390] usb usb1: SerialNumber: 10180000.usb > [ 1.096000] hub 1-0:1.0: USB hub found > [ 1.099884] hub 1-0:1.0: 1 port detected > [ 1.104668] 101c0000.usb supply vusb_d not found, using dummy regulator > [ 1.111428] 101c0000.usb supply vusb_a not found, using dummy regulator > [ 1.247968] dwc2 101c0000.usb: DWC OTG Controller > [ 1.252743] dwc2 101c0000.usb: new USB bus registered, assigned bus number 2 > [ 1.259879] dwc2 101c0000.usb: irq 25, io mem 0x00000000 > [ 1.265604] usb usb2: New USB device found, idVendor=1d6b, idProduct=0002 > [ 1.272447] usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1 > [ 1.279721] usb usb2: Product: DWC OTG Controller > [ 1.284448] usb usb2: Manufacturer: Linux 4.6.0-rc2+ dwc2_hsotg > [ 1.290404] usb usb2: SerialNumber: 101c0000.usb > [ 1.295951] hub 2-0:1.0: USB hub found > [ 1.299830] hub 2-0:1.0: 1 port detected > [ 1.305694] usbcore: registered new interface driver usb-storage > > > root@c0rock:~# cat debug/10180000.usb/regdump | grep GHWCFG > [ 1770.718938] dwc2 10180000.usb: Mode Mismatch Interrupt: currently in Host mode > [ 1770.729974] dwc2 10180000.usb: Mode Mismatch Interrupt: currently in Host mode > GHWCFG1 = 0x00006664 > GHWCFG2 = 0x22aa2450 > GHWCFG3 = 0x03cc00e8 > GHWCFG4 = 0xdbf04060 > root@c0rock:~# cat debug/101c0000.usb/regdump | grep GHWCFG > GHWCFG1 = 0x00000000 > GHWCFG2 = 0x22a7c856 > GHWCFG3 = 0x03380068 > GHWCFG4 = 0x09f04060 > > > ========================== > > > bad usb, your patches > > [ 0.420098] usbcore: registered new interface driver usbfs > [ 0.426218] usbcore: registered new interface driver hub > [ 0.432221] usbcore: registered new device driver usb > [...] > [ 0.854505] 10180000.usb supply vusb_d not found, using dummy regulator > [ 0.861295] 10180000.usb supply vusb_a not found, using dummy regulator > [ 0.868126] dwc2 10180000.usb: Configuration mismatch. dr_mode forced to host > [ 0.917648] dwc2 10180000.usb: dwc2_wait_for_mode: Couldn't set host mode > [ 0.924472] dwc2 10180000.usb: 128 invalid for host_nperio_tx_fifo_size. Check H. > [ 0.933314] dwc2 10180000.usb: 256 invalid for host_perio_tx_fifo_size. Check HW. > [ 0.977914] dwc2 10180000.usb: DWC OTG Controller > [ 0.982687] dwc2 10180000.usb: new USB bus registered, assigned bus number 1 > [ 0.989827] dwc2 10180000.usb: irq 24, io mem 0x00000000 > [ 0.995543] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002 > [ 1.002390] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1 > [ 1.009666] usb usb1: Product: DWC OTG Controller > [ 1.014391] usb usb1: Manufacturer: Linux 4.6.0-rc2+ dwc2_hsotg > [ 1.020347] usb usb1: SerialNumber: 10180000.usb > [ 1.025937] hub 1-0:1.0: USB hub found > [ 1.029819] hub 1-0:1.0: 1 port detected > [ 1.034605] 101c0000.usb supply vusb_d not found, using dummy regulator > [ 1.041367] 101c0000.usb supply vusb_a not found, using dummy regulator > [ 1.048480] dwc2 101c0000.usb: DWC OTG Controller > [ 1.053251] dwc2 101c0000.usb: new USB bus registered, assigned bus number 2 > [ 1.060389] dwc2 101c0000.usb: irq 25, io mem 0x00000000 > [ 1.066121] usb usb2: New USB device found, idVendor=1d6b, idProduct=0002 > [ 1.072972] usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1 > [ 1.080251] usb usb2: Product: DWC OTG Controller > [ 1.084978] usb usb2: Manufacturer: Linux 4.6.0-rc2+ dwc2_hsotg > [ 1.090935] usb usb2: SerialNumber: 101c0000.usb > [ 1.096504] hub 2-0:1.0: USB hub found > [ 1.100387] hub 2-0:1.0: 1 port detected > [ 1.106271] usbcore: registered new interface driver usb-storage > > > root@c0rock:~# cat debug/10180000.usb/regdump | grep GHWCFG > [ 224.592330] dwc2 10180000.usb: Mode Mismatch Interrupt: currently in Host mode > [ 224.603277] dwc2 10180000.usb: Mode Mismatch Interrupt: currently in Host mode > GHWCFG1 = 0x00006664 > GHWCFG2 = 0x22aa2450 > GHWCFG3 = 0x03cc00e8 > GHWCFG4 = 0xdbf04060 This controller is OTG with the IDDIG filter. So it will default to device on every reset and there will be a delay before it goes into host mode. Could you try to increase the timeout to see if it helps anything? Patch is below. > root@c0rock:~# cat debug/101c0000.usb/regdump | grep GHWCFG > GHWCFG1 = 0x00000000 > GHWCFG2 = 0x22a7c856 > GHWCFG3 = 0x03380068 > GHWCFG4 = 0x09f04060 Looks like this one is a host-only controller so it should work fine, correct? Regards, John ---->8---- diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c index ab25620..c72f4e5 100644 --- a/drivers/usb/dwc2/core.c +++ b/drivers/usb/dwc2/core.c @@ -262,15 +262,15 @@ static void dwc2_wait_for_mode(struct dwc2_hsotg *hsotg, { ktime_t start; ktime_t end; + s64 ms = 0; dev_vdbg(hsotg->dev, "Waiting for %s mode\n", host_mode ? "host" : "device"); start = ktime_get(); + timeout = 300; while (1) { - s64 ms; - if (dwc2_is_host_mode(hsotg) == host_mode) { dev_vdbg(hsotg->dev, "%s mode set\n", host_mode ? "Host" : "Device"); @@ -288,6 +288,9 @@ static void dwc2_wait_for_mode(struct dwc2_hsotg *hsotg, usleep_range(1000, 2000); } + + dev_info(hsotg->dev, "%s: mode=%d, time=%lld ms\n", __func__, + dwc2_is_host_mode(hsotg), ms); }