linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V4 0/9] Add support for Tegra210 AGIC
@ 2016-05-12 15:19 Jon Hunter
  2016-05-12 15:19 ` [PATCH V4 1/9] irqdomain: Fix handling of type settings for existing mappings Jon Hunter
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Jon Hunter @ 2016-05-12 15:19 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Stephen Warren, Thierry Reding
  Cc: Kevin Hilman, Geert Uytterhoeven, Grygorii Strashko,
	Lars-Peter Clausen, Linus Walleij, linux-tegra, devicetree,
	linux-kernel, 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 branch [0].

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
[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                       | 187 +++++++++++++++++++++
 drivers/irqchip/irq-gic.c                          | 136 ++++++++++-----
 drivers/irqchip/irq-gic.h                          |  30 ++++
 include/linux/irq.h                                |   4 +
 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, 451 insertions(+), 55 deletions(-)
 create mode 100644 drivers/irqchip/irq-gic-pm.c
 create mode 100644 drivers/irqchip/irq-gic.h

-- 
2.1.4

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH V4 1/9] irqdomain: Fix handling of type settings for existing mappings
  2016-05-12 15:19 [PATCH V4 0/9] Add support for Tegra210 AGIC Jon Hunter
@ 2016-05-12 15:19 ` Jon Hunter
  2016-05-12 15:19 ` [PATCH V4 2/9] genirq: Look-up trigger type if not specified by caller Jon Hunter
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Jon Hunter @ 2016-05-12 15:19 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Stephen Warren, Thierry Reding
  Cc: Kevin Hilman, Geert Uytterhoeven, Grygorii Strashko,
	Lars-Peter Clausen, Linus Walleij, linux-tegra, devicetree,
	linux-kernel, 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 503c5b9dd030..a69e67ab99ae 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -593,15 +593,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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V4 2/9] genirq: Look-up trigger type if not specified by caller
  2016-05-12 15:19 [PATCH V4 0/9] Add support for Tegra210 AGIC Jon Hunter
  2016-05-12 15:19 ` [PATCH V4 1/9] irqdomain: Fix handling of type settings for existing mappings Jon Hunter
@ 2016-05-12 15:19 ` Jon Hunter
  2016-06-04  9:41   ` Marc Zyngier
  2016-05-12 15:19 ` [PATCH V4 3/9] irqdomain: Don't set type when mapping an IRQ Jon Hunter
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Jon Hunter @ 2016-05-12 15:19 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Stephen Warren, Thierry Reding
  Cc: Kevin Hilman, Geert Uytterhoeven, Grygorii Strashko,
	Lars-Peter Clausen, Linus Walleij, linux-tegra, devicetree,
	linux-kernel, 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>
---
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V4 3/9] irqdomain: Don't set type when mapping an IRQ
  2016-05-12 15:19 [PATCH V4 0/9] Add support for Tegra210 AGIC Jon Hunter
  2016-05-12 15:19 ` [PATCH V4 1/9] irqdomain: Fix handling of type settings for existing mappings Jon Hunter
  2016-05-12 15:19 ` [PATCH V4 2/9] genirq: Look-up trigger type if not specified by caller Jon Hunter
@ 2016-05-12 15:19 ` Jon Hunter
  2016-06-04  9:51   ` Marc Zyngier
  2016-05-12 15:19 ` [PATCH V4 4/9] genirq: Add runtime power management support for IRQ chips Jon Hunter
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Jon Hunter @ 2016-05-12 15:19 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Stephen Warren, Thierry Reding
  Cc: Kevin Hilman, Geert Uytterhoeven, Grygorii Strashko,
	Lars-Peter Clausen, Linus Walleij, linux-tegra, devicetree,
	linux-kernel, 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>
---
 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 a69e67ab99ae..91e0a8646c8e 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -572,6 +572,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;
@@ -619,7 +620,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;
 		}
 
@@ -639,10 +644,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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V4 4/9] genirq: Add runtime power management support for IRQ chips
  2016-05-12 15:19 [PATCH V4 0/9] Add support for Tegra210 AGIC Jon Hunter
                   ` (2 preceding siblings ...)
  2016-05-12 15:19 ` [PATCH V4 3/9] irqdomain: Don't set type when mapping an IRQ Jon Hunter
@ 2016-05-12 15:19 ` Jon Hunter
  2016-06-04  9:53   ` Marc Zyngier
  2016-05-12 15:19 ` [PATCH V4 5/9] irqchip/gic: Isolate early GIC initialisation code Jon Hunter
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Jon Hunter @ 2016-05-12 15:19 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Stephen Warren, Thierry Reding
  Cc: Kevin Hilman, Geert Uytterhoeven, Grygorii Strashko,
	Lars-Peter Clausen, Linus Walleij, linux-tegra, devicetree,
	linux-kernel, 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>
---
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V4 5/9] irqchip/gic: Isolate early GIC initialisation code
  2016-05-12 15:19 [PATCH V4 0/9] Add support for Tegra210 AGIC Jon Hunter
                   ` (3 preceding siblings ...)
  2016-05-12 15:19 ` [PATCH V4 4/9] genirq: Add runtime power management support for IRQ chips Jon Hunter
@ 2016-05-12 15:19 ` Jon Hunter
  2016-05-12 15:19 ` [PATCH V4 6/9] irqchip/gic: Add helper function for chip initialisation Jon Hunter
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Jon Hunter @ 2016-05-12 15:19 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Stephen Warren, Thierry Reding
  Cc: Kevin Hilman, Geert Uytterhoeven, Grygorii Strashko,
	Lars-Peter Clausen, Linus Walleij, linux-tegra, devicetree,
	linux-kernel, 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 113e2d02c812..58ef17fa23a3 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1029,14 +1029,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;
 
 	gic_check_cpu_features();
 
@@ -1137,23 +1134,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)
@@ -1176,6 +1156,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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V4 6/9] irqchip/gic: Add helper function for chip initialisation
  2016-05-12 15:19 [PATCH V4 0/9] Add support for Tegra210 AGIC Jon Hunter
                   ` (4 preceding siblings ...)
  2016-05-12 15:19 ` [PATCH V4 5/9] irqchip/gic: Isolate early GIC initialisation code Jon Hunter
@ 2016-05-12 15:19 ` Jon Hunter
  2016-05-12 15:19 ` [PATCH V4 7/9] irqchip/gic: Prepare for adding platform driver Jon Hunter
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Jon Hunter @ 2016-05-12 15:19 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Stephen Warren, Thierry Reding
  Cc: Kevin Hilman, Geert Uytterhoeven, Grygorii Strashko,
	Lars-Peter Clausen, Linus Walleij, linux-tegra, devicetree,
	linux-kernel, 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 | 45 +++++++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 16 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 58ef17fa23a3..e043a19631b1 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1029,31 +1029,33 @@ 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;
-
-	gic_check_cpu_features();
-
 	/* 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;
+
+	gic_check_cpu_features();
 
 	if (IS_ENABLED(CONFIG_GIC_NON_BANKED) && gic->percpu_offset) {
 		/* Frankein-GIC without banked registers... */
@@ -1151,8 +1153,6 @@ error:
 		free_percpu(gic->cpu_base.percpu_base);
 	}
 
-	kfree(gic->chip.name);
-
 	return ret;
 }
 
@@ -1160,7 +1160,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;
@@ -1182,7 +1183,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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V4 7/9] irqchip/gic: Prepare for adding platform driver
  2016-05-12 15:19 [PATCH V4 0/9] Add support for Tegra210 AGIC Jon Hunter
                   ` (5 preceding siblings ...)
  2016-05-12 15:19 ` [PATCH V4 6/9] irqchip/gic: Add helper function for chip initialisation Jon Hunter
@ 2016-05-12 15:19 ` Jon Hunter
  2016-06-04 10:06   ` Marc Zyngier
  2016-05-12 15:19 ` [PATCH V4 8/9] dt-bindings: arm-gic: Add documentation for Tegra210 AGIC Jon Hunter
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Jon Hunter @ 2016-05-12 15:19 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Stephen Warren, Thierry Reding
  Cc: Kevin Hilman, Geert Uytterhoeven, Grygorii Strashko,
	Lars-Peter Clausen, Linus Walleij, linux-tegra, devicetree,
	linux-kernel, 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 a
    local 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        | 18 ++++++++++--------
 drivers/irqchip/irq-gic.h        | 28 ++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+), 10 deletions(-)
 create mode 100644 drivers/irqchip/irq-gic.h

diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
index 97c0028e8388..c510cbd264b1 100644
--- a/drivers/irqchip/irq-gic-common.c
+++ b/drivers/irqchip/irq-gic-common.c
@@ -77,8 +77,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 e043a19631b1..9f8ab124898b 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -48,6 +48,7 @@
 #include <asm/smp_plat.h>
 #include <asm/virt.h>
 
+#include "irq-gic.h"
 #include "irq-gic-common.h"
 
 #ifdef CONFIG_ARM64
@@ -363,7 +364,7 @@ static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
 	} while (1);
 }
 
-static void gic_handle_cascade_irq(struct irq_desc *desc)
+void gic_handle_cascade_irq(struct irq_desc *desc)
 {
 	struct gic_chip_data *chip_data = irq_desc_get_handler_data(desc);
 	struct irq_chip *chip = irq_desc_get_chip(desc);
@@ -447,7 +448,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;
@@ -532,7 +533,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;
@@ -571,7 +572,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;
@@ -617,7 +618,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;
@@ -647,7 +648,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;
@@ -724,7 +725,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));
@@ -754,7 +755,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;
 }
@@ -1178,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/drivers/irqchip/irq-gic.h b/drivers/irqchip/irq-gic.h
new file mode 100644
index 000000000000..646e92614b2c
--- /dev/null
+++ b/drivers/irqchip/irq-gic.h
@@ -0,0 +1,28 @@
+/*
+ * 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/>.
+ */
+
+#ifndef _IRQ_GIC_H
+#define _IRQ_GIC_H
+
+struct gic_chip_data;
+
+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);
+void gic_handle_cascade_irq(struct irq_desc *desc);
+
+#endif /* _IRQ_GIC_H */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V4 8/9] dt-bindings: arm-gic: Add documentation for Tegra210 AGIC
  2016-05-12 15:19 [PATCH V4 0/9] Add support for Tegra210 AGIC Jon Hunter
                   ` (6 preceding siblings ...)
  2016-05-12 15:19 ` [PATCH V4 7/9] irqchip/gic: Prepare for adding platform driver Jon Hunter
@ 2016-05-12 15:19 ` Jon Hunter
  2016-05-16 16:05   ` Rob Herring
  2016-05-12 15:19 ` [PATCH V4 9/9] irqchip/gic: Add platform driver for non-root GICs that require RPM Jon Hunter
  2016-05-20  9:07 ` [PATCH V4 0/9] Add support for Tegra210 AGIC Jon Hunter
  9 siblings, 1 reply; 19+ messages in thread
From: Jon Hunter @ 2016-05-12 15:19 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Stephen Warren, Thierry Reding
  Cc: Kevin Hilman, Geert Uytterhoeven, Grygorii Strashko,
	Lars-Peter Clausen, Linus Walleij, linux-tegra, devicetree,
	linux-kernel, Jon Hunter

The Tegra AGIC is an interrupt controller that is compatible with the
ARM GIC-400, however, it is a 2nd level controller and requires runtime
power-management because it is outside the main CPU complex. Ideally,
the AGIC would use the existing "arm,gic-400" compatibility string, but
because of the power management requirements it cannot be initialised
early and is initialised by a platform driver. Therefore, to distinguish
between the Tegra AGIC and a normal GIC-400, and to ensure that the
appropriate driver initialises the AGIC, 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>
---
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V4 9/9] irqchip/gic: Add platform driver for non-root GICs that require RPM
  2016-05-12 15:19 [PATCH V4 0/9] Add support for Tegra210 AGIC Jon Hunter
                   ` (7 preceding siblings ...)
  2016-05-12 15:19 ` [PATCH V4 8/9] dt-bindings: arm-gic: Add documentation for Tegra210 AGIC Jon Hunter
@ 2016-05-12 15:19 ` Jon Hunter
  2016-06-04 10:10   ` Marc Zyngier
  2016-05-20  9:07 ` [PATCH V4 0/9] Add support for Tegra210 AGIC Jon Hunter
  9 siblings, 1 reply; 19+ messages in thread
From: Jon Hunter @ 2016-05-12 15:19 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Stephen Warren, Thierry Reding
  Cc: Kevin Hilman, Geert Uytterhoeven, Grygorii Strashko,
	Lars-Peter Clausen, Linus Walleij, linux-tegra, devicetree,
	linux-kernel, 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, of_gic_init(), has been added which calls various private
functions to initialise the GIC.

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 | 187 +++++++++++++++++++++++++++++++++++++++++++
 drivers/irqchip/irq-gic.c    |  34 +++++++-
 drivers/irqchip/irq-gic.h    |   2 +
 5 files changed, 227 insertions(+), 3 deletions(-)
 create mode 100644 drivers/irqchip/irq-gic-pm.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 81f88ada3a61..68f65c84fdfd 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 f828244b44c2..eab28e80cd8d 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..a14331007bcf
--- /dev/null
+++ b/drivers/irqchip/irq-gic-pm.c
@@ -0,0 +1,187 @@
+/*
+ * 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/platform_device.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+
+#include "irq-gic.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 = of_gic_init(dev, &gic);
+	if (ret)
+		goto rpm_put;
+
+	irq_set_chained_handler_and_data(irq, gic_handle_cascade_irq, gic);
+
+	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 9f8ab124898b..e65090f7afa4 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -76,7 +76,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(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)];
@@ -526,7 +526,7 @@ int gic_cpu_if_down(unsigned int gic_nr)
 	return 0;
 }
 
-#ifdef CONFIG_CPU_PM
+#if defined(CONFIG_CPU_PM) || defined(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,30 @@ error:
 	return -ENOMEM;
 }
 
+int of_gic_init(struct device *dev, struct gic_chip_data **gic)
+{
+	int ret;
+
+	if (!dev || !dev->of_node || !gic)
+		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;
+}
+
 int __init
 gic_of_init(struct device_node *node, struct device_node *parent)
 {
@@ -1351,7 +1375,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 of_gic_init(struct device *dev, struct gic_chip_data **gic)
+{
+	return -ENOTSUPP;
+}
 #endif
 
 #ifdef CONFIG_ACPI
diff --git a/drivers/irqchip/irq-gic.h b/drivers/irqchip/irq-gic.h
index 646e92614b2c..b03ec4bac795 100644
--- a/drivers/irqchip/irq-gic.h
+++ b/drivers/irqchip/irq-gic.h
@@ -25,4 +25,6 @@ void gic_dist_save(struct gic_chip_data *gic);
 void gic_dist_restore(struct gic_chip_data *gic);
 void gic_handle_cascade_irq(struct irq_desc *desc);
 
+int of_gic_init(struct device *dev, struct gic_chip_data **gic);
+
 #endif /* _IRQ_GIC_H */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH V4 8/9] dt-bindings: arm-gic: Add documentation for Tegra210 AGIC
  2016-05-12 15:19 ` [PATCH V4 8/9] dt-bindings: arm-gic: Add documentation for Tegra210 AGIC Jon Hunter
@ 2016-05-16 16:05   ` Rob Herring
  0 siblings, 0 replies; 19+ messages in thread
From: Rob Herring @ 2016-05-16 16:05 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Stephen Warren,
	Thierry Reding, Kevin Hilman, Geert Uytterhoeven,
	Grygorii Strashko, Lars-Peter Clausen, Linus Walleij,
	linux-tegra, devicetree, linux-kernel

On Thu, May 12, 2016 at 04:19:31PM +0100, Jon Hunter wrote:
> The Tegra AGIC is an interrupt controller that is compatible with the
> ARM GIC-400, however, it is a 2nd level controller and requires runtime
> power-management because it is outside the main CPU complex. Ideally,
> the AGIC would use the existing "arm,gic-400" compatibility string, but
> because of the power management requirements it cannot be initialised
> early and is initialised by a platform driver. Therefore, to distinguish
> between the Tegra AGIC and a normal GIC-400, and to ensure that the
> appropriate driver initialises the AGIC, 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>
> ---
>  Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V4 0/9] Add support for Tegra210 AGIC
  2016-05-12 15:19 [PATCH V4 0/9] Add support for Tegra210 AGIC Jon Hunter
                   ` (8 preceding siblings ...)
  2016-05-12 15:19 ` [PATCH V4 9/9] irqchip/gic: Add platform driver for non-root GICs that require RPM Jon Hunter
@ 2016-05-20  9:07 ` Jon Hunter
  9 siblings, 0 replies; 19+ messages in thread
From: Jon Hunter @ 2016-05-20  9:07 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Stephen Warren, Thierry Reding
  Cc: Kevin Hilman, Geert Uytterhoeven, Grygorii Strashko,
	Lars-Peter Clausen, Linus Walleij, linux-tegra, devicetree,
	linux-kernel

Hi Marc,

Let me know if you have any thoughts on this.

Cheers
Jon

On 12/05/16 16:19, Jon Hunter wrote:
> 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 branch [0].
> 
> 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
> [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                       | 187 +++++++++++++++++++++
>  drivers/irqchip/irq-gic.c                          | 136 ++++++++++-----
>  drivers/irqchip/irq-gic.h                          |  30 ++++
>  include/linux/irq.h                                |   4 +
>  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, 451 insertions(+), 55 deletions(-)
>  create mode 100644 drivers/irqchip/irq-gic-pm.c
>  create mode 100644 drivers/irqchip/irq-gic.h
> 

-- 
nvpublic

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V4 2/9] genirq: Look-up trigger type if not specified by caller
  2016-05-12 15:19 ` [PATCH V4 2/9] genirq: Look-up trigger type if not specified by caller Jon Hunter
@ 2016-06-04  9:41   ` Marc Zyngier
  0 siblings, 0 replies; 19+ messages in thread
From: Marc Zyngier @ 2016-06-04  9:41 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Thomas Gleixner, Jason Cooper, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Stephen Warren,
	Thierry Reding, Kevin Hilman, Geert Uytterhoeven,
	Grygorii Strashko, Lars-Peter Clausen, Linus Walleij,
	linux-tegra, devicetree, linux-kernel

On Thu, 12 May 2016 16:19:25 +0100
Jon Hunter <jonathanh@nvidia.com> wrote:

> 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>

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V4 3/9] irqdomain: Don't set type when mapping an IRQ
  2016-05-12 15:19 ` [PATCH V4 3/9] irqdomain: Don't set type when mapping an IRQ Jon Hunter
@ 2016-06-04  9:51   ` Marc Zyngier
  0 siblings, 0 replies; 19+ messages in thread
From: Marc Zyngier @ 2016-06-04  9:51 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Thomas Gleixner, Jason Cooper, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Stephen Warren,
	Thierry Reding, Kevin Hilman, Geert Uytterhoeven,
	Grygorii Strashko, Lars-Peter Clausen, Linus Walleij,
	linux-tegra, devicetree, linux-kernel

On Thu, 12 May 2016 16:19:26 +0100
Jon Hunter <jonathanh@nvidia.com> wrote:

> 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>

As discussed just before the 4.7 merge window, this patch is going to
uncover quite a few broken configurations (the main one being the PPI
triggers for the ARM architected timer, which seems to be wrong on a lot
of different platforms).

I'm proposing to put this patch[1] in the same queue in order to
uncover the guilty ones, but we have to be prepared for more breakage
(I've also identified kvmtool as a sinner).

Thanks,

	M.
[1] https://lkml.org/lkml/2016/5/9/546
-- 
Jazz is not dead. It just smells funny.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V4 4/9] genirq: Add runtime power management support for IRQ chips
  2016-05-12 15:19 ` [PATCH V4 4/9] genirq: Add runtime power management support for IRQ chips Jon Hunter
@ 2016-06-04  9:53   ` Marc Zyngier
  0 siblings, 0 replies; 19+ messages in thread
From: Marc Zyngier @ 2016-06-04  9:53 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Thomas Gleixner, Jason Cooper, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Stephen Warren,
	Thierry Reding, Kevin Hilman, Geert Uytterhoeven,
	Grygorii Strashko, Lars-Peter Clausen, Linus Walleij,
	linux-tegra, devicetree, linux-kernel

On Thu, 12 May 2016 16:19:27 +0100
Jon Hunter <jonathanh@nvidia.com> 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>

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V4 7/9] irqchip/gic: Prepare for adding platform driver
  2016-05-12 15:19 ` [PATCH V4 7/9] irqchip/gic: Prepare for adding platform driver Jon Hunter
@ 2016-06-04 10:06   ` Marc Zyngier
  2016-06-06  8:24     ` Jon Hunter
  0 siblings, 1 reply; 19+ messages in thread
From: Marc Zyngier @ 2016-06-04 10:06 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Thomas Gleixner, Jason Cooper, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Stephen Warren,
	Thierry Reding, Kevin Hilman, Geert Uytterhoeven,
	Grygorii Strashko, Lars-Peter Clausen, Linus Walleij,
	linux-tegra, devicetree, linux-kernel

On Thu, 12 May 2016 16:19:30 +0100
Jon Hunter <jonathanh@nvidia.com> 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 a
>     local 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        | 18 ++++++++++--------
>  drivers/irqchip/irq-gic.h        | 28 ++++++++++++++++++++++++++++
>  3 files changed, 40 insertions(+), 10 deletions(-)
>  create mode 100644 drivers/irqchip/irq-gic.h
> 
> diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
> index 97c0028e8388..c510cbd264b1 100644
> --- a/drivers/irqchip/irq-gic-common.c
> +++ b/drivers/irqchip/irq-gic-common.c
> @@ -77,8 +77,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 e043a19631b1..9f8ab124898b 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -48,6 +48,7 @@
>  #include <asm/smp_plat.h>
>  #include <asm/virt.h>
>  
> +#include "irq-gic.h"
>  #include "irq-gic-common.h"
>  
>  #ifdef CONFIG_ARM64
> @@ -363,7 +364,7 @@ static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
>  	} while (1);
>  }
>  
> -static void gic_handle_cascade_irq(struct irq_desc *desc)
> +void gic_handle_cascade_irq(struct irq_desc *desc)
>  {
>  	struct gic_chip_data *chip_data = irq_desc_get_handler_data(desc);
>  	struct irq_chip *chip = irq_desc_get_chip(desc);
> @@ -447,7 +448,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;
> @@ -532,7 +533,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;
> @@ -571,7 +572,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;
> @@ -617,7 +618,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;
> @@ -647,7 +648,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;
> @@ -724,7 +725,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));
> @@ -754,7 +755,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;
>  }
> @@ -1178,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/drivers/irqchip/irq-gic.h b/drivers/irqchip/irq-gic.h
> new file mode 100644
> index 000000000000..646e92614b2c
> --- /dev/null
> +++ b/drivers/irqchip/irq-gic.h
> @@ -0,0 +1,28 @@
> +/*
> + * 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/>.
> + */
> +
> +#ifndef _IRQ_GIC_H
> +#define _IRQ_GIC_H
> +
> +struct gic_chip_data;
> +
> +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);
> +void gic_handle_cascade_irq(struct irq_desc *desc);
> +
> +#endif /* _IRQ_GIC_H */

Any reason why all of this is not part of include/irqchip/arm-gic.h
instead of creating yet another include file?

Thanks,

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V4 9/9] irqchip/gic: Add platform driver for non-root GICs that require RPM
  2016-05-12 15:19 ` [PATCH V4 9/9] irqchip/gic: Add platform driver for non-root GICs that require RPM Jon Hunter
@ 2016-06-04 10:10   ` Marc Zyngier
  2016-06-06  8:26     ` Jon Hunter
  0 siblings, 1 reply; 19+ messages in thread
From: Marc Zyngier @ 2016-06-04 10:10 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Thomas Gleixner, Jason Cooper, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Stephen Warren,
	Thierry Reding, Kevin Hilman, Geert Uytterhoeven,
	Grygorii Strashko, Lars-Peter Clausen, Linus Walleij,
	linux-tegra, devicetree, linux-kernel

On Thu, 12 May 2016 16:19:32 +0100
Jon Hunter <jonathanh@nvidia.com> wrote:

> 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, of_gic_init(), has been added which calls various private
> functions to initialise the GIC.
> 
> 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 | 187 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/irqchip/irq-gic.c    |  34 +++++++-
>  drivers/irqchip/irq-gic.h    |   2 +
>  5 files changed, 227 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/irqchip/irq-gic-pm.c
> 
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 81f88ada3a61..68f65c84fdfd 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 f828244b44c2..eab28e80cd8d 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..a14331007bcf
> --- /dev/null
> +++ b/drivers/irqchip/irq-gic-pm.c
> @@ -0,0 +1,187 @@
> +/*
> + * 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/platform_device.h>
> +#include <linux/pm_clock.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +
> +#include "irq-gic.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 = of_gic_init(dev, &gic);
> +	if (ret)
> +		goto rpm_put;
> +
> +	irq_set_chained_handler_and_data(irq, gic_handle_cascade_irq, gic);

nit: Rather than exporting the gic_handle_cascade entry point, I'd
rather you exported a helper from the GIC driver. Something like:

void gic_setup_cascade(int irq, struct gic_data *gic);

and do the handler registration there. Alternatively, you could also
move the irq parsing and handler registration into of_gic_init, and
have this single initialization entry point.

> +
> +	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 9f8ab124898b..e65090f7afa4 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -76,7 +76,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(ARM_GIC_PM)

Shouldn't that be 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)];
> @@ -526,7 +526,7 @@ int gic_cpu_if_down(unsigned int gic_nr)
>  	return 0;
>  }
>  
> -#ifdef CONFIG_CPU_PM
> +#if defined(CONFIG_CPU_PM) || defined(ARM_GIC_PM)

Same here?

>  /*
>   * 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,30 @@ error:
>  	return -ENOMEM;
>  }
>  
> +int of_gic_init(struct device *dev, struct gic_chip_data **gic)
> +{
> +	int ret;
> +
> +	if (!dev || !dev->of_node || !gic)
> +		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;
> +}
> +
>  int __init
>  gic_of_init(struct device_node *node, struct device_node *parent)
>  {
> @@ -1351,7 +1375,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 of_gic_init(struct device *dev, struct gic_chip_data **gic)
> +{
> +	return -ENOTSUPP;
> +}
>  #endif
>  
>  #ifdef CONFIG_ACPI
> diff --git a/drivers/irqchip/irq-gic.h b/drivers/irqchip/irq-gic.h
> index 646e92614b2c..b03ec4bac795 100644
> --- a/drivers/irqchip/irq-gic.h
> +++ b/drivers/irqchip/irq-gic.h
> @@ -25,4 +25,6 @@ void gic_dist_save(struct gic_chip_data *gic);
>  void gic_dist_restore(struct gic_chip_data *gic);
>  void gic_handle_cascade_irq(struct irq_desc *desc);
>  
> +int of_gic_init(struct device *dev, struct gic_chip_data **gic);
> +
>  #endif /* _IRQ_GIC_H */

Otherwise, looks good.

Thanks,

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V4 7/9] irqchip/gic: Prepare for adding platform driver
  2016-06-04 10:06   ` Marc Zyngier
@ 2016-06-06  8:24     ` Jon Hunter
  0 siblings, 0 replies; 19+ messages in thread
From: Jon Hunter @ 2016-06-06  8:24 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Thomas Gleixner, Jason Cooper, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Stephen Warren,
	Thierry Reding, Kevin Hilman, Geert Uytterhoeven,
	Grygorii Strashko, Lars-Peter Clausen, Linus Walleij,
	linux-tegra, devicetree, linux-kernel


On 04/06/16 11:06, Marc Zyngier wrote:
> On Thu, 12 May 2016 16:19:30 +0100
> Jon Hunter <jonathanh@nvidia.com> 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 a
>>     local 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        | 18 ++++++++++--------
>>  drivers/irqchip/irq-gic.h        | 28 ++++++++++++++++++++++++++++
>>  3 files changed, 40 insertions(+), 10 deletions(-)
>>  create mode 100644 drivers/irqchip/irq-gic.h
>>
>> diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
>> index 97c0028e8388..c510cbd264b1 100644
>> --- a/drivers/irqchip/irq-gic-common.c
>> +++ b/drivers/irqchip/irq-gic-common.c
>> @@ -77,8 +77,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 e043a19631b1..9f8ab124898b 100644
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
>> @@ -48,6 +48,7 @@
>>  #include <asm/smp_plat.h>
>>  #include <asm/virt.h>
>>  
>> +#include "irq-gic.h"
>>  #include "irq-gic-common.h"
>>  
>>  #ifdef CONFIG_ARM64
>> @@ -363,7 +364,7 @@ static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
>>  	} while (1);
>>  }
>>  
>> -static void gic_handle_cascade_irq(struct irq_desc *desc)
>> +void gic_handle_cascade_irq(struct irq_desc *desc)
>>  {
>>  	struct gic_chip_data *chip_data = irq_desc_get_handler_data(desc);
>>  	struct irq_chip *chip = irq_desc_get_chip(desc);
>> @@ -447,7 +448,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;
>> @@ -532,7 +533,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;
>> @@ -571,7 +572,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;
>> @@ -617,7 +618,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;
>> @@ -647,7 +648,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;
>> @@ -724,7 +725,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));
>> @@ -754,7 +755,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;
>>  }
>> @@ -1178,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/drivers/irqchip/irq-gic.h b/drivers/irqchip/irq-gic.h
>> new file mode 100644
>> index 000000000000..646e92614b2c
>> --- /dev/null
>> +++ b/drivers/irqchip/irq-gic.h
>> @@ -0,0 +1,28 @@
>> +/*
>> + * 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/>.
>> + */
>> +
>> +#ifndef _IRQ_GIC_H
>> +#define _IRQ_GIC_H
>> +
>> +struct gic_chip_data;
>> +
>> +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);
>> +void gic_handle_cascade_irq(struct irq_desc *desc);
>> +
>> +#endif /* _IRQ_GIC_H */
> 
> Any reason why all of this is not part of include/irqchip/arm-gic.h
> instead of creating yet another include file?

No not really, I can definitely move to that header and avoid adding
this one.

Cheers
Jon


-- 
nvpublic

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V4 9/9] irqchip/gic: Add platform driver for non-root GICs that require RPM
  2016-06-04 10:10   ` Marc Zyngier
@ 2016-06-06  8:26     ` Jon Hunter
  0 siblings, 0 replies; 19+ messages in thread
From: Jon Hunter @ 2016-06-06  8:26 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Thomas Gleixner, Jason Cooper, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Stephen Warren,
	Thierry Reding, Kevin Hilman, Geert Uytterhoeven,
	Grygorii Strashko, Lars-Peter Clausen, Linus Walleij,
	linux-tegra, devicetree, linux-kernel


On 04/06/16 11:10, Marc Zyngier wrote:
> On Thu, 12 May 2016 16:19:32 +0100
> Jon Hunter <jonathanh@nvidia.com> wrote:
> 
>> 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, of_gic_init(), has been added which calls various private
>> functions to initialise the GIC.
>>
>> 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 | 187 +++++++++++++++++++++++++++++++++++++++++++
>>  drivers/irqchip/irq-gic.c    |  34 +++++++-
>>  drivers/irqchip/irq-gic.h    |   2 +
>>  5 files changed, 227 insertions(+), 3 deletions(-)
>>  create mode 100644 drivers/irqchip/irq-gic-pm.c
>>
>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>> index 81f88ada3a61..68f65c84fdfd 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 f828244b44c2..eab28e80cd8d 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..a14331007bcf
>> --- /dev/null
>> +++ b/drivers/irqchip/irq-gic-pm.c
>> @@ -0,0 +1,187 @@
>> +/*
>> + * 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/platform_device.h>
>> +#include <linux/pm_clock.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/slab.h>
>> +
>> +#include "irq-gic.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 = of_gic_init(dev, &gic);
>> +	if (ret)
>> +		goto rpm_put;
>> +
>> +	irq_set_chained_handler_and_data(irq, gic_handle_cascade_irq, gic);
> 
> nit: Rather than exporting the gic_handle_cascade entry point, I'd
> rather you exported a helper from the GIC driver. Something like:
> 
> void gic_setup_cascade(int irq, struct gic_data *gic);
> 
> and do the handler registration there. Alternatively, you could also
> move the irq parsing and handler registration into of_gic_init, and
> have this single initialization entry point.

Ok, yes I will do the latter.

>> +
>> +	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 9f8ab124898b..e65090f7afa4 100644
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
>> @@ -76,7 +76,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(ARM_GIC_PM)
> 
> Shouldn't that be CONFIG_ARM_GIC_PM?


Oops yes, will fix.

>>  	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)];
>> @@ -526,7 +526,7 @@ int gic_cpu_if_down(unsigned int gic_nr)
>>  	return 0;
>>  }
>>  
>> -#ifdef CONFIG_CPU_PM
>> +#if defined(CONFIG_CPU_PM) || defined(ARM_GIC_PM)
> 
> Same here?

Indeed! Cheers.

>>  /*
>>   * 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,30 @@ error:
>>  	return -ENOMEM;
>>  }
>>  
>> +int of_gic_init(struct device *dev, struct gic_chip_data **gic)
>> +{
>> +	int ret;
>> +
>> +	if (!dev || !dev->of_node || !gic)
>> +		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;
>> +}
>> +
>>  int __init
>>  gic_of_init(struct device_node *node, struct device_node *parent)
>>  {
>> @@ -1351,7 +1375,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 of_gic_init(struct device *dev, struct gic_chip_data **gic)
>> +{
>> +	return -ENOTSUPP;
>> +}
>>  #endif
>>  
>>  #ifdef CONFIG_ACPI
>> diff --git a/drivers/irqchip/irq-gic.h b/drivers/irqchip/irq-gic.h
>> index 646e92614b2c..b03ec4bac795 100644
>> --- a/drivers/irqchip/irq-gic.h
>> +++ b/drivers/irqchip/irq-gic.h
>> @@ -25,4 +25,6 @@ void gic_dist_save(struct gic_chip_data *gic);
>>  void gic_dist_restore(struct gic_chip_data *gic);
>>  void gic_handle_cascade_irq(struct irq_desc *desc);
>>  
>> +int of_gic_init(struct device *dev, struct gic_chip_data **gic);
>> +
>>  #endif /* _IRQ_GIC_H */
> 
> Otherwise, looks good.

Thanks!
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-06-06  8:29 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-12 15:19 [PATCH V4 0/9] Add support for Tegra210 AGIC Jon Hunter
2016-05-12 15:19 ` [PATCH V4 1/9] irqdomain: Fix handling of type settings for existing mappings Jon Hunter
2016-05-12 15:19 ` [PATCH V4 2/9] genirq: Look-up trigger type if not specified by caller Jon Hunter
2016-06-04  9:41   ` Marc Zyngier
2016-05-12 15:19 ` [PATCH V4 3/9] irqdomain: Don't set type when mapping an IRQ Jon Hunter
2016-06-04  9:51   ` Marc Zyngier
2016-05-12 15:19 ` [PATCH V4 4/9] genirq: Add runtime power management support for IRQ chips Jon Hunter
2016-06-04  9:53   ` Marc Zyngier
2016-05-12 15:19 ` [PATCH V4 5/9] irqchip/gic: Isolate early GIC initialisation code Jon Hunter
2016-05-12 15:19 ` [PATCH V4 6/9] irqchip/gic: Add helper function for chip initialisation Jon Hunter
2016-05-12 15:19 ` [PATCH V4 7/9] irqchip/gic: Prepare for adding platform driver Jon Hunter
2016-06-04 10:06   ` Marc Zyngier
2016-06-06  8:24     ` Jon Hunter
2016-05-12 15:19 ` [PATCH V4 8/9] dt-bindings: arm-gic: Add documentation for Tegra210 AGIC Jon Hunter
2016-05-16 16:05   ` Rob Herring
2016-05-12 15:19 ` [PATCH V4 9/9] irqchip/gic: Add platform driver for non-root GICs that require RPM Jon Hunter
2016-06-04 10:10   ` Marc Zyngier
2016-06-06  8:26     ` Jon Hunter
2016-05-20  9:07 ` [PATCH V4 0/9] Add support for Tegra210 AGIC Jon Hunter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).