From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932803AbdCaHnt (ORCPT ); Fri, 31 Mar 2017 03:43:49 -0400 Received: from fllnx210.ext.ti.com ([198.47.19.17]:47007 "EHLO fllnx210.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751394AbdCaHnr (ORCPT ); Fri, 31 Mar 2017 03:43:47 -0400 Subject: Re: [PATCH v2 4/4] usb: dwc3: Workaround for super-speed host on dra7 in dual-role mode To: Felipe Balbi 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> CC: , , From: Roger Quadros Message-ID: <8f7c0e29-2313-5d0a-ce5c-2c729c5f2b9d@ti.com> Date: Fri, 31 Mar 2017 10:43:39 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <87d1d0l6f4.fsf@linux.intel.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 29/03/17 13:32, Felipe Balbi wrote: > > 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 implementation >>> 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 other >> than the OTG controller so we have to rely on the OTG controller for that. :( > > 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. > > Here's what we do: > > * First we re-factor all necessary code around so the API for OTG/DRD > is resumed to calling: > > 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); > > the semantics of these should be easy to understand and you can > implement each in their respective host.c/gadget.c files. > > * 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: > > case OTG: > dwc3_add_udc(dwc); > break; > > We should *not* add HCD in this case yet. > > * 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: > > case OTG: > dwc3_add_otg(dwc); > break; > > 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) > > otg_isr() > { > > /* don't forget to remove preivous mode if necessary */ > if (perimode) > dwc3_add_udc(dwc); > else > dwc3_add_hcd(dwc); > } > > * The next patch would be to choose default conditionally based on > PERIMODE or whatever. > > 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. > > 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. > > 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 ;-) > Just to clarify debugfs mode behaviour. Currently it is just changing PRTCAPDIR. What we need to do is that if dr_mode == "otg", then we call dwc3_host/gadget_init/exit() accordingly as well. Does this make sense? cheers, -roger