Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1404296

Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core

From Peter Chen <hzpeterchen@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core
Date 2016-05-20 12:00 +0200
Message-ID <rAPnQ-3SW-17@gated-at.bofh.it> (permalink)
References (2 earlier) <rzmHg-52I-11@gated-at.bofh.it> <rA9oB-2yO-3@gated-at.bofh.it> <rA9oB-2yO-1@gated-at.bofh.it> <rAO8q-3ca-29@gated-at.bofh.it> <rAOL8-3Eg-19@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Fri, May 20, 2016 at 12:19:07PM +0300, Roger Quadros wrote:
> On 20/05/16 11:31, Roger Quadros wrote:
> > On 18/05/16 15:59, Roger Quadros wrote:
> >> Hi Peter,
> >>
> >> On 18/05/16 10:45, Peter Chen wrote:
> >>>
> >>>
> >>> On Mon, May 16, 2016 at 5:00 PM, Roger Quadros <rogerq@ti.com <mailto:rogerq@ti.com>> wrote:
> >>>
> >>>     On 13/05/16 13:03, Roger Quadros wrote:
> >>>     > It provides APIs for the following tasks
> >>>     >
> >>>     > - Registering an OTG/dual-role capable controller
> >>>     > - Registering Host and Gadget controllers to OTG core
> >>>     > - Providing inputs to and kicking the OTG state machine
> >>>     >
> >>>     > Provide a dual-role device (DRD) state machine.
> >>>     > DRD mode is a reduced functionality OTG mode. In this mode
> >>>     > we don't support SRP, HNP and dynamic role-swap.
> >>>     >
> >>>     > In DRD operation, the controller mode (Host or Peripheral)
> >>>     > is decided based on the ID pin status. Once a cable plug (Type-A
> >>>     > or Type-B) is attached the controller selects the state
> >>>     > and doesn't change till the cable in unplugged and a different
> >>>     > cable type is inserted.
> >>>     >
> >>>     > As we don't need most of the complex OTG states and OTG timers
> >>>     > we implement a lean DRD state machine in usb-otg.c.
> >>>     > The DRD state machine is only interested in 2 hardware inputs
> >>>     > 'id' and 'b_sess_vld'.
> >>>     >
> >>>     > Signed-off-by: Roger Quadros <rogerq@ti.com <mailto:rogerq@ti.com>>
> >>>     > ---
> >>>     >  drivers/usb/common/Makefile  |    2 +-
> >>>     >  drivers/usb/common/usb-otg.c | 1042 ++++++++++++++++++++++++++++++++++++++++++
> >>>     >  drivers/usb/core/Kconfig     |    4 +-
> >>>     >  include/linux/usb/gadget.h   |    2 +
> >>>     >  include/linux/usb/hcd.h      |    1 +
> >>>     >  include/linux/usb/otg-fsm.h  |    7 +
> >>>     >  include/linux/usb/otg.h      |  154 ++++++-
> >>>     >  7 files changed, 1206 insertions(+), 6 deletions(-)
> >>>     >  create mode 100644 drivers/usb/common/usb-otg.c
> >>>
> >>>
> >>>     This patch causes the following build issues when CONFIG_USB_GADGET=m, CONFIG_USB=m,
> >>>     CONFIG_USB_COMMON=m and CONFIG_USB_OTG=y
> >>>
> >>>     ERROR: "usb_otg_register_gadget" [drivers/usb/gadget/udc/udc-core.ko] undefined!
> >>>     ERROR: "usb_otg_unregister_gadget" [drivers/usb/gadget/udc/udc-core.ko] undefined!
> >>>     ERROR: "usb_otg_register_hcd" [drivers/usb/core/usbcore.ko] undefined!
> >>>     ERROR: "usb_otg_unregister_hcd" [drivers/usb/core/usbcore.ko] undefined!
> >>>     ERROR: "otg_statemachine" [drivers/usb/chipidea/ci_hdrc.ko] undefined!
> >>>     scripts/Makefile.modpost:91: recipe for target '__modpost' failed
> >>>     make[1]: *** [__modpost] Error 1
> >>>     Makefile:1141: recipe for target 'modules' failed
> >>>     make: *** [modules] Error 2
> >>>     make: *** Waiting for unfinished jobs....
> >>>
> >>>     drivers/built-in.o: In function `drd_set_state':
> >>>     usb-otg.c:(.text+0x2b4242): undefined reference to `usb_otg_state_string'
> >>>     drivers/built-in.o: In function `drd_statemachine':
> >>>     (.text+0x2b4b4c): undefined reference to `usb_otg_state_string'
> >>>     Makefile:937: recipe for target 'vmlinux' failed
> >>>
> >>>     I'll fix it up with the following diff.
> >>>
> >>>     diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> >>>     index dca7856..16a5b55 100644
> >>>     --- a/drivers/usb/Makefile
> >>>     +++ b/drivers/usb/Makefile
> >>>     @@ -59,5 +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_USBIP_CORE)       += usbip/
> >>>     diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
> >>>     index 77048aa..17e449e 100644
> >>>     --- a/drivers/usb/common/usb-otg.c
> >>>     +++ b/drivers/usb/common/usb-otg.c
> >>>     @@ -56,6 +56,30 @@ static int usb_otg_hcd_is_primary_hcd(struct usb_hcd *hcd)
> >>>             return hcd == hcd->primary_hcd;
> >>>      }
> >>>
> >>>     +static const char *otg_state_string(enum usb_otg_state state)
> >>>     +{
> >>>     +       static const char *const names[] = {
> >>>     +               [OTG_STATE_A_IDLE] = "a_idle",
> >>>     +               [OTG_STATE_A_WAIT_VRISE] = "a_wait_vrise",
> >>>     +               [OTG_STATE_A_WAIT_BCON] = "a_wait_bcon",
> >>>     +               [OTG_STATE_A_HOST] = "a_host",
> >>>     +               [OTG_STATE_A_SUSPEND] = "a_suspend",
> >>>     +               [OTG_STATE_A_PERIPHERAL] = "a_peripheral",
> >>>     +               [OTG_STATE_A_WAIT_VFALL] = "a_wait_vfall",
> >>>     +               [OTG_STATE_A_VBUS_ERR] = "a_vbus_err",
> >>>     +               [OTG_STATE_B_IDLE] = "b_idle",
> >>>     +               [OTG_STATE_B_SRP_INIT] = "b_srp_init",
> >>>     +               [OTG_STATE_B_PERIPHERAL] = "b_peripheral",
> >>>     +               [OTG_STATE_B_WAIT_ACON] = "b_wait_acon",
> >>>     +               [OTG_STATE_B_HOST] = "b_host",
> >>>     +       };
> >>>     +
> >>>     +       if (state < 0 || state >= ARRAY_SIZE(names))
> >>>     +               return "UNDEFINED";
> >>>     +
> >>>     +       return names[state];
> >>>     +}
> >>>     +
> >>>
> >>>
> >>>
> >>> From my point, make another copy for otg stuff is not a good way,
> >>> could we make folder under usb/ named otg for dedicated otg stuffs,
> >>> in that case, build otg stuffs can not depend on USB_COMMON.
> >>
> >> OK. I can try that. I'll delete otg_state_string from usb-common.c and
> >> move it into usb/otg/usb-otg.c
> >>
> >> I'll also move usb-otg-fsm.c to usb/otg/.
> > 
> > But we can't delete usb_otg_state_string() from usb-common.c. That is used at a
> > number of places whether OTG is enabled or not. 
> > 
> > One option is to make usb-common built in when otg is enabled. What do you say?
> > 
> 
> This should also get solved if we make USB_OTG tristate so that it is same as
> USB_COMMON.
> 
> However I haven't had success in making Kconfig behave like this.
> 
> USB_OTG = y if USB == y && GADGET = m
> USB_OTG = y if USB == m && GADGET = y
> 
> Is there any Kconfig trickery to get this behaviour?
> 

Unless let the USB_OTG works like USB_COMMON which is selected by GADGET
or HCD. In fact, HCD and Gadget code uses USB OTG symbol directly in
this framework, it seems like HCD and Gadget depends on OTG, but not
otherwise.

If we want OTG to depend on HCD && GADGET, we need not to use OTG symbol
at HCD and GADGET, and the OTG can use HCD and GADGET symbol directly.

-- 

Best Regards,
Peter Chen

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v8 00/14] USB OTG/dual-role framework Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
  [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
    Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-16 09:20 +0200
      Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-16 10:30 +0200
        Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-16 11:40 +0200
          Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-16 12:00 +0200
            RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Jun Li <jun.li@nxp.com> - 2016-05-17 09:40 +0200
              Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-17 10:10 +0200
                RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Jun Li <jun.li@nxp.com> - 2016-05-17 10:50 +0200
                Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-18 14:50 +0200
                Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-18 15:50 +0200
                RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Jun Li <jun.li@nxp.com> - 2016-05-18 16:50 +0200
                Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-19 09:40 +0200
                Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-21 04:40 +0200
                Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-23 05:30 +0200
                Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-23 12:20 +0200
                RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Jun Li <jun.li@nxp.com> - 2016-05-23 12:40 +0200
                Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-23 12:40 +0200
                Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-24 05:00 +0200
                RE: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Jun Li <jun.li@nxp.com> - 2016-05-18 15:50 +0200
            Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-18 05:30 +0200
              Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-18 14:50 +0200
                Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-20 03:50 +0200
                Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-20 09:30 +0200
                Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core Peter Chen <hzpeterchen@gmail.com> - 2016-05-21 05:00 +0200
  [PATCH v8 10/14] usb: otg: add hcd companion support Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
    Re: [PATCH v8 10/14] usb: otg: add hcd companion support Rob Herring <robh@kernel.org> - 2016-05-13 20:20 +0200
      Re: [PATCH v8 10/14] usb: otg: add hcd companion support Roger Quadros <rogerq@ti.com> - 2016-05-16 10:20 +0200
    [PATCH v9 10/14] usb: otg: add hcd companion support Roger Quadros <rogerq@ti.com> - 2016-05-20 11:40 +0200
      Re: [PATCH v9 10/14] usb: otg: add hcd companion support Rob Herring <robh@kernel.org> - 2016-05-23 23:10 +0200
  [PATCH v8 11/14] usb: otg: use dev_dbg() instead of VDBG() Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
  [PATCH v8 12/14] usb: hcd: Adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
  [PATCH v8 14/14] usb: host: xhci-plat: Add otg device to platform data Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
  [PATCH v8 03/14] usb: hcd.h: Add OTG to HCD interface Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
  [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
    Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-16 11:10 +0200
      Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-18 15:10 +0200
        Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-20 10:40 +0200
          Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-20 11:20 +0200
            Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Peter Chen <hzpeterchen@gmail.com> - 2016-05-20 12:00 +0200
              Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-23 12:10 +0200
    Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-24 11:50 +0200
      Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Peter Chen <hzpeterchen@gmail.com> - 2016-05-25 04:50 +0200
        RE: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Jun Li <jun.li@nxp.com> - 2016-05-25 06:00 +0200
          Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-25 14:30 +0200
        Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-05-25 14:30 +0200
          RE: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Jun Li <jun.li@nxp.com> - 2016-05-25 16:50 +0200
  [PATCH v8 01/14] usb: hcd: Initialize hcd->flags to 0 Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
  [PATCH v8 07/14] usb: otg: get rid of CONFIG_USB_OTG_FSM in favour of CONFIG_USB_OTG Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
  [PATCH v8 05/14] usb: otg-fsm: move host controller operations into usb_otg->hcd_ops Roger Quadros <rogerq@ti.com> - 2016-05-13 12:10 +0200
  Re: [PATCH v8 00/14] USB OTG/dual-role framework Peter Chen <hzpeterchen@gmail.com> - 2016-05-30 11:40 +0200
    Re: [PATCH v8 00/14] USB OTG/dual-role framework Roger Quadros <rogerq@ti.com> - 2016-05-30 16:10 +0200

csiph-web