On 20/05/16 11:31, Roger Quadros wrote:

On 18/05/16 15:59, Roger Quadros wrote:

quoted

Hi Peter,

On 18/05/16 10:45, Peter Chen wrote:

quoted

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?

cheers,
-roger

cheers,
-roger

quoted

quoted

     /**
      * Check if the OTG device is in our wait list and return
      * otg_wait_data, else NULL.
    @@ -433,7 +457,7 @@ static void drd_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)

            fsm->state_changed = 1;
            dev_dbg(otg->dev, "otg: set state: %s\n",
    -               usb_otg_state_string(new_state));
    +               otg_state_string(new_state));
            switch (new_state) {
            case OTG_STATE_B_IDLE:
                    drd_set_protocol(fsm, PROTO_UNDEF);
    @@ -449,7 +473,7 @@ static void drd_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)
                    break;
            default:
                    dev_warn(otg->dev, "%s: otg: invalid state: %s\n",
    -                        __func__, usb_otg_state_string(new_state));
    +                        __func__, otg_state_string(new_state));
                    break;
            }

    @@ -507,7 +531,7 @@ int drd_statemachine(struct usb_otg *otg)

            default:
                    dev_err(otg->dev, "%s: otg: invalid usb-drd state: %s\n",
    -                       __func__, usb_otg_state_string(state));
    +                       __func__, otg_state_string(state));
                    break;
            }

    --
    cheers,
    -roger
    --
    To unsubscribe from this list: send the line "unsubscribe linux-usb" in
    the body of a message to majordomo@vger.kernel.org <mailto:majordomo@vger.kernel.org>
    More majordomo info at  http://vger.kernel.org/majordomo-info.html




-- 
BR,
Peter Chen

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html