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
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 <[email protected]>
Suggested-by: Lars-Peter Clausen <[email protected]>
Signed-off-by: Soren Brinkmann <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Soren Brinkmann <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: Soren Brinkmann <[email protected]>
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