[PATCH V5 0/9] Add support for Tegra210 AGIC

[PATCH V5 0/9] Add support for Tegra210 AGIC

[Jon Hunter]

The Tegra210 has a 2nd level interrupt controller located in a separate
power domain to the main GIC interrupt controller and hence requires
runtime-pm support.

Add a platform driver for the GICs that require runtime-pm and make the
necessary changes to the genirq and irqdomain core to support IRQ chips
that require runtime-pm.

This series is based upon Marc Z's irq/irqchip-v4.7-rc1 branch [0].

Changes since V4:
- Removed header file drivers/irqchip/irq-gic.h.
- Avoid exporting gic_handle_cascade_irq entry point.
- Renamed of_gic_init() to gic_of_init_child() to distinguish it from the
  existing gic_of_int().
- Corrected preprocessor directive for CONFIG_ARM_GIC_PM.

Changes since V3:
- Split the series into two, placing the clean-up/fixes patches into
  another series [1]. The remaining patches in this series are for
  adding runtime-pm support for irqchips and adding a platform driver
  of second level GICs.
- Dropped the patch to prevent early init of GICs if 'clocks' or
  'power-domains' properties are present and re-added the patch to add
  the compatibilty string for the Tegra210 AGIC.

Changes since V2:
- Corrected RPM support for irqchips in genirq core as pointed out by
  Kevin Hilman.
- Corrected patch to save the irq type when mapping the interrupt.
- Dropped changes to DT binding documentation and added patch to prevent
  early init of GICs if the 'clocks' and/or 'power-domains' DT
  properties are present (as we have discussed).
- Moved platform driver code into it's own source file.
- Separate changes for preparing for the platform driver into 3 patches
  in an attempt to make it more readable!
- Added fix for checking an interrupt descriptor is valid during IRQ
  setup.

Changes since V1:
- Updated GIC to only WARN and not return an error if configuring a PPI
  fails but will still return an error if an SPI fails (per discussion
  with Marc).
- Dropped change to mask sense bits for GIC-v3 (as this is not
  necessary)
- Split patch to avoid setting interrupt type when mapping the IRQ into
  two patches per TGLX's feedback.
- Changed name of irqchip device structure to "parent_device"
- Moved call to irq_chip_pm_get() outside of chip_bus_lock().
- Dropped patch to remove clock names from GIC DT documentation and
  added AGIC clock names.
- Update GIC platform driver to look-up clocks names from static list.

[0] http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/log/?h=irq/irqchip-4.7-rc1
[1] http://marc.info/?l=linux-tegra&m=146289329915585&w=2

Jon Hunter (9):
  irqdomain: Fix handling of type settings for existing mappings
  genirq: Look-up trigger type if not specified by caller
  irqdomain: Don't set type when mapping an IRQ
  genirq: Add runtime power management support for IRQ chips
  irqchip/gic: Isolate early GIC initialisation code
  irqchip/gic: Add helper function for chip initialisation
  irqchip/gic: Prepare for adding platform driver
  dt-bindings: arm-gic: Add documentation for Tegra210 AGIC
  irqchip/gic: Add platform driver for non-root GICs that require RPM

 .../bindings/interrupt-controller/arm,gic.txt      |   3 +-
 drivers/irqchip/Kconfig                            |   6 +
 drivers/irqchip/Makefile                           |   1 +
 drivers/irqchip/irq-gic-common.c                   |   4 +-
 drivers/irqchip/irq-gic-pm.c                       | 184 +++++++++++++++++++++
 drivers/irqchip/irq-gic.c                          | 133 ++++++++++-----
 include/linux/irq.h                                |   4 +
 include/linux/irqchip/arm-gic.h                    |  11 ++
 include/linux/irqdomain.h                          |   3 +
 kernel/irq/chip.c                                  |  35 ++++
 kernel/irq/internals.h                             |   1 +
 kernel/irq/irqdomain.c                             |  58 ++++++-
 kernel/irq/manage.c                                |  38 ++++-
 13 files changed, 429 insertions(+), 52 deletions(-)
 create mode 100644 drivers/irqchip/irq-gic-pm.c

-- 
2.1.4

[PATCH V5 1/9] irqdomain: Fix handling of type settings for existing mappings

[Jon Hunter]

When mapping an IRQ, it is possible that a mapping for the IRQ already
exists. If mapping does exist then there are the following issues with
regard to the handling of the IRQ type settings ...
1. If the domain is part of a hierarchy, then:
   a. We do not check that the type settings for the existing mapping
      match those of the new mapping.
   b. We do not check to see if the type settings have been programmed
      yet (and they might not have been) and so we may never set the
      type.
2. If the domain is NOT part of a hierarchy, we will overwrite the
   current type settings programmed if they are different from the
   previous mapping. Please note that irq_create_mapping()
   calls irq_find_mapping() to check if a mapping already exists.

Although, it may be unlikely that the type settings for a shared
interrupt would not match, nonetheless we should check for this.
Therefore, to fix this check if a mapping exists (regardless of whether
the domain is part of a hierarchy or not) and if it does then:
1. Return the IRQ number if the type settings match or are not
   specified.
2. Program the type settings and return the IRQ number if the type
   settings have not been programmed yet.
3. Otherwise if the type setting do not match, then print a warning
   and don't return the IRQ number.

Furthermore, add a warning if the type return by irq_domain_translate()
has bits outside the sense mask set and then clear these bits. If these
bits are not cleared then this will cause the comparision of the type
settings for an existing mapping to fail with that of the new mapping
even if the sense bit themselves match. The reason being is that the
existing type settings are read by calling irq_get_trigger_type() which
will clear any bits outside the sense mask. This will allow us to detect
irqchips that are not correctly clearing these bits and fix them.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
---
 kernel/irq/irqdomain.c | 37 ++++++++++++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 8798b6c9e945..f3ff1eb8dd09 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -588,15 +588,42 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
 	if (irq_domain_translate(domain, fwspec, &hwirq, &type))
 		return 0;
 
-	if (irq_domain_is_hierarchy(domain)) {
+	/*
+	 * WARN if the irqchip returns a type with bits
+	 * outside the sense mask set and clear these bits.
+	 */
+	if (WARN_ON(type & ~IRQ_TYPE_SENSE_MASK))
+		type &= IRQ_TYPE_SENSE_MASK;
+
+	/*
+	 * If we've already configured this interrupt,
+	 * don't do it again, or hell will break loose.
+	 */
+	virq = irq_find_mapping(domain, hwirq);
+	if (virq) {
 		/*
-		 * If we've already configured this interrupt,
-		 * don't do it again, or hell will break loose.
+		 * If the trigger type is not specified or matches the
+		 * current trigger type then we are done so return the
+		 * interrupt number.
 		 */
-		virq = irq_find_mapping(domain, hwirq);
-		if (virq)
+		if (type == IRQ_TYPE_NONE || type == irq_get_trigger_type(virq))
 			return virq;
 
+		/*
+		 * If the trigger type has not been set yet, then set
+		 * it now and return the interrupt number.
+		 */
+		if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
+			irq_set_irq_type(virq, type);
+			return virq;
+		}
+
+		pr_warn("type mismatch, failed to map hwirq-%lu for %s!\n",
+			hwirq, of_node_full_name(to_of_node(fwspec->fwnode)));
+		return 0;
+	}
+
+	if (irq_domain_is_hierarchy(domain)) {
 		virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, fwspec);
 		if (virq <= 0)
 			return 0;
-- 
2.1.4

[PATCH V5 2/9] genirq: Look-up trigger type if not specified by caller

[Jon Hunter]

For some devices the IRQ trigger type for a device is read from
firmware, such as device-tree. The IRQ trigger type is typically read
when the mapping for IRQ is created, which is before the IRQ is
requested. Hence, the IRQ trigger type is programmed when mapping the
IRQ and not when requesting the IRQ.

Although this works for most cases, in order to support IRQ chips which
require runtime power management, which may not be accessible prior
to requesting the IRQ, it is desirable to look-up the IRQ trigger type
when it is requested. Therefore, if the IRQ trigger type is not
specified when __setup_irq() is called, look-up the saved IRQ trigger
type. This will allow us to defer the programming of the trigger type
from when the IRQ is mapped to when it is actually requested.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
---
 kernel/irq/manage.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index ef0bc02c3a70..eaedeb74b49d 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1117,6 +1117,13 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
 	new->irq = irq;
 
 	/*
+	 * If the trigger type is not specified by the caller,
+	 * then use the default for this interrupt.
+	 */
+	if (!(new->flags & IRQF_TRIGGER_MASK))
+		new->flags |= irqd_get_trigger_type(&desc->irq_data);
+
+	/*
 	 * Check whether the interrupt nests into another interrupt
 	 * thread.
 	 */
-- 
2.1.4

[PATCH V5 3/9] irqdomain: Don't set type when mapping an IRQ

[Jon Hunter]

Some IRQ chips, such as GPIO controllers or secondary level interrupt
controllers, may require require additional runtime power management
control to ensure they are accessible. For such IRQ chips, it makes sense
to enable the IRQ chip when interrupts are requested and disabled them
again once all interrupts have been freed.

When mapping an IRQ, the IRQ type settings are read and then programmed.
The mapping of the IRQ happens before the IRQ is requested and so the
programming of the type settings occurs before the IRQ is requested. This
is a problem for IRQ chips that require additional power management
control because they may not be accessible yet. Therefore, when mapping
the IRQ, don't program the type settings, just save them and then program
these saved settings when the IRQ is requested (so long as if they are not
overridden via the call to request the IRQ).

Add a stub function for irq_domain_free_irqs() to avoid any compilation
errors when CONFIG_IRQ_DOMAIN_HIERARCHY is not selected.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
---
 include/linux/irqdomain.h |  3 +++
 kernel/irq/irqdomain.c    | 23 ++++++++++++++++++-----
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index f1f36e04d885..317503763314 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -452,6 +452,9 @@ static inline int irq_domain_alloc_irqs(struct irq_domain *domain,
 	return -1;
 }
 
+static inline void irq_domain_free_irqs(unsigned int virq,
+					unsigned int nr_irqs) { }
+
 static inline bool irq_domain_is_hierarchy(struct irq_domain *domain)
 {
 	return false;
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index f3ff1eb8dd09..caa6a63d26f0 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -567,6 +567,7 @@ static void of_phandle_args_to_fwspec(struct of_phandle_args *irq_data,
 unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
 {
 	struct irq_domain *domain;
+	struct irq_data *irq_data;
 	irq_hw_number_t hwirq;
 	unsigned int type = IRQ_TYPE_NONE;
 	int virq;
@@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
 		 * it now and return the interrupt number.
 		 */
 		if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
-			irq_set_irq_type(virq, type);
+			irq_data = irq_get_irq_data(virq);
+			if (!irq_data)
+				return 0;
+
+			irqd_set_trigger_type(irq_data, type);
 			return virq;
 		}
 
@@ -634,10 +639,18 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
 			return virq;
 	}
 
-	/* Set type if specified and different than the current one */
-	if (type != IRQ_TYPE_NONE &&
-	    type != irq_get_trigger_type(virq))
-		irq_set_irq_type(virq, type);
+	irq_data = irq_get_irq_data(virq);
+	if (!irq_data) {
+		if (irq_domain_is_hierarchy(domain))
+			irq_domain_free_irqs(virq, 1);
+		else
+			irq_dispose_mapping(virq);
+		return 0;
+	}
+
+	/* Store trigger type */
+	irqd_set_trigger_type(irq_data, type);
+
 	return virq;
 }
 EXPORT_SYMBOL_GPL(irq_create_fwspec_mapping);
-- 
2.1.4

[PATCH V5 4/9] genirq: Add runtime power management support for IRQ chips

[Jon Hunter]

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 CONFIG_PM is selected in the kernel
configuration, 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 <jonathanh@nvidia.com>
Reviewed-by: Kevin Hilman <khilman@baylibre.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
---
 include/linux/irq.h    |  4 ++++
 kernel/irq/chip.c      | 35 +++++++++++++++++++++++++++++++++++
 kernel/irq/internals.h |  1 +
 kernel/irq/manage.c    | 31 ++++++++++++++++++++++++++++++-
 4 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 4d758a7c604a..6c92a847394d 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -315,6 +315,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
 /**
  * struct irq_chip - hardware interrupt chip descriptor
  *
+ * @parent_device:	pointer to parent device for irqchip
  * @name:		name for /proc/interrupts
  * @irq_startup:	start up the interrupt (defaults to ->enable if NULL)
  * @irq_shutdown:	shut down the interrupt (defaults to ->disable if NULL)
@@ -354,6 +355,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
  * @flags:		chip specific flags
  */
 struct irq_chip {
+	struct device	*parent_device;
 	const char	*name;
 	unsigned int	(*irq_startup)(struct irq_data *data);
 	void		(*irq_shutdown)(struct irq_data *data);
@@ -488,6 +490,8 @@ extern void handle_bad_irq(struct irq_desc *desc);
 extern void handle_nested_irq(unsigned int irq);
 
 extern int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg);
+extern int irq_chip_pm_get(struct irq_data *data);
+extern int irq_chip_pm_put(struct irq_data *data);
 #ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
 extern void irq_chip_enable_parent(struct irq_data *data);
 extern void irq_chip_disable_parent(struct irq_data *data);
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 2f9f2b0e79f2..b09226e895c7 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -1093,3 +1093,38 @@ int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 
 	return 0;
 }
+
+/**
+ * irq_chip_pm_get - Enable power for an IRQ chip
+ * @data:	Pointer to interrupt specific data
+ *
+ * Enable the power to the IRQ chip referenced by the interrupt data
+ * structure.
+ */
+int irq_chip_pm_get(struct irq_data *data)
+{
+	int retval = 0;
+
+	if (IS_ENABLED(CONFIG_PM) && data->chip->parent_device)
+		retval = pm_runtime_get_sync(data->chip->parent_device);
+
+	return (retval < 0) ? retval : 0;
+}
+
+/**
+ * irq_chip_pm_put - Disable power for an IRQ chip
+ * @data:	Pointer to interrupt specific data
+ *
+ * Disable the power to the IRQ chip referenced by the interrupt data
+ * structure, belongs. Note that power will only be disabled, once this
+ * function has been called for all IRQs that have called irq_chip_pm_get().
+ */
+int irq_chip_pm_put(struct irq_data *data)
+{
+	int retval = 0;
+
+	if (IS_ENABLED(CONFIG_PM) && data->chip->parent_device)
+		retval = pm_runtime_put(data->chip->parent_device);
+
+	return (retval < 0) ? retval : 0;
+}
diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index 09be2c903c6d..d5edcdc9382a 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -7,6 +7,7 @@
  */
 #include <linux/irqdesc.h>
 #include <linux/kernel_stat.h>
+#include <linux/pm_runtime.h>
 
 #ifdef CONFIG_SPARSE_IRQ
 # define IRQ_BITMAP_BITS	(NR_IRQS + 8196)
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index eaedeb74b49d..f8fd1fbc02ea 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1416,10 +1416,18 @@ int setup_irq(unsigned int irq, struct irqaction *act)
 
 	if (!desc || WARN_ON(irq_settings_is_per_cpu_devid(desc)))
 		return -EINVAL;
+
+	retval = irq_chip_pm_get(&desc->irq_data);
+	if (retval < 0)
+		return retval;
+
 	chip_bus_lock(desc);
 	retval = __setup_irq(irq, desc, act);
 	chip_bus_sync_unlock(desc);
 
+	if (retval)
+		irq_chip_pm_put(&desc->irq_data);
+
 	return retval;
 }
 EXPORT_SYMBOL_GPL(setup_irq);
@@ -1513,6 +1521,7 @@ static struct irqaction *__free_irq(unsigned int irq, void *dev_id)
 		}
 	}
 
+	irq_chip_pm_put(&desc->irq_data);
 	module_put(desc->owner);
 	kfree(action->secondary);
 	return action;
@@ -1655,11 +1664,16 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler,
 	action->name = devname;
 	action->dev_id = dev_id;
 
+	retval = irq_chip_pm_get(&desc->irq_data);
+	if (retval < 0)
+		return retval;
+
 	chip_bus_lock(desc);
 	retval = __setup_irq(irq, desc, action);
 	chip_bus_sync_unlock(desc);
 
 	if (retval) {
+		irq_chip_pm_put(&desc->irq_data);
 		kfree(action->secondary);
 		kfree(action);
 	}
@@ -1829,6 +1843,7 @@ static struct irqaction *__free_percpu_irq(unsigned int irq, void __percpu *dev_
 
 	unregister_handler_proc(irq, action);
 
+	irq_chip_pm_put(&desc->irq_data);
 	module_put(desc->owner);
 	return action;
 
@@ -1891,10 +1906,18 @@ int setup_percpu_irq(unsigned int irq, struct irqaction *act)
 
 	if (!desc || !irq_settings_is_per_cpu_devid(desc))
 		return -EINVAL;
+
+	retval = irq_chip_pm_get(&desc->irq_data);
+	if (retval < 0)
+		return retval;
+
 	chip_bus_lock(desc);
 	retval = __setup_irq(irq, desc, act);
 	chip_bus_sync_unlock(desc);
 
+	if (retval)
+		irq_chip_pm_put(&desc->irq_data);
+
 	return retval;
 }
 
@@ -1938,12 +1961,18 @@ int request_percpu_irq(unsigned int irq, irq_handler_t handler,
 	action->name = devname;
 	action->percpu_dev_id = dev_id;
 
+	retval = irq_chip_pm_get(&desc->irq_data);
+	if (retval < 0)
+		return retval;
+
 	chip_bus_lock(desc);
 	retval = __setup_irq(irq, desc, action);
 	chip_bus_sync_unlock(desc);
 
-	if (retval)
+	if (retval) {
+		irq_chip_pm_put(&desc->irq_data);
 		kfree(action);
+	}
 
 	return retval;
 }
-- 
2.1.4

Re: [PATCH V5 4/9] genirq: Add runtime power management support for IRQ chips

[Grygorii Strashko]

On 06/06/2016 02:53 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 CONFIG_PM is selected in the kernel
> configuration, 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 <jonathanh@nvidia.com>
> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>   include/linux/irq.h    |  4 ++++
>   kernel/irq/chip.c      | 35 +++++++++++++++++++++++++++++++++++
>   kernel/irq/internals.h |  1 +
>   kernel/irq/manage.c    | 31 ++++++++++++++++++++++++++++++-
>   4 files changed, 70 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 4d758a7c604a..6c92a847394d 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -315,6 +315,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
>   /**
>    * struct irq_chip - hardware interrupt chip descriptor
>    *
> + * @parent_device:	pointer to parent device for irqchip
>    * @name:		name for /proc/interrupts
>    * @irq_startup:	start up the interrupt (defaults to ->enable if NULL)
>    * @irq_shutdown:	shut down the interrupt (defaults to ->disable if NULL)
> @@ -354,6 +355,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
>    * @flags:		chip specific flags
>    */
>   struct irq_chip {
> +	struct device	*parent_device;
>   	const char	*name;
>   	unsigned int	(*irq_startup)(struct irq_data *data);
>   	void		(*irq_shutdown)(struct irq_data *data);
> @@ -488,6 +490,8 @@ extern void handle_bad_irq(struct irq_desc *desc);
>   extern void handle_nested_irq(unsigned int irq);
>   
>   extern int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg);
> +extern int irq_chip_pm_get(struct irq_data *data);
> +extern int irq_chip_pm_put(struct irq_data *data);
>   #ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
>   extern void irq_chip_enable_parent(struct irq_data *data);
>   extern void irq_chip_disable_parent(struct irq_data *data);
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 2f9f2b0e79f2..b09226e895c7 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -1093,3 +1093,38 @@ int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
>   
>   	return 0;
>   }
> +
> +/**
> + * irq_chip_pm_get - Enable power for an IRQ chip
> + * @data:	Pointer to interrupt specific data
> + *
> + * Enable the power to the IRQ chip referenced by the interrupt data
> + * structure.
> + */
> +int irq_chip_pm_get(struct irq_data *data)
> +{
> +	int retval = 0;
> +
> +	if (IS_ENABLED(CONFIG_PM) && data->chip->parent_device)
> +		retval = pm_runtime_get_sync(data->chip->parent_device);

Sry, for the late comment - above require pm_runtime_put_noidle(data->chip->parent_device);
in case of failure.

[...]


-- 
regards,
-grygorii

Re: [PATCH V5 4/9] genirq: Add runtime power management support for IRQ chips

[Jon Hunter]

On 06/06/16 15:13, Grygorii Strashko wrote:
> On 06/06/2016 02:53 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 CONFIG_PM is selected in the kernel
>> configuration, 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 <jonathanh@nvidia.com>
>> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
>> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>   include/linux/irq.h    |  4 ++++
>>   kernel/irq/chip.c      | 35 +++++++++++++++++++++++++++++++++++
>>   kernel/irq/internals.h |  1 +
>>   kernel/irq/manage.c    | 31 ++++++++++++++++++++++++++++++-
>>   4 files changed, 70 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>> index 4d758a7c604a..6c92a847394d 100644
>> --- a/include/linux/irq.h
>> +++ b/include/linux/irq.h
>> @@ -315,6 +315,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
>>   /**
>>    * struct irq_chip - hardware interrupt chip descriptor
>>    *
>> + * @parent_device:	pointer to parent device for irqchip
>>    * @name:		name for /proc/interrupts
>>    * @irq_startup:	start up the interrupt (defaults to ->enable if NULL)
>>    * @irq_shutdown:	shut down the interrupt (defaults to ->disable if NULL)
>> @@ -354,6 +355,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
>>    * @flags:		chip specific flags
>>    */
>>   struct irq_chip {
>> +	struct device	*parent_device;
>>   	const char	*name;
>>   	unsigned int	(*irq_startup)(struct irq_data *data);
>>   	void		(*irq_shutdown)(struct irq_data *data);
>> @@ -488,6 +490,8 @@ extern void handle_bad_irq(struct irq_desc *desc);
>>   extern void handle_nested_irq(unsigned int irq);
>>   
>>   extern int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg);
>> +extern int irq_chip_pm_get(struct irq_data *data);
>> +extern int irq_chip_pm_put(struct irq_data *data);
>>   #ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
>>   extern void irq_chip_enable_parent(struct irq_data *data);
>>   extern void irq_chip_disable_parent(struct irq_data *data);
>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>> index 2f9f2b0e79f2..b09226e895c7 100644
>> --- a/kernel/irq/chip.c
>> +++ b/kernel/irq/chip.c
>> @@ -1093,3 +1093,38 @@ int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
>>   
>>   	return 0;
>>   }
>> +
>> +/**
>> + * irq_chip_pm_get - Enable power for an IRQ chip
>> + * @data:	Pointer to interrupt specific data
>> + *
>> + * Enable the power to the IRQ chip referenced by the interrupt data
>> + * structure.
>> + */
>> +int irq_chip_pm_get(struct irq_data *data)
>> +{
>> +	int retval = 0;
>> +
>> +	if (IS_ENABLED(CONFIG_PM) && data->chip->parent_device)
>> +		retval = pm_runtime_get_sync(data->chip->parent_device);
> 
> Sry, for the late comment - above require pm_runtime_put_noidle(data->chip->parent_device);
> in case of failure.

No problem. Sorry, can you elaborate? I am not familiar with the
_put_noidle().

Cheers
Jon

-- 
nvpublic

Re: [PATCH V5 4/9] genirq: Add runtime power management support for IRQ chips

[Grygorii Strashko]

On 06/06/2016 05:30 PM, Jon Hunter wrote:
> 
> On 06/06/16 15:13, Grygorii Strashko wrote:
>> On 06/06/2016 02:53 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 CONFIG_PM is selected in the kernel
>>> configuration, 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 <jonathanh@nvidia.com>
>>> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
>>> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
>>> ---
>>>    include/linux/irq.h    |  4 ++++
>>>    kernel/irq/chip.c      | 35 +++++++++++++++++++++++++++++++++++
>>>    kernel/irq/internals.h |  1 +
>>>    kernel/irq/manage.c    | 31 ++++++++++++++++++++++++++++++-
>>>    4 files changed, 70 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>>> index 4d758a7c604a..6c92a847394d 100644
>>> --- a/include/linux/irq.h
>>> +++ b/include/linux/irq.h
>>> @@ -315,6 +315,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
>>>    /**
>>>     * struct irq_chip - hardware interrupt chip descriptor
>>>     *
>>> + * @parent_device:	pointer to parent device for irqchip
>>>     * @name:		name for /proc/interrupts
>>>     * @irq_startup:	start up the interrupt (defaults to ->enable if NULL)
>>>     * @irq_shutdown:	shut down the interrupt (defaults to ->disable if NULL)
>>> @@ -354,6 +355,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
>>>     * @flags:		chip specific flags
>>>     */
>>>    struct irq_chip {
>>> +	struct device	*parent_device;
>>>    	const char	*name;
>>>    	unsigned int	(*irq_startup)(struct irq_data *data);
>>>    	void		(*irq_shutdown)(struct irq_data *data);
>>> @@ -488,6 +490,8 @@ extern void handle_bad_irq(struct irq_desc *desc);
>>>    extern void handle_nested_irq(unsigned int irq);
>>>    
>>>    extern int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg);
>>> +extern int irq_chip_pm_get(struct irq_data *data);
>>> +extern int irq_chip_pm_put(struct irq_data *data);
>>>    #ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
>>>    extern void irq_chip_enable_parent(struct irq_data *data);
>>>    extern void irq_chip_disable_parent(struct irq_data *data);
>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>> index 2f9f2b0e79f2..b09226e895c7 100644
>>> --- a/kernel/irq/chip.c
>>> +++ b/kernel/irq/chip.c
>>> @@ -1093,3 +1093,38 @@ int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
>>>    
>>>    	return 0;
>>>    }
>>> +
>>> +/**
>>> + * irq_chip_pm_get - Enable power for an IRQ chip
>>> + * @data:	Pointer to interrupt specific data
>>> + *
>>> + * Enable the power to the IRQ chip referenced by the interrupt data
>>> + * structure.
>>> + */
>>> +int irq_chip_pm_get(struct irq_data *data)
>>> +{
>>> +	int retval = 0;
>>> +
>>> +	if (IS_ENABLED(CONFIG_PM) && data->chip->parent_device)
>>> +		retval = pm_runtime_get_sync(data->chip->parent_device);
>>
>> Sry, for the late comment - above require pm_runtime_put_noidle(data->chip->parent_device);
>> in case of failure.
> 
> No problem. Sorry, can you elaborate? I am not familiar with the
> _put_noidle().
> 

Question here in use counter - pm_runtime_get_sync() will increment usage_count
always and it will not decrement it in case of failure.
pm_runtime_put_noidle() expected to restore usage_count state (-1).


-- 
regards,
-grygorii

Re: [PATCH V5 4/9] genirq: Add runtime power management support for IRQ chips

[Jon Hunter]

On 06/06/16 15:36, Grygorii Strashko wrote:
> On 06/06/2016 05:30 PM, Jon Hunter wrote:
>>
>> On 06/06/16 15:13, Grygorii Strashko wrote:
>>> On 06/06/2016 02:53 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 CONFIG_PM is selected in the kernel
>>>> configuration, 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 <jonathanh@nvidia.com>
>>>> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
>>>> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
>>>> ---
>>>>    include/linux/irq.h    |  4 ++++
>>>>    kernel/irq/chip.c      | 35 +++++++++++++++++++++++++++++++++++
>>>>    kernel/irq/internals.h |  1 +
>>>>    kernel/irq/manage.c    | 31 ++++++++++++++++++++++++++++++-
>>>>    4 files changed, 70 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>>>> index 4d758a7c604a..6c92a847394d 100644
>>>> --- a/include/linux/irq.h
>>>> +++ b/include/linux/irq.h
>>>> @@ -315,6 +315,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
>>>>    /**
>>>>     * struct irq_chip - hardware interrupt chip descriptor
>>>>     *
>>>> + * @parent_device:	pointer to parent device for irqchip
>>>>     * @name:		name for /proc/interrupts
>>>>     * @irq_startup:	start up the interrupt (defaults to ->enable if NULL)
>>>>     * @irq_shutdown:	shut down the interrupt (defaults to ->disable if NULL)
>>>> @@ -354,6 +355,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
>>>>     * @flags:		chip specific flags
>>>>     */
>>>>    struct irq_chip {
>>>> +	struct device	*parent_device;
>>>>    	const char	*name;
>>>>    	unsigned int	(*irq_startup)(struct irq_data *data);
>>>>    	void		(*irq_shutdown)(struct irq_data *data);
>>>> @@ -488,6 +490,8 @@ extern void handle_bad_irq(struct irq_desc *desc);
>>>>    extern void handle_nested_irq(unsigned int irq);
>>>>    
>>>>    extern int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg);
>>>> +extern int irq_chip_pm_get(struct irq_data *data);
>>>> +extern int irq_chip_pm_put(struct irq_data *data);
>>>>    #ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
>>>>    extern void irq_chip_enable_parent(struct irq_data *data);
>>>>    extern void irq_chip_disable_parent(struct irq_data *data);
>>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>>> index 2f9f2b0e79f2..b09226e895c7 100644
>>>> --- a/kernel/irq/chip.c
>>>> +++ b/kernel/irq/chip.c
>>>> @@ -1093,3 +1093,38 @@ int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
>>>>    
>>>>    	return 0;
>>>>    }
>>>> +
>>>> +/**
>>>> + * irq_chip_pm_get - Enable power for an IRQ chip
>>>> + * @data:	Pointer to interrupt specific data
>>>> + *
>>>> + * Enable the power to the IRQ chip referenced by the interrupt data
>>>> + * structure.
>>>> + */
>>>> +int irq_chip_pm_get(struct irq_data *data)
>>>> +{
>>>> +	int retval = 0;
>>>> +
>>>> +	if (IS_ENABLED(CONFIG_PM) && data->chip->parent_device)
>>>> +		retval = pm_runtime_get_sync(data->chip->parent_device);
>>>
>>> Sry, for the late comment - above require pm_runtime_put_noidle(data->chip->parent_device);
>>> in case of failure.
>>
>> No problem. Sorry, can you elaborate? I am not familiar with the
>> _put_noidle().
>>
> 
> Question here in use counter - pm_runtime_get_sync() will increment usage_count
> always and it will not decrement it in case of failure.
> pm_runtime_put_noidle() expected to restore usage_count state (-1).

Thanks was not aware of that. 

Kevin, Marc, given that you have reviewed this one, are you ok with the
above change Grygorii is proposing? 

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index b09226e895c7..63f1860acc4c 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -1105,8 +1105,11 @@ int irq_chip_pm_get(struct irq_data *data)
 {
        int retval = 0;
 
-       if (IS_ENABLED(CONFIG_PM) && data->chip->parent_device)
+       if (IS_ENABLED(CONFIG_PM) && data->chip->parent_device) {
                retval = pm_runtime_get_sync(data->chip->parent_device);
+               if (retval < 0)
+                       pm_runtime_put_noidle(data->chip->parent_device);
+       }
 
        return (retval < 0) ? retval : 0;
 }
 
Cheers
Jon

-- 
nvpublic

Re: [PATCH V5 4/9] genirq: Add runtime power management support for IRQ chips

[Marc Zyngier]

On 06/06/16 16:06, Jon Hunter wrote:
> 
> On 06/06/16 15:36, Grygorii Strashko wrote:
>> On 06/06/2016 05:30 PM, Jon Hunter wrote:
>>>
>>> On 06/06/16 15:13, Grygorii Strashko wrote:
>>>> On 06/06/2016 02:53 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 CONFIG_PM is selected in the kernel
>>>>> configuration, 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 <jonathanh@nvidia.com>
>>>>> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
>>>>> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
>>>>> ---
>>>>>    include/linux/irq.h    |  4 ++++
>>>>>    kernel/irq/chip.c      | 35 +++++++++++++++++++++++++++++++++++
>>>>>    kernel/irq/internals.h |  1 +
>>>>>    kernel/irq/manage.c    | 31 ++++++++++++++++++++++++++++++-
>>>>>    4 files changed, 70 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>>>>> index 4d758a7c604a..6c92a847394d 100644
>>>>> --- a/include/linux/irq.h
>>>>> +++ b/include/linux/irq.h
>>>>> @@ -315,6 +315,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
>>>>>    /**
>>>>>     * struct irq_chip - hardware interrupt chip descriptor
>>>>>     *
>>>>> + * @parent_device:	pointer to parent device for irqchip
>>>>>     * @name:		name for /proc/interrupts
>>>>>     * @irq_startup:	start up the interrupt (defaults to ->enable if NULL)
>>>>>     * @irq_shutdown:	shut down the interrupt (defaults to ->disable if NULL)
>>>>> @@ -354,6 +355,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
>>>>>     * @flags:		chip specific flags
>>>>>     */
>>>>>    struct irq_chip {
>>>>> +	struct device	*parent_device;
>>>>>    	const char	*name;
>>>>>    	unsigned int	(*irq_startup)(struct irq_data *data);
>>>>>    	void		(*irq_shutdown)(struct irq_data *data);
>>>>> @@ -488,6 +490,8 @@ extern void handle_bad_irq(struct irq_desc *desc);
>>>>>    extern void handle_nested_irq(unsigned int irq);
>>>>>    
>>>>>    extern int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg);
>>>>> +extern int irq_chip_pm_get(struct irq_data *data);
>>>>> +extern int irq_chip_pm_put(struct irq_data *data);
>>>>>    #ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
>>>>>    extern void irq_chip_enable_parent(struct irq_data *data);
>>>>>    extern void irq_chip_disable_parent(struct irq_data *data);
>>>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>>>> index 2f9f2b0e79f2..b09226e895c7 100644
>>>>> --- a/kernel/irq/chip.c
>>>>> +++ b/kernel/irq/chip.c
>>>>> @@ -1093,3 +1093,38 @@ int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
>>>>>    
>>>>>    	return 0;
>>>>>    }
>>>>> +
>>>>> +/**
>>>>> + * irq_chip_pm_get - Enable power for an IRQ chip
>>>>> + * @data:	Pointer to interrupt specific data
>>>>> + *
>>>>> + * Enable the power to the IRQ chip referenced by the interrupt data
>>>>> + * structure.
>>>>> + */
>>>>> +int irq_chip_pm_get(struct irq_data *data)
>>>>> +{
>>>>> +	int retval = 0;
>>>>> +
>>>>> +	if (IS_ENABLED(CONFIG_PM) && data->chip->parent_device)
>>>>> +		retval = pm_runtime_get_sync(data->chip->parent_device);
>>>>
>>>> Sry, for the late comment - above require pm_runtime_put_noidle(data->chip->parent_device);
>>>> in case of failure.
>>>
>>> No problem. Sorry, can you elaborate? I am not familiar with the
>>> _put_noidle().
>>>
>>
>> Question here in use counter - pm_runtime_get_sync() will increment usage_count
>> always and it will not decrement it in case of failure.
>> pm_runtime_put_noidle() expected to restore usage_count state (-1).
> 
> Thanks was not aware of that. 
> 
> Kevin, Marc, given that you have reviewed this one, are you ok with the
> above change Grygorii is proposing? 
> 
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index b09226e895c7..63f1860acc4c 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -1105,8 +1105,11 @@ int irq_chip_pm_get(struct irq_data *data)
>  {
>         int retval = 0;
>  
> -       if (IS_ENABLED(CONFIG_PM) && data->chip->parent_device)
> +       if (IS_ENABLED(CONFIG_PM) && data->chip->parent_device) {
>                 retval = pm_runtime_get_sync(data->chip->parent_device);
> +               if (retval < 0)
> +                       pm_runtime_put_noidle(data->chip->parent_device);
> +       }
>  
>         return (retval < 0) ? retval : 0;
>  }
>  

That looks correct to me (having had a look at
Documentation/power/runtime_pm.txt).

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH V5 4/9] genirq: Add runtime power management support for IRQ chips

[Kevin Hilman]

Jon Hunter <jonathanh@nvidia.com> writes:

> On 06/06/16 15:36, Grygorii Strashko wrote:
>> On 06/06/2016 05:30 PM, Jon Hunter wrote:
>>>
>>> On 06/06/16 15:13, Grygorii Strashko wrote:
>>>> On 06/06/2016 02:53 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 CONFIG_PM is selected in the kernel
>>>>> configuration, 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 <jonathanh@nvidia.com>
>>>>> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
>>>>> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
>>>>> ---
>>>>>    include/linux/irq.h    |  4 ++++
>>>>>    kernel/irq/chip.c      | 35 +++++++++++++++++++++++++++++++++++
>>>>>    kernel/irq/internals.h |  1 +
>>>>>    kernel/irq/manage.c    | 31 ++++++++++++++++++++++++++++++-
>>>>>    4 files changed, 70 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>>>>> index 4d758a7c604a..6c92a847394d 100644
>>>>> --- a/include/linux/irq.h
>>>>> +++ b/include/linux/irq.h
>>>>> @@ -315,6 +315,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
>>>>>    /**
>>>>>     * struct irq_chip - hardware interrupt chip descriptor
>>>>>     *
>>>>> + * @parent_device:	pointer to parent device for irqchip
>>>>>     * @name:		name for /proc/interrupts
>>>>>     * @irq_startup:	start up the interrupt (defaults to ->enable if NULL)
>>>>>     * @irq_shutdown:	shut down the interrupt (defaults to ->disable if NULL)
>>>>> @@ -354,6 +355,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
>>>>>     * @flags:		chip specific flags
>>>>>     */
>>>>>    struct irq_chip {
>>>>> +	struct device	*parent_device;
>>>>>    	const char	*name;
>>>>>    	unsigned int	(*irq_startup)(struct irq_data *data);
>>>>>    	void		(*irq_shutdown)(struct irq_data *data);
>>>>> @@ -488,6 +490,8 @@ extern void handle_bad_irq(struct irq_desc *desc);
>>>>>    extern void handle_nested_irq(unsigned int irq);
>>>>>    
>>>>>    extern int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg);
>>>>> +extern int irq_chip_pm_get(struct irq_data *data);
>>>>> +extern int irq_chip_pm_put(struct irq_data *data);
>>>>>    #ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
>>>>>    extern void irq_chip_enable_parent(struct irq_data *data);
>>>>>    extern void irq_chip_disable_parent(struct irq_data *data);
>>>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>>>> index 2f9f2b0e79f2..b09226e895c7 100644
>>>>> --- a/kernel/irq/chip.c
>>>>> +++ b/kernel/irq/chip.c
>>>>> @@ -1093,3 +1093,38 @@ int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
>>>>>    
>>>>>    	return 0;
>>>>>    }
>>>>> +
>>>>> +/**
>>>>> + * irq_chip_pm_get - Enable power for an IRQ chip
>>>>> + * @data:	Pointer to interrupt specific data
>>>>> + *
>>>>> + * Enable the power to the IRQ chip referenced by the interrupt data
>>>>> + * structure.
>>>>> + */
>>>>> +int irq_chip_pm_get(struct irq_data *data)
>>>>> +{
>>>>> +	int retval = 0;
>>>>> +
>>>>> +	if (IS_ENABLED(CONFIG_PM) && data->chip->parent_device)
>>>>> +		retval = pm_runtime_get_sync(data->chip->parent_device);
>>>>
>>>> Sry, for the late comment - above require pm_runtime_put_noidle(data->chip->parent_device);
>>>> in case of failure.
>>>
>>> No problem. Sorry, can you elaborate? I am not familiar with the
>>> _put_noidle().
>>>
>> 
>> Question here in use counter - pm_runtime_get_sync() will increment usage_count
>> always and it will not decrement it in case of failure.
>> pm_runtime_put_noidle() expected to restore usage_count state (-1).
>
> Thanks was not aware of that. 
>
> Kevin, Marc, given that you have reviewed this one, are you ok with the
> above change Grygorii is proposing? 

Yes, that's the right thing to do on error.

Kevin

Re: [PATCH V5 4/9] genirq: Add runtime power management support for IRQ chips

[Jon Hunter]

On 09/06/16 23:56, Kevin Hilman wrote:
> Jon Hunter <jonathanh@nvidia.com> writes:
> 
>> On 06/06/16 15:36, Grygorii Strashko wrote:
>>> On 06/06/2016 05:30 PM, Jon Hunter wrote:
>>>>
>>>> On 06/06/16 15:13, Grygorii Strashko wrote:
>>>>> On 06/06/2016 02:53 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 CONFIG_PM is selected in the kernel
>>>>>> configuration, 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 <jonathanh@nvidia.com>
>>>>>> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
>>>>>> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
>>>>>> ---
>>>>>>    include/linux/irq.h    |  4 ++++
>>>>>>    kernel/irq/chip.c      | 35 +++++++++++++++++++++++++++++++++++
>>>>>>    kernel/irq/internals.h |  1 +
>>>>>>    kernel/irq/manage.c    | 31 ++++++++++++++++++++++++++++++-
>>>>>>    4 files changed, 70 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>>>>>> index 4d758a7c604a..6c92a847394d 100644
>>>>>> --- a/include/linux/irq.h
>>>>>> +++ b/include/linux/irq.h
>>>>>> @@ -315,6 +315,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
>>>>>>    /**
>>>>>>     * struct irq_chip - hardware interrupt chip descriptor
>>>>>>     *
>>>>>> + * @parent_device:	pointer to parent device for irqchip
>>>>>>     * @name:		name for /proc/interrupts
>>>>>>     * @irq_startup:	start up the interrupt (defaults to ->enable if NULL)
>>>>>>     * @irq_shutdown:	shut down the interrupt (defaults to ->disable if NULL)
>>>>>> @@ -354,6 +355,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
>>>>>>     * @flags:		chip specific flags
>>>>>>     */
>>>>>>    struct irq_chip {
>>>>>> +	struct device	*parent_device;
>>>>>>    	const char	*name;
>>>>>>    	unsigned int	(*irq_startup)(struct irq_data *data);
>>>>>>    	void		(*irq_shutdown)(struct irq_data *data);
>>>>>> @@ -488,6 +490,8 @@ extern void handle_bad_irq(struct irq_desc *desc);
>>>>>>    extern void handle_nested_irq(unsigned int irq);
>>>>>>    
>>>>>>    extern int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg);
>>>>>> +extern int irq_chip_pm_get(struct irq_data *data);
>>>>>> +extern int irq_chip_pm_put(struct irq_data *data);
>>>>>>    #ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
>>>>>>    extern void irq_chip_enable_parent(struct irq_data *data);
>>>>>>    extern void irq_chip_disable_parent(struct irq_data *data);
>>>>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>>>>> index 2f9f2b0e79f2..b09226e895c7 100644
>>>>>> --- a/kernel/irq/chip.c
>>>>>> +++ b/kernel/irq/chip.c
>>>>>> @@ -1093,3 +1093,38 @@ int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
>>>>>>    
>>>>>>    	return 0;
>>>>>>    }
>>>>>> +
>>>>>> +/**
>>>>>> + * irq_chip_pm_get - Enable power for an IRQ chip
>>>>>> + * @data:	Pointer to interrupt specific data
>>>>>> + *
>>>>>> + * Enable the power to the IRQ chip referenced by the interrupt data
>>>>>> + * structure.
>>>>>> + */
>>>>>> +int irq_chip_pm_get(struct irq_data *data)
>>>>>> +{
>>>>>> +	int retval = 0;
>>>>>> +
>>>>>> +	if (IS_ENABLED(CONFIG_PM) && data->chip->parent_device)
>>>>>> +		retval = pm_runtime_get_sync(data->chip->parent_device);
>>>>>
>>>>> Sry, for the late comment - above require pm_runtime_put_noidle(data->chip->parent_device);
>>>>> in case of failure.
>>>>
>>>> No problem. Sorry, can you elaborate? I am not familiar with the
>>>> _put_noidle().
>>>>
>>>
>>> Question here in use counter - pm_runtime_get_sync() will increment usage_count
>>> always and it will not decrement it in case of failure.
>>> pm_runtime_put_noidle() expected to restore usage_count state (-1).
>>
>> Thanks was not aware of that. 
>>
>> Kevin, Marc, given that you have reviewed this one, are you ok with the
>> above change Grygorii is proposing? 
> 
> Yes, that's the right thing to do on error.

Thanks. I have added this in V6. Please take a look and add your
reviewed-by again if all looks good.

Cheers
Jon

-- 
nvpublic

[PATCH V5 5/9] irqchip/gic: Isolate early GIC initialisation code

[Jon Hunter]

To re-use the code that initialises the GIC (found in
__gic_init_bases()), from within a platform driver, it is necessary to
move the code from the __init section so that it is always present and
not removed. Unfortunately, it is not possible to simply drop the __init
from the function declaration for __gic_init_bases() because it contains
calls to set_smp_cross_call() and set_handle_irq() which are both
located in the __init section. Fortunately, these calls are only
required for the root controller and because the initial platform driver
will only support non-root controllers that can be initialised later in
the boot process, we can move these calls to another function.

Move the bulk of the code from __gic_init_bases() to a new function
called gic_init_bases() which is not located in the __init section and
can be used by the platform driver. Update __gic_init_bases() to call
gic_init_bases() and if necessary, set_smp_cross_call() and
set_handle_irq().

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/irqchip/irq-gic.c | 55 +++++++++++++++++++++++++++--------------------
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index fbc4ae2afd29..fa0dd98993fa 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1032,14 +1032,11 @@ static const struct irq_domain_ops gic_irq_domain_ops = {
 	.unmap = gic_irq_domain_unmap,
 };
 
-static int __init __gic_init_bases(struct gic_chip_data *gic, int irq_start,
-				   struct fwnode_handle *handle)
+static int gic_init_bases(struct gic_chip_data *gic, int irq_start,
+			  struct fwnode_handle *handle)
 {
 	irq_hw_number_t hwirq_base;
-	int gic_irqs, irq_base, i, ret;
-
-	if (WARN_ON(!gic || gic->domain))
-		return -EINVAL;
+	int gic_irqs, irq_base, ret;
 
 	/* Initialize irq_chip */
 	gic->chip = gic_chip;
@@ -1138,23 +1135,6 @@ static int __init __gic_init_bases(struct gic_chip_data *gic, int irq_start,
 		goto error;
 	}
 
-	if (gic == &gic_data[0]) {
-		/*
-		 * Initialize the CPU interface map to all CPUs.
-		 * It will be refined as each CPU probes its ID.
-		 * This is only necessary for the primary GIC.
-		 */
-		for (i = 0; i < NR_GIC_CPU_IF; i++)
-			gic_cpu_map[i] = 0xff;
-#ifdef CONFIG_SMP
-		set_smp_cross_call(gic_raise_softirq);
-		register_cpu_notifier(&gic_cpu_notifier);
-#endif
-		set_handle_irq(gic_handle_irq);
-		if (static_key_true(&supports_deactivate))
-			pr_info("GIC: Using split EOI/Deactivate mode\n");
-	}
-
 	gic_dist_init(gic);
 	ret = gic_cpu_init(gic);
 	if (ret)
@@ -1177,6 +1157,35 @@ error:
 	return ret;
 }
 
+static int __init __gic_init_bases(struct gic_chip_data *gic,
+				   int irq_start,
+				   struct fwnode_handle *handle)
+{
+	int i;
+
+	if (WARN_ON(!gic || gic->domain))
+		return -EINVAL;
+
+	if (gic == &gic_data[0]) {
+		/*
+		 * Initialize the CPU interface map to all CPUs.
+		 * It will be refined as each CPU probes its ID.
+		 * This is only necessary for the primary GIC.
+		 */
+		for (i = 0; i < NR_GIC_CPU_IF; i++)
+			gic_cpu_map[i] = 0xff;
+#ifdef CONFIG_SMP
+		set_smp_cross_call(gic_raise_softirq);
+		register_cpu_notifier(&gic_cpu_notifier);
+#endif
+		set_handle_irq(gic_handle_irq);
+		if (static_key_true(&supports_deactivate))
+			pr_info("GIC: Using split EOI/Deactivate mode\n");
+	}
+
+	return gic_init_bases(gic, irq_start, handle);
+}
+
 void __init gic_init(unsigned int gic_nr, int irq_start,
 		     void __iomem *dist_base, void __iomem *cpu_base)
 {
-- 
2.1.4

[PATCH V5 6/9] irqchip/gic: Add helper function for chip initialisation

[Jon Hunter]

For GICs that require runtime power-management it is necessary to
populate the 'parent_device' member of the irqchip structure. In
preparation for supporting such GICs, move the code that initialises
the irqchip structure for a GIC into its own function called
gic_init_chip() where the parent device pointer is also set.

Instead of calling gic_init_chip() from within gic_init_bases(), move
the calls to outside of this function, so that in the future we can
avoid having to pass additional parameters to gic_init_bases() in order
set the parent device pointer or set the name to a specific string.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/irqchip/irq-gic.c | 41 +++++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index fa0dd98993fa..94eab6e23124 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1032,29 +1032,31 @@ static const struct irq_domain_ops gic_irq_domain_ops = {
 	.unmap = gic_irq_domain_unmap,
 };
 
-static int gic_init_bases(struct gic_chip_data *gic, int irq_start,
-			  struct fwnode_handle *handle)
+static void gic_init_chip(struct gic_chip_data *gic, struct device *dev,
+			  const char *name, bool use_eoimode1)
 {
-	irq_hw_number_t hwirq_base;
-	int gic_irqs, irq_base, ret;
-
 	/* Initialize irq_chip */
 	gic->chip = gic_chip;
+	gic->chip.name = name;
+	gic->chip.parent_device = dev;
 
-	if (static_key_true(&supports_deactivate) && gic == &gic_data[0]) {
+	if (use_eoimode1) {
 		gic->chip.irq_mask = gic_eoimode1_mask_irq;
 		gic->chip.irq_eoi = gic_eoimode1_eoi_irq;
 		gic->chip.irq_set_vcpu_affinity = gic_irq_set_vcpu_affinity;
-		gic->chip.name = kasprintf(GFP_KERNEL, "GICv2");
-	} else {
-		gic->chip.name = kasprintf(GFP_KERNEL, "GIC-%d",
-					   (int)(gic - &gic_data[0]));
 	}
 
 #ifdef CONFIG_SMP
 	if (gic == &gic_data[0])
 		gic->chip.irq_set_affinity = gic_set_affinity;
 #endif
+}
+
+static int gic_init_bases(struct gic_chip_data *gic, int irq_start,
+			  struct fwnode_handle *handle)
+{
+	irq_hw_number_t hwirq_base;
+	int gic_irqs, irq_base, ret;
 
 	if (IS_ENABLED(CONFIG_GIC_NON_BANKED) && gic->percpu_offset) {
 		/* Frankein-GIC without banked registers... */
@@ -1152,8 +1154,6 @@ error:
 		free_percpu(gic->cpu_base.percpu_base);
 	}
 
-	kfree(gic->chip.name);
-
 	return ret;
 }
 
@@ -1161,7 +1161,8 @@ static int __init __gic_init_bases(struct gic_chip_data *gic,
 				   int irq_start,
 				   struct fwnode_handle *handle)
 {
-	int i;
+	char *name;
+	int i, ret;
 
 	if (WARN_ON(!gic || gic->domain))
 		return -EINVAL;
@@ -1183,7 +1184,19 @@ static int __init __gic_init_bases(struct gic_chip_data *gic,
 			pr_info("GIC: Using split EOI/Deactivate mode\n");
 	}
 
-	return gic_init_bases(gic, irq_start, handle);
+	if (static_key_true(&supports_deactivate) && gic == &gic_data[0]) {
+		name = kasprintf(GFP_KERNEL, "GICv2");
+		gic_init_chip(gic, NULL, name, true);
+	} else {
+		name = kasprintf(GFP_KERNEL, "GIC-%d", (int)(gic-&gic_data[0]));
+		gic_init_chip(gic, NULL, name, false);
+	}
+
+	ret = gic_init_bases(gic, irq_start, handle);
+	if (ret)
+		kfree(name);
+
+	return ret;
 }
 
 void __init gic_init(unsigned int gic_nr, int irq_start,
-- 
2.1.4

[PATCH V5 7/9] irqchip/gic: Prepare for adding platform driver

[Jon Hunter]

To support GICs that require runtime power management, it is necessary
to add a platform driver, so that the probing of the chip can be
deferred if resources, such as a power-domain, is not yet available.

To prepare for adding a platform driver:
 1. Drop the __init section from the gic_dist_config() so this can be
    re-used by the platform driver.
 2. Add prototypes for functions required by the platform driver to the
    GIC header file so they can be re-used.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/irqchip/irq-gic-common.c |  4 ++--
 drivers/irqchip/irq-gic.c        | 15 ++++++++-------
 include/linux/irqchip/arm-gic.h  |  5 +++++
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
index 89e7423f0ebb..9ae71804b5dd 100644
--- a/drivers/irqchip/irq-gic-common.c
+++ b/drivers/irqchip/irq-gic-common.c
@@ -90,8 +90,8 @@ int gic_configure_irq(unsigned int irq, unsigned int type,
 	return ret;
 }
 
-void __init gic_dist_config(void __iomem *base, int gic_irqs,
-			    void (*sync_access)(void))
+void gic_dist_config(void __iomem *base, int gic_irqs,
+		     void (*sync_access)(void))
 {
 	unsigned int i;
 
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 94eab6e23124..3d3ab9045244 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -449,7 +449,7 @@ static void gic_cpu_if_up(struct gic_chip_data *gic)
 }
 
 
-static void __init gic_dist_init(struct gic_chip_data *gic)
+static void gic_dist_init(struct gic_chip_data *gic)
 {
 	unsigned int i;
 	u32 cpumask;
@@ -535,7 +535,7 @@ int gic_cpu_if_down(unsigned int gic_nr)
  * this function, no interrupts will be delivered by the GIC, and another
  * platform-specific wakeup source must be enabled.
  */
-static void gic_dist_save(struct gic_chip_data *gic)
+void gic_dist_save(struct gic_chip_data *gic)
 {
 	unsigned int gic_irqs;
 	void __iomem *dist_base;
@@ -574,7 +574,7 @@ static void gic_dist_save(struct gic_chip_data *gic)
  * handled normally, but any edge interrupts that occured will not be seen by
  * the GIC and need to be handled by the platform-specific wakeup source.
  */
-static void gic_dist_restore(struct gic_chip_data *gic)
+void gic_dist_restore(struct gic_chip_data *gic)
 {
 	unsigned int gic_irqs;
 	unsigned int i;
@@ -620,7 +620,7 @@ static void gic_dist_restore(struct gic_chip_data *gic)
 	writel_relaxed(GICD_ENABLE, dist_base + GIC_DIST_CTRL);
 }
 
-static void gic_cpu_save(struct gic_chip_data *gic)
+void gic_cpu_save(struct gic_chip_data *gic)
 {
 	int i;
 	u32 *ptr;
@@ -650,7 +650,7 @@ static void gic_cpu_save(struct gic_chip_data *gic)
 
 }
 
-static void gic_cpu_restore(struct gic_chip_data *gic)
+void gic_cpu_restore(struct gic_chip_data *gic)
 {
 	int i;
 	u32 *ptr;
@@ -727,7 +727,7 @@ static struct notifier_block gic_notifier_block = {
 	.notifier_call = gic_notifier,
 };
 
-static int __init gic_pm_init(struct gic_chip_data *gic)
+static int gic_pm_init(struct gic_chip_data *gic)
 {
 	gic->saved_ppi_enable = __alloc_percpu(DIV_ROUND_UP(32, 32) * 4,
 		sizeof(u32));
@@ -757,7 +757,7 @@ free_ppi_enable:
 	return -ENOMEM;
 }
 #else
-static int __init gic_pm_init(struct gic_chip_data *gic)
+static int gic_pm_init(struct gic_chip_data *gic)
 {
 	return 0;
 }
@@ -1179,6 +1179,7 @@ static int __init __gic_init_bases(struct gic_chip_data *gic,
 		set_smp_cross_call(gic_raise_softirq);
 		register_cpu_notifier(&gic_cpu_notifier);
 #endif
+
 		set_handle_irq(gic_handle_irq);
 		if (static_key_true(&supports_deactivate))
 			pr_info("GIC: Using split EOI/Deactivate mode\n");
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index fd051855539b..ffcbd8b9a4ff 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -101,9 +101,14 @@
 #include <linux/irqdomain.h>
 
 struct device_node;
+struct gic_chip_data;
 
 void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
 int gic_cpu_if_down(unsigned int gic_nr);
+void gic_cpu_save(struct gic_chip_data *gic);
+void gic_cpu_restore(struct gic_chip_data *gic);
+void gic_dist_save(struct gic_chip_data *gic);
+void gic_dist_restore(struct gic_chip_data *gic);
 
 /*
  * Subdrivers that need some preparatory work can initialize their
-- 
2.1.4

Re: [PATCH V5 7/9] irqchip/gic: Prepare for adding platform driver

[Jon Hunter]

On 06/06/16 12:53, Jon Hunter wrote:
> To support GICs that require runtime power management, it is necessary
> to add a platform driver, so that the probing of the chip can be
> deferred if resources, such as a power-domain, is not yet available.
> 
> To prepare for adding a platform driver:
>  1. Drop the __init section from the gic_dist_config() so this can be
>     re-used by the platform driver.
>  2. Add prototypes for functions required by the platform driver to the
>     GIC header file so they can be re-used.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  drivers/irqchip/irq-gic-common.c |  4 ++--
>  drivers/irqchip/irq-gic.c        | 15 ++++++++-------
>  include/linux/irqchip/arm-gic.h  |  5 +++++
>  3 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
> index 89e7423f0ebb..9ae71804b5dd 100644
> --- a/drivers/irqchip/irq-gic-common.c
> +++ b/drivers/irqchip/irq-gic-common.c
> @@ -90,8 +90,8 @@ int gic_configure_irq(unsigned int irq, unsigned int type,
>  	return ret;
>  }
>  
> -void __init gic_dist_config(void __iomem *base, int gic_irqs,
> -			    void (*sync_access)(void))
> +void gic_dist_config(void __iomem *base, int gic_irqs,
> +		     void (*sync_access)(void))
>  {
>  	unsigned int i;
>  
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 94eab6e23124..3d3ab9045244 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -449,7 +449,7 @@ static void gic_cpu_if_up(struct gic_chip_data *gic)
>  }
>  
>  
> -static void __init gic_dist_init(struct gic_chip_data *gic)
> +static void gic_dist_init(struct gic_chip_data *gic)
>  {
>  	unsigned int i;
>  	u32 cpumask;
> @@ -535,7 +535,7 @@ int gic_cpu_if_down(unsigned int gic_nr)
>   * this function, no interrupts will be delivered by the GIC, and another
>   * platform-specific wakeup source must be enabled.
>   */
> -static void gic_dist_save(struct gic_chip_data *gic)
> +void gic_dist_save(struct gic_chip_data *gic)
>  {
>  	unsigned int gic_irqs;
>  	void __iomem *dist_base;
> @@ -574,7 +574,7 @@ static void gic_dist_save(struct gic_chip_data *gic)
>   * handled normally, but any edge interrupts that occured will not be seen by
>   * the GIC and need to be handled by the platform-specific wakeup source.
>   */
> -static void gic_dist_restore(struct gic_chip_data *gic)
> +void gic_dist_restore(struct gic_chip_data *gic)
>  {
>  	unsigned int gic_irqs;
>  	unsigned int i;
> @@ -620,7 +620,7 @@ static void gic_dist_restore(struct gic_chip_data *gic)
>  	writel_relaxed(GICD_ENABLE, dist_base + GIC_DIST_CTRL);
>  }
>  
> -static void gic_cpu_save(struct gic_chip_data *gic)
> +void gic_cpu_save(struct gic_chip_data *gic)
>  {
>  	int i;
>  	u32 *ptr;
> @@ -650,7 +650,7 @@ static void gic_cpu_save(struct gic_chip_data *gic)
>  
>  }
>  
> -static void gic_cpu_restore(struct gic_chip_data *gic)
> +void gic_cpu_restore(struct gic_chip_data *gic)
>  {
>  	int i;
>  	u32 *ptr;
> @@ -727,7 +727,7 @@ static struct notifier_block gic_notifier_block = {
>  	.notifier_call = gic_notifier,
>  };
>  
> -static int __init gic_pm_init(struct gic_chip_data *gic)
> +static int gic_pm_init(struct gic_chip_data *gic)
>  {
>  	gic->saved_ppi_enable = __alloc_percpu(DIV_ROUND_UP(32, 32) * 4,
>  		sizeof(u32));
> @@ -757,7 +757,7 @@ free_ppi_enable:
>  	return -ENOMEM;
>  }
>  #else
> -static int __init gic_pm_init(struct gic_chip_data *gic)
> +static int gic_pm_init(struct gic_chip_data *gic)
>  {
>  	return 0;
>  }
> @@ -1179,6 +1179,7 @@ static int __init __gic_init_bases(struct gic_chip_data *gic,
>  		set_smp_cross_call(gic_raise_softirq);
>  		register_cpu_notifier(&gic_cpu_notifier);
>  #endif
> +

Not sure where this extra line came from but shouldn't be here.

Marc let me know if you want me to resend.

Jon

-- 
nvpublic

[PATCH V5 8/9] dt-bindings: arm-gic: Add documentation for Tegra210 AGIC

[Jon Hunter]

The Tegra AGIC interrupt controller is compatible with the ARM GIC-400
interrupt controller. Add the compatible string and clock information
for the AGIC to the GIC device-tree binding documentation.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt b/Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt
index 793c20ff8fcc..5393e2a45a42 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt
@@ -21,6 +21,7 @@ Main node required properties:
 	"arm,pl390"
 	"arm,tc11mp-gic"
 	"brcm,brahma-b15-gic"
+	"nvidia,tegra210-agic"
 	"qcom,msm-8660-qgic"
 	"qcom,msm-qgic2"
 - interrupt-controller : Identifies the node as an interrupt controller
@@ -68,7 +69,7 @@ Optional
 	"ic_clk" (for "arm,arm11mp-gic")
 	"PERIPHCLKEN" (for "arm,cortex-a15-gic")
 	"PERIPHCLK", "PERIPHCLKEN" (for "arm,cortex-a9-gic")
-	"clk" (for "arm,gic-400")
+	"clk" (for "arm,gic-400" and "nvidia,tegra210")
 	"gclk" (for "arm,pl390")
 
 - power-domains : A phandle and PM domain specifier as defined by bindings of
-- 
2.1.4

[PATCH V5 9/9] irqchip/gic: Add platform driver for non-root GICs that require RPM

[Jon Hunter]

Add a platform driver to support non-root GICs that require runtime
power-management. Currently, only non-root GICs are supported because
the functions, smp_cross_call() and set_handle_irq(), that need to
be called for a root controller are located in the __init section and
so cannot be called by the platform driver.

The GIC platform driver re-uses many functions from the existing GIC
driver including some functions to save and restore the GIC context
during power transitions. The functions for saving and restoring the
GIC context are currently only defined if CONFIG_CPU_PM is enabled and
to ensure that these functions are always defined when the platform
driver is enabled, a dependency on CONFIG_ARM_GIC_PM (which selects the
platform driver) has been added.

In order to re-use the private GIC initialisation code, a new public
function, gic_of_init_child(), has been added which calls various
private functions to initialise the GIC. This is different from the
existing gic_of_init() because it only supports non-root GICs (ie. does
not call smp_cross_call() is set_handle_irq()) and is not located in
the __init section (so can be used by platform drivers). Furthermore,
gic_of_init_child() dynamically allocates memory for the GIC chip data
which is also different from gic_of_init().

There is no specific suspend handling for GICs registered as platform
devices. Non-wakeup interrupts will be disabled by the kernel during
late suspend, however, this alone will not power down the GIC if
interrupts have been requested and not freed. Therefore, requestors of
non-wakeup interrupts will need to free them on entering suspend in
order to power-down the GIC.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/irqchip/Kconfig         |   6 ++
 drivers/irqchip/Makefile        |   1 +
 drivers/irqchip/irq-gic-pm.c    | 184 ++++++++++++++++++++++++++++++++++++++++
 drivers/irqchip/irq-gic.c       |  38 ++++++++-
 include/linux/irqchip/arm-gic.h |   6 ++
 5 files changed, 232 insertions(+), 3 deletions(-)
 create mode 100644 drivers/irqchip/irq-gic-pm.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index fa33c50b0e5a..5495a5ba8039 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -8,6 +8,12 @@ config ARM_GIC
 	select IRQ_DOMAIN_HIERARCHY
 	select MULTI_IRQ_HANDLER
 
+config ARM_GIC_PM
+	bool
+	depends on PM
+	select ARM_GIC
+	select PM_CLK
+
 config ARM_GIC_MAX_NR
 	int
 	default 2 if ARCH_REALVIEW
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 38853a187607..bd0257e0aab6 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_ARCH_SUNXI)		+= irq-sun4i.o
 obj-$(CONFIG_ARCH_SUNXI)		+= irq-sunxi-nmi.o
 obj-$(CONFIG_ARCH_SPEAR3XX)		+= spear-shirq.o
 obj-$(CONFIG_ARM_GIC)			+= irq-gic.o irq-gic-common.o
+obj-$(CONFIG_ARM_GIC_PM)		+= irq-gic-pm.o
 obj-$(CONFIG_REALVIEW_DT)		+= irq-gic-realview.o
 obj-$(CONFIG_ARM_GIC_V2M)		+= irq-gic-v2m.o
 obj-$(CONFIG_ARM_GIC_V3)		+= irq-gic-v3.o irq-gic-common.o
diff --git a/drivers/irqchip/irq-gic-pm.c b/drivers/irqchip/irq-gic-pm.c
new file mode 100644
index 000000000000..4cbffba3ff13
--- /dev/null
+++ b/drivers/irqchip/irq-gic-pm.c
@@ -0,0 +1,184 @@
+/*
+ * Copyright (C) 2016 NVIDIA CORPORATION, All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/irqchip/arm-gic.h>
+#include <linux/platform_device.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+
+struct gic_clk_data {
+	unsigned int num_clocks;
+	const char *const *clocks;
+};
+
+static int gic_runtime_resume(struct device *dev)
+{
+	struct gic_chip_data *gic = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_clk_resume(dev);
+	if (ret)
+		return ret;
+
+	/*
+	 * On the very first resume, the pointer to the driver data
+	 * will be NULL and this is intentional, because we do not
+	 * want to restore the GIC on the very first resume. So if
+	 * the pointer is not valid just return.
+	 */
+	if (!gic)
+		return 0;
+
+	gic_dist_restore(gic);
+	gic_cpu_restore(gic);
+
+	return 0;
+}
+
+static int gic_runtime_suspend(struct device *dev)
+{
+	struct gic_chip_data *gic = dev_get_drvdata(dev);
+
+	gic_dist_save(gic);
+	gic_cpu_save(gic);
+
+	return pm_clk_suspend(dev);
+}
+
+static int gic_get_clocks(struct device *dev, const struct gic_clk_data *data)
+{
+	struct clk *clk;
+	unsigned int i;
+	int ret;
+
+	if (!dev || !data)
+		return -EINVAL;
+
+	ret = pm_clk_create(dev);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < data->num_clocks; i++) {
+		clk = of_clk_get_by_name(dev->of_node, data->clocks[i]);
+		if (IS_ERR(clk)) {
+			dev_err(dev, "failed to get clock %s\n",
+				data->clocks[i]);
+			ret = PTR_ERR(clk);
+			goto error;
+		}
+
+		ret = pm_clk_add_clk(dev, clk);
+		if (ret) {
+			dev_err(dev, "failed to add clock at index %d\n", i);
+			clk_put(clk);
+			goto error;
+		}
+	}
+
+	return 0;
+
+error:
+	pm_clk_destroy(dev);
+
+	return ret;
+}
+
+static int gic_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct gic_clk_data *data;
+	struct gic_chip_data *gic;
+	int ret, irq;
+
+	data = of_device_get_match_data(&pdev->dev);
+	if (!data) {
+		dev_err(&pdev->dev, "no device match found\n");
+		return -ENODEV;
+	}
+
+	irq = irq_of_parse_and_map(dev->of_node, 0);
+	if (!irq) {
+		dev_err(dev, "no parent interrupt found!\n");
+		return -EINVAL;
+	}
+
+	ret = gic_get_clocks(dev, data);
+	if (ret)
+		goto irq_dispose;
+
+	pm_runtime_enable(dev);
+
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0)
+		goto rpm_disable;
+
+	ret = gic_of_init_child(dev, &gic, irq);
+	if (ret)
+		goto rpm_put;
+
+	platform_set_drvdata(pdev, gic);
+
+	pm_runtime_put(dev);
+
+	dev_info(dev, "GIC IRQ controller registered\n");
+
+	return 0;
+
+rpm_put:
+	pm_runtime_put_sync(dev);
+rpm_disable:
+	pm_runtime_disable(dev);
+	pm_clk_destroy(dev);
+irq_dispose:
+	irq_dispose_mapping(irq);
+
+	return ret;
+}
+
+static const struct dev_pm_ops gic_pm_ops = {
+	SET_RUNTIME_PM_OPS(gic_runtime_suspend,
+			   gic_runtime_resume, NULL)
+};
+
+static const char * const gic400_clocks[] = {
+	"clk",
+};
+
+static const struct gic_clk_data gic400_data = {
+	.num_clocks = ARRAY_SIZE(gic400_clocks),
+	.clocks = gic400_clocks,
+};
+
+static const struct of_device_id gic_match[] = {
+	{ .compatible = "nvidia,tegra210-agic",	.data = &gic400_data },
+	{},
+};
+MODULE_DEVICE_TABLE(of, gic_match);
+
+static struct platform_driver gic_driver = {
+	.probe		= gic_probe,
+	.driver		= {
+		.name	= "gic",
+		.of_match_table	= gic_match,
+		.pm	= &gic_pm_ops,
+	}
+};
+
+builtin_platform_driver(gic_driver);
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 3d3ab9045244..f15b7d1a66d3 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -75,7 +75,7 @@ struct gic_chip_data {
 	void __iomem *raw_dist_base;
 	void __iomem *raw_cpu_base;
 	u32 percpu_offset;
-#ifdef CONFIG_CPU_PM
+#if defined(CONFIG_CPU_PM) || defined(CONFIG_ARM_GIC_PM)
 	u32 saved_spi_enable[DIV_ROUND_UP(1020, 32)];
 	u32 saved_spi_active[DIV_ROUND_UP(1020, 32)];
 	u32 saved_spi_conf[DIV_ROUND_UP(1020, 16)];
@@ -528,7 +528,7 @@ int gic_cpu_if_down(unsigned int gic_nr)
 	return 0;
 }
 
-#ifdef CONFIG_CPU_PM
+#if defined(CONFIG_CPU_PM) || defined(CONFIG_ARM_GIC_PM)
 /*
  * Saves the GIC distributor registers during suspend or idle.  Must be called
  * with interrupts disabled but before powering down the GIC.  After calling
@@ -1297,6 +1297,34 @@ error:
 	return -ENOMEM;
 }
 
+int gic_of_init_child(struct device *dev, struct gic_chip_data **gic, int irq)
+{
+	int ret;
+
+	if (!dev || !dev->of_node || !gic || !irq)
+		return -EINVAL;
+
+	*gic = devm_kzalloc(dev, sizeof(**gic), GFP_KERNEL);
+	if (!*gic)
+		return -ENOMEM;
+
+	gic_init_chip(*gic, dev, dev->of_node->name, false);
+
+	ret = gic_of_setup(*gic, dev->of_node);
+	if (ret)
+		return ret;
+
+	ret = gic_init_bases(*gic, -1, &dev->of_node->fwnode);
+	if (ret) {
+		gic_teardown(*gic);
+		return ret;
+	}
+
+	irq_set_chained_handler_and_data(irq, gic_handle_cascade_irq, *gic);
+
+	return 0;
+}
+
 static void __init gic_of_setup_kvm_info(struct device_node *node)
 {
 	int ret;
@@ -1376,7 +1404,11 @@ IRQCHIP_DECLARE(cortex_a7_gic, "arm,cortex-a7-gic", gic_of_init);
 IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
 IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
 IRQCHIP_DECLARE(pl390, "arm,pl390", gic_of_init);
-
+#else
+int gic_of_init_child(struct device *dev, struct gic_chip_data **gic, int irq)
+{
+	return -ENOTSUPP;
+}
 #endif
 
 #ifdef CONFIG_ACPI
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index ffcbd8b9a4ff..eafc965b3eb8 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -117,6 +117,12 @@ void gic_dist_restore(struct gic_chip_data *gic);
 int gic_of_init(struct device_node *node, struct device_node *parent);
 
 /*
+ * Initialises and registers a non-root or child GIC chip. Memory for
+ * the gic_chip_data structure is dynamically allocated.
+ */
+int gic_of_init_child(struct device *dev, struct gic_chip_data **gic, int irq);
+
+/*
  * Legacy platforms not converted to DT yet must use this to init
  * their GIC
  */
-- 
2.1.4