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@xxxxxx> >> --- >> 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@xxxxxxxxxxxxx> >> +M: Roger Quadros <rogerq@xxxxxx> >> T: git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git >> L: linux-usb@xxxxxxxxxxxxxxx >> 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@xxxxxxxxx> >> 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@xxxxxx> >> + * >> + * 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@xxxxxx> >> + * >> + * 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-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
