From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754000AbcEWKeT (ORCPT ); Mon, 23 May 2016 06:34:19 -0400 Received: from mail-db3on0055.outbound.protection.outlook.com ([157.55.234.55]:62880 "EHLO emea01-db3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753949AbcEWKeQ convert rfc822-to-8bit (ORCPT ); Mon, 23 May 2016 06:34:16 -0400 From: Jun Li To: Roger Quadros , Peter Chen CC: "peter.chen@freescale.com" , "balbi@kernel.org" , "tony@atomide.com" , "gregkh@linuxfoundation.org" , "dan.j.williams@intel.com" , "mathias.nyman@linux.intel.com" , "Joao.Pinto@synopsys.com" , "sergei.shtylyov@cogentembedded.com" , "jun.li@freescale.com" , "grygorii.strashko@ti.com" , "yoshihiro.shimoda.uh@renesas.com" , "robh@kernel.org" , "nsekhar@ti.com" , "b-liu@ti.com" , "linux-usb@vger.kernel.org" , "linux-omap@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "devicetree@vger.kernel.org" Subject: RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Thread-Topic: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Thread-Index: AQHRrP7fiGOaT66xRk2Vwa/MP6wYS5+7J+uAgAAXgoCAAA/EgIAAB/eAgAFqGKCAAAt5AIAAAifQgAHcsYCAAAOXcIAADYOAgAAE45CAASW4AIACz/wAgAMzPICAAHKMAIAABA9g Date: Mon, 23 May 2016 10:34:12 +0000 Message-ID: References: <57399839.90706@ti.com> <573AD198.6030307@ti.com> <573C6347.6080705@ti.com> <573C719F.5010107@ti.com> <573D6C1C.4090600@ti.com> <20160521022940.GA15088@shlinux2> <20160523032149.GA20438@shlinux2> <5742D764.2010507@ti.com> In-Reply-To: <5742D764.2010507@ti.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: ti.com; dkim=none (message not signed) header.d=none;ti.com; dmarc=none action=none header.from=nxp.com; x-originating-ip: [123.151.195.1] x-ms-office365-filtering-correlation-id: eecc6ec7-e17a-4fb0-5f55-08d382f5c7e2 x-microsoft-exchange-diagnostics: 1;AM4PR04MB2130;5:fZvULOh0o2bIb4K8dw0bOwafU/1ZNYXxlfWUf2kqz0rTxTxvF2vifK0VtlO99eXB3phwQxN0Y9V8LhikbsA0mHI0OR5E254DUG9u48DZRgw27g9eLS4SrVgsUtAqC8YvuuTyG5c63261P7uPM4sFmw==;24:eVNJpJ82Df9iQOQ8kek96hwhHC8XaKTf8IEzsQvF1YT6mZBGzN1RTy5edOKgWMGMuQYD+PpsNYLsVRCfyDthcRI3Rq0k1cN5pRlxPy3rA0E=;7:0arkPBUpNGwEZwnL2LiRQHBC7ErFjbleZdgd+4hgALzypd5rM2QkAgDycWlDW//ChT98Zf0NAUX/oe2ie+/fGuQTsAp7eh+re/KJi80HvoqYfrwBUgg3B4tgj9XTc2vW4q9MSrUHRgDBVTDFfLMzyyYPS4GgNSc11DANdUxG+N2zy8SGYW8GFMqsJnUcmuSz x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AM4PR04MB2130; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046)(6055026);SRVR:AM4PR04MB2130;BCL:0;PCL:0;RULEID:;SRVR:AM4PR04MB2130; x-forefront-prvs: 0951AB0A30 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(6009001)(13464003)(377454003)(24454002)(76104003)(3280700002)(1220700001)(33656002)(8936002)(2906002)(3660700001)(8666003)(586003)(66066001)(92566002)(6116002)(76576001)(102836003)(189998001)(77096005)(9686002)(50986999)(54356999)(122556002)(3846002)(10400500002)(2950100001)(76176999)(11100500001)(87936001)(93886004)(5008740100001)(106116001)(5002640100001)(8676002)(19580395003)(5004730100002)(5001770100001)(81166006)(19580405001)(74316001)(5003600100002)(2900100001)(4326007)(86362001)(7059030);DIR:OUT;SFP:1101;SCL:1;SRVR:AM4PR04MB2130;H:AM4PR04MB2130.eurprd04.prod.outlook.com;FPR:;SPF:None;MLV:sfv;LANG:en; spamdiagnosticoutput: 1:23 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: 23 May 2016 10:34:12.0378 (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: Roger Quadros [mailto:rogerq@ti.com] > Sent: Monday, May 23, 2016 6:12 PM > To: Peter Chen > Cc: Jun Li ; peter.chen@freescale.com; balbi@kernel.org; > tony@atomide.com; gregkh@linuxfoundation.org; dan.j.williams@intel.com; > mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com; > sergei.shtylyov@cogentembedded.com; jun.li@freescale.com; > grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com; > robh@kernel.org; nsekhar@ti.com; b-liu@ti.com; linux-usb@vger.kernel.org; > linux-omap@vger.kernel.org; linux-kernel@vger.kernel.org; > devicetree@vger.kernel.org > Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core > > On 23/05/16 06:21, Peter Chen wrote: > > On Sat, May 21, 2016 at 10:29:40AM +0800, Peter Chen wrote: > >> On Thu, May 19, 2016 at 10:32:44AM +0300, Roger Quadros wrote: > >>> On 18/05/16 17:46, Jun Li wrote: > >>>> > >>>> > >>>>>>> > >>>>>>> I didn't want to have complex Kconfig so decided to have otg as > >>>>>>> built-in only. > >>>>>>> What do you want me to change in existing code? and why? > >>>>>> > >>>>>> Remove those stuff which only for pass diff driver config Like > >>>>>> every controller driver need a duplicated > >>>>>> > >>>>>> static struct otg_hcd_ops ci_hcd_ops = { > >>>>>> ... > >>>>>> } > >>>>> > >>>>> This is an exception only. Every controller driver doesn't need to > >>>>> implement hcd_ops. It is implemented in the hcd core. > >>>>> > >>>>>> > >>>>>> And here is another example, for gadget connect, otg driver can > >>>>>> directly call to usb_udc_vbus_handler() in drd state machine, but > >>>>>> you create another interface: > >>>>>> > >>>>>> .connect_control = usb_gadget_connect_control, > >>>>>> > >>>>>> If the symbol is defined in one driver which is 'm', another > >>>>>> driver reference it should be 'm' as well, then there is no this > >>>>>> kind of problem as my understanding. > >>>>> > >>>>> That is fine as long as all are 'm'. but how do you solve the case > >>>>> when Gadget is built in and host is 'm'? OTG has to be built-in > >>>>> and you will need an hcd to gadget interface. > >>>> > >>>> Hcd to gadget interface? Or you want to say otg to host interface? > >>> > >>> Sorry, I meant to say host to otg interface. > >>> > >>>> > >>>> I think hcd and gadget are independent each other, now > >>>> > >>>> Hcd --> otg; and gadget --> otg, (hcd and gadget use otg's symbol) > >>> > >>> It is actually a circular dependency for both. > >>> hcd <--> otg and gadget <--> otg > >>> > >>> hcd -> otg for usb_otg_register/unregister_hcd otg -> hcd for > >>> usb_add/remove_hcd, usb_bus_start_enum, usb_control_msg, > >>> usb_hub_find_child > >>> > >>> gadget -> otg for usb_otg_register/unregister_gadget > >>> otg -> gadget for usb_gadget_start/stop, usb_udc_vbus_handler > >>> > >>> Now consider what will happen if I get rid of the otg_hcd and > otg_gadget interfaces. > >>> 'y' means built-in, 'm' means module. > >>> > >>> 1) hcd 'y', gadget 'y' > >>> otg has to be 'y' for proper build. > >>> > >>> 2) hcd 'm', gadget 'm' > >>> otg has to be 'm' for proper build. > >>> > >>> 3) hcd 'y', gadget 'm' > >>> Build will fail always. > >>> If otg is 'y', otg build will fail due to dependency on gadget. > >>> If otg is 'm', hcd build will fail due to dependency on otg. > >>> > >>> 4) hcd 'm', gadget 'y' > >>> Build will fail always. > >>> If otg is 'y', otg build will fail due to dependency on hcd. > >>> If otg is 'm', gadget build will fails due to dependency on otg. > >>> > >>> So I solve this problem by adding the otg_hcd_ops and otg_gadget_ops > >>> to remove otg->hcd and otg->gadget dependency. > >>> > >>> Now we can address 3) and 4) like so > >>> > >>> 3) hcd 'y', gadget 'm' > >>> otg has to be 'y' for proper build. > >>> > >>> 4) hcd 'm', gadget 'y' > >>> otg has to be 'y' for proper build. > >>> > >> > >> How about this: > >> Moving usb_otg_register/unregister_hcd to host driver to remove > >> dependency hcd->otg. And moving usb_otg_get_data to common.c. > >> > >> Delete the wait queue at usb-otg.c, and if calling usb_otg_get_data > >> returns NULL, the host/device driver's probe return -EPROBE_DEFER. > >> When the otg driver is probed successfully, the host/device will be > >> re-probed again, and usb_otg_register_hcd will be called again. > >> > >> And let OTG depends on HCD && GADGET, and delete otg_hcd_ops and > >> otg_gadget_ops. Below build dependency issues can be fixed. > >> What do you think? > >> > >>> 1) hcd 'y', gadget 'y' > >>> otg has to be 'y' for proper build. > >>> > >>> 2) hcd 'm', gadget 'm' > >>> otg has to be 'm' for proper build. > >>> > >>> 3) hcd 'y', gadget 'm' > >>> Build will fail always. > >>> If otg is 'y', otg build will fail due to dependency on gadget. > >>> If otg is 'm', hcd build will fail due to dependency on otg. > >>> > >>> 4) hcd 'm', gadget 'y' > >>> Build will fail always. > >>> If otg is 'y', otg build will fail due to dependency on hcd. > >>> If otg is 'm', gadget build will fails due to dependency on otg. > >> -- > >> > > > > After thinking more, my suggestion can't work. How about moving > > CONFIG_USB_OTG out of CONFIG_USB, in that case, CONFIG_USB_OTG can > > only be built in. > > > I tried this but it still doesn't resolve 3 and 4. I.e. > it can't be automatically set to 'y' when either of hcd/gadget is 'y' > and the other is 'm'. USB_OTG only can be selected by *user*, can't be automatically set to 'y' like you said, no matter what's hcd/gadget config, USB_OTG can be disabled in any cases. I think Peter's intention is to make USB_OTG to be 'bool', either disabled; or built-in(if enabled), something like your original idea? Li Jun > > I think some kconfig trickery can be done to get what we want. > > cheers, > -roger From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jun Li Subject: RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Date: Mon, 23 May 2016 10:34:12 +0000 Message-ID: References: <57399839.90706@ti.com> <573AD198.6030307@ti.com> <573C6347.6080705@ti.com> <573C719F.5010107@ti.com> <573D6C1C.4090600@ti.com> <20160521022940.GA15088@shlinux2> <20160523032149.GA20438@shlinux2> <5742D764.2010507@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT Return-path: In-Reply-To: <5742D764.2010507@ti.com> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: Roger Quadros , Peter Chen Cc: "peter.chen@freescale.com" , "balbi@kernel.org" , "tony@atomide.com" , "gregkh@linuxfoundation.org" , "dan.j.williams@intel.com" , "mathias.nyman@linux.intel.com" , "Joao.Pinto@synopsys.com" , "sergei.shtylyov@cogentembedded.com" , "jun.li@freescale.com" , "grygorii.strashko@ti.com" , "yoshihiro.shimoda.uh@renesas.com" , "robh@kernel.org" , "nsekhar@ti.com" , "b-liu@ti.com" , "linux-usb@vger.kernel.org" , "linux-omap@vger.kernel.org" , "linux-kernel@vger.kernel.org" List-Id: devicetree@vger.kernel.org Hi > -----Original Message----- > From: Roger Quadros [mailto:rogerq@ti.com] > Sent: Monday, May 23, 2016 6:12 PM > To: Peter Chen > Cc: Jun Li ; peter.chen@freescale.com; balbi@kernel.org; > tony@atomide.com; gregkh@linuxfoundation.org; dan.j.williams@intel.com; > mathias.nyman@linux.intel.com; Joao.Pinto@synopsys.com; > sergei.shtylyov@cogentembedded.com; jun.li@freescale.com; > grygorii.strashko@ti.com; yoshihiro.shimoda.uh@renesas.com; > robh@kernel.org; nsekhar@ti.com; b-liu@ti.com; linux-usb@vger.kernel.org; > linux-omap@vger.kernel.org; linux-kernel@vger.kernel.org; > devicetree@vger.kernel.org > Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core > > On 23/05/16 06:21, Peter Chen wrote: > > On Sat, May 21, 2016 at 10:29:40AM +0800, Peter Chen wrote: > >> On Thu, May 19, 2016 at 10:32:44AM +0300, Roger Quadros wrote: > >>> On 18/05/16 17:46, Jun Li wrote: > >>>> > >>>> > >>>>>>> > >>>>>>> I didn't want to have complex Kconfig so decided to have otg as > >>>>>>> built-in only. > >>>>>>> What do you want me to change in existing code? and why? > >>>>>> > >>>>>> Remove those stuff which only for pass diff driver config Like > >>>>>> every controller driver need a duplicated > >>>>>> > >>>>>> static struct otg_hcd_ops ci_hcd_ops = { > >>>>>> ... > >>>>>> } > >>>>> > >>>>> This is an exception only. Every controller driver doesn't need to > >>>>> implement hcd_ops. It is implemented in the hcd core. > >>>>> > >>>>>> > >>>>>> And here is another example, for gadget connect, otg driver can > >>>>>> directly call to usb_udc_vbus_handler() in drd state machine, but > >>>>>> you create another interface: > >>>>>> > >>>>>> .connect_control = usb_gadget_connect_control, > >>>>>> > >>>>>> If the symbol is defined in one driver which is 'm', another > >>>>>> driver reference it should be 'm' as well, then there is no this > >>>>>> kind of problem as my understanding. > >>>>> > >>>>> That is fine as long as all are 'm'. but how do you solve the case > >>>>> when Gadget is built in and host is 'm'? OTG has to be built-in > >>>>> and you will need an hcd to gadget interface. > >>>> > >>>> Hcd to gadget interface? Or you want to say otg to host interface? > >>> > >>> Sorry, I meant to say host to otg interface. > >>> > >>>> > >>>> I think hcd and gadget are independent each other, now > >>>> > >>>> Hcd --> otg; and gadget --> otg, (hcd and gadget use otg's symbol) > >>> > >>> It is actually a circular dependency for both. > >>> hcd <--> otg and gadget <--> otg > >>> > >>> hcd -> otg for usb_otg_register/unregister_hcd otg -> hcd for > >>> usb_add/remove_hcd, usb_bus_start_enum, usb_control_msg, > >>> usb_hub_find_child > >>> > >>> gadget -> otg for usb_otg_register/unregister_gadget > >>> otg -> gadget for usb_gadget_start/stop, usb_udc_vbus_handler > >>> > >>> Now consider what will happen if I get rid of the otg_hcd and > otg_gadget interfaces. > >>> 'y' means built-in, 'm' means module. > >>> > >>> 1) hcd 'y', gadget 'y' > >>> otg has to be 'y' for proper build. > >>> > >>> 2) hcd 'm', gadget 'm' > >>> otg has to be 'm' for proper build. > >>> > >>> 3) hcd 'y', gadget 'm' > >>> Build will fail always. > >>> If otg is 'y', otg build will fail due to dependency on gadget. > >>> If otg is 'm', hcd build will fail due to dependency on otg. > >>> > >>> 4) hcd 'm', gadget 'y' > >>> Build will fail always. > >>> If otg is 'y', otg build will fail due to dependency on hcd. > >>> If otg is 'm', gadget build will fails due to dependency on otg. > >>> > >>> So I solve this problem by adding the otg_hcd_ops and otg_gadget_ops > >>> to remove otg->hcd and otg->gadget dependency. > >>> > >>> Now we can address 3) and 4) like so > >>> > >>> 3) hcd 'y', gadget 'm' > >>> otg has to be 'y' for proper build. > >>> > >>> 4) hcd 'm', gadget 'y' > >>> otg has to be 'y' for proper build. > >>> > >> > >> How about this: > >> Moving usb_otg_register/unregister_hcd to host driver to remove > >> dependency hcd->otg. And moving usb_otg_get_data to common.c. > >> > >> Delete the wait queue at usb-otg.c, and if calling usb_otg_get_data > >> returns NULL, the host/device driver's probe return -EPROBE_DEFER. > >> When the otg driver is probed successfully, the host/device will be > >> re-probed again, and usb_otg_register_hcd will be called again. > >> > >> And let OTG depends on HCD && GADGET, and delete otg_hcd_ops and > >> otg_gadget_ops. Below build dependency issues can be fixed. > >> What do you think? > >> > >>> 1) hcd 'y', gadget 'y' > >>> otg has to be 'y' for proper build. > >>> > >>> 2) hcd 'm', gadget 'm' > >>> otg has to be 'm' for proper build. > >>> > >>> 3) hcd 'y', gadget 'm' > >>> Build will fail always. > >>> If otg is 'y', otg build will fail due to dependency on gadget. > >>> If otg is 'm', hcd build will fail due to dependency on otg. > >>> > >>> 4) hcd 'm', gadget 'y' > >>> Build will fail always. > >>> If otg is 'y', otg build will fail due to dependency on hcd. > >>> If otg is 'm', gadget build will fails due to dependency on otg. > >> -- > >> > > > > After thinking more, my suggestion can't work. How about moving > > CONFIG_USB_OTG out of CONFIG_USB, in that case, CONFIG_USB_OTG can > > only be built in. > > > I tried this but it still doesn't resolve 3 and 4. I.e. > it can't be automatically set to 'y' when either of hcd/gadget is 'y' > and the other is 'm'. USB_OTG only can be selected by *user*, can't be automatically set to 'y' like you said, no matter what's hcd/gadget config, USB_OTG can be disabled in any cases. I think Peter's intention is to make USB_OTG to be 'bool', either disabled; or built-in(if enabled), something like your original idea? Li Jun > > I think some kconfig trickery can be done to get what we want. > > cheers, > -roger