From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751731AbbCTHrB (ORCPT ); Fri, 20 Mar 2015 03:47:01 -0400 Received: from mail-bl2on0109.outbound.protection.outlook.com ([65.55.169.109]:45994 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751343AbbCTHq7 (ORCPT ); Fri, 20 Mar 2015 03:46:59 -0400 X-Greylist: delayed 101756 seconds by postgrey-1.27 at vger.kernel.org; Fri, 20 Mar 2015 03:46:58 EDT Date: Fri, 20 Mar 2015 15:45:51 +0800 From: Peter Chen To: Roger Quadros CC: , , , , , , , , Subject: Re: [RFC][PATCH 3/9] usb: otg: add OTG core Message-ID: <20150320074550.GG7537@shlinux2> References: <1426686963-11613-1-git-send-email-rogerq@ti.com> <1426686963-11613-4-git-send-email-rogerq@ti.com> <20150319034034.GA7779@shlinux2> <550AA28F.6040201@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <550AA28F.6040201@ti.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-EOPAttributedMessage: 0 Authentication-Results: spf=fail (sender IP is 192.88.158.2) smtp.mailfrom=peter.chen@freescale.com; vger.kernel.org; dkim=none (message not signed) header.d=none; X-Forefront-Antispam-Report: CIP:192.88.158.2;CTRY:US;IPV:NLI;EFV:NLI;BMV:1;SFV:NSPM;SFS:(10019020)(6009001)(339900001)(51704005)(189002)(199003)(479174004)(86362001)(16601075003)(97756001)(50986999)(77096005)(2950100001)(54356999)(46406003)(104016003)(15975445007)(47776003)(62966003)(77156002)(106466001)(110136001)(105606002)(83506001)(33656002)(23726002)(85426001)(87936001)(19580395003)(92566002)(93886004)(50466002)(33716001)(19580405001)(46102003)(6806004)(2004002);DIR:OUT;SFP:1102;SCL:1;SRVR:BN3PR0301MB1218;H:az84smr01.freescale.net;FPR:;SPF:Fail;MLV:sfv;MX:1;A:1;LANG:en; X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BN3PR0301MB1218; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(5002010)(5005006);SRVR:BN3PR0301MB1218;BCL:0;PCL:0;RULEID:;SRVR:BN3PR0301MB1218; X-Forefront-PRVS: 05214FD68E X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Mar 2015 07:46:53.3074 (UTC) X-MS-Exchange-CrossTenant-Id: 710a03f5-10f6-4d38-9ff4-a80b81da590d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=710a03f5-10f6-4d38-9ff4-a80b81da590d;Ip=[192.88.158.2];Helo=[az84smr01.freescale.net] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN3PR0301MB1218 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Mar 19, 2015 at 12:18:55PM +0200, Roger Quadros wrote: > On 19/03/15 05:40, Peter Chen wrote: > > On Wed, Mar 18, 2015 at 03:55:57PM +0200, 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 > >> > >> TODO: > >> - sysfs interface to allow application inputs to OTG state machine > >> - otg class? > >> > >> Signed-off-by: Roger Quadros > >> --- > >> drivers/usb/Makefile | 1 + > >> drivers/usb/common/Makefile | 1 + > >> drivers/usb/common/usb-otg.c | 732 +++++++++++++++++++++++++++++++++++++++++++ > >> drivers/usb/common/usb-otg.h | 71 +++++ > >> drivers/usb/core/Kconfig | 8 + > >> include/linux/usb/usb-otg.h | 86 +++++ > >> 6 files changed, 899 insertions(+) > >> create mode 100644 drivers/usb/common/usb-otg.c > >> create mode 100644 drivers/usb/common/usb-otg.h > >> create mode 100644 include/linux/usb/usb-otg.h > >> > >> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile > >> index 2f1e2aa..07f59a5 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_CORE) += common/ > >> > >> obj-$(CONFIG_USBIP_CORE) += usbip/ > >> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile > >> index ca2f8bd..573fc75 100644 > >> --- a/drivers/usb/common/Makefile > >> +++ b/drivers/usb/common/Makefile > >> @@ -7,3 +7,4 @@ 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_OTG_CORE) += usb-otg.o > >> diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c > >> new file mode 100644 > >> index 0000000..1433fc9 > >> --- /dev/null > >> +++ b/drivers/usb/common/usb-otg.c > >> @@ -0,0 +1,732 @@ > >> +/** > >> + * drivers/usb/common/usb-otg.c - USB OTG core > >> + * > >> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com > >> + * Author: Roger Quadros > >> + * > >> + * 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 > >> +#include > >> +#include > >> +#include > >> +#include /* enum usb_otg_state */ > >> +#include > >> +#include > >> +#include > >> + > >> +#include "usb-otg.h" > >> + > >> +/* to link timer with callback data */ > >> +struct otg_timer { > >> + struct timer_list timer; > >> + /* callback data */ > >> + int *timeout_bit; > >> + struct otg_data *otgd; > >> +}; > >> + > >> +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; > >> + struct usb_hcd *shared_hcd; /* if shared HCD registered */ > >> + > >> + /* 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; > >> + bool gadget_can_start; /* OTG FSM says gadget can start */ > >> + bool host_can_start; /* OTG FSM says host can start */ > >> + > >> + /* use otg->fsm.lock for serializing access */ > >> +}; > > > > What's the relationship between struct usb_otg otg and this one? > > Did you mean why struct usb_otg otg is there in struct otg_data? > Just for easy allocation. fsm_ops only contains the pointer to struct usb_otg. > The reason why I ask this question is the most structures at usb_otg (only enum usb_otg_state)are useless for otg_fsm, this structure may only for hardware otg fsm driver, so your OTG framework should only for software FSM drivers, right? Peter > > > >> + > >> +/* OTG device list */ > >> +LIST_HEAD(otg_list); > >> +static DEFINE_MUTEX(otg_list_mutex); > >> + > >> +/** > >> + * 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 void set_tmout(unsigned long data) > >> +{ > >> + struct otg_timer *tmr_data; > >> + > >> + tmr_data = (struct otg_timer *)data; > >> + > >> + if (tmr_data->timeout_bit) > >> + *tmr_data->timeout_bit = 1; > >> + > >> + usb_otg_sync_inputs(&tmr_data->otgd->fsm); > >> +} > >> + > >> +/** > >> + * Initialize one OTG timer with callback, timeout and timeout bit > >> + */ > >> +static void otg_timer_init(enum otg_fsm_timer id, struct otg_data *otgd, > >> + void (*callback)(unsigned long), > >> + unsigned long expires_ms, > >> + int *timeout_bit) > >> +{ > >> + struct otg_timer *otgtimer = &otgd->timers[id]; > >> + struct timer_list *timer = &otgtimer->timer; > >> + > >> + init_timer(timer); > >> + timer->function = callback; > >> + timer->expires = jiffies + msecs_to_jiffies(expires_ms); > >> + timer->data = (unsigned long)otgtimer; > >> + > > > > The timer for TB_DATA_PLS is about 10ms or less, it is not suitable > > for using kernel timer, hrtimer is suitable choice. > > good catch. I will switch to hrtimer then. > > > > >> + 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); > >> +} > > > > cheers, > -roger > -- Best Regards, Peter Chen From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Chen Subject: Re: [RFC][PATCH 3/9] usb: otg: add OTG core Date: Fri, 20 Mar 2015 15:45:51 +0800 Message-ID: <20150320074550.GG7537@shlinux2> References: <1426686963-11613-1-git-send-email-rogerq@ti.com> <1426686963-11613-4-git-send-email-rogerq@ti.com> <20150319034034.GA7779@shlinux2> <550AA28F.6040201@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Return-path: Content-Disposition: inline In-Reply-To: <550AA28F.6040201@ti.com> Sender: linux-kernel-owner@vger.kernel.org To: Roger Quadros Cc: gregkh@linuxfoundation.org, balbi@ti.com, stern@rowland.harvard.edu, dan.j.williams@intel.com, jun.li@freescale.com, mathias.nyman@linux.intel.com, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org List-Id: linux-omap@vger.kernel.org On Thu, Mar 19, 2015 at 12:18:55PM +0200, Roger Quadros wrote: > On 19/03/15 05:40, Peter Chen wrote: > > On Wed, Mar 18, 2015 at 03:55:57PM +0200, 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 > >> > >> TODO: > >> - sysfs interface to allow application inputs to OTG state machine > >> - otg class? > >> > >> Signed-off-by: Roger Quadros > >> --- > >> drivers/usb/Makefile | 1 + > >> drivers/usb/common/Makefile | 1 + > >> drivers/usb/common/usb-otg.c | 732 +++++++++++++++++++++++++++++++++++++++++++ > >> drivers/usb/common/usb-otg.h | 71 +++++ > >> drivers/usb/core/Kconfig | 8 + > >> include/linux/usb/usb-otg.h | 86 +++++ > >> 6 files changed, 899 insertions(+) > >> create mode 100644 drivers/usb/common/usb-otg.c > >> create mode 100644 drivers/usb/common/usb-otg.h > >> create mode 100644 include/linux/usb/usb-otg.h > >> > >> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile > >> index 2f1e2aa..07f59a5 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_CORE) += common/ > >> > >> obj-$(CONFIG_USBIP_CORE) += usbip/ > >> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile > >> index ca2f8bd..573fc75 100644 > >> --- a/drivers/usb/common/Makefile > >> +++ b/drivers/usb/common/Makefile > >> @@ -7,3 +7,4 @@ 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_OTG_CORE) += usb-otg.o > >> diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c > >> new file mode 100644 > >> index 0000000..1433fc9 > >> --- /dev/null > >> +++ b/drivers/usb/common/usb-otg.c > >> @@ -0,0 +1,732 @@ > >> +/** > >> + * drivers/usb/common/usb-otg.c - USB OTG core > >> + * > >> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com > >> + * Author: Roger Quadros > >> + * > >> + * 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 > >> +#include > >> +#include > >> +#include > >> +#include /* enum usb_otg_state */ > >> +#include > >> +#include > >> +#include > >> + > >> +#include "usb-otg.h" > >> + > >> +/* to link timer with callback data */ > >> +struct otg_timer { > >> + struct timer_list timer; > >> + /* callback data */ > >> + int *timeout_bit; > >> + struct otg_data *otgd; > >> +}; > >> + > >> +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; > >> + struct usb_hcd *shared_hcd; /* if shared HCD registered */ > >> + > >> + /* 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; > >> + bool gadget_can_start; /* OTG FSM says gadget can start */ > >> + bool host_can_start; /* OTG FSM says host can start */ > >> + > >> + /* use otg->fsm.lock for serializing access */ > >> +}; > > > > What's the relationship between struct usb_otg otg and this one? > > Did you mean why struct usb_otg otg is there in struct otg_data? > Just for easy allocation. fsm_ops only contains the pointer to struct usb_otg. > The reason why I ask this question is the most structures at usb_otg (only enum usb_otg_state)are useless for otg_fsm, this structure may only for hardware otg fsm driver, so your OTG framework should only for software FSM drivers, right? Peter > > > >> + > >> +/* OTG device list */ > >> +LIST_HEAD(otg_list); > >> +static DEFINE_MUTEX(otg_list_mutex); > >> + > >> +/** > >> + * 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 void set_tmout(unsigned long data) > >> +{ > >> + struct otg_timer *tmr_data; > >> + > >> + tmr_data = (struct otg_timer *)data; > >> + > >> + if (tmr_data->timeout_bit) > >> + *tmr_data->timeout_bit = 1; > >> + > >> + usb_otg_sync_inputs(&tmr_data->otgd->fsm); > >> +} > >> + > >> +/** > >> + * Initialize one OTG timer with callback, timeout and timeout bit > >> + */ > >> +static void otg_timer_init(enum otg_fsm_timer id, struct otg_data *otgd, > >> + void (*callback)(unsigned long), > >> + unsigned long expires_ms, > >> + int *timeout_bit) > >> +{ > >> + struct otg_timer *otgtimer = &otgd->timers[id]; > >> + struct timer_list *timer = &otgtimer->timer; > >> + > >> + init_timer(timer); > >> + timer->function = callback; > >> + timer->expires = jiffies + msecs_to_jiffies(expires_ms); > >> + timer->data = (unsigned long)otgtimer; > >> + > > > > The timer for TB_DATA_PLS is about 10ms or less, it is not suitable > > for using kernel timer, hrtimer is suitable choice. > > good catch. I will switch to hrtimer then. > > > > >> + 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); > >> +} > > > > cheers, > -roger > -- Best Regards, Peter Chen