From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755728AbcFHDEr (ORCPT ); Tue, 7 Jun 2016 23:04:47 -0400 Received: from mail-db3on0095.outbound.protection.outlook.com ([157.55.234.95]:31296 "EHLO emea01-db3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751791AbcFHDEp convert rfc822-to-8bit (ORCPT ); Tue, 7 Jun 2016 23:04:45 -0400 From: Jun Li To: Felipe Balbi , Roger Quadros , Lu Baolu , Peter Chen CC: Mathias Nyman , Greg Kroah-Hartman , Lee Jones , "Heikki Krogerus" , Liam Girdwood , Mark Brown , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: RE: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Thread-Topic: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Thread-Index: AQHRvG9p/Bw7HKLasE2LJUprPzQksJ/XXbyAgACNEACAAK3vAIAB3QMAgAE2I4CAABueAIAAQmoAgAACU6CAAb/KgIAAM4+AgAAByYCAABBYAIAAEXWAgADFAHA= Date: Wed, 8 Jun 2016 03:04:41 +0000 Message-ID: References: <1464831449-8973-1-git-send-email-baolu.lu@linux.intel.com> <1464831449-8973-3-git-send-email-baolu.lu@linux.intel.com> <20160603074113.GA30006@shlinux2> <5751AAEE.2090001@linux.intel.com> <20160604022838.GA26936@shlinux2> <5753CCFC.2060504@linux.intel.com> <20160606012557.GA16012@shlinux2> <5754E850.1020707@linux.intel.com> <57552006.7080108@ti.com> <5756999B.9020809@linux.intel.com> <5756C4DB.1050400@ti.com> <87a8ixb15w.fsf@linux.intel.com> <5756D410.30003@ti.com> <874m95avka.fsf@linux.intel.com> In-Reply-To: <874m95avka.fsf@linux.intel.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=jun.li@nxp.com; x-originating-ip: [199.59.225.131] x-ms-office365-filtering-correlation-id: b1e98ac5-9dfd-4537-44c5-08d38f49a2b9 x-microsoft-exchange-diagnostics: 1;AM4PR04MB2130;6:gPTuLlN+iipyEFtzJ64XuPZ2xai3m7PsTYYWkLGFftysg70p5VhlfdxIXpdvVXjx74m7Q3Vsef6bdiD/+3ELnwB6Ga7GXKSfoRZOPBR8wVy1/9FJTVH8OrkiGf600HVg2VdQ9GOj7L/ywFlk/VuXnMjwgMB4b+64IRO73HEIndGvaW2tIdTEl9DaJIZ/H4r0N8jWHfI3ecKHRPutSA4igMjm07jHCGZxXEOhHLPoLcAXtwi/SK60xAGwsqSBFilnC3EwkXHe+ZQ78NEocE3i4PnkMJNf9s5OKwx+gYXM9VYTMMUeq5ob9O1nF7KxCrMbPaNptrv2dStpk0rrQe0pVQ==;5:wQEwRat6k/acxxgEYCh8OydqC6L6agPNP3XyMk5f07NKQdjjJnaXZfsy9/xmWviC1oWZ43wZitr4nHLuTEqHIdET3RggwN1V/4gbJFG9gsQKPZe8XYKmEhloMMZhjdR8rkcW8vISrKonOaFwzXM74A==;24:lwZRB+w8lBooJBIfusGSPhUN2D/iDf43vK5hdgG2oRLyC7x1RxRiBUgQjmnQ3ZpmCMI6Oy3aYEWXuM/WMdS60zqJLMsKapUeO6D5KAQIGRM=;7:dhv5ZjWuGqPY8RvnNQFDloN5fOoyWFWD9ZrZoBfR877epRda9h+7TbgYuozk3y4ZanhFp1jBitJqisgPE7wKhAM64xETdrQR5MplozQZiX0vEDxLNCz0HfzNEC3ny9l/J0qEHTeusmC321JBHT0PLNR/gjLi56HMc5MVyf5B8q3ihH5cIppdmCohpPHGtOT/SDiFUiaHs3X3aQmeNTWU8e1e8vdkNu8Yb5vdbnc+rf4= x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AM4PR04MB2130; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(31051911155226)(9452136761055)(185117386973197)(228905959029699); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6055026);SRVR:AM4PR04MB2130;BCL:0;PCL:0;RULEID:;SRVR:AM4PR04MB2130; x-forefront-prvs: 0967749BC1 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(6009001)(189002)(199003)(13464003)(377454003)(8676002)(66066001)(81166006)(5002640100001)(9686002)(81156014)(4326007)(101416001)(97736004)(5001770100001)(93886004)(77096005)(2900100001)(2950100001)(586003)(2906002)(92566002)(3846002)(8936002)(102836003)(19580395003)(3660700001)(68736007)(3280700002)(5003600100002)(6116002)(19580405001)(11100500001)(189998001)(74316001)(86362001)(122556002)(76576001)(87936001)(33656002)(5004730100002)(5008740100001)(10400500002)(106116001)(50986999)(54356999)(76176999)(105586002)(106356001)(41533002);DIR:OUT;SFP:1101;SCL:1;SRVR:AM4PR04MB2130;H:AM4PR04MB2130.eurprd04.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;CAT:NONE;LANG:en;CAT:NONE; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Jun 2016 03:04:41.3400 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR04MB2130 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, > -----Original Message----- > From: Felipe Balbi [mailto:felipe.balbi@linux.intel.com] > Sent: Tuesday, June 07, 2016 11:05 PM > To: Roger Quadros ; Lu Baolu ; > Jun Li ; Peter Chen > Cc: Mathias Nyman ; Greg Kroah-Hartman > ; Lee Jones ; Heikki > Krogerus ; Liam Girdwood > ; Mark Brown ; linux- > usb@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port > mux > > > Hi, > > Roger Quadros writes: > >> I might be able to find some time to implement a proof of concept > >> which would allow your platforms to get dual-role with code we > >> already have, but I need DWC3's OTG support which, I'm assuming, you > >> already have :-) > >> > >> If you wanna try something offline, just ping me ;-) I'll be happy to > >> help. > > > > What you are proposing is a dwc3 only solution. With the otg/dual-role > > series we are trying to be generic as much as possible. > > Well, if there is a need for that, sure. Take MUSB for instance. It makes > use of nothing of the sorts, because it doesn't have to. > > > Whether controller drivers want to use it or not is upto the driver > > maintainers but we should at least ensure that user space ABI if any, > > is consistent across different implementations. > > Role decisions should not be exposed to userspace unless as debug feature > (using e.g. DebugFS). That should be done either by the HW or within the > kernel. In many cases the role decision is made by usersapce, this also should be covered. This patchset also expose it to userspace but I think it isn't for debug: /sys/bus/platform/devices/.../portmux.N/state Li Jun > > If we're discussing userspace ABI here, there's something very wrong with > OTG/DRD layer design. > > >>> How are you switching the port mux between host and peripheral? Only > >>> by sysfs or do you have a GPIO for ID pin as well? > >> > >> depends. Some SoCs have GPIO-controller muxes while some just have > >> mux's select signals (one for ID, one for VBUS) mapped on xHCI's > >> address space. > >> > >>> What happens to the gadget controller when the port is muxed to the > >>> host controller? Is it stopped or it continues to run? > >> > >> it continues running, but that's pretty irrelevant for Intel's > >> dual-role > > > > Isn't that unnecessary waste of power? Or you have firmware assisted > > low power mode? > > that's an implementation detail which brings nothing to this discussion, > right? :-) > > We can, certainly, put the other side to D3. > > >> setup. We have an actual physical (inside the die, though) mux which > >> muxes USB signals to XHCI (not DWC3's XHCI) or to a peripheral-only > >> DWC3. > >> > > > > Probably irrelevant for Intel's dual-role but many platforms that > > share the port can't have device controller running when port is in > > host mode and vice versa. > > but that doesn't mean we need an entire new layer added to the kernel > ;-) > > DWC3 already gives us all the information necessary to make a decision on > which role we should assume. Just consider your options. Here's how things > would look like without any OTG/DRD layer: > > -> DWC3 OTG IRQ > -> readl(OSTS); > -> if (OSTS & BIT(4)) > -> dwc3_host_exit(); __dwc3_gadget_start(); > -> else > -> __dwc3_gadget_stop(); dwc3_host_init(); > > Can you draw something similar for your proposed OTG/DRD layer? > > I remember there were at least two schedule_work(). IIRC it looked > something like below: > > -> DWC3 OTG IRQ > -> readl(OSTS); > -> if (OSTS & BIT(4)) > -> otg_set_mode(PERIPHERAL); > -> schedule_work(); > -> otg_ops->stop_host(); > -> usb_del_hcd(); > -> otg_ops->start_peripheral(); > -> usb_gadget_add_udc(); > -> else > -> otg_set_mode(HOST); > -> schedule_work(); > -> otg_ops->stop_peripheral(); > -> usb_gadget_del_udc(); > -> otg_ops->start_host(); > -> usb_add_hcd(); > > I'm probably missing some steps there. > > > So there has to be a central point of control where the respective > > controllers are started/stopped. > > some implementations might need this, yes. DWC3 and MUSB don't seem to be > this type of system. > > > That is the other point we are trying to address with the common > > otg/dual-role code. > > > > Even in the TI dwc3 implementation we use dwc3's XHCI so I guess we > > need to stop the host controller for device mode, right? > > yes, see above. We already have that code. > > > If so then who will deal with start/stop of the controllers then? > > dwc3 itself. > > -- > balbi