From patchwork Mon Jul 1 13:49:03 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vivek Gautam X-Patchwork-Id: 256143 X-Patchwork-Delegate: marek.vasut@gmail.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id 4F58E2C0090 for ; Mon, 1 Jul 2013 23:55:16 +1000 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id B76AF4A220; Mon, 1 Jul 2013 15:55:14 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id FucdiWLsdMZO; Mon, 1 Jul 2013 15:55:14 +0200 (CEST) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id EE1AE4A1D6; Mon, 1 Jul 2013 15:55:11 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id AFEE74A1D6 for ; Mon, 1 Jul 2013 15:55:04 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ruAu3LgifUgT for ; Mon, 1 Jul 2013 15:54:59 +0200 (CEST) X-Greylist: delayed 348 seconds by postgrey-1.27 at theia; Mon, 01 Jul 2013 15:54:51 CEST X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from mail-bk0-f54.google.com (mail-bk0-f54.google.com [209.85.214.54]) by theia.denx.de (Postfix) with ESMTPS id 30E0F4A1D2 for ; Mon, 1 Jul 2013 15:54:51 +0200 (CEST) Received: by mail-bk0-f54.google.com with SMTP id it16so1746085bkc.13 for ; Mon, 01 Jul 2013 06:54:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=epnioWFwVVnvKkVi3A4X2fg/c+mFZ9plOzBPiVgPFEQ=; b=HJJcIK/cOCCNj+YyG64nchS5iYe/2k6yldefIF2FSzE+vXTkMnxwB4ONOb0mb2W2vC EiPHXRUJF1zOJoITl/vLMfDt6mEixFyNUNObWiPRDlJWw1QU52sXZ4M5RDywG+peyL2V nbF2K/DVLfDUVDm8ZTuoEDBKmxdb6E7ZDU1cD2ks5RTCIBKKdF1f2XrMt9zxQnFCFoC8 s8xLfrHwf5yy4osIuAU228FB31KK5CXAYKFpvrf6piWfCYkfDDrVmhu/np0FFWTW2m46 oDN2gpVz6HEWR55y/hSPcI37PlhcfPdyyd59io8LZeMjVc1jYd9+1OT2iUWz4wZjU0NQ 70hw== MIME-Version: 1.0 X-Received: by 10.204.231.204 with SMTP id jr12mr876344bkb.53.1372686543545; Mon, 01 Jul 2013 06:49:03 -0700 (PDT) Received: by 10.204.9.67 with HTTP; Mon, 1 Jul 2013 06:49:03 -0700 (PDT) In-Reply-To: <201306301838.15844.marex@denx.de> References: <51BA3ECA.3070101@wwwdotorg.org> <51BA41B3.5070900@wwwdotorg.org> <201306301838.15844.marex@denx.de> Date: Mon, 1 Jul 2013 19:19:03 +0530 Message-ID: From: Vivek Gautam To: Marek Vasut Cc: "U-Boot@lists.denx.de" , Tom Warren , Vivek Gautam Subject: Re: [U-Boot] Regression due to 020bbcb "usb: hub: Power-cycle on root-hub ports" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.11 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: u-boot-bounces@lists.denx.de Errors-To: u-boot-bounces@lists.denx.de Hi Marek, On Sun, Jun 30, 2013 at 10:08 PM, Marek Vasut wrote: > Dear Stephen Warren, > >> (Sorry to those on to/cc; I'm resending this so it goes to the correct >> mailing list) Dear Stephen, sorry for the delay in responding to this. >> >> Commit 020bbcb "usb: hub: Power-cycle on root-hub ports" causes a >> regression on Tegra systems. >> >> The first time "usb start" is executed, it appears to work correctly. >> However, any subsequent time it is executed, it takes a long time, and >> eventually fails to find any USB devices. >> >> This situation can happen quite often; for example, if the user forgets >> to plug in a USB device before booting, runs "usb start", realizes that, >> then plugs it in and runs "usb start" again. This is compounded on at >> least one of the Tegra boards, since CONFIG_PREBOOT is set to "usb >> start" on systems (laptops/clamshells) which have built-in USB keyboards. >> >> If I simply revert this patch, then everything works again. (Yes, >> reverting requires fixing a small merge conflict.) >> >> Do you have any idea what the problem can be? I'm tempted to simply ask >> for the patch to be reverted since it causes a regression. >> >> Thanks for any idea how to fix this! > > BUMP? Vivek, any ideas? Otherwise I'm reverting this. Tried this at my end on smdk5250 board, and since we have 3 ports on EHCI (which we haven't defined in 5250's config -- CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS); the issue was very much reproducible. For this following commits should be under scan: 0bf796f usb: hub: Parallelize power-cycling of root-hub ports 020bbcb usb: hub: Power-cycle on root-hub ports There's one BUG that i could see in " 0bf796f " commit. Now that we parallelized the sequence to power cycle ports, so if get_port_status for any port failed, it returns from hub_power_on() and not power-on any of the port. Below is the change i suggest. Dear Stephen, can you please confirm if you problem is related to this BUG in the sequence of power-cycling the ports. With this change i can see that USB 2.0 does not detect attached device for the first time we give 'usb start', but subsequently every time it comes and detects the device. But similar behavior is observed when i revert both of above mentioned commits, so this i should look into. diff --git a/common/usb_hub.c b/common/usb_hub.c index 774ba63..437a51f 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -127,7 +127,7 @@ static void usb_hub_power_on(struct usb_hub_device *hub) for (i = 0; i < dev->maxchild; i++) { ret = usb_get_port_status(dev, i + 1, portsts); if (ret < 0) { - debug("port %d: get_port_status failed\n", i + 1); + printf("port %d: get_port_status failed\n", i + 1); return; } @@ -142,12 +142,10 @@ static void usb_hub_power_on(struct usb_hub_device *hub) */ portstatus = le16_to_cpu(portsts->wPortStatus); if (portstatus & (USB_PORT_STAT_POWER << 1)) { - debug("port %d: Port power change failed\n", i + 1); + printf("port %d: Port power change failed\n", i + 1); return; } - } - for (i = 0; i < dev->maxchild; i++) { usb_set_port_feature(dev, i + 1, USB_PORT_FEAT_POWER); debug("port %d returns %lX\n", i + 1, dev->status); }