From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932838AbdCaHrL (ORCPT ); Fri, 31 Mar 2017 03:47:11 -0400 Received: from mga11.intel.com ([192.55.52.93]:41670 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932765AbdCaHrK (ORCPT ); Fri, 31 Mar 2017 03:47:10 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,251,1486454400"; d="asc'?scan'208";a="72151055" From: Felipe Balbi To: Roger Quadros Cc: vivek.gautam@codeaurora.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 4/4] usb: dwc3: Workaround for super-speed host on dra7 in dual-role mode In-Reply-To: <8f7c0e29-2313-5d0a-ce5c-2c729c5f2b9d@ti.com> References: <1487250377-13653-1-git-send-email-rogerq@ti.com> <1487250377-13653-5-git-send-email-rogerq@ti.com> <87o9wlhd1j.fsf@linux.intel.com> <87d1d0l6f4.fsf@linux.intel.com> <8f7c0e29-2313-5d0a-ce5c-2c729c5f2b9d@ti.com> Date: Fri, 31 Mar 2017 10:46:59 +0300 Message-ID: <87zig1j3bg.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Roger Quadros writes: >>>> Roger Quadros writes: >>>>> dra7 OTG core limits the host controller to USB2.0 (high-speed) mode >>>>> when we're operating in dual-role. >>>> >>>> yeah, that's not a quirk. DRA7 supports OTGv2, not OTGv3. There was no >>>> USB3 when OTGv2 was written. >>>> >>>> DRA7 just shouldn't use OTG core altogether. In fact, this is the very >>>> thing I've been saying for a long time. Make the simplest implementati= on >>>> possible. The dead simple, does-one-thing-only sort of implementation. >>>> >>>> All we need for Dual-Role (without OTG extras) is some input for ID and >>>> VBUS, then we add/remove HCD/UDC conditionally and set PRTCAPDIR. >>>> >>> >>> The catch is that on AM437x there is no way to get ID and VBUS events o= ther >>> than the OTG controller so we have to rely on the OTG controller for th= at. :( >>=20 >> okay, so AM437x can get OTG interrupts properly. That's fine. We can >> still do everything we need using code that's already existing in dwc3 >> if we refactor it a bit and hook it up to the OTG IRQ handler. >>=20 >> Here's what we do: >>=20 >> * First we re-factor all necessary code around so the API for OTG/DRD >> is resumed to calling: >>=20 >> dwc3_add_udc(dwc); >> dwc3_del_udc(dwc); >> dwc3_add_hcd(dwc); >> dwc3_del_hcd(dwc); > > Why do we need these new APIs? don't these suffice? > dwc3_gadget_init(dwc); > dwc3_gadget_exit(dwc); > dwc3_host_init(dwc); > dwc3_host_exit(dwc); well, if they do what we want, sure. They suffice. >> the semantics of these should be easy to understand and you can >> implement each in their respective host.c/gadget.c files. >>=20 >> * Second step is to modify our dwc3_init_mode() (or whatever that >> function was called, sorry, didn't check) to make sure we have >> something like: >>=20 >> case OTG: >> dwc3_add_udc(dwc); >> break; >>=20 >> We should *not* add HCD in this case yet. >>=20 >> * After that we add otg.c (or drd.c, no preference) and make that call >> dwc3_add_udc(dwc) and, also, provide >> dwc3_add_otg(dwc)/dwc3_del_otg(dwc) calls. Then patch the switch >> statement above to: >>=20 >> case OTG: >> dwc3_add_otg(dwc); >> break; >>=20 >> Note that at this point, this is simply a direct replacement of >> dwc3_add_udc() to dwc3_add_otg(). This should maintain current behavior >> (which is starting with peripheral mode by default), but it should also >> add support for OTG interrupts to change the mode (from an interrupt >> thead) >>=20 >> otg_isr() >> { >>=20 >> /* don't forget to remove preivous mode if necessary */ >> if (perimode) >> dwc3_add_udc(dwc); >> else >> dwc3_add_hcd(dwc); >> } >>=20 >> * The next patch would be to choose default conditionally based on >> PERIMODE or whatever. >>=20 >> Of course, this is an oversimplified view of reality. You still need to >> poke around at PRTCAPDIR, etc. But all this can, actually, be prototyped >> using our "mode" debugfs file. Just make that call >> dwc3_add/del_udc/hcd() apart from fiddling with PRTCAPDIR in GCTL. > > We also need to ensure that system suspend/resume doesn't break. > Mainly if we suspend/resume with UDC removed. right, why would it break in that case? I'm missing something... >> Your first implementation could be just that. Refactoring what needs to >> be refactored, then patching "mode" debugfs to work properly in that >> case. Only add otg.c/drd.c after "mode" debugfs file is stable, because >> then you know what needs to be taken into consideration. >>=20 >> Just to be clear, I'm not saying we should *ONLY* get the debugfs >> interface for v4.12, I'm saying you should start with that and get that >> stable and working properly (make an infinite loop constantly changing >> modes and keep it running over the weekend) before you add support for >> OTG interrupts, which could come in the same series ;-) >>=20 > > Just to clarify debugfs mode behaviour. > > Currently it is just changing PRTCAPDIR. What we need to do is that if > dr_mode =3D=3D "otg", then we call dwc3_host/gadget_init/exit() according= ly as well. > > Does this make sense? it does. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAljeCXMACgkQzL64meEa mQb51g/8DMSXY4i7Is/WNn95XNoI0MhfnFGoCZ3ZxDVXHLDMtaGJX+jLMJ0skyW3 EXHl/PPIrwjb+d+c6jRV0nvTAc8ttsjNtPo12vbpmOeTw7M8yLgtVInl9+fSac7W 3T4o4IsY7UNiVcWCDHoiF1mUtirc8VL2PT51+wCeaGT0Qxj+vDU99VEg7zn5cvSi yo+HlN/zUP1eJoIDms6ijV2dkdvjqZGk/zDqCEthgn7ypaVZHPiA+1CytpN90nuP +Lc3CEpE5TCd0ZxNnjjMKPmD/iS6p9YhWEtBOhxYAzKk5uHVpkBGerHNV74hplEg bvMPVHkrerthYMhFfkQ7qB/sY0t624ceM1sQzsb9Q8CcWDFSRM3qQaCk6Bx2ByWG QdGp33qxd30Z8JcAqvEZbilzvWCDAPeRaUrOs4BFcpazmCxj8n7E7jMg5RVCDF5B SSDgBZCO8UomZXih0uvg8ut0xaJucfZHH2IvB44nd/UqJN8XE7ZAdNRvxHPWGpTU fx4IMKtTW/eKeGIxy1ziCiaR85Tcwu8xg3bEA8GkTmu+rDVDDXQZeUnCX/vxex9q huQJlrzrBtj2RiP+Cdf2nyqNvcfC8/RAj8eqJ4wNkBhhQhGv8iT7Z4AVgBgkefYY uEzlUkxh0kN2SuWJqxyrhQboxTDRJ1cVgu2SEEch65NR7sT3APo= =gA36 -----END PGP SIGNATURE----- --=-=-=--