diff mbox

[v3,07/11] usb: otg: add OTG core

Message ID 1436350777-28056-8-git-send-email-rogerq@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Roger Quadros July 8, 2015, 10:19 a.m. UTC
The OTG core instantiates the OTG Finite State Machine
per OTG controller and manages starting/stopping the
host and gadget controllers based on the bus state.

It provides APIs for the following tasks

- Registering an OTG capable controller
- Registering Host and Gadget controllers to OTG core
- Providing inputs to and kicking the OTG state machine

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 MAINTAINERS                  |   4 +-
 drivers/usb/Kconfig          |   2 +-
 drivers/usb/Makefile         |   1 +
 drivers/usb/common/Makefile  |   3 +-
 drivers/usb/common/usb-otg.c | 768 +++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/common/usb-otg.h |  71 ++++
 drivers/usb/core/Kconfig     |  11 +-
 include/linux/usb/otg.h      |  91 ++++-
 8 files changed, 930 insertions(+), 21 deletions(-)
 create mode 100644 drivers/usb/common/usb-otg.c
 create mode 100644 drivers/usb/common/usb-otg.h

Comments

Peter Chen July 13, 2015, 2:14 a.m. UTC | #1
On Wed, Jul 08, 2015 at 01:19:33PM +0300, Roger Quadros wrote:
> The OTG core instantiates the OTG Finite State Machine
> per OTG controller and manages starting/stopping the
> host and gadget controllers based on the bus state.
> 
> It provides APIs for the following tasks
> 
> - Registering an OTG capable controller
> - Registering Host and Gadget controllers to OTG core
> - Providing inputs to and kicking the OTG state machine
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  MAINTAINERS                  |   4 +-
>  drivers/usb/Kconfig          |   2 +-
>  drivers/usb/Makefile         |   1 +
>  drivers/usb/common/Makefile  |   3 +-
>  drivers/usb/common/usb-otg.c | 768 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/usb/common/usb-otg.h |  71 ++++
>  drivers/usb/core/Kconfig     |  11 +-
>  include/linux/usb/otg.h      |  91 ++++-
>  8 files changed, 930 insertions(+), 21 deletions(-)
>  create mode 100644 drivers/usb/common/usb-otg.c
>  create mode 100644 drivers/usb/common/usb-otg.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8133cef..b21278e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10640,12 +10640,14 @@ S:	Maintained
>  F:	Documentation/usb/ohci.txt
>  F:	drivers/usb/host/ohci*
>  
> -USB OTG FSM (Finite State Machine)
> +USB OTG/DRD core and FSM (Finite State Machine)
>  M:	Peter Chen <Peter.Chen@freescale.com>
> +M:	Roger Quadros <rogerq@ti.com>
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
>  L:	linux-usb@vger.kernel.org
>  S:	Maintained
>  F:	drivers/usb/common/usb-otg-fsm.c
> +F:	drivers/usb/common/usb-otg.c
>  
>  USB OVER IP DRIVER
>  M:	Valentina Manea <valentina.manea.m@gmail.com>
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index 8ed451d..5b625e2 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -32,7 +32,7 @@ if USB_SUPPORT
>  config USB_COMMON
>  	tristate
>  	default y
> -	depends on USB || USB_GADGET
> +	depends on USB || USB_GADGET || USB_OTG
>  

USB_OTG can depends on USB || UB_GADGET?

>  config USB_ARCH_HAS_HCD
>  	def_bool y
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index d8926c6..769d13b 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -60,5 +60,6 @@ obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
>  obj-$(CONFIG_USB_GADGET)	+= gadget/
>  
>  obj-$(CONFIG_USB_COMMON)	+= common/
> +obj-$(CONFIG_USB_OTG)		+= common/
>  

The comment like above.

>  obj-$(CONFIG_USBIP_CORE)	+= usbip/
> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
> index 6bbb3ec..730d928 100644
> --- a/drivers/usb/common/Makefile
> +++ b/drivers/usb/common/Makefile
> @@ -6,5 +6,6 @@ obj-$(CONFIG_USB_COMMON)	  += usb-common.o
>  usb-common-y			  += common.o
>  usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>  
> -obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
>  obj-$(CONFIG_USB_ULPI_BUS)	+= ulpi.o
> +usbotg-y		:= usb-otg.o usb-otg-fsm.o
> +obj-$(CONFIG_USB_OTG)	+= usbotg.o
> diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
> new file mode 100644
> index 0000000..0379034
> --- /dev/null
> +++ b/drivers/usb/common/usb-otg.c
> @@ -0,0 +1,768 @@
> +/**
> + * drivers/usb/common/usb-otg.c - USB OTG core
> + *
> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
> + * Author: Roger Quadros <rogerq@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/ktime.h>
> +#include <linux/hrtimer.h>
> +#include <linux/list.h>
> +#include <linux/usb/otg.h>
> +#include <linux/usb/phy.h>	/* enum usb_otg_state */
> +#include <linux/usb/gadget.h>
> +#include <linux/workqueue.h>
> +
> +#include "usb-otg.h"
> +
> +/* to link timer with callback data */
> +struct otg_timer {
> +	struct hrtimer timer;
> +	ktime_t timeout;
> +	/* callback data */
> +	int *timeout_bit;
> +	struct otg_data *otgd;
> +};
> +
> +struct otg_hcd {
> +	struct usb_hcd *hcd;
> +	unsigned int irqnum;
> +	unsigned long irqflags;
> +	struct otg_hcd_ops *ops;
> +};
> +
> +struct otg_data {
> +	struct device *dev;	/* HCD & GCD's parent device */
> +
> +	struct otg_fsm fsm;
> +	/* HCD, GCD and usb_otg_state are present in otg_fsm->otg
> +	 * HCD is bus_to_hcd(fsm->otg->host)
> +	 * GCD is fsm->otg->gadget
> +	 */
> +	struct otg_fsm_ops fsm_ops;	/* private copy for override */
> +	struct usb_otg otg;		/* allocator for fsm->otg */
> +
> +	struct otg_hcd primary_hcd;
> +	struct otg_hcd shared_hcd;
> +
> +	struct otg_gadget_ops *gadget_ops; /* interface to gadget f/w */
> +
> +	/* saved hooks to OTG device */
> +	int (*start_host)(struct otg_fsm *fsm, int on);
> +	int (*start_gadget)(struct otg_fsm *fsm, int on);
> +
> +	struct list_head list;
> +
> +	struct work_struct work;	/* OTG FSM work */
> +	struct workqueue_struct *wq;
> +
> +	struct otg_timer timers[NUM_OTG_FSM_TIMERS];
> +
> +	bool fsm_running;
> +	/* use otg->fsm.lock for serializing access */
> +};
> +

Would you consider enhance struct usb_otg, and instead of this
structure, I am wonder similar structures may increase the confuse
in future.

Peter

> +/* OTG device list */
> +LIST_HEAD(otg_list);
> +static DEFINE_MUTEX(otg_list_mutex);
> +
> +static int usb_otg_hcd_is_primary_hcd(struct usb_hcd *hcd)
> +{
> +	if (!hcd->primary_hcd)
> +		return 1;
> +	return hcd == hcd->primary_hcd;
> +}
> +
> +/**
> + * check if device is in our OTG list and return
> + * otg_data, else NULL.
> + *
> + * otg_list_mutex must be held.
> + */
> +static struct otg_data *usb_otg_device_get_otgd(struct device *parent_dev)
> +{
> +	struct otg_data *otgd;
> +
> +	list_for_each_entry(otgd, &otg_list, list) {
> +		if (otgd->dev == parent_dev)
> +			return otgd;
> +	}
> +
> +	return NULL;
> +}
> +
> +/**
> + * timer callback to set timeout bit and kick FSM
> + */
> +static enum hrtimer_restart set_tmout(struct hrtimer *data)
> +{
> +	struct otg_timer *otgtimer;
> +
> +	otgtimer = container_of(data, struct otg_timer, timer);
> +	if (otgtimer->timeout_bit)
> +		*otgtimer->timeout_bit = 1;
> +
> +	usb_otg_sync_inputs(&otgtimer->otgd->fsm);
> +
> +	return HRTIMER_NORESTART;
> +}
> +
> +/**
> + * Initialize one OTG timer with callback, timeout and timeout bit
> + */
> +static void otg_timer_init(enum otg_fsm_timer id, struct otg_data *otgd,
> +			   enum hrtimer_restart (*callback)(struct hrtimer *),
> +			   unsigned long expires_ms,
> +			   int *timeout_bit)
> +{
> +	struct otg_timer *otgtimer = &otgd->timers[id];
> +	struct hrtimer *timer = &otgtimer->timer;
> +
> +	otgtimer->timeout = ms_to_ktime(expires_ms);
> +	hrtimer_init(timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +	timer->function = callback;
> +
> +	otgtimer->timeout_bit = timeout_bit;
> +	otgtimer->otgd = otgd;
> +}
> +
> +/**
> + * Initialize standard OTG timers
> + */
> +static void usb_otg_init_timers(struct otg_data *otgd)
> +{
> +	struct otg_fsm *fsm = &otgd->fsm;
> +
> +	otg_timer_init(A_WAIT_VRISE, otgd, set_tmout, TA_WAIT_VRISE,
> +		       &fsm->a_wait_vrise_tmout);
> +	otg_timer_init(A_WAIT_VFALL, otgd, set_tmout, TA_WAIT_VFALL,
> +		       &fsm->a_wait_vfall_tmout);
> +	otg_timer_init(A_WAIT_BCON, otgd, set_tmout, TA_WAIT_BCON,
> +		       &fsm->a_wait_bcon_tmout);
> +	otg_timer_init(A_AIDL_BDIS, otgd, set_tmout, TA_AIDL_BDIS,
> +		       &fsm->a_aidl_bdis_tmout);
> +	otg_timer_init(A_BIDL_ADIS, otgd, set_tmout, TA_BIDL_ADIS,
> +		       &fsm->a_bidl_adis_tmout);
> +	otg_timer_init(B_ASE0_BRST, otgd, set_tmout, TB_ASE0_BRST,
> +		       &fsm->b_ase0_brst_tmout);
> +
> +	otg_timer_init(B_SE0_SRP, otgd, set_tmout, TB_SE0_SRP, &fsm->b_se0_srp);
> +	otg_timer_init(B_SRP_FAIL, otgd, set_tmout, TB_SRP_FAIL,
> +		       &fsm->b_srp_done);
> +
> +	otg_timer_init(A_WAIT_ENUM, otgd, set_tmout, TB_SRP_FAIL, NULL);
> +}
> +
> +/**
> + * OTG FSM ops function to add timer
> + */
> +static void usb_otg_add_timer(struct otg_fsm *fsm, enum otg_fsm_timer id)
> +{
> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> +	struct otg_timer *otgtimer = &otgd->timers[id];
> +	struct hrtimer *timer = &otgtimer->timer;
> +
> +	if (!otgd->fsm_running)
> +		return;
> +
> +	/* if timer is already active, exit */
> +	if (hrtimer_active(timer)) {
> +		dev_err(otgd->dev, "otg: timer %d is already running\n", id);
> +		return;
> +	}
> +
> +	hrtimer_start(timer, otgtimer->timeout, HRTIMER_MODE_REL);
> +}
> +
> +/**
> + * OTG FSM ops function to delete timer
> + */
> +static void usb_otg_del_timer(struct otg_fsm *fsm, enum otg_fsm_timer id)
> +{
> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> +	struct hrtimer *timer = &otgd->timers[id].timer;
> +
> +	hrtimer_cancel(timer);
> +}
> +
> +/**
> + * OTG FSM ops function to start/stop host
> + */
> +static int usb_otg_start_host(struct otg_fsm *fsm, int on)
> +{
> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> +	struct otg_hcd_ops *hcd_ops;
> +
> +	dev_dbg(otgd->dev, "otg: %s %d\n", __func__, on);
> +	if (!fsm->otg->host) {
> +		WARN_ONCE(1, "otg: fsm running without host\n");
> +		return 0;
> +	}
> +
> +	if (on) {
> +		/* OTG device operations */
> +		if (otgd->start_host)
> +			otgd->start_host(fsm, on);
> +
> +		/* start host */
> +		hcd_ops = otgd->primary_hcd.ops;
> +		hcd_ops->add(otgd->primary_hcd.hcd, otgd->primary_hcd.irqnum,
> +			     otgd->primary_hcd.irqflags);
> +		if (otgd->shared_hcd.hcd) {
> +			hcd_ops = otgd->shared_hcd.ops;
> +			hcd_ops->add(otgd->shared_hcd.hcd,
> +				     otgd->shared_hcd.irqnum,
> +				     otgd->shared_hcd.irqflags);
> +		}
> +	} else {
> +		/* stop host */
> +		if (otgd->shared_hcd.hcd) {
> +			hcd_ops = otgd->shared_hcd.ops;
> +			hcd_ops->remove(otgd->shared_hcd.hcd);
> +		}
> +		hcd_ops = otgd->primary_hcd.ops;
> +		hcd_ops->remove(otgd->primary_hcd.hcd);
> +
> +		/* OTG device operations */
> +		if (otgd->start_host)
> +			otgd->start_host(fsm, on);
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * OTG FSM ops function to start/stop gadget
> + */
> +static int usb_otg_start_gadget(struct otg_fsm *fsm, int on)
> +{
> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> +	struct usb_gadget *gadget = fsm->otg->gadget;
> +
> +	dev_dbg(otgd->dev, "otg: %s %d\n", __func__, on);
> +	if (!gadget) {
> +		WARN_ONCE(1, "otg: fsm running without gadget\n");
> +		return 0;
> +	}
> +
> +	if (on) {
> +		/* OTG device operations */
> +		if (otgd->start_gadget)
> +			otgd->start_gadget(fsm, on);
> +
> +		otgd->gadget_ops->start(fsm->otg->gadget);
> +	} else {
> +		otgd->gadget_ops->stop(fsm->otg->gadget);
> +
> +		/* OTG device operations */
> +		if (otgd->start_gadget)
> +			otgd->start_gadget(fsm, on);
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * OTG FSM work function
> + */
> +static void usb_otg_work(struct work_struct *work)
> +{
> +	struct otg_data *otgd = container_of(work, struct otg_data, work);
> +
> +	otg_statemachine(&otgd->fsm);
> +}
> +
> +/**
> + * usb_otg_register() - Register the OTG device to OTG core
> + * @parent_device:	parent device of Host & Gadget controllers.
> + * @otg_fsm_ops:	otg state machine ops.
> + *
> + * Register parent device that contains both HCD and GCD into
> + * the USB OTG core. HCD and GCD will be prevented from starting
> + * till both are available for use.
> + *
> + * Return: struct otg_fsm * if success, NULL if error.
> + */
> +struct otg_fsm *usb_otg_register(struct device *parent_dev,
> +				 struct otg_fsm_ops *fsm_ops)
> +{
> +	struct otg_data *otgd;
> +	int ret = 0;
> +
> +	if (!parent_dev || !fsm_ops)
> +		return ERR_PTR(-EINVAL);
> +
> +	/* already in list? */
> +	mutex_lock(&otg_list_mutex);
> +	if (usb_otg_device_get_otgd(parent_dev)) {
> +		dev_err(parent_dev, "otg: %s: device already in otg list\n",
> +			__func__);
> +		ret = -EINVAL;
> +		goto unlock;
> +	}
> +
> +	/* allocate and add to list */
> +	otgd = kzalloc(sizeof(*otgd), GFP_KERNEL);
> +	if (!otgd) {
> +		ret = -ENOMEM;
> +		goto unlock;
> +	}
> +
> +	otgd->dev = parent_dev;
> +	INIT_WORK(&otgd->work, usb_otg_work);
> +	otgd->wq = create_singlethread_workqueue("usb_otg");
> +	if (!otgd->wq) {
> +		dev_err(parent_dev, "otg: %s: can't create workqueue\n",
> +			__func__);
> +		ret = -ENODEV;
> +		goto err_wq;
> +	}
> +
> +	usb_otg_init_timers(otgd);
> +
> +	/* save original start host/gadget ops */
> +	otgd->start_host = fsm_ops->start_host;
> +	otgd->start_gadget = fsm_ops->start_gadget;
> +	/* create copy of original ops */
> +	otgd->fsm_ops = *fsm_ops;
> +	/* override ops */
> +	otgd->fsm_ops.start_host = usb_otg_start_host;
> +	otgd->fsm_ops.start_gadget = usb_otg_start_gadget;
> +	/* FIXME: we ignore caller's timer ops */
> +	otgd->fsm_ops.add_timer = usb_otg_add_timer;
> +	otgd->fsm_ops.del_timer = usb_otg_del_timer;
> +	/* set otg ops */
> +	otgd->fsm.ops = &otgd->fsm_ops;
> +	otgd->fsm.otg = &otgd->otg;
> +
> +	mutex_init(&otgd->fsm.lock);
> +
> +	list_add_tail(&otgd->list, &otg_list);
> +	mutex_unlock(&otg_list_mutex);
> +	return &otgd->fsm;
> +
> +err_wq:
> +	kfree(otgd);
> +unlock:
> +	mutex_unlock(&otg_list_mutex);
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_register);
> +
> +/**
> + * usb_otg_unregister() - Unregister the OTG device from USB OTG core
> + * @parent_device:	parent device of Host & Gadget controllers.
> + *
> + * Unregister parent OTG deviced from USB OTG core.
> + * Prevents unregistering till both Host and Gadget controllers
> + * have unregistered from the OTG core.
> + *
> + * Return: 0 on success, error value otherwise.
> + */
> +int usb_otg_unregister(struct device *parent_dev)
> +{
> +	struct otg_data *otgd;
> +
> +	mutex_lock(&otg_list_mutex);
> +	otgd = usb_otg_device_get_otgd(parent_dev);
> +	if (!otgd) {
> +		dev_err(parent_dev, "otg: %s: device not in otg list\n",
> +			__func__);
> +		mutex_unlock(&otg_list_mutex);
> +		return -EINVAL;
> +	}
> +
> +	/* prevent unregister till both host & gadget have unregistered */
> +	if (otgd->fsm.otg->host || otgd->fsm.otg->gadget) {
> +		dev_err(parent_dev, "otg: %s: host/gadget still registered\n",
> +			__func__);
> +		return -EBUSY;
> +	}
> +
> +	/* OTG FSM is halted when host/gadget unregistered */
> +	destroy_workqueue(otgd->wq);
> +
> +	/* remove from otg list */
> +	list_del(&otgd->list);
> +	kfree(otgd);
> +	mutex_unlock(&otg_list_mutex);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_unregister);
> +
> +/**
> + * start/kick the OTG FSM if we can
> + * fsm->lock must be held
> + */
> +static void usb_otg_start_fsm(struct otg_fsm *fsm)
> +{
> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> +
> +	if (otgd->fsm_running)
> +		goto kick_fsm;
> +
> +	if (!fsm->otg->host) {
> +		dev_info(otgd->dev, "otg: can't start till host registers\n");
> +		return;
> +	}
> +
> +	if (!fsm->otg->gadget) {
> +		dev_info(otgd->dev, "otg: can't start till gadget registers\n");
> +		return;
> +	}
> +
> +	otgd->fsm_running = true;
> +kick_fsm:
> +	queue_work(otgd->wq, &otgd->work);
> +}
> +
> +/**
> + * stop the OTG FSM. Stops Host & Gadget controllers as well.
> + * fsm->lock must be held
> + */
> +static void usb_otg_stop_fsm(struct otg_fsm *fsm)
> +{
> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> +	int i;
> +
> +	if (!otgd->fsm_running)
> +		return;
> +
> +	/* no more new events queued */
> +	otgd->fsm_running = false;
> +
> +	/* Stop state machine / timers */
> +	for (i = 0; i < ARRAY_SIZE(otgd->timers); i++)
> +		hrtimer_cancel(&otgd->timers[i].timer);
> +
> +	flush_workqueue(otgd->wq);
> +	fsm->otg->state = OTG_STATE_UNDEFINED;
> +
> +	/* stop host/gadget immediately */
> +	if (fsm->protocol == PROTO_HOST)
> +		otg_start_host(fsm, 0);
> +	else if (fsm->protocol == PROTO_GADGET)
> +		otg_start_gadget(fsm, 0);
> +	fsm->protocol = PROTO_UNDEF;
> +}
> +
> +/**
> + * usb_otg_sync_inputs - Sync OTG inputs with the OTG state machine
> + * @fsm:	OTG FSM instance
> + *
> + * Used by the OTG driver to update the inputs to the OTG
> + * state machine.
> + *
> + * Can be called in IRQ context.
> + */
> +void usb_otg_sync_inputs(struct otg_fsm *fsm)
> +{
> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> +
> +	/* Don't kick FSM till it has started */
> +	if (!otgd->fsm_running)
> +		return;
> +
> +	/* Kick FSM */
> +	queue_work(otgd->wq, &otgd->work);
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_sync_inputs);
> +
> +/**
> + * usb_otg_kick_fsm - Kick the OTG state machine
> + * @hcd_gcd_device:	Host/Gadget controller device
> + *
> + * Used by USB host/device stack to sync OTG related
> + * events to the OTG state machine.
> + * e.g. change in host_bus->b_hnp_enable, gadget->b_hnp_enable
> + *
> + * Returns: 0 on success, error value otherwise.
> + */
> +int usb_otg_kick_fsm(struct device *hcd_gcd_device)
> +{
> +	struct otg_data *otgd;
> +
> +	mutex_lock(&otg_list_mutex);
> +	otgd = usb_otg_device_get_otgd(hcd_gcd_device->parent);
> +	if (!otgd) {
> +		dev_err(hcd_gcd_device, "otg: %s: invalid host/gadget device\n",
> +			__func__);
> +		mutex_unlock(&otg_list_mutex);
> +		return -ENODEV;
> +	}
> +
> +	mutex_unlock(&otg_list_mutex);
> +	usb_otg_sync_inputs(&otgd->fsm);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_kick_fsm);
> +
> +/**
> + * usb_otg_register_hcd - Register Host controller to OTG core
> + * @hcd:	Host controller device
> + * @irqnum:	interrupt number
> + * @irqflags:	interrupt flags
> + * @ops:	HCD ops to add/remove the HCD
> + *
> + * This is used by the USB Host stack to register the Host controller
> + * to the OTG core. Host controller must not be started by the
> + * caller as it is left upto the OTG state machine to do so.
> + *
> + * Returns: 0 on success, error value otherwise.
> + */
> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
> +			 unsigned long irqflags, struct otg_hcd_ops *ops)
> +{
> +	struct otg_data *otgd;
> +	struct device *otg_dev = hcd->self.controller->parent;
> +
> +	mutex_lock(&otg_list_mutex);
> +	otgd = usb_otg_device_get_otgd(otg_dev);
> +	if (!otgd) {
> +		dev_dbg(otg_dev, "otg: %s: device not registered to otg core\n",
> +			__func__);
> +		mutex_unlock(&otg_list_mutex);
> +		return -EINVAL;
> +	}
> +
> +	mutex_unlock(&otg_list_mutex);
> +	/* HCD will be started by OTG fsm when needed */
> +	mutex_lock(&otgd->fsm.lock);
> +	if (otgd->primary_hcd.hcd) {
> +		/* probably a shared HCD ? */
> +		if (usb_otg_hcd_is_primary_hcd(hcd)) {
> +			dev_err(otg_dev, "otg: primary host already registered\n");
> +			goto err;
> +		}
> +
> +		if (hcd->shared_hcd == otgd->primary_hcd.hcd) {
> +			if (otgd->shared_hcd.hcd) {
> +				dev_err(otg_dev, "otg: shared host already registered\n");
> +				goto err;
> +			}
> +
> +			otgd->shared_hcd.hcd = hcd;
> +			otgd->shared_hcd.irqnum = irqnum;
> +			otgd->shared_hcd.irqflags = irqflags;
> +			otgd->shared_hcd.ops = ops;
> +			dev_info(otg_dev, "otg: shared host %s registered\n",
> +				 dev_name(hcd->self.controller));
> +		} else {
> +			dev_err(otg_dev, "otg: invalid shared host %s\n",
> +				dev_name(hcd->self.controller));
> +			goto err;
> +		}
> +	} else {
> +		if (!usb_otg_hcd_is_primary_hcd(hcd)) {
> +			dev_err(otg_dev, "otg: primary host must be registered first\n");
> +			goto err;
> +		}
> +
> +		otgd->primary_hcd.hcd = hcd;
> +		otgd->primary_hcd.irqnum = irqnum;
> +		otgd->primary_hcd.irqflags = irqflags;
> +		otgd->primary_hcd.ops = ops;
> +		dev_info(otg_dev, "otg: primary host %s registered\n",
> +			 dev_name(hcd->self.controller));
> +	}
> +
> +	/*
> +	 * we're ready only if we have shared HCD
> +	 * or we don't need shared HCD.
> +	 */
> +	if (otgd->shared_hcd.hcd || !otgd->primary_hcd.hcd->shared_hcd) {
> +		otgd->fsm.otg->host = hcd_to_bus(hcd);
> +		/* FIXME: set bus->otg_port if this is true OTG port with HNP */
> +
> +		/* start FSM */
> +		usb_otg_start_fsm(&otgd->fsm);
> +	} else {
> +		dev_dbg(otg_dev, "otg: can't start till shared host registers\n");
> +	}
> +
> +	mutex_unlock(&otgd->fsm.lock);
> +
> +	return 0;
> +
> +err:
> +	mutex_unlock(&otgd->fsm.lock);
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_register_hcd);
> +
> +/**
> + * usb_otg_unregister_hcd - Unregister Host controller from OTG core
> + * @hcd:	Host controller device
> + *
> + * This is used by the USB Host stack to unregister the Host controller
> + * from the OTG core. Ensures that Host controller is not running
> + * on successful return.
> + *
> + * Returns: 0 on success, error value otherwise.
> + */
> +int usb_otg_unregister_hcd(struct usb_hcd *hcd)
> +{
> +	struct otg_data *otgd;
> +	struct usb_bus *bus = hcd_to_bus(hcd);
> +	struct device *otg_dev = bus->controller->parent;
> +
> +	mutex_lock(&otg_list_mutex);
> +	otgd = usb_otg_device_get_otgd(otg_dev);
> +	if (!otgd) {
> +		dev_err(otg_dev, "otg: %s: device not registered to otg core\n",
> +			__func__);
> +		mutex_unlock(&otg_list_mutex);
> +		return -EINVAL;
> +	}
> +
> +	mutex_unlock(&otg_list_mutex);
> +
> +	mutex_lock(&otgd->fsm.lock);
> +	if (hcd == otgd->primary_hcd.hcd) {
> +		otgd->primary_hcd.hcd = NULL;
> +		dev_info(otg_dev, "otg: primary host %s unregistered\n",
> +			 dev_name(bus->controller));
> +	} else if (hcd == otgd->shared_hcd.hcd) {
> +		otgd->shared_hcd.hcd = NULL;
> +		dev_info(otg_dev, "otg: shared host %s unregistered\n",
> +			 dev_name(bus->controller));
> +	} else {
> +		dev_err(otg_dev, "otg: host %s wasn't registered with otg\n",
> +			dev_name(bus->controller));
> +		mutex_unlock(&otgd->fsm.lock);
> +		return -EINVAL;
> +	}
> +
> +	/* stop FSM & Host */
> +	usb_otg_stop_fsm(&otgd->fsm);
> +	otgd->fsm.otg->host = NULL;
> +
> +	mutex_unlock(&otgd->fsm.lock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_unregister_hcd);
> +
> +/**
> + * usb_otg_register_gadget - Register Gadget controller to OTG core
> + * @gadget:	Gadget controller
> + *
> + * This is used by the USB Gadget stack to register the Gadget controller
> + * to the OTG core. Gadget controller must not be started by the
> + * caller as it is left upto the OTG state machine to do so.
> + *
> + * Gadget core must call this only when all resources required for
> + * gadget controller to run are available.
> + * i.e. gadget function driver is available.
> + *
> + * Returns: 0 on success, error value otherwise.
> + */
> +int usb_otg_register_gadget(struct usb_gadget *gadget,
> +			    struct otg_gadget_ops *ops)
> +{
> +	struct otg_data *otgd;
> +	struct device *otg_dev = gadget->dev.parent;
> +
> +	mutex_lock(&otg_list_mutex);
> +	otgd = usb_otg_device_get_otgd(otg_dev);
> +	if (!otgd) {
> +		dev_err(otg_dev, "otg: %s: device not registered to otg core\n",
> +			__func__);
> +		mutex_unlock(&otg_list_mutex);
> +		return -EINVAL;
> +	}
> +
> +	mutex_unlock(&otg_list_mutex);
> +
> +	mutex_lock(&otgd->fsm.lock);
> +	if (otgd->fsm.otg->gadget) {
> +		dev_err(otg_dev, "otg: gadget already registered with otg\n");
> +		mutex_unlock(&otgd->fsm.lock);
> +		return -EINVAL;
> +	}
> +
> +	otgd->fsm.otg->gadget = gadget;
> +	otgd->gadget_ops = ops;
> +	dev_info(otg_dev, "otg: gadget %s registered\n",
> +		 dev_name(&gadget->dev));
> +
> +	/* start FSM */
> +	usb_otg_start_fsm(&otgd->fsm);
> +	mutex_unlock(&otgd->fsm.lock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_register_gadget);
> +
> +/**
> + * usb_otg_unregister_gadget - Unregister Gadget controller from OTG core
> + * @gadget:	Gadget controller
> + *
> + * This is used by the USB Gadget stack to unregister the Gadget controller
> + * from the OTG core. Ensures that Gadget controller is not running
> + * on successful return.
> + *
> + * Returns: 0 on success, error value otherwise.
> + */
> +int usb_otg_unregister_gadget(struct usb_gadget *gadget)
> +{
> +	struct otg_data *otgd;
> +	struct device *otg_dev = gadget->dev.parent;
> +
> +	mutex_lock(&otg_list_mutex);
> +	otgd = usb_otg_device_get_otgd(otg_dev);
> +	if (!otgd) {
> +		dev_err(otg_dev, "otg: %s: device not registered to otg core\n",
> +			__func__);
> +		mutex_unlock(&otg_list_mutex);
> +		return -EINVAL;
> +	}
> +
> +	mutex_unlock(&otg_list_mutex);
> +
> +	mutex_lock(&otgd->fsm.lock);
> +	if (otgd->fsm.otg->gadget != gadget) {
> +		dev_err(otg_dev, "otg: gadget %s wasn't registered with otg\n",
> +			dev_name(&gadget->dev));
> +		mutex_unlock(&otgd->fsm.lock);
> +		return -EINVAL;
> +	}
> +
> +	/* Stop FSM & gadget */
> +	usb_otg_stop_fsm(&otgd->fsm);
> +	otgd->fsm.otg->gadget = NULL;
> +	mutex_unlock(&otgd->fsm.lock);
> +
> +	dev_info(otg_dev, "otg: gadget %s unregistered\n",
> +		 dev_name(&gadget->dev));
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_unregister_gadget);
> +
> +/**
> + * usb_otg_fsm_to_dev - Get OTG controller device from struct otg_fsm
> + * @fsm:	otg_fsm data structure
> + *
> + * This is used by the OTG controller driver to get it's device node
> + * from any of the otg_fsm->ops.
> + */
> +struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm)
> +{
> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> +
> +	return otgd->dev;
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_fsm_to_dev);
> diff --git a/drivers/usb/common/usb-otg.h b/drivers/usb/common/usb-otg.h
> new file mode 100644
> index 0000000..05331f0
> --- /dev/null
> +++ b/drivers/usb/common/usb-otg.h
> @@ -0,0 +1,71 @@
> +/**
> + * drivers/usb/common/usb-otg.h - USB OTG core local header
> + *
> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
> + * Author: Roger Quadros <rogerq@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __DRIVERS_USB_COMMON_USB_OTG_H
> +#define __DRIVERS_USB_COMMON_USB_OTG_H
> +
> +/*
> + *  A-DEVICE timing constants
> + */
> +
> +/* Wait for VBUS Rise  */
> +#define TA_WAIT_VRISE        (100)	/* a_wait_vrise: section 7.1.2
> +					 * a_wait_vrise_tmr: section 7.4.5.1
> +					 * TA_VBUS_RISE <= 100ms, section 4.4
> +					 * Table 4-1: Electrical Characteristics
> +					 * ->DC Electrical Timing
> +					 */
> +/* Wait for VBUS Fall  */
> +#define TA_WAIT_VFALL        (1000)	/* a_wait_vfall: section 7.1.7
> +					 * a_wait_vfall_tmr: section: 7.4.5.2
> +					 */
> +/* Wait for B-Connect */
> +#define TA_WAIT_BCON         (10000)	/* a_wait_bcon: section 7.1.3
> +					 * TA_WAIT_BCON: should be between 1100
> +					 * and 30000 ms, section 5.5, Table 5-1
> +					 */
> +/* A-Idle to B-Disconnect */
> +#define TA_AIDL_BDIS         (5000)	/* a_suspend min 200 ms, section 5.2.1
> +					 * TA_AIDL_BDIS: section 5.5, Table 5-1
> +					 */
> +/* B-Idle to A-Disconnect */
> +#define TA_BIDL_ADIS         (500)	/* TA_BIDL_ADIS: section 5.2.1
> +					 * 500ms is used for B switch to host
> +					 * for safe
> +					 */
> +
> +/*
> + * B-device timing constants
> + */
> +
> +/* Data-Line Pulse Time*/
> +#define TB_DATA_PLS          (10)	/* b_srp_init,continue 5~10ms
> +					 * section:5.1.3
> +					 */
> +/* SRP Fail Time  */
> +#define TB_SRP_FAIL          (6000)	/* b_srp_init,fail time 5~6s
> +					 * section:5.1.6
> +					 */
> +/* A-SE0 to B-Reset  */
> +#define TB_ASE0_BRST         (155)	/* minimum 155 ms, section:5.3.1 */
> +/* SE0 Time Before SRP */
> +#define TB_SE0_SRP           (1000)	/* b_idle,minimum 1s, section:5.1.2 */
> +/* SSEND time before SRP */
> +#define TB_SSEND_SRP         (1500)	/* minimum 1.5 sec, section:5.1.2 */
> +
> +#define TB_SESS_VLD          (1000)
> +
> +#endif /* __DRIVERS_USB_COMMON_USB_OTG_H */
> diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
> index a99c89e..b468a9f 100644
> --- a/drivers/usb/core/Kconfig
> +++ b/drivers/usb/core/Kconfig
> @@ -42,7 +42,7 @@ config USB_DYNAMIC_MINORS
>  	  If you are unsure about this, say N here.
>  
>  config USB_OTG
> -	bool "OTG support"
> +	bool "OTG/Dual-role support"
>  	depends on PM
>  	default n
>  	help
> @@ -75,15 +75,6 @@ config USB_OTG_BLACKLIST_HUB
>  	  and software costs by not supporting external hubs.  So
>  	  are "Embedded Hosts" that don't offer OTG support.
>  
> -config USB_OTG_FSM
> -	tristate "USB 2.0 OTG FSM implementation"
> -	depends on USB
> -	select USB_OTG
> -	select USB_PHY
> -	help
> -	  Implements OTG Finite State Machine as specified in On-The-Go
> -	  and Embedded Host Supplement to the USB Revision 2.0 Specification.
> -
>  config USB_ULPI_BUS
>  	tristate "USB ULPI PHY interface support"
>  	depends on USB_SUPPORT
> diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
> index 52661c5..ce6f8d8 100644
> --- a/include/linux/usb/otg.h
> +++ b/include/linux/usb/otg.h
> @@ -10,6 +10,11 @@
>  #define __LINUX_USB_OTG_H
>  
>  #include <linux/phy/phy.h>
> +#include <linux/device.h>
> +#include <linux/usb.h>
> +#include <linux/usb/hcd.h>
> +#include <linux/usb/gadget.h>
> +#include <linux/usb/otg-fsm.h>
>  #include <linux/usb/phy.h>
>  
>  struct usb_otg {
> @@ -23,6 +28,7 @@ struct usb_otg {
>  
>  	enum usb_otg_state	state;
>  
> +/*------------- deprecated interface -----------------------------*/
>  	/* bind/unbind the host controller */
>  	int	(*set_host)(struct usb_otg *otg, struct usb_bus *host);
>  
> @@ -38,11 +44,85 @@ struct usb_otg {
>  
>  	/* start or continue HNP role switch */
>  	int	(*start_hnp)(struct usb_otg *otg);
> -
> +/*---------------------------------------------------------------*/
>  };
>  
>  extern const char *usb_otg_state_string(enum usb_otg_state state);
>  
> +enum usb_dr_mode {
> +	USB_DR_MODE_UNKNOWN,
> +	USB_DR_MODE_HOST,
> +	USB_DR_MODE_PERIPHERAL,
> +	USB_DR_MODE_OTG,
> +};
> +
> +#if IS_ENABLED(CONFIG_USB_OTG)
> +struct otg_fsm *usb_otg_register(struct device *parent_dev,
> +				 struct otg_fsm_ops *fsm_ops);
> +int usb_otg_unregister(struct device *parent_dev);
> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
> +			 unsigned long irqflags, struct otg_hcd_ops *ops);
> +int usb_otg_unregister_hcd(struct usb_hcd *hcd);
> +int usb_otg_register_gadget(struct usb_gadget *gadget,
> +			    struct otg_gadget_ops *ops);
> +int usb_otg_unregister_gadget(struct usb_gadget *gadget);
> +void usb_otg_sync_inputs(struct otg_fsm *fsm);
> +int usb_otg_kick_fsm(struct device *hcd_gcd_device);
> +struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm);
> +
> +#else /* CONFIG_USB_OTG */
> +
> +static inline struct otg_fsm *usb_otg_register(struct device *parent_dev,
> +					       struct otg_fsm_ops *fsm_ops)
> +{
> +	return ERR_PTR(-ENOSYS);
> +}
> +
> +static inline int usb_otg_unregister(struct device *parent_dev)
> +{
> +	return -ENOSYS;
> +}
> +
> +static inline int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
> +				       unsigned long irqflags,
> +				       struct otg_hcd_ops *ops)
> +{
> +	return -ENOSYS;
> +}
> +
> +static inline int usb_otg_unregister_hcd(struct usb_hcd *hcd)
> +{
> +	return -ENOSYS;
> +}
> +
> +static inline int usb_otg_register_gadget(struct usb_gadget *gadget,
> +					  struct otg_gadget_ops *ops)
> +{
> +	return -ENOSYS;
> +}
> +
> +static inline int usb_otg_unregister_gadget(struct usb_gadget *gadget)
> +{
> +	return -ENOSYS;
> +}
> +
> +static inline void usb_otg_sync_inputs(struct otg_fsm *fsm)
> +{
> +}
> +
> +static inline int usb_otg_kick_fsm(struct device *hcd_gcd_device)
> +{
> +	return -ENOSYS;
> +}
> +
> +static inline struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm)
> +{
> +	return NULL;
> +}
> +
> +#endif /* CONFIG_USB_OTG */
> +
> +/*------------- deprecated interface -----------------------------*/
>  /* Context: can sleep */
>  static inline int
>  otg_start_hnp(struct usb_otg *otg)
> @@ -94,14 +174,9 @@ otg_start_srp(struct usb_otg *otg)
>  	return -ENOTSUPP;
>  }
>  
> +/*---------------------------------------------------------------*/
> +
>  /* for OTG controller drivers (and maybe other stuff) */
>  extern int usb_bus_start_enum(struct usb_bus *bus, unsigned port_num);
>  
> -enum usb_dr_mode {
> -	USB_DR_MODE_UNKNOWN,
> -	USB_DR_MODE_HOST,
> -	USB_DR_MODE_PERIPHERAL,
> -	USB_DR_MODE_OTG,
> -};
> -
>  #endif /* __LINUX_USB_OTG_H */
> -- 
> 2.1.4
>
Roger Quadros July 13, 2015, 10:20 a.m. UTC | #2
On 13/07/15 05:14, Peter Chen wrote:
> On Wed, Jul 08, 2015 at 01:19:33PM +0300, Roger Quadros wrote:
>> The OTG core instantiates the OTG Finite State Machine
>> per OTG controller and manages starting/stopping the
>> host and gadget controllers based on the bus state.
>>
>> It provides APIs for the following tasks
>>
>> - Registering an OTG capable controller
>> - Registering Host and Gadget controllers to OTG core
>> - Providing inputs to and kicking the OTG state machine
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  MAINTAINERS                  |   4 +-
>>  drivers/usb/Kconfig          |   2 +-
>>  drivers/usb/Makefile         |   1 +
>>  drivers/usb/common/Makefile  |   3 +-
>>  drivers/usb/common/usb-otg.c | 768 +++++++++++++++++++++++++++++++++++++++++++
>>  drivers/usb/common/usb-otg.h |  71 ++++
>>  drivers/usb/core/Kconfig     |  11 +-
>>  include/linux/usb/otg.h      |  91 ++++-
>>  8 files changed, 930 insertions(+), 21 deletions(-)
>>  create mode 100644 drivers/usb/common/usb-otg.c
>>  create mode 100644 drivers/usb/common/usb-otg.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 8133cef..b21278e 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -10640,12 +10640,14 @@ S:	Maintained
>>  F:	Documentation/usb/ohci.txt
>>  F:	drivers/usb/host/ohci*
>>  
>> -USB OTG FSM (Finite State Machine)
>> +USB OTG/DRD core and FSM (Finite State Machine)
>>  M:	Peter Chen <Peter.Chen@freescale.com>
>> +M:	Roger Quadros <rogerq@ti.com>
>>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
>>  L:	linux-usb@vger.kernel.org
>>  S:	Maintained
>>  F:	drivers/usb/common/usb-otg-fsm.c
>> +F:	drivers/usb/common/usb-otg.c
>>  
>>  USB OVER IP DRIVER
>>  M:	Valentina Manea <valentina.manea.m@gmail.com>
>> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
>> index 8ed451d..5b625e2 100644
>> --- a/drivers/usb/Kconfig
>> +++ b/drivers/usb/Kconfig
>> @@ -32,7 +32,7 @@ if USB_SUPPORT
>>  config USB_COMMON
>>  	tristate
>>  	default y
>> -	depends on USB || USB_GADGET
>> +	depends on USB || USB_GADGET || USB_OTG
>>  
> 
> USB_OTG can depends on USB || UB_GADGET?

I didn't understand. The above is for USB_COMMON.

> 
>>  config USB_ARCH_HAS_HCD
>>  	def_bool y
>> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
>> index d8926c6..769d13b 100644
>> --- a/drivers/usb/Makefile
>> +++ b/drivers/usb/Makefile
>> @@ -60,5 +60,6 @@ obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
>>  obj-$(CONFIG_USB_GADGET)	+= gadget/
>>  
>>  obj-$(CONFIG_USB_COMMON)	+= common/
>> +obj-$(CONFIG_USB_OTG)		+= common/
>>  
> 
> The comment like above.

What should it look like?

> 
>>  obj-$(CONFIG_USBIP_CORE)	+= usbip/
>> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
>> index 6bbb3ec..730d928 100644
>> --- a/drivers/usb/common/Makefile
>> +++ b/drivers/usb/common/Makefile
>> @@ -6,5 +6,6 @@ obj-$(CONFIG_USB_COMMON)	  += usb-common.o
>>  usb-common-y			  += common.o
>>  usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>>  
>> -obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
>>  obj-$(CONFIG_USB_ULPI_BUS)	+= ulpi.o
>> +usbotg-y		:= usb-otg.o usb-otg-fsm.o
>> +obj-$(CONFIG_USB_OTG)	+= usbotg.o
>> diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
>> new file mode 100644
>> index 0000000..0379034
>> --- /dev/null
>> +++ b/drivers/usb/common/usb-otg.c
>> @@ -0,0 +1,768 @@
>> +/**
>> + * drivers/usb/common/usb-otg.c - USB OTG core
>> + *
>> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
>> + * Author: Roger Quadros <rogerq@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/ktime.h>
>> +#include <linux/hrtimer.h>
>> +#include <linux/list.h>
>> +#include <linux/usb/otg.h>
>> +#include <linux/usb/phy.h>	/* enum usb_otg_state */
>> +#include <linux/usb/gadget.h>
>> +#include <linux/workqueue.h>
>> +
>> +#include "usb-otg.h"
>> +
>> +/* to link timer with callback data */
>> +struct otg_timer {
>> +	struct hrtimer timer;
>> +	ktime_t timeout;
>> +	/* callback data */
>> +	int *timeout_bit;
>> +	struct otg_data *otgd;
>> +};
>> +
>> +struct otg_hcd {
>> +	struct usb_hcd *hcd;
>> +	unsigned int irqnum;
>> +	unsigned long irqflags;
>> +	struct otg_hcd_ops *ops;
>> +};
>> +
>> +struct otg_data {
>> +	struct device *dev;	/* HCD & GCD's parent device */
>> +
>> +	struct otg_fsm fsm;
>> +	/* HCD, GCD and usb_otg_state are present in otg_fsm->otg
>> +	 * HCD is bus_to_hcd(fsm->otg->host)
>> +	 * GCD is fsm->otg->gadget
>> +	 */
>> +	struct otg_fsm_ops fsm_ops;	/* private copy for override */
>> +	struct usb_otg otg;		/* allocator for fsm->otg */
>> +
>> +	struct otg_hcd primary_hcd;
>> +	struct otg_hcd shared_hcd;
>> +
>> +	struct otg_gadget_ops *gadget_ops; /* interface to gadget f/w */
>> +
>> +	/* saved hooks to OTG device */
>> +	int (*start_host)(struct otg_fsm *fsm, int on);
>> +	int (*start_gadget)(struct otg_fsm *fsm, int on);
>> +
>> +	struct list_head list;
>> +
>> +	struct work_struct work;	/* OTG FSM work */
>> +	struct workqueue_struct *wq;
>> +
>> +	struct otg_timer timers[NUM_OTG_FSM_TIMERS];
>> +
>> +	bool fsm_running;
>> +	/* use otg->fsm.lock for serializing access */
>> +};
>> +
> 
> Would you consider enhance struct usb_otg, and instead of this
> structure, I am wonder similar structures may increase the confuse
> in future.

Yes, that should be possible.

cheers,
-roger
--
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
Li Jun July 17, 2015, 7:48 a.m. UTC | #3
Hi, Roger

On Wed, Jul 08, 2015 at 01:19:33PM +0300, Roger Quadros wrote:
> The OTG core instantiates the OTG Finite State Machine
> per OTG controller and manages starting/stopping the
> host and gadget controllers based on the bus state.
> 
> It provides APIs for the following tasks
> 
> - Registering an OTG capable controller
> - Registering Host and Gadget controllers to OTG core
> - Providing inputs to and kicking the OTG state machine
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  MAINTAINERS                  |   4 +-
>  drivers/usb/Kconfig          |   2 +-
>  drivers/usb/Makefile         |   1 +
>  drivers/usb/common/Makefile  |   3 +-
>  drivers/usb/common/usb-otg.c | 768 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/usb/common/usb-otg.h |  71 ++++
>  drivers/usb/core/Kconfig     |  11 +-
>  include/linux/usb/otg.h      |  91 ++++-
>  8 files changed, 930 insertions(+), 21 deletions(-)
>  create mode 100644 drivers/usb/common/usb-otg.c
>  create mode 100644 drivers/usb/common/usb-otg.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8133cef..b21278e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10640,12 +10640,14 @@ S:	Maintained
>  F:	Documentation/usb/ohci.txt
>  F:	drivers/usb/host/ohci*
>  
> -USB OTG FSM (Finite State Machine)
> +USB OTG/DRD core and FSM (Finite State Machine)
>  M:	Peter Chen <Peter.Chen@freescale.com>
> +M:	Roger Quadros <rogerq@ti.com>
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
>  L:	linux-usb@vger.kernel.org
>  S:	Maintained
>  F:	drivers/usb/common/usb-otg-fsm.c
> +F:	drivers/usb/common/usb-otg.c
>  
>  USB OVER IP DRIVER
>  M:	Valentina Manea <valentina.manea.m@gmail.com>
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index 8ed451d..5b625e2 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -32,7 +32,7 @@ if USB_SUPPORT
>  config USB_COMMON
>  	tristate
>  	default y
> -	depends on USB || USB_GADGET
> +	depends on USB || USB_GADGET || USB_OTG
>  
>  config USB_ARCH_HAS_HCD
>  	def_bool y
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index d8926c6..769d13b 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -60,5 +60,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/Makefile b/drivers/usb/common/Makefile
> index 6bbb3ec..730d928 100644
> --- a/drivers/usb/common/Makefile
> +++ b/drivers/usb/common/Makefile
> @@ -6,5 +6,6 @@ obj-$(CONFIG_USB_COMMON)	  += usb-common.o
>  usb-common-y			  += common.o
>  usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>  
> -obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
>  obj-$(CONFIG_USB_ULPI_BUS)	+= ulpi.o
> +usbotg-y		:= usb-otg.o usb-otg-fsm.o
> +obj-$(CONFIG_USB_OTG)	+= usbotg.o
> diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
> new file mode 100644
> index 0000000..0379034
> --- /dev/null
> +++ b/drivers/usb/common/usb-otg.c
> @@ -0,0 +1,768 @@
> +/**
> + * drivers/usb/common/usb-otg.c - USB OTG core
> + *
> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
> + * Author: Roger Quadros <rogerq@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/ktime.h>
> +#include <linux/hrtimer.h>
> +#include <linux/list.h>
> +#include <linux/usb/otg.h>
> +#include <linux/usb/phy.h>	/* enum usb_otg_state */
> +#include <linux/usb/gadget.h>
> +#include <linux/workqueue.h>
> +
> +#include "usb-otg.h"
> +
> +/* to link timer with callback data */
> +struct otg_timer {
> +	struct hrtimer timer;
> +	ktime_t timeout;
> +	/* callback data */
> +	int *timeout_bit;
> +	struct otg_data *otgd;
> +};
> +
> +struct otg_hcd {
> +	struct usb_hcd *hcd;
> +	unsigned int irqnum;
> +	unsigned long irqflags;
> +	struct otg_hcd_ops *ops;
> +};
> +
> +struct otg_data {
> +	struct device *dev;	/* HCD & GCD's parent device */
> +
> +	struct otg_fsm fsm;
> +	/* HCD, GCD and usb_otg_state are present in otg_fsm->otg
> +	 * HCD is bus_to_hcd(fsm->otg->host)
> +	 * GCD is fsm->otg->gadget
> +	 */
> +	struct otg_fsm_ops fsm_ops;	/* private copy for override */
> +	struct usb_otg otg;		/* allocator for fsm->otg */
> +
> +	struct otg_hcd primary_hcd;
> +	struct otg_hcd shared_hcd;
> +
> +	struct otg_gadget_ops *gadget_ops; /* interface to gadget f/w */
> +
> +	/* saved hooks to OTG device */
> +	int (*start_host)(struct otg_fsm *fsm, int on);
> +	int (*start_gadget)(struct otg_fsm *fsm, int on);
> +
> +	struct list_head list;
> +
> +	struct work_struct work;	/* OTG FSM work */
> +	struct workqueue_struct *wq;
> +
> +	struct otg_timer timers[NUM_OTG_FSM_TIMERS];
> +
> +	bool fsm_running;
> +	/* use otg->fsm.lock for serializing access */
> +};
> +
> +/* OTG device list */
> +LIST_HEAD(otg_list);
> +static DEFINE_MUTEX(otg_list_mutex);
> +
> +static int usb_otg_hcd_is_primary_hcd(struct usb_hcd *hcd)
> +{
> +	if (!hcd->primary_hcd)
> +		return 1;
> +	return hcd == hcd->primary_hcd;
> +}
> +
> +/**
> + * check if device is in our OTG list and return
> + * otg_data, else NULL.
> + *
> + * otg_list_mutex must be held.
> + */
> +static struct otg_data *usb_otg_device_get_otgd(struct device *parent_dev)
> +{
> +	struct otg_data *otgd;
> +
> +	list_for_each_entry(otgd, &otg_list, list) {
> +		if (otgd->dev == parent_dev)
> +			return otgd;
> +	}
> +
> +	return NULL;
> +}
> +
> +/**
> + * timer callback to set timeout bit and kick FSM
> + */
> +static enum hrtimer_restart set_tmout(struct hrtimer *data)
> +{
> +	struct otg_timer *otgtimer;
> +
> +	otgtimer = container_of(data, struct otg_timer, timer);
> +	if (otgtimer->timeout_bit)
> +		*otgtimer->timeout_bit = 1;
> +
> +	usb_otg_sync_inputs(&otgtimer->otgd->fsm);
> +
> +	return HRTIMER_NORESTART;
> +}
> +
> +/**
> + * Initialize one OTG timer with callback, timeout and timeout bit
> + */
> +static void otg_timer_init(enum otg_fsm_timer id, struct otg_data *otgd,
> +			   enum hrtimer_restart (*callback)(struct hrtimer *),
> +			   unsigned long expires_ms,
> +			   int *timeout_bit)
> +{
> +	struct otg_timer *otgtimer = &otgd->timers[id];
> +	struct hrtimer *timer = &otgtimer->timer;
> +
> +	otgtimer->timeout = ms_to_ktime(expires_ms);
> +	hrtimer_init(timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +	timer->function = callback;
> +
> +	otgtimer->timeout_bit = timeout_bit;
> +	otgtimer->otgd = otgd;
> +}
> +
> +/**
> + * Initialize standard OTG timers
> + */
> +static void usb_otg_init_timers(struct otg_data *otgd)
> +{
> +	struct otg_fsm *fsm = &otgd->fsm;
> +
> +	otg_timer_init(A_WAIT_VRISE, otgd, set_tmout, TA_WAIT_VRISE,
> +		       &fsm->a_wait_vrise_tmout);
> +	otg_timer_init(A_WAIT_VFALL, otgd, set_tmout, TA_WAIT_VFALL,
> +		       &fsm->a_wait_vfall_tmout);
> +	otg_timer_init(A_WAIT_BCON, otgd, set_tmout, TA_WAIT_BCON,
> +		       &fsm->a_wait_bcon_tmout);
> +	otg_timer_init(A_AIDL_BDIS, otgd, set_tmout, TA_AIDL_BDIS,
> +		       &fsm->a_aidl_bdis_tmout);
> +	otg_timer_init(A_BIDL_ADIS, otgd, set_tmout, TA_BIDL_ADIS,
> +		       &fsm->a_bidl_adis_tmout);
> +	otg_timer_init(B_ASE0_BRST, otgd, set_tmout, TB_ASE0_BRST,
> +		       &fsm->b_ase0_brst_tmout);
> +
> +	otg_timer_init(B_SE0_SRP, otgd, set_tmout, TB_SE0_SRP, &fsm->b_se0_srp);
> +	otg_timer_init(B_SRP_FAIL, otgd, set_tmout, TB_SRP_FAIL,
> +		       &fsm->b_srp_done);
> +
> +	otg_timer_init(A_WAIT_ENUM, otgd, set_tmout, TB_SRP_FAIL, NULL);

2 timers are missing: B_DATA_PLS, B_SSEND_SRP.

> +}
> +
> +/**
> + * OTG FSM ops function to add timer
> + */
> +static void usb_otg_add_timer(struct otg_fsm *fsm, enum otg_fsm_timer id)
> +{
> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> +	struct otg_timer *otgtimer = &otgd->timers[id];
> +	struct hrtimer *timer = &otgtimer->timer;
> +
> +	if (!otgd->fsm_running)
> +		return;
> +
> +	/* if timer is already active, exit */
> +	if (hrtimer_active(timer)) {
> +		dev_err(otgd->dev, "otg: timer %d is already running\n", id);
> +		return;
> +	}
> +
> +	hrtimer_start(timer, otgtimer->timeout, HRTIMER_MODE_REL);
> +}
> +
> +/**
> + * OTG FSM ops function to delete timer
> + */
> +static void usb_otg_del_timer(struct otg_fsm *fsm, enum otg_fsm_timer id)
> +{
> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> +	struct hrtimer *timer = &otgd->timers[id].timer;
> +
> +	hrtimer_cancel(timer);
> +}
> +
> +/**
> + * OTG FSM ops function to start/stop host
> + */
> +static int usb_otg_start_host(struct otg_fsm *fsm, int on)
> +{
> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> +	struct otg_hcd_ops *hcd_ops;
> +
> +	dev_dbg(otgd->dev, "otg: %s %d\n", __func__, on);
> +	if (!fsm->otg->host) {
> +		WARN_ONCE(1, "otg: fsm running without host\n");
> +		return 0;
> +	}
> +
> +	if (on) {
> +		/* OTG device operations */
> +		if (otgd->start_host)
> +			otgd->start_host(fsm, on);
> +
> +		/* start host */
> +		hcd_ops = otgd->primary_hcd.ops;
> +		hcd_ops->add(otgd->primary_hcd.hcd, otgd->primary_hcd.irqnum,
> +			     otgd->primary_hcd.irqflags);
> +		if (otgd->shared_hcd.hcd) {
> +			hcd_ops = otgd->shared_hcd.ops;
> +			hcd_ops->add(otgd->shared_hcd.hcd,
> +				     otgd->shared_hcd.irqnum,
> +				     otgd->shared_hcd.irqflags);
> +		}
> +	} else {
> +		/* stop host */
> +		if (otgd->shared_hcd.hcd) {
> +			hcd_ops = otgd->shared_hcd.ops;
> +			hcd_ops->remove(otgd->shared_hcd.hcd);
> +		}
> +		hcd_ops = otgd->primary_hcd.ops;
> +		hcd_ops->remove(otgd->primary_hcd.hcd);
> +
> +		/* OTG device operations */
> +		if (otgd->start_host)
> +			otgd->start_host(fsm, on);
> +	}
> +
> +	return 0;
> +}

I do not see much benefit by this override function, usb_add/remove_hcd
can be simply included by controller's start_host function, also there
maybe some additional operations after usb_add_hcd, but this override
function limit usb_add_hcd() is the last step.

Maybe your intention is to make usb_add_hcd is the only operation required
to start host, so ideally controller driver need not define its start_host
routine for this otg ops, I am not sure if this can work for different otg
platforms. If the shared code is only usb_add/remove_hcd(), maybe leave this
ops defined by controller driver can make core code simple and give flexibility
to controller drivers.

> +
> +/**
> + * OTG FSM ops function to start/stop gadget
> + */
> +static int usb_otg_start_gadget(struct otg_fsm *fsm, int on)
> +{
> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> +	struct usb_gadget *gadget = fsm->otg->gadget;
> +
> +	dev_dbg(otgd->dev, "otg: %s %d\n", __func__, on);
> +	if (!gadget) {
> +		WARN_ONCE(1, "otg: fsm running without gadget\n");
> +		return 0;
> +	}
> +
> +	if (on) {
> +		/* OTG device operations */
> +		if (otgd->start_gadget)
> +			otgd->start_gadget(fsm, on);
> +
> +		otgd->gadget_ops->start(fsm->otg->gadget);
> +	} else {
> +		otgd->gadget_ops->stop(fsm->otg->gadget);
> +
> +		/* OTG device operations */
> +		if (otgd->start_gadget)
> +			otgd->start_gadget(fsm, on);
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * OTG FSM work function
> + */
> +static void usb_otg_work(struct work_struct *work)
> +{
> +	struct otg_data *otgd = container_of(work, struct otg_data, work);
> +
> +	otg_statemachine(&otgd->fsm);

Need consider runtime pm, or you want to rely on controller driver take
care of it?

> +}
> +
> +/**
> + * usb_otg_register() - Register the OTG device to OTG core
> + * @parent_device:	parent device of Host & Gadget controllers.
> + * @otg_fsm_ops:	otg state machine ops.
> + *
> + * Register parent device that contains both HCD and GCD into
> + * the USB OTG core. HCD and GCD will be prevented from starting
> + * till both are available for use.
> + *
> + * Return: struct otg_fsm * if success, NULL if error.
> + */
> +struct otg_fsm *usb_otg_register(struct device *parent_dev,
> +				 struct otg_fsm_ops *fsm_ops)
> +{
> +	struct otg_data *otgd;
> +	int ret = 0;
> +
> +	if (!parent_dev || !fsm_ops)
> +		return ERR_PTR(-EINVAL);
> +
> +	/* already in list? */
> +	mutex_lock(&otg_list_mutex);
> +	if (usb_otg_device_get_otgd(parent_dev)) {
> +		dev_err(parent_dev, "otg: %s: device already in otg list\n",
> +			__func__);
> +		ret = -EINVAL;
> +		goto unlock;
> +	}
> +
> +	/* allocate and add to list */
> +	otgd = kzalloc(sizeof(*otgd), GFP_KERNEL);
> +	if (!otgd) {
> +		ret = -ENOMEM;
> +		goto unlock;
> +	}
> +
> +	otgd->dev = parent_dev;
> +	INIT_WORK(&otgd->work, usb_otg_work);
> +	otgd->wq = create_singlethread_workqueue("usb_otg");
> +	if (!otgd->wq) {
> +		dev_err(parent_dev, "otg: %s: can't create workqueue\n",
> +			__func__);
> +		ret = -ENODEV;
> +		goto err_wq;
> +	}
> +
> +	usb_otg_init_timers(otgd);
> +
> +	/* save original start host/gadget ops */
> +	otgd->start_host = fsm_ops->start_host;
> +	otgd->start_gadget = fsm_ops->start_gadget;
> +	/* create copy of original ops */
> +	otgd->fsm_ops = *fsm_ops;
> +	/* override ops */
> +	otgd->fsm_ops.start_host = usb_otg_start_host;
> +	otgd->fsm_ops.start_gadget = usb_otg_start_gadget;
> +	/* FIXME: we ignore caller's timer ops */
> +	otgd->fsm_ops.add_timer = usb_otg_add_timer;
> +	otgd->fsm_ops.del_timer = usb_otg_del_timer;
> +	/* set otg ops */
> +	otgd->fsm.ops = &otgd->fsm_ops;
> +	otgd->fsm.otg = &otgd->otg;
> +
> +	mutex_init(&otgd->fsm.lock);
> +
> +	list_add_tail(&otgd->list, &otg_list);
> +	mutex_unlock(&otg_list_mutex);
> +	return &otgd->fsm;
> +
> +err_wq:
> +	kfree(otgd);
> +unlock:
> +	mutex_unlock(&otg_list_mutex);
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_register);
> +
> +/**
> + * usb_otg_unregister() - Unregister the OTG device from USB OTG core
> + * @parent_device:	parent device of Host & Gadget controllers.
> + *
> + * Unregister parent OTG deviced from USB OTG core.
> + * Prevents unregistering till both Host and Gadget controllers
> + * have unregistered from the OTG core.
> + *
> + * Return: 0 on success, error value otherwise.
> + */
> +int usb_otg_unregister(struct device *parent_dev)
> +{
> +	struct otg_data *otgd;
> +
> +	mutex_lock(&otg_list_mutex);
> +	otgd = usb_otg_device_get_otgd(parent_dev);
> +	if (!otgd) {
> +		dev_err(parent_dev, "otg: %s: device not in otg list\n",
> +			__func__);
> +		mutex_unlock(&otg_list_mutex);
> +		return -EINVAL;
> +	}
> +
> +	/* prevent unregister till both host & gadget have unregistered */
> +	if (otgd->fsm.otg->host || otgd->fsm.otg->gadget) {
> +		dev_err(parent_dev, "otg: %s: host/gadget still registered\n",
> +			__func__);
> +		return -EBUSY;
> +	}
> +
> +	/* OTG FSM is halted when host/gadget unregistered */
> +	destroy_workqueue(otgd->wq);
> +
> +	/* remove from otg list */
> +	list_del(&otgd->list);
> +	kfree(otgd);
> +	mutex_unlock(&otg_list_mutex);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_unregister);
> +
> +/**
> + * start/kick the OTG FSM if we can
> + * fsm->lock must be held
> + */
> +static void usb_otg_start_fsm(struct otg_fsm *fsm)
> +{
> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> +
> +	if (otgd->fsm_running)
> +		goto kick_fsm;
> +
> +	if (!fsm->otg->host) {
> +		dev_info(otgd->dev, "otg: can't start till host registers\n");
> +		return;
> +	}
> +

This cannot work, fsm->otg->host is set in usb_otg_register_hcd(), which is
called by usb_add_hcd(), but usb_add_hcd() should be called only if otg fsm
already started to some A-device state, deadlock.

> +	if (!fsm->otg->gadget) {
> +		dev_info(otgd->dev, "otg: can't start till gadget registers\n");
> +		return;
> +	}
> +
> +	otgd->fsm_running = true;
> +kick_fsm:
> +	queue_work(otgd->wq, &otgd->work);
> +}
> +
> +/**
> + * stop the OTG FSM. Stops Host & Gadget controllers as well.
> + * fsm->lock must be held
> + */
> +static void usb_otg_stop_fsm(struct otg_fsm *fsm)
> +{
> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> +	int i;
> +
> +	if (!otgd->fsm_running)
> +		return;
> +
> +	/* no more new events queued */
> +	otgd->fsm_running = false;
> +
> +	/* Stop state machine / timers */
> +	for (i = 0; i < ARRAY_SIZE(otgd->timers); i++)
> +		hrtimer_cancel(&otgd->timers[i].timer);
> +
> +	flush_workqueue(otgd->wq);
> +	fsm->otg->state = OTG_STATE_UNDEFINED;
> +
> +	/* stop host/gadget immediately */
> +	if (fsm->protocol == PROTO_HOST)
> +		otg_start_host(fsm, 0);
> +	else if (fsm->protocol == PROTO_GADGET)
> +		otg_start_gadget(fsm, 0);
> +	fsm->protocol = PROTO_UNDEF;
> +}
> +
> +/**
> + * usb_otg_sync_inputs - Sync OTG inputs with the OTG state machine
> + * @fsm:	OTG FSM instance
> + *
> + * Used by the OTG driver to update the inputs to the OTG
> + * state machine.
> + *
> + * Can be called in IRQ context.
> + */
> +void usb_otg_sync_inputs(struct otg_fsm *fsm)
> +{
> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> +
> +	/* Don't kick FSM till it has started */
> +	if (!otgd->fsm_running)
> +		return;
> +
> +	/* Kick FSM */
> +	queue_work(otgd->wq, &otgd->work);
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_sync_inputs);
> +
> +/**
> + * usb_otg_kick_fsm - Kick the OTG state machine
> + * @hcd_gcd_device:	Host/Gadget controller device
> + *
> + * Used by USB host/device stack to sync OTG related
> + * events to the OTG state machine.
> + * e.g. change in host_bus->b_hnp_enable, gadget->b_hnp_enable
> + *
> + * Returns: 0 on success, error value otherwise.
> + */
> +int usb_otg_kick_fsm(struct device *hcd_gcd_device)
> +{
> +	struct otg_data *otgd;
> +
> +	mutex_lock(&otg_list_mutex);
> +	otgd = usb_otg_device_get_otgd(hcd_gcd_device->parent);
> +	if (!otgd) {
> +		dev_err(hcd_gcd_device, "otg: %s: invalid host/gadget device\n",
> +			__func__);
> +		mutex_unlock(&otg_list_mutex);
> +		return -ENODEV;
> +	}
> +
> +	mutex_unlock(&otg_list_mutex);
> +	usb_otg_sync_inputs(&otgd->fsm);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_kick_fsm);
> +
> +/**
> + * usb_otg_register_hcd - Register Host controller to OTG core
> + * @hcd:	Host controller device
> + * @irqnum:	interrupt number
> + * @irqflags:	interrupt flags
> + * @ops:	HCD ops to add/remove the HCD
> + *
> + * This is used by the USB Host stack to register the Host controller
> + * to the OTG core. Host controller must not be started by the
> + * caller as it is left upto the OTG state machine to do so.
> + *
> + * Returns: 0 on success, error value otherwise.
> + */
> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
> +			 unsigned long irqflags, struct otg_hcd_ops *ops)
> +{
> +	struct otg_data *otgd;
> +	struct device *otg_dev = hcd->self.controller->parent;
> +
> +	mutex_lock(&otg_list_mutex);
> +	otgd = usb_otg_device_get_otgd(otg_dev);
> +	if (!otgd) {
> +		dev_dbg(otg_dev, "otg: %s: device not registered to otg core\n",
> +			__func__);
> +		mutex_unlock(&otg_list_mutex);
> +		return -EINVAL;
> +	}
> +
> +	mutex_unlock(&otg_list_mutex);
> +	/* HCD will be started by OTG fsm when needed */
> +	mutex_lock(&otgd->fsm.lock);
> +	if (otgd->primary_hcd.hcd) {
> +		/* probably a shared HCD ? */
> +		if (usb_otg_hcd_is_primary_hcd(hcd)) {
> +			dev_err(otg_dev, "otg: primary host already registered\n");
> +			goto err;
> +		}
> +
> +		if (hcd->shared_hcd == otgd->primary_hcd.hcd) {
> +			if (otgd->shared_hcd.hcd) {
> +				dev_err(otg_dev, "otg: shared host already registered\n");
> +				goto err;
> +			}
> +
> +			otgd->shared_hcd.hcd = hcd;
> +			otgd->shared_hcd.irqnum = irqnum;
> +			otgd->shared_hcd.irqflags = irqflags;
> +			otgd->shared_hcd.ops = ops;
> +			dev_info(otg_dev, "otg: shared host %s registered\n",
> +				 dev_name(hcd->self.controller));
> +		} else {
> +			dev_err(otg_dev, "otg: invalid shared host %s\n",
> +				dev_name(hcd->self.controller));
> +			goto err;
> +		}
> +	} else {
> +		if (!usb_otg_hcd_is_primary_hcd(hcd)) {
> +			dev_err(otg_dev, "otg: primary host must be registered first\n");
> +			goto err;
> +		}
> +
> +		otgd->primary_hcd.hcd = hcd;
> +		otgd->primary_hcd.irqnum = irqnum;
> +		otgd->primary_hcd.irqflags = irqflags;
> +		otgd->primary_hcd.ops = ops;
> +		dev_info(otg_dev, "otg: primary host %s registered\n",
> +			 dev_name(hcd->self.controller));
> +	}
> +
> +	/*
> +	 * we're ready only if we have shared HCD
> +	 * or we don't need shared HCD.
> +	 */
> +	if (otgd->shared_hcd.hcd || !otgd->primary_hcd.hcd->shared_hcd) {
> +		otgd->fsm.otg->host = hcd_to_bus(hcd);
> +		/* FIXME: set bus->otg_port if this is true OTG port with HNP */
> +
> +		/* start FSM */
> +		usb_otg_start_fsm(&otgd->fsm);

usb_otg_register_hcd() is called before usb_otg_add_hcd(), start fsm on
this point can make sense since hcd has not been added?

> +	} else {
> +		dev_dbg(otg_dev, "otg: can't start till shared host registers\n");
> +	}
> +
> +	mutex_unlock(&otgd->fsm.lock);
> +
> +	return 0;
> +
> +err:
> +	mutex_unlock(&otgd->fsm.lock);
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_register_hcd);
> +
> +/**
> + * usb_otg_unregister_hcd - Unregister Host controller from OTG core
> + * @hcd:	Host controller device
> + *
> + * This is used by the USB Host stack to unregister the Host controller
> + * from the OTG core. Ensures that Host controller is not running
> + * on successful return.
> + *
> + * Returns: 0 on success, error value otherwise.
> + */
> +int usb_otg_unregister_hcd(struct usb_hcd *hcd)
> +{
> +	struct otg_data *otgd;
> +	struct usb_bus *bus = hcd_to_bus(hcd);
> +	struct device *otg_dev = bus->controller->parent;
> +
> +	mutex_lock(&otg_list_mutex);
> +	otgd = usb_otg_device_get_otgd(otg_dev);
> +	if (!otgd) {
> +		dev_err(otg_dev, "otg: %s: device not registered to otg core\n",
> +			__func__);
> +		mutex_unlock(&otg_list_mutex);
> +		return -EINVAL;
> +	}
> +
> +	mutex_unlock(&otg_list_mutex);
> +
> +	mutex_lock(&otgd->fsm.lock);
> +	if (hcd == otgd->primary_hcd.hcd) {
> +		otgd->primary_hcd.hcd = NULL;
> +		dev_info(otg_dev, "otg: primary host %s unregistered\n",
> +			 dev_name(bus->controller));
> +	} else if (hcd == otgd->shared_hcd.hcd) {
> +		otgd->shared_hcd.hcd = NULL;
> +		dev_info(otg_dev, "otg: shared host %s unregistered\n",
> +			 dev_name(bus->controller));
> +	} else {
> +		dev_err(otg_dev, "otg: host %s wasn't registered with otg\n",
> +			dev_name(bus->controller));
> +		mutex_unlock(&otgd->fsm.lock);
> +		return -EINVAL;
> +	}
> +
> +	/* stop FSM & Host */
> +	usb_otg_stop_fsm(&otgd->fsm);
> +	otgd->fsm.otg->host = NULL;
> +
> +	mutex_unlock(&otgd->fsm.lock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_unregister_hcd);
> +
> +/**
> + * usb_otg_register_gadget - Register Gadget controller to OTG core
> + * @gadget:	Gadget controller
> + *
> + * This is used by the USB Gadget stack to register the Gadget controller
> + * to the OTG core. Gadget controller must not be started by the
> + * caller as it is left upto the OTG state machine to do so.
> + *
> + * Gadget core must call this only when all resources required for
> + * gadget controller to run are available.
> + * i.e. gadget function driver is available.
> + *
> + * Returns: 0 on success, error value otherwise.
> + */
> +int usb_otg_register_gadget(struct usb_gadget *gadget,
> +			    struct otg_gadget_ops *ops)
> +{
> +	struct otg_data *otgd;
> +	struct device *otg_dev = gadget->dev.parent;
> +
> +	mutex_lock(&otg_list_mutex);
> +	otgd = usb_otg_device_get_otgd(otg_dev);
> +	if (!otgd) {
> +		dev_err(otg_dev, "otg: %s: device not registered to otg core\n",
> +			__func__);
> +		mutex_unlock(&otg_list_mutex);
> +		return -EINVAL;
> +	}
> +
> +	mutex_unlock(&otg_list_mutex);
> +
> +	mutex_lock(&otgd->fsm.lock);
> +	if (otgd->fsm.otg->gadget) {
> +		dev_err(otg_dev, "otg: gadget already registered with otg\n");
> +		mutex_unlock(&otgd->fsm.lock);
> +		return -EINVAL;
> +	}
> +
> +	otgd->fsm.otg->gadget = gadget;
> +	otgd->gadget_ops = ops;
> +	dev_info(otg_dev, "otg: gadget %s registered\n",
> +		 dev_name(&gadget->dev));
> +
> +	/* start FSM */
> +	usb_otg_start_fsm(&otgd->fsm);
> +	mutex_unlock(&otgd->fsm.lock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_register_gadget);
> +
> +/**
> + * usb_otg_unregister_gadget - Unregister Gadget controller from OTG core
> + * @gadget:	Gadget controller
> + *
> + * This is used by the USB Gadget stack to unregister the Gadget controller
> + * from the OTG core. Ensures that Gadget controller is not running
> + * on successful return.
> + *
> + * Returns: 0 on success, error value otherwise.
> + */
> +int usb_otg_unregister_gadget(struct usb_gadget *gadget)
> +{
> +	struct otg_data *otgd;
> +	struct device *otg_dev = gadget->dev.parent;
> +
> +	mutex_lock(&otg_list_mutex);
> +	otgd = usb_otg_device_get_otgd(otg_dev);
> +	if (!otgd) {
> +		dev_err(otg_dev, "otg: %s: device not registered to otg core\n",
> +			__func__);
> +		mutex_unlock(&otg_list_mutex);
> +		return -EINVAL;
> +	}
> +
> +	mutex_unlock(&otg_list_mutex);
> +
> +	mutex_lock(&otgd->fsm.lock);
> +	if (otgd->fsm.otg->gadget != gadget) {
> +		dev_err(otg_dev, "otg: gadget %s wasn't registered with otg\n",
> +			dev_name(&gadget->dev));
> +		mutex_unlock(&otgd->fsm.lock);
> +		return -EINVAL;
> +	}
> +
> +	/* Stop FSM & gadget */
> +	usb_otg_stop_fsm(&otgd->fsm);
> +	otgd->fsm.otg->gadget = NULL;
> +	mutex_unlock(&otgd->fsm.lock);
> +
> +	dev_info(otg_dev, "otg: gadget %s unregistered\n",
> +		 dev_name(&gadget->dev));
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_unregister_gadget);
> +
> +/**
> + * usb_otg_fsm_to_dev - Get OTG controller device from struct otg_fsm
> + * @fsm:	otg_fsm data structure
> + *
> + * This is used by the OTG controller driver to get it's device node
> + * from any of the otg_fsm->ops.
> + */
> +struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm)
> +{
> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> +
> +	return otgd->dev;
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_fsm_to_dev);
> diff --git a/drivers/usb/common/usb-otg.h b/drivers/usb/common/usb-otg.h
> new file mode 100644
> index 0000000..05331f0
> --- /dev/null
> +++ b/drivers/usb/common/usb-otg.h
> @@ -0,0 +1,71 @@
> +/**
> + * drivers/usb/common/usb-otg.h - USB OTG core local header
> + *
> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
> + * Author: Roger Quadros <rogerq@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __DRIVERS_USB_COMMON_USB_OTG_H
> +#define __DRIVERS_USB_COMMON_USB_OTG_H
> +
> +/*
> + *  A-DEVICE timing constants
> + */
> +
> +/* Wait for VBUS Rise  */
> +#define TA_WAIT_VRISE        (100)	/* a_wait_vrise: section 7.1.2
> +					 * a_wait_vrise_tmr: section 7.4.5.1
> +					 * TA_VBUS_RISE <= 100ms, section 4.4
> +					 * Table 4-1: Electrical Characteristics
> +					 * ->DC Electrical Timing
> +					 */
> +/* Wait for VBUS Fall  */
> +#define TA_WAIT_VFALL        (1000)	/* a_wait_vfall: section 7.1.7
> +					 * a_wait_vfall_tmr: section: 7.4.5.2
> +					 */
> +/* Wait for B-Connect */
> +#define TA_WAIT_BCON         (10000)	/* a_wait_bcon: section 7.1.3
> +					 * TA_WAIT_BCON: should be between 1100
> +					 * and 30000 ms, section 5.5, Table 5-1
> +					 */
> +/* A-Idle to B-Disconnect */
> +#define TA_AIDL_BDIS         (5000)	/* a_suspend min 200 ms, section 5.2.1
> +					 * TA_AIDL_BDIS: section 5.5, Table 5-1
> +					 */
> +/* B-Idle to A-Disconnect */
> +#define TA_BIDL_ADIS         (500)	/* TA_BIDL_ADIS: section 5.2.1
> +					 * 500ms is used for B switch to host
> +					 * for safe
> +					 */
> +
> +/*
> + * B-device timing constants
> + */
> +
> +/* Data-Line Pulse Time*/
> +#define TB_DATA_PLS          (10)	/* b_srp_init,continue 5~10ms
> +					 * section:5.1.3
> +					 */
> +/* SRP Fail Time  */
> +#define TB_SRP_FAIL          (6000)	/* b_srp_init,fail time 5~6s
> +					 * section:5.1.6
> +					 */
> +/* A-SE0 to B-Reset  */
> +#define TB_ASE0_BRST         (155)	/* minimum 155 ms, section:5.3.1 */
> +/* SE0 Time Before SRP */
> +#define TB_SE0_SRP           (1000)	/* b_idle,minimum 1s, section:5.1.2 */
> +/* SSEND time before SRP */
> +#define TB_SSEND_SRP         (1500)	/* minimum 1.5 sec, section:5.1.2 */
> +
> +#define TB_SESS_VLD          (1000)
> +

All otg timer timeout value should be in some *range* defined by otg spec,
not some specific value, I don't think one specific value can meet all otg
platforms, so we need find a way to make those value can be configured by
controller drivers.

> +#endif /* __DRIVERS_USB_COMMON_USB_OTG_H */
> diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
> index a99c89e..b468a9f 100644
> --- a/drivers/usb/core/Kconfig
> +++ b/drivers/usb/core/Kconfig
> @@ -42,7 +42,7 @@ config USB_DYNAMIC_MINORS
>  	  If you are unsure about this, say N here.
>  
>  config USB_OTG
> -	bool "OTG support"
> +	bool "OTG/Dual-role support"
>  	depends on PM
>  	default n
>  	help
> @@ -75,15 +75,6 @@ config USB_OTG_BLACKLIST_HUB
>  	  and software costs by not supporting external hubs.  So
>  	  are "Embedded Hosts" that don't offer OTG support.
>  
> -config USB_OTG_FSM
> -	tristate "USB 2.0 OTG FSM implementation"
> -	depends on USB
> -	select USB_OTG
> -	select USB_PHY
> -	help
> -	  Implements OTG Finite State Machine as specified in On-The-Go
> -	  and Embedded Host Supplement to the USB Revision 2.0 Specification.
> -
>  config USB_ULPI_BUS
>  	tristate "USB ULPI PHY interface support"
>  	depends on USB_SUPPORT
> diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
> index 52661c5..ce6f8d8 100644
> --- a/include/linux/usb/otg.h
> +++ b/include/linux/usb/otg.h
> @@ -10,6 +10,11 @@
>  #define __LINUX_USB_OTG_H
>  
>  #include <linux/phy/phy.h>
> +#include <linux/device.h>
> +#include <linux/usb.h>
> +#include <linux/usb/hcd.h>
> +#include <linux/usb/gadget.h>
> +#include <linux/usb/otg-fsm.h>
>  #include <linux/usb/phy.h>
>  
>  struct usb_otg {
> @@ -23,6 +28,7 @@ struct usb_otg {
>  
>  	enum usb_otg_state	state;
>  
> +/*------------- deprecated interface -----------------------------*/
>  	/* bind/unbind the host controller */
>  	int	(*set_host)(struct usb_otg *otg, struct usb_bus *host);
>  
> @@ -38,11 +44,85 @@ struct usb_otg {
>  
>  	/* start or continue HNP role switch */
>  	int	(*start_hnp)(struct usb_otg *otg);
> -
> +/*---------------------------------------------------------------*/
>  };
>  
>  extern const char *usb_otg_state_string(enum usb_otg_state state);
>  
> +enum usb_dr_mode {
> +	USB_DR_MODE_UNKNOWN,
> +	USB_DR_MODE_HOST,
> +	USB_DR_MODE_PERIPHERAL,
> +	USB_DR_MODE_OTG,
> +};
> +
> +#if IS_ENABLED(CONFIG_USB_OTG)
> +struct otg_fsm *usb_otg_register(struct device *parent_dev,
> +				 struct otg_fsm_ops *fsm_ops);
> +int usb_otg_unregister(struct device *parent_dev);
> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
> +			 unsigned long irqflags, struct otg_hcd_ops *ops);
> +int usb_otg_unregister_hcd(struct usb_hcd *hcd);
> +int usb_otg_register_gadget(struct usb_gadget *gadget,
> +			    struct otg_gadget_ops *ops);
> +int usb_otg_unregister_gadget(struct usb_gadget *gadget);
> +void usb_otg_sync_inputs(struct otg_fsm *fsm);
> +int usb_otg_kick_fsm(struct device *hcd_gcd_device);
> +struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm);
> +
> +#else /* CONFIG_USB_OTG */
> +
> +static inline struct otg_fsm *usb_otg_register(struct device *parent_dev,
> +					       struct otg_fsm_ops *fsm_ops)
> +{
> +	return ERR_PTR(-ENOSYS);
> +}
> +
> +static inline int usb_otg_unregister(struct device *parent_dev)
> +{
> +	return -ENOSYS;
> +}
> +
> +static inline int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
> +				       unsigned long irqflags,
> +				       struct otg_hcd_ops *ops)
> +{
> +	return -ENOSYS;
> +}
> +
> +static inline int usb_otg_unregister_hcd(struct usb_hcd *hcd)
> +{
> +	return -ENOSYS;
> +}
> +
> +static inline int usb_otg_register_gadget(struct usb_gadget *gadget,
> +					  struct otg_gadget_ops *ops)
> +{
> +	return -ENOSYS;
> +}
> +
> +static inline int usb_otg_unregister_gadget(struct usb_gadget *gadget)
> +{
> +	return -ENOSYS;
> +}
> +
> +static inline void usb_otg_sync_inputs(struct otg_fsm *fsm)
> +{
> +}
> +
> +static inline int usb_otg_kick_fsm(struct device *hcd_gcd_device)
> +{
> +	return -ENOSYS;
> +}
> +
> +static inline struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm)
> +{
> +	return NULL;
> +}
> +
> +#endif /* CONFIG_USB_OTG */
> +
> +/*------------- deprecated interface -----------------------------*/
>  /* Context: can sleep */
>  static inline int
>  otg_start_hnp(struct usb_otg *otg)
> @@ -94,14 +174,9 @@ otg_start_srp(struct usb_otg *otg)
>  	return -ENOTSUPP;
>  }
>  
> +/*---------------------------------------------------------------*/
> +
>  /* for OTG controller drivers (and maybe other stuff) */
>  extern int usb_bus_start_enum(struct usb_bus *bus, unsigned port_num);
>  
> -enum usb_dr_mode {
> -	USB_DR_MODE_UNKNOWN,
> -	USB_DR_MODE_HOST,
> -	USB_DR_MODE_PERIPHERAL,
> -	USB_DR_MODE_OTG,
> -};
> -
>  #endif /* __LINUX_USB_OTG_H */
> -- 
> 2.1.4
> 

I assume I will have more comments after try more.

Li Jun 
--
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
Roger Quadros July 17, 2015, 12:06 p.m. UTC | #4
Hi Li,

On 17/07/15 10:48, Li Jun wrote:
> Hi, Roger
> 
> On Wed, Jul 08, 2015 at 01:19:33PM +0300, Roger Quadros wrote:
>> The OTG core instantiates the OTG Finite State Machine
>> per OTG controller and manages starting/stopping the
>> host and gadget controllers based on the bus state.
>>
>> It provides APIs for the following tasks
>>
>> - Registering an OTG capable controller
>> - Registering Host and Gadget controllers to OTG core
>> - Providing inputs to and kicking the OTG state machine
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  MAINTAINERS                  |   4 +-
>>  drivers/usb/Kconfig          |   2 +-
>>  drivers/usb/Makefile         |   1 +
>>  drivers/usb/common/Makefile  |   3 +-
>>  drivers/usb/common/usb-otg.c | 768 +++++++++++++++++++++++++++++++++++++++++++
>>  drivers/usb/common/usb-otg.h |  71 ++++
>>  drivers/usb/core/Kconfig     |  11 +-
>>  include/linux/usb/otg.h      |  91 ++++-
>>  8 files changed, 930 insertions(+), 21 deletions(-)
>>  create mode 100644 drivers/usb/common/usb-otg.c
>>  create mode 100644 drivers/usb/common/usb-otg.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 8133cef..b21278e 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -10640,12 +10640,14 @@ S:	Maintained
>>  F:	Documentation/usb/ohci.txt
>>  F:	drivers/usb/host/ohci*
>>  
>> -USB OTG FSM (Finite State Machine)
>> +USB OTG/DRD core and FSM (Finite State Machine)
>>  M:	Peter Chen <Peter.Chen@freescale.com>
>> +M:	Roger Quadros <rogerq@ti.com>
>>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
>>  L:	linux-usb@vger.kernel.org
>>  S:	Maintained
>>  F:	drivers/usb/common/usb-otg-fsm.c
>> +F:	drivers/usb/common/usb-otg.c
>>  
>>  USB OVER IP DRIVER
>>  M:	Valentina Manea <valentina.manea.m@gmail.com>
>> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
>> index 8ed451d..5b625e2 100644
>> --- a/drivers/usb/Kconfig
>> +++ b/drivers/usb/Kconfig
>> @@ -32,7 +32,7 @@ if USB_SUPPORT
>>  config USB_COMMON
>>  	tristate
>>  	default y
>> -	depends on USB || USB_GADGET
>> +	depends on USB || USB_GADGET || USB_OTG
>>  
>>  config USB_ARCH_HAS_HCD
>>  	def_bool y
>> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
>> index d8926c6..769d13b 100644
>> --- a/drivers/usb/Makefile
>> +++ b/drivers/usb/Makefile
>> @@ -60,5 +60,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/Makefile b/drivers/usb/common/Makefile
>> index 6bbb3ec..730d928 100644
>> --- a/drivers/usb/common/Makefile
>> +++ b/drivers/usb/common/Makefile
>> @@ -6,5 +6,6 @@ obj-$(CONFIG_USB_COMMON)	  += usb-common.o
>>  usb-common-y			  += common.o
>>  usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>>  
>> -obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
>>  obj-$(CONFIG_USB_ULPI_BUS)	+= ulpi.o
>> +usbotg-y		:= usb-otg.o usb-otg-fsm.o
>> +obj-$(CONFIG_USB_OTG)	+= usbotg.o
>> diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
>> new file mode 100644
>> index 0000000..0379034
>> --- /dev/null
>> +++ b/drivers/usb/common/usb-otg.c
>> @@ -0,0 +1,768 @@
>> +/**
>> + * drivers/usb/common/usb-otg.c - USB OTG core
>> + *
>> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
>> + * Author: Roger Quadros <rogerq@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/ktime.h>
>> +#include <linux/hrtimer.h>
>> +#include <linux/list.h>
>> +#include <linux/usb/otg.h>
>> +#include <linux/usb/phy.h>	/* enum usb_otg_state */
>> +#include <linux/usb/gadget.h>
>> +#include <linux/workqueue.h>
>> +
>> +#include "usb-otg.h"
>> +
>> +/* to link timer with callback data */
>> +struct otg_timer {
>> +	struct hrtimer timer;
>> +	ktime_t timeout;
>> +	/* callback data */
>> +	int *timeout_bit;
>> +	struct otg_data *otgd;
>> +};
>> +
>> +struct otg_hcd {
>> +	struct usb_hcd *hcd;
>> +	unsigned int irqnum;
>> +	unsigned long irqflags;
>> +	struct otg_hcd_ops *ops;
>> +};
>> +
>> +struct otg_data {
>> +	struct device *dev;	/* HCD & GCD's parent device */
>> +
>> +	struct otg_fsm fsm;
>> +	/* HCD, GCD and usb_otg_state are present in otg_fsm->otg
>> +	 * HCD is bus_to_hcd(fsm->otg->host)
>> +	 * GCD is fsm->otg->gadget
>> +	 */
>> +	struct otg_fsm_ops fsm_ops;	/* private copy for override */
>> +	struct usb_otg otg;		/* allocator for fsm->otg */
>> +
>> +	struct otg_hcd primary_hcd;
>> +	struct otg_hcd shared_hcd;
>> +
>> +	struct otg_gadget_ops *gadget_ops; /* interface to gadget f/w */
>> +
>> +	/* saved hooks to OTG device */
>> +	int (*start_host)(struct otg_fsm *fsm, int on);
>> +	int (*start_gadget)(struct otg_fsm *fsm, int on);
>> +
>> +	struct list_head list;
>> +
>> +	struct work_struct work;	/* OTG FSM work */
>> +	struct workqueue_struct *wq;
>> +
>> +	struct otg_timer timers[NUM_OTG_FSM_TIMERS];
>> +
>> +	bool fsm_running;
>> +	/* use otg->fsm.lock for serializing access */
>> +};
>> +
>> +/* OTG device list */
>> +LIST_HEAD(otg_list);
>> +static DEFINE_MUTEX(otg_list_mutex);
>> +
>> +static int usb_otg_hcd_is_primary_hcd(struct usb_hcd *hcd)
>> +{
>> +	if (!hcd->primary_hcd)
>> +		return 1;
>> +	return hcd == hcd->primary_hcd;
>> +}
>> +
>> +/**
>> + * check if device is in our OTG list and return
>> + * otg_data, else NULL.
>> + *
>> + * otg_list_mutex must be held.
>> + */
>> +static struct otg_data *usb_otg_device_get_otgd(struct device *parent_dev)
>> +{
>> +	struct otg_data *otgd;
>> +
>> +	list_for_each_entry(otgd, &otg_list, list) {
>> +		if (otgd->dev == parent_dev)
>> +			return otgd;
>> +	}
>> +
>> +	return NULL;
>> +}
>> +
>> +/**
>> + * timer callback to set timeout bit and kick FSM
>> + */
>> +static enum hrtimer_restart set_tmout(struct hrtimer *data)
>> +{
>> +	struct otg_timer *otgtimer;
>> +
>> +	otgtimer = container_of(data, struct otg_timer, timer);
>> +	if (otgtimer->timeout_bit)
>> +		*otgtimer->timeout_bit = 1;
>> +
>> +	usb_otg_sync_inputs(&otgtimer->otgd->fsm);
>> +
>> +	return HRTIMER_NORESTART;
>> +}
>> +
>> +/**
>> + * Initialize one OTG timer with callback, timeout and timeout bit
>> + */
>> +static void otg_timer_init(enum otg_fsm_timer id, struct otg_data *otgd,
>> +			   enum hrtimer_restart (*callback)(struct hrtimer *),
>> +			   unsigned long expires_ms,
>> +			   int *timeout_bit)
>> +{
>> +	struct otg_timer *otgtimer = &otgd->timers[id];
>> +	struct hrtimer *timer = &otgtimer->timer;
>> +
>> +	otgtimer->timeout = ms_to_ktime(expires_ms);
>> +	hrtimer_init(timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>> +	timer->function = callback;
>> +
>> +	otgtimer->timeout_bit = timeout_bit;
>> +	otgtimer->otgd = otgd;
>> +}
>> +
>> +/**
>> + * Initialize standard OTG timers
>> + */
>> +static void usb_otg_init_timers(struct otg_data *otgd)
>> +{
>> +	struct otg_fsm *fsm = &otgd->fsm;
>> +
>> +	otg_timer_init(A_WAIT_VRISE, otgd, set_tmout, TA_WAIT_VRISE,
>> +		       &fsm->a_wait_vrise_tmout);
>> +	otg_timer_init(A_WAIT_VFALL, otgd, set_tmout, TA_WAIT_VFALL,
>> +		       &fsm->a_wait_vfall_tmout);
>> +	otg_timer_init(A_WAIT_BCON, otgd, set_tmout, TA_WAIT_BCON,
>> +		       &fsm->a_wait_bcon_tmout);
>> +	otg_timer_init(A_AIDL_BDIS, otgd, set_tmout, TA_AIDL_BDIS,
>> +		       &fsm->a_aidl_bdis_tmout);
>> +	otg_timer_init(A_BIDL_ADIS, otgd, set_tmout, TA_BIDL_ADIS,
>> +		       &fsm->a_bidl_adis_tmout);
>> +	otg_timer_init(B_ASE0_BRST, otgd, set_tmout, TB_ASE0_BRST,
>> +		       &fsm->b_ase0_brst_tmout);
>> +
>> +	otg_timer_init(B_SE0_SRP, otgd, set_tmout, TB_SE0_SRP, &fsm->b_se0_srp);
>> +	otg_timer_init(B_SRP_FAIL, otgd, set_tmout, TB_SRP_FAIL,
>> +		       &fsm->b_srp_done);
>> +
>> +	otg_timer_init(A_WAIT_ENUM, otgd, set_tmout, TB_SRP_FAIL, NULL);
> 
> 2 timers are missing: B_DATA_PLS, B_SSEND_SRP.

Those 2 are not used by usb-otg-fsm.c. We can add it when usb-otg-fsm.c is updated.

> 
>> +}
>> +
>> +/**
>> + * OTG FSM ops function to add timer
>> + */
>> +static void usb_otg_add_timer(struct otg_fsm *fsm, enum otg_fsm_timer id)
>> +{
>> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
>> +	struct otg_timer *otgtimer = &otgd->timers[id];
>> +	struct hrtimer *timer = &otgtimer->timer;
>> +
>> +	if (!otgd->fsm_running)
>> +		return;
>> +
>> +	/* if timer is already active, exit */
>> +	if (hrtimer_active(timer)) {
>> +		dev_err(otgd->dev, "otg: timer %d is already running\n", id);
>> +		return;
>> +	}
>> +
>> +	hrtimer_start(timer, otgtimer->timeout, HRTIMER_MODE_REL);
>> +}
>> +
>> +/**
>> + * OTG FSM ops function to delete timer
>> + */
>> +static void usb_otg_del_timer(struct otg_fsm *fsm, enum otg_fsm_timer id)
>> +{
>> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
>> +	struct hrtimer *timer = &otgd->timers[id].timer;
>> +
>> +	hrtimer_cancel(timer);
>> +}
>> +
>> +/**
>> + * OTG FSM ops function to start/stop host
>> + */
>> +static int usb_otg_start_host(struct otg_fsm *fsm, int on)
>> +{
>> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
>> +	struct otg_hcd_ops *hcd_ops;
>> +
>> +	dev_dbg(otgd->dev, "otg: %s %d\n", __func__, on);
>> +	if (!fsm->otg->host) {
>> +		WARN_ONCE(1, "otg: fsm running without host\n");
>> +		return 0;
>> +	}
>> +
>> +	if (on) {
>> +		/* OTG device operations */
>> +		if (otgd->start_host)
>> +			otgd->start_host(fsm, on);
>> +
>> +		/* start host */
>> +		hcd_ops = otgd->primary_hcd.ops;
>> +		hcd_ops->add(otgd->primary_hcd.hcd, otgd->primary_hcd.irqnum,
>> +			     otgd->primary_hcd.irqflags);
>> +		if (otgd->shared_hcd.hcd) {
>> +			hcd_ops = otgd->shared_hcd.ops;
>> +			hcd_ops->add(otgd->shared_hcd.hcd,
>> +				     otgd->shared_hcd.irqnum,
>> +				     otgd->shared_hcd.irqflags);
>> +		}
>> +	} else {
>> +		/* stop host */
>> +		if (otgd->shared_hcd.hcd) {
>> +			hcd_ops = otgd->shared_hcd.ops;
>> +			hcd_ops->remove(otgd->shared_hcd.hcd);
>> +		}
>> +		hcd_ops = otgd->primary_hcd.ops;
>> +		hcd_ops->remove(otgd->primary_hcd.hcd);
>> +
>> +		/* OTG device operations */
>> +		if (otgd->start_host)
>> +			otgd->start_host(fsm, on);
>> +	}
>> +
>> +	return 0;
>> +}
> 
> I do not see much benefit by this override function, usb_add/remove_hcd
> can be simply included by controller's start_host function, also there
> maybe some additional operations after usb_add_hcd, but this override
> function limit usb_add_hcd() is the last step.

I had tried host start/stop way before but Alan's suggestion was to use
bind/unbind the host controller completely as that is much simpler

[1] http://article.gmane.org/gmane.linux.usb.general/123842

> 
> Maybe your intention is to make usb_add_hcd is the only operation required
> to start host, so ideally controller driver need not define its start_host
> routine for this otg ops, I am not sure if this can work for different otg

Yes that was the intention.

> platforms. If the shared code is only usb_add/remove_hcd(), maybe leave this
> ops defined by controller driver can make core code simple and give flexibility
> to controller drivers.

We don't completely override start/stop_host(). The flexibility is still there.
We call controllers start_host(1) before starting the controller and controllers
start_host(0) after stopping the controller.
So the the controller can still do what they want in otg_fsm_ops.start_host/gadget().

The OTG core only takes care of actually starting/stopping the host controller.

If we don't do that then the code in usb_otg_start_host() has to be pasted
in every OTG controller driver. This is code duplication.

> 
>> +
>> +/**
>> + * OTG FSM ops function to start/stop gadget
>> + */
>> +static int usb_otg_start_gadget(struct otg_fsm *fsm, int on)
>> +{
>> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
>> +	struct usb_gadget *gadget = fsm->otg->gadget;
>> +
>> +	dev_dbg(otgd->dev, "otg: %s %d\n", __func__, on);
>> +	if (!gadget) {
>> +		WARN_ONCE(1, "otg: fsm running without gadget\n");
>> +		return 0;
>> +	}
>> +
>> +	if (on) {
>> +		/* OTG device operations */
>> +		if (otgd->start_gadget)
>> +			otgd->start_gadget(fsm, on);
>> +
>> +		otgd->gadget_ops->start(fsm->otg->gadget);
>> +	} else {
>> +		otgd->gadget_ops->stop(fsm->otg->gadget);
>> +
>> +		/* OTG device operations */
>> +		if (otgd->start_gadget)
>> +			otgd->start_gadget(fsm, on);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * OTG FSM work function
>> + */
>> +static void usb_otg_work(struct work_struct *work)
>> +{
>> +	struct otg_data *otgd = container_of(work, struct otg_data, work);
>> +
>> +	otg_statemachine(&otgd->fsm);
> 
> Need consider runtime pm, or you want to rely on controller driver take
> care of it?

For simplicity let's say that controller driver takes care of it.

> 
>> +}
>> +
>> +/**
>> + * usb_otg_register() - Register the OTG device to OTG core
>> + * @parent_device:	parent device of Host & Gadget controllers.
>> + * @otg_fsm_ops:	otg state machine ops.
>> + *
>> + * Register parent device that contains both HCD and GCD into
>> + * the USB OTG core. HCD and GCD will be prevented from starting
>> + * till both are available for use.
>> + *
>> + * Return: struct otg_fsm * if success, NULL if error.
>> + */
>> +struct otg_fsm *usb_otg_register(struct device *parent_dev,
>> +				 struct otg_fsm_ops *fsm_ops)
>> +{
>> +	struct otg_data *otgd;
>> +	int ret = 0;
>> +
>> +	if (!parent_dev || !fsm_ops)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	/* already in list? */
>> +	mutex_lock(&otg_list_mutex);
>> +	if (usb_otg_device_get_otgd(parent_dev)) {
>> +		dev_err(parent_dev, "otg: %s: device already in otg list\n",
>> +			__func__);
>> +		ret = -EINVAL;
>> +		goto unlock;
>> +	}
>> +
>> +	/* allocate and add to list */
>> +	otgd = kzalloc(sizeof(*otgd), GFP_KERNEL);
>> +	if (!otgd) {
>> +		ret = -ENOMEM;
>> +		goto unlock;
>> +	}
>> +
>> +	otgd->dev = parent_dev;
>> +	INIT_WORK(&otgd->work, usb_otg_work);
>> +	otgd->wq = create_singlethread_workqueue("usb_otg");
>> +	if (!otgd->wq) {
>> +		dev_err(parent_dev, "otg: %s: can't create workqueue\n",
>> +			__func__);
>> +		ret = -ENODEV;
>> +		goto err_wq;
>> +	}
>> +
>> +	usb_otg_init_timers(otgd);
>> +
>> +	/* save original start host/gadget ops */
>> +	otgd->start_host = fsm_ops->start_host;
>> +	otgd->start_gadget = fsm_ops->start_gadget;
>> +	/* create copy of original ops */
>> +	otgd->fsm_ops = *fsm_ops;
>> +	/* override ops */
>> +	otgd->fsm_ops.start_host = usb_otg_start_host;
>> +	otgd->fsm_ops.start_gadget = usb_otg_start_gadget;
>> +	/* FIXME: we ignore caller's timer ops */
>> +	otgd->fsm_ops.add_timer = usb_otg_add_timer;
>> +	otgd->fsm_ops.del_timer = usb_otg_del_timer;
>> +	/* set otg ops */
>> +	otgd->fsm.ops = &otgd->fsm_ops;
>> +	otgd->fsm.otg = &otgd->otg;
>> +
>> +	mutex_init(&otgd->fsm.lock);
>> +
>> +	list_add_tail(&otgd->list, &otg_list);
>> +	mutex_unlock(&otg_list_mutex);
>> +	return &otgd->fsm;
>> +
>> +err_wq:
>> +	kfree(otgd);
>> +unlock:
>> +	mutex_unlock(&otg_list_mutex);
>> +	return ERR_PTR(ret);
>> +}
>> +EXPORT_SYMBOL_GPL(usb_otg_register);
>> +
>> +/**
>> + * usb_otg_unregister() - Unregister the OTG device from USB OTG core
>> + * @parent_device:	parent device of Host & Gadget controllers.
>> + *
>> + * Unregister parent OTG deviced from USB OTG core.
>> + * Prevents unregistering till both Host and Gadget controllers
>> + * have unregistered from the OTG core.
>> + *
>> + * Return: 0 on success, error value otherwise.
>> + */
>> +int usb_otg_unregister(struct device *parent_dev)
>> +{
>> +	struct otg_data *otgd;
>> +
>> +	mutex_lock(&otg_list_mutex);
>> +	otgd = usb_otg_device_get_otgd(parent_dev);
>> +	if (!otgd) {
>> +		dev_err(parent_dev, "otg: %s: device not in otg list\n",
>> +			__func__);
>> +		mutex_unlock(&otg_list_mutex);
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* prevent unregister till both host & gadget have unregistered */
>> +	if (otgd->fsm.otg->host || otgd->fsm.otg->gadget) {
>> +		dev_err(parent_dev, "otg: %s: host/gadget still registered\n",
>> +			__func__);
>> +		return -EBUSY;
>> +	}
>> +
>> +	/* OTG FSM is halted when host/gadget unregistered */
>> +	destroy_workqueue(otgd->wq);
>> +
>> +	/* remove from otg list */
>> +	list_del(&otgd->list);
>> +	kfree(otgd);
>> +	mutex_unlock(&otg_list_mutex);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_otg_unregister);
>> +
>> +/**
>> + * start/kick the OTG FSM if we can
>> + * fsm->lock must be held
>> + */
>> +static void usb_otg_start_fsm(struct otg_fsm *fsm)
>> +{
>> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
>> +
>> +	if (otgd->fsm_running)
>> +		goto kick_fsm;
>> +
>> +	if (!fsm->otg->host) {
>> +		dev_info(otgd->dev, "otg: can't start till host registers\n");
>> +		return;
>> +	}
>> +
> 
> This cannot work, fsm->otg->host is set in usb_otg_register_hcd(), which is
> called by usb_add_hcd(), but usb_add_hcd() should be called only if otg fsm
> already started to some A-device state, deadlock.

I've changed usb_add_hcd() behaviour. Now usb_otg_add_hcd() does the real work of adding
the hcd. usb_add_hcd() prevents the add if it is an otg hcd and just registers
with OTG core.

> 
>> +	if (!fsm->otg->gadget) {
>> +		dev_info(otgd->dev, "otg: can't start till gadget registers\n");
>> +		return;
>> +	}
>> +
>> +	otgd->fsm_running = true;
>> +kick_fsm:
>> +	queue_work(otgd->wq, &otgd->work);
>> +}
>> +
>> +/**
>> + * stop the OTG FSM. Stops Host & Gadget controllers as well.
>> + * fsm->lock must be held
>> + */
>> +static void usb_otg_stop_fsm(struct otg_fsm *fsm)
>> +{
>> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
>> +	int i;
>> +
>> +	if (!otgd->fsm_running)
>> +		return;
>> +
>> +	/* no more new events queued */
>> +	otgd->fsm_running = false;
>> +
>> +	/* Stop state machine / timers */
>> +	for (i = 0; i < ARRAY_SIZE(otgd->timers); i++)
>> +		hrtimer_cancel(&otgd->timers[i].timer);
>> +
>> +	flush_workqueue(otgd->wq);
>> +	fsm->otg->state = OTG_STATE_UNDEFINED;
>> +
>> +	/* stop host/gadget immediately */
>> +	if (fsm->protocol == PROTO_HOST)
>> +		otg_start_host(fsm, 0);
>> +	else if (fsm->protocol == PROTO_GADGET)
>> +		otg_start_gadget(fsm, 0);
>> +	fsm->protocol = PROTO_UNDEF;
>> +}
>> +
>> +/**
>> + * usb_otg_sync_inputs - Sync OTG inputs with the OTG state machine
>> + * @fsm:	OTG FSM instance
>> + *
>> + * Used by the OTG driver to update the inputs to the OTG
>> + * state machine.
>> + *
>> + * Can be called in IRQ context.
>> + */
>> +void usb_otg_sync_inputs(struct otg_fsm *fsm)
>> +{
>> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
>> +
>> +	/* Don't kick FSM till it has started */
>> +	if (!otgd->fsm_running)
>> +		return;
>> +
>> +	/* Kick FSM */
>> +	queue_work(otgd->wq, &otgd->work);
>> +}
>> +EXPORT_SYMBOL_GPL(usb_otg_sync_inputs);
>> +
>> +/**
>> + * usb_otg_kick_fsm - Kick the OTG state machine
>> + * @hcd_gcd_device:	Host/Gadget controller device
>> + *
>> + * Used by USB host/device stack to sync OTG related
>> + * events to the OTG state machine.
>> + * e.g. change in host_bus->b_hnp_enable, gadget->b_hnp_enable
>> + *
>> + * Returns: 0 on success, error value otherwise.
>> + */
>> +int usb_otg_kick_fsm(struct device *hcd_gcd_device)
>> +{
>> +	struct otg_data *otgd;
>> +
>> +	mutex_lock(&otg_list_mutex);
>> +	otgd = usb_otg_device_get_otgd(hcd_gcd_device->parent);
>> +	if (!otgd) {
>> +		dev_err(hcd_gcd_device, "otg: %s: invalid host/gadget device\n",
>> +			__func__);
>> +		mutex_unlock(&otg_list_mutex);
>> +		return -ENODEV;
>> +	}
>> +
>> +	mutex_unlock(&otg_list_mutex);
>> +	usb_otg_sync_inputs(&otgd->fsm);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_otg_kick_fsm);
>> +
>> +/**
>> + * usb_otg_register_hcd - Register Host controller to OTG core
>> + * @hcd:	Host controller device
>> + * @irqnum:	interrupt number
>> + * @irqflags:	interrupt flags
>> + * @ops:	HCD ops to add/remove the HCD
>> + *
>> + * This is used by the USB Host stack to register the Host controller
>> + * to the OTG core. Host controller must not be started by the
>> + * caller as it is left upto the OTG state machine to do so.
>> + *
>> + * Returns: 0 on success, error value otherwise.
>> + */
>> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
>> +			 unsigned long irqflags, struct otg_hcd_ops *ops)
>> +{
>> +	struct otg_data *otgd;
>> +	struct device *otg_dev = hcd->self.controller->parent;
>> +
>> +	mutex_lock(&otg_list_mutex);
>> +	otgd = usb_otg_device_get_otgd(otg_dev);
>> +	if (!otgd) {
>> +		dev_dbg(otg_dev, "otg: %s: device not registered to otg core\n",
>> +			__func__);
>> +		mutex_unlock(&otg_list_mutex);
>> +		return -EINVAL;
>> +	}
>> +
>> +	mutex_unlock(&otg_list_mutex);
>> +	/* HCD will be started by OTG fsm when needed */
>> +	mutex_lock(&otgd->fsm.lock);
>> +	if (otgd->primary_hcd.hcd) {
>> +		/* probably a shared HCD ? */
>> +		if (usb_otg_hcd_is_primary_hcd(hcd)) {
>> +			dev_err(otg_dev, "otg: primary host already registered\n");
>> +			goto err;
>> +		}
>> +
>> +		if (hcd->shared_hcd == otgd->primary_hcd.hcd) {
>> +			if (otgd->shared_hcd.hcd) {
>> +				dev_err(otg_dev, "otg: shared host already registered\n");
>> +				goto err;
>> +			}
>> +
>> +			otgd->shared_hcd.hcd = hcd;
>> +			otgd->shared_hcd.irqnum = irqnum;
>> +			otgd->shared_hcd.irqflags = irqflags;
>> +			otgd->shared_hcd.ops = ops;
>> +			dev_info(otg_dev, "otg: shared host %s registered\n",
>> +				 dev_name(hcd->self.controller));
>> +		} else {
>> +			dev_err(otg_dev, "otg: invalid shared host %s\n",
>> +				dev_name(hcd->self.controller));
>> +			goto err;
>> +		}
>> +	} else {
>> +		if (!usb_otg_hcd_is_primary_hcd(hcd)) {
>> +			dev_err(otg_dev, "otg: primary host must be registered first\n");
>> +			goto err;
>> +		}
>> +
>> +		otgd->primary_hcd.hcd = hcd;
>> +		otgd->primary_hcd.irqnum = irqnum;
>> +		otgd->primary_hcd.irqflags = irqflags;
>> +		otgd->primary_hcd.ops = ops;
>> +		dev_info(otg_dev, "otg: primary host %s registered\n",
>> +			 dev_name(hcd->self.controller));
>> +	}
>> +
>> +	/*
>> +	 * we're ready only if we have shared HCD
>> +	 * or we don't need shared HCD.
>> +	 */
>> +	if (otgd->shared_hcd.hcd || !otgd->primary_hcd.hcd->shared_hcd) {
>> +		otgd->fsm.otg->host = hcd_to_bus(hcd);
>> +		/* FIXME: set bus->otg_port if this is true OTG port with HNP */
>> +
>> +		/* start FSM */
>> +		usb_otg_start_fsm(&otgd->fsm);
> 
> usb_otg_register_hcd() is called before usb_otg_add_hcd(), start fsm on
> this point can make sense since hcd has not been added?

for OTG/DRD HCD case:
- usb_add_hcd() does not really ADD (or START) the HCD. It just registers with OTG core.
- FSM takes care of ADDing (or STARTing) the HCD when it wants using the
usb_otg_add_hcd() call.
- FSM does not need HCD to be already added. It just needs it to be registered.
It takes care of strting it when it wants to.

> 
>> +	} else {
>> +		dev_dbg(otg_dev, "otg: can't start till shared host registers\n");
>> +	}
>> +
>> +	mutex_unlock(&otgd->fsm.lock);
>> +
>> +	return 0;
>> +
>> +err:
>> +	mutex_unlock(&otgd->fsm.lock);
>> +	return -EINVAL;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_otg_register_hcd);
>> +
>> +/**
>> + * usb_otg_unregister_hcd - Unregister Host controller from OTG core
>> + * @hcd:	Host controller device
>> + *
>> + * This is used by the USB Host stack to unregister the Host controller
>> + * from the OTG core. Ensures that Host controller is not running
>> + * on successful return.
>> + *
>> + * Returns: 0 on success, error value otherwise.
>> + */
>> +int usb_otg_unregister_hcd(struct usb_hcd *hcd)
>> +{
>> +	struct otg_data *otgd;
>> +	struct usb_bus *bus = hcd_to_bus(hcd);
>> +	struct device *otg_dev = bus->controller->parent;
>> +
>> +	mutex_lock(&otg_list_mutex);
>> +	otgd = usb_otg_device_get_otgd(otg_dev);
>> +	if (!otgd) {
>> +		dev_err(otg_dev, "otg: %s: device not registered to otg core\n",
>> +			__func__);
>> +		mutex_unlock(&otg_list_mutex);
>> +		return -EINVAL;
>> +	}
>> +
>> +	mutex_unlock(&otg_list_mutex);
>> +
>> +	mutex_lock(&otgd->fsm.lock);
>> +	if (hcd == otgd->primary_hcd.hcd) {
>> +		otgd->primary_hcd.hcd = NULL;
>> +		dev_info(otg_dev, "otg: primary host %s unregistered\n",
>> +			 dev_name(bus->controller));
>> +	} else if (hcd == otgd->shared_hcd.hcd) {
>> +		otgd->shared_hcd.hcd = NULL;
>> +		dev_info(otg_dev, "otg: shared host %s unregistered\n",
>> +			 dev_name(bus->controller));
>> +	} else {
>> +		dev_err(otg_dev, "otg: host %s wasn't registered with otg\n",
>> +			dev_name(bus->controller));
>> +		mutex_unlock(&otgd->fsm.lock);
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* stop FSM & Host */
>> +	usb_otg_stop_fsm(&otgd->fsm);
>> +	otgd->fsm.otg->host = NULL;
>> +
>> +	mutex_unlock(&otgd->fsm.lock);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_otg_unregister_hcd);
>> +
>> +/**
>> + * usb_otg_register_gadget - Register Gadget controller to OTG core
>> + * @gadget:	Gadget controller
>> + *
>> + * This is used by the USB Gadget stack to register the Gadget controller
>> + * to the OTG core. Gadget controller must not be started by the
>> + * caller as it is left upto the OTG state machine to do so.
>> + *
>> + * Gadget core must call this only when all resources required for
>> + * gadget controller to run are available.
>> + * i.e. gadget function driver is available.
>> + *
>> + * Returns: 0 on success, error value otherwise.
>> + */
>> +int usb_otg_register_gadget(struct usb_gadget *gadget,
>> +			    struct otg_gadget_ops *ops)
>> +{
>> +	struct otg_data *otgd;
>> +	struct device *otg_dev = gadget->dev.parent;
>> +
>> +	mutex_lock(&otg_list_mutex);
>> +	otgd = usb_otg_device_get_otgd(otg_dev);
>> +	if (!otgd) {
>> +		dev_err(otg_dev, "otg: %s: device not registered to otg core\n",
>> +			__func__);
>> +		mutex_unlock(&otg_list_mutex);
>> +		return -EINVAL;
>> +	}
>> +
>> +	mutex_unlock(&otg_list_mutex);
>> +
>> +	mutex_lock(&otgd->fsm.lock);
>> +	if (otgd->fsm.otg->gadget) {
>> +		dev_err(otg_dev, "otg: gadget already registered with otg\n");
>> +		mutex_unlock(&otgd->fsm.lock);
>> +		return -EINVAL;
>> +	}
>> +
>> +	otgd->fsm.otg->gadget = gadget;
>> +	otgd->gadget_ops = ops;
>> +	dev_info(otg_dev, "otg: gadget %s registered\n",
>> +		 dev_name(&gadget->dev));
>> +
>> +	/* start FSM */
>> +	usb_otg_start_fsm(&otgd->fsm);
>> +	mutex_unlock(&otgd->fsm.lock);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_otg_register_gadget);
>> +
>> +/**
>> + * usb_otg_unregister_gadget - Unregister Gadget controller from OTG core
>> + * @gadget:	Gadget controller
>> + *
>> + * This is used by the USB Gadget stack to unregister the Gadget controller
>> + * from the OTG core. Ensures that Gadget controller is not running
>> + * on successful return.
>> + *
>> + * Returns: 0 on success, error value otherwise.
>> + */
>> +int usb_otg_unregister_gadget(struct usb_gadget *gadget)
>> +{
>> +	struct otg_data *otgd;
>> +	struct device *otg_dev = gadget->dev.parent;
>> +
>> +	mutex_lock(&otg_list_mutex);
>> +	otgd = usb_otg_device_get_otgd(otg_dev);
>> +	if (!otgd) {
>> +		dev_err(otg_dev, "otg: %s: device not registered to otg core\n",
>> +			__func__);
>> +		mutex_unlock(&otg_list_mutex);
>> +		return -EINVAL;
>> +	}
>> +
>> +	mutex_unlock(&otg_list_mutex);
>> +
>> +	mutex_lock(&otgd->fsm.lock);
>> +	if (otgd->fsm.otg->gadget != gadget) {
>> +		dev_err(otg_dev, "otg: gadget %s wasn't registered with otg\n",
>> +			dev_name(&gadget->dev));
>> +		mutex_unlock(&otgd->fsm.lock);
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Stop FSM & gadget */
>> +	usb_otg_stop_fsm(&otgd->fsm);
>> +	otgd->fsm.otg->gadget = NULL;
>> +	mutex_unlock(&otgd->fsm.lock);
>> +
>> +	dev_info(otg_dev, "otg: gadget %s unregistered\n",
>> +		 dev_name(&gadget->dev));
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_otg_unregister_gadget);
>> +
>> +/**
>> + * usb_otg_fsm_to_dev - Get OTG controller device from struct otg_fsm
>> + * @fsm:	otg_fsm data structure
>> + *
>> + * This is used by the OTG controller driver to get it's device node
>> + * from any of the otg_fsm->ops.
>> + */
>> +struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm)
>> +{
>> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
>> +
>> +	return otgd->dev;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_otg_fsm_to_dev);
>> diff --git a/drivers/usb/common/usb-otg.h b/drivers/usb/common/usb-otg.h
>> new file mode 100644
>> index 0000000..05331f0
>> --- /dev/null
>> +++ b/drivers/usb/common/usb-otg.h
>> @@ -0,0 +1,71 @@
>> +/**
>> + * drivers/usb/common/usb-otg.h - USB OTG core local header
>> + *
>> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
>> + * Author: Roger Quadros <rogerq@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#ifndef __DRIVERS_USB_COMMON_USB_OTG_H
>> +#define __DRIVERS_USB_COMMON_USB_OTG_H
>> +
>> +/*
>> + *  A-DEVICE timing constants
>> + */
>> +
>> +/* Wait for VBUS Rise  */
>> +#define TA_WAIT_VRISE        (100)	/* a_wait_vrise: section 7.1.2
>> +					 * a_wait_vrise_tmr: section 7.4.5.1
>> +					 * TA_VBUS_RISE <= 100ms, section 4.4
>> +					 * Table 4-1: Electrical Characteristics
>> +					 * ->DC Electrical Timing
>> +					 */
>> +/* Wait for VBUS Fall  */
>> +#define TA_WAIT_VFALL        (1000)	/* a_wait_vfall: section 7.1.7
>> +					 * a_wait_vfall_tmr: section: 7.4.5.2
>> +					 */
>> +/* Wait for B-Connect */
>> +#define TA_WAIT_BCON         (10000)	/* a_wait_bcon: section 7.1.3
>> +					 * TA_WAIT_BCON: should be between 1100
>> +					 * and 30000 ms, section 5.5, Table 5-1
>> +					 */
>> +/* A-Idle to B-Disconnect */
>> +#define TA_AIDL_BDIS         (5000)	/* a_suspend min 200 ms, section 5.2.1
>> +					 * TA_AIDL_BDIS: section 5.5, Table 5-1
>> +					 */
>> +/* B-Idle to A-Disconnect */
>> +#define TA_BIDL_ADIS         (500)	/* TA_BIDL_ADIS: section 5.2.1
>> +					 * 500ms is used for B switch to host
>> +					 * for safe
>> +					 */
>> +
>> +/*
>> + * B-device timing constants
>> + */
>> +
>> +/* Data-Line Pulse Time*/
>> +#define TB_DATA_PLS          (10)	/* b_srp_init,continue 5~10ms
>> +					 * section:5.1.3
>> +					 */
>> +/* SRP Fail Time  */
>> +#define TB_SRP_FAIL          (6000)	/* b_srp_init,fail time 5~6s
>> +					 * section:5.1.6
>> +					 */
>> +/* A-SE0 to B-Reset  */
>> +#define TB_ASE0_BRST         (155)	/* minimum 155 ms, section:5.3.1 */
>> +/* SE0 Time Before SRP */
>> +#define TB_SE0_SRP           (1000)	/* b_idle,minimum 1s, section:5.1.2 */
>> +/* SSEND time before SRP */
>> +#define TB_SSEND_SRP         (1500)	/* minimum 1.5 sec, section:5.1.2 */
>> +
>> +#define TB_SESS_VLD          (1000)
>> +
> 
> All otg timer timeout value should be in some *range* defined by otg spec,
> not some specific value, I don't think one specific value can meet all otg
> platforms, so we need find a way to make those value can be configured by
> controller drivers.

OK. How about introducing 'struct usb_otg_config' which must be passed
to usb_otg_register().

/* otg controller configuration */
struct usb_otg_config {
	/* OTG caps */
	struct usb_otg_caps otg_caps;

	/* OTG Timer timeouts in ms. If 0, sane default will be used */
	int ta_wait_vrise;
	...
};

> 
>> +#endif /* __DRIVERS_USB_COMMON_USB_OTG_H */
>> diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
>> index a99c89e..b468a9f 100644
>> --- a/drivers/usb/core/Kconfig
>> +++ b/drivers/usb/core/Kconfig
>> @@ -42,7 +42,7 @@ config USB_DYNAMIC_MINORS
>>  	  If you are unsure about this, say N here.
>>  
>>  config USB_OTG
>> -	bool "OTG support"
>> +	bool "OTG/Dual-role support"
>>  	depends on PM
>>  	default n
>>  	help
>> @@ -75,15 +75,6 @@ config USB_OTG_BLACKLIST_HUB
>>  	  and software costs by not supporting external hubs.  So
>>  	  are "Embedded Hosts" that don't offer OTG support.
>>  
>> -config USB_OTG_FSM
>> -	tristate "USB 2.0 OTG FSM implementation"
>> -	depends on USB
>> -	select USB_OTG
>> -	select USB_PHY
>> -	help
>> -	  Implements OTG Finite State Machine as specified in On-The-Go
>> -	  and Embedded Host Supplement to the USB Revision 2.0 Specification.
>> -
>>  config USB_ULPI_BUS
>>  	tristate "USB ULPI PHY interface support"
>>  	depends on USB_SUPPORT
>> diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
>> index 52661c5..ce6f8d8 100644
>> --- a/include/linux/usb/otg.h
>> +++ b/include/linux/usb/otg.h
>> @@ -10,6 +10,11 @@
>>  #define __LINUX_USB_OTG_H
>>  
>>  #include <linux/phy/phy.h>
>> +#include <linux/device.h>
>> +#include <linux/usb.h>
>> +#include <linux/usb/hcd.h>
>> +#include <linux/usb/gadget.h>
>> +#include <linux/usb/otg-fsm.h>
>>  #include <linux/usb/phy.h>
>>  
>>  struct usb_otg {
>> @@ -23,6 +28,7 @@ struct usb_otg {
>>  
>>  	enum usb_otg_state	state;
>>  
>> +/*------------- deprecated interface -----------------------------*/
>>  	/* bind/unbind the host controller */
>>  	int	(*set_host)(struct usb_otg *otg, struct usb_bus *host);
>>  
>> @@ -38,11 +44,85 @@ struct usb_otg {
>>  
>>  	/* start or continue HNP role switch */
>>  	int	(*start_hnp)(struct usb_otg *otg);
>> -
>> +/*---------------------------------------------------------------*/
>>  };
>>  
>>  extern const char *usb_otg_state_string(enum usb_otg_state state);
>>  
>> +enum usb_dr_mode {
>> +	USB_DR_MODE_UNKNOWN,
>> +	USB_DR_MODE_HOST,
>> +	USB_DR_MODE_PERIPHERAL,
>> +	USB_DR_MODE_OTG,
>> +};
>> +
>> +#if IS_ENABLED(CONFIG_USB_OTG)
>> +struct otg_fsm *usb_otg_register(struct device *parent_dev,
>> +				 struct otg_fsm_ops *fsm_ops);
>> +int usb_otg_unregister(struct device *parent_dev);
>> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
>> +			 unsigned long irqflags, struct otg_hcd_ops *ops);
>> +int usb_otg_unregister_hcd(struct usb_hcd *hcd);
>> +int usb_otg_register_gadget(struct usb_gadget *gadget,
>> +			    struct otg_gadget_ops *ops);
>> +int usb_otg_unregister_gadget(struct usb_gadget *gadget);
>> +void usb_otg_sync_inputs(struct otg_fsm *fsm);
>> +int usb_otg_kick_fsm(struct device *hcd_gcd_device);
>> +struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm);
>> +
>> +#else /* CONFIG_USB_OTG */
>> +
>> +static inline struct otg_fsm *usb_otg_register(struct device *parent_dev,
>> +					       struct otg_fsm_ops *fsm_ops)
>> +{
>> +	return ERR_PTR(-ENOSYS);
>> +}
>> +
>> +static inline int usb_otg_unregister(struct device *parent_dev)
>> +{
>> +	return -ENOSYS;
>> +}
>> +
>> +static inline int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
>> +				       unsigned long irqflags,
>> +				       struct otg_hcd_ops *ops)
>> +{
>> +	return -ENOSYS;
>> +}
>> +
>> +static inline int usb_otg_unregister_hcd(struct usb_hcd *hcd)
>> +{
>> +	return -ENOSYS;
>> +}
>> +
>> +static inline int usb_otg_register_gadget(struct usb_gadget *gadget,
>> +					  struct otg_gadget_ops *ops)
>> +{
>> +	return -ENOSYS;
>> +}
>> +
>> +static inline int usb_otg_unregister_gadget(struct usb_gadget *gadget)
>> +{
>> +	return -ENOSYS;
>> +}
>> +
>> +static inline void usb_otg_sync_inputs(struct otg_fsm *fsm)
>> +{
>> +}
>> +
>> +static inline int usb_otg_kick_fsm(struct device *hcd_gcd_device)
>> +{
>> +	return -ENOSYS;
>> +}
>> +
>> +static inline struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm)
>> +{
>> +	return NULL;
>> +}
>> +
>> +#endif /* CONFIG_USB_OTG */
>> +
>> +/*------------- deprecated interface -----------------------------*/
>>  /* Context: can sleep */
>>  static inline int
>>  otg_start_hnp(struct usb_otg *otg)
>> @@ -94,14 +174,9 @@ otg_start_srp(struct usb_otg *otg)
>>  	return -ENOTSUPP;
>>  }
>>  
>> +/*---------------------------------------------------------------*/
>> +
>>  /* for OTG controller drivers (and maybe other stuff) */
>>  extern int usb_bus_start_enum(struct usb_bus *bus, unsigned port_num);
>>  
>> -enum usb_dr_mode {
>> -	USB_DR_MODE_UNKNOWN,
>> -	USB_DR_MODE_HOST,
>> -	USB_DR_MODE_PERIPHERAL,
>> -	USB_DR_MODE_OTG,
>> -};
>> -
>>  #endif /* __LINUX_USB_OTG_H */
>> -- 
>> 2.1.4
>>
> 
> I assume I will have more comments after try more.

Sure, no worries :)

cheers,
-roger
--
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
Peter Chen July 20, 2015, 1:45 a.m. UTC | #5
On Fri, Jul 17, 2015 at 03:06:18PM +0300, Roger Quadros wrote:
> >> +
> >> +/**
> >> + * OTG FSM ops function to start/stop host
> >> + */
> >> +static int usb_otg_start_host(struct otg_fsm *fsm, int on)
> >> +{
> >> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> >> +	struct otg_hcd_ops *hcd_ops;
> >> +
> >> +	dev_dbg(otgd->dev, "otg: %s %d\n", __func__, on);
> >> +	if (!fsm->otg->host) {
> >> +		WARN_ONCE(1, "otg: fsm running without host\n");
> >> +		return 0;
> >> +	}
> >> +
> >> +	if (on) {
> >> +		/* OTG device operations */
> >> +		if (otgd->start_host)
> >> +			otgd->start_host(fsm, on);
> >> +
> >> +		/* start host */
> >> +		hcd_ops = otgd->primary_hcd.ops;
> >> +		hcd_ops->add(otgd->primary_hcd.hcd, otgd->primary_hcd.irqnum,
> >> +			     otgd->primary_hcd.irqflags);
> >> +		if (otgd->shared_hcd.hcd) {
> >> +			hcd_ops = otgd->shared_hcd.ops;
> >> +			hcd_ops->add(otgd->shared_hcd.hcd,
> >> +				     otgd->shared_hcd.irqnum,
> >> +				     otgd->shared_hcd.irqflags);
> >> +		}
> >> +	} else {
> >> +		/* stop host */
> >> +		if (otgd->shared_hcd.hcd) {
> >> +			hcd_ops = otgd->shared_hcd.ops;
> >> +			hcd_ops->remove(otgd->shared_hcd.hcd);
> >> +		}
> >> +		hcd_ops = otgd->primary_hcd.ops;
> >> +		hcd_ops->remove(otgd->primary_hcd.hcd);
> >> +
> >> +		/* OTG device operations */
> >> +		if (otgd->start_host)
> >> +			otgd->start_host(fsm, on);
> >> +	}
> >> +
> >> +	return 0;
> >> +}
> > 
> > I do not see much benefit by this override function, usb_add/remove_hcd
> > can be simply included by controller's start_host function, also there
> > maybe some additional operations after usb_add_hcd, but this override
> > function limit usb_add_hcd() is the last step.
> 
> I had tried host start/stop way before but Alan's suggestion was to use
> bind/unbind the host controller completely as that is much simpler
> 
> [1] http://article.gmane.org/gmane.linux.usb.general/123842
> 

Jun may want we have some ways to override otg_fsm->ops by platform
drivers, I think we all agree to call usb_add_hcd/usb_remove_hcd
to start/stop host roles, just some platforms may need more than
just call them.
Li Jun July 21, 2015, 10:52 a.m. UTC | #6
Hi,

[...]

> >> +	otg_timer_init(A_WAIT_ENUM, otgd, set_tmout, TB_SRP_FAIL, NULL);
> > 
> > 2 timers are missing: B_DATA_PLS, B_SSEND_SRP.
> 
> Those 2 are not used by usb-otg-fsm.c. We can add it when usb-otg-fsm.c is updated.
> 

ok.

> > 
> >> +}

[...]

> >> +
> >> +/**
> >> + * OTG FSM ops function to start/stop host
> >> + */
> >> +static int usb_otg_start_host(struct otg_fsm *fsm, int on)
> >> +{
> >> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> >> +	struct otg_hcd_ops *hcd_ops;
> >> +
> >> +	dev_dbg(otgd->dev, "otg: %s %d\n", __func__, on);
> >> +	if (!fsm->otg->host) {
> >> +		WARN_ONCE(1, "otg: fsm running without host\n");
> >> +		return 0;
> >> +	}
> >> +
> >> +	if (on) {
> >> +		/* OTG device operations */
> >> +		if (otgd->start_host)
> >> +			otgd->start_host(fsm, on);
> >> +
> >> +		/* start host */
> >> +		hcd_ops = otgd->primary_hcd.ops;
> >> +		hcd_ops->add(otgd->primary_hcd.hcd, otgd->primary_hcd.irqnum,
> >> +			     otgd->primary_hcd.irqflags);
> >> +		if (otgd->shared_hcd.hcd) {
> >> +			hcd_ops = otgd->shared_hcd.ops;
> >> +			hcd_ops->add(otgd->shared_hcd.hcd,
> >> +				     otgd->shared_hcd.irqnum,
> >> +				     otgd->shared_hcd.irqflags);
> >> +		}
> >> +	} else {
> >> +		/* stop host */
> >> +		if (otgd->shared_hcd.hcd) {
> >> +			hcd_ops = otgd->shared_hcd.ops;
> >> +			hcd_ops->remove(otgd->shared_hcd.hcd);
> >> +		}
> >> +		hcd_ops = otgd->primary_hcd.ops;
> >> +		hcd_ops->remove(otgd->primary_hcd.hcd);
> >> +
> >> +		/* OTG device operations */
> >> +		if (otgd->start_host)
> >> +			otgd->start_host(fsm, on);
> >> +	}
> >> +
> >> +	return 0;
> >> +}
> > 
> > I do not see much benefit by this override function, usb_add/remove_hcd
> > can be simply included by controller's start_host function, also there
> > maybe some additional operations after usb_add_hcd, but this override
> > function limit usb_add_hcd() is the last step.
> 
> I had tried host start/stop way before but Alan's suggestion was to use
> bind/unbind the host controller completely as that is much simpler
> 
> [1] http://article.gmane.org/gmane.linux.usb.general/123842
> 

I did not mean host start/stop in your first version, I agree using
usb_add/remove_hcd() for simple.

> > 
> > Maybe your intention is to make usb_add_hcd is the only operation required
> > to start host, so ideally controller driver need not define its start_host
> > routine for this otg ops, I am not sure if this can work for different otg
> 
> Yes that was the intention.
> 
> > platforms. If the shared code is only usb_add/remove_hcd(), maybe leave this
> > ops defined by controller driver can make core code simple and give flexibility
> > to controller drivers.
> 
> We don't completely override start/stop_host(). The flexibility is still there.
> We call controllers start_host(1) before starting the controller and controllers
> start_host(0) after stopping the controller.
> So the the controller can still do what they want in otg_fsm_ops.start_host/gadget().
> 

But if controller driver wants to do something after usb_otg_add_hcd(),
it's impossible with your current usb_otg_start_host().

> The OTG core only takes care of actually starting/stopping the host controller.
> 
> If we don't do that then the code in usb_otg_start_host() has to be pasted
> in every OTG controller driver. This is code duplication.
> 

Actually the only duplication code may be a function call to original
usb_add/remove_hcd().

> > 
> >> +
> >> +/**
> >> + * OTG FSM ops function to start/stop gadget
> >> + */
> >> +static int usb_otg_start_gadget(struct otg_fsm *fsm, int on)
> >> +{
> >> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> >> +	struct usb_gadget *gadget = fsm->otg->gadget;
> >> +
> >> +	dev_dbg(otgd->dev, "otg: %s %d\n", __func__, on);
> >> +	if (!gadget) {
> >> +		WARN_ONCE(1, "otg: fsm running without gadget\n");
> >> +		return 0;
> >> +	}
> >> +
> >> +	if (on) {
> >> +		/* OTG device operations */
> >> +		if (otgd->start_gadget)
> >> +			otgd->start_gadget(fsm, on);
> >> +
> >> +		otgd->gadget_ops->start(fsm->otg->gadget);
> >> +	} else {
> >> +		otgd->gadget_ops->stop(fsm->otg->gadget);
> >> +
> >> +		/* OTG device operations */
> >> +		if (otgd->start_gadget)
> >> +			otgd->start_gadget(fsm, on);
> >> +	}
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +/**
> >> + * OTG FSM work function
> >> + */
> >> +static void usb_otg_work(struct work_struct *work)
> >> +{
> >> +	struct otg_data *otgd = container_of(work, struct otg_data, work);
> >> +
> >> +	otg_statemachine(&otgd->fsm);
> > 
> > Need consider runtime pm, or you want to rely on controller driver take
> > care of it?
> 
> For simplicity let's say that controller driver takes care of it.
> 

Then controller driver need add runtime pm for every otg fsm ops.

Code like below can make it simple:
runtime_pm_get_sync(otgd->dev);
otg_statemachine(&otgd->fsm);
runtime_pm_get_put(otgd->dev);

There is another problem, otg work will only do one state transition, but
in some cases we may need successive state transitions.  

> > 
> >> +}
> >> +
> >> +/**
> >> + * usb_otg_register() - Register the OTG device to OTG core
> >> + * @parent_device:	parent device of Host & Gadget controllers.
> >> + * @otg_fsm_ops:	otg state machine ops.
> >> + *

[...]

> >> +/**
> >> + * start/kick the OTG FSM if we can
> >> + * fsm->lock must be held
> >> + */
> >> +static void usb_otg_start_fsm(struct otg_fsm *fsm)
> >> +{
> >> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> >> +
> >> +	if (otgd->fsm_running)
> >> +		goto kick_fsm;
> >> +
> >> +	if (!fsm->otg->host) {
> >> +		dev_info(otgd->dev, "otg: can't start till host registers\n");
> >> +		return;
> >> +	}
> >> +
> > 
> > This cannot work, fsm->otg->host is set in usb_otg_register_hcd(), which is
> > called by usb_add_hcd(), but usb_add_hcd() should be called only if otg fsm
> > already started to some A-device state, deadlock.
> 
> I've changed usb_add_hcd() behaviour. Now usb_otg_add_hcd() does the real work of adding
> the hcd. usb_add_hcd() prevents the add if it is an otg hcd and just registers
> with OTG core.
> 

So you expect the controller driver still call usb_add_hcd() before otg fsm
start, in which it only registers the created hcd with OTG core.

> > 
> >> +	if (!fsm->otg->gadget) {
> >> +		dev_info(otgd->dev, "otg: can't start till gadget registers\n");
> >> +		return;
> >> +	}
> >> +
> >> +	otgd->fsm_running = true;
> >> +kick_fsm:
> >> +	queue_work(otgd->wq, &otgd->work);
> >> +}
> >> +

[...]

> >> +
> >> +/**
> >> + * usb_otg_register_hcd - Register Host controller to OTG core
> >> + * @hcd:	Host controller device
> >> + * @irqnum:	interrupt number
> >> + * @irqflags:	interrupt flags
> >> + * @ops:	HCD ops to add/remove the HCD
> >> + *
> >> + * This is used by the USB Host stack to register the Host controller
> >> + * to the OTG core. Host controller must not be started by the
> >> + * caller as it is left upto the OTG state machine to do so.
> >> + *
> >> + * Returns: 0 on success, error value otherwise.
> >> + */
> >> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
> >> +			 unsigned long irqflags, struct otg_hcd_ops *ops)
> >> +{
> >> +	struct otg_data *otgd;
> >> +	struct device *otg_dev = hcd->self.controller->parent;
> >> +

I see normally we directly use controller dev for hcd->self.controller,
usb_create_hcd(... struct device *dev, ...)
{
	... ...
	hcd->self.controller = dev;
	... ...
}

For register gadget, it's okay since:
int usb_add_gadget_udc_release(struct device *parent, ...)
{
	... ...
	gadget->dev.parent = parent;
	... ...
}

So we need parent dev for usb_otg_register(struct device *dev,...), and child dev
for usb_create_hcd(struct device *dev,...)? dwc3 is designed like this?

> >> +	mutex_lock(&otg_list_mutex);
> >> +	otgd = usb_otg_device_get_otgd(otg_dev);
> >> +	if (!otgd) {
> >> +		dev_dbg(otg_dev, "otg: %s: device not registered to otg core\n",
> >> +			__func__);
> >> +		mutex_unlock(&otg_list_mutex);
> >> +		return -EINVAL;
> >> +	}
> >> +
> >> +	mutex_unlock(&otg_list_mutex);
> >> +	/* HCD will be started by OTG fsm when needed */
> >> +	mutex_lock(&otgd->fsm.lock);
> >> +	if (otgd->primary_hcd.hcd) {
> >> +		/* probably a shared HCD ? */
> >> +		if (usb_otg_hcd_is_primary_hcd(hcd)) {
> >> +			dev_err(otg_dev, "otg: primary host already registered\n");
> >> +			goto err;
> >> +		}
> >> +
> >> +		if (hcd->shared_hcd == otgd->primary_hcd.hcd) {
> >> +			if (otgd->shared_hcd.hcd) {
> >> +				dev_err(otg_dev, "otg: shared host already registered\n");
> >> +				goto err;
> >> +			}
> >> +
> >> +			otgd->shared_hcd.hcd = hcd;
> >> +			otgd->shared_hcd.irqnum = irqnum;
> >> +			otgd->shared_hcd.irqflags = irqflags;
> >> +			otgd->shared_hcd.ops = ops;
> >> +			dev_info(otg_dev, "otg: shared host %s registered\n",
> >> +				 dev_name(hcd->self.controller));
> >> +		} else {
> >> +			dev_err(otg_dev, "otg: invalid shared host %s\n",
> >> +				dev_name(hcd->self.controller));
> >> +			goto err;
> >> +		}
> >> +	} else {
> >> +		if (!usb_otg_hcd_is_primary_hcd(hcd)) {
> >> +			dev_err(otg_dev, "otg: primary host must be registered first\n");
> >> +			goto err;
> >> +		}
> >> +
> >> +		otgd->primary_hcd.hcd = hcd;
> >> +		otgd->primary_hcd.irqnum = irqnum;
> >> +		otgd->primary_hcd.irqflags = irqflags;
> >> +		otgd->primary_hcd.ops = ops;
> >> +		dev_info(otg_dev, "otg: primary host %s registered\n",
> >> +			 dev_name(hcd->self.controller));
> >> +	}
> >> +
> >> +	/*
> >> +	 * we're ready only if we have shared HCD
> >> +	 * or we don't need shared HCD.
> >> +	 */
> >> +	if (otgd->shared_hcd.hcd || !otgd->primary_hcd.hcd->shared_hcd) {
> >> +		otgd->fsm.otg->host = hcd_to_bus(hcd);
> >> +		/* FIXME: set bus->otg_port if this is true OTG port with HNP */
> >> +
> >> +		/* start FSM */
> >> +		usb_otg_start_fsm(&otgd->fsm);
> > 
> > usb_otg_register_hcd() is called before usb_otg_add_hcd(), start fsm on
> > this point can make sense since hcd has not been added?
> 
> for OTG/DRD HCD case:
> - usb_add_hcd() does not really ADD (or START) the HCD. It just registers with OTG core.
> - FSM takes care of ADDing (or STARTing) the HCD when it wants using the
> usb_otg_add_hcd() call.

Understood.

> - FSM does not need HCD to be already added. It just needs it to be registered.

My point is only registering hcd to OTG core cannot be a valid *input* to make
otg fsm state can be changed, so it's making no sense to call usb_otg_start_fsm(),
but it's no harm.

> It takes care of strting it when it wants to.
> 

Any otg fsm state change(or start it to make its state change) need some otg fsm
input or variables change happen.

> > 
> >> +	} else {
> >> +		dev_dbg(otg_dev, "otg: can't start till shared host registers\n");
> >> +	}
> >> +
> >> +	mutex_unlock(&otgd->fsm.lock);
> >> +
> >> +	return 0;
> >> +
> >> +err:
> >> +	mutex_unlock(&otgd->fsm.lock);
> >> +	return -EINVAL;
> >> +}
> >> +EXPORT_SYMBOL_GPL(usb_otg_register_hcd);

[...]

> >> +#define TB_ASE0_BRST         (155)	/* minimum 155 ms, section:5.3.1 */
> >> +/* SE0 Time Before SRP */
> >> +#define TB_SE0_SRP           (1000)	/* b_idle,minimum 1s, section:5.1.2 */
> >> +/* SSEND time before SRP */
> >> +#define TB_SSEND_SRP         (1500)	/* minimum 1.5 sec, section:5.1.2 */
> >> +
> >> +#define TB_SESS_VLD          (1000)
> >> +
> > 
> > All otg timer timeout value should be in some *range* defined by otg spec,
> > not some specific value, I don't think one specific value can meet all otg
> > platforms, so we need find a way to make those value can be configured by
> > controller drivers.
> 
> OK. How about introducing 'struct usb_otg_config' which must be passed
> to usb_otg_register().
>

I think it's okay.
 
> /* otg controller configuration */
> struct usb_otg_config {
> 	/* OTG caps */
> 	struct usb_otg_caps otg_caps;

You can use a pointer to avoid data copy.

> 
> 	/* OTG Timer timeouts in ms. If 0, sane default will be used */
> 	int ta_wait_vrise;
> 	...
> };
> 

> > 
> >> 2.1.4
> >>
> > 
> > I assume I will have more comments after try more.
> 
> Sure, no worries :)
> 
> cheers,
> -roger
--
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
Roger Quadros July 27, 2015, 10:03 a.m. UTC | #7
Hi,

On 21/07/15 13:52, Li Jun wrote:
> Hi,
> 
> [...]
> 
>>>> +	otg_timer_init(A_WAIT_ENUM, otgd, set_tmout, TB_SRP_FAIL, NULL);
>>>
>>> 2 timers are missing: B_DATA_PLS, B_SSEND_SRP.
>>
>> Those 2 are not used by usb-otg-fsm.c. We can add it when usb-otg-fsm.c is updated.
>>
> 
> ok.
> 
>>>
>>>> +}
> 
> [...]
> 
>>>> +
>>>> +/**
>>>> + * OTG FSM ops function to start/stop host
>>>> + */
>>>> +static int usb_otg_start_host(struct otg_fsm *fsm, int on)
>>>> +{
>>>> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
>>>> +	struct otg_hcd_ops *hcd_ops;
>>>> +
>>>> +	dev_dbg(otgd->dev, "otg: %s %d\n", __func__, on);
>>>> +	if (!fsm->otg->host) {
>>>> +		WARN_ONCE(1, "otg: fsm running without host\n");
>>>> +		return 0;
>>>> +	}
>>>> +
>>>> +	if (on) {
>>>> +		/* OTG device operations */
>>>> +		if (otgd->start_host)
>>>> +			otgd->start_host(fsm, on);
>>>> +
>>>> +		/* start host */
>>>> +		hcd_ops = otgd->primary_hcd.ops;
>>>> +		hcd_ops->add(otgd->primary_hcd.hcd, otgd->primary_hcd.irqnum,
>>>> +			     otgd->primary_hcd.irqflags);
>>>> +		if (otgd->shared_hcd.hcd) {
>>>> +			hcd_ops = otgd->shared_hcd.ops;
>>>> +			hcd_ops->add(otgd->shared_hcd.hcd,
>>>> +				     otgd->shared_hcd.irqnum,
>>>> +				     otgd->shared_hcd.irqflags);
>>>> +		}
>>>> +	} else {
>>>> +		/* stop host */
>>>> +		if (otgd->shared_hcd.hcd) {
>>>> +			hcd_ops = otgd->shared_hcd.ops;
>>>> +			hcd_ops->remove(otgd->shared_hcd.hcd);
>>>> +		}
>>>> +		hcd_ops = otgd->primary_hcd.ops;
>>>> +		hcd_ops->remove(otgd->primary_hcd.hcd);
>>>> +
>>>> +		/* OTG device operations */
>>>> +		if (otgd->start_host)
>>>> +			otgd->start_host(fsm, on);
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +}
>>>
>>> I do not see much benefit by this override function, usb_add/remove_hcd
>>> can be simply included by controller's start_host function, also there
>>> maybe some additional operations after usb_add_hcd, but this override
>>> function limit usb_add_hcd() is the last step.
>>
>> I had tried host start/stop way before but Alan's suggestion was to use
>> bind/unbind the host controller completely as that is much simpler
>>
>> [1] http://article.gmane.org/gmane.linux.usb.general/123842
>>
> 
> I did not mean host start/stop in your first version, I agree using
> usb_add/remove_hcd() for simple.
> 
>>>
>>> Maybe your intention is to make usb_add_hcd is the only operation required
>>> to start host, so ideally controller driver need not define its start_host
>>> routine for this otg ops, I am not sure if this can work for different otg
>>
>> Yes that was the intention.
>>
>>> platforms. If the shared code is only usb_add/remove_hcd(), maybe leave this
>>> ops defined by controller driver can make core code simple and give flexibility
>>> to controller drivers.
>>
>> We don't completely override start/stop_host(). The flexibility is still there.
>> We call controllers start_host(1) before starting the controller and controllers
>> start_host(0) after stopping the controller.
>> So the the controller can still do what they want in otg_fsm_ops.start_host/gadget().
>>
> 
> But if controller driver wants to do something after usb_otg_add_hcd(),
> it's impossible with your current usb_otg_start_host().

Agree with that point. I can't forsee if any driver will need to do that but
we don't want to limit it so i'll consider your point of letting the controller drivers
do whatever they want in start/stop ops.

I can move the existing starts/stop to a library function so they can re-use it if they
don't want anything special.

> 
>> The OTG core only takes care of actually starting/stopping the host controller.
>>
>> If we don't do that then the code in usb_otg_start_host() has to be pasted
>> in every OTG controller driver. This is code duplication.
>>
> 
> Actually the only duplication code may be a function call to original
> usb_add/remove_hcd().

For USB hosts having primary and shared controllers it is not that simple
but they can use the library function in that case.

> 
>>>
>>>> +
>>>> +/**
>>>> + * OTG FSM ops function to start/stop gadget
>>>> + */
>>>> +static int usb_otg_start_gadget(struct otg_fsm *fsm, int on)
>>>> +{
>>>> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
>>>> +	struct usb_gadget *gadget = fsm->otg->gadget;
>>>> +
>>>> +	dev_dbg(otgd->dev, "otg: %s %d\n", __func__, on);
>>>> +	if (!gadget) {
>>>> +		WARN_ONCE(1, "otg: fsm running without gadget\n");
>>>> +		return 0;
>>>> +	}
>>>> +
>>>> +	if (on) {
>>>> +		/* OTG device operations */
>>>> +		if (otgd->start_gadget)
>>>> +			otgd->start_gadget(fsm, on);
>>>> +
>>>> +		otgd->gadget_ops->start(fsm->otg->gadget);
>>>> +	} else {
>>>> +		otgd->gadget_ops->stop(fsm->otg->gadget);
>>>> +
>>>> +		/* OTG device operations */
>>>> +		if (otgd->start_gadget)
>>>> +			otgd->start_gadget(fsm, on);
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +/**
>>>> + * OTG FSM work function
>>>> + */
>>>> +static void usb_otg_work(struct work_struct *work)
>>>> +{
>>>> +	struct otg_data *otgd = container_of(work, struct otg_data, work);
>>>> +
>>>> +	otg_statemachine(&otgd->fsm);
>>>
>>> Need consider runtime pm, or you want to rely on controller driver take
>>> care of it?
>>
>> For simplicity let's say that controller driver takes care of it.
>>
> 
> Then controller driver need add runtime pm for every otg fsm ops.
> 
> Code like below can make it simple:
> runtime_pm_get_sync(otgd->dev);
> otg_statemachine(&otgd->fsm);
> runtime_pm_get_put(otgd->dev);

That can be done.
> 
> There is another problem, otg work will only do one state transition, but
> in some cases we may need successive state transitions.  

How can we fix this?

> 
>>>
>>>> +}
>>>> +
>>>> +/**
>>>> + * usb_otg_register() - Register the OTG device to OTG core
>>>> + * @parent_device:	parent device of Host & Gadget controllers.
>>>> + * @otg_fsm_ops:	otg state machine ops.
>>>> + *
> 
> [...]
> 
>>>> +/**
>>>> + * start/kick the OTG FSM if we can
>>>> + * fsm->lock must be held
>>>> + */
>>>> +static void usb_otg_start_fsm(struct otg_fsm *fsm)
>>>> +{
>>>> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
>>>> +
>>>> +	if (otgd->fsm_running)
>>>> +		goto kick_fsm;
>>>> +
>>>> +	if (!fsm->otg->host) {
>>>> +		dev_info(otgd->dev, "otg: can't start till host registers\n");
>>>> +		return;
>>>> +	}
>>>> +
>>>
>>> This cannot work, fsm->otg->host is set in usb_otg_register_hcd(), which is
>>> called by usb_add_hcd(), but usb_add_hcd() should be called only if otg fsm
>>> already started to some A-device state, deadlock.
>>
>> I've changed usb_add_hcd() behaviour. Now usb_otg_add_hcd() does the real work of adding
>> the hcd. usb_add_hcd() prevents the add if it is an otg hcd and just registers
>> with OTG core.
>>
> 
> So you expect the controller driver still call usb_add_hcd() before otg fsm
> start, in which it only registers the created hcd with OTG core.

Yes.
> 
>>>
>>>> +	if (!fsm->otg->gadget) {
>>>> +		dev_info(otgd->dev, "otg: can't start till gadget registers\n");
>>>> +		return;
>>>> +	}
>>>> +
>>>> +	otgd->fsm_running = true;
>>>> +kick_fsm:
>>>> +	queue_work(otgd->wq, &otgd->work);
>>>> +}
>>>> +
> 
> [...]
> 
>>>> +
>>>> +/**
>>>> + * usb_otg_register_hcd - Register Host controller to OTG core
>>>> + * @hcd:	Host controller device
>>>> + * @irqnum:	interrupt number
>>>> + * @irqflags:	interrupt flags
>>>> + * @ops:	HCD ops to add/remove the HCD
>>>> + *
>>>> + * This is used by the USB Host stack to register the Host controller
>>>> + * to the OTG core. Host controller must not be started by the
>>>> + * caller as it is left upto the OTG state machine to do so.
>>>> + *
>>>> + * Returns: 0 on success, error value otherwise.
>>>> + */
>>>> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
>>>> +			 unsigned long irqflags, struct otg_hcd_ops *ops)
>>>> +{
>>>> +	struct otg_data *otgd;
>>>> +	struct device *otg_dev = hcd->self.controller->parent;
>>>> +
> 
> I see normally we directly use controller dev for hcd->self.controller,
> usb_create_hcd(... struct device *dev, ...)
> {
> 	... ...
> 	hcd->self.controller = dev;
> 	... ...
> }
> 
> For register gadget, it's okay since:
> int usb_add_gadget_udc_release(struct device *parent, ...)
> {
> 	... ...
> 	gadget->dev.parent = parent;
> 	... ...
> }
> 
> So we need parent dev for usb_otg_register(struct device *dev,...), and child dev
> for usb_create_hcd(struct device *dev,...)? dwc3 is designed like this?

Yes. But as we discussed in the cover letter this parent child relationship
doesn't need to be a requirement for device tree case.

> 
>>>> +	mutex_lock(&otg_list_mutex);
>>>> +	otgd = usb_otg_device_get_otgd(otg_dev);
>>>> +	if (!otgd) {
>>>> +		dev_dbg(otg_dev, "otg: %s: device not registered to otg core\n",
>>>> +			__func__);
>>>> +		mutex_unlock(&otg_list_mutex);
>>>> +		return -EINVAL;
>>>> +	}
>>>> +
>>>> +	mutex_unlock(&otg_list_mutex);
>>>> +	/* HCD will be started by OTG fsm when needed */
>>>> +	mutex_lock(&otgd->fsm.lock);
>>>> +	if (otgd->primary_hcd.hcd) {
>>>> +		/* probably a shared HCD ? */
>>>> +		if (usb_otg_hcd_is_primary_hcd(hcd)) {
>>>> +			dev_err(otg_dev, "otg: primary host already registered\n");
>>>> +			goto err;
>>>> +		}
>>>> +
>>>> +		if (hcd->shared_hcd == otgd->primary_hcd.hcd) {
>>>> +			if (otgd->shared_hcd.hcd) {
>>>> +				dev_err(otg_dev, "otg: shared host already registered\n");
>>>> +				goto err;
>>>> +			}
>>>> +
>>>> +			otgd->shared_hcd.hcd = hcd;
>>>> +			otgd->shared_hcd.irqnum = irqnum;
>>>> +			otgd->shared_hcd.irqflags = irqflags;
>>>> +			otgd->shared_hcd.ops = ops;
>>>> +			dev_info(otg_dev, "otg: shared host %s registered\n",
>>>> +				 dev_name(hcd->self.controller));
>>>> +		} else {
>>>> +			dev_err(otg_dev, "otg: invalid shared host %s\n",
>>>> +				dev_name(hcd->self.controller));
>>>> +			goto err;
>>>> +		}
>>>> +	} else {
>>>> +		if (!usb_otg_hcd_is_primary_hcd(hcd)) {
>>>> +			dev_err(otg_dev, "otg: primary host must be registered first\n");
>>>> +			goto err;
>>>> +		}
>>>> +
>>>> +		otgd->primary_hcd.hcd = hcd;
>>>> +		otgd->primary_hcd.irqnum = irqnum;
>>>> +		otgd->primary_hcd.irqflags = irqflags;
>>>> +		otgd->primary_hcd.ops = ops;
>>>> +		dev_info(otg_dev, "otg: primary host %s registered\n",
>>>> +			 dev_name(hcd->self.controller));
>>>> +	}
>>>> +
>>>> +	/*
>>>> +	 * we're ready only if we have shared HCD
>>>> +	 * or we don't need shared HCD.
>>>> +	 */
>>>> +	if (otgd->shared_hcd.hcd || !otgd->primary_hcd.hcd->shared_hcd) {
>>>> +		otgd->fsm.otg->host = hcd_to_bus(hcd);
>>>> +		/* FIXME: set bus->otg_port if this is true OTG port with HNP */
>>>> +
>>>> +		/* start FSM */
>>>> +		usb_otg_start_fsm(&otgd->fsm);
>>>
>>> usb_otg_register_hcd() is called before usb_otg_add_hcd(), start fsm on
>>> this point can make sense since hcd has not been added?
>>
>> for OTG/DRD HCD case:
>> - usb_add_hcd() does not really ADD (or START) the HCD. It just registers with OTG core.
>> - FSM takes care of ADDing (or STARTing) the HCD when it wants using the
>> usb_otg_add_hcd() call.
> 
> Understood.
> 
>> - FSM does not need HCD to be already added. It just needs it to be registered.
> 
> My point is only registering hcd to OTG core cannot be a valid *input* to make
> otg fsm state can be changed, so it's making no sense to call usb_otg_start_fsm(),
> but it's no harm.
> 
>> It takes care of strting it when it wants to.
>>
> 
> Any otg fsm state change(or start it to make its state change) need some otg fsm
> input or variables change happen.

Yes but the OTG FSM can't start till all the resources it needs are available.
i.e. both host and gadget controllers are ready.

> 
>>>
>>>> +	} else {
>>>> +		dev_dbg(otg_dev, "otg: can't start till shared host registers\n");
>>>> +	}
>>>> +
>>>> +	mutex_unlock(&otgd->fsm.lock);
>>>> +
>>>> +	return 0;
>>>> +
>>>> +err:
>>>> +	mutex_unlock(&otgd->fsm.lock);
>>>> +	return -EINVAL;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(usb_otg_register_hcd);
> 
> [...]
> 
>>>> +#define TB_ASE0_BRST         (155)	/* minimum 155 ms, section:5.3.1 */
>>>> +/* SE0 Time Before SRP */
>>>> +#define TB_SE0_SRP           (1000)	/* b_idle,minimum 1s, section:5.1.2 */
>>>> +/* SSEND time before SRP */
>>>> +#define TB_SSEND_SRP         (1500)	/* minimum 1.5 sec, section:5.1.2 */
>>>> +
>>>> +#define TB_SESS_VLD          (1000)
>>>> +
>>>
>>> All otg timer timeout value should be in some *range* defined by otg spec,
>>> not some specific value, I don't think one specific value can meet all otg
>>> platforms, so we need find a way to make those value can be configured by
>>> controller drivers.
>>
>> OK. How about introducing 'struct usb_otg_config' which must be passed
>> to usb_otg_register().
>>
> 
> I think it's okay.
>  
>> /* otg controller configuration */
>> struct usb_otg_config {
>> 	/* OTG caps */
>> 	struct usb_otg_caps otg_caps;
> 
> You can use a pointer to avoid data copy.

OK.

> 
>>
>> 	/* OTG Timer timeouts in ms. If 0, sane default will be used */
>> 	int ta_wait_vrise;
>> 	...
>> };
>>

cheers,
-roger
--
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
Peter Chen Aug. 14, 2015, 8:43 a.m. UTC | #8
On Fri, Aug 14, 2015 at 12:42:38PM +0300, Roger Quadros wrote:
> Hi Peter,
> 
> On 13/07/15 13:20, Roger Quadros wrote:
> > On 13/07/15 05:14, Peter Chen wrote:
> >> On Wed, Jul 08, 2015 at 01:19:33PM +0300, Roger Quadros wrote:
> >>> The OTG core instantiates the OTG Finite State Machine
> >>> per OTG controller and manages starting/stopping the
> >>> host and gadget controllers based on the bus state.
> >>>
> >>> It provides APIs for the following tasks
> >>>
> >>> - Registering an OTG capable controller
> >>> - Registering Host and Gadget controllers to OTG core
> >>> - Providing inputs to and kicking the OTG state machine
> >>>
> >>> Signed-off-by: Roger Quadros <rogerq@ti.com>
> >>> ---
> >>>  MAINTAINERS                  |   4 +-
> >>>  drivers/usb/Kconfig          |   2 +-
> >>>  drivers/usb/Makefile         |   1 +
> >>>  drivers/usb/common/Makefile  |   3 +-
> >>>  drivers/usb/common/usb-otg.c | 768 +++++++++++++++++++++++++++++++++++++++++++
> >>>  drivers/usb/common/usb-otg.h |  71 ++++
> >>>  drivers/usb/core/Kconfig     |  11 +-
> >>>  include/linux/usb/otg.h      |  91 ++++-
> >>>  8 files changed, 930 insertions(+), 21 deletions(-)
> >>>  create mode 100644 drivers/usb/common/usb-otg.c
> >>>  create mode 100644 drivers/usb/common/usb-otg.h
> >>>
> >>> diff --git a/MAINTAINERS b/MAINTAINERS
> >>> index 8133cef..b21278e 100644
> >>> --- a/MAINTAINERS
> >>> +++ b/MAINTAINERS
> >>> @@ -10640,12 +10640,14 @@ S:	Maintained
> >>>  F:	Documentation/usb/ohci.txt
> >>>  F:	drivers/usb/host/ohci*
> >>>  
> >>> -USB OTG FSM (Finite State Machine)
> >>> +USB OTG/DRD core and FSM (Finite State Machine)
> >>>  M:	Peter Chen <Peter.Chen@freescale.com>
> >>> +M:	Roger Quadros <rogerq@ti.com>
> >>>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
> >>>  L:	linux-usb@vger.kernel.org
> >>>  S:	Maintained
> >>>  F:	drivers/usb/common/usb-otg-fsm.c
> >>> +F:	drivers/usb/common/usb-otg.c
> >>>  
> >>>  USB OVER IP DRIVER
> >>>  M:	Valentina Manea <valentina.manea.m@gmail.com>
> >>> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> >>> index 8ed451d..5b625e2 100644
> >>> --- a/drivers/usb/Kconfig
> >>> +++ b/drivers/usb/Kconfig
> >>> @@ -32,7 +32,7 @@ if USB_SUPPORT
> >>>  config USB_COMMON
> >>>  	tristate
> >>>  	default y
> >>> -	depends on USB || USB_GADGET
> >>> +	depends on USB || USB_GADGET || USB_OTG
> >>>  
> >>
> >> USB_OTG can depends on USB || UB_GADGET?
> > 
> > I didn't understand. The above is for USB_COMMON.
> > 
> >>
> >>>  config USB_ARCH_HAS_HCD
> >>>  	def_bool y
> >>> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> >>> index d8926c6..769d13b 100644
> >>> --- a/drivers/usb/Makefile
> >>> +++ b/drivers/usb/Makefile
> >>> @@ -60,5 +60,6 @@ obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
> >>>  obj-$(CONFIG_USB_GADGET)	+= gadget/
> >>>  
> >>>  obj-$(CONFIG_USB_COMMON)	+= common/
> >>> +obj-$(CONFIG_USB_OTG)		+= common/
> >>>  
> >>
> >> The comment like above.
> > 
> > What should it look like?
> > 
> 
> Can you please clarify what you meant at the above two
> comments? Thanks.
> 

Forget them, I had thought the USB_OTG could be module.
Roger Quadros Aug. 14, 2015, 9:42 a.m. UTC | #9
Hi Peter,

On 13/07/15 13:20, Roger Quadros wrote:
> On 13/07/15 05:14, Peter Chen wrote:
>> On Wed, Jul 08, 2015 at 01:19:33PM +0300, Roger Quadros wrote:
>>> The OTG core instantiates the OTG Finite State Machine
>>> per OTG controller and manages starting/stopping the
>>> host and gadget controllers based on the bus state.
>>>
>>> It provides APIs for the following tasks
>>>
>>> - Registering an OTG capable controller
>>> - Registering Host and Gadget controllers to OTG core
>>> - Providing inputs to and kicking the OTG state machine
>>>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>>  MAINTAINERS                  |   4 +-
>>>  drivers/usb/Kconfig          |   2 +-
>>>  drivers/usb/Makefile         |   1 +
>>>  drivers/usb/common/Makefile  |   3 +-
>>>  drivers/usb/common/usb-otg.c | 768 +++++++++++++++++++++++++++++++++++++++++++
>>>  drivers/usb/common/usb-otg.h |  71 ++++
>>>  drivers/usb/core/Kconfig     |  11 +-
>>>  include/linux/usb/otg.h      |  91 ++++-
>>>  8 files changed, 930 insertions(+), 21 deletions(-)
>>>  create mode 100644 drivers/usb/common/usb-otg.c
>>>  create mode 100644 drivers/usb/common/usb-otg.h
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 8133cef..b21278e 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -10640,12 +10640,14 @@ S:	Maintained
>>>  F:	Documentation/usb/ohci.txt
>>>  F:	drivers/usb/host/ohci*
>>>  
>>> -USB OTG FSM (Finite State Machine)
>>> +USB OTG/DRD core and FSM (Finite State Machine)
>>>  M:	Peter Chen <Peter.Chen@freescale.com>
>>> +M:	Roger Quadros <rogerq@ti.com>
>>>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
>>>  L:	linux-usb@vger.kernel.org
>>>  S:	Maintained
>>>  F:	drivers/usb/common/usb-otg-fsm.c
>>> +F:	drivers/usb/common/usb-otg.c
>>>  
>>>  USB OVER IP DRIVER
>>>  M:	Valentina Manea <valentina.manea.m@gmail.com>
>>> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
>>> index 8ed451d..5b625e2 100644
>>> --- a/drivers/usb/Kconfig
>>> +++ b/drivers/usb/Kconfig
>>> @@ -32,7 +32,7 @@ if USB_SUPPORT
>>>  config USB_COMMON
>>>  	tristate
>>>  	default y
>>> -	depends on USB || USB_GADGET
>>> +	depends on USB || USB_GADGET || USB_OTG
>>>  
>>
>> USB_OTG can depends on USB || UB_GADGET?
> 
> I didn't understand. The above is for USB_COMMON.
> 
>>
>>>  config USB_ARCH_HAS_HCD
>>>  	def_bool y
>>> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
>>> index d8926c6..769d13b 100644
>>> --- a/drivers/usb/Makefile
>>> +++ b/drivers/usb/Makefile
>>> @@ -60,5 +60,6 @@ obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
>>>  obj-$(CONFIG_USB_GADGET)	+= gadget/
>>>  
>>>  obj-$(CONFIG_USB_COMMON)	+= common/
>>> +obj-$(CONFIG_USB_OTG)		+= common/
>>>  
>>
>> The comment like above.
> 
> What should it look like?
> 

Can you please clarify what you meant at the above two
comments? Thanks.

cheers,
-roger
--
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
diff mbox

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 8133cef..b21278e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10640,12 +10640,14 @@  S:	Maintained
 F:	Documentation/usb/ohci.txt
 F:	drivers/usb/host/ohci*
 
-USB OTG FSM (Finite State Machine)
+USB OTG/DRD core and FSM (Finite State Machine)
 M:	Peter Chen <Peter.Chen@freescale.com>
+M:	Roger Quadros <rogerq@ti.com>
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
 L:	linux-usb@vger.kernel.org
 S:	Maintained
 F:	drivers/usb/common/usb-otg-fsm.c
+F:	drivers/usb/common/usb-otg.c
 
 USB OVER IP DRIVER
 M:	Valentina Manea <valentina.manea.m@gmail.com>
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 8ed451d..5b625e2 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -32,7 +32,7 @@  if USB_SUPPORT
 config USB_COMMON
 	tristate
 	default y
-	depends on USB || USB_GADGET
+	depends on USB || USB_GADGET || USB_OTG
 
 config USB_ARCH_HAS_HCD
 	def_bool y
diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index d8926c6..769d13b 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -60,5 +60,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/Makefile b/drivers/usb/common/Makefile
index 6bbb3ec..730d928 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -6,5 +6,6 @@  obj-$(CONFIG_USB_COMMON)	  += usb-common.o
 usb-common-y			  += common.o
 usb-common-$(CONFIG_USB_LED_TRIG) += led.o
 
-obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
 obj-$(CONFIG_USB_ULPI_BUS)	+= ulpi.o
+usbotg-y		:= usb-otg.o usb-otg-fsm.o
+obj-$(CONFIG_USB_OTG)	+= usbotg.o
diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
new file mode 100644
index 0000000..0379034
--- /dev/null
+++ b/drivers/usb/common/usb-otg.c
@@ -0,0 +1,768 @@ 
+/**
+ * drivers/usb/common/usb-otg.c - USB OTG core
+ *
+ * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
+ * Author: Roger Quadros <rogerq@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/ktime.h>
+#include <linux/hrtimer.h>
+#include <linux/list.h>
+#include <linux/usb/otg.h>
+#include <linux/usb/phy.h>	/* enum usb_otg_state */
+#include <linux/usb/gadget.h>
+#include <linux/workqueue.h>
+
+#include "usb-otg.h"
+
+/* to link timer with callback data */
+struct otg_timer {
+	struct hrtimer timer;
+	ktime_t timeout;
+	/* callback data */
+	int *timeout_bit;
+	struct otg_data *otgd;
+};
+
+struct otg_hcd {
+	struct usb_hcd *hcd;
+	unsigned int irqnum;
+	unsigned long irqflags;
+	struct otg_hcd_ops *ops;
+};
+
+struct otg_data {
+	struct device *dev;	/* HCD & GCD's parent device */
+
+	struct otg_fsm fsm;
+	/* HCD, GCD and usb_otg_state are present in otg_fsm->otg
+	 * HCD is bus_to_hcd(fsm->otg->host)
+	 * GCD is fsm->otg->gadget
+	 */
+	struct otg_fsm_ops fsm_ops;	/* private copy for override */
+	struct usb_otg otg;		/* allocator for fsm->otg */
+
+	struct otg_hcd primary_hcd;
+	struct otg_hcd shared_hcd;
+
+	struct otg_gadget_ops *gadget_ops; /* interface to gadget f/w */
+
+	/* saved hooks to OTG device */
+	int (*start_host)(struct otg_fsm *fsm, int on);
+	int (*start_gadget)(struct otg_fsm *fsm, int on);
+
+	struct list_head list;
+
+	struct work_struct work;	/* OTG FSM work */
+	struct workqueue_struct *wq;
+
+	struct otg_timer timers[NUM_OTG_FSM_TIMERS];
+
+	bool fsm_running;
+	/* use otg->fsm.lock for serializing access */
+};
+
+/* OTG device list */
+LIST_HEAD(otg_list);
+static DEFINE_MUTEX(otg_list_mutex);
+
+static int usb_otg_hcd_is_primary_hcd(struct usb_hcd *hcd)
+{
+	if (!hcd->primary_hcd)
+		return 1;
+	return hcd == hcd->primary_hcd;
+}
+
+/**
+ * check if device is in our OTG list and return
+ * otg_data, else NULL.
+ *
+ * otg_list_mutex must be held.
+ */
+static struct otg_data *usb_otg_device_get_otgd(struct device *parent_dev)
+{
+	struct otg_data *otgd;
+
+	list_for_each_entry(otgd, &otg_list, list) {
+		if (otgd->dev == parent_dev)
+			return otgd;
+	}
+
+	return NULL;
+}
+
+/**
+ * timer callback to set timeout bit and kick FSM
+ */
+static enum hrtimer_restart set_tmout(struct hrtimer *data)
+{
+	struct otg_timer *otgtimer;
+
+	otgtimer = container_of(data, struct otg_timer, timer);
+	if (otgtimer->timeout_bit)
+		*otgtimer->timeout_bit = 1;
+
+	usb_otg_sync_inputs(&otgtimer->otgd->fsm);
+
+	return HRTIMER_NORESTART;
+}
+
+/**
+ * Initialize one OTG timer with callback, timeout and timeout bit
+ */
+static void otg_timer_init(enum otg_fsm_timer id, struct otg_data *otgd,
+			   enum hrtimer_restart (*callback)(struct hrtimer *),
+			   unsigned long expires_ms,
+			   int *timeout_bit)
+{
+	struct otg_timer *otgtimer = &otgd->timers[id];
+	struct hrtimer *timer = &otgtimer->timer;
+
+	otgtimer->timeout = ms_to_ktime(expires_ms);
+	hrtimer_init(timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	timer->function = callback;
+
+	otgtimer->timeout_bit = timeout_bit;
+	otgtimer->otgd = otgd;
+}
+
+/**
+ * Initialize standard OTG timers
+ */
+static void usb_otg_init_timers(struct otg_data *otgd)
+{
+	struct otg_fsm *fsm = &otgd->fsm;
+
+	otg_timer_init(A_WAIT_VRISE, otgd, set_tmout, TA_WAIT_VRISE,
+		       &fsm->a_wait_vrise_tmout);
+	otg_timer_init(A_WAIT_VFALL, otgd, set_tmout, TA_WAIT_VFALL,
+		       &fsm->a_wait_vfall_tmout);
+	otg_timer_init(A_WAIT_BCON, otgd, set_tmout, TA_WAIT_BCON,
+		       &fsm->a_wait_bcon_tmout);
+	otg_timer_init(A_AIDL_BDIS, otgd, set_tmout, TA_AIDL_BDIS,
+		       &fsm->a_aidl_bdis_tmout);
+	otg_timer_init(A_BIDL_ADIS, otgd, set_tmout, TA_BIDL_ADIS,
+		       &fsm->a_bidl_adis_tmout);
+	otg_timer_init(B_ASE0_BRST, otgd, set_tmout, TB_ASE0_BRST,
+		       &fsm->b_ase0_brst_tmout);
+
+	otg_timer_init(B_SE0_SRP, otgd, set_tmout, TB_SE0_SRP, &fsm->b_se0_srp);
+	otg_timer_init(B_SRP_FAIL, otgd, set_tmout, TB_SRP_FAIL,
+		       &fsm->b_srp_done);
+
+	otg_timer_init(A_WAIT_ENUM, otgd, set_tmout, TB_SRP_FAIL, NULL);
+}
+
+/**
+ * OTG FSM ops function to add timer
+ */
+static void usb_otg_add_timer(struct otg_fsm *fsm, enum otg_fsm_timer id)
+{
+	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
+	struct otg_timer *otgtimer = &otgd->timers[id];
+	struct hrtimer *timer = &otgtimer->timer;
+
+	if (!otgd->fsm_running)
+		return;
+
+	/* if timer is already active, exit */
+	if (hrtimer_active(timer)) {
+		dev_err(otgd->dev, "otg: timer %d is already running\n", id);
+		return;
+	}
+
+	hrtimer_start(timer, otgtimer->timeout, HRTIMER_MODE_REL);
+}
+
+/**
+ * OTG FSM ops function to delete timer
+ */
+static void usb_otg_del_timer(struct otg_fsm *fsm, enum otg_fsm_timer id)
+{
+	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
+	struct hrtimer *timer = &otgd->timers[id].timer;
+
+	hrtimer_cancel(timer);
+}
+
+/**
+ * OTG FSM ops function to start/stop host
+ */
+static int usb_otg_start_host(struct otg_fsm *fsm, int on)
+{
+	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
+	struct otg_hcd_ops *hcd_ops;
+
+	dev_dbg(otgd->dev, "otg: %s %d\n", __func__, on);
+	if (!fsm->otg->host) {
+		WARN_ONCE(1, "otg: fsm running without host\n");
+		return 0;
+	}
+
+	if (on) {
+		/* OTG device operations */
+		if (otgd->start_host)
+			otgd->start_host(fsm, on);
+
+		/* start host */
+		hcd_ops = otgd->primary_hcd.ops;
+		hcd_ops->add(otgd->primary_hcd.hcd, otgd->primary_hcd.irqnum,
+			     otgd->primary_hcd.irqflags);
+		if (otgd->shared_hcd.hcd) {
+			hcd_ops = otgd->shared_hcd.ops;
+			hcd_ops->add(otgd->shared_hcd.hcd,
+				     otgd->shared_hcd.irqnum,
+				     otgd->shared_hcd.irqflags);
+		}
+	} else {
+		/* stop host */
+		if (otgd->shared_hcd.hcd) {
+			hcd_ops = otgd->shared_hcd.ops;
+			hcd_ops->remove(otgd->shared_hcd.hcd);
+		}
+		hcd_ops = otgd->primary_hcd.ops;
+		hcd_ops->remove(otgd->primary_hcd.hcd);
+
+		/* OTG device operations */
+		if (otgd->start_host)
+			otgd->start_host(fsm, on);
+	}
+
+	return 0;
+}
+
+/**
+ * OTG FSM ops function to start/stop gadget
+ */
+static int usb_otg_start_gadget(struct otg_fsm *fsm, int on)
+{
+	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
+	struct usb_gadget *gadget = fsm->otg->gadget;
+
+	dev_dbg(otgd->dev, "otg: %s %d\n", __func__, on);
+	if (!gadget) {
+		WARN_ONCE(1, "otg: fsm running without gadget\n");
+		return 0;
+	}
+
+	if (on) {
+		/* OTG device operations */
+		if (otgd->start_gadget)
+			otgd->start_gadget(fsm, on);
+
+		otgd->gadget_ops->start(fsm->otg->gadget);
+	} else {
+		otgd->gadget_ops->stop(fsm->otg->gadget);
+
+		/* OTG device operations */
+		if (otgd->start_gadget)
+			otgd->start_gadget(fsm, on);
+	}
+
+	return 0;
+}
+
+/**
+ * OTG FSM work function
+ */
+static void usb_otg_work(struct work_struct *work)
+{
+	struct otg_data *otgd = container_of(work, struct otg_data, work);
+
+	otg_statemachine(&otgd->fsm);
+}
+
+/**
+ * usb_otg_register() - Register the OTG device to OTG core
+ * @parent_device:	parent device of Host & Gadget controllers.
+ * @otg_fsm_ops:	otg state machine ops.
+ *
+ * Register parent device that contains both HCD and GCD into
+ * the USB OTG core. HCD and GCD will be prevented from starting
+ * till both are available for use.
+ *
+ * Return: struct otg_fsm * if success, NULL if error.
+ */
+struct otg_fsm *usb_otg_register(struct device *parent_dev,
+				 struct otg_fsm_ops *fsm_ops)
+{
+	struct otg_data *otgd;
+	int ret = 0;
+
+	if (!parent_dev || !fsm_ops)
+		return ERR_PTR(-EINVAL);
+
+	/* already in list? */
+	mutex_lock(&otg_list_mutex);
+	if (usb_otg_device_get_otgd(parent_dev)) {
+		dev_err(parent_dev, "otg: %s: device already in otg list\n",
+			__func__);
+		ret = -EINVAL;
+		goto unlock;
+	}
+
+	/* allocate and add to list */
+	otgd = kzalloc(sizeof(*otgd), GFP_KERNEL);
+	if (!otgd) {
+		ret = -ENOMEM;
+		goto unlock;
+	}
+
+	otgd->dev = parent_dev;
+	INIT_WORK(&otgd->work, usb_otg_work);
+	otgd->wq = create_singlethread_workqueue("usb_otg");
+	if (!otgd->wq) {
+		dev_err(parent_dev, "otg: %s: can't create workqueue\n",
+			__func__);
+		ret = -ENODEV;
+		goto err_wq;
+	}
+
+	usb_otg_init_timers(otgd);
+
+	/* save original start host/gadget ops */
+	otgd->start_host = fsm_ops->start_host;
+	otgd->start_gadget = fsm_ops->start_gadget;
+	/* create copy of original ops */
+	otgd->fsm_ops = *fsm_ops;
+	/* override ops */
+	otgd->fsm_ops.start_host = usb_otg_start_host;
+	otgd->fsm_ops.start_gadget = usb_otg_start_gadget;
+	/* FIXME: we ignore caller's timer ops */
+	otgd->fsm_ops.add_timer = usb_otg_add_timer;
+	otgd->fsm_ops.del_timer = usb_otg_del_timer;
+	/* set otg ops */
+	otgd->fsm.ops = &otgd->fsm_ops;
+	otgd->fsm.otg = &otgd->otg;
+
+	mutex_init(&otgd->fsm.lock);
+
+	list_add_tail(&otgd->list, &otg_list);
+	mutex_unlock(&otg_list_mutex);
+	return &otgd->fsm;
+
+err_wq:
+	kfree(otgd);
+unlock:
+	mutex_unlock(&otg_list_mutex);
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(usb_otg_register);
+
+/**
+ * usb_otg_unregister() - Unregister the OTG device from USB OTG core
+ * @parent_device:	parent device of Host & Gadget controllers.
+ *
+ * Unregister parent OTG deviced from USB OTG core.
+ * Prevents unregistering till both Host and Gadget controllers
+ * have unregistered from the OTG core.
+ *
+ * Return: 0 on success, error value otherwise.
+ */
+int usb_otg_unregister(struct device *parent_dev)
+{
+	struct otg_data *otgd;
+
+	mutex_lock(&otg_list_mutex);
+	otgd = usb_otg_device_get_otgd(parent_dev);
+	if (!otgd) {
+		dev_err(parent_dev, "otg: %s: device not in otg list\n",
+			__func__);
+		mutex_unlock(&otg_list_mutex);
+		return -EINVAL;
+	}
+
+	/* prevent unregister till both host & gadget have unregistered */
+	if (otgd->fsm.otg->host || otgd->fsm.otg->gadget) {
+		dev_err(parent_dev, "otg: %s: host/gadget still registered\n",
+			__func__);
+		return -EBUSY;
+	}
+
+	/* OTG FSM is halted when host/gadget unregistered */
+	destroy_workqueue(otgd->wq);
+
+	/* remove from otg list */
+	list_del(&otgd->list);
+	kfree(otgd);
+	mutex_unlock(&otg_list_mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_otg_unregister);
+
+/**
+ * start/kick the OTG FSM if we can
+ * fsm->lock must be held
+ */
+static void usb_otg_start_fsm(struct otg_fsm *fsm)
+{
+	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
+
+	if (otgd->fsm_running)
+		goto kick_fsm;
+
+	if (!fsm->otg->host) {
+		dev_info(otgd->dev, "otg: can't start till host registers\n");
+		return;
+	}
+
+	if (!fsm->otg->gadget) {
+		dev_info(otgd->dev, "otg: can't start till gadget registers\n");
+		return;
+	}
+
+	otgd->fsm_running = true;
+kick_fsm:
+	queue_work(otgd->wq, &otgd->work);
+}
+
+/**
+ * stop the OTG FSM. Stops Host & Gadget controllers as well.
+ * fsm->lock must be held
+ */
+static void usb_otg_stop_fsm(struct otg_fsm *fsm)
+{
+	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
+	int i;
+
+	if (!otgd->fsm_running)
+		return;
+
+	/* no more new events queued */
+	otgd->fsm_running = false;
+
+	/* Stop state machine / timers */
+	for (i = 0; i < ARRAY_SIZE(otgd->timers); i++)
+		hrtimer_cancel(&otgd->timers[i].timer);
+
+	flush_workqueue(otgd->wq);
+	fsm->otg->state = OTG_STATE_UNDEFINED;
+
+	/* stop host/gadget immediately */
+	if (fsm->protocol == PROTO_HOST)
+		otg_start_host(fsm, 0);
+	else if (fsm->protocol == PROTO_GADGET)
+		otg_start_gadget(fsm, 0);
+	fsm->protocol = PROTO_UNDEF;
+}
+
+/**
+ * usb_otg_sync_inputs - Sync OTG inputs with the OTG state machine
+ * @fsm:	OTG FSM instance
+ *
+ * Used by the OTG driver to update the inputs to the OTG
+ * state machine.
+ *
+ * Can be called in IRQ context.
+ */
+void usb_otg_sync_inputs(struct otg_fsm *fsm)
+{
+	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
+
+	/* Don't kick FSM till it has started */
+	if (!otgd->fsm_running)
+		return;
+
+	/* Kick FSM */
+	queue_work(otgd->wq, &otgd->work);
+}
+EXPORT_SYMBOL_GPL(usb_otg_sync_inputs);
+
+/**
+ * usb_otg_kick_fsm - Kick the OTG state machine
+ * @hcd_gcd_device:	Host/Gadget controller device
+ *
+ * Used by USB host/device stack to sync OTG related
+ * events to the OTG state machine.
+ * e.g. change in host_bus->b_hnp_enable, gadget->b_hnp_enable
+ *
+ * Returns: 0 on success, error value otherwise.
+ */
+int usb_otg_kick_fsm(struct device *hcd_gcd_device)
+{
+	struct otg_data *otgd;
+
+	mutex_lock(&otg_list_mutex);
+	otgd = usb_otg_device_get_otgd(hcd_gcd_device->parent);
+	if (!otgd) {
+		dev_err(hcd_gcd_device, "otg: %s: invalid host/gadget device\n",
+			__func__);
+		mutex_unlock(&otg_list_mutex);
+		return -ENODEV;
+	}
+
+	mutex_unlock(&otg_list_mutex);
+	usb_otg_sync_inputs(&otgd->fsm);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_otg_kick_fsm);
+
+/**
+ * usb_otg_register_hcd - Register Host controller to OTG core
+ * @hcd:	Host controller device
+ * @irqnum:	interrupt number
+ * @irqflags:	interrupt flags
+ * @ops:	HCD ops to add/remove the HCD
+ *
+ * This is used by the USB Host stack to register the Host controller
+ * to the OTG core. Host controller must not be started by the
+ * caller as it is left upto the OTG state machine to do so.
+ *
+ * Returns: 0 on success, error value otherwise.
+ */
+int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
+			 unsigned long irqflags, struct otg_hcd_ops *ops)
+{
+	struct otg_data *otgd;
+	struct device *otg_dev = hcd->self.controller->parent;
+
+	mutex_lock(&otg_list_mutex);
+	otgd = usb_otg_device_get_otgd(otg_dev);
+	if (!otgd) {
+		dev_dbg(otg_dev, "otg: %s: device not registered to otg core\n",
+			__func__);
+		mutex_unlock(&otg_list_mutex);
+		return -EINVAL;
+	}
+
+	mutex_unlock(&otg_list_mutex);
+	/* HCD will be started by OTG fsm when needed */
+	mutex_lock(&otgd->fsm.lock);
+	if (otgd->primary_hcd.hcd) {
+		/* probably a shared HCD ? */
+		if (usb_otg_hcd_is_primary_hcd(hcd)) {
+			dev_err(otg_dev, "otg: primary host already registered\n");
+			goto err;
+		}
+
+		if (hcd->shared_hcd == otgd->primary_hcd.hcd) {
+			if (otgd->shared_hcd.hcd) {
+				dev_err(otg_dev, "otg: shared host already registered\n");
+				goto err;
+			}
+
+			otgd->shared_hcd.hcd = hcd;
+			otgd->shared_hcd.irqnum = irqnum;
+			otgd->shared_hcd.irqflags = irqflags;
+			otgd->shared_hcd.ops = ops;
+			dev_info(otg_dev, "otg: shared host %s registered\n",
+				 dev_name(hcd->self.controller));
+		} else {
+			dev_err(otg_dev, "otg: invalid shared host %s\n",
+				dev_name(hcd->self.controller));
+			goto err;
+		}
+	} else {
+		if (!usb_otg_hcd_is_primary_hcd(hcd)) {
+			dev_err(otg_dev, "otg: primary host must be registered first\n");
+			goto err;
+		}
+
+		otgd->primary_hcd.hcd = hcd;
+		otgd->primary_hcd.irqnum = irqnum;
+		otgd->primary_hcd.irqflags = irqflags;
+		otgd->primary_hcd.ops = ops;
+		dev_info(otg_dev, "otg: primary host %s registered\n",
+			 dev_name(hcd->self.controller));
+	}
+
+	/*
+	 * we're ready only if we have shared HCD
+	 * or we don't need shared HCD.
+	 */
+	if (otgd->shared_hcd.hcd || !otgd->primary_hcd.hcd->shared_hcd) {
+		otgd->fsm.otg->host = hcd_to_bus(hcd);
+		/* FIXME: set bus->otg_port if this is true OTG port with HNP */
+
+		/* start FSM */
+		usb_otg_start_fsm(&otgd->fsm);
+	} else {
+		dev_dbg(otg_dev, "otg: can't start till shared host registers\n");
+	}
+
+	mutex_unlock(&otgd->fsm.lock);
+
+	return 0;
+
+err:
+	mutex_unlock(&otgd->fsm.lock);
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(usb_otg_register_hcd);
+
+/**
+ * usb_otg_unregister_hcd - Unregister Host controller from OTG core
+ * @hcd:	Host controller device
+ *
+ * This is used by the USB Host stack to unregister the Host controller
+ * from the OTG core. Ensures that Host controller is not running
+ * on successful return.
+ *
+ * Returns: 0 on success, error value otherwise.
+ */
+int usb_otg_unregister_hcd(struct usb_hcd *hcd)
+{
+	struct otg_data *otgd;
+	struct usb_bus *bus = hcd_to_bus(hcd);
+	struct device *otg_dev = bus->controller->parent;
+
+	mutex_lock(&otg_list_mutex);
+	otgd = usb_otg_device_get_otgd(otg_dev);
+	if (!otgd) {
+		dev_err(otg_dev, "otg: %s: device not registered to otg core\n",
+			__func__);
+		mutex_unlock(&otg_list_mutex);
+		return -EINVAL;
+	}
+
+	mutex_unlock(&otg_list_mutex);
+
+	mutex_lock(&otgd->fsm.lock);
+	if (hcd == otgd->primary_hcd.hcd) {
+		otgd->primary_hcd.hcd = NULL;
+		dev_info(otg_dev, "otg: primary host %s unregistered\n",
+			 dev_name(bus->controller));
+	} else if (hcd == otgd->shared_hcd.hcd) {
+		otgd->shared_hcd.hcd = NULL;
+		dev_info(otg_dev, "otg: shared host %s unregistered\n",
+			 dev_name(bus->controller));
+	} else {
+		dev_err(otg_dev, "otg: host %s wasn't registered with otg\n",
+			dev_name(bus->controller));
+		mutex_unlock(&otgd->fsm.lock);
+		return -EINVAL;
+	}
+
+	/* stop FSM & Host */
+	usb_otg_stop_fsm(&otgd->fsm);
+	otgd->fsm.otg->host = NULL;
+
+	mutex_unlock(&otgd->fsm.lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_otg_unregister_hcd);
+
+/**
+ * usb_otg_register_gadget - Register Gadget controller to OTG core
+ * @gadget:	Gadget controller
+ *
+ * This is used by the USB Gadget stack to register the Gadget controller
+ * to the OTG core. Gadget controller must not be started by the
+ * caller as it is left upto the OTG state machine to do so.
+ *
+ * Gadget core must call this only when all resources required for
+ * gadget controller to run are available.
+ * i.e. gadget function driver is available.
+ *
+ * Returns: 0 on success, error value otherwise.
+ */
+int usb_otg_register_gadget(struct usb_gadget *gadget,
+			    struct otg_gadget_ops *ops)
+{
+	struct otg_data *otgd;
+	struct device *otg_dev = gadget->dev.parent;
+
+	mutex_lock(&otg_list_mutex);
+	otgd = usb_otg_device_get_otgd(otg_dev);
+	if (!otgd) {
+		dev_err(otg_dev, "otg: %s: device not registered to otg core\n",
+			__func__);
+		mutex_unlock(&otg_list_mutex);
+		return -EINVAL;
+	}
+
+	mutex_unlock(&otg_list_mutex);
+
+	mutex_lock(&otgd->fsm.lock);
+	if (otgd->fsm.otg->gadget) {
+		dev_err(otg_dev, "otg: gadget already registered with otg\n");
+		mutex_unlock(&otgd->fsm.lock);
+		return -EINVAL;
+	}
+
+	otgd->fsm.otg->gadget = gadget;
+	otgd->gadget_ops = ops;
+	dev_info(otg_dev, "otg: gadget %s registered\n",
+		 dev_name(&gadget->dev));
+
+	/* start FSM */
+	usb_otg_start_fsm(&otgd->fsm);
+	mutex_unlock(&otgd->fsm.lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_otg_register_gadget);
+
+/**
+ * usb_otg_unregister_gadget - Unregister Gadget controller from OTG core
+ * @gadget:	Gadget controller
+ *
+ * This is used by the USB Gadget stack to unregister the Gadget controller
+ * from the OTG core. Ensures that Gadget controller is not running
+ * on successful return.
+ *
+ * Returns: 0 on success, error value otherwise.
+ */
+int usb_otg_unregister_gadget(struct usb_gadget *gadget)
+{
+	struct otg_data *otgd;
+	struct device *otg_dev = gadget->dev.parent;
+
+	mutex_lock(&otg_list_mutex);
+	otgd = usb_otg_device_get_otgd(otg_dev);
+	if (!otgd) {
+		dev_err(otg_dev, "otg: %s: device not registered to otg core\n",
+			__func__);
+		mutex_unlock(&otg_list_mutex);
+		return -EINVAL;
+	}
+
+	mutex_unlock(&otg_list_mutex);
+
+	mutex_lock(&otgd->fsm.lock);
+	if (otgd->fsm.otg->gadget != gadget) {
+		dev_err(otg_dev, "otg: gadget %s wasn't registered with otg\n",
+			dev_name(&gadget->dev));
+		mutex_unlock(&otgd->fsm.lock);
+		return -EINVAL;
+	}
+
+	/* Stop FSM & gadget */
+	usb_otg_stop_fsm(&otgd->fsm);
+	otgd->fsm.otg->gadget = NULL;
+	mutex_unlock(&otgd->fsm.lock);
+
+	dev_info(otg_dev, "otg: gadget %s unregistered\n",
+		 dev_name(&gadget->dev));
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_otg_unregister_gadget);
+
+/**
+ * usb_otg_fsm_to_dev - Get OTG controller device from struct otg_fsm
+ * @fsm:	otg_fsm data structure
+ *
+ * This is used by the OTG controller driver to get it's device node
+ * from any of the otg_fsm->ops.
+ */
+struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm)
+{
+	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
+
+	return otgd->dev;
+}
+EXPORT_SYMBOL_GPL(usb_otg_fsm_to_dev);
diff --git a/drivers/usb/common/usb-otg.h b/drivers/usb/common/usb-otg.h
new file mode 100644
index 0000000..05331f0
--- /dev/null
+++ b/drivers/usb/common/usb-otg.h
@@ -0,0 +1,71 @@ 
+/**
+ * drivers/usb/common/usb-otg.h - USB OTG core local header
+ *
+ * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
+ * Author: Roger Quadros <rogerq@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __DRIVERS_USB_COMMON_USB_OTG_H
+#define __DRIVERS_USB_COMMON_USB_OTG_H
+
+/*
+ *  A-DEVICE timing constants
+ */
+
+/* Wait for VBUS Rise  */
+#define TA_WAIT_VRISE        (100)	/* a_wait_vrise: section 7.1.2
+					 * a_wait_vrise_tmr: section 7.4.5.1
+					 * TA_VBUS_RISE <= 100ms, section 4.4
+					 * Table 4-1: Electrical Characteristics
+					 * ->DC Electrical Timing
+					 */
+/* Wait for VBUS Fall  */
+#define TA_WAIT_VFALL        (1000)	/* a_wait_vfall: section 7.1.7
+					 * a_wait_vfall_tmr: section: 7.4.5.2
+					 */
+/* Wait for B-Connect */
+#define TA_WAIT_BCON         (10000)	/* a_wait_bcon: section 7.1.3
+					 * TA_WAIT_BCON: should be between 1100
+					 * and 30000 ms, section 5.5, Table 5-1
+					 */
+/* A-Idle to B-Disconnect */
+#define TA_AIDL_BDIS         (5000)	/* a_suspend min 200 ms, section 5.2.1
+					 * TA_AIDL_BDIS: section 5.5, Table 5-1
+					 */
+/* B-Idle to A-Disconnect */
+#define TA_BIDL_ADIS         (500)	/* TA_BIDL_ADIS: section 5.2.1
+					 * 500ms is used for B switch to host
+					 * for safe
+					 */
+
+/*
+ * B-device timing constants
+ */
+
+/* Data-Line Pulse Time*/
+#define TB_DATA_PLS          (10)	/* b_srp_init,continue 5~10ms
+					 * section:5.1.3
+					 */
+/* SRP Fail Time  */
+#define TB_SRP_FAIL          (6000)	/* b_srp_init,fail time 5~6s
+					 * section:5.1.6
+					 */
+/* A-SE0 to B-Reset  */
+#define TB_ASE0_BRST         (155)	/* minimum 155 ms, section:5.3.1 */
+/* SE0 Time Before SRP */
+#define TB_SE0_SRP           (1000)	/* b_idle,minimum 1s, section:5.1.2 */
+/* SSEND time before SRP */
+#define TB_SSEND_SRP         (1500)	/* minimum 1.5 sec, section:5.1.2 */
+
+#define TB_SESS_VLD          (1000)
+
+#endif /* __DRIVERS_USB_COMMON_USB_OTG_H */
diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
index a99c89e..b468a9f 100644
--- a/drivers/usb/core/Kconfig
+++ b/drivers/usb/core/Kconfig
@@ -42,7 +42,7 @@  config USB_DYNAMIC_MINORS
 	  If you are unsure about this, say N here.
 
 config USB_OTG
-	bool "OTG support"
+	bool "OTG/Dual-role support"
 	depends on PM
 	default n
 	help
@@ -75,15 +75,6 @@  config USB_OTG_BLACKLIST_HUB
 	  and software costs by not supporting external hubs.  So
 	  are "Embedded Hosts" that don't offer OTG support.
 
-config USB_OTG_FSM
-	tristate "USB 2.0 OTG FSM implementation"
-	depends on USB
-	select USB_OTG
-	select USB_PHY
-	help
-	  Implements OTG Finite State Machine as specified in On-The-Go
-	  and Embedded Host Supplement to the USB Revision 2.0 Specification.
-
 config USB_ULPI_BUS
 	tristate "USB ULPI PHY interface support"
 	depends on USB_SUPPORT
diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
index 52661c5..ce6f8d8 100644
--- a/include/linux/usb/otg.h
+++ b/include/linux/usb/otg.h
@@ -10,6 +10,11 @@ 
 #define __LINUX_USB_OTG_H
 
 #include <linux/phy/phy.h>
+#include <linux/device.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+#include <linux/usb/gadget.h>
+#include <linux/usb/otg-fsm.h>
 #include <linux/usb/phy.h>
 
 struct usb_otg {
@@ -23,6 +28,7 @@  struct usb_otg {
 
 	enum usb_otg_state	state;
 
+/*------------- deprecated interface -----------------------------*/
 	/* bind/unbind the host controller */
 	int	(*set_host)(struct usb_otg *otg, struct usb_bus *host);
 
@@ -38,11 +44,85 @@  struct usb_otg {
 
 	/* start or continue HNP role switch */
 	int	(*start_hnp)(struct usb_otg *otg);
-
+/*---------------------------------------------------------------*/
 };
 
 extern const char *usb_otg_state_string(enum usb_otg_state state);
 
+enum usb_dr_mode {
+	USB_DR_MODE_UNKNOWN,
+	USB_DR_MODE_HOST,
+	USB_DR_MODE_PERIPHERAL,
+	USB_DR_MODE_OTG,
+};
+
+#if IS_ENABLED(CONFIG_USB_OTG)
+struct otg_fsm *usb_otg_register(struct device *parent_dev,
+				 struct otg_fsm_ops *fsm_ops);
+int usb_otg_unregister(struct device *parent_dev);
+int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
+			 unsigned long irqflags, struct otg_hcd_ops *ops);
+int usb_otg_unregister_hcd(struct usb_hcd *hcd);
+int usb_otg_register_gadget(struct usb_gadget *gadget,
+			    struct otg_gadget_ops *ops);
+int usb_otg_unregister_gadget(struct usb_gadget *gadget);
+void usb_otg_sync_inputs(struct otg_fsm *fsm);
+int usb_otg_kick_fsm(struct device *hcd_gcd_device);
+struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm);
+
+#else /* CONFIG_USB_OTG */
+
+static inline struct otg_fsm *usb_otg_register(struct device *parent_dev,
+					       struct otg_fsm_ops *fsm_ops)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
+static inline int usb_otg_unregister(struct device *parent_dev)
+{
+	return -ENOSYS;
+}
+
+static inline int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
+				       unsigned long irqflags,
+				       struct otg_hcd_ops *ops)
+{
+	return -ENOSYS;
+}
+
+static inline int usb_otg_unregister_hcd(struct usb_hcd *hcd)
+{
+	return -ENOSYS;
+}
+
+static inline int usb_otg_register_gadget(struct usb_gadget *gadget,
+					  struct otg_gadget_ops *ops)
+{
+	return -ENOSYS;
+}
+
+static inline int usb_otg_unregister_gadget(struct usb_gadget *gadget)
+{
+	return -ENOSYS;
+}
+
+static inline void usb_otg_sync_inputs(struct otg_fsm *fsm)
+{
+}
+
+static inline int usb_otg_kick_fsm(struct device *hcd_gcd_device)
+{
+	return -ENOSYS;
+}
+
+static inline struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm)
+{
+	return NULL;
+}
+
+#endif /* CONFIG_USB_OTG */
+
+/*------------- deprecated interface -----------------------------*/
 /* Context: can sleep */
 static inline int
 otg_start_hnp(struct usb_otg *otg)
@@ -94,14 +174,9 @@  otg_start_srp(struct usb_otg *otg)
 	return -ENOTSUPP;
 }
 
+/*---------------------------------------------------------------*/
+
 /* for OTG controller drivers (and maybe other stuff) */
 extern int usb_bus_start_enum(struct usb_bus *bus, unsigned port_num);
 
-enum usb_dr_mode {
-	USB_DR_MODE_UNKNOWN,
-	USB_DR_MODE_HOST,
-	USB_DR_MODE_PERIPHERAL,
-	USB_DR_MODE_OTG,
-};
-
 #endif /* __LINUX_USB_OTG_H */