Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756119AbbESOBW (ORCPT ); Tue, 19 May 2015 10:01:22 -0400 Received: from v094114.home.net.pl ([79.96.170.134]:52601 "HELO v094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1755399AbbESOBT (ORCPT ); Tue, 19 May 2015 10:01:19 -0400 From: "Rafael J. Wysocki" To: Tony Lindgren Cc: Felipe Balbi , "Rafael J. Wysocki" , Alan Stern , Andreas Fenkart , Greg Kroah-Hartman , Huiquan Zhong , Kevin Hilman , NeilBrown , Mika Westerberg , Nishanth Menon , Peter Hurley , Sebastian Andrzej Siewior , Ulf Hansson , Thomas Gleixner , linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org, linux-omap@vger.kernel.org Subject: Re: [PATCH 2/5] PM / Wakeirq: Add automated device wake IRQ handling Date: Tue, 19 May 2015 16:26:37 +0200 Message-ID: <3371662.LPKmgz3rb9@vostro.rjw.lan> User-Agent: KMail/4.11.5 (Linux/4.0.0+; KDE/4.11.5; x86_64; ; ) In-Reply-To: <3554198.gIGuGWK14O@vostro.rjw.lan> References: <1431560196-5722-1-git-send-email-tony@atomide.com> <20150518234400.GN10274@atomide.com> <3554198.gIGuGWK14O@vostro.rjw.lan> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="utf-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3877 Lines: 131 On Tuesday, May 19, 2015 04:04:43 PM Rafael J. Wysocki wrote: > On Monday, May 18, 2015 04:44:01 PM Tony Lindgren wrote: > > * Tony Lindgren [150518 15:06]: > > > +/** > > > + * dev_pm_set_wake_irq - Attach device IO interrupt as wake IRQ > > > + * @dev: Device entry > > > + * @irq: Device IO interrupt > > > + * > > > + * Attach a device IO interrupt as a wake IRQ. The wake IRQ gets > > > + * automatically configured for wake-up from suspend based > > > +void dev_pm_clear_wake_irq(struct device *dev) > > > +{ > > > + struct wake_irq *wirq = dev->power.wakeirq; > > > + unsigned long flags; > > > + > > > + if (!wirq) > > > + return; > > > + > > > + device_wakeup_detach_irq(dev); > > > + > > > + spin_lock_irqsave(&dev->power.lock, flags); > > > + if (wirq->manage_irq) { > > > + free_irq(wirq->irq, wirq); > > > + wirq->manage_irq = false; > > > + } > > > + dev->power.wakeirq = NULL; > > > + spin_unlock_irqrestore(&dev->power.lock, flags); > > > + > > > + wirq->irq = -EINVAL; > > > + kfree(wirq); > > > +} > > > > I just noticed most of the dev_pm_clear_wake_irq is no longer needed. > > We're now freeing it anyways. so it can be just: > > > > void dev_pm_clear_wake_irq(struct device *dev) > > { > > struct wake_irq *wirq = dev->power.wakeirq; > > unsigned long flags; > > > > if (!wirq) > > return; > > > > spin_lock_irqsave(&dev->power.lock, flags); > > dev->power.wakeirq = NULL; > > spin_unlock_irqrestore(&dev->power.lock, flags); > > > > device_wakeup_detach_irq(dev); > > if (wirq->manage_irq) > > free_irq(wirq->irq, wirq); > > kfree(wirq); > > } > > > > > > Regards, > > > > Tony > > > > 8< --------------------- > > From: Tony Lindgren > > Date: Mon, 18 May 2015 15:40:29 -0700 > > Subject: [PATCH] PM / Wakeirq: Add automated device wake IRQ handling > > > > Turns out we can automate the handling for the device_may_wakeup() > > quite a bit by using the kernel wakeup source list. > > > > And as some hardware has separate dedicated wake-up interrupt > > in addition to the IO interrupt, we can automate the handling by > > adding a generic threaded interrupt handler that just calls the > > device PM runtime to wake up the device. > > > > This allows dropping code from device drivers as we currently > > are doing it in multiple ways, and often wrong. > > > > For most drivers, we should be able to drop the following > > boilerplate code from runtime_suspend and runtime_resume > > functions: > > > > ... > > device_init_wakeup(dev, true); > > ... > > if (device_may_wakeup(dev) > > enable_irq_wake(irq); > > ... > > if (device_may_wakeup(dev) > > enable_irq_wake(irq); > > Closing parens are missin in the above two if () statements. > > Also, should the second one be disable_irq_wake(irq)? > > > ... > > device_init_wakeup(dev, false); > > ... > > > > We can replace it with just the following init and exit > > time code: > > > > ... > > device_init_wakeup(dev, true); > > dev_pm_set_wake_irq(dev, irq); > > ... > > dev_pm_clear_wake_irq(dev); > > device_init_wakeup(dev, false); > > ... > > > > And for hardware with dedicated wake-up interrupts: > > > > ... > > device_init_wakeup(dev, true); > > dev_pm_set_dedicated_wake_irq(dev, irq); > > ... > > dev_pm_clear_wake_irq(dev); > > device_init_wakeup(dev, false); > > ... > > > > For now, let's only enable it for select PM_WAKEIRQ. > > Why? What would be wrong with doing that unconditionally? I mean, what about making it depend on CONFIG_PM directly? -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- 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/