From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754552AbbIJKlZ (ORCPT ); Thu, 10 Sep 2015 06:41:25 -0400 Received: from mail-bl2on0141.outbound.protection.outlook.com ([65.55.169.141]:27601 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752521AbbIJKlS (ORCPT ); Thu, 10 Sep 2015 06:41:18 -0400 Authentication-Results: spf=fail (sender IP is 192.88.158.2) 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: Thu, 10 Sep 2015 17:28:56 +0800 From: Li Jun To: Roger Quadros CC: , , , , , , , , , , , , Subject: Re: [PATCH v4 07/13] usb: otg: add OTG core Message-ID: <20150910092854.GA7703@shlinux2> References: <1440422484-4737-1-git-send-email-rogerq@ti.com> <1440422484-4737-8-git-send-email-rogerq@ti.com> <20150907074002.GA30578@shlinux2> <55ED6C9F.2000502@ti.com> <20150909062006.GB812@shlinux2> <55F0036A.60403@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <55F0036A.60403@ti.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-EOPAttributedMessage: 0 X-Microsoft-Exchange-Diagnostics: 1;BN1BFFO11FD033;1:rZZaMXCd8L/Gsry/a+0iSoWceYlHnuG4CC490ex2qScwAPelg5dnIM33ypNrWcHTo4eWAyYn5+vmSci3ypGYKP8LI8vPyEGm2zDKRzq6WNtvSDHP9TRyglD39NT8ACkUWt+1R4zdKluHnkRf2bQs3mmF1T/G5bcl4DSGDKFw5Pimo+FebmlK0N4SPZGwW47wxpj77OOCrDCmn+nQbZJ47yQUciflNWrUuefbG/6UBFc6QEZ/gzUzVkzuDPg6t2BnFf2l4uRRhpCY/KFg/oP44M7pnKaTPyHHBRzUt9o0kKw2NXt+L9pH8HvqJWfahNST5G0dXMOFD77US01K1eCPZnmlkr6VxVT4WnNjSwMmRl8UbeFLzPMUDcIjkx4GYGjw5SuY5z49oeVp2rvXvOPWQgSQ/Evwj1oTlk1hzxkJOrk= X-Forefront-Antispam-Report: CIP:192.88.158.2;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10019020)(6009001)(2980300002)(1110001)(1109001)(339900001)(199003)(57704003)(189002)(24454002)(5007970100001)(87936001)(5001960100002)(189998001)(92566002)(33716001)(69596002)(23726002)(68736005)(11100500001)(81156007)(4001540100001)(110136002)(77096005)(97736004)(4001350100001)(64706001)(106466001)(93886004)(104016003)(46102003)(83506001)(62966003)(50986999)(85426001)(2950100001)(33656002)(5001860100001)(46406003)(105606002)(50466002)(97756001)(6806004)(47776003)(54356999)(76176999)(77156002)(5001830100001);DIR:OUT;SFP:1102;SCL:1;SRVR:DM2PR0301MB0862;H:az84smr01.freescale.net;FPR:;SPF:Fail;PTR:InfoDomainNonexistent;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;DM2PR0301MB0862;2:JZD+ZTKgZ6K+dVVvl9ClWl0syYc+MSPM4TEMJomPWshBh/4aTOu+nfU8f/NBR4XPVnDxS2UBRjsMBog2iJ5ATvaNLrwwsK9/bNzd4qBJ5GcBHabygHXPgZ92/kCYVKRa7CJwQdEsOXkFZJuuuNTX69tUZJV/hQ21eJLI3qDaEcg=;3:CTYxyVY7jOJ7mGHBPgHET8ER2+fEx7H5LKHyiNd/GI7TZ6jKGO9DDgQkF3LeG3hmg9TYgN63o5nltdrbHSNddfxhCnfGwKlDzMs+JGSCB17jWvri8qu/PSZ7SipfT013oEKUEGL2//1dyHFmPbeJhbaf6FdweelZuiW+fSHLC0uogSuZG7YEu9jL+nwPybrsTVwfIggkAKz3uouRqODxKa9osqAMEWJhlqieOC+rihM=;25:LhYFf2WCdmr/LVV7Uo/dSIf218LGNzaRts6QKXQ+pgkKuql/xDDPH+brFNczjbAmem5BaFF8UxWg1qNnIXo1+K5mj+94N/PAyqkFNQZQmdCC1hwts+SWvwLe49Ae5mmY7OA7AjELU1poII6+/IhXQlPOGYbBLGtVBfrcEls9SEydn22ExR2a5u2h6MlrStzQ2OLiJvPEMBDfvZ+/09k6RciPTBJJFBih+Xrjdu+OD7X2YPWdOfinnvhM/harUJT3dHSZIYTHz9PZjpoPRjFIjg== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DM2PR0301MB0862; X-Microsoft-Exchange-Diagnostics: 1;DM2PR0301MB0862;20:Kj5q71B+xB0Z4oVmhsS/NtmUOF0pCjIA1Y704tPO9/nw1bxB+7+GHkWHG5AukwFGXM2TO0NhFSCoxiAN2B8YuvVFMNRs9ABDrYfFw99XiNL6OGSDjOIFh4V0vm1F2VKazhXUOq3xsCmYm+9VjEpveBjqFkIKdPM0c61Fs+DevF1AfAosMtvJmjnsfD70v6T90+MIJ1kuXEjksLZGs1Gn1RKd1cdk9eH8s2AFL/wQvqeHKv8ADB79Xb2UCg4J9WwT8bbI9cs1tYwMAIWE8yaaKAmH+9JlQwiK8pAQyAoCIxiqLofMu9MaXxtUu2KAKEH8ICnqLAXpNtJFZNeveLmCwPgR+AeFruHWt8of5xfkQsc=;4:33ffgyNm32H1qQ7gNrt6F15LrQUKtcXDac2vAU3yOG2dkMhjc1QTy3T6preA4tr3QNDzAe8waBsSVcpQQBq+cqMXZRzO9Dw22s2q8cg4BdYLf9rFXXNnRmnTiIC1J94FjE8b2Z4oS5PnO/fNc2RlYcdr69+doDlfRqdLfTDEMbMDd8wqEN6yOjFkyPnuo5s1iXCk+acRXWGi/l3l8R9lXwYcKEZ5BqBPThkQVjCGJXsZ+FuEBZjsbz2FBfAi3EA8O8ruykUjyX/obaqjJQwypXfWNCg1LQYG344a7WhpTVgVp77qw9YBcMAbK75JlbSE 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:DM2PR0301MB0862;BCL:0;PCL:0;RULEID:;SRVR:DM2PR0301MB0862; X-Forefront-PRVS: 06952FC175 X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;DM2PR0301MB0862;23:5UCSFm1G7bOlrrAYP29yhQjjc8mUjAXGGweZfeV?= =?us-ascii?Q?dpIo39wXfW0Y1kgLJWb9pfRe5Vh8/bgew+Nolyjg4mC0wjQN3tZMG/XQX9dG?= =?us-ascii?Q?5vVMyjncAflRmOcmPRz2uJ4mWBmf1/ArToNbKuKBUJlJCZLf2GLDDI37hQrH?= =?us-ascii?Q?sh9JTX7Qmo3wYKm3ewT7RWqvytF753HVecrHYd81fEurDOUPjKxxGKucNlPi?= =?us-ascii?Q?XQxJmTNM4le5YQuE+YPHpGAcliRTt7PhD6EIVE0YSmwWdbkO+DQpOlWrYfAs?= =?us-ascii?Q?gLlJMZPN4S+UFaLvV6raWkteIF+ZtJLiuQhp6AVjpk1vo1dYnRilSRk0fYrp?= =?us-ascii?Q?BTfP9Vb92rIQ98+Zd/1W8qbY2Mu6cEtLrYvCBpSd+Ukkz5L6clECInSM2RvV?= =?us-ascii?Q?Ty5jYmQiO89ErmLV9M8nhQDsdbeqR5calPaYj8EBzO0boBeTkNA/Emom5Ziz?= =?us-ascii?Q?Q6LaUvFhnt+ctrBT98SS1V7W1+9nS2WyPBUzq8MzWgnKpNgoajGAXWUTIJAo?= =?us-ascii?Q?8VqqBlbOvQLe3RfroaGG2Blo4rXtnlUfwqU8Zmxz2WNVeCymC7evPmNYO3r5?= =?us-ascii?Q?o5+hoS3nT4ebWdkWqYDmEGG5wY3qs5I4jytKuKqUFRybHc7gxNS+YVRWr5rP?= =?us-ascii?Q?mCV7oJA1qCdvrpPucR+L+QWySdCRdLhb3bNynauEDvFUtZmKAbANQrQx+tXm?= =?us-ascii?Q?p9gXdsCK0Av6bgGid0akGfxWctKKmqtDSAAxb+MqGt8GRTZ1Kf60nJO7Nvo5?= =?us-ascii?Q?Ae/ZRrET6R+1jZpPeNGHkV9gGawcnAf2kE17uFmr6j2OjFd8duWMSbPYpXXv?= =?us-ascii?Q?1l9J0vzVkl0iO9Vrwy4htuqEx0ZitUkjjIqpNAwWTqAPz7nwk67tUqfLbrDq?= =?us-ascii?Q?1hUSfjlWhKM72sEH50nOAK8LQFKqBUjTgdSBlfvZJxVKEs+Q8uSB6MfaT3Q9?= =?us-ascii?Q?YeERln5LxbnI0kgmEbIjtcyA9X93YLkm6yGvINzf635llB+OYazbFd4hAOVi?= =?us-ascii?Q?n0+R33auGJigM0/s4wW47D/EIeZrA6AMUKp2a2so5V7bHLaByOVBe0MQZffD?= =?us-ascii?Q?jW/DvEgseTjwmTOSVRXTxk8ZHhgxF7p3MfCnsF0yCwE/Px4n9e4rZGKxQMOi?= =?us-ascii?Q?ZRfxKfRZ6h+FL3le7L5KSW/gZ7H9XpeC56N+5UuPUvjs/kp0i5QKSmni6xWA?= =?us-ascii?Q?mpyq/UxAtsEZAO6+DoLJdBzYM/pjYGQq35QkM+epI0WDbwsVVVtIxe7HkSki?= =?us-ascii?Q?+SkOC3HR9AQRUUJ8QwO4=3D?= X-Microsoft-Exchange-Diagnostics: 1;DM2PR0301MB0862;5:3WmCSHC1HU5pQymcbfyt/vXbbIY7HrfXjBX7oSW2UkaWYDnct05YXj8uvKEGGnkR/nyKPH5IEX7juUcPfk0R9J5A6+SaOCYQEuMPkMs1A+Px18plemQBBLGwyyQQtFxhcSoOMsgg68VCmyp2VTWzfg==;24:S4ie/kfi099eRdHi4jNukJapag3TIgnf5u9u1pBfLIBjxbmcZd5ZXO8wy/GIF5BD2vBActHRiXjvja5R9NUfvU7EzYNdFB+i8pa9Nmo+6q4=;20:bC9M4gqVWXLivcjRqvZioRzQjm3Qnv8sWxDWQDQMxcm0Vp6UIDz0jUwyQP6/digVsIxDUq16e+aybO17ZJMC6w== X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Sep 2015 10:41:14.3840 (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.158.2];Helo=[az84smr01.freescale.net] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM2PR0301MB0862 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 09, 2015 at 01:01:14PM +0300, Roger Quadros wrote: ... ... > >>>> + return -EINVAL; > >>> > >>> Return non-zero, then if err, do we need call usb_otg_add_hcd() after > >>> usb_otg_register_hcd() fails? > >> > >> You should not call usb_otg_register_hcd() but usb_add_hcd(). > >> If that fails then you fail as ususal. > > > > My point is if we use usb_add_hcd(), but failed in usb_otg_register_hcd(), > > then usb_otg_add_hcd() will be called in *all* error case, is this your > > expectation? > > if (usb_otg_register_hcd(hcd, irqnum, irqflags, &otg_hcd_intf)) > > return usb_otg_add_hcd(hcd, irqnum, irqflags); > > > > Yes, my intention was that if otg fails then it is a non otg HCD so register normally. > Let me correct my previous statement. If you are absolutely sure > that the HCD is for otg/dual-role usage then you should call usb_otg_register_hcd(). > I think this is not just about a statement, in your usb_otg_register_hcd() implementation, there are several places will return error, I think only the first two are for a non-otg HCD case, the following error cases seems mean this is for otg usage, but it fails in middle of registration, if that is the case, is it reasonable to call usb_otg_add_hcd()? > >>>> + * @fsm_running: state machine running/stopped indicator > >>>> + */ > >>>> struct usb_otg { > >>>> u8 default_a; > >>>> > >>>> struct phy *phy; > >>>> /* old usb_phy interface */ > >>>> struct usb_phy *usb_phy; > >>>> + > >>> > >>> add a blank line? > >>> > > > > You missed this. > > Sorry. Did you suggest to remove that blank line > or add a new one before usb_phy? > Remove it. > > > >>>> struct usb_bus *host; > >>>> struct usb_gadget *gadget; > >>>> > >>>> enum usb_otg_state state; > >>>> > >>>> + struct device *dev; > >>>> + struct usb_otg_caps *caps; > >>>> + struct otg_fsm fsm; > >>>> + struct otg_fsm_ops fsm_ops; > >>>> + > >>>> + /* internal use only */ > >>>> + struct otg_hcd primary_hcd; > >>>> + struct otg_hcd shared_hcd; > >>>> + struct otg_gadget_ops *gadget_ops; > >>>> + struct otg_timer timers[NUM_OTG_FSM_TIMERS]; > >>>> + struct list_head list; > >>>> + struct work_struct work; > >>>> -- > >>>> 2.1.4 > > > > -- > cheers, > -roger