From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752795AbcEXC7F (ORCPT ); Mon, 23 May 2016 22:59:05 -0400 Received: from mail-pa0-f67.google.com ([209.85.220.67]:32799 "EHLO mail-pa0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751996AbcEXC7B (ORCPT ); Mon, 23 May 2016 22:59:01 -0400 Date: Tue, 24 May 2016 10:53:56 +0800 From: Peter Chen To: Roger Quadros 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 Message-ID: <20160524025356.GA6675@shlinux2> References: <573C6347.6080705@ti.com> <573C719F.5010107@ti.com> <573D6C1C.4090600@ti.com> <20160521022940.GA15088@shlinux2> <20160523032149.GA20438@shlinux2> <5742D764.2010507@ti.com> <5742DD43.6060703@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5742DD43.6060703@ti.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, May 23, 2016 at 01:36:51PM +0300, Roger Quadros wrote: > On 23/05/16 13:34, Jun Li wrote: > > 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. > > Yes. I am on the same page with that. > > > > 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? > > I'm fine with my original idea if you guys can confirm :). > Please do it. -- Best Regards, Peter Chen From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Chen Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Date: Tue, 24 May 2016 10:53:56 +0800 Message-ID: <20160524025356.GA6675@shlinux2> References: <573C6347.6080705@ti.com> <573C719F.5010107@ti.com> <573D6C1C.4090600@ti.com> <20160521022940.GA15088@shlinux2> <20160523032149.GA20438@shlinux2> <5742D764.2010507@ti.com> <5742DD43.6060703@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <5742DD43.6060703@ti.com> Sender: linux-kernel-owner@vger.kernel.org To: Roger Quadros 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.ke List-Id: devicetree@vger.kernel.org On Mon, May 23, 2016 at 01:36:51PM +0300, Roger Quadros wrote: > On 23/05/16 13:34, Jun Li wrote: > > 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. > > Yes. I am on the same page with that. > > > > 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? > > I'm fine with my original idea if you guys can confirm :). > Please do it. -- Best Regards, Peter Chen