From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751506AbbCTGsw (ORCPT ); Fri, 20 Mar 2015 02:48:52 -0400 Received: from mail-bl2on0124.outbound.protection.outlook.com ([65.55.169.124]:12576 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750914AbbCTGst (ORCPT ); Fri, 20 Mar 2015 02:48:49 -0400 Date: Fri, 20 Mar 2015 14:32:50 +0800 From: Peter Chen To: Roger Quadros CC: Alan Stern , , , , , , , , Subject: Re: [RFC][PATCH 1/9] usb: hcd: Introduce usb_start/stop_hcd() Message-ID: <20150320063249.GE7537@shlinux2> References: <550AB538.7090806@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <550AB538.7090806@ti.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-EOPAttributedMessage: 0 Authentication-Results: spf=fail (sender IP is 192.88.168.50) smtp.mailfrom=peter.chen@freescale.com; vger.kernel.org; dkim=none (message not signed) header.d=none; X-Forefront-Antispam-Report: CIP:192.88.168.50;CTRY:US;IPV:NLI;EFV:NLI;BMV:1;SFV:NSPM;SFS:(10019020)(6009001)(339900001)(199003)(189002)(479174004)(51704005)(106466001)(105606002)(46102003)(77156002)(77096005)(62966003)(110136001)(104016003)(6806004)(83506001)(23726002)(19580405001)(19580395003)(87936001)(85426001)(33656002)(46406003)(47776003)(86362001)(97756001)(33716001)(50466002)(50986999)(2950100001)(54356999)(92566002);DIR:OUT;SFP:1102;SCL:1;SRVR:BY1PR0301MB1221;H:tx30smr01.am.freescale.net;FPR:;SPF:Fail;MLV:sfv;A:1;MX:1;LANG:en; X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BY1PR0301MB1221; 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:BY1PR0301MB1221;BCL:0;PCL:0;RULEID:;SRVR:BY1PR0301MB1221; X-Forefront-PRVS: 05214FD68E X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Mar 2015 06:34:02.5001 (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: BY1PR0301MB1221 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Mar 19, 2015 at 01:38:32PM +0200, Roger Quadros wrote: > On 18/03/15 21:49, Alan Stern wrote: > > On Wed, 18 Mar 2015, Roger Quadros wrote: > > > >> To support OTG we want a mechanism to start and stop > >> the HCD from the OTG state machine. Add usb_start_hcd() > >> and usb_stop_hcd(). > >> > >> Signed-off-by: Roger Quadros > > > > There are a few problems in this proposed patch. > > > >> +int usb_start_hcd(struct usb_hcd *hcd) > >> +{ > >> + int retval; > >> + struct usb_device *rhdev = hcd->self.root_hub; > >> + > >> + if (hcd->state != HC_STATE_HALT) { > >> + dev_err(hcd->self.controller, "not starting a running HCD\n"); > >> + return -EINVAL; > >> + } > >> + > >> + hcd->state = HC_STATE_RUNNING; > >> + retval = hcd->driver->start(hcd); > >> + if (retval < 0) { > >> + dev_err(hcd->self.controller, "startup error %d\n", retval); > >> + hcd->state = HC_STATE_HALT; > >> + return retval; > >> + } > >> + > >> + /* starting here, usbcore will pay attention to this root hub */ > >> + if ((retval = register_root_hub(hcd)) != 0) > >> + goto err_register_root_hub; > > > > If the host controller is started more than once, you will end up > > unregistering and re-registering the root hub. The device core does > > not allow this. Once a device has been unregistered, you must not try > > to register it again -- you have to allocate a new device and register > > it instead. > > Understood. > > > > > Also, although you call the driver's ->start method multiple times, the > > ->reset method is called only once, when the controller is first > > probed. It's not clear that this will work in a situation where the HC > > and the UDC share hardware state; after the UDC is stopped it may be > > necessary to reset the HC before it can run again. > > Yes, good point. > > > > It might be possible to make this work, but I suspect quite a few > > drivers would need rewriting first. As another example of the problems > > you face, consider how stopping a host controller will interact with > > the driver's PM support (both system suspend and runtime suspend). > > Right. This needs more work than I thought. > > > > It would be a lot simpler to unbind the host controller driver > > completely when switching to device mode and rebind it when switching > > back. I guess that is the sort of heavy-duty approach you want to > > avoid, but it may be the only practical way forward. > > So you mean directly calling usb_add/remove_hcd() from the OTG core? > I don't see any issues with that other than it being a heavy-duty operation > like you said and hope that it doesn't violate the OTG spec timing. > > Looking at Figure 5-3: "HNP Sequence of Events (FS)" of the OTG 2.0 spec > we have about 150ms (X10) to switch from B-Device detected A connect (b_wait_acon state) > to driving bus reset (b_host state). I don't think this should be an issue in modern SoCs > but I'm not very sure. > It is not related toadd/remove hcd, it is the time from receiving PCD to issue BUS_RESET, the Linux stack can't satisfy OTG spec (150ms) due to there are some de-bounce waitings. > In any case I can migrate to the add/remove hcd approach to simplify things. > It should be no problem, we use it more than 1 years. -- Best Regards, Peter Chen