Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757271AbbDPMO7 (ORCPT ); Thu, 16 Apr 2015 08:14:59 -0400 Received: from mail-bn1bon0115.outbound.protection.outlook.com ([157.56.111.115]:45763 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754513AbbDPMOt (ORCPT ); Thu, 16 Apr 2015 08:14:49 -0400 Authentication-Results: spf=temperror (sender IP is 192.88.168.50) smtp.mailfrom=freescale.com; vger.kernel.org; dkim=none (message not signed) header.d=none; Date: Thu, 16 Apr 2015 20:12:12 +0800 From: Peter Chen To: Roger Quadros CC: , , , , , , , , , Subject: Re: [RFC][PATCH v2 04/13] usb: gadget: add usb_gadget_start/stop() Message-ID: <20150416121211.GH3181@shlinux2> References: <1429008120-5395-1-git-send-email-rogerq@ti.com> <1429008120-5395-5-git-send-email-rogerq@ti.com> <20150416114839.GD3181@shlinux2> <552FA60D.5030707@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <552FA60D.5030707@ti.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-EOPAttributedMessage: 0 X-Forefront-Antispam-Report: CIP:192.88.168.50;CTRY:US;IPV:NLI;EFV:NLI;BMV:1;SFV:NSPM;SFS:(10019020)(6009001)(199003)(189002)(479174004)(51704005)(24454002)(86362001)(87936001)(76176999)(104016003)(77096005)(77156002)(19580405001)(19580395003)(62966003)(97756001)(46406003)(47776003)(33716001)(54356999)(106466001)(50986999)(110136001)(2950100001)(50466002)(23726002)(46102003)(4001350100001)(33656002)(83506001)(6806004)(92566002);DIR:OUT;SFP:1102;SCL:1;SRVR:DM2PR0301MB1231;H:tx30smr01.am.freescale.net;FPR:;SPF:TempError;MLV:sfv;A:1;MX:1;LANG:en; X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DM2PR0301MB1231; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(5005006)(5002010);SRVR:DM2PR0301MB1231;BCL:0;PCL:0;RULEID:;SRVR:DM2PR0301MB1231; X-Forefront-PRVS: 0548586081 X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Apr 2015 12:14:43.7482 (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.168.50];Helo=[tx30smr01.am.freescale.net] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM2PR0301MB1231 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5126 Lines: 158 On Thu, Apr 16, 2015 at 03:07:41PM +0300, Roger Quadros wrote: > On 16/04/15 14:48, Peter Chen wrote: > > On Tue, Apr 14, 2015 at 01:41:51PM +0300, Roger Quadros wrote: > >> The OTG state machine needs a mechanism to start and > >> stop the gadget controller. Add usb_gadget_start() > >> and usb_gadget_stop(). > >> > >> Signed-off-by: Roger Quadros > >> --- > >> drivers/usb/gadget/udc/udc-core.c | 74 +++++++++++++++++++++++++++++++++++++++ > >> include/linux/usb/gadget.h | 3 ++ > >> 2 files changed, 77 insertions(+) > >> > >> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c > >> index 5a81cb0..3aa5dd5 100644 > >> --- a/drivers/usb/gadget/udc/udc-core.c > >> +++ b/drivers/usb/gadget/udc/udc-core.c > >> @@ -187,6 +187,7 @@ EXPORT_SYMBOL_GPL(usb_gadget_udc_reset); > >> */ > >> static inline int usb_gadget_udc_start(struct usb_udc *udc) > >> { > >> + dev_dbg(&udc->dev, "%s\n", __func__); > >> return udc->gadget->ops->udc_start(udc->gadget, udc->driver); > >> } > >> > >> @@ -204,10 +205,83 @@ static inline int usb_gadget_udc_start(struct usb_udc *udc) > >> */ > >> static inline void usb_gadget_udc_stop(struct usb_udc *udc) > >> { > >> + dev_dbg(&udc->dev, "%s\n", __func__); > >> udc->gadget->ops->udc_stop(udc->gadget); > >> } > >> > >> /** > >> + * usb_gadget_start - start the usb gadget controller and connect to bus > >> + * @gadget: the gadget device to start > >> + * > >> + * This is external API for use by OTG core. > >> + * > >> + * Start the usb device controller and connect to bus (enable pull). > >> + */ > >> +int usb_gadget_start(struct usb_gadget *gadget) > >> +{ > >> + int ret; > >> + struct usb_udc *udc = NULL; > >> + > >> + dev_dbg(&gadget->dev, "%s\n", __func__); > >> + mutex_lock(&udc_lock); > >> + list_for_each_entry(udc, &udc_list, list) > >> + if (udc->gadget == gadget) > >> + goto found; > >> + > >> + dev_err(gadget->dev.parent, "%s: gadget not registered.\n", > >> + __func__); > >> + mutex_unlock(&udc_lock); > >> + return -EINVAL; > >> + > >> +found: > >> + ret = usb_gadget_udc_start(udc); > >> + if (ret) > >> + dev_err(&udc->dev, "USB Device Controller didn't start: %d\n", > >> + ret); > >> + else > >> + usb_gadget_connect(udc->gadget); > > > > OTG FSM manages its pullup/pulldown itself through its fsm->ops->loc_conn > > That API usb_gadget_udc_start() is used by DRD (dual-role) FSM as well > and it doesn't use ops->loc_conn. So i'm wondering how to make this work for > both. > > I could probably remove the pull up control from usb_gadget_start/stop() > and move it into the DRD FSM. > Just like I comment your at your 5th patch, if we want both DRD and OTG FSM devices run the same code. From our experience, it can cover both at runtime if we can handle otg descriptor well (no otg descriptor for DRD device). Peter > > > > >> + > >> + mutex_unlock(&udc_lock); > >> + > >> + return ret; > >> +} > >> +EXPORT_SYMBOL_GPL(usb_gadget_start); > >> + > >> +/** > >> + * usb_gadget_stop - disconnect from bus and stop the usb gadget > >> + * @gadget: The gadget device we want to stop > >> + * > >> + * This is external API for use by OTG core. > >> + * > >> + * Disconnect from the bus (disable pull) and stop the > >> + * gadget controller. > >> + */ > >> +int usb_gadget_stop(struct usb_gadget *gadget) > >> +{ > >> + struct usb_udc *udc = NULL; > >> + > >> + dev_dbg(&gadget->dev, "%s\n", __func__); > >> + mutex_lock(&udc_lock); > >> + list_for_each_entry(udc, &udc_list, list) > >> + if (udc->gadget == gadget) > >> + goto found; > >> + > >> + dev_err(gadget->dev.parent, "%s: gadget not registered.\n", > >> + __func__); > >> + mutex_unlock(&udc_lock); > >> + return -EINVAL; > >> + > >> +found: > >> + usb_gadget_disconnect(udc->gadget); > >> + udc->driver->disconnect(udc->gadget); > >> + usb_gadget_udc_stop(udc); > >> + mutex_unlock(&udc_lock); > >> + > >> + return 0; > >> +} > >> +EXPORT_SYMBOL_GPL(usb_gadget_stop); > >> + > >> +/** > >> * usb_udc_release - release the usb_udc struct > >> * @dev: the dev member within usb_udc > >> * > >> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h > >> index e2f00fd..7ada7e6 100644 > >> --- a/include/linux/usb/gadget.h > >> +++ b/include/linux/usb/gadget.h > >> @@ -922,6 +922,9 @@ int usb_gadget_probe_driver(struct usb_gadget_driver *driver); > >> */ > >> int usb_gadget_unregister_driver(struct usb_gadget_driver *driver); > >> > >> +int usb_gadget_start(struct usb_gadget *gadget); > >> +int usb_gadget_stop(struct usb_gadget *gadget); > >> + > >> extern int usb_add_gadget_udc_release(struct device *parent, > >> struct usb_gadget *gadget, void (*release)(struct device *dev)); > >> extern int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget); > >> -- > >> 2.1.0 > >> > > > -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/