Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757393AbcCaOar (ORCPT ); Thu, 31 Mar 2016 10:30:47 -0400 Received: from mga03.intel.com ([134.134.136.65]:2055 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757192AbcCaOap (ORCPT ); Thu, 31 Mar 2016 10:30:45 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,422,1455004800"; d="asc'?scan'208";a="76611711" From: Felipe Balbi To: Roger Quadros , John Youn Cc: "nsekhar\@ti.com" , "linux-usb\@vger.kernel.org" , "linux-kernel\@vger.kernel.org" Subject: Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit() In-Reply-To: <56FD2E13.2010903@ti.com> References: <1458133551-3071-1-git-send-email-rogerq@ti.com> <1458133551-3071-2-git-send-email-rogerq@ti.com> <87r3fah8h3.fsf@intel.com> <87lh5ih6hs.fsf@intel.com> <56EC5452.8030509@synopsys.com> <56F03FD7.30202@synopsys.com> <874mbzyq1l.fsf@intel.com> <56F3376E.8050305@synopsys.com> <87d1qkxtbb.fsf@intel.com> <56FC1E92.60403@synopsys.com> <56FD2E13.2010903@ti.com> User-Agent: Notmuch/0.21+96~g9bbc54b (http://notmuchmail.org) Emacs/25.0.90.3 (x86_64-pc-linux-gnu) Date: Thu, 31 Mar 2016 17:26:35 +0300 Message-ID: <87r3eqg1us.fsf@intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5866 Lines: 154 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Roger Quadros writes: >>>>>>>>>> We will need this function for a workaround. >>>>>>>>>> The function issues a softreset only to the device >>>>>>>>>> controller and performs minimal re-initialization >>>>>>>>>> so that the device controller can be usable. >>>>>>>>>> >>>>>>>>>> As some code is similar to dwc3_core_init() take out >>>>>>>>>> common code into dwc3_get_gctl_quirks(). >>>>>>>>>> >>>>>>>>>> We add a new member (prtcap_mode) to struct dwc3 to >>>>>>>>>> keep track of the current mode in the PRTCAPDIR register. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Roger Quadros >>>>>>>>> >>>>>>>>> I must say, I don't like this at all :-p There's ONE known silico= n which >>>>>>>>> needs this because of a poor silicon integration which took an IP= with a >>>>>>>>> known erratum where it can't be made to work on lower speeds and = STILL >>>>>>>>> was integrated without a superspeed PHY. >>>>>>>>> >>>>>>>>> There's a reason why I never tried to push this upstream myself ;= -) >>>>>>>>> >>>>>>>>> I'm really thinking we might be better off adding a quirk flag to= skip >>>>>>>>> the metastability workaround and allow this ONE silicon to set the >>>>>>>>> controller to lower speed. >>>>>>>>> >>>>>>>>> John, can you check with your colleagues if we would ever fall in= to >>>>>>>>> STAR#9000525659 if we set maximum speed to high speed during driv= er >>>>>>>>> probe and never touch it again ? I would assume we don't really f= all >>>>>>>>> into the metastability workaround, right ? We're not doing any so= rt of >>>>>>>>> PM for dwc3... >>>>>>>>> >>>>>> >>>>>> Hi Felipe, >>>>>> >>>>>> Do you mean to keep DCFG.speed to SS and set dwc->maximum_speed to H= S? >>>>>> I don't see an issue with that as long as we always ignore >>>>>> dwc->maximum_speed when programming DCFG.speed for all affected >>>>>> versions of the core. As long as the DCFG.speed =3D SS, you should n= ot >>>>>> hit the STAR. >>>>> >>>>> I actually mean changing DCFG.speed during driver probe and never >>>>> touching it again. Would that still cause problems ? >>>>> >>>> >>>> In that case I'm not sure. The engineer who would know is off until >>>> next week so I'll get back to you as soon as I can talk to him about >>>> it. >>> >>=20 >> So the engineers said that this issue can occur while set to HS and >> the run/stop bit is changed so it seems that won't work. > > Thanks John. > > Felipe, > > any suggestion how we can fix this upstream? no idea, I don't have a lot of memory about this problem. I really don't remember the details about this, is there an openly available errata document which I could read ? /me goes search for it. I found [1] which tells me, the following: | i819 | A Device Control Bit Meta-Stability for USB3.0 Controller i= n USB2.0 Mode | |-------------+------------------------------------------------------------= ----------------| | Criticality | Medium = | | | = | | Descritiion | When USB3.0 controller core is programmed to be a USB 2.0-o= nly device | | | hardware meta-stability on USB_DCTL[31]RUNSTOP bit causing = the core to | | | attempt high speed as well as SuperSpeed connection or comp= letely miss | | | the attach request. = | | | = | | Workaround | If the requirement is to always function in USB 2.0 mode, t= here is no | | | workaround. = | | | Otherwise, you can always program the USB controller core t= o be SuperSpeed | | | 3.0 capable (USB_DCFG[2:0]DEVSPD =3D 0x4) = | | | = | | Revisions | SR 1.1, 2.0 = | | Impacted | = | |-------------+------------------------------------------------------------= ----------------| So, TI's own documentation says that there is _no_ workaround. My question is, then: How are you sure that resetting the device actually solves the issue ? Did you really hit the metastability problem and noted that it works after a soft-reset ? How did you verify that Run/Stop was in a metastable state, considering that Run/Stop signal is not visible outside the die ? It seems to me that resetting the IP is just as "dangerous" as setting the IP to High-speed in the first place. No ? [1] http://www.ti.com/lit/er/sprz429h/sprz429h.pdf =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJW/TOcAAoJEIaOsuA1yqREycIP/iRYnKwj2vLpk/LUoHGsUNDF PaTlaWk0qC2XbM5EtCdypDFfvcJSv6KXn36hSkoabKUwGNAGCl2vppgF7xmjH76L 0NVDtkbNbSV1SWAKkuIuLjrf5pPCW2r4+ZYe0dUfNmc1kxD7sX+AS3HCjxHTOCyh SOshqE1nOxo2N7B5yKwVj3LgFpar7pLwYn9uPjVwip4A90fKWfqz78KvDJFqN5Kx oCVJjP8wM0Aj9EPxa792fQ64QoL/VO70bC7YLWEfwZBtarUQAZl61HC1aIWb21D3 LyyvZ5ObDvP0ES9Mg1sLg2SkGx/EB/9J0/24Z6phASTqC1szrC80J7cJcfaFI0Pg yc7YuwTcKoRC2mBV1VQxa6DN1er2yaTFgww5pU88gAlO9Ph2gSWRhVB6mHwiTpNU sjCN7TSipEQ36Or8bAsEPW7CoQ2HcXNKe+CybQ3OibjSnzvqKe2Wly9/7XGVEvZO MqzKrdTUH/kGzDIwmWuajqxE1ZZQv6fDgPOAw+1GxHLwNGVoLhX8yMpAPmPT386T Xo9EIoKHAfrAHXbE0+yatrao3tfpw9Heeen08SnZpvk2cKnc2zuc+R8ZqRRMBRiJ XFD6A5RFyjHJX/WV2YSLW4P/IyBGY/FXGBR41R1O28QPUi522MLpw2hk7qWDZiR2 Ophz4CpJgemfE0W4EpeP =O9Uo -----END PGP SIGNATURE----- --=-=-=--