From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751353AbbIKDDK (ORCPT ); Thu, 10 Sep 2015 23:03:10 -0400 Received: from mail-bn1bn0106.outbound.protection.outlook.com ([157.56.110.106]:53600 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751007AbbIKDDH (ORCPT ); Thu, 10 Sep 2015 23:03:07 -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: Fri, 11 Sep 2015 09:50:41 +0800 From: Peter Chen To: Roger Quadros CC: , , , , , , , , , , , Subject: Re: [PATCH v4 07/13] usb: otg: add OTG core Message-ID: <20150911015040.GB17405@shlinux2> References: <20150908083059.GD7802@shlinux2> <55EED3B5.5030806@ti.com> <20150909022116.GF7802@shlinux2> <55EFF6FA.1000705@ti.com> <20150909081304.GM7802@shlinux2> <55EFFCE0.5070109@ti.com> <20150909084544.GO7802@shlinux2> <55F0083E.6050802@ti.com> <20150910053507.GA19400@shlinux2> <55F19100.3020209@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <55F19100.3020209@ti.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-EOPAttributedMessage: 0 X-Microsoft-Exchange-Diagnostics: 1;BN1BFFO11FD019;1:EJzZmxj16GgCTaaxGCt5qf3ODrAsuAm7Uvu1nOco702L0HGBckyqB7raVrtr/bgUToj6b5+TY7qUV8FoAxae9MgdWF3gqJUAl3LMyAlLy5FqXGmfWMjEnE8Aml+ylCpZkqxehxS6W+dQ94CEeZ28WiREYPdi8nuwIlBwZu8kLH6Hm8xx3Lzsq2WtjXO5UvFk62Ib2RAXMerkh0YDsJeYviann+K+Li2EGFGLthOhx+Ki3g4vOFPmc79OV35DoEztgEZbLhOj8hpdlODhSXD1vrzHa7rJ6hQD6dOMymGJR29lhQ/XcqeTOdWwSxHm5LrhiQdYYJDOnh4uo0Pko5900qxhd1tHpRD/hDlSVQelo/jyoW8aSM7x2HFnaXRa3l3ZOEOarKW3OybtmCWcKHsVIhr14moR9rf9MNCzYqRZ3pw= 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)(24454002)(479174004)(189002)(5001960100002)(83506001)(86362001)(6806004)(5001830100001)(33656002)(110136002)(76176999)(54356999)(105606002)(85426001)(104016003)(93886004)(50466002)(50986999)(46406003)(189998001)(68736005)(106466001)(81156007)(47776003)(64706001)(5001920100001)(97736004)(23726002)(33716001)(97756001)(92566002)(4001350100001)(62966003)(87936001)(4001540100001)(2950100001)(5001860100001)(77096005)(77156002)(5007970100001)(46102003);DIR:OUT;SFP:1102;SCL:1;SRVR:CY1PR0301MB1228;H:tx30smr01.am.freescale.net;FPR:;SPF:Fail;PTR:InfoDomainNonexistent;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;CY1PR0301MB1228;2:8FiSKi5pouzX4m7fLBNGE6Llc4CDBu2hj33nvhVlMzcUdEPnrysPG4dy6zCMTyekl/l90s86uE+xP6GCFRG1XLBtm38RxRBKT0dpJeVO8ithsMM/qdbXsBSPVac5JQfRGH5MbTBRWVUJ2CYJBorYRiI29tgw4VxeQTiIYuK0cBs=;3:Flz5dbsoYyUr9asfY6uG1nlWCCWGZjNZIQNhSfD1OCFWQVSf+R8VQVbL/n3Xa2LdUu5QfEsB6VhnwtNvphe4fcE05XJvu9elaymIgUZNCm+Ui6zCy9j5/ubWSE6/y4vpFs/yfrAbfZh9AOvjt4DcuuZbjn7HgzQTb6D64zPBOHdrGWLHmQd+f4nckY4G70eUWLJ46KeC6qoFu78Dnl7OL3kYJqiPBX5rAYteQAN4ZvM=;25:1KWC5XGMC4eYjwy/Weu3xcNNXCtU0wXRiEVEwreuH6M/br+whIhO7EFDwoyTme3wBRmSYQPZp0qdnuBj3SAxITpDIjSgpSfqbx3VWGbf/5uTBDBxeQ5EPGXYD4fMfKKBjJqYJU3eyPGBB74J20X3I1qXBGzBuydSux//nCjfbLbAfSiie8oaNrilZ/LJmCLIY0JuQo2hVwI8UDRQDNkMgqC4iPuHQjZym+2qLUWHs7Y/elCwru4Dqrpw2m5gqj/4P3ORvpx2k9gUibNZ7svnzQ== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:CY1PR0301MB1228; X-Microsoft-Exchange-Diagnostics: 1;CY1PR0301MB1228;20:3H823P0uDdiZaYJ+pRk3goGLdzOQ9hW2CT0wxJ4oFZGhpVIuvOqab/Tf3iUmKh8nALQNNYOgmurVVcnVTjT8M382RThPAQCKmx/UPOe7n24fOswd3vfS53IM5DsOicyP/e7JFnLoHQdMJY387H+FfJcgBaQxo8WicXSWib+13zFBI6NwvWLU4EXzU8TCy8ZrHphqnXsT549E6yp/0es+NGvkpEL59ETzM3gM312iOfb7BQq6lvuJ7nr7c24nBXnBRSaoyLZm2OD0cuU/OI6c+CvXeVz6qyTjU99d6dp+MYJYqwzGufJhbrJLeoCHSPZtf27H42JHtaLITXoAOan/nyOITQvknWhH/FDTEIfb9mo=;4:4QUF7HgGmC5Isb/T3dBbub/l5l62Fi8yRn5IwIzu0pVUN3gmPU1Qf07jeOJQVKAH/vh69d5I+yRp6WR/iTgM6WNWi9a1FeHO5CnoatOHKWBIgPCAyFPDc/v73I5jNZcvCoz10YdYZrbm6kbf5OC2M6b/gNuC0ZnYiGEkTZ80LqHOBB1/oQfH2AQIAxbre6LaKMXEetT+jP+vzUvt5aisSivUEQYXpeZyONwYOssxX4tgkJMfdisQEhKfQ8zjLxFoNh0uQenMgCL0HrFcpbkEtp48OS9beky4Wy6dvdURSE5atX6YB51oR5uCb2uS9bmv X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(8121501046)(5005006)(3002001);SRVR:CY1PR0301MB1228;BCL:0;PCL:0;RULEID:;SRVR:CY1PR0301MB1228; X-Forefront-PRVS: 06968FD8C4 X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;CY1PR0301MB1228;23:Xm2cG3KHOzfMn7CAT5PzBQtf2k0VG70WqiUT4t4?= =?us-ascii?Q?0G1nnXAKQMTiI9XZbrkk/q7zMuPuS5g3UttAltDnSjMDGltmsZ3oAcslEZlO?= =?us-ascii?Q?25bsgcPRMybOL1rQyU3bG0JL7gZ3GUDxdFed70ycD8MN10LPBFPPscHENVkI?= =?us-ascii?Q?JiAYUVLYk2pG8z4i6qgSKzSWEtzyKA7SF/Nijf2fjYNJVb7bOBWpRNHUDTQJ?= =?us-ascii?Q?UBN56uJ/RZ1LYw57fkKo0OKPETzTCCNTBQv6w2zYT/psb8HZfyct5P0Ap2yw?= =?us-ascii?Q?dvhUxCRV85UsWViKMTx3lJ9IgfFvjYIGmST9MF4UVmGXeevKdCtMKxthtoaa?= =?us-ascii?Q?b4trEcsygRvXKNOGrzz5Frzxs7YG14B32lDuMIbwF1p6D1c39r5EKtQLzivp?= =?us-ascii?Q?h0n3uelfcRhADNRF3WCn2sveobk1KxUWVPYVBcs5DkQC7e+AvEU/ckpiE2UJ?= =?us-ascii?Q?/335mMgvjXpiyP30naEhQr/0gUvy4dtiWOoT/CR+WaU9SfZMbyy1Og5MFtcq?= =?us-ascii?Q?IRLCYRK2ZdIbso573YPwzWUqMT3NLyOfitVx5n+YxXbsmp7DFd0d2Eu7dspI?= =?us-ascii?Q?zsiJ00Udo+Cr/MjIwVwNzr5GId3BOmRI+3E/Jl36f8yyj0+nYBhNilYYf+yY?= =?us-ascii?Q?bKpfI2T5iQ4Z7vOmRS1zNsT+dy6sJJRgY+1uboDBcp6Ud4Uw3KY7KcyAZ6+S?= =?us-ascii?Q?wQhA95GxDUy638ocGNdE//xm6oVwz6LgdtdGhYeim63Hum5tG5qlf6G66Q3n?= =?us-ascii?Q?mEGoN9jpzP+0rPF0Pc78OQap3K7rphY/wp4w6z2So3LfbG+7RnXbnMN9f/XS?= =?us-ascii?Q?8DxRkQd+VQyLrDDrTJWLwnaPs0KAj4cGgQppn9RHeDL+yx9QcglKuwSXil5x?= =?us-ascii?Q?8KBtp/VKi2TU1UqoFt69oGREFyB0dJL5UeErlVROI/U8YbKnN6E3jH0nVxHK?= =?us-ascii?Q?guU4XHI7AGlqgecVms1rcfYPIzdBrOiVTHlKHpV76eDVW+VoWK8ELYeHMXBN?= =?us-ascii?Q?kKqIpY7Z+MDgxP6DFq+lMlV9vH8lweYHMsECgctWi9wDS4pZpPq/+VL1sB1W?= =?us-ascii?Q?N7EpI9vHaNWkP8xNtwHs3O8drwwwuFs00J1qc+6ftUhYP9IzId0bR4418TPw?= =?us-ascii?Q?PqTqAcnklSjWr0e9epS3iKppfcwfGhb6YGcAr4ZPvurNfPqlAUu8EXNvO2DQ?= =?us-ascii?Q?Z9HqR8P2tsWy1Wn64Cg8gmVie9vnzBNbJ3Yg+dG+HyZx3WciFZlAF2oUKbx/?= =?us-ascii?Q?XGIpTWy9NqyS4FrCXXoc=3D?= X-Microsoft-Exchange-Diagnostics: 1;CY1PR0301MB1228;5:jy5kM8IVxTe2gPFVJeDAEdHXxI60L98osLqkoXc78/UjLcH7sbbvd/UTwPlu43xsb6t6/kDBhN3xu+u664lH9HBSV6R1R9lAs+qnCH7Q1Y6ck0BAZHKgrkDGRlablJ84hI249GDGCiFHZycvCAVe1A==;24:criBrGFnc3R5wfXxXdnHaB40TJ4AyINEBN7757QI+PBNCocHxEaxoVFRVkAO2AN4izqMi+f+Ttf1QioHGHhG3Wnku5kvEVPdgttrmFhXYKA=;20:1/sMEnK2PQDMzRHF3Z46OQjan9Aus/PMKBxR3aKJAYcScDMr/2ZQ5VPjKWzXFpSKqn08ST3rbBf0j7PxY4Q9Xg== X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Sep 2015 03:03:01.8919 (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: CY1PR0301MB1228 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Sep 10, 2015 at 05:17:36PM +0300, Roger Quadros wrote: > On 10/09/15 08:35, Peter Chen wrote: > > On Wed, Sep 09, 2015 at 01:21:50PM +0300, Roger Quadros wrote: > >> On 09/09/15 11:45, Peter Chen wrote: > >>> On Wed, Sep 09, 2015 at 12:33:20PM +0300, Roger Quadros wrote: > >>>> On 09/09/15 11:13, Peter Chen wrote: > >>>>> 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? > >>>> > >>>> For existing cases this is sufficient. > >>>> The otg device is either the one supplied during usb_otg_register_hcd > >>>> (cases B and C) or it is the parent device (case A). > >>> > >>> How we differentiate case A and case B at usb_otg_register_hcd? > >>> Would you show me the sample code? > >> > >> Case A: > >> > >> hcd platform driver doesn't know about otg device so it calls > >> > >> usb_add_hcd(hcd,..)->usb_otg_register_hcd(NULL, hcd,..); > >> > >> Case B: > >> > >> core driver knows about both otg and hcd so it calls > >> usb_otg_register_hcd(otg, hcd,...); > >> > > > > Ok, Get your points, you mean invoke usb_otg_register_hcd at platform > > driver directly instead of at hcd.c. It may be not a good solution > > due to we use different otg APIs for two cases, it may confuse the > > users, unless we can have some APIs (flags) are easy to read and well > > documentation. > > > > I need to think how else we can solve this problem so that it is usable > for all scenarios. If you get some bright ideas please do share :) > If we want to call OTG stuff at hcd/gadget driver, we'd better store struct usb_otg pointer at struct usb_hcd and struct usb_gadget, since the parent/child relationship can't be used now. The platform driver can call usb_otg_register_hcd/usb_otg_register_gadget to achieve this, it is a little different with your current design. -- Best Regards, Peter Chen