From mboxrd@z Thu Jan 1 00:00:00 1970 From: Grygorii Strashko Subject: Re: [RFC PATCH V2 3/8] genirq: Add runtime power management support for IRQ chips Date: Tue, 12 Jan 2016 20:40:42 +0200 Message-ID: <569548AA.8090903@ti.com> References: <1450349309-8107-1-git-send-email-jonathanh@nvidia.com> <1450349309-8107-4-git-send-email-jonathanh@nvidia.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1450349309-8107-4-git-send-email-jonathanh@nvidia.com> Sender: linux-kernel-owner@vger.kernel.org To: Jon Hunter , Thomas Gleixner , Jason Cooper , Marc Zyngier , Jiang Liu , Stephen Warren , Thierry Reding , Soren Brinkmann Cc: Kevin Hilman , Geert Uytterhoeven , Lars-Peter Clausen , Linus Walleij , linux-kernel@vger.kernel.org, linux-tegra@vger.kernel.org, Linux PM list List-Id: linux-pm@vger.kernel.org Hi Jon, On 12/17/2015 12:48 PM, Jon Hunter wrote: > Some IRQ chips may be located in a power domain outside of the CPU > subsystem and hence will require device specific runtime power management. > In order to support such IRQ chips, add a pointer for a device structure > to the irq_chip structure, and if this pointer is populated by the IRQ > chip driver and the flag CHIP_HAS_RPM is set, then the pm_runtime_get/put > APIs for this chip will be called when an IRQ is requested/freed, > respectively. > > Signed-off-by: Jon Hunter I've tried to test these patches with OMAP GPIO and I see it works, in general. "In general" - because OMAP GPIO has some code which is expected to be used very late during suspend or when entering deep CPUIdle states, so I can't use this approach "out-of-the-box" until i find the way to sort it out. Hope some one else can try to test it with GPIO. Soren? > --- > include/linux/irq.h | 4 ++++ > kernel/irq/internals.h | 24 ++++++++++++++++++++++++ > kernel/irq/manage.c | 7 +++++++ > 3 files changed, 35 insertions(+) > > diff --git a/include/linux/irq.h b/include/linux/irq.h > index 3c1c96786248..7a61a7f76177 100644 > --- a/include/linux/irq.h > +++ b/include/linux/irq.h ... > > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c > index 2a429b061171..8a96e4f1e985 100644 > --- a/kernel/irq/manage.c > +++ b/kernel/irq/manage.c > @@ -1116,6 +1116,10 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) > if (!try_module_get(desc->owner)) > return -ENODEV; > > + ret = chip_pm_get(desc); > + if (ret < 0) > + return ret; > + > new->irq = irq; > > /* > @@ -1400,6 +1404,7 @@ out_thread: > put_task_struct(t); > } > out_mput: > + chip_pm_put(desc); > module_put(desc->owner); > return ret; > } Here I still think, that for this solution to be complete It might be good to add additional API to request/free chained IRQs. This is not usual case for GPIO drivers, but with AGIC it seems possible. If no objection I can do rfc. > @@ -1513,6 +1518,7 @@ static struct irqaction *__free_irq(unsigned int irq, void *dev_id) > } > } > > + chip_pm_put(desc); > module_put(desc->owner); > kfree(action->secondary); > return action; > @@ -1799,6 +1805,7 @@ static struct irqaction *__free_percpu_irq(unsigned int irq, void __percpu *dev_ > > unregister_handler_proc(irq, action); > > + chip_pm_put(desc); > module_put(desc->owner); > return action; > > -- regards, -grygorii From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932684AbcALSlT (ORCPT ); Tue, 12 Jan 2016 13:41:19 -0500 Received: from arroyo.ext.ti.com ([192.94.94.40]:42117 "EHLO arroyo.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1763011AbcALSlO (ORCPT ); Tue, 12 Jan 2016 13:41:14 -0500 Subject: Re: [RFC PATCH V2 3/8] genirq: Add runtime power management support for IRQ chips To: Jon Hunter , Thomas Gleixner , Jason Cooper , Marc Zyngier , Jiang Liu , Stephen Warren , Thierry Reding , Soren Brinkmann References: <1450349309-8107-1-git-send-email-jonathanh@nvidia.com> <1450349309-8107-4-git-send-email-jonathanh@nvidia.com> CC: Kevin Hilman , Geert Uytterhoeven , Lars-Peter Clausen , Linus Walleij , , , Linux PM list From: Grygorii Strashko Message-ID: <569548AA.8090903@ti.com> Date: Tue, 12 Jan 2016 20:40:42 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <1450349309-8107-4-git-send-email-jonathanh@nvidia.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Jon, On 12/17/2015 12:48 PM, Jon Hunter wrote: > Some IRQ chips may be located in a power domain outside of the CPU > subsystem and hence will require device specific runtime power management. > In order to support such IRQ chips, add a pointer for a device structure > to the irq_chip structure, and if this pointer is populated by the IRQ > chip driver and the flag CHIP_HAS_RPM is set, then the pm_runtime_get/put > APIs for this chip will be called when an IRQ is requested/freed, > respectively. > > Signed-off-by: Jon Hunter I've tried to test these patches with OMAP GPIO and I see it works, in general. "In general" - because OMAP GPIO has some code which is expected to be used very late during suspend or when entering deep CPUIdle states, so I can't use this approach "out-of-the-box" until i find the way to sort it out. Hope some one else can try to test it with GPIO. Soren? > --- > include/linux/irq.h | 4 ++++ > kernel/irq/internals.h | 24 ++++++++++++++++++++++++ > kernel/irq/manage.c | 7 +++++++ > 3 files changed, 35 insertions(+) > > diff --git a/include/linux/irq.h b/include/linux/irq.h > index 3c1c96786248..7a61a7f76177 100644 > --- a/include/linux/irq.h > +++ b/include/linux/irq.h ... > > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c > index 2a429b061171..8a96e4f1e985 100644 > --- a/kernel/irq/manage.c > +++ b/kernel/irq/manage.c > @@ -1116,6 +1116,10 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new) > if (!try_module_get(desc->owner)) > return -ENODEV; > > + ret = chip_pm_get(desc); > + if (ret < 0) > + return ret; > + > new->irq = irq; > > /* > @@ -1400,6 +1404,7 @@ out_thread: > put_task_struct(t); > } > out_mput: > + chip_pm_put(desc); > module_put(desc->owner); > return ret; > } Here I still think, that for this solution to be complete It might be good to add additional API to request/free chained IRQs. This is not usual case for GPIO drivers, but with AGIC it seems possible. If no objection I can do rfc. > @@ -1513,6 +1518,7 @@ static struct irqaction *__free_irq(unsigned int irq, void *dev_id) > } > } > > + chip_pm_put(desc); > module_put(desc->owner); > kfree(action->secondary); > return action; > @@ -1799,6 +1805,7 @@ static struct irqaction *__free_percpu_irq(unsigned int irq, void __percpu *dev_ > > unregister_handler_proc(irq, action); > > + chip_pm_put(desc); > module_put(desc->owner); > return action; > > -- regards, -grygorii