[PATCH v2 0/2] Power management callbacks for irqchips

[PATCH v2 0/2] Power management callbacks for irqchips

[Soren Brinkmann]

Hi,

we recently discussed how to handle power management with GPIO devices
that can generate interrupts here: https://lkml.org/lkml/2015/10/23/305.
At the end of that discussion stood Lars' suggestion to add a new pair of
callbacks to the irqchip for this purpose. This is a first, naive
implementation of that approach.

        Thanks,
        Sören

v2:
 - add mailing lists (sorry for forgetting that in v1)

Sören Brinkmann (2):
  genirq: Add irq_pm_(get|put) callbacks to the irqchip
  gpio: zynq: Implement irq_pm_(get|put)

 drivers/gpio/gpio-zynq.c | 20 ++++++++++++++++++++
 include/linux/irq.h      |  4 ++++
 kernel/irq/internals.h   | 14 ++++++++++++++
 kernel/irq/manage.c      | 20 ++++++++++++++++----
 4 files changed, 54 insertions(+), 4 deletions(-)

-- 
2.6.2.3.ga463a5b

[PATCH v2 1/2] genirq: Add irq_pm_(get|put) callbacks to the irqchip

[Soren Brinkmann]

Add two new IRQ chip callbacks for power management purposes. These
callbacks are supposed to bring the HW into a state that allows it to
generate interrupts. The callbacks are called in request_irq and
free_irq, respectively, from normal context.

Cc: Thomas Gleixner <tglx@linutronix.de>
Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
v2:
 - report errors up the callchain
 - add error handling (needed some refactoring of the existing error
   handling in request_threaded_irq)
---
 include/linux/irq.h    |  4 ++++
 kernel/irq/internals.h | 14 ++++++++++++++
 kernel/irq/manage.c    | 20 ++++++++++++++++----
 3 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 3c1c96786248..279f6b7118c8 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -341,6 +341,8 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
  * @irq_get_irqchip_state:	return the internal state of an interrupt
  * @irq_set_irqchip_state:	set the internal state of a interrupt
  * @irq_set_vcpu_affinity:	optional to target a vCPU in a virtual machine
+ * @irq_pm_get:	optional to bring the HW in a state that enables IRQ generation
+ * @irq_pm_put:	undo any effects of @irq_pm_get
  * @flags:		chip specific flags
  */
 struct irq_chip {
@@ -384,6 +386,8 @@ struct irq_chip {
 	int		(*irq_set_irqchip_state)(struct irq_data *data, enum irqchip_irq_state which, bool state);
 
 	int		(*irq_set_vcpu_affinity)(struct irq_data *data, void *vcpu_info);
+	int		(*irq_pm_get)(struct irq_data *data);
+	void		(*irq_pm_put)(struct irq_data *data);
 
 	unsigned long	flags;
 };
diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index 05c2188271b8..8a5842431d6c 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -112,6 +112,20 @@ extern void irq_set_thread_affinity(struct irq_desc *desc);
 extern int irq_do_set_affinity(struct irq_data *data,
 			       const struct cpumask *dest, bool force);
 
+static inline int chip_pm_get(struct irq_desc *desc)
+{
+	if (unlikely(desc->irq_data.chip->irq_pm_get))
+		return desc->irq_data.chip->irq_pm_get(&desc->irq_data);
+
+	return 0;
+}
+
+static inline void chip_pm_put(struct irq_desc *desc)
+{
+	if (unlikely(desc->irq_data.chip->irq_pm_put))
+		desc->irq_data.chip->irq_pm_put(&desc->irq_data);
+}
+
 /* Inline functions for support of irq chips on slow busses */
 static inline void chip_bus_lock(struct irq_desc *desc)
 {
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 0eebaeef317b..0f6dee35afaa 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1556,6 +1556,7 @@ void free_irq(unsigned int irq, void *dev_id)
 	chip_bus_lock(desc);
 	kfree(__free_irq(irq, dev_id));
 	chip_bus_sync_unlock(desc);
+	chip_pm_put(desc);
 }
 EXPORT_SYMBOL(free_irq);
 
@@ -1647,14 +1648,16 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler,
 	action->name = devname;
 	action->dev_id = dev_id;
 
+	retval = chip_pm_get(desc);
+	if (retval < 0)
+		goto err_pm_get;
+
 	chip_bus_lock(desc);
 	retval = __setup_irq(irq, desc, action);
 	chip_bus_sync_unlock(desc);
 
-	if (retval) {
-		kfree(action->secondary);
-		kfree(action);
-	}
+	if (retval)
+		goto err_setup_irq;
 
 #ifdef CONFIG_DEBUG_SHIRQ_FIXME
 	if (!retval && (irqflags & IRQF_SHARED)) {
@@ -1675,6 +1678,15 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler,
 		enable_irq(irq);
 	}
 #endif
+
+	return 0;
+
+err_setup_irq:
+	chip_pm_put(desc);
+err_pm_get:
+	kfree(action->secondary);
+	kfree(action);
+
 	return retval;
 }
 EXPORT_SYMBOL(request_threaded_irq);
-- 
2.6.2.3.ga463a5b

[PATCH v2 2/2] gpio: zynq: Implement irq_pm_(get|put)

[Soren Brinkmann]

The driver uses runtime PM to leverage low power techniques. For
use-cases using GPIO as interrupt the device needs to be in an
appropriate state.

Reported-by: John Linn <linnj@xilinx.com>
Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
v2:
 - declare callbacks static
---
 drivers/gpio/gpio-zynq.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpio/gpio-zynq.c b/drivers/gpio/gpio-zynq.c
index 8abeacac5885..98197f778694 100644
--- a/drivers/gpio/gpio-zynq.c
+++ b/drivers/gpio/gpio-zynq.c
@@ -471,6 +471,22 @@ static int zynq_gpio_set_wake(struct irq_data *data, unsigned int on)
 	return 0;
 }
 
+static int zynq_gpio_irq_pm_get(struct irq_data *data)
+{
+	struct zynq_gpio *gpio = irq_data_get_irq_chip_data(data);
+	struct device *dev = gpio->chip.dev;
+
+	return pm_runtime_get_sync(dev);
+}
+
+static void zynq_gpio_irq_pm_put(struct irq_data *data)
+{
+	struct zynq_gpio *gpio = irq_data_get_irq_chip_data(data);
+	struct device *dev = gpio->chip.dev;
+
+	pm_runtime_put(dev);
+}
+
 /* irq chip descriptor */
 static struct irq_chip zynq_gpio_level_irqchip = {
 	.name		= DRIVER_NAME,
@@ -480,6 +496,8 @@ static struct irq_chip zynq_gpio_level_irqchip = {
 	.irq_unmask	= zynq_gpio_irq_unmask,
 	.irq_set_type	= zynq_gpio_set_irq_type,
 	.irq_set_wake	= zynq_gpio_set_wake,
+	.irq_pm_get	= zynq_gpio_irq_pm_get,
+	.irq_pm_put	= zynq_gpio_irq_pm_put,
 	.flags		= IRQCHIP_EOI_THREADED | IRQCHIP_EOI_IF_HANDLED |
 			  IRQCHIP_MASK_ON_SUSPEND,
 };
@@ -492,6 +510,8 @@ static struct irq_chip zynq_gpio_edge_irqchip = {
 	.irq_unmask	= zynq_gpio_irq_unmask,
 	.irq_set_type	= zynq_gpio_set_irq_type,
 	.irq_set_wake	= zynq_gpio_set_wake,
+	.irq_pm_get	= zynq_gpio_irq_pm_get,
+	.irq_pm_put	= zynq_gpio_irq_pm_put,
 	.flags		= IRQCHIP_MASK_ON_SUSPEND,
 };
 
-- 
2.6.2.3.ga463a5b

Re: [PATCH v2 2/2] gpio: zynq: Implement irq_pm_(get|put)

[Grygorii Strashko]

Hi Soren,

On 11/04/2015 10:16 PM, Soren Brinkmann wrote:
> The driver uses runtime PM to leverage low power techniques. For
> use-cases using GPIO as interrupt the device needs to be in an
> appropriate state.
> 
> Reported-by: John Linn <linnj@xilinx.com>
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>

I've one question here. As I know, there is one code path when 
GPIO HW can be accessed before request_irq() during OF boot:

irq_create_of_mapping()
- irq_create_fwspec_mapping()
  - irq_set_irq_type()

How do you handle this situation? :(

> ---
> v2:
>   - declare callbacks static
> ---
>   drivers/gpio/gpio-zynq.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/gpio/gpio-zynq.c b/drivers/gpio/gpio-zynq.c
> index 8abeacac5885..98197f778694 100644
> --- a/drivers/gpio/gpio-zynq.c
> +++ b/drivers/gpio/gpio-zynq.c
> @@ -471,6 +471,22 @@ static int zynq_gpio_set_wake(struct irq_data *data, unsigned int on)
>   	return 0;
>   }
>   
> +static int zynq_gpio_irq_pm_get(struct irq_data *data)
> +{
> +	struct zynq_gpio *gpio = irq_data_get_irq_chip_data(data);
> +	struct device *dev = gpio->chip.dev;
> +
> +	return pm_runtime_get_sync(dev);
> +}
> +
> +static void zynq_gpio_irq_pm_put(struct irq_data *data)
> +{
> +	struct zynq_gpio *gpio = irq_data_get_irq_chip_data(data);
> +	struct device *dev = gpio->chip.dev;
> +
> +	pm_runtime_put(dev);
> +}
> +
>   /* irq chip descriptor */
>   static struct irq_chip zynq_gpio_level_irqchip = {
>   	.name		= DRIVER_NAME,
> @@ -480,6 +496,8 @@ static struct irq_chip zynq_gpio_level_irqchip = {
>   	.irq_unmask	= zynq_gpio_irq_unmask,
>   	.irq_set_type	= zynq_gpio_set_irq_type,
>   	.irq_set_wake	= zynq_gpio_set_wake,
> +	.irq_pm_get	= zynq_gpio_irq_pm_get,
> +	.irq_pm_put	= zynq_gpio_irq_pm_put,
>   	.flags		= IRQCHIP_EOI_THREADED | IRQCHIP_EOI_IF_HANDLED |
>   			  IRQCHIP_MASK_ON_SUSPEND,
>   };
> @@ -492,6 +510,8 @@ static struct irq_chip zynq_gpio_edge_irqchip = {
>   	.irq_unmask	= zynq_gpio_irq_unmask,
>   	.irq_set_type	= zynq_gpio_set_irq_type,
>   	.irq_set_wake	= zynq_gpio_set_wake,
> +	.irq_pm_get	= zynq_gpio_irq_pm_get,
> +	.irq_pm_put	= zynq_gpio_irq_pm_put,
>   	.flags		= IRQCHIP_MASK_ON_SUSPEND,
>   };
>   
> 


-- 
regards,
-grygorii