All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roger Quadros <rogerq@ti.com>
To: Li Jun <b47624@freescale.com>
Cc: <stern@rowland.harvard.edu>, <balbi@ti.com>,
	<gregkh@linuxfoundation.org>, <peter.chen@freescale.com>,
	<dan.j.williams@intel.com>, <jun.li@freescale.com>,
	<mathias.nyman@linux.intel.com>, <tony@atomide.com>,
	<Joao.Pinto@synopsys.com>, <abrestic@chromium.org>,
	<linux-usb@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-omap@vger.kernel.org>
Subject: Re: [PATCH v4 07/13] usb: otg: add OTG core
Date: Mon, 7 Sep 2015 13:53:19 +0300	[thread overview]
Message-ID: <55ED6C9F.2000502@ti.com> (raw)
In-Reply-To: <20150907074002.GA30578@shlinux2>

On 07/09/15 10:40, Li Jun wrote:
> On Mon, Aug 24, 2015 at 04:21:18PM +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 | 1061 ++++++++++++++++++++++++++++++++++++++++++
>>  drivers/usb/common/usb-otg.h |   71 +++
>>  drivers/usb/core/Kconfig     |   11 +-
>>  include/linux/usb/otg.h      |  189 +++++++-
>>  8 files changed, 1321 insertions(+), 21 deletions(-)
>>  create mode 100644 drivers/usb/common/usb-otg.c
>>  create mode 100644 drivers/usb/common/usb-otg.h
>>
> 
> ... ...
> 
>> +
>> +/**
>> + * Get OTG device from host or gadget device.
>> + *
>> + * For non device tree boot, the OTG controller is assumed to be
>> + * the parent of the host/gadget device.
> 
> This assumption/restriction maybe a problem, as I pointed in your previous
> version, usb_create_hcd() use the passed dev as its dev, but,
> usb_add_gadget_udc() use the passed dev as its parent dev, so often the
> host and gadget don't share the same parent device, at least it doesn't
> apply to chipidea case.

Let's provide a way for OTG driver to provide the OTG core exactly which is
the related host/gadget device.

> 
>> + * For device tree boot, the OTG controller is derived from the
>> + * "otg-controller" property.
>> + */
>> +static struct device *usb_otg_get_device(struct device *hcd_gcd_dev)
>> +{
>> +	struct device *otg_dev;
>> +
>> +	if (!hcd_gcd_dev)
>> +		return NULL;
>> +
>> +	if (hcd_gcd_dev->of_node) {
>> +		struct device_node *np;
>> +		struct platform_device *pdev;
>> +
>> +		np = of_parse_phandle(hcd_gcd_dev->of_node, "otg-controller",
>> +				      0);
>> +		if (!np)
>> +			goto legacy;	/* continue legacy way */
>> +
>> +		pdev = of_find_device_by_node(np);
>> +		of_node_put(np);
>> +		if (!pdev) {
>> +			dev_err(&pdev->dev, "couldn't get otg-controller device\n");
>> +			return NULL;
>> +		}
>> +
>> +		otg_dev = &pdev->dev;
>> +		return otg_dev;
>> +	}
>> +
>> +legacy:
>> +	/* otg device is parent and must be registered */
>> +	otg_dev = hcd_gcd_dev->parent;
>> +	if (!usb_otg_get_data(otg_dev))
>> +		return NULL;
>> +
>> +	return otg_dev;
>> +}
>> +
> 
> ... ...
> 
>> +static void usb_otg_init_timers(struct usb_otg *otgd, unsigned *timeouts)
>> +{
>> +	struct otg_fsm *fsm = &otgd->fsm;
>> +	unsigned long tmouts[NUM_OTG_FSM_TIMERS];
>> +	int i;
>> +
>> +	/* set default timeouts */
>> +	tmouts[A_WAIT_VRISE] = TA_WAIT_VRISE;
>> +	tmouts[A_WAIT_VFALL] = TA_WAIT_VFALL;
>> +	tmouts[A_WAIT_BCON] = TA_WAIT_BCON;
>> +	tmouts[A_AIDL_BDIS] = TA_AIDL_BDIS;
>> +	tmouts[A_BIDL_ADIS] = TA_BIDL_ADIS;
>> +	tmouts[B_ASE0_BRST] = TB_ASE0_BRST;
>> +	tmouts[B_SE0_SRP] = TB_SE0_SRP;
>> +	tmouts[B_SRP_FAIL] = TB_SRP_FAIL;
>> +
>> +	/* set controller provided timeouts */
>> +	if (timeouts) {
>> +		for (i = 0; i < NUM_OTG_FSM_TIMERS; i++) {
>> +			if (timeouts[i])
>> +				tmouts[i] = timeouts[i];
>> +		}
>> +	}
>> +
>> +	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);
>> +
>> +	/* FIXME: what about A_WAIT_ENUM? */
> 
> Either you init it as other timers, or you remove all of it, otherwise
> there will be NULL pointer crash.

I want to initialize it but was not sure about the timeout value.
What timeout value I must use?

> 
>> +}
>> +
>> +/**
>> + * OTG FSM ops function to add timer
>> + */
>> +static void usb_otg_add_timer(struct otg_fsm *fsm, enum otg_fsm_timer id)
>> +{
>> +	struct usb_otg *otgd = container_of(fsm, struct usb_otg, 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 usb_otg *otgd = container_of(fsm, struct usb_otg, fsm);
>> +	struct hrtimer *timer = &otgd->timers[id].timer;
>> +
>> +	hrtimer_cancel(timer);
>> +}
>> +
>> +/**
>> + * Helper function to start/stop otg host. For use by otg controller.
>> + */
>> +int usb_otg_start_host(struct otg_fsm *fsm, int on)
>> +{
>> +	struct usb_otg *otgd = container_of(fsm, struct usb_otg, 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) {
>> +		/* 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);
>> +	}
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_otg_start_host);
>> +
>> +/**
>> + * Helper function to start/stop otg gadget. For use by otg controller.
>> + */
>> +int usb_otg_start_gadget(struct otg_fsm *fsm, int on)
>> +{
>> +	struct usb_otg *otgd = container_of(fsm, struct usb_otg, 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)
>> +		otgd->gadget_ops->start(fsm->otg->gadget);
>> +	else
>> +		otgd->gadget_ops->stop(fsm->otg->gadget);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_otg_start_gadget);
>> +
>> +/**
>> + * OTG FSM work function
>> + */
>> +static void usb_otg_work(struct work_struct *work)
>> +{
>> +	struct usb_otg *otgd = container_of(work, struct usb_otg, work);
>> +
> 
> Add runtime pm for it
> 	pm_runtime_get_sync(otgd->dev);
>> +	otg_statemachine(&otgd->fsm);
> 	pm_runtime_put_sync(otgd->dev);
> 

Sorry missed this one. Will add.

> Also as I raised in your previous version, we need successive state machine
> transition, how about:
> 	if (otg_statemachine(&otgd->fsm))
> 		usb_otg_sync_inputs(&otgd->fsm);

OK. will add this too.

>> +}
>> +
>> +/**
>> + * usb_otg_register() - Register the OTG device to OTG core
>> + * @dev: OTG controller device.
>> + * @config: OTG configuration.
>> + *
>> + * Register the OTG controller device with the USB OTG core.
>> + * The associated Host and Gadget controllers will be prevented from
>> + * being started till both are available for use.
>> + *
>> + * For non device tree boots, the OTG controller device must be the
>> + * parent node of the Host and Gadget controllers.
>> + *
>> + * For device tree case, the otg-controller property must be present
>> + * in the Host and Gadget controller node and it must point to the
>> + * same OTG controller node.
>> + *
>> + * Return: struct otg_fsm * if success, NULL if error.
>> + */
>> +struct otg_fsm *usb_otg_register(struct device *dev,
>> +				 struct usb_otg_config *config)
> 
> Why not return usb_otg? Since you create usb_otg which contains all stuff
> for otg.

yes, returning usb_otg makes more sense now.

> 
>> +{
>> +	struct usb_otg *otgd;
>> +	struct otg_wait_data *wait;
>> +	int ret = 0;
>> +
>> +	if (!dev || !config || !config->fsm_ops)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	/* already in list? */
>> +	mutex_lock(&otg_list_mutex);
>> +	if (usb_otg_get_data(dev)) {
>> +		dev_err(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 = dev;
>> +	otgd->caps = &config->otg_caps;
> 
> How about define otgd->caps as a pointer, then don't need copy it.

otgd->caps is a pointer.

> 
>> +	INIT_WORK(&otgd->work, usb_otg_work);
>> +	otgd->wq = create_singlethread_workqueue("usb_otg");
>> +	if (!otgd->wq) {
>> +		dev_err(dev, "otg: %s: can't create workqueue\n",
>> +			__func__);
>> +		ret = -ENOMEM;
>> +		goto err_wq;
>> +	}
>> +
>> +	usb_otg_init_timers(otgd, config->otg_timeouts);
>> +
>> +	/* create copy of original ops */
>> +	otgd->fsm_ops = *config->fsm_ops;
> 
> The same, use a pointer is enough?

We are creating a copy because we are overriding timer ops.

> 
>> +	/* 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;
>> +
>> +	mutex_init(&otgd->fsm.lock);
>> +
>> +	list_add_tail(&otgd->list, &otg_list);
>> +	mutex_unlock(&otg_list_mutex);
>> +
>> +	/* were we in wait list? */
>> +	mutex_lock(&wait_list_mutex);
>> +	wait = usb_otg_get_wait(dev);
>> +	mutex_unlock(&wait_list_mutex);
>> +	if (wait) {
>> +		/* register pending host/gadget and flush from list */
>> +		usb_otg_flush_wait(dev);
>> +	}
>> +
>> +	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
>> + * @dev: OTG controller device.
>> + *
>> + * Unregister OTG controller device from USB OTG core.
>> + * Prevents unregistering till both the associated Host and Gadget controllers
>> + * have unregistered from the OTG core.
>> + *
>> + * Return: 0 on success, error value otherwise.
>> + */
>> +int usb_otg_unregister(struct device *dev)
>> +{
>> +	struct usb_otg *otgd;
>> +
>> +	mutex_lock(&otg_list_mutex);
>> +	otgd = usb_otg_get_data(dev);
>> +	if (!otgd) {
>> +		dev_err(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(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 usb_otg *otgd = container_of(fsm, struct usb_otg, 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;
> 
> So we need register hcd(usb_otg_register_hcd) before start fsm, right?

Right. But you must use the plain old usb_add_hcd().

> 
>> +	}
>> +
>> +	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 usb_otg *otgd = container_of(fsm, struct usb_otg, 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 usb_otg *otgd = container_of(fsm, struct usb_otg, 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 usb_otg *otgd;
>> +
>> +	mutex_lock(&otg_list_mutex);
>> +	otgd = usb_otg_get_data(usb_otg_get_device(hcd_gcd_device));
>> +	mutex_unlock(&otg_list_mutex);
>> +	if (!otgd) {
>> +		dev_dbg(hcd_gcd_device, "otg: %s: invalid host/gadget device\n",
>> +			__func__);
>> +		return -ENODEV;
>> +	}
>> +
>> +	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.
> 
> I am confused on how to use this function.
> - This function should be called before start fsm per usb_otg_start_fsm().

yes.

> - Called by usb_add_hcd(), so we need call usb_add_hcd() before start fsm.

yes.

> - If I want to add hcd when switch to host role, and remove hcd when switch
>   to peripheral, with this design, I cannot use this function?

You add hcd only once during the life of the OTG device. If it is linked to the
OTG controller the OTG fsm manages the start/stop of hcd using the otg_hcd_ops.

"usb/core/hcd.c"
static struct otg_hcd_ops otg_hcd_intf = {
        .add = usb_otg_add_hcd,
        .remove = usb_otg_remove_hcd,
};

Your otg driver must use teh usb_otg_add/remove_hcd to start/stop the controller.
Using usb_remove_hcd() means the hcd resource is no longer available and the
otg fsm will be stopped.

> - How about split it out of usb_add_hcd()?

Adding the HCD and starting/stopping the hcd is split into
usb_add/remove_hcd() and usb_otg_add/remove_hcd() for OTG case.

> 
>> + *
>> + * 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 usb_otg *otgd;
>> +	struct device *hcd_dev = hcd->self.controller;
>> +	struct device *otg_dev = usb_otg_get_device(hcd_dev);
>> +
>> +	if (!otg_dev)
>> +		return -EINVAL;	/* we're definitely not OTG */
>> +
>> +	/* we're otg but otg controller might not yet be registered */
>> +	mutex_lock(&otg_list_mutex);
>> +	otgd = usb_otg_get_data(otg_dev);
>> +	mutex_unlock(&otg_list_mutex);
>> +	if (!otgd) {
>> +		dev_dbg(hcd_dev,
>> +			"otg: controller not yet registered. waiting..\n");
>> +		/*
>> +		 * otg controller might register later. Put the hcd in
>> +		 * wait list and call us back when ready
>> +		 */
>> +		if (usb_otg_hcd_wait_add(otg_dev, hcd, irqnum, irqflags, ops)) {
>> +			dev_dbg(hcd_dev, "otg: failed to add to wait list\n");
>> +			return -EINVAL;
>> +		}
>> +
>> +		return 0;
>> +	}
>> +
>> +	/* HCD will be started by OTG fsm when needed */
>> +	mutex_lock(&otgd->fsm.lock);
> 
> If call usb_add_hcd() when start host role, deadlock.

No. You must call usb_otg_add_hcd() to start host role.

> 
>> +	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);
> 
> otgd->host = hcd_to_bus(hcd);

ok. So we set host at both places. struct usb_otg in struct otg_fsm starts to
feel redundant now. I think we should get rid of it and get the usb_otg struct
using container_of() instead.

> 
>> +		/* 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;
> 
> Return non-zero, then if err, do we need call usb_otg_add_hcd() after
> usb_otg_register_hcd() fails?

You should not call usb_otg_register_hcd() but usb_add_hcd().
If that fails then you fail as ususal.

> 
>> +}
>> +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 usb_otg *otgd;
>> +	struct device *hcd_dev = hcd_to_bus(hcd)->controller;
>> +	struct device *otg_dev = usb_otg_get_device(hcd_dev);
>> +
>> +	if (!otg_dev)
>> +		return -EINVAL;	/* we're definitely not OTG */
>> +
>> +	mutex_lock(&otg_list_mutex);
>> +	otgd = usb_otg_get_data(otg_dev);
>> +	mutex_unlock(&otg_list_mutex);
>> +	if (!otgd) {
>> +		/* are we in wait list? */
>> +		if (!usb_otg_hcd_wait_remove(hcd))
>> +			return 0;
>> +
>> +		dev_dbg(hcd_dev, "otg: host wasn't registered with otg\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	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(hcd_dev));
>> +	} else if (hcd == otgd->shared_hcd.hcd) {
>> +		otgd->shared_hcd.hcd = NULL;
>> +		dev_info(otg_dev, "otg: shared host %s unregistered\n",
>> +			 dev_name(hcd_dev));
>> +	} else {
>> +		dev_err(otg_dev, "otg: host %s wasn't registered with otg\n",
>> +			dev_name(hcd_dev));
>> +		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 usb_otg *otgd;
>> +	struct device *gadget_dev = &gadget->dev;
>> +	struct device *otg_dev = usb_otg_get_device(gadget_dev);
>> +
>> +	if (!otg_dev)
>> +		return -EINVAL;	/* we're definitely not OTG */
>> +
>> +	/* we're otg but otg controller might not yet be registered */
>> +	mutex_lock(&otg_list_mutex);
>> +	otgd = usb_otg_get_data(otg_dev);
>> +	mutex_unlock(&otg_list_mutex);
>> +	if (!otgd) {
>> +		dev_dbg(gadget_dev,
>> +			"otg: controller not yet registered. waiting..\n");
>> +		/*
>> +		 * otg controller might register later. Put the gadget in
>> +		 * wait list and call us back when ready
>> +		 */
>> +		if (usb_otg_gadget_wait_add(otg_dev, gadget, ops)) {
>> +			dev_dbg(gadget_dev, "otg: failed to add to wait list\n");
>> +			return -EINVAL;
>> +		}
>> +
>> +		return 0;
>> +	}
>> +
>> +	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 usb_otg *otgd;
>> +	struct device *gadget_dev = &gadget->dev;
>> +	struct device *otg_dev = usb_otg_get_device(gadget_dev);
>> +
>> +	if (!otg_dev)
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&otg_list_mutex);
>> +	otgd = usb_otg_get_data(otg_dev);
>> +	mutex_unlock(&otg_list_mutex);
>> +	if (!otgd) {
>> +		/* are we in wait list? */
>> +		if (!usb_otg_gadget_wait_remove(gadget))
>> +			return 0;
>> +
>> +		dev_dbg(gadget_dev, "otg: gadget wasn't registered with otg\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	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 usb_otg *otgd = container_of(fsm, struct usb_otg, 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 bd1dcf8..38cabe0 100644
>> --- a/include/linux/usb/otg.h
>> +++ b/include/linux/usb/otg.h
>> @@ -10,19 +10,100 @@
>>  #define __LINUX_USB_OTG_H
>>  
>>  #include <linux/phy/phy.h>
>> +#include <linux/device.h>
>> +#include <linux/hrtimer.h>
>> +#include <linux/ktime.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 otg_hcd - host controller state and interface
>> + *
>> + * @hcd: host controller
>> + * @irqnum: irq number
>> + * @irqflags: irq flags
>> + * @ops: otg to host controller interface
>> + */
>> +struct otg_hcd {
>> +	struct usb_hcd *hcd;
>> +	unsigned int irqnum;
>> +	unsigned long irqflags;
>> +	struct otg_hcd_ops *ops;
>> +};
>> +
>> +struct usb_otg;
>> +
>> +/**
>> + * struct otg_timer - otg timer data
>> + *
>> + * @timer: high resolution timer
>> + * @timeout: timeout value
>> + * @timetout_bit: pointer to variable that is set on timeout
>> + * @otgd: usb otg data
>> + */
>> +struct otg_timer {
>> +	struct hrtimer timer;
>> +	ktime_t timeout;
>> +	/* callback data */
>> +	int *timeout_bit;
>> +	struct usb_otg *otgd;
>> +};
>> +
>> +/**
>> + * struct usb_otg - usb otg controller state
>> + *
>> + * @default_a: Indicates we are an A device. i.e. Host.
>> + * @phy: USB phy interface
>> + * @usb_phy: old usb_phy interface
>> + * @host: host controller bus
>> + * @gadget: gadget device
>> + * @state: current otg state
>> + * @dev: otg controller device
>> + * @caps: otg capabilities revision, hnp, srp, etc
>> + * @fsm: otg finite state machine
>> + * @fsm_ops: controller hooks for the state machine
>> + * ------- internal use only -------
>> + * @primary_hcd: primary host state and interface
>> + * @shared_hcd: shared host state and interface
>> + * @gadget_ops: gadget interface
>> + * @timers: otg timers for state machine
>> + * @list: list of otg controllers
>> + * @work: otg state machine work
>> + * @wq: otg state machine work queue
>> + * @fsm_running: state machine running/stopped indicator
>> + */
>>  struct usb_otg {
>>  	u8			default_a;
>>  
>>  	struct phy		*phy;
>>  	/* old usb_phy interface */
>>  	struct usb_phy		*usb_phy;
>> +
> 
> add a blank line?
> 
>>  	struct usb_bus		*host;
>>  	struct usb_gadget	*gadget;
>>  
>>  	enum usb_otg_state	state;
>>  
>> +	struct device *dev;
>> +	struct usb_otg_caps *caps;
>> +	struct otg_fsm fsm;
>> +	struct otg_fsm_ops fsm_ops;
>> +
>> +	/* internal use only */
>> +	struct otg_hcd primary_hcd;
>> +	struct otg_hcd shared_hcd;
>> +	struct otg_gadget_ops *gadget_ops;
>> +	struct otg_timer timers[NUM_OTG_FSM_TIMERS];
>> +	struct list_head list;
>> +	struct work_struct work;
>> +	struct workqueue_struct *wq;
>> +	bool fsm_running;
> 
> Should fsm_running be added in otg_fsm struct?

Yes, can be moved there.

> 
>> +	/* use otg->fsm.lock for serializing access */
>> +
>> +/*------------- deprecated interface -----------------------------*/
>>  	/* bind/unbind the host controller */
>>  	int	(*set_host)(struct usb_otg *otg, struct usb_bus *host);
>>  
>> @@ -38,7 +119,7 @@ struct usb_otg {
>>  
>>  	/* start or continue HNP role switch */
>>  	int	(*start_hnp)(struct usb_otg *otg);
>> -
>> +/*---------------------------------------------------------------*/
>>  };
>>  
>>  /**
>> @@ -56,8 +137,105 @@ struct usb_otg_caps {
>>  	bool adp_support;
>>  };
>>  
>> +/**
>> + * struct usb_otg_config - otg controller configuration
>> + * @caps: otg capabilities of the controller
>> + * @ops: otg fsm operations
>> + * @otg_timeouts: override default otg fsm timeouts
>> + */
>> +struct usb_otg_config {
>> +	struct usb_otg_caps otg_caps;
>> +	struct otg_fsm_ops *fsm_ops;
>> +	unsigned otg_timeouts[NUM_OTG_FSM_TIMERS];
>> +};
>> +
>>  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 *dev,
>> +				 struct usb_otg_config *config);
>> +int usb_otg_unregister(struct device *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);
>> +int usb_otg_start_host(struct otg_fsm *fsm, int on);
>> +int usb_otg_start_gadget(struct otg_fsm *fsm, int on);
>> +
>> +#else /* CONFIG_USB_OTG */
>> +
>> +static inline struct otg_fsm *usb_otg_register(struct device *dev,
>> +					       struct usb_otg_config *config)
>> +{
>> +	return ERR_PTR(-ENOTSUPP);
>> +}
>> +
>> +static inline int usb_otg_unregister(struct device *dev)
>> +{
>> +	return -ENOTSUPP;
>> +}
>> +
>> +static inline int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
>> +				       unsigned long irqflags,
>> +				       struct otg_hcd_ops *ops)
>> +{
>> +	return -ENOTSUPP;
>> +}
>> +
>> +static inline int usb_otg_unregister_hcd(struct usb_hcd *hcd)
>> +{
>> +	return -ENOTSUPP;
>> +}
>> +
>> +static inline int usb_otg_register_gadget(struct usb_gadget *gadget,
>> +					  struct otg_gadget_ops *ops)
>> +{
>> +	return -ENOTSUPP;
>> +}
>> +
>> +static inline int usb_otg_unregister_gadget(struct usb_gadget *gadget)
>> +{
>> +	return -ENOTSUPP;
>> +}
>> +
>> +static inline void usb_otg_sync_inputs(struct otg_fsm *fsm)
>> +{
>> +}
>> +
>> +static inline int usb_otg_kick_fsm(struct device *hcd_gcd_device)
>> +{
>> +	return -ENOTSUPP;
>> +}
>> +
>> +static inline struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm)
>> +{
>> +	return NULL;
>> +}
>> +
>> +static inline int usb_otg_start_host(struct otg_fsm *fsm, int on)
>> +{
>> +	return -ENOTSUPP;
>> +}
>> +
>> +static inline int usb_otg_start_gadget(struct otg_fsm *fsm, int on)
>> +{
>> +	return -ENOTSUPP;
>> +}
>> +#endif /* CONFIG_USB_OTG */
>> +
>> +/*------------- deprecated interface -----------------------------*/
>>  /* Context: can sleep */
>>  static inline int
>>  otg_start_hnp(struct usb_otg *otg)
>> @@ -109,14 +287,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
>>

WARNING: multiple messages have this Message-ID (diff)
From: Roger Quadros <rogerq@ti.com>
To: Li Jun <b47624@freescale.com>
Cc: stern@rowland.harvard.edu, balbi@ti.com,
	gregkh@linuxfoundation.org, peter.chen@freescale.com,
	dan.j.williams@intel.com, jun.li@freescale.com,
	mathias.nyman@linux.intel.com, tony@atomide.com,
	Joao.Pinto@synopsys.com, abrestic@chromium.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-omap@vger.kernel.org
Subject: Re: [PATCH v4 07/13] usb: otg: add OTG core
Date: Mon, 7 Sep 2015 13:53:19 +0300	[thread overview]
Message-ID: <55ED6C9F.2000502@ti.com> (raw)
In-Reply-To: <20150907074002.GA30578@shlinux2>

On 07/09/15 10:40, Li Jun wrote:
> On Mon, Aug 24, 2015 at 04:21:18PM +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 | 1061 ++++++++++++++++++++++++++++++++++++++++++
>>  drivers/usb/common/usb-otg.h |   71 +++
>>  drivers/usb/core/Kconfig     |   11 +-
>>  include/linux/usb/otg.h      |  189 +++++++-
>>  8 files changed, 1321 insertions(+), 21 deletions(-)
>>  create mode 100644 drivers/usb/common/usb-otg.c
>>  create mode 100644 drivers/usb/common/usb-otg.h
>>
> 
> ... ...
> 
>> +
>> +/**
>> + * Get OTG device from host or gadget device.
>> + *
>> + * For non device tree boot, the OTG controller is assumed to be
>> + * the parent of the host/gadget device.
> 
> This assumption/restriction maybe a problem, as I pointed in your previous
> version, usb_create_hcd() use the passed dev as its dev, but,
> usb_add_gadget_udc() use the passed dev as its parent dev, so often the
> host and gadget don't share the same parent device, at least it doesn't
> apply to chipidea case.

Let's provide a way for OTG driver to provide the OTG core exactly which is
the related host/gadget device.

> 
>> + * For device tree boot, the OTG controller is derived from the
>> + * "otg-controller" property.
>> + */
>> +static struct device *usb_otg_get_device(struct device *hcd_gcd_dev)
>> +{
>> +	struct device *otg_dev;
>> +
>> +	if (!hcd_gcd_dev)
>> +		return NULL;
>> +
>> +	if (hcd_gcd_dev->of_node) {
>> +		struct device_node *np;
>> +		struct platform_device *pdev;
>> +
>> +		np = of_parse_phandle(hcd_gcd_dev->of_node, "otg-controller",
>> +				      0);
>> +		if (!np)
>> +			goto legacy;	/* continue legacy way */
>> +
>> +		pdev = of_find_device_by_node(np);
>> +		of_node_put(np);
>> +		if (!pdev) {
>> +			dev_err(&pdev->dev, "couldn't get otg-controller device\n");
>> +			return NULL;
>> +		}
>> +
>> +		otg_dev = &pdev->dev;
>> +		return otg_dev;
>> +	}
>> +
>> +legacy:
>> +	/* otg device is parent and must be registered */
>> +	otg_dev = hcd_gcd_dev->parent;
>> +	if (!usb_otg_get_data(otg_dev))
>> +		return NULL;
>> +
>> +	return otg_dev;
>> +}
>> +
> 
> ... ...
> 
>> +static void usb_otg_init_timers(struct usb_otg *otgd, unsigned *timeouts)
>> +{
>> +	struct otg_fsm *fsm = &otgd->fsm;
>> +	unsigned long tmouts[NUM_OTG_FSM_TIMERS];
>> +	int i;
>> +
>> +	/* set default timeouts */
>> +	tmouts[A_WAIT_VRISE] = TA_WAIT_VRISE;
>> +	tmouts[A_WAIT_VFALL] = TA_WAIT_VFALL;
>> +	tmouts[A_WAIT_BCON] = TA_WAIT_BCON;
>> +	tmouts[A_AIDL_BDIS] = TA_AIDL_BDIS;
>> +	tmouts[A_BIDL_ADIS] = TA_BIDL_ADIS;
>> +	tmouts[B_ASE0_BRST] = TB_ASE0_BRST;
>> +	tmouts[B_SE0_SRP] = TB_SE0_SRP;
>> +	tmouts[B_SRP_FAIL] = TB_SRP_FAIL;
>> +
>> +	/* set controller provided timeouts */
>> +	if (timeouts) {
>> +		for (i = 0; i < NUM_OTG_FSM_TIMERS; i++) {
>> +			if (timeouts[i])
>> +				tmouts[i] = timeouts[i];
>> +		}
>> +	}
>> +
>> +	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);
>> +
>> +	/* FIXME: what about A_WAIT_ENUM? */
> 
> Either you init it as other timers, or you remove all of it, otherwise
> there will be NULL pointer crash.

I want to initialize it but was not sure about the timeout value.
What timeout value I must use?

> 
>> +}
>> +
>> +/**
>> + * OTG FSM ops function to add timer
>> + */
>> +static void usb_otg_add_timer(struct otg_fsm *fsm, enum otg_fsm_timer id)
>> +{
>> +	struct usb_otg *otgd = container_of(fsm, struct usb_otg, 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 usb_otg *otgd = container_of(fsm, struct usb_otg, fsm);
>> +	struct hrtimer *timer = &otgd->timers[id].timer;
>> +
>> +	hrtimer_cancel(timer);
>> +}
>> +
>> +/**
>> + * Helper function to start/stop otg host. For use by otg controller.
>> + */
>> +int usb_otg_start_host(struct otg_fsm *fsm, int on)
>> +{
>> +	struct usb_otg *otgd = container_of(fsm, struct usb_otg, 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) {
>> +		/* 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);
>> +	}
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_otg_start_host);
>> +
>> +/**
>> + * Helper function to start/stop otg gadget. For use by otg controller.
>> + */
>> +int usb_otg_start_gadget(struct otg_fsm *fsm, int on)
>> +{
>> +	struct usb_otg *otgd = container_of(fsm, struct usb_otg, 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)
>> +		otgd->gadget_ops->start(fsm->otg->gadget);
>> +	else
>> +		otgd->gadget_ops->stop(fsm->otg->gadget);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_otg_start_gadget);
>> +
>> +/**
>> + * OTG FSM work function
>> + */
>> +static void usb_otg_work(struct work_struct *work)
>> +{
>> +	struct usb_otg *otgd = container_of(work, struct usb_otg, work);
>> +
> 
> Add runtime pm for it
> 	pm_runtime_get_sync(otgd->dev);
>> +	otg_statemachine(&otgd->fsm);
> 	pm_runtime_put_sync(otgd->dev);
> 

Sorry missed this one. Will add.

> Also as I raised in your previous version, we need successive state machine
> transition, how about:
> 	if (otg_statemachine(&otgd->fsm))
> 		usb_otg_sync_inputs(&otgd->fsm);

OK. will add this too.

>> +}
>> +
>> +/**
>> + * usb_otg_register() - Register the OTG device to OTG core
>> + * @dev: OTG controller device.
>> + * @config: OTG configuration.
>> + *
>> + * Register the OTG controller device with the USB OTG core.
>> + * The associated Host and Gadget controllers will be prevented from
>> + * being started till both are available for use.
>> + *
>> + * For non device tree boots, the OTG controller device must be the
>> + * parent node of the Host and Gadget controllers.
>> + *
>> + * For device tree case, the otg-controller property must be present
>> + * in the Host and Gadget controller node and it must point to the
>> + * same OTG controller node.
>> + *
>> + * Return: struct otg_fsm * if success, NULL if error.
>> + */
>> +struct otg_fsm *usb_otg_register(struct device *dev,
>> +				 struct usb_otg_config *config)
> 
> Why not return usb_otg? Since you create usb_otg which contains all stuff
> for otg.

yes, returning usb_otg makes more sense now.

> 
>> +{
>> +	struct usb_otg *otgd;
>> +	struct otg_wait_data *wait;
>> +	int ret = 0;
>> +
>> +	if (!dev || !config || !config->fsm_ops)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	/* already in list? */
>> +	mutex_lock(&otg_list_mutex);
>> +	if (usb_otg_get_data(dev)) {
>> +		dev_err(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 = dev;
>> +	otgd->caps = &config->otg_caps;
> 
> How about define otgd->caps as a pointer, then don't need copy it.

otgd->caps is a pointer.

> 
>> +	INIT_WORK(&otgd->work, usb_otg_work);
>> +	otgd->wq = create_singlethread_workqueue("usb_otg");
>> +	if (!otgd->wq) {
>> +		dev_err(dev, "otg: %s: can't create workqueue\n",
>> +			__func__);
>> +		ret = -ENOMEM;
>> +		goto err_wq;
>> +	}
>> +
>> +	usb_otg_init_timers(otgd, config->otg_timeouts);
>> +
>> +	/* create copy of original ops */
>> +	otgd->fsm_ops = *config->fsm_ops;
> 
> The same, use a pointer is enough?

We are creating a copy because we are overriding timer ops.

> 
>> +	/* 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;
>> +
>> +	mutex_init(&otgd->fsm.lock);
>> +
>> +	list_add_tail(&otgd->list, &otg_list);
>> +	mutex_unlock(&otg_list_mutex);
>> +
>> +	/* were we in wait list? */
>> +	mutex_lock(&wait_list_mutex);
>> +	wait = usb_otg_get_wait(dev);
>> +	mutex_unlock(&wait_list_mutex);
>> +	if (wait) {
>> +		/* register pending host/gadget and flush from list */
>> +		usb_otg_flush_wait(dev);
>> +	}
>> +
>> +	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
>> + * @dev: OTG controller device.
>> + *
>> + * Unregister OTG controller device from USB OTG core.
>> + * Prevents unregistering till both the associated Host and Gadget controllers
>> + * have unregistered from the OTG core.
>> + *
>> + * Return: 0 on success, error value otherwise.
>> + */
>> +int usb_otg_unregister(struct device *dev)
>> +{
>> +	struct usb_otg *otgd;
>> +
>> +	mutex_lock(&otg_list_mutex);
>> +	otgd = usb_otg_get_data(dev);
>> +	if (!otgd) {
>> +		dev_err(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(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 usb_otg *otgd = container_of(fsm, struct usb_otg, 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;
> 
> So we need register hcd(usb_otg_register_hcd) before start fsm, right?

Right. But you must use the plain old usb_add_hcd().

> 
>> +	}
>> +
>> +	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 usb_otg *otgd = container_of(fsm, struct usb_otg, 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 usb_otg *otgd = container_of(fsm, struct usb_otg, 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 usb_otg *otgd;
>> +
>> +	mutex_lock(&otg_list_mutex);
>> +	otgd = usb_otg_get_data(usb_otg_get_device(hcd_gcd_device));
>> +	mutex_unlock(&otg_list_mutex);
>> +	if (!otgd) {
>> +		dev_dbg(hcd_gcd_device, "otg: %s: invalid host/gadget device\n",
>> +			__func__);
>> +		return -ENODEV;
>> +	}
>> +
>> +	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.
> 
> I am confused on how to use this function.
> - This function should be called before start fsm per usb_otg_start_fsm().

yes.

> - Called by usb_add_hcd(), so we need call usb_add_hcd() before start fsm.

yes.

> - If I want to add hcd when switch to host role, and remove hcd when switch
>   to peripheral, with this design, I cannot use this function?

You add hcd only once during the life of the OTG device. If it is linked to the
OTG controller the OTG fsm manages the start/stop of hcd using the otg_hcd_ops.

"usb/core/hcd.c"
static struct otg_hcd_ops otg_hcd_intf = {
        .add = usb_otg_add_hcd,
        .remove = usb_otg_remove_hcd,
};

Your otg driver must use teh usb_otg_add/remove_hcd to start/stop the controller.
Using usb_remove_hcd() means the hcd resource is no longer available and the
otg fsm will be stopped.

> - How about split it out of usb_add_hcd()?

Adding the HCD and starting/stopping the hcd is split into
usb_add/remove_hcd() and usb_otg_add/remove_hcd() for OTG case.

> 
>> + *
>> + * 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 usb_otg *otgd;
>> +	struct device *hcd_dev = hcd->self.controller;
>> +	struct device *otg_dev = usb_otg_get_device(hcd_dev);
>> +
>> +	if (!otg_dev)
>> +		return -EINVAL;	/* we're definitely not OTG */
>> +
>> +	/* we're otg but otg controller might not yet be registered */
>> +	mutex_lock(&otg_list_mutex);
>> +	otgd = usb_otg_get_data(otg_dev);
>> +	mutex_unlock(&otg_list_mutex);
>> +	if (!otgd) {
>> +		dev_dbg(hcd_dev,
>> +			"otg: controller not yet registered. waiting..\n");
>> +		/*
>> +		 * otg controller might register later. Put the hcd in
>> +		 * wait list and call us back when ready
>> +		 */
>> +		if (usb_otg_hcd_wait_add(otg_dev, hcd, irqnum, irqflags, ops)) {
>> +			dev_dbg(hcd_dev, "otg: failed to add to wait list\n");
>> +			return -EINVAL;
>> +		}
>> +
>> +		return 0;
>> +	}
>> +
>> +	/* HCD will be started by OTG fsm when needed */
>> +	mutex_lock(&otgd->fsm.lock);
> 
> If call usb_add_hcd() when start host role, deadlock.

No. You must call usb_otg_add_hcd() to start host role.

> 
>> +	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);
> 
> otgd->host = hcd_to_bus(hcd);

ok. So we set host at both places. struct usb_otg in struct otg_fsm starts to
feel redundant now. I think we should get rid of it and get the usb_otg struct
using container_of() instead.

> 
>> +		/* 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;
> 
> Return non-zero, then if err, do we need call usb_otg_add_hcd() after
> usb_otg_register_hcd() fails?

You should not call usb_otg_register_hcd() but usb_add_hcd().
If that fails then you fail as ususal.

> 
>> +}
>> +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 usb_otg *otgd;
>> +	struct device *hcd_dev = hcd_to_bus(hcd)->controller;
>> +	struct device *otg_dev = usb_otg_get_device(hcd_dev);
>> +
>> +	if (!otg_dev)
>> +		return -EINVAL;	/* we're definitely not OTG */
>> +
>> +	mutex_lock(&otg_list_mutex);
>> +	otgd = usb_otg_get_data(otg_dev);
>> +	mutex_unlock(&otg_list_mutex);
>> +	if (!otgd) {
>> +		/* are we in wait list? */
>> +		if (!usb_otg_hcd_wait_remove(hcd))
>> +			return 0;
>> +
>> +		dev_dbg(hcd_dev, "otg: host wasn't registered with otg\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	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(hcd_dev));
>> +	} else if (hcd == otgd->shared_hcd.hcd) {
>> +		otgd->shared_hcd.hcd = NULL;
>> +		dev_info(otg_dev, "otg: shared host %s unregistered\n",
>> +			 dev_name(hcd_dev));
>> +	} else {
>> +		dev_err(otg_dev, "otg: host %s wasn't registered with otg\n",
>> +			dev_name(hcd_dev));
>> +		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 usb_otg *otgd;
>> +	struct device *gadget_dev = &gadget->dev;
>> +	struct device *otg_dev = usb_otg_get_device(gadget_dev);
>> +
>> +	if (!otg_dev)
>> +		return -EINVAL;	/* we're definitely not OTG */
>> +
>> +	/* we're otg but otg controller might not yet be registered */
>> +	mutex_lock(&otg_list_mutex);
>> +	otgd = usb_otg_get_data(otg_dev);
>> +	mutex_unlock(&otg_list_mutex);
>> +	if (!otgd) {
>> +		dev_dbg(gadget_dev,
>> +			"otg: controller not yet registered. waiting..\n");
>> +		/*
>> +		 * otg controller might register later. Put the gadget in
>> +		 * wait list and call us back when ready
>> +		 */
>> +		if (usb_otg_gadget_wait_add(otg_dev, gadget, ops)) {
>> +			dev_dbg(gadget_dev, "otg: failed to add to wait list\n");
>> +			return -EINVAL;
>> +		}
>> +
>> +		return 0;
>> +	}
>> +
>> +	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 usb_otg *otgd;
>> +	struct device *gadget_dev = &gadget->dev;
>> +	struct device *otg_dev = usb_otg_get_device(gadget_dev);
>> +
>> +	if (!otg_dev)
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&otg_list_mutex);
>> +	otgd = usb_otg_get_data(otg_dev);
>> +	mutex_unlock(&otg_list_mutex);
>> +	if (!otgd) {
>> +		/* are we in wait list? */
>> +		if (!usb_otg_gadget_wait_remove(gadget))
>> +			return 0;
>> +
>> +		dev_dbg(gadget_dev, "otg: gadget wasn't registered with otg\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	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 usb_otg *otgd = container_of(fsm, struct usb_otg, 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 bd1dcf8..38cabe0 100644
>> --- a/include/linux/usb/otg.h
>> +++ b/include/linux/usb/otg.h
>> @@ -10,19 +10,100 @@
>>  #define __LINUX_USB_OTG_H
>>  
>>  #include <linux/phy/phy.h>
>> +#include <linux/device.h>
>> +#include <linux/hrtimer.h>
>> +#include <linux/ktime.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 otg_hcd - host controller state and interface
>> + *
>> + * @hcd: host controller
>> + * @irqnum: irq number
>> + * @irqflags: irq flags
>> + * @ops: otg to host controller interface
>> + */
>> +struct otg_hcd {
>> +	struct usb_hcd *hcd;
>> +	unsigned int irqnum;
>> +	unsigned long irqflags;
>> +	struct otg_hcd_ops *ops;
>> +};
>> +
>> +struct usb_otg;
>> +
>> +/**
>> + * struct otg_timer - otg timer data
>> + *
>> + * @timer: high resolution timer
>> + * @timeout: timeout value
>> + * @timetout_bit: pointer to variable that is set on timeout
>> + * @otgd: usb otg data
>> + */
>> +struct otg_timer {
>> +	struct hrtimer timer;
>> +	ktime_t timeout;
>> +	/* callback data */
>> +	int *timeout_bit;
>> +	struct usb_otg *otgd;
>> +};
>> +
>> +/**
>> + * struct usb_otg - usb otg controller state
>> + *
>> + * @default_a: Indicates we are an A device. i.e. Host.
>> + * @phy: USB phy interface
>> + * @usb_phy: old usb_phy interface
>> + * @host: host controller bus
>> + * @gadget: gadget device
>> + * @state: current otg state
>> + * @dev: otg controller device
>> + * @caps: otg capabilities revision, hnp, srp, etc
>> + * @fsm: otg finite state machine
>> + * @fsm_ops: controller hooks for the state machine
>> + * ------- internal use only -------
>> + * @primary_hcd: primary host state and interface
>> + * @shared_hcd: shared host state and interface
>> + * @gadget_ops: gadget interface
>> + * @timers: otg timers for state machine
>> + * @list: list of otg controllers
>> + * @work: otg state machine work
>> + * @wq: otg state machine work queue
>> + * @fsm_running: state machine running/stopped indicator
>> + */
>>  struct usb_otg {
>>  	u8			default_a;
>>  
>>  	struct phy		*phy;
>>  	/* old usb_phy interface */
>>  	struct usb_phy		*usb_phy;
>> +
> 
> add a blank line?
> 
>>  	struct usb_bus		*host;
>>  	struct usb_gadget	*gadget;
>>  
>>  	enum usb_otg_state	state;
>>  
>> +	struct device *dev;
>> +	struct usb_otg_caps *caps;
>> +	struct otg_fsm fsm;
>> +	struct otg_fsm_ops fsm_ops;
>> +
>> +	/* internal use only */
>> +	struct otg_hcd primary_hcd;
>> +	struct otg_hcd shared_hcd;
>> +	struct otg_gadget_ops *gadget_ops;
>> +	struct otg_timer timers[NUM_OTG_FSM_TIMERS];
>> +	struct list_head list;
>> +	struct work_struct work;
>> +	struct workqueue_struct *wq;
>> +	bool fsm_running;
> 
> Should fsm_running be added in otg_fsm struct?

Yes, can be moved there.

> 
>> +	/* use otg->fsm.lock for serializing access */
>> +
>> +/*------------- deprecated interface -----------------------------*/
>>  	/* bind/unbind the host controller */
>>  	int	(*set_host)(struct usb_otg *otg, struct usb_bus *host);
>>  
>> @@ -38,7 +119,7 @@ struct usb_otg {
>>  
>>  	/* start or continue HNP role switch */
>>  	int	(*start_hnp)(struct usb_otg *otg);
>> -
>> +/*---------------------------------------------------------------*/
>>  };
>>  
>>  /**
>> @@ -56,8 +137,105 @@ struct usb_otg_caps {
>>  	bool adp_support;
>>  };
>>  
>> +/**
>> + * struct usb_otg_config - otg controller configuration
>> + * @caps: otg capabilities of the controller
>> + * @ops: otg fsm operations
>> + * @otg_timeouts: override default otg fsm timeouts
>> + */
>> +struct usb_otg_config {
>> +	struct usb_otg_caps otg_caps;
>> +	struct otg_fsm_ops *fsm_ops;
>> +	unsigned otg_timeouts[NUM_OTG_FSM_TIMERS];
>> +};
>> +
>>  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 *dev,
>> +				 struct usb_otg_config *config);
>> +int usb_otg_unregister(struct device *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);
>> +int usb_otg_start_host(struct otg_fsm *fsm, int on);
>> +int usb_otg_start_gadget(struct otg_fsm *fsm, int on);
>> +
>> +#else /* CONFIG_USB_OTG */
>> +
>> +static inline struct otg_fsm *usb_otg_register(struct device *dev,
>> +					       struct usb_otg_config *config)
>> +{
>> +	return ERR_PTR(-ENOTSUPP);
>> +}
>> +
>> +static inline int usb_otg_unregister(struct device *dev)
>> +{
>> +	return -ENOTSUPP;
>> +}
>> +
>> +static inline int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
>> +				       unsigned long irqflags,
>> +				       struct otg_hcd_ops *ops)
>> +{
>> +	return -ENOTSUPP;
>> +}
>> +
>> +static inline int usb_otg_unregister_hcd(struct usb_hcd *hcd)
>> +{
>> +	return -ENOTSUPP;
>> +}
>> +
>> +static inline int usb_otg_register_gadget(struct usb_gadget *gadget,
>> +					  struct otg_gadget_ops *ops)
>> +{
>> +	return -ENOTSUPP;
>> +}
>> +
>> +static inline int usb_otg_unregister_gadget(struct usb_gadget *gadget)
>> +{
>> +	return -ENOTSUPP;
>> +}
>> +
>> +static inline void usb_otg_sync_inputs(struct otg_fsm *fsm)
>> +{
>> +}
>> +
>> +static inline int usb_otg_kick_fsm(struct device *hcd_gcd_device)
>> +{
>> +	return -ENOTSUPP;
>> +}
>> +
>> +static inline struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm)
>> +{
>> +	return NULL;
>> +}
>> +
>> +static inline int usb_otg_start_host(struct otg_fsm *fsm, int on)
>> +{
>> +	return -ENOTSUPP;
>> +}
>> +
>> +static inline int usb_otg_start_gadget(struct otg_fsm *fsm, int on)
>> +{
>> +	return -ENOTSUPP;
>> +}
>> +#endif /* CONFIG_USB_OTG */
>> +
>> +/*------------- deprecated interface -----------------------------*/
>>  /* Context: can sleep */
>>  static inline int
>>  otg_start_hnp(struct usb_otg *otg)
>> @@ -109,14 +287,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
>>

  reply	other threads:[~2015-09-07 10:53 UTC|newest]

Thread overview: 102+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-24 13:21 [PATCH v4 00/13] USB: OTG/DRD Core functionality Roger Quadros
2015-08-24 13:21 ` Roger Quadros
2015-08-24 13:21 ` [PATCH v4 01/13] usb: otg-fsm: Add documentation for struct otg_fsm Roger Quadros
2015-08-24 13:21   ` Roger Quadros
2015-08-24 13:21 ` [PATCH v4 02/13] usb: otg-fsm: support multiple instances Roger Quadros
2015-08-24 13:21   ` Roger Quadros
2015-09-06  5:52   ` Peter Chen
2015-09-06  5:52     ` Peter Chen
2015-08-24 13:21 ` [PATCH v4 03/13] usb: otg-fsm: Prevent build warning "VDBG" redefined Roger Quadros
2015-08-24 13:21   ` Roger Quadros
2015-08-24 13:21 ` [PATCH v4 04/13] otg-fsm: move usb_bus_start_enum into otg-fsm->ops Roger Quadros
2015-08-24 13:21   ` Roger Quadros
2015-09-07  1:24   ` Peter Chen
2015-09-07  1:24     ` Peter Chen
2015-09-07  9:57     ` Roger Quadros
2015-09-07  9:57       ` Roger Quadros
2015-09-08  6:54       ` Peter Chen
2015-09-08  6:54         ` Peter Chen
2015-09-08  8:24         ` Roger Quadros
2015-09-08  8:24           ` Roger Quadros
2015-08-24 13:21 ` [PATCH v4 05/13] usb: hcd.h: Add OTG to HCD interface Roger Quadros
2015-08-24 13:21   ` Roger Quadros
2015-08-24 13:21 ` [PATCH v4 06/13] usb: gadget.h: Add OTG to gadget interface Roger Quadros
2015-08-24 13:21   ` Roger Quadros
2015-08-24 13:21 ` [PATCH v4 07/13] usb: otg: add OTG core Roger Quadros
2015-08-24 13:21   ` Roger Quadros
2015-09-07  1:23   ` Peter Chen
2015-09-07  1:23     ` Peter Chen
2015-09-07 10:23     ` Roger Quadros
2015-09-07 10:23       ` Roger Quadros
2015-09-08  8:31       ` Peter Chen
2015-09-08  8:31         ` Peter Chen
2015-09-08 12:25         ` Roger Quadros
2015-09-08 12:25           ` Roger Quadros
2015-09-08 14:34           ` Alan Stern
2015-09-08 14:34             ` Alan Stern
2015-09-08 17:29             ` Roger Quadros
2015-09-08 17:29               ` Roger Quadros
2015-09-09  2:21           ` Peter Chen
2015-09-09  2:21             ` Peter Chen
2015-09-09  9:08             ` Roger Quadros
2015-09-09  9:08               ` Roger Quadros
2015-09-09  8:13               ` Peter Chen
2015-09-09  8:13                 ` Peter Chen
2015-09-09  9:33                 ` Roger Quadros
2015-09-09  9:33                   ` Roger Quadros
2015-09-09  8:45                   ` Peter Chen
2015-09-09  8:45                     ` Peter Chen
2015-09-09 10:21                     ` Roger Quadros
2015-09-09 10:21                       ` Roger Quadros
2015-09-10  5:35                       ` Peter Chen
2015-09-10  5:35                         ` Peter Chen
2015-09-10 14:17                         ` Roger Quadros
2015-09-10 14:17                           ` Roger Quadros
2015-09-11  1:50                           ` Peter Chen
2015-09-11  1:50                             ` Peter Chen
2015-09-07  7:40   ` Li Jun
2015-09-07  7:40     ` Li Jun
2015-09-07 10:53     ` Roger Quadros [this message]
2015-09-07 10:53       ` Roger Quadros
2015-09-09  6:20       ` Li Jun
2015-09-09  6:20         ` Li Jun
2015-09-09 10:01         ` Roger Quadros
2015-09-09 10:01           ` Roger Quadros
2015-09-10  9:28           ` Li Jun
2015-09-10  9:28             ` Li Jun
2015-09-10 14:14             ` Roger Quadros
2015-09-10 14:14               ` Roger Quadros
2015-08-24 13:21 ` [PATCH v4 08/13] usb: doc: dt-binding: Add otg-controller property Roger Quadros
2015-08-24 13:21   ` Roger Quadros
2015-08-24 13:21 ` [PATCH v4 09/13] usb: chipidea: move from CONFIG_USB_OTG_FSM to CONFIG_USB_OTG Roger Quadros
2015-08-24 13:21   ` Roger Quadros
2015-08-24 13:21 ` [PATCH v4 10/13] usb: hcd: Adapt to OTG core Roger Quadros
2015-08-24 13:21   ` Roger Quadros
2015-09-09  2:23   ` Peter Chen
2015-09-09  2:23     ` Peter Chen
2015-09-09  9:39     ` Roger Quadros
2015-09-09  9:39       ` Roger Quadros
2015-08-24 13:21 ` [PATCH v4 11/13] usb: core: hub: Notify OTG fsm when A device sets b_hnp_enable Roger Quadros
2015-08-24 13:21   ` Roger Quadros
2015-08-24 13:21 ` [PATCH v4 12/13] usb: gadget: udc: adapt to OTG core Roger Quadros
2015-08-24 13:21   ` Roger Quadros
2015-08-24 13:21 ` [PATCH v4 13/13] usb: otg: Add dual-role device (DRD) support Roger Quadros
2015-08-24 13:21   ` Roger Quadros
2015-09-07  7:53   ` Li Jun
2015-09-07  7:53     ` Li Jun
2015-09-07  9:51     ` Roger Quadros
2015-09-07  9:51       ` Roger Quadros
2015-08-26  6:19 ` [PATCH v4 00/13] USB: OTG/DRD Core functionality Peter Chen
2015-08-26  6:19   ` Peter Chen
2015-09-06  7:06 ` Peter Chen
2015-09-06  7:06   ` Peter Chen
2015-09-07 11:42   ` Roger Quadros
2015-09-07 11:42     ` Roger Quadros
2015-12-03  8:19 ` Peter Chen
2015-12-03  8:19   ` Peter Chen
2015-12-03  8:54   ` Roger Quadros
2015-12-03  8:54     ` Roger Quadros
     [not found] <Pine.LNX.4.44L0.1509081334190.1298-100000@iolanthe.rowland.org>
2015-09-09 11:10 ` [PATCH v4 07/13] usb: otg: add OTG core Roger Quadros
2015-09-09 11:10   ` Roger Quadros
2015-09-09 15:26   ` Alan Stern
2015-09-09 15:26     ` Alan Stern

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55ED6C9F.2000502@ti.com \
    --to=rogerq@ti.com \
    --cc=Joao.Pinto@synopsys.com \
    --cc=abrestic@chromium.org \
    --cc=b47624@freescale.com \
    --cc=balbi@ti.com \
    --cc=dan.j.williams@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jun.li@freescale.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@linux.intel.com \
    --cc=peter.chen@freescale.com \
    --cc=stern@rowland.harvard.edu \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.