From: Peter Chen <hzpeterchen@gmail.com>
To: Roger Quadros <rogerq@ti.com>
Cc: Jun Li <jun.li@nxp.com>,
"peter.chen@freescale.com" <peter.chen@freescale.com>,
"balbi@kernel.org" <balbi@kernel.org>,
"tony@atomide.com" <tony@atomide.com>,
"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
"dan.j.williams@intel.com" <dan.j.williams@intel.com>,
"mathias.nyman@linux.intel.com" <mathias.nyman@linux.intel.com>,
"Joao.Pinto@synopsys.com" <Joao.Pinto@synopsys.com>,
"sergei.shtylyov@cogentembedded.com"
<sergei.shtylyov@cogentembedded.com>,
"jun.li@freescale.com" <jun.li@freescale.com>,
"grygorii.strashko@ti.com" <grygorii.strashko@ti.com>,
"yoshihiro.shimoda.uh@renesas.com"
<yoshihiro.shimoda.uh@renesas.com>,
"robh@kernel.org" <robh@kernel.org>,
"nsekhar@ti.com" <nsekhar@ti.com>, "b-liu@ti.com" <b-liu@ti.com>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
Date: Wed, 8 Jun 2016 17:05:58 +0800 [thread overview]
Message-ID: <20160608090558.GB27133@shlinux2> (raw)
In-Reply-To: <5757CA03.5020409@ti.com>
On Wed, Jun 08, 2016 at 10:32:19AM +0300, Roger Quadros wrote:
> Hi,
>
> On 24/05/16 05:53, Peter Chen wrote:
> > 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 <hzpeterchen@gmail.com>
> >>>> Cc: Jun Li <jun.li@nxp.com>; 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.
> >
>
> OK. I managed to fix this and get usbotg to be as module when both USB and GADGET
> are modules with the following patch.
>
> What we do is use USB_OTG_CORE as a hidden tristate symbol that is used to build usbotg.o
> only if USB_OTG is enabled.
>
> USB_OTG is used to enable/disable OTG and it has to be bool since it is used at mutiple
> places in the code.
>
I am ok with that, but better you can write down above at your commit
log, then the reader can know your design purpose.
Peter
> cheers,
> -roger
>
> ---
>
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index 8689dcb..ed596ec 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -32,6 +32,23 @@ if USB_SUPPORT
> config USB_COMMON
> tristate
>
> +config USB_OTG_CORE
> + tristate
> +
> +config USB_OTG
> + bool "OTG/Dual-role support"
> + depends on PM && USB && USB_GADGET
> + default n
> + ---help---
> + The most notable feature of USB OTG is support for a
> + "Dual-Role" device, which can act as either a device
> + or a host. The initial role is decided by the type of
> + plug inserted and can be changed later when two dual
> + role devices talk to each other.
> +
> + Select this only if your board has Mini-AB/Micro-AB
> + connector.
> +
> config USB_ARCH_HAS_HCD
> def_bool y
>
> @@ -40,6 +57,7 @@ config USB
> tristate "Support for Host-side USB"
> depends on USB_ARCH_HAS_HCD
> select USB_COMMON
> + select USB_OTG_CORE
> select NLS # for UTF-8 strings
> ---help---
> Universal Serial Bus (USB) is a specification for a serial bus
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index 16a5b55..03f7204 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -59,6 +59,6 @@ obj-$(CONFIG_USB_RENESAS_USBHS) += renesas_usbhs/
> obj-$(CONFIG_USB_GADGET) += gadget/
>
> obj-$(CONFIG_USB_COMMON) += common/
> -obj-$(CONFIG_USB_OTG) += common/
> +obj-$(CONFIG_USB_OTG_CORE) += common/
>
> obj-$(CONFIG_USBIP_CORE) += usbip/
> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
> index 730d928..5122b3f 100644
> --- a/drivers/usb/common/Makefile
> +++ b/drivers/usb/common/Makefile
> @@ -7,5 +7,7 @@ usb-common-y += common.o
> usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>
> obj-$(CONFIG_USB_ULPI_BUS) += ulpi.o
> +ifeq ($(CONFIG_USB_OTG),y)
> usbotg-y := usb-otg.o usb-otg-fsm.o
> -obj-$(CONFIG_USB_OTG) += usbotg.o
> +obj-$(CONFIG_USB_OTG_CORE) += usbotg.o
> +endif
> diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
> index f5eb7f5..98fcfa2 100644
> --- a/drivers/usb/common/usb-otg.c
> +++ b/drivers/usb/common/usb-otg.c
> @@ -16,6 +16,7 @@
>
> #include <linux/kernel.h>
> #include <linux/list.h>
> +#include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_platform.h>
> #include <linux/usb/of.h>
> @@ -841,3 +842,5 @@ int usb_otg_gadget_ready(struct usb_gadget *gadget, bool ready)
> return 0;
> }
> EXPORT_SYMBOL_GPL(usb_otg_gadget_ready);
> +
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
> index de371e6..37f8c54 100644
> --- a/drivers/usb/core/Kconfig
> +++ b/drivers/usb/core/Kconfig
> @@ -41,20 +41,6 @@ config USB_DYNAMIC_MINORS
>
> If you are unsure about this, say N here.
>
> -config USB_OTG
> - bool "OTG/Dual-role support"
> - depends on PM && USB_GADGET
> - default n
> - help
> - The most notable feature of USB OTG is support for a
> - "Dual-Role" device, which can act as either a device
> - or a host. The initial role is decided by the type of
> - plug inserted and can be changed later when two dual
> - role devices talk to each other.
> -
> - Select this only if your board has Mini-AB/Micro-AB
> - connector.
> -
> config USB_OTG_WHITELIST
> bool "Rely on OTG and EH Targeted Peripherals List"
> depends on USB
> diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
> index 2057add..9d55384 100644
> --- a/drivers/usb/gadget/Kconfig
> +++ b/drivers/usb/gadget/Kconfig
> @@ -16,6 +16,7 @@
> menuconfig USB_GADGET
> tristate "USB Gadget Support"
> select USB_COMMON
> + select USB_OTG_CORE
> select NLS
> help
> USB is a master/slave protocol, organized with one master
> --
> 2.7.4
>
>
>
--
Best Regards,
Peter Chen
next prev parent reply other threads:[~2016-06-08 9:11 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-13 10:03 [PATCH v8 00/14] USB OTG/dual-role framework Roger Quadros
2016-05-13 10:03 ` [PATCH v8 01/14] usb: hcd: Initialize hcd->flags to 0 Roger Quadros
2016-05-13 10:03 ` [PATCH v8 02/14] usb: otg-fsm: Prevent build warning "VDBG" redefined Roger Quadros
2016-05-13 10:03 ` [PATCH v8 03/14] usb: hcd.h: Add OTG to HCD interface Roger Quadros
2016-05-13 10:03 ` [PATCH v8 04/14] usb: otg-fsm: use usb_otg wherever possible Roger Quadros
2016-05-13 10:03 ` [PATCH v8 05/14] usb: otg-fsm: move host controller operations into usb_otg->hcd_ops Roger Quadros
2016-05-13 10:03 ` [PATCH v8 06/14] usb: gadget.h: Add OTG to gadget interface Roger Quadros
2016-05-13 10:03 ` [PATCH v8 07/14] usb: otg: get rid of CONFIG_USB_OTG_FSM in favour of CONFIG_USB_OTG Roger Quadros
2016-05-13 10:03 ` [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros
2016-05-16 9:00 ` Roger Quadros
[not found] ` <CAL411-qb0dHQrn+AarSApCq6L=toKPRerLKDcx_H+5WDWu_VDg@mail.gmail.com>
2016-05-18 12:59 ` Roger Quadros
2016-05-20 8:31 ` Roger Quadros
2016-05-20 9:19 ` Roger Quadros
2016-05-20 9:53 ` Peter Chen
2016-05-23 10:06 ` Roger Quadros
2016-05-24 9:45 ` Roger Quadros
2016-05-25 2:44 ` Peter Chen
2016-05-25 3:19 ` Jun Li
2016-05-25 12:26 ` Roger Quadros
2016-05-25 12:21 ` Roger Quadros
2016-05-25 14:44 ` Jun Li
2016-05-13 10:03 ` [PATCH v8 09/14] usb: of: add an API to get OTG device from USB controller node Roger Quadros
2016-05-20 9:29 ` [PATCH v9 " Roger Quadros
2016-05-23 21:06 ` Rob Herring
2016-05-13 10:03 ` [PATCH v8 10/14] usb: otg: add hcd companion support Roger Quadros
2016-05-13 18:13 ` Rob Herring
2016-05-16 8:12 ` Roger Quadros
2016-05-20 9:32 ` [PATCH v9 " Roger Quadros
2016-05-23 21:07 ` Rob Herring
2016-05-13 10:03 ` [PATCH v8 11/14] usb: otg: use dev_dbg() instead of VDBG() Roger Quadros
2016-05-13 10:03 ` [PATCH v8 12/14] usb: hcd: Adapt to OTG core Roger Quadros
2016-05-13 10:03 ` [PATCH v8 13/14] usb: gadget: udc: adapt " Roger Quadros
2016-05-16 7:02 ` Peter Chen
2016-05-16 8:26 ` Roger Quadros
2016-05-16 9:23 ` Peter Chen
2016-05-16 9:51 ` Roger Quadros
2016-05-17 7:38 ` Jun Li
2016-05-17 8:08 ` Roger Quadros
2016-05-17 8:28 ` Jun Li
2016-05-18 12:42 ` Roger Quadros
2016-05-18 13:12 ` Jun Li
2016-05-18 13:43 ` Roger Quadros
2016-05-18 14:46 ` Jun Li
2016-05-19 7:32 ` Roger Quadros
2016-05-21 2:29 ` Peter Chen
2016-05-23 3:21 ` Peter Chen
2016-05-23 10:11 ` Roger Quadros
2016-05-23 10:34 ` Jun Li
2016-05-23 10:36 ` Roger Quadros
2016-05-24 2:53 ` Peter Chen
2016-06-08 7:32 ` Roger Quadros
2016-06-08 9:05 ` Peter Chen [this message]
2016-05-18 3:18 ` Peter Chen
2016-05-18 12:45 ` Roger Quadros
2016-05-20 1:39 ` Peter Chen
2016-05-20 7:26 ` Roger Quadros
2016-05-21 2:44 ` Peter Chen
2016-06-01 7:38 ` Peter Chen
2016-06-02 11:07 ` Roger Quadros
2016-05-13 10:03 ` [PATCH v8 14/14] usb: host: xhci-plat: Add otg device to platform data Roger Quadros
2016-05-30 9:29 ` [PATCH v8 00/14] USB OTG/dual-role framework Peter Chen
2016-05-30 14:04 ` Roger Quadros
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160608090558.GB27133@shlinux2 \
--to=hzpeterchen@gmail.com \
--cc=Joao.Pinto@synopsys.com \
--cc=b-liu@ti.com \
--cc=balbi@kernel.org \
--cc=dan.j.williams@intel.com \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=grygorii.strashko@ti.com \
--cc=jun.li@freescale.com \
--cc=jun.li@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@linux.intel.com \
--cc=nsekhar@ti.com \
--cc=peter.chen@freescale.com \
--cc=robh@kernel.org \
--cc=rogerq@ti.com \
--cc=sergei.shtylyov@cogentembedded.com \
--cc=tony@atomide.com \
--cc=yoshihiro.shimoda.uh@renesas.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).