From patchwork Mon Jun 24 15:09:07 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roger Quadros X-Patchwork-Id: 2771861 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id A4318C0AB1 for ; Mon, 24 Jun 2013 15:09:51 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id DCF5D20219 for ; Mon, 24 Jun 2013 15:09:46 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7F47720216 for ; Mon, 24 Jun 2013 15:09:45 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Ur8Ok-0003Z5-4p; Mon, 24 Jun 2013 15:09:42 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Ur8Oh-0004Uk-KC; Mon, 24 Jun 2013 15:09:39 +0000 Received: from arroyo.ext.ti.com ([192.94.94.40]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Ur8Oe-0004TW-2Q for linux-arm-kernel@lists.infradead.org; Mon, 24 Jun 2013 15:09:37 +0000 Received: from dlelxv90.itg.ti.com ([172.17.2.17]) by arroyo.ext.ti.com (8.13.7/8.13.7) with ESMTP id r5OF9CqM021194; Mon, 24 Jun 2013 10:09:12 -0500 Received: from DFLE72.ent.ti.com (dfle72.ent.ti.com [128.247.5.109]) by dlelxv90.itg.ti.com (8.14.3/8.13.8) with ESMTP id r5OF9Chf016382; Mon, 24 Jun 2013 10:09:12 -0500 Received: from dlelxv22.itg.ti.com (172.17.1.197) by DFLE72.ent.ti.com (128.247.5.109) with Microsoft SMTP Server id 14.2.342.3; Mon, 24 Jun 2013 10:09:11 -0500 Received: from [172.24.56.234] (h56-234.vpn.ti.com [172.24.56.234]) by dlelxv22.itg.ti.com (8.13.8/8.13.8) with ESMTP id r5OF97x0001165; Mon, 24 Jun 2013 10:09:08 -0500 Message-ID: <51C86113.1090902@ti.com> Date: Mon, 24 Jun 2013 18:09:07 +0300 From: Roger Quadros User-Agent: Mozilla/5.0 (X11; Linux i686; rv:16.0) Gecko/20121011 Thunderbird/16.0.1 MIME-Version: 1.0 To: Alan Stern Subject: Re: [RFC PATCH 4/6] USB: ehci-omap: Suspend the controller during bus suspend References: In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130624_110936_445002_3600AFA9 X-CRM114-Status: GOOD ( 20.97 ) X-Spam-Score: -8.0 (--------) Cc: linux-usb@vger.kernel.org, tony@atomide.com, ruslan.bilovol@ti.com, linux-kernel@vger.kernel.org, balbi@ti.com, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-5.3 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Alan, On 06/20/2013 08:33 PM, Alan Stern wrote: > On Thu, 20 Jun 2013, Roger Quadros wrote: > >>> runtime_resume(dev) >>> { >>> ... >>> >>> if (omap->flags & OMAP_EHCI_IRQ_PENDING) { >>> process_pending_irqs(omap); >> >> OK, thanks. >> >> But I'm not sure if the generic ehci_irq handler is able to >> run in a process context. Maybe if we replace spin_lock(&ehci->lock); >> with spin_lock_irqsave() there, it will work. >> >> Alan is this a doable option? > > ehci_irq() will work okay in process context, provided the caller > disables interrupts. > > But maybe none of this will be needed after Roger redesigns the > controller suspend to work at the right time. Or if it is, we could > adopt a simpler alternative: the controller's resume routine could > always call usb_hcd_resume_root_hub(). After all, about the only > reason for doing a runtime resume of the controller is because the root > hub needs to do something. > OK I've tried to handle all this in an alternate way. Now the controller suspend/resume and runtime suspend/resume is independent of bus suspend. The controller now runtime suspends when all devices on the bus have suspended and the hub auto suspends. NOTE: HW_ACCESSIBLE is still set on runtime_suspend. The challenge here is to process the interrupt in this state. I've tried to handle this state. (i.e. interrupt while controller is runtime suspended), by disabling the interrupt till we are ready and calling usb_hcd_resume_root_hub(). We mark a flag (HW_IRQ_DISABLED) stating that the interrupt was disabled and based on that enable the IRQ and clear the flag in hcd_resume_work(). Do let me know what you think of this approach. diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index d53547d..8879cd2 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2136,6 +2136,11 @@ static void hcd_resume_work(struct work_struct *work) usb_lock_device(udev); usb_remote_wakeup(udev); usb_unlock_device(udev); + if (HCD_IRQ_DISABLED(hcd)) { + /* Interrupt was disabled */ + clear_bit(HCD_FLAG_IRQ_DISABLED, &hcd->flags); + enable_irq(hcd->irq); + } } /** @@ -2225,6 +2230,16 @@ irqreturn_t usb_hcd_irq (int irq, void *__hcd) if (unlikely(HCD_DEAD(hcd) || !HCD_HW_ACCESSIBLE(hcd))) rc = IRQ_NONE; + else if (pm_runtime_status_suspended(hcd->self.controller)) { + /* + * We can't handle it yet so disable IRQ, make note of it + * and resume root hub (i.e. controller as well) + */ + disable_irq_nosync(hcd->irq); + set_bit(HCD_FLAG_IRQ_DISABLED, &hcd->flags); + usb_hcd_resume_root_hub(hcd); + rc = IRQ_HANDLED; + } else if (hcd->driver->irq(hcd) == IRQ_NONE) rc = IRQ_NONE; else diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index f5f5c7d..6c6287e 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -110,6 +110,7 @@ struct usb_hcd { #define HCD_FLAG_WAKEUP_PENDING 4 /* root hub is resuming? */ #define HCD_FLAG_RH_RUNNING 5 /* root hub is running? */ #define HCD_FLAG_DEAD 6 /* controller has died? */ +#define HCD_FLAG_IRQ_DISABLED 7 /* Interrupt was disabled */ /* The flags can be tested using these macros; they are likely to * be slightly faster than test_bit(). @@ -120,6 +121,7 @@ struct usb_hcd { #define HCD_WAKEUP_PENDING(hcd) ((hcd)->flags & (1U << HCD_FLAG_WAKEUP_PENDING)) #define HCD_RH_RUNNING(hcd) ((hcd)->flags & (1U << HCD_FLAG_RH_RUNNING)) #define HCD_DEAD(hcd) ((hcd)->flags & (1U << HCD_FLAG_DEAD)) +#define HCD_IRQ_DISABLED(hcd) ((hcd)->flags & (1U << HCD_FLAG_IRQ_DISABLED)) /* Flags that get set only during HCD registration or removal. */ unsigned rh_registered:1;/* is root hub registered? */ diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c index 16d7150..c5320c7 100644 --- a/drivers/usb/host/ehci-omap.c +++ b/drivers/usb/host/ehci-omap.c @@ -225,6 +225,8 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev) usb_phy_set_suspend(omap->phy[i], 0); } + pm_runtime_put_sync(dev); + return 0; err_pm_runtime: