From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754011AbbIIJZc (ORCPT ); Wed, 9 Sep 2015 05:25:32 -0400 Received: from mail-bn1bon0147.outbound.protection.outlook.com ([157.56.111.147]:28433 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751886AbbIIJZX (ORCPT ); Wed, 9 Sep 2015 05:25:23 -0400 Authentication-Results: spf=fail (sender IP is 192.88.168.50) smtp.mailfrom=freescale.com; vger.kernel.org; dkim=none (message not signed) header.d=none;vger.kernel.org; dmarc=none action=none header.from=freescale.com; Date: Wed, 9 Sep 2015 16:13:05 +0800 From: Peter Chen To: Roger Quadros CC: , , , , , , , , , , , Subject: Re: [PATCH v4 07/13] usb: otg: add OTG core Message-ID: <20150909081304.GM7802@shlinux2> References: <1440422484-4737-1-git-send-email-rogerq@ti.com> <1440422484-4737-8-git-send-email-rogerq@ti.com> <20150907012327.GG4914@shlinux2> <55ED6585.5050200@ti.com> <20150908083059.GD7802@shlinux2> <55EED3B5.5030806@ti.com> <20150909022116.GF7802@shlinux2> <55EFF6FA.1000705@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <55EFF6FA.1000705@ti.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-EOPAttributedMessage: 0 X-Microsoft-Exchange-Diagnostics: 1;BL2FFO11OLC012;1:5hG1w56yM+f1WgbDSuYh4X3Z1KJ01l1GuPZejZ3W54TyBiN1HfUe/tb+WNPS9IRamfVHljuAg5HXbIBduArfVblXZ54j3IDOhc3YDWsLSXzGa6mzAYpsrJv4x8aqlfIXqFaZCXf1EvZAQh9P7+9viwZporqHSJchVFgcS1eqjQoxUpA+iavKunrcBYtdJonK5e4GTVbVcmFAFtkbaYLCsaEGusvTR0xL515HR3efwJrH6Eooxw02wo8FFqaF5YdHVV2eDuyOarQFkgLj29ZbZj7qHrDGXBZpg8qwX0bLtIZFEmfOYdn/PO0k8rH2ugYPJkNZ+NFDFYxn1OIRLmtB6zQf9UH9OZXogufIAs8r6NFGLtC032hayjsBcZH5u44cCnIFVNVcwX9DGxUnNuix60FwNHtk64WjltqK6EuEi78= X-Forefront-Antispam-Report: CIP:192.88.168.50;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10019020)(6009001)(2980300002)(1109001)(1110001)(339900001)(199003)(479174004)(189002)(24454002)(104016003)(87936001)(64706001)(5007970100001)(86362001)(4001350100001)(46406003)(97736004)(11100500001)(50466002)(6806004)(81156007)(83506001)(4001540100001)(33716001)(47776003)(54356999)(76176999)(110136002)(5001860100001)(50986999)(85426001)(5001960100002)(105606002)(46102003)(5001830100001)(5001920100001)(62966003)(92566002)(106466001)(23726002)(93886004)(77156002)(2950100001)(77096005)(97756001)(33656002)(189998001)(68736005);DIR:OUT;SFP:1102;SCL:1;SRVR:DM2PR0301MB1230;H:tx30smr01.am.freescale.net;FPR:;SPF:Fail;PTR:InfoDomainNonexistent;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;DM2PR0301MB1230;2:kBGJnIKW/UX8pzAVzXoQun3hTehi2MiJDT94IYUzvg4AK1plllxSr1wWOQzkXiZPwPy78GhQ7cSHm0ULv+E0mqLiBkpFUzIAuK6D6US1jSGzZg6sic16kqSd+XrdPCYYqW8TdF14HrkpfmS59RzLlduRS4U5pTzsFGP8oSBfLWo=;3:mRXPpySnSZ6T8TTpzw2NrJfhvVWwa4AZ052Ya27r+9uU9nMLFruQ0msSncbi+mcGygxqNojuuzktJlDxyBSfOzR3u1+nivvTo+yWmbtarF+siT0wr2s9UZT6g7hf1UuKk3K5NTgm/jCCBCvbp6nrkji9oukAO4csDKRL15W0GsuD5Yoy4/4fKBav4VFWDY2tj0qNJ92XCupKi5LLVgpCQzxOxNS6RrqpkHvFEiCQsvg=;25:QGW6HKBaApHKx//qXpxoISG3hiNDUk1OeAXa8R9t0ROi7FrbC6toRAvQza7LniJsf2P5rzapoUvyFrK9mQ2Ytcq7MejfHmFjVm/RD27iSJV8Qsfgq+oX9mZ16LLo8qVMryRsU5EP4pY1HQQLGcdNE8cIZJ98U086KPGmnmPkGyYC/we9ge6i9ijsSIYro6kN9rQ/UqpAsRpOeYkMI9bGINeffgT96Q14l5zx1+dnjlbePtRbVnbKP90GvoyoUqdWmQW4H5U4emuXERX5MtzgCw== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DM2PR0301MB1230; X-Microsoft-Exchange-Diagnostics: 1;DM2PR0301MB1230;20:lqv7Eel3Lz+OAFQc6iep3/ULUwI0bu9ZQ0bjh4g6oOjkJec760L+SAfbsUoOqm02lKFsh546a25eKBkNnFzXkjbJ1wC3fq436Kw8qaI2EEgcrxDwto7DXHdbyB2ywNGQc7IdrWgk7yax2pqfeTRI60QOicabJjpwxtmJw/IaMmaaCyk1XEestkO4/toKNOelODkpudrHq/36zQZU7pdWYbdA567IR+derl3UkCI7WiCssmNdh9ApeAVhQRtYIdvMa3eRIVzlkI+kTdY7ouSEWDjUmFiJw0xtPT/MGayeE8nvO/kkpJ3cF8YqrwE5BIZlvzQeBzR+QgHATWf1tQYZz/KtreqTanPPWnod6YipPpA=;4:FiWH/bWN5HgH68RI8NHuyIuNMPRddhE5w72KpD26A4x+iBNlBhUbIubPGO9KvIqEL7qwsJUgRRMaNSjnITjuH4egPUJbf048h1+YvzumvDR6XXHkdSW1AXrfNN+2o2KtDLLvNsAbStseGf9N/EHu89yNxeygpphliG76jkGWVfCVNxY1thhtaHmzux/Xkm+qr3xKycmt/rdbzf5hebrLdKYkvR9mNPtvtQKwoRFyuD1tiojUjotfY+xbBUr/s5dFmAOSHLwyMq8u+L2kAktMZj7kC1LXzM3zcxiYj4/zqJu7Pi4hAL/ouyGL6m00jbZG X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(5005006)(8121501046)(3002001);SRVR:DM2PR0301MB1230;BCL:0;PCL:0;RULEID:;SRVR:DM2PR0301MB1230; X-Forefront-PRVS: 0694C54398 X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;DM2PR0301MB1230;23:koCZcdctKatxkexKRCHiKEWxFRwEElLvjumy9zm?= =?us-ascii?Q?W+6yhdQjaWWlF2I1L/bfFbuizYAoQch/ewNWtb2TDksj3x4Uejq19w2+PVIq?= =?us-ascii?Q?xk4bS0z0TTVPFdgTIrbMYX+Fvl1ETeInWv9WmtxRDr0JiPAFkb3Qf8gWuF7v?= =?us-ascii?Q?3d4Gv5JdPliPSY6mmWe80AEEhmvftCmVSHSbkRFfAF2wiJPf+fg813GCXEdX?= =?us-ascii?Q?GqMYD0HeNV9/cBL7Wengl1cxm/7kwpGC9P9MWxqBsbgzBGgVmBhGepdgCZ1m?= =?us-ascii?Q?yAfj7RV8XTS1QgOC9+u4YoaKaXFfLLnV8aB/fAKpZcL2UClf0rNRbZaDLTPl?= =?us-ascii?Q?KtoGV71QheY1pIFsA5Uaxcv1rY9URHmgY84IVjQMzAxxh0EivvOxfahygx3K?= =?us-ascii?Q?8UxxcDgbxykH7ELFXRCpId+wJehxxQ9GmtXe7/Bu8zIFrZQgPb31G42nLX8a?= =?us-ascii?Q?gZXXN5jufjL6p3oQUYkFEeHp12XkHJPg2U7TmckiKS0vJ6zK0huCFCtn/DS0?= =?us-ascii?Q?bZuFM/Mcl0mAclLpQNeFJMFpF+Z/5JYpn5zy2pBMQ8oZOBHtUlBAL2O+qVCX?= =?us-ascii?Q?S4NI0Daue+F9hsxgWhOfgU642ASTM4+wzRbgDeCsrA3LksmDR+/ElgR570rh?= =?us-ascii?Q?IIbjXUTqXL5yTjq/7qDsCHJINMmGCshNKwpMVKU38Q16w9m4bSy7TP+E/6Vx?= =?us-ascii?Q?3qLtGV7nc+bhc0AmPldoNt/wNwn6YsSgimUQLwEesB+DuZ5NWBmniIJ1Q8Bv?= =?us-ascii?Q?bZXkWYi5yqGFy9RlKYuzmeBs2uoTortkT7QYZnSTt4dXMnmojqOUxHjplZt9?= =?us-ascii?Q?lUhbUsQGZACR5taMECepExgKFTQ+6XuypqLvcDWvpeI67yQbqD/IOwl4V0va?= =?us-ascii?Q?U+tAaNZlilvqHPApC5Y5rdPGTtJuQI/t+JHtYk02Ln3h7E7P2NY2As6KsJ0p?= =?us-ascii?Q?vafhRidD4/1xD+lB8JDdDBUZygLWGIaq9ZAbRt7HY5yYRTGXwxYo87ApPgZv?= =?us-ascii?Q?LTfXsG4Uf9/Osb2QPSs4G1onEEvm1j3VIDV+Om6zvNdCHbEUcKIpl4fqsoGP?= =?us-ascii?Q?QnI0U2zp0phOp/h/PnUQU1VpGxdGFmAi7pbisjRJF4l7JyZiDYpwLZtdwEiG?= =?us-ascii?Q?wfiQM7NkLwBqx7eS83KRXSgTknmO5iKZ/EAowevrkZ3ImEwchOWBsWZroq1o?= =?us-ascii?Q?66J8C8RwUleX80kbYHIYs47OlUagVMkIVsqZrrNjROcLJ0dc8Af2QmrNXayV?= =?us-ascii?Q?Je5osnTgNMPqAwSbF7AJmmzjbM1YAofIo7JXZuJeW?= X-Microsoft-Exchange-Diagnostics: 1;DM2PR0301MB1230;5:j+7LBIRPcX+WoSBtsLk7XJfyb3c0077AKWMJ1CrQYU9I//P1phIy/ti7YlkfEy+FLOp36Zf8Eu3ZbrCSpz9CdE4ns+I2c6HSTJOgzRMsl3GE9QR74Elxo3yxPrMog5YTWQXo/Somvb4VpKesn5loCw==;24:NEcXq39cMOYC1m7Z+IctfuOu69GyA445SyFsVeWUr6/4yv3KeZ0u06NjpjnxpovgMk7m9RB85YuX18Amja1A7/rxkj/P6wyQvNeVcpOjyGQ=;20:f3d2kokQPsXxOR+nN+QSqkObAE6HquKzICs7Zz6isk6lzYEhV093psppXLfto6hYdslxRnOuM4ddJc9/Z1mEnA== X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Sep 2015 09:25:20.8850 (UTC) X-MS-Exchange-CrossTenant-Id: 710a03f5-10f6-4d38-9ff4-a80b81da590d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=710a03f5-10f6-4d38-9ff4-a80b81da590d;Ip=[192.88.168.50];Helo=[tx30smr01.am.freescale.net] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM2PR0301MB1230 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 09, 2015 at 12:08:10PM +0300, Roger Quadros wrote: > On 09/09/15 05:21, Peter Chen wrote: > > On Tue, Sep 08, 2015 at 03:25:25PM +0300, Roger Quadros wrote: > >> > >> > >> On 08/09/15 11:31, Peter Chen wrote: > >>> On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote: > >>>> On 07/09/15 04:23, Peter Chen wrote: > >>>>> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote: > >>>>>> + * This is used by the USB Host stack to register the Host controller > >>>>>> + * to the OTG core. Host controller must not be started by the > >>>>>> + * caller as it is left upto the OTG state machine to do so. > >>>>>> + * > >>>>>> + * Returns: 0 on success, error value otherwise. > >>>>>> + */ > >>>>>> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum, > >>>>>> + unsigned long irqflags, struct otg_hcd_ops *ops) > >>>>>> +{ > >>>>>> + struct usb_otg *otgd; > >>>>>> + struct device *hcd_dev = hcd->self.controller; > >>>>>> + struct device *otg_dev = usb_otg_get_device(hcd_dev); > >>>>>> + > >>>>> > >>>>> One big problem here is: there are two designs for current (IP) driver > >>>>> code, one creates dedicated hcd device as roothub's parent, like dwc3. > >>>>> Another one doesn't do this, roothub's parent is core device (or otg device > >>>>> in your patch), like chipidea and dwc2. > >>>>> > >>>>> Then, otg_dev will be glue layer device for chipidea after that. > >>>> > >>>> OK. Let's add a way for the otg controller driver to provide the host and gadget > >>>> information to the otg core for such devices like chipidea and dwc2. > >>>> > >>> > >>> Roger, not only chipidea and dwc2, I think the musb uses the same > >>> hierarchy. If the host, device, and otg share the same register > >>> region, host part can't be a platform driver since we don't want > >>> to remap the same register region again. > >>> > >>> So, in the design, we may need to consider both situations, one > >>> is otg/host/device has its own register region, and host is a > >>> separate platform device (A), the other is three parts share the > >>> same register region, there is only one platform driver (B). > >>> > >>> A: > >>> > >>> IP core device > >>> | > >>> | > >>> |-----|-----| > >>> gadget host platform device > >>> | > >>> roothub > >>> > >>> B: > >>> > >>> IP core device > >>> | > >>> | > >>> |-----|-----| > >>> gadget roothub > >>> > >>> > >>>> This API must be called before the hcd/gadget-driver is added so that the otg > >>>> core knows it's linked to an OTG controller. > >>>> > >>>> Any better idea? > >>>> > >>> > >>> A flag stands for this hcd controller is the same with otg controller > >>> can be used, this flag can be stored at struct usb_otg_config. > >> > >> What if there is another architecture like so? > >> > >> C: > >> [Parent] > >> | > >> | > >> |------------------|--------------| > >> [OTG core] [gadget] [host] > >> > >> We need a more flexible mechanism to link the gadget and > >> host device to the otg core for non DT case. > >> > >> How about adding struct usb_otg parameter to usb_otg_register_hcd()? > >> > >> e.g. > >> int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..) > >> > >> If otg is NULL it will try DT otg-controller property or parent to > >> get the otg controller. > > > > How usb_otg_register_hcd get struct usb_otg, from where? > > This only works when the parent driver creating the hcd has registered the > otg controller too. > Sorry? So we need to find another way to solve this issue, right? > > > >> > >>> > >>> Peter > >>> > >>> P.S: I still read your code, I find not all APIs in this file are used > >>> in your dwc3 example. > >> > >> Which ones? The ones for registering/unregistered host/gadget are used > >> by hcd/udc core as part of usb_add/remove_hcd() and > >> udc_bind_to_driver()/usb_gadget_remove_driver() > >> > > > > Ok, now I understand your design, usb_create_hcd must be called before > > the fsm kicks off. The call flow like below: > > > > usb_otg_register->usb_create_hcd->usb_add_hcd->usb_otg_register_hcd-> > > usb_otg_start_host->usb_otg_add_hcd > > Actually these are the discrete steps > - usb_otg_register > - usb_create_hcd > - usb_add_hcd->usb_otg_register_hcd (HCD is not really added till FSM in host role) > > Above 3 are prerequisite for FSM to start in addition to gadget controller and > driver being ready. > > When FSM enters in host role > - usb_otg_start_host(true)->usb_otg_add_hcd (Now HCD is added) > > When FSM exits host role > - usb_otg_start_host(false)->usb_otg_remove_hcd (Now HCD is removed, but not unregistered) > > If HCD hardware really goes away > - usb_remove_hcd->usb_otg_unregister_hcd (Now FSM stops as host resource not available) > > > > > We need to make some changes to let chipidea work since usb_create_hcd > > is included at host->start. > > > > Yes, you just need to do the usb_add_hcd() in probe > and usb_otg_start_host() during role switch. > Thanks. -- Best Regards, Peter Chen