[PATCH V3 0/2] irqchip: qcom: Add IRQ combiner driver

[PATCH V3 0/2] irqchip: qcom: Add IRQ combiner driver

[Agustin Vega-Frias]

Add support for IRQ combiners in the Top-level Control and Status
Registers (TCSR) hardware block in Qualcomm Technologies chips.

The first patch adds support for ResourceSource/IRQ domain mapping
when using Extended IRQ Resources with a specific ResourceSource.
The core ACPI resource management code has been changed to lookup
the IRQ domain when an IRQ resource indicates a ResourceSource,
and register the IRQ on that domain, instead of a GSI.

The second patch takes advantage of the new capabilities to implement
the driver for the IRQ combiners.

Changes V1 -> V2:
* Remove use of GPIO library for the combiner
* Refactor to use fwnode/ResourceSource to IRQ domain mapping
  introduced in ACPI core

Changes V2 -> V3:
* Removed parsing of _PRS to find IRQs
* Removed acpi_irq_domain_create and acpi_irq_domain_remove

Agustin Vega-Frias (2):
  ACPI: Add support for ResourceSource/IRQ domain mapping
  irqchip: qcom: Add IRQ combiner driver

 drivers/acpi/Makefile               |   1 +
 drivers/acpi/irqdomain.c            | 108 +++++++++++
 drivers/acpi/resource.c             |  23 ++-
 drivers/irqchip/Kconfig             |   8 +
 drivers/irqchip/Makefile            |   1 +
 drivers/irqchip/qcom-irq-combiner.c | 344 ++++++++++++++++++++++++++++++++++++
 include/linux/acpi.h                |   6 +
 7 files changed, 483 insertions(+), 8 deletions(-)
 create mode 100644 drivers/acpi/irqdomain.c
 create mode 100644 drivers/irqchip/qcom-irq-combiner.c

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

[PATCH V3 1/2] ACPI: Add support for ResourceSource/IRQ domain mapping

[Agustin Vega-Frias]

This allows irqchip drivers to associate an ACPI DSDT device to
an IRQ domain and provides support for using the ResourceSource
in Extended IRQ Resources to find the domain and map the IRQs
specified on that domain.

Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
---
 drivers/acpi/Makefile    |   1 +
 drivers/acpi/irqdomain.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/resource.c  |  23 ++++++----
 include/linux/acpi.h     |   6 +++
 4 files changed, 130 insertions(+), 8 deletions(-)
 create mode 100644 drivers/acpi/irqdomain.c

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index b12fa64..79db78f 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -56,6 +56,7 @@ acpi-$(CONFIG_ACPI_NUMA)	+= numa.o
 acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
 acpi-y				+= acpi_lpat.o
 acpi-$(CONFIG_ACPI_GENERIC_GSI) += gsi.o
+acpi-y				+= irqdomain.o
 
 # These are (potentially) separate modules
 
diff --git a/drivers/acpi/irqdomain.c b/drivers/acpi/irqdomain.c
new file mode 100644
index 0000000..0fd10a9
--- /dev/null
+++ b/drivers/acpi/irqdomain.c
@@ -0,0 +1,108 @@
+/*
+ * ACPI ResourceSource/IRQ domain mapping support
+ *
+ * Copyright (c) 2016, The Linux Foundation. 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 and
+ * only 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.
+ */
+#include <linux/acpi.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+
+/**
+ * acpi_irq_domain_register_irq() - Register the mapping for an IRQ produced
+ *                                  by the given acpi_resource_source to a
+ *                                  Linux IRQ number
+ * @source: IRQ source
+ * @rcirq: IRQ number
+ * @trigger: trigger type of the IRQ number to be mapped
+ * @polarity: polarity of the IRQ to be mapped
+ *
+ * Returns: a valid linux IRQ number on success
+ *          -ENODEV if the given acpi_resource_source cannot be found
+ *          -EPROBE_DEFER if the IRQ domain has not been registered
+ *          -EINVAL for all other errors
+ */
+int acpi_irq_domain_register_irq(struct acpi_resource_source *source, u32 rcirq,
+				 int trigger, int polarity)
+{
+	struct irq_domain *domain;
+	struct acpi_device *device;
+	acpi_handle handle;
+	acpi_status status;
+	unsigned int type;
+	int ret;
+
+	status = acpi_get_handle(NULL, source->string_ptr, &handle);
+	if (ACPI_FAILURE(status))
+		return -ENODEV;
+
+	device = acpi_bus_get_acpi_device(handle);
+	if (!device)
+		return -ENODEV;
+
+	domain = irq_find_matching_fwnode(&device->fwnode, DOMAIN_BUS_ANY);
+	if (!domain) {
+		ret = -EPROBE_DEFER;
+		goto out_put_device;
+	}
+
+	type = acpi_dev_get_irq_type(trigger, polarity);
+	ret = irq_create_mapping(domain, rcirq);
+	if (ret)
+		irq_set_irq_type(ret, type);
+
+out_put_device:
+	acpi_bus_put_acpi_device(device);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(acpi_irq_domain_register_irq);
+
+/**
+ * acpi_irq_domain_unregister_irq() - Delete the mapping for an IRQ produced
+ *                                    by the given acpi_resource_source to a
+ *                                    Linux IRQ number
+ * @source: IRQ source
+ * @rcirq: IRQ number
+ *
+ * Returns: 0 on success
+ *          -ENODEV if the given acpi_resource_source cannot be found
+ *          -EINVAL for all other errors
+ */
+int acpi_irq_domain_unregister_irq(struct acpi_resource_source *source,
+				   u32 rcirq)
+{
+	struct irq_domain *domain;
+	struct acpi_device *device;
+	acpi_handle handle;
+	acpi_status status;
+	int ret = 0;
+
+	status = acpi_get_handle(NULL, source->string_ptr, &handle);
+	if (ACPI_FAILURE(status))
+		return -ENODEV;
+
+	device = acpi_bus_get_acpi_device(handle);
+	if (!device)
+		return -ENODEV;
+
+	domain = irq_find_matching_fwnode(&device->fwnode, DOMAIN_BUS_ANY);
+	if (!domain) {
+		ret = -EINVAL;
+		goto out_put_device;
+	}
+
+	irq_dispose_mapping(irq_find_mapping(domain, rcirq));
+
+out_put_device:
+	acpi_bus_put_acpi_device(device);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(acpi_irq_domain_unregister_irq);
diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 56241eb..1551698 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -381,14 +381,15 @@ static void acpi_dev_irqresource_disabled(struct resource *res, u32 gsi)
 	res->flags = IORESOURCE_IRQ | IORESOURCE_DISABLED | IORESOURCE_UNSET;
 }
 
-static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
+static void acpi_dev_get_irqresource(struct resource *res, u32 rcirq,
+				     struct acpi_resource_source *source,
 				     u8 triggering, u8 polarity, u8 shareable,
 				     bool legacy)
 {
 	int irq, p, t;
 
-	if (!valid_IRQ(gsi)) {
-		acpi_dev_irqresource_disabled(res, gsi);
+	if ((source->string_length == 0) && !valid_IRQ(rcirq)) {
+		acpi_dev_irqresource_disabled(res, rcirq);
 		return;
 	}
 
@@ -402,12 +403,12 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
 	 * using extended IRQ descriptors we take the IRQ configuration
 	 * from _CRS directly.
 	 */
-	if (legacy && !acpi_get_override_irq(gsi, &t, &p)) {
+	if (legacy && !acpi_get_override_irq(rcirq, &t, &p)) {
 		u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
 		u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
 
 		if (triggering != trig || polarity != pol) {
-			pr_warning("ACPI: IRQ %d override to %s, %s\n", gsi,
+			pr_warning("ACPI: IRQ %d override to %s, %s\n", rcirq,
 				   t ? "level" : "edge", p ? "low" : "high");
 			triggering = trig;
 			polarity = pol;
@@ -415,12 +416,16 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
 	}
 
 	res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
-	irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
+	if (source->string_length > 0)
+		irq = acpi_irq_domain_register_irq(source, rcirq, triggering,
+						   polarity);
+	else
+		irq = acpi_register_gsi(NULL, rcirq, triggering, polarity);
 	if (irq >= 0) {
 		res->start = irq;
 		res->end = irq;
 	} else {
-		acpi_dev_irqresource_disabled(res, gsi);
+		acpi_dev_irqresource_disabled(res, rcirq);
 	}
 }
 
@@ -448,6 +453,7 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
 {
 	struct acpi_resource_irq *irq;
 	struct acpi_resource_extended_irq *ext_irq;
+	struct acpi_resource_source dummy = { 0, 0, NULL };
 
 	switch (ares->type) {
 	case ACPI_RESOURCE_TYPE_IRQ:
@@ -460,7 +466,7 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
 			acpi_dev_irqresource_disabled(res, 0);
 			return false;
 		}
-		acpi_dev_get_irqresource(res, irq->interrupts[index],
+		acpi_dev_get_irqresource(res, irq->interrupts[index], &dummy,
 					 irq->triggering, irq->polarity,
 					 irq->sharable, true);
 		break;
@@ -471,6 +477,7 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
 			return false;
 		}
 		acpi_dev_get_irqresource(res, ext_irq->interrupts[index],
+					 &ext_irq->resource_source,
 					 ext_irq->triggering, ext_irq->polarity,
 					 ext_irq->sharable, false);
 		break;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 06ed7e5..f4f515e 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -26,6 +26,7 @@
 #include <linux/resource_ext.h>
 #include <linux/device.h>
 #include <linux/property.h>
+#include <linux/irqdomain.h>
 
 #ifndef _LINUX
 #define _LINUX
@@ -294,6 +295,11 @@ int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 *gsi);
 void acpi_set_irq_model(enum acpi_irq_model_id model,
 			struct fwnode_handle *fwnode);
 
+int acpi_irq_domain_register_irq(struct acpi_resource_source *source, u32 rcirq,
+		      int trigger, int polarity);
+int acpi_irq_domain_unregister_irq(struct acpi_resource_source *source,
+				   u32 rcirq);
+
 #ifdef CONFIG_X86_IO_APIC
 extern int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity);
 #else
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH V3 1/2] ACPI: Add support for ResourceSource/IRQ domain mapping

[agustinv]

Adding linux-arm-kernel and linux-acpi

On 2016-05-13 12:16, Agustin Vega-Frias wrote:
> This allows irqchip drivers to associate an ACPI DSDT device to
> an IRQ domain and provides support for using the ResourceSource
> in Extended IRQ Resources to find the domain and map the IRQs
> specified on that domain.
> 
> Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
> ---
>  drivers/acpi/Makefile    |   1 +
>  drivers/acpi/irqdomain.c | 108 
> +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/resource.c  |  23 ++++++----
>  include/linux/acpi.h     |   6 +++
>  4 files changed, 130 insertions(+), 8 deletions(-)
>  create mode 100644 drivers/acpi/irqdomain.c
> 
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index b12fa64..79db78f 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -56,6 +56,7 @@ acpi-$(CONFIG_ACPI_NUMA)	+= numa.o
>  acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
>  acpi-y				+= acpi_lpat.o
>  acpi-$(CONFIG_ACPI_GENERIC_GSI) += gsi.o
> +acpi-y				+= irqdomain.o
> 
>  # These are (potentially) separate modules
> 
> diff --git a/drivers/acpi/irqdomain.c b/drivers/acpi/irqdomain.c
> new file mode 100644
> index 0000000..0fd10a9
> --- /dev/null
> +++ b/drivers/acpi/irqdomain.c
> @@ -0,0 +1,108 @@
> +/*
> + * ACPI ResourceSource/IRQ domain mapping support
> + *
> + * Copyright (c) 2016, The Linux Foundation. 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 and
> + * only 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.
> + */
> +#include <linux/acpi.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +
> +/**
> + * acpi_irq_domain_register_irq() - Register the mapping for an IRQ 
> produced
> + *                                  by the given acpi_resource_source 
> to a
> + *                                  Linux IRQ number
> + * @source: IRQ source
> + * @rcirq: IRQ number
> + * @trigger: trigger type of the IRQ number to be mapped
> + * @polarity: polarity of the IRQ to be mapped
> + *
> + * Returns: a valid linux IRQ number on success
> + *          -ENODEV if the given acpi_resource_source cannot be found
> + *          -EPROBE_DEFER if the IRQ domain has not been registered
> + *          -EINVAL for all other errors
> + */
> +int acpi_irq_domain_register_irq(struct acpi_resource_source *source,
> u32 rcirq,
> +				 int trigger, int polarity)
> +{
> +	struct irq_domain *domain;
> +	struct acpi_device *device;
> +	acpi_handle handle;
> +	acpi_status status;
> +	unsigned int type;
> +	int ret;
> +
> +	status = acpi_get_handle(NULL, source->string_ptr, &handle);
> +	if (ACPI_FAILURE(status))
> +		return -ENODEV;
> +
> +	device = acpi_bus_get_acpi_device(handle);
> +	if (!device)
> +		return -ENODEV;
> +
> +	domain = irq_find_matching_fwnode(&device->fwnode, DOMAIN_BUS_ANY);
> +	if (!domain) {
> +		ret = -EPROBE_DEFER;
> +		goto out_put_device;
> +	}
> +
> +	type = acpi_dev_get_irq_type(trigger, polarity);
> +	ret = irq_create_mapping(domain, rcirq);
> +	if (ret)
> +		irq_set_irq_type(ret, type);
> +
> +out_put_device:
> +	acpi_bus_put_acpi_device(device);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(acpi_irq_domain_register_irq);
> +
> +/**
> + * acpi_irq_domain_unregister_irq() - Delete the mapping for an IRQ 
> produced
> + *                                    by the given 
> acpi_resource_source to a
> + *                                    Linux IRQ number
> + * @source: IRQ source
> + * @rcirq: IRQ number
> + *
> + * Returns: 0 on success
> + *          -ENODEV if the given acpi_resource_source cannot be found
> + *          -EINVAL for all other errors
> + */
> +int acpi_irq_domain_unregister_irq(struct acpi_resource_source 
> *source,
> +				   u32 rcirq)
> +{
> +	struct irq_domain *domain;
> +	struct acpi_device *device;
> +	acpi_handle handle;
> +	acpi_status status;
> +	int ret = 0;
> +
> +	status = acpi_get_handle(NULL, source->string_ptr, &handle);
> +	if (ACPI_FAILURE(status))
> +		return -ENODEV;
> +
> +	device = acpi_bus_get_acpi_device(handle);
> +	if (!device)
> +		return -ENODEV;
> +
> +	domain = irq_find_matching_fwnode(&device->fwnode, DOMAIN_BUS_ANY);
> +	if (!domain) {
> +		ret = -EINVAL;
> +		goto out_put_device;
> +	}
> +
> +	irq_dispose_mapping(irq_find_mapping(domain, rcirq));
> +
> +out_put_device:
> +	acpi_bus_put_acpi_device(device);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(acpi_irq_domain_unregister_irq);
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 56241eb..1551698 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -381,14 +381,15 @@ static void acpi_dev_irqresource_disabled(struct
> resource *res, u32 gsi)
>  	res->flags = IORESOURCE_IRQ | IORESOURCE_DISABLED | IORESOURCE_UNSET;
>  }
> 
> -static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
> +static void acpi_dev_get_irqresource(struct resource *res, u32 rcirq,
> +				     struct acpi_resource_source *source,
>  				     u8 triggering, u8 polarity, u8 shareable,
>  				     bool legacy)
>  {
>  	int irq, p, t;
> 
> -	if (!valid_IRQ(gsi)) {
> -		acpi_dev_irqresource_disabled(res, gsi);
> +	if ((source->string_length == 0) && !valid_IRQ(rcirq)) {
> +		acpi_dev_irqresource_disabled(res, rcirq);
>  		return;
>  	}
> 
> @@ -402,12 +403,12 @@ static void acpi_dev_get_irqresource(struct
> resource *res, u32 gsi,
>  	 * using extended IRQ descriptors we take the IRQ configuration
>  	 * from _CRS directly.
>  	 */
> -	if (legacy && !acpi_get_override_irq(gsi, &t, &p)) {
> +	if (legacy && !acpi_get_override_irq(rcirq, &t, &p)) {
>  		u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
>  		u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
> 
>  		if (triggering != trig || polarity != pol) {
> -			pr_warning("ACPI: IRQ %d override to %s, %s\n", gsi,
> +			pr_warning("ACPI: IRQ %d override to %s, %s\n", rcirq,
>  				   t ? "level" : "edge", p ? "low" : "high");
>  			triggering = trig;
>  			polarity = pol;
> @@ -415,12 +416,16 @@ static void acpi_dev_get_irqresource(struct
> resource *res, u32 gsi,
>  	}
> 
>  	res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
> -	irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
> +	if (source->string_length > 0)
> +		irq = acpi_irq_domain_register_irq(source, rcirq, triggering,
> +						   polarity);
> +	else
> +		irq = acpi_register_gsi(NULL, rcirq, triggering, polarity);
>  	if (irq >= 0) {
>  		res->start = irq;
>  		res->end = irq;
>  	} else {
> -		acpi_dev_irqresource_disabled(res, gsi);
> +		acpi_dev_irqresource_disabled(res, rcirq);
>  	}
>  }
> 
> @@ -448,6 +453,7 @@ bool acpi_dev_resource_interrupt(struct
> acpi_resource *ares, int index,
>  {
>  	struct acpi_resource_irq *irq;
>  	struct acpi_resource_extended_irq *ext_irq;
> +	struct acpi_resource_source dummy = { 0, 0, NULL };
> 
>  	switch (ares->type) {
>  	case ACPI_RESOURCE_TYPE_IRQ:
> @@ -460,7 +466,7 @@ bool acpi_dev_resource_interrupt(struct
> acpi_resource *ares, int index,
>  			acpi_dev_irqresource_disabled(res, 0);
>  			return false;
>  		}
> -		acpi_dev_get_irqresource(res, irq->interrupts[index],
> +		acpi_dev_get_irqresource(res, irq->interrupts[index], &dummy,
>  					 irq->triggering, irq->polarity,
>  					 irq->sharable, true);
>  		break;
> @@ -471,6 +477,7 @@ bool acpi_dev_resource_interrupt(struct
> acpi_resource *ares, int index,
>  			return false;
>  		}
>  		acpi_dev_get_irqresource(res, ext_irq->interrupts[index],
> +					 &ext_irq->resource_source,
>  					 ext_irq->triggering, ext_irq->polarity,
>  					 ext_irq->sharable, false);
>  		break;
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 06ed7e5..f4f515e 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -26,6 +26,7 @@
>  #include <linux/resource_ext.h>
>  #include <linux/device.h>
>  #include <linux/property.h>
> +#include <linux/irqdomain.h>
> 
>  #ifndef _LINUX
>  #define _LINUX
> @@ -294,6 +295,11 @@ int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 
> *gsi);
>  void acpi_set_irq_model(enum acpi_irq_model_id model,
>  			struct fwnode_handle *fwnode);
> 
> +int acpi_irq_domain_register_irq(struct acpi_resource_source *source,
> u32 rcirq,
> +		      int trigger, int polarity);
> +int acpi_irq_domain_unregister_irq(struct acpi_resource_source 
> *source,
> +				   u32 rcirq);
> +
>  #ifdef CONFIG_X86_IO_APIC
>  extern int acpi_get_override_irq(u32 gsi, int *trigger, int 
> *polarity);
>  #else
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
> Forum,
> a Linux Foundation Collaborative Project.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH V3 1/2] ACPI: Add support for ResourceSource/IRQ domain mapping

[Marc Zyngier]

On Fri, 13 May 2016 12:16:42 -0400
Agustin Vega-Frias <agustinv@codeaurora.org> wrote:

> This allows irqchip drivers to associate an ACPI DSDT device to
> an IRQ domain and provides support for using the ResourceSource
> in Extended IRQ Resources to find the domain and map the IRQs
> specified on that domain.
> 
> Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
> ---
>  drivers/acpi/Makefile    |   1 +
>  drivers/acpi/irqdomain.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/resource.c  |  23 ++++++----
>  include/linux/acpi.h     |   6 +++
>  4 files changed, 130 insertions(+), 8 deletions(-)
>  create mode 100644 drivers/acpi/irqdomain.c
> 
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index b12fa64..79db78f 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -56,6 +56,7 @@ acpi-$(CONFIG_ACPI_NUMA)	+= numa.o
>  acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
>  acpi-y				+= acpi_lpat.o
>  acpi-$(CONFIG_ACPI_GENERIC_GSI) += gsi.o
> +acpi-y				+= irqdomain.o
>  
>  # These are (potentially) separate modules
>  
> diff --git a/drivers/acpi/irqdomain.c b/drivers/acpi/irqdomain.c
> new file mode 100644
> index 0000000..0fd10a9
> --- /dev/null
> +++ b/drivers/acpi/irqdomain.c
> @@ -0,0 +1,108 @@
> +/*
> + * ACPI ResourceSource/IRQ domain mapping support
> + *
> + * Copyright (c) 2016, The Linux Foundation. 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 and
> + * only 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.
> + */
> +#include <linux/acpi.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +
> +/**
> + * acpi_irq_domain_register_irq() - Register the mapping for an IRQ produced
> + *                                  by the given acpi_resource_source to a
> + *                                  Linux IRQ number
> + * @source: IRQ source

I'm a bit uncertain if this describe the interrupt producer (the device
that shakes an interrupt line) or the interrupt collator (the device
that presents a bunch of interrupts to a downstream element). I'm
tempted to say that this is the latter, but that's far from being clear.

> + * @rcirq: IRQ number
> + * @trigger: trigger type of the IRQ number to be mapped
> + * @polarity: polarity of the IRQ to be mapped

So if I'm right in my above understanding, you've reinvented an
existing abstraction (irq_fwspec).

> + *
> + * Returns: a valid linux IRQ number on success
> + *          -ENODEV if the given acpi_resource_source cannot be found
> + *          -EPROBE_DEFER if the IRQ domain has not been registered
> + *          -EINVAL for all other errors
> + */
> +int acpi_irq_domain_register_irq(struct acpi_resource_source *source, u32 rcirq,
> +				 int trigger, int polarity)
> +{
> +	struct irq_domain *domain;
> +	struct acpi_device *device;
> +	acpi_handle handle;
> +	acpi_status status;
> +	unsigned int type;
> +	int ret;
> +
> +	status = acpi_get_handle(NULL, source->string_ptr, &handle);
> +	if (ACPI_FAILURE(status))
> +		return -ENODEV;
> +
> +	device = acpi_bus_get_acpi_device(handle);
> +	if (!device)
> +		return -ENODEV;
> +

So at this point, you should be able to create a irq_fwspec, and call
into irq_create_fwspec_mapping(), without the need to open-code stuff
we already have. And as a bonus point, you'd end-up with code that'd be
similar to what is in gsi.c...

> +	domain = irq_find_matching_fwnode(&device->fwnode, DOMAIN_BUS_ANY);
> +	if (!domain) {
> +		ret = -EPROBE_DEFER;
> +		goto out_put_device;
> +	}
> +
> +	type = acpi_dev_get_irq_type(trigger, polarity);
> +	ret = irq_create_mapping(domain, rcirq);
> +	if (ret)
> +		irq_set_irq_type(ret, type);
> +
> +out_put_device:
> +	acpi_bus_put_acpi_device(device);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(acpi_irq_domain_register_irq);
> +
> +/**
> + * acpi_irq_domain_unregister_irq() - Delete the mapping for an IRQ produced
> + *                                    by the given acpi_resource_source to a
> + *                                    Linux IRQ number
> + * @source: IRQ source
> + * @rcirq: IRQ number
> + *
> + * Returns: 0 on success
> + *          -ENODEV if the given acpi_resource_source cannot be found
> + *          -EINVAL for all other errors
> + */
> +int acpi_irq_domain_unregister_irq(struct acpi_resource_source *source,
> +				   u32 rcirq)
> +{
> +	struct irq_domain *domain;
> +	struct acpi_device *device;
> +	acpi_handle handle;
> +	acpi_status status;
> +	int ret = 0;
> +
> +	status = acpi_get_handle(NULL, source->string_ptr, &handle);
> +	if (ACPI_FAILURE(status))
> +		return -ENODEV;
> +
> +	device = acpi_bus_get_acpi_device(handle);
> +	if (!device)
> +		return -ENODEV;
> +
> +	domain = irq_find_matching_fwnode(&device->fwnode, DOMAIN_BUS_ANY);
> +	if (!domain) {
> +		ret = -EINVAL;
> +		goto out_put_device;
> +	}
> +
> +	irq_dispose_mapping(irq_find_mapping(domain, rcirq));
> +
> +out_put_device:
> +	acpi_bus_put_acpi_device(device);
> +	return ret;
> +}

Again, this smell a lot like gsi.c, with added sugar on top.

> +EXPORT_SYMBOL_GPL(acpi_irq_domain_unregister_irq);
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 56241eb..1551698 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -381,14 +381,15 @@ static void acpi_dev_irqresource_disabled(struct resource *res, u32 gsi)
>  	res->flags = IORESOURCE_IRQ | IORESOURCE_DISABLED | IORESOURCE_UNSET;
>  }
>  
> -static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
> +static void acpi_dev_get_irqresource(struct resource *res, u32 rcirq,
> +				     struct acpi_resource_source *source,
>  				     u8 triggering, u8 polarity, u8 shareable,
>  				     bool legacy)
>  {
>  	int irq, p, t;
>  
> -	if (!valid_IRQ(gsi)) {
> -		acpi_dev_irqresource_disabled(res, gsi);
> +	if ((source->string_length == 0) && !valid_IRQ(rcirq)) {
> +		acpi_dev_irqresource_disabled(res, rcirq);
>  		return;
>  	}
>  
> @@ -402,12 +403,12 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>  	 * using extended IRQ descriptors we take the IRQ configuration
>  	 * from _CRS directly.
>  	 */
> -	if (legacy && !acpi_get_override_irq(gsi, &t, &p)) {
> +	if (legacy && !acpi_get_override_irq(rcirq, &t, &p)) {
>  		u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
>  		u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
>  
>  		if (triggering != trig || polarity != pol) {
> -			pr_warning("ACPI: IRQ %d override to %s, %s\n", gsi,
> +			pr_warning("ACPI: IRQ %d override to %s, %s\n", rcirq,
>  				   t ? "level" : "edge", p ? "low" : "high");
>  			triggering = trig;
>  			polarity = pol;
> @@ -415,12 +416,16 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>  	}
>  
>  	res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
> -	irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
> +	if (source->string_length > 0)
> +		irq = acpi_irq_domain_register_irq(source, rcirq, triggering,
> +						   polarity);
> +	else
> +		irq = acpi_register_gsi(NULL, rcirq, triggering, polarity);

Do you see what I mean about these being similar?

>  	if (irq >= 0) {
>  		res->start = irq;
>  		res->end = irq;
>  	} else {
> -		acpi_dev_irqresource_disabled(res, gsi);
> +		acpi_dev_irqresource_disabled(res, rcirq);
>  	}
>  }
>  
> @@ -448,6 +453,7 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>  {
>  	struct acpi_resource_irq *irq;
>  	struct acpi_resource_extended_irq *ext_irq;
> +	struct acpi_resource_source dummy = { 0, 0, NULL };
>  
>  	switch (ares->type) {
>  	case ACPI_RESOURCE_TYPE_IRQ:
> @@ -460,7 +466,7 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>  			acpi_dev_irqresource_disabled(res, 0);
>  			return false;
>  		}
> -		acpi_dev_get_irqresource(res, irq->interrupts[index],
> +		acpi_dev_get_irqresource(res, irq->interrupts[index], &dummy,
>  					 irq->triggering, irq->polarity,
>  					 irq->sharable, true);
>  		break;
> @@ -471,6 +477,7 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>  			return false;
>  		}
>  		acpi_dev_get_irqresource(res, ext_irq->interrupts[index],
> +					 &ext_irq->resource_source,
>  					 ext_irq->triggering, ext_irq->polarity,
>  					 ext_irq->sharable, false);
>  		break;
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 06ed7e5..f4f515e 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -26,6 +26,7 @@
>  #include <linux/resource_ext.h>
>  #include <linux/device.h>
>  #include <linux/property.h>
> +#include <linux/irqdomain.h>
>  
>  #ifndef _LINUX
>  #define _LINUX
> @@ -294,6 +295,11 @@ int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 *gsi);
>  void acpi_set_irq_model(enum acpi_irq_model_id model,
>  			struct fwnode_handle *fwnode);
>  
> +int acpi_irq_domain_register_irq(struct acpi_resource_source *source, u32 rcirq,
> +		      int trigger, int polarity);
> +int acpi_irq_domain_unregister_irq(struct acpi_resource_source *source,
> +				   u32 rcirq);
> +
>  #ifdef CONFIG_X86_IO_APIC
>  extern int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity);
>  #else

Thanks,

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

Re: [PATCH V3 1/2] ACPI: Add support for ResourceSource/IRQ domain mapping

[agustinv]

On 2016-06-04 08:30, Marc Zyngier wrote:
> On Fri, 13 May 2016 12:16:42 -0400
> Agustin Vega-Frias <agustinv@codeaurora.org> wrote:
> 
>> This allows irqchip drivers to associate an ACPI DSDT device to
>> an IRQ domain and provides support for using the ResourceSource
>> in Extended IRQ Resources to find the domain and map the IRQs
>> specified on that domain.
>> 
>> Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
>> ---
>>  drivers/acpi/Makefile    |   1 +
>>  drivers/acpi/irqdomain.c | 108 
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/acpi/resource.c  |  23 ++++++----
>>  include/linux/acpi.h     |   6 +++
>>  4 files changed, 130 insertions(+), 8 deletions(-)
>>  create mode 100644 drivers/acpi/irqdomain.c
>> 
>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>> index b12fa64..79db78f 100644
>> --- a/drivers/acpi/Makefile
>> +++ b/drivers/acpi/Makefile
>> @@ -56,6 +56,7 @@ acpi-$(CONFIG_ACPI_NUMA)	+= numa.o
>>  acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
>>  acpi-y				+= acpi_lpat.o
>>  acpi-$(CONFIG_ACPI_GENERIC_GSI) += gsi.o
>> +acpi-y				+= irqdomain.o
>> 
>>  # These are (potentially) separate modules
>> 
>> diff --git a/drivers/acpi/irqdomain.c b/drivers/acpi/irqdomain.c
>> new file mode 100644
>> index 0000000..0fd10a9
>> --- /dev/null
>> +++ b/drivers/acpi/irqdomain.c
>> @@ -0,0 +1,108 @@
>> +/*
>> + * ACPI ResourceSource/IRQ domain mapping support
>> + *
>> + * Copyright (c) 2016, The Linux Foundation. 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 and
>> + * only 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.
>> + */
>> +#include <linux/acpi.h>
>> +#include <linux/irq.h>
>> +#include <linux/irqdomain.h>
>> +
>> +/**
>> + * acpi_irq_domain_register_irq() - Register the mapping for an IRQ 
>> produced
>> + *                                  by the given acpi_resource_source 
>> to a
>> + *                                  Linux IRQ number
>> + * @source: IRQ source
> 
> I'm a bit uncertain if this describe the interrupt producer (the device
> that shakes an interrupt line) or the interrupt collator (the device
> that presents a bunch of interrupts to a downstream element). I'm
> tempted to say that this is the latter, but that's far from being 
> clear.

That is correct, acpi_resource_source is a device that produces IRQs for 
other devices.

> 
>> + * @rcirq: IRQ number
>> + * @trigger: trigger type of the IRQ number to be mapped
>> + * @polarity: polarity of the IRQ to be mapped
> 
> So if I'm right in my above understanding, you've reinvented an
> existing abstraction (irq_fwspec).
> 
>> + *
>> + * Returns: a valid linux IRQ number on success
>> + *          -ENODEV if the given acpi_resource_source cannot be found
>> + *          -EPROBE_DEFER if the IRQ domain has not been registered
>> + *          -EINVAL for all other errors
>> + */
>> +int acpi_irq_domain_register_irq(struct acpi_resource_source *source, 
>> u32 rcirq,
>> +				 int trigger, int polarity)
>> +{
>> +	struct irq_domain *domain;
>> +	struct acpi_device *device;
>> +	acpi_handle handle;
>> +	acpi_status status;
>> +	unsigned int type;
>> +	int ret;
>> +
>> +	status = acpi_get_handle(NULL, source->string_ptr, &handle);
>> +	if (ACPI_FAILURE(status))
>> +		return -ENODEV;
>> +
>> +	device = acpi_bus_get_acpi_device(handle);
>> +	if (!device)
>> +		return -ENODEV;
>> +
> 
> So at this point, you should be able to create a irq_fwspec, and call
> into irq_create_fwspec_mapping(), without the need to open-code stuff
> we already have. And as a bonus point, you'd end-up with code that'd be
> similar to what is in gsi.c...
> 

Got it.

>> +	domain = irq_find_matching_fwnode(&device->fwnode, DOMAIN_BUS_ANY);
>> +	if (!domain) {
>> +		ret = -EPROBE_DEFER;
>> +		goto out_put_device;
>> +	}
>> +
>> +	type = acpi_dev_get_irq_type(trigger, polarity);
>> +	ret = irq_create_mapping(domain, rcirq);
>> +	if (ret)
>> +		irq_set_irq_type(ret, type);
>> +
>> +out_put_device:
>> +	acpi_bus_put_acpi_device(device);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(acpi_irq_domain_register_irq);
>> +
>> +/**
>> + * acpi_irq_domain_unregister_irq() - Delete the mapping for an IRQ 
>> produced
>> + *                                    by the given 
>> acpi_resource_source to a
>> + *                                    Linux IRQ number
>> + * @source: IRQ source
>> + * @rcirq: IRQ number
>> + *
>> + * Returns: 0 on success
>> + *          -ENODEV if the given acpi_resource_source cannot be found
>> + *          -EINVAL for all other errors
>> + */
>> +int acpi_irq_domain_unregister_irq(struct acpi_resource_source 
>> *source,
>> +				   u32 rcirq)
>> +{
>> +	struct irq_domain *domain;
>> +	struct acpi_device *device;
>> +	acpi_handle handle;
>> +	acpi_status status;
>> +	int ret = 0;
>> +
>> +	status = acpi_get_handle(NULL, source->string_ptr, &handle);
>> +	if (ACPI_FAILURE(status))
>> +		return -ENODEV;
>> +
>> +	device = acpi_bus_get_acpi_device(handle);
>> +	if (!device)
>> +		return -ENODEV;
>> +
>> +	domain = irq_find_matching_fwnode(&device->fwnode, DOMAIN_BUS_ANY);
>> +	if (!domain) {
>> +		ret = -EINVAL;
>> +		goto out_put_device;
>> +	}
>> +
>> +	irq_dispose_mapping(irq_find_mapping(domain, rcirq));
>> +
>> +out_put_device:
>> +	acpi_bus_put_acpi_device(device);
>> +	return ret;
>> +}
> 
> Again, this smell a lot like gsi.c, with added sugar on top.

Yes, this can go away since a client can just call irq_dispose_mapping 
which finds the domain from the irq_data.

> 
>> +EXPORT_SYMBOL_GPL(acpi_irq_domain_unregister_irq);
>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
>> index 56241eb..1551698 100644
>> --- a/drivers/acpi/resource.c
>> +++ b/drivers/acpi/resource.c
>> @@ -381,14 +381,15 @@ static void acpi_dev_irqresource_disabled(struct 
>> resource *res, u32 gsi)
>>  	res->flags = IORESOURCE_IRQ | IORESOURCE_DISABLED | 
>> IORESOURCE_UNSET;
>>  }
>> 
>> -static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>> +static void acpi_dev_get_irqresource(struct resource *res, u32 rcirq,
>> +				     struct acpi_resource_source *source,
>>  				     u8 triggering, u8 polarity, u8 shareable,
>>  				     bool legacy)
>>  {
>>  	int irq, p, t;
>> 
>> -	if (!valid_IRQ(gsi)) {
>> -		acpi_dev_irqresource_disabled(res, gsi);
>> +	if ((source->string_length == 0) && !valid_IRQ(rcirq)) {
>> +		acpi_dev_irqresource_disabled(res, rcirq);
>>  		return;
>>  	}
>> 
>> @@ -402,12 +403,12 @@ static void acpi_dev_get_irqresource(struct 
>> resource *res, u32 gsi,
>>  	 * using extended IRQ descriptors we take the IRQ configuration
>>  	 * from _CRS directly.
>>  	 */
>> -	if (legacy && !acpi_get_override_irq(gsi, &t, &p)) {
>> +	if (legacy && !acpi_get_override_irq(rcirq, &t, &p)) {
>>  		u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
>>  		u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
>> 
>>  		if (triggering != trig || polarity != pol) {
>> -			pr_warning("ACPI: IRQ %d override to %s, %s\n", gsi,
>> +			pr_warning("ACPI: IRQ %d override to %s, %s\n", rcirq,
>>  				   t ? "level" : "edge", p ? "low" : "high");
>>  			triggering = trig;
>>  			polarity = pol;
>> @@ -415,12 +416,16 @@ static void acpi_dev_get_irqresource(struct 
>> resource *res, u32 gsi,
>>  	}
>> 
>>  	res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
>> -	irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
>> +	if (source->string_length > 0)
>> +		irq = acpi_irq_domain_register_irq(source, rcirq, triggering,
>> +						   polarity);
>> +	else
>> +		irq = acpi_register_gsi(NULL, rcirq, triggering, polarity);
> 
> Do you see what I mean about these being similar?
> 
>>  	if (irq >= 0) {
>>  		res->start = irq;
>>  		res->end = irq;
>>  	} else {
>> -		acpi_dev_irqresource_disabled(res, gsi);
>> +		acpi_dev_irqresource_disabled(res, rcirq);
>>  	}
>>  }
>> 
>> @@ -448,6 +453,7 @@ bool acpi_dev_resource_interrupt(struct 
>> acpi_resource *ares, int index,
>>  {
>>  	struct acpi_resource_irq *irq;
>>  	struct acpi_resource_extended_irq *ext_irq;
>> +	struct acpi_resource_source dummy = { 0, 0, NULL };
>> 
>>  	switch (ares->type) {
>>  	case ACPI_RESOURCE_TYPE_IRQ:
>> @@ -460,7 +466,7 @@ bool acpi_dev_resource_interrupt(struct 
>> acpi_resource *ares, int index,
>>  			acpi_dev_irqresource_disabled(res, 0);
>>  			return false;
>>  		}
>> -		acpi_dev_get_irqresource(res, irq->interrupts[index],
>> +		acpi_dev_get_irqresource(res, irq->interrupts[index], &dummy,
>>  					 irq->triggering, irq->polarity,
>>  					 irq->sharable, true);
>>  		break;
>> @@ -471,6 +477,7 @@ bool acpi_dev_resource_interrupt(struct 
>> acpi_resource *ares, int index,
>>  			return false;
>>  		}
>>  		acpi_dev_get_irqresource(res, ext_irq->interrupts[index],
>> +					 &ext_irq->resource_source,
>>  					 ext_irq->triggering, ext_irq->polarity,
>>  					 ext_irq->sharable, false);
>>  		break;
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index 06ed7e5..f4f515e 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -26,6 +26,7 @@
>>  #include <linux/resource_ext.h>
>>  #include <linux/device.h>
>>  #include <linux/property.h>
>> +#include <linux/irqdomain.h>
>> 
>>  #ifndef _LINUX
>>  #define _LINUX
>> @@ -294,6 +295,11 @@ int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 
>> *gsi);
>>  void acpi_set_irq_model(enum acpi_irq_model_id model,
>>  			struct fwnode_handle *fwnode);
>> 
>> +int acpi_irq_domain_register_irq(struct acpi_resource_source *source, 
>> u32 rcirq,
>> +		      int trigger, int polarity);
>> +int acpi_irq_domain_unregister_irq(struct acpi_resource_source 
>> *source,
>> +				   u32 rcirq);
>> +
>>  #ifdef CONFIG_X86_IO_APIC
>>  extern int acpi_get_override_irq(u32 gsi, int *trigger, int 
>> *polarity);
>>  #else
> 
> Thanks,
> 
> 	M.
> --
> Jazz is not dead. It just smells funny.

Thanks.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH V3 1/2] ACPI: Add support for ResourceSource/IRQ domain mapping

[Hanjun Guo]

+cc linux-acpi, linux-arm-kernel
(blocked, send again, sorry for the noise)

On 2016/6/12 19:22, Hanjun Guo wrote:
> Hi,
>
> On 2016/6/7 5:54, agustinv@codeaurora.org wrote:
>> On 2016-06-04 08:30, Marc Zyngier wrote:
>>> On Fri, 13 May 2016 12:16:42 -0400
>>> Agustin Vega-Frias <agustinv@codeaurora.org> wrote:
>
>>>
>>>> + * @rcirq: IRQ number
>>>> + * @trigger: trigger type of the IRQ number to be mapped
>>>> + * @polarity: polarity of the IRQ to be mapped
>>>
>>> So if I'm right in my above understanding, you've reinvented an
>>> existing abstraction (irq_fwspec).
>>>
>>>
>
>>> So at this point, you should be able to create a irq_fwspec, and call
>>> into irq_create_fwspec_mapping(), without the need to open-code stuff
>>> we already have. And as a bonus point, you'd end-up with code that'd be
>>> similar to what is in gsi.c...
>>>
>>
>> Got it.
>>
>>>>
>
>>>
>>> Again, this smell a lot like gsi.c, with added sugar on top.
>>
>> Yes, this can go away since a client can just call irq_dispose_mapping which finds the domain from the irq_data.
>
> I reworked my previous patches [1] which trying to support a mbi-gen interrupt
> controller, here is the updated one for discussion to see it's a option or not:
>
> [1]: http://git.linaro.org/people/hanjun.guo/acpi.git/shortlog/refs/heads/7-topic-d02-mbi-gen
> patch: ACPI: resource: pass acpi dev to acpi_register_gsi()
> acpi: gsi: make the interrupt parent be selectable</people/hanjun.guo/acpi.git/commit/28867116a69e4907e6f08971e75b533ec90a4dd2>
>
 drivers/acpi/gsi.c      |  8 +++--
 drivers/acpi/resource.c | 85 ++++++++++++++++++++++++++++++++++---------------
 include/acpi/acpi_bus.h |  1 +
 3 files changed, 66 insertions(+), 28 deletions(-)

diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
index fa4585a..afcb343 100644
--- a/drivers/acpi/gsi.c
+++ b/drivers/acpi/gsi.c
@@ -74,13 +74,17 @@ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
 		      int polarity)
 {
 	struct irq_fwspec fwspec;
+	struct acpi_device *adev = dev ? to_acpi_device(dev) : NULL;
 
-	if (WARN_ON(!acpi_gsi_domain_id)) {
+	if (acpi_gsi_domain_id)
+		fwspec.fwnode = acpi_gsi_domain_id;
+	else if (adev && &adev->fwnode && adev->interrupt_parent)
+		fwspec.fwnode = adev->interrupt_parent;
+	else {
 		pr_warn("GSI: No registered irqchip, giving up\n");
 		return -EINVAL;
 	}
 
-	fwspec.fwnode = acpi_gsi_domain_id;
 	fwspec.param[0] = gsi;
 	fwspec.param[1] = acpi_gsi_get_irq_type(trigger, polarity);
 	fwspec.param_count = 2;
diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 627f8fb..ed9491d 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -355,7 +355,7 @@ static void acpi_dev_irqresource_disabled(struct resource *res, u32 gsi)
 	res->flags = IORESOURCE_IRQ | IORESOURCE_DISABLED | IORESOURCE_UNSET;
 }
 
-static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
+static void acpi_dev_get_irqresource(struct acpi_device *adev, struct resource *res, u32 gsi,
 				     u8 triggering, u8 polarity, u8 shareable,
 				     bool legacy)
 {
@@ -389,7 +389,7 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
 	}
 
 	res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
-	irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
+	irq = acpi_register_gsi(&adev->dev, gsi, triggering, polarity);
 	if (irq >= 0) {
 		res->start = irq;
 		res->end = irq;
@@ -398,27 +398,9 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
 	}
 }
 
-/**
- * acpi_dev_resource_interrupt - Extract ACPI interrupt resource information.
- * @ares: Input ACPI resource object.
- * @index: Index into the array of GSIs represented by the resource.
- * @res: Output generic resource object.
- *
- * Check if the given ACPI resource object represents an interrupt resource
- * and @index does not exceed the resource's interrupt count (true is returned
- * in that case regardless of the results of the other checks)).  If that's the
- * case, register the GSI corresponding to @index from the array of interrupts
- * represented by the resource and populate the generic resource object pointed
- * to by @res accordingly.  If the registration of the GSI is not successful,
- * IORESOURCE_DISABLED will be set it that object's flags.
- *
- * Return:
- * 1) false with res->flags setting to zero: not the expected resource type
- * 2) false with IORESOURCE_DISABLED in res->flags: valid unassigned resource
- * 3) true: valid assigned resource
- */
-bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
-				 struct resource *res)
+static bool __acpi_dev_resource_interrupt(struct acpi_device *adev,
+					  struct acpi_resource *ares, int index,
+					  struct resource *res)
 {
 	struct acpi_resource_irq *irq;
 	struct acpi_resource_extended_irq *ext_irq;
@@ -434,7 +416,7 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
 			acpi_dev_irqresource_disabled(res, 0);
 			return false;
 		}
-		acpi_dev_get_irqresource(res, irq->interrupts[index],
+		acpi_dev_get_irqresource(adev, res, irq->interrupts[index],
 					 irq->triggering, irq->polarity,
 					 irq->sharable, true);
 		break;
@@ -444,7 +426,31 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
 			acpi_dev_irqresource_disabled(res, 0);
 			return false;
 		}
-		acpi_dev_get_irqresource(res, ext_irq->interrupts[index],
+
+		/*
+		 * It's a interrupt consumer and connecting to specfic
+		 * interrupt controller. For now, we only support connecting
+		 * interrupts to one irq controller for a single device (fix me!)
+		 */
+		if (ext_irq->producer_consumer == ACPI_CONSUMER
+		    && ext_irq->resource_source.string_length != 0
+		    && !adev->interrupt_parent) {
+			acpi_status status;
+			acpi_handle handle;
+			struct acpi_device *device;
+
+			status = acpi_get_handle(NULL, ext_irq->resource_source.string_ptr, &handle);
+			if (ACPI_FAILURE(status))
+				return false;
+
+			device = acpi_bus_get_acpi_device(handle);
+			if (!device)
+				return false;
+
+			adev->interrupt_parent = &device->fwnode;
+		}
+
+		acpi_dev_get_irqresource(adev, res, ext_irq->interrupts[index],
 					 ext_irq->triggering, ext_irq->polarity,
 					 ext_irq->sharable, false);
 		break;
@@ -455,6 +461,31 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
 
 	return true;
 }
+
+/**
+ * acpi_dev_resource_interrupt - Extract ACPI interrupt resource information.
+ * @ares: Input ACPI resource object.
+ * @index: Index into the array of GSIs represented by the resource.
+ * @res: Output generic resource object.
+ *
+ * Check if the given ACPI resource object represents an interrupt resource
+ * and @index does not exceed the resource's interrupt count (true is returned
+ * in that case regardless of the results of the other checks)).  If that's the
+ * case, register the GSI corresponding to @index from the array of interrupts
+ * represented by the resource and populate the generic resource object pointed
+ * to by @res accordingly.  If the registration of the GSI is not successful,
+ * IORESOURCE_DISABLED will be set it that object's flags.
+ *
+ * Return:
+ * 1) false with res->flags setting to zero: not the expected resource type
+ * 2) false with IORESOURCE_DISABLED in res->flags: valid unassigned resource
+ * 3) true: valid assigned resource
+ */
+bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
+				 struct resource *res)
+{
+	return __acpi_dev_resource_interrupt(NULL, ares, index, res);
+}
 EXPORT_SYMBOL_GPL(acpi_dev_resource_interrupt);
 
 /**
@@ -473,6 +504,7 @@ struct res_proc_context {
 	void *preproc_data;
 	int count;
 	int error;
+	struct acpi_device *adev;
 };
 
 static acpi_status acpi_dev_new_resource_entry(struct resource_win *win,
@@ -520,7 +552,7 @@ static acpi_status acpi_dev_process_resource(struct acpi_resource *ares,
 	    || acpi_dev_resource_ext_address_space(ares, &win))
 		return acpi_dev_new_resource_entry(&win, c);
 
-	for (i = 0; acpi_dev_resource_interrupt(ares, i, res); i++) {
+	for (i = 0; __acpi_dev_resource_interrupt(c->adev, ares, i, res); i++) {
 		acpi_status status;
 
 		status = acpi_dev_new_resource_entry(&win, c);
@@ -573,6 +605,7 @@ int acpi_dev_get_resources(struct acpi_device *adev, struct list_head *list,
 	c.preproc_data = preproc_data;
 	c.count = 0;
 	c.error = 0;
+	c.adev = adev;
 	status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
 				     acpi_dev_process_resource, &c);
 	if (ACPI_FAILURE(status)) {
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index ea4d2b5..371a086 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -353,6 +353,7 @@ struct acpi_device {
 	int device_type;
 	acpi_handle handle;		/* no handle for fixed hardware */
 	struct fwnode_handle fwnode;
+	struct fwnode_handle *interrupt_parent;
 	struct acpi_device *parent;
 	struct list_head children;
 	struct list_head node;
-- 1.9.1

[PATCH V3 2/2] irqchip: qcom: Add IRQ combiner driver

[Agustin Vega-Frias]

Driver for interrupt combiners in the Top-level Control and Status
Registers (TCSR) hardware block in Qualcomm Technologies chips.

An interrupt combiner in this block combines a set of interrupts by
OR'ing the individual interrupt signals into a summary interrupt
signal routed to a parent interrupt controller, and provides read-
only, 32-bit registers to query the status of individual interrupts.
The status bit for IRQ n is bit (n % 32) within register (n / 32)
of the given combiner. Thus, each combiner can be described as a set
of register offsets and the number of IRQs managed.

Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
---
 drivers/irqchip/Kconfig             |   8 +
 drivers/irqchip/Makefile            |   1 +
 drivers/irqchip/qcom-irq-combiner.c | 344 ++++++++++++++++++++++++++++++++++++
 3 files changed, 353 insertions(+)
 create mode 100644 drivers/irqchip/qcom-irq-combiner.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index f582e0d..609ff28 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -245,3 +245,11 @@ config IRQ_MXS
 config MVEBU_ODMI
 	bool
 	select GENERIC_MSI_IRQ_DOMAIN
+
+config QCOM_IRQ_COMBINER
+	bool "QCOM IRQ combiner support"
+	depends on ARCH_QCOM
+	select IRQ_DOMAIN
+	help
+	  Say yes here to add support for the IRQ combiner devices embedded
+	  in Qualcomm Technologies chips.
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index b03cfcb..05c688b 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -65,3 +65,4 @@ obj-$(CONFIG_INGENIC_IRQ)		+= irq-ingenic.o
 obj-$(CONFIG_IMX_GPCV2)			+= irq-imx-gpcv2.o
 obj-$(CONFIG_PIC32_EVIC)		+= irq-pic32-evic.o
 obj-$(CONFIG_MVEBU_ODMI)		+= irq-mvebu-odmi.o
+obj-$(CONFIG_QCOM_IRQ_COMBINER)		+= qcom-irq-combiner.o
diff --git a/drivers/irqchip/qcom-irq-combiner.c b/drivers/irqchip/qcom-irq-combiner.c
new file mode 100644
index 0000000..b4f767c
--- /dev/null
+++ b/drivers/irqchip/qcom-irq-combiner.c
@@ -0,0 +1,344 @@
+/* Copyright (c) 2015-2016, The Linux Foundation. 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 and
+ * only 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.
+ */
+
+/*
+ * Driver for interrupt combiners in the Top-level Control and Status
+ * Registers (TCSR) hardware block in Qualcomm Technologies chips.
+ * An interrupt combiner in this block combines a set of interrupts by
+ * OR'ing the individual interrupt signals into a summary interrupt
+ * signal routed to a parent interrupt controller, and provides read-
+ * only, 32-bit registers to query the status of individual interrupts.
+ * The status bit for IRQ n is bit (n % 32) within register (n / 32)
+ * of the given combiner. Thus, each combiner can be described as a set
+ * of register offsets and the number of IRQs managed.
+ */
+
+#include <linux/acpi.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+
+#define REG_SIZE 32
+
+struct combiner_reg {
+	void __iomem *addr;
+	unsigned long mask;
+};
+
+struct combiner {
+	struct irq_chip     irq_chip;
+	struct irq_domain   *domain;
+	int                 parent_irq;
+	u32                 nirqs;
+	u32                 nregs;
+	struct combiner_reg regs[0];
+};
+
+static inline u32 irq_register(int irq)
+{
+	return irq / REG_SIZE;
+}
+
+static inline u32 irq_bit(int irq)
+{
+	return irq % REG_SIZE;
+
+}
+
+static inline int irq_nr(u32 reg, u32 bit)
+{
+	return reg * REG_SIZE + bit;
+}
+
+/*
+ * Handler for the cascaded IRQ.
+ */
+static void combiner_handle_irq(struct irq_desc *desc)
+{
+	struct combiner *combiner = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	u32 reg;
+
+	chained_irq_enter(chip, desc);
+
+	for (reg = 0; reg < combiner->nregs; reg++) {
+		int virq;
+		int hwirq;
+		u32 bit;
+		u32 status;
+
+		if (combiner->regs[reg].mask == 0)
+			continue;
+
+		status = readl_relaxed(combiner->regs[reg].addr);
+		status &= combiner->regs[reg].mask;
+
+		while (status) {
+			bit = __ffs(status);
+			status &= ~(1 << bit);
+			hwirq = irq_nr(reg, bit);
+			virq = irq_find_mapping(combiner->domain, hwirq);
+			if (virq >= 0)
+				generic_handle_irq(virq);
+
+		}
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+/*
+ * irqchip callbacks
+ */
+
+static void combiner_irq_chip_mask_irq(struct irq_data *data)
+{
+	struct combiner *combiner = irq_data_get_irq_chip_data(data);
+	struct combiner_reg *reg = combiner->regs + irq_register(data->hwirq);
+
+	clear_bit(irq_bit(data->hwirq), &reg->mask);
+}
+
+static void combiner_irq_chip_unmask_irq(struct irq_data *data)
+{
+	struct combiner *combiner = irq_data_get_irq_chip_data(data);
+	struct combiner_reg *reg = combiner->regs + irq_register(data->hwirq);
+
+	set_bit(irq_bit(data->hwirq), &reg->mask);
+}
+
+#ifdef CONFIG_SMP
+static int combiner_irq_chip_set_affinity(struct irq_data *data,
+					 const struct cpumask *mask, bool force)
+{
+	struct combiner *combiner = irq_data_get_irq_chip_data(data);
+	struct irq_data *pdata = irq_get_irq_data(combiner->parent_irq);
+
+	if (pdata->chip && pdata->chip->irq_set_affinity)
+		return pdata->chip->irq_set_affinity(pdata, mask, force);
+	else
+		return -EINVAL;
+}
+#endif
+
+/*
+ * domain callbacks
+ */
+
+static int combiner_irq_map(struct irq_domain *domain, unsigned int irq,
+				   irq_hw_number_t hwirq)
+{
+	struct combiner *combiner = domain->host_data;
+
+	if (hwirq >= combiner->nirqs)
+		return -EINVAL;
+
+	irq_set_chip_and_handler(irq, &combiner->irq_chip, handle_level_irq);
+	irq_set_chip_data(irq, combiner);
+	irq_set_parent(irq, combiner->parent_irq);
+	irq_set_noprobe(irq);
+	return 0;
+}
+
+static void combiner_irq_unmap(struct irq_domain *domain, unsigned int irq)
+{
+	irq_set_chip_and_handler(irq, NULL, NULL);
+	irq_set_chip_data(irq, NULL);
+	irq_set_parent(irq, -1);
+}
+
+static struct irq_domain_ops domain_ops = {
+	.map = combiner_irq_map,
+	.unmap = combiner_irq_unmap
+};
+
+/*
+ * Probing and initialization.
+ *
+ * Combiner devices reside inside the TCSR block so the resulting DSDT
+ * topology is:
+ *
+ * Device (TCS0)
+ * {
+ *         Name (_HID, "QCOM80B0") // Qualcomm TCSR controller
+ *         Name (_UID, 0)
+ *
+ *         Method (_CRS, 0x0, Serialized) {
+ *                 Name (RBUF, ResourceTemplate ()
+ *                 {
+ *                         Memory32Fixed (ReadWrite, 0x2E10000, 0x00001000)
+ *                 })
+ *                 Return (RBUF)
+ *         }
+ *
+ *         Device (QIC0)
+ *         {
+ *                 Name(_HID,"QCOM80B1") // Qualcomm TCSR IRQ combiner
+ *                 ...
+ *         } // end Device QIC0
+ *         ...
+ * }
+ *
+ * Thus all combiner devices same the same memory mapping from the parent
+ * device.
+ */
+
+static int __init combiner_probe(struct platform_device *pdev)
+{
+	struct platform_device *tcsr_pdev;
+	struct combiner *combiner;
+	void __iomem *tcsr_base;
+	size_t alloc_sz;
+	u32 nregs;
+	u32 nirqs;
+
+	tcsr_pdev = to_platform_device(pdev->dev.parent);
+	tcsr_base = platform_get_drvdata(tcsr_pdev);
+	if (!tcsr_base)
+		return -ENODEV;
+
+	if (device_property_read_u32(&pdev->dev, "qcom,combiner-nr-irqs",
+				     &nirqs)) {
+		dev_err(&pdev->dev, "Error reading number of IRQs\n");
+		return -EINVAL;
+	}
+
+	nregs = device_property_read_u32_array(&pdev->dev, "qcom,combiner-regs",
+					       NULL, 0);
+	if (nregs < DIV_ROUND_UP(nirqs, REG_SIZE)) {
+		dev_err(&pdev->dev, "Error reading regs property\n");
+		return -EINVAL;
+	}
+
+	alloc_sz = sizeof(*combiner) + sizeof(struct combiner_reg) * nregs;
+	combiner = devm_kzalloc(&pdev->dev, alloc_sz, GFP_KERNEL);
+	if (combiner) {
+		int i;
+		u32 regs[nregs];
+
+		if (device_property_read_u32_array(&pdev->dev,
+						   "qcom,combiner-regs",
+						   regs, nregs)) {
+			dev_err(&pdev->dev, "Error reading regs property\n");
+			return -EINVAL;
+		}
+
+		combiner->nirqs = nirqs;
+		combiner->nregs = nregs;
+		for (i = 0; i < nregs; i++) {
+			combiner->regs[i].addr = tcsr_base + regs[i];
+			combiner->regs[i].mask = 0;
+		}
+	} else {
+		return -ENOMEM;
+	}
+
+	combiner->irq_chip.irq_mask   = combiner_irq_chip_mask_irq;
+	combiner->irq_chip.irq_unmask = combiner_irq_chip_unmask_irq;
+#ifdef CONFIG_SMP
+	combiner->irq_chip.irq_set_affinity = combiner_irq_chip_set_affinity;
+#endif
+
+	combiner->parent_irq = platform_get_irq(pdev, 0);
+	if (combiner->parent_irq <= 0) {
+		dev_err(&pdev->dev, "Error getting IRQ resource\n");
+		return -EINVAL;
+	}
+
+	combiner->domain = irq_domain_create_linear(pdev->dev.fwnode, nirqs,
+						    &domain_ops, combiner);
+	if (!combiner->domain)
+		/* Errors printed by irq_domain_create_linear */
+		return -ENODEV;
+
+	irq_set_chained_handler_and_data(combiner->parent_irq,
+					 combiner_handle_irq, combiner);
+
+	if (device_property_read_string(&pdev->dev, "qcom,combiner-name",
+					&combiner->irq_chip.name) != 0)
+		combiner->irq_chip.name = "qcom-irq-combiner";
+
+	dev_info(&pdev->dev, "Initialized with [p=%d,n=%d,r=%p]\n",
+		 combiner->parent_irq, nirqs, combiner->regs[0].addr);
+	return 0;
+}
+
+static const struct acpi_device_id qcom_irq_combiner_acpi_match[] = {
+	{ "QCOM80B1", },
+	{ }
+};
+
+static struct platform_driver qcom_irq_combiner_probe = {
+	.driver = {
+		.name = "qcom-irq-combiner",
+		.owner = THIS_MODULE,
+		.acpi_match_table = ACPI_PTR(qcom_irq_combiner_acpi_match),
+	},
+	.probe = combiner_probe,
+};
+
+static int __init register_qcom_irq_combiner(void)
+{
+	return platform_driver_register(&qcom_irq_combiner_probe);
+}
+arch_initcall(register_qcom_irq_combiner);
+
+static int __init tcsr_probe(struct platform_device *pdev)
+{
+	struct resource *mr;
+	void __iomem *tcsr_base;
+
+	mr = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (mr == NULL) {
+		dev_err(&pdev->dev, "Error getting memory resource\n");
+		return -EINVAL;
+	}
+
+	tcsr_base = devm_ioremap_resource(&pdev->dev, mr);
+	if (IS_ERR(tcsr_base)) {
+		dev_err(&pdev->dev, "Error mapping memory resource\n");
+		return PTR_ERR(tcsr_base);
+	}
+
+	dev_info(&pdev->dev, "Initialized TCSR block @%pa\n", &mr->start);
+	platform_set_drvdata(pdev, tcsr_base);
+	return 0;
+}
+
+static const struct acpi_device_id qcom_tcsr_acpi_match[] = {
+	{ "QCOM80B0", },
+	{ }
+};
+
+static struct platform_driver qcom_tcsr_probe = {
+	.driver = {
+		.name = "qcom-tcsr",
+		.owner = THIS_MODULE,
+		.acpi_match_table = ACPI_PTR(qcom_tcsr_acpi_match),
+	},
+	.probe = tcsr_probe,
+};
+
+static int __init register_qcom_tcsr(void)
+{
+	return platform_driver_register(&qcom_tcsr_probe);
+}
+arch_initcall(register_qcom_tcsr);
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH V3 2/2] irqchip: qcom: Add IRQ combiner driver

[agustinv]

Adding linux-arm-kernel and linux-acpi

On 2016-05-13 12:16, Agustin Vega-Frias wrote:
> Driver for interrupt combiners in the Top-level Control and Status
> Registers (TCSR) hardware block in Qualcomm Technologies chips.
> 
> An interrupt combiner in this block combines a set of interrupts by
> OR'ing the individual interrupt signals into a summary interrupt
> signal routed to a parent interrupt controller, and provides read-
> only, 32-bit registers to query the status of individual interrupts.
> The status bit for IRQ n is bit (n % 32) within register (n / 32)
> of the given combiner. Thus, each combiner can be described as a set
> of register offsets and the number of IRQs managed.
> 
> Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
> ---
>  drivers/irqchip/Kconfig             |   8 +
>  drivers/irqchip/Makefile            |   1 +
>  drivers/irqchip/qcom-irq-combiner.c | 344 
> ++++++++++++++++++++++++++++++++++++
>  3 files changed, 353 insertions(+)
>  create mode 100644 drivers/irqchip/qcom-irq-combiner.c
> 
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index f582e0d..609ff28 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -245,3 +245,11 @@ config IRQ_MXS
>  config MVEBU_ODMI
>  	bool
>  	select GENERIC_MSI_IRQ_DOMAIN
> +
> +config QCOM_IRQ_COMBINER
> +	bool "QCOM IRQ combiner support"
> +	depends on ARCH_QCOM
> +	select IRQ_DOMAIN
> +	help
> +	  Say yes here to add support for the IRQ combiner devices embedded
> +	  in Qualcomm Technologies chips.
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index b03cfcb..05c688b 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -65,3 +65,4 @@ obj-$(CONFIG_INGENIC_IRQ)		+= irq-ingenic.o
>  obj-$(CONFIG_IMX_GPCV2)			+= irq-imx-gpcv2.o
>  obj-$(CONFIG_PIC32_EVIC)		+= irq-pic32-evic.o
>  obj-$(CONFIG_MVEBU_ODMI)		+= irq-mvebu-odmi.o
> +obj-$(CONFIG_QCOM_IRQ_COMBINER)		+= qcom-irq-combiner.o
> diff --git a/drivers/irqchip/qcom-irq-combiner.c
> b/drivers/irqchip/qcom-irq-combiner.c
> new file mode 100644
> index 0000000..b4f767c
> --- /dev/null
> +++ b/drivers/irqchip/qcom-irq-combiner.c
> @@ -0,0 +1,344 @@
> +/* Copyright (c) 2015-2016, The Linux Foundation. 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 and
> + * only 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.
> + */
> +
> +/*
> + * Driver for interrupt combiners in the Top-level Control and Status
> + * Registers (TCSR) hardware block in Qualcomm Technologies chips.
> + * An interrupt combiner in this block combines a set of interrupts by
> + * OR'ing the individual interrupt signals into a summary interrupt
> + * signal routed to a parent interrupt controller, and provides read-
> + * only, 32-bit registers to query the status of individual 
> interrupts.
> + * The status bit for IRQ n is bit (n % 32) within register (n / 32)
> + * of the given combiner. Thus, each combiner can be described as a 
> set
> + * of register offsets and the number of IRQs managed.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/slab.h>
> +
> +#define REG_SIZE 32
> +
> +struct combiner_reg {
> +	void __iomem *addr;
> +	unsigned long mask;
> +};
> +
> +struct combiner {
> +	struct irq_chip     irq_chip;
> +	struct irq_domain   *domain;
> +	int                 parent_irq;
> +	u32                 nirqs;
> +	u32                 nregs;
> +	struct combiner_reg regs[0];
> +};
> +
> +static inline u32 irq_register(int irq)
> +{
> +	return irq / REG_SIZE;
> +}
> +
> +static inline u32 irq_bit(int irq)
> +{
> +	return irq % REG_SIZE;
> +
> +}
> +
> +static inline int irq_nr(u32 reg, u32 bit)
> +{
> +	return reg * REG_SIZE + bit;
> +}
> +
> +/*
> + * Handler for the cascaded IRQ.
> + */
> +static void combiner_handle_irq(struct irq_desc *desc)
> +{
> +	struct combiner *combiner = irq_desc_get_handler_data(desc);
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	u32 reg;
> +
> +	chained_irq_enter(chip, desc);
> +
> +	for (reg = 0; reg < combiner->nregs; reg++) {
> +		int virq;
> +		int hwirq;
> +		u32 bit;
> +		u32 status;
> +
> +		if (combiner->regs[reg].mask == 0)
> +			continue;
> +
> +		status = readl_relaxed(combiner->regs[reg].addr);
> +		status &= combiner->regs[reg].mask;
> +
> +		while (status) {
> +			bit = __ffs(status);
> +			status &= ~(1 << bit);
> +			hwirq = irq_nr(reg, bit);
> +			virq = irq_find_mapping(combiner->domain, hwirq);
> +			if (virq >= 0)
> +				generic_handle_irq(virq);
> +
> +		}
> +	}
> +
> +	chained_irq_exit(chip, desc);
> +}
> +
> +/*
> + * irqchip callbacks
> + */
> +
> +static void combiner_irq_chip_mask_irq(struct irq_data *data)
> +{
> +	struct combiner *combiner = irq_data_get_irq_chip_data(data);
> +	struct combiner_reg *reg = combiner->regs + 
> irq_register(data->hwirq);
> +
> +	clear_bit(irq_bit(data->hwirq), &reg->mask);
> +}
> +
> +static void combiner_irq_chip_unmask_irq(struct irq_data *data)
> +{
> +	struct combiner *combiner = irq_data_get_irq_chip_data(data);
> +	struct combiner_reg *reg = combiner->regs + 
> irq_register(data->hwirq);
> +
> +	set_bit(irq_bit(data->hwirq), &reg->mask);
> +}
> +
> +#ifdef CONFIG_SMP
> +static int combiner_irq_chip_set_affinity(struct irq_data *data,
> +					 const struct cpumask *mask, bool force)
> +{
> +	struct combiner *combiner = irq_data_get_irq_chip_data(data);
> +	struct irq_data *pdata = irq_get_irq_data(combiner->parent_irq);
> +
> +	if (pdata->chip && pdata->chip->irq_set_affinity)
> +		return pdata->chip->irq_set_affinity(pdata, mask, force);
> +	else
> +		return -EINVAL;
> +}
> +#endif
> +
> +/*
> + * domain callbacks
> + */
> +
> +static int combiner_irq_map(struct irq_domain *domain, unsigned int 
> irq,
> +				   irq_hw_number_t hwirq)
> +{
> +	struct combiner *combiner = domain->host_data;
> +
> +	if (hwirq >= combiner->nirqs)
> +		return -EINVAL;
> +
> +	irq_set_chip_and_handler(irq, &combiner->irq_chip, handle_level_irq);
> +	irq_set_chip_data(irq, combiner);
> +	irq_set_parent(irq, combiner->parent_irq);
> +	irq_set_noprobe(irq);
> +	return 0;
> +}
> +
> +static void combiner_irq_unmap(struct irq_domain *domain, unsigned int 
> irq)
> +{
> +	irq_set_chip_and_handler(irq, NULL, NULL);
> +	irq_set_chip_data(irq, NULL);
> +	irq_set_parent(irq, -1);
> +}
> +
> +static struct irq_domain_ops domain_ops = {
> +	.map = combiner_irq_map,
> +	.unmap = combiner_irq_unmap
> +};
> +
> +/*
> + * Probing and initialization.
> + *
> + * Combiner devices reside inside the TCSR block so the resulting DSDT
> + * topology is:
> + *
> + * Device (TCS0)
> + * {
> + *         Name (_HID, "QCOM80B0") // Qualcomm TCSR controller
> + *         Name (_UID, 0)
> + *
> + *         Method (_CRS, 0x0, Serialized) {
> + *                 Name (RBUF, ResourceTemplate ()
> + *                 {
> + *                         Memory32Fixed (ReadWrite, 0x2E10000, 
> 0x00001000)
> + *                 })
> + *                 Return (RBUF)
> + *         }
> + *
> + *         Device (QIC0)
> + *         {
> + *                 Name(_HID,"QCOM80B1") // Qualcomm TCSR IRQ combiner
> + *                 ...
> + *         } // end Device QIC0
> + *         ...
> + * }
> + *
> + * Thus all combiner devices same the same memory mapping from the 
> parent
> + * device.
> + */
> +
> +static int __init combiner_probe(struct platform_device *pdev)
> +{
> +	struct platform_device *tcsr_pdev;
> +	struct combiner *combiner;
> +	void __iomem *tcsr_base;
> +	size_t alloc_sz;
> +	u32 nregs;
> +	u32 nirqs;
> +
> +	tcsr_pdev = to_platform_device(pdev->dev.parent);
> +	tcsr_base = platform_get_drvdata(tcsr_pdev);
> +	if (!tcsr_base)
> +		return -ENODEV;
> +
> +	if (device_property_read_u32(&pdev->dev, "qcom,combiner-nr-irqs",
> +				     &nirqs)) {
> +		dev_err(&pdev->dev, "Error reading number of IRQs\n");
> +		return -EINVAL;
> +	}
> +
> +	nregs = device_property_read_u32_array(&pdev->dev, 
> "qcom,combiner-regs",
> +					       NULL, 0);
> +	if (nregs < DIV_ROUND_UP(nirqs, REG_SIZE)) {
> +		dev_err(&pdev->dev, "Error reading regs property\n");
> +		return -EINVAL;
> +	}
> +
> +	alloc_sz = sizeof(*combiner) + sizeof(struct combiner_reg) * nregs;
> +	combiner = devm_kzalloc(&pdev->dev, alloc_sz, GFP_KERNEL);
> +	if (combiner) {
> +		int i;
> +		u32 regs[nregs];
> +
> +		if (device_property_read_u32_array(&pdev->dev,
> +						   "qcom,combiner-regs",
> +						   regs, nregs)) {
> +			dev_err(&pdev->dev, "Error reading regs property\n");
> +			return -EINVAL;
> +		}
> +
> +		combiner->nirqs = nirqs;
> +		combiner->nregs = nregs;
> +		for (i = 0; i < nregs; i++) {
> +			combiner->regs[i].addr = tcsr_base + regs[i];
> +			combiner->regs[i].mask = 0;
> +		}
> +	} else {
> +		return -ENOMEM;
> +	}
> +
> +	combiner->irq_chip.irq_mask   = combiner_irq_chip_mask_irq;
> +	combiner->irq_chip.irq_unmask = combiner_irq_chip_unmask_irq;
> +#ifdef CONFIG_SMP
> +	combiner->irq_chip.irq_set_affinity = combiner_irq_chip_set_affinity;
> +#endif
> +
> +	combiner->parent_irq = platform_get_irq(pdev, 0);
> +	if (combiner->parent_irq <= 0) {
> +		dev_err(&pdev->dev, "Error getting IRQ resource\n");
> +		return -EINVAL;
> +	}
> +
> +	combiner->domain = irq_domain_create_linear(pdev->dev.fwnode, nirqs,
> +						    &domain_ops, combiner);
> +	if (!combiner->domain)
> +		/* Errors printed by irq_domain_create_linear */
> +		return -ENODEV;
> +
> +	irq_set_chained_handler_and_data(combiner->parent_irq,
> +					 combiner_handle_irq, combiner);
> +
> +	if (device_property_read_string(&pdev->dev, "qcom,combiner-name",
> +					&combiner->irq_chip.name) != 0)
> +		combiner->irq_chip.name = "qcom-irq-combiner";
> +
> +	dev_info(&pdev->dev, "Initialized with [p=%d,n=%d,r=%p]\n",
> +		 combiner->parent_irq, nirqs, combiner->regs[0].addr);
> +	return 0;
> +}
> +
> +static const struct acpi_device_id qcom_irq_combiner_acpi_match[] = {
> +	{ "QCOM80B1", },
> +	{ }
> +};
> +
> +static struct platform_driver qcom_irq_combiner_probe = {
> +	.driver = {
> +		.name = "qcom-irq-combiner",
> +		.owner = THIS_MODULE,
> +		.acpi_match_table = ACPI_PTR(qcom_irq_combiner_acpi_match),
> +	},
> +	.probe = combiner_probe,
> +};
> +
> +static int __init register_qcom_irq_combiner(void)
> +{
> +	return platform_driver_register(&qcom_irq_combiner_probe);
> +}
> +arch_initcall(register_qcom_irq_combiner);
> +
> +static int __init tcsr_probe(struct platform_device *pdev)
> +{
> +	struct resource *mr;
> +	void __iomem *tcsr_base;
> +
> +	mr = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (mr == NULL) {
> +		dev_err(&pdev->dev, "Error getting memory resource\n");
> +		return -EINVAL;
> +	}
> +
> +	tcsr_base = devm_ioremap_resource(&pdev->dev, mr);
> +	if (IS_ERR(tcsr_base)) {
> +		dev_err(&pdev->dev, "Error mapping memory resource\n");
> +		return PTR_ERR(tcsr_base);
> +	}
> +
> +	dev_info(&pdev->dev, "Initialized TCSR block @%pa\n", &mr->start);
> +	platform_set_drvdata(pdev, tcsr_base);
> +	return 0;
> +}
> +
> +static const struct acpi_device_id qcom_tcsr_acpi_match[] = {
> +	{ "QCOM80B0", },
> +	{ }
> +};
> +
> +static struct platform_driver qcom_tcsr_probe = {
> +	.driver = {
> +		.name = "qcom-tcsr",
> +		.owner = THIS_MODULE,
> +		.acpi_match_table = ACPI_PTR(qcom_tcsr_acpi_match),
> +	},
> +	.probe = tcsr_probe,
> +};
> +
> +static int __init register_qcom_tcsr(void)
> +{
> +	return platform_driver_register(&qcom_tcsr_probe);
> +}
> +arch_initcall(register_qcom_tcsr);
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
> Forum,
> a Linux Foundation Collaborative Project.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH V3 2/2] irqchip: qcom: Add IRQ combiner driver

[Marc Zyngier]

On Fri, 13 May 2016 12:16:43 -0400
Agustin Vega-Frias <agustinv@codeaurora.org> wrote:

> Driver for interrupt combiners in the Top-level Control and Status
> Registers (TCSR) hardware block in Qualcomm Technologies chips.
> 
> An interrupt combiner in this block combines a set of interrupts by
> OR'ing the individual interrupt signals into a summary interrupt
> signal routed to a parent interrupt controller, and provides read-
> only, 32-bit registers to query the status of individual interrupts.
> The status bit for IRQ n is bit (n % 32) within register (n / 32)
> of the given combiner. Thus, each combiner can be described as a set
> of register offsets and the number of IRQs managed.
> 
> Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
> ---
>  drivers/irqchip/Kconfig             |   8 +
>  drivers/irqchip/Makefile            |   1 +
>  drivers/irqchip/qcom-irq-combiner.c | 344 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 353 insertions(+)
>  create mode 100644 drivers/irqchip/qcom-irq-combiner.c
> 
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index f582e0d..609ff28 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -245,3 +245,11 @@ config IRQ_MXS
>  config MVEBU_ODMI
>  	bool
>  	select GENERIC_MSI_IRQ_DOMAIN
> +
> +config QCOM_IRQ_COMBINER
> +	bool "QCOM IRQ combiner support"
> +	depends on ARCH_QCOM
> +	select IRQ_DOMAIN
> +	help
> +	  Say yes here to add support for the IRQ combiner devices embedded
> +	  in Qualcomm Technologies chips.
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index b03cfcb..05c688b 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -65,3 +65,4 @@ obj-$(CONFIG_INGENIC_IRQ)		+= irq-ingenic.o
>  obj-$(CONFIG_IMX_GPCV2)			+= irq-imx-gpcv2.o
>  obj-$(CONFIG_PIC32_EVIC)		+= irq-pic32-evic.o
>  obj-$(CONFIG_MVEBU_ODMI)		+= irq-mvebu-odmi.o
> +obj-$(CONFIG_QCOM_IRQ_COMBINER)		+= qcom-irq-combiner.o
> diff --git a/drivers/irqchip/qcom-irq-combiner.c b/drivers/irqchip/qcom-irq-combiner.c
> new file mode 100644
> index 0000000..b4f767c
> --- /dev/null
> +++ b/drivers/irqchip/qcom-irq-combiner.c
> @@ -0,0 +1,344 @@
> +/* Copyright (c) 2015-2016, The Linux Foundation. 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 and
> + * only 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.
> + */
> +
> +/*
> + * Driver for interrupt combiners in the Top-level Control and Status
> + * Registers (TCSR) hardware block in Qualcomm Technologies chips.
> + * An interrupt combiner in this block combines a set of interrupts by
> + * OR'ing the individual interrupt signals into a summary interrupt
> + * signal routed to a parent interrupt controller, and provides read-
> + * only, 32-bit registers to query the status of individual interrupts.
> + * The status bit for IRQ n is bit (n % 32) within register (n / 32)
> + * of the given combiner. Thus, each combiner can be described as a set
> + * of register offsets and the number of IRQs managed.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/slab.h>
> +
> +#define REG_SIZE 32
> +
> +struct combiner_reg {
> +	void __iomem *addr;
> +	unsigned long mask;
> +};
> +
> +struct combiner {
> +	struct irq_chip     irq_chip;
> +	struct irq_domain   *domain;
> +	int                 parent_irq;
> +	u32                 nirqs;
> +	u32                 nregs;
> +	struct combiner_reg regs[0];
> +};
> +
> +static inline u32 irq_register(int irq)
> +{
> +	return irq / REG_SIZE;
> +}
> +
> +static inline u32 irq_bit(int irq)
> +{
> +	return irq % REG_SIZE;
> +
> +}
> +
> +static inline int irq_nr(u32 reg, u32 bit)
> +{
> +	return reg * REG_SIZE + bit;
> +}
> +
> +/*
> + * Handler for the cascaded IRQ.
> + */
> +static void combiner_handle_irq(struct irq_desc *desc)
> +{
> +	struct combiner *combiner = irq_desc_get_handler_data(desc);
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	u32 reg;
> +
> +	chained_irq_enter(chip, desc);
> +
> +	for (reg = 0; reg < combiner->nregs; reg++) {
> +		int virq;
> +		int hwirq;
> +		u32 bit;
> +		u32 status;
> +
> +		if (combiner->regs[reg].mask == 0)
> +			continue;
> +
> +		status = readl_relaxed(combiner->regs[reg].addr);
> +		status &= combiner->regs[reg].mask;
> +
> +		while (status) {
> +			bit = __ffs(status);
> +			status &= ~(1 << bit);
> +			hwirq = irq_nr(reg, bit);
> +			virq = irq_find_mapping(combiner->domain, hwirq);
> +			if (virq >= 0)
> +				generic_handle_irq(virq);
> +
> +		}
> +	}
> +
> +	chained_irq_exit(chip, desc);
> +}
> +
> +/*
> + * irqchip callbacks
> + */
> +
> +static void combiner_irq_chip_mask_irq(struct irq_data *data)
> +{
> +	struct combiner *combiner = irq_data_get_irq_chip_data(data);
> +	struct combiner_reg *reg = combiner->regs + irq_register(data->hwirq);
> +
> +	clear_bit(irq_bit(data->hwirq), &reg->mask);
> +}
> +
> +static void combiner_irq_chip_unmask_irq(struct irq_data *data)
> +{
> +	struct combiner *combiner = irq_data_get_irq_chip_data(data);
> +	struct combiner_reg *reg = combiner->regs + irq_register(data->hwirq);
> +
> +	set_bit(irq_bit(data->hwirq), &reg->mask);
> +}
> +
> +#ifdef CONFIG_SMP
> +static int combiner_irq_chip_set_affinity(struct irq_data *data,
> +					 const struct cpumask *mask, bool force)
> +{
> +	struct combiner *combiner = irq_data_get_irq_chip_data(data);
> +	struct irq_data *pdata = irq_get_irq_data(combiner->parent_irq);
> +
> +	if (pdata->chip && pdata->chip->irq_set_affinity)
> +		return pdata->chip->irq_set_affinity(pdata, mask, force);

So you're forcing the affinity of all the interrupts connected to this
irqchip in one go? Sorry, but that's not the semantic of set_affinity.
It you want to perform this, it is at the root irqchip that it has to
be done.

> +	else
> +		return -EINVAL;
> +}
> +#endif
> +
> +/*
> + * domain callbacks
> + */
> +
> +static int combiner_irq_map(struct irq_domain *domain, unsigned int irq,
> +				   irq_hw_number_t hwirq)
> +{
> +	struct combiner *combiner = domain->host_data;
> +
> +	if (hwirq >= combiner->nirqs)
> +		return -EINVAL;
> +
> +	irq_set_chip_and_handler(irq, &combiner->irq_chip, handle_level_irq);
> +	irq_set_chip_data(irq, combiner);
> +	irq_set_parent(irq, combiner->parent_irq);
> +	irq_set_noprobe(irq);
> +	return 0;
> +}
> +
> +static void combiner_irq_unmap(struct irq_domain *domain, unsigned int irq)
> +{
> +	irq_set_chip_and_handler(irq, NULL, NULL);
> +	irq_set_chip_data(irq, NULL);
> +	irq_set_parent(irq, -1);
> +}
> +
> +static struct irq_domain_ops domain_ops = {
> +	.map = combiner_irq_map,
> +	.unmap = combiner_irq_unmap

Once you've switched to the irq_fwspec abstraction, you'll need
a .translate method as well.

> +};
> +
> +/*
> + * Probing and initialization.
> + *
> + * Combiner devices reside inside the TCSR block so the resulting DSDT
> + * topology is:
> + *
> + * Device (TCS0)
> + * {
> + *         Name (_HID, "QCOM80B0") // Qualcomm TCSR controller
> + *         Name (_UID, 0)
> + *
> + *         Method (_CRS, 0x0, Serialized) {
> + *                 Name (RBUF, ResourceTemplate ()
> + *                 {
> + *                         Memory32Fixed (ReadWrite, 0x2E10000, 0x00001000)
> + *                 })
> + *                 Return (RBUF)
> + *         }
> + *
> + *         Device (QIC0)
> + *         {
> + *                 Name(_HID,"QCOM80B1") // Qualcomm TCSR IRQ combiner
> + *                 ...
> + *         } // end Device QIC0
> + *         ...
> + * }
> + *
> + * Thus all combiner devices same the same memory mapping from the parent
> + * device.
> + */
> +
> +static int __init combiner_probe(struct platform_device *pdev)
> +{
> +	struct platform_device *tcsr_pdev;
> +	struct combiner *combiner;
> +	void __iomem *tcsr_base;
> +	size_t alloc_sz;
> +	u32 nregs;
> +	u32 nirqs;
> +
> +	tcsr_pdev = to_platform_device(pdev->dev.parent);
> +	tcsr_base = platform_get_drvdata(tcsr_pdev);
> +	if (!tcsr_base)
> +		return -ENODEV;
> +
> +	if (device_property_read_u32(&pdev->dev, "qcom,combiner-nr-irqs",
> +				     &nirqs)) {

Where are these bindings documented?

> +		dev_err(&pdev->dev, "Error reading number of IRQs\n");
> +		return -EINVAL;
> +	}
> +
> +	nregs = device_property_read_u32_array(&pdev->dev, "qcom,combiner-regs",
> +					       NULL, 0);
> +	if (nregs < DIV_ROUND_UP(nirqs, REG_SIZE)) {
> +		dev_err(&pdev->dev, "Error reading regs property\n");
> +		return -EINVAL;
> +	}
> +
> +	alloc_sz = sizeof(*combiner) + sizeof(struct combiner_reg) * nregs;
> +	combiner = devm_kzalloc(&pdev->dev, alloc_sz, GFP_KERNEL);
> +	if (combiner) {
> +		int i;
> +		u32 regs[nregs];
> +
> +		if (device_property_read_u32_array(&pdev->dev,
> +						   "qcom,combiner-regs",
> +						   regs, nregs)) {
> +			dev_err(&pdev->dev, "Error reading regs property\n");
> +			return -EINVAL;
> +		}
> +
> +		combiner->nirqs = nirqs;
> +		combiner->nregs = nregs;
> +		for (i = 0; i < nregs; i++) {
> +			combiner->regs[i].addr = tcsr_base + regs[i];
> +			combiner->regs[i].mask = 0;
> +		}
> +	} else {
> +		return -ENOMEM;
> +	}
> +
> +	combiner->irq_chip.irq_mask   = combiner_irq_chip_mask_irq;
> +	combiner->irq_chip.irq_unmask = combiner_irq_chip_unmask_irq;
> +#ifdef CONFIG_SMP
> +	combiner->irq_chip.irq_set_affinity = combiner_irq_chip_set_affinity;
> +#endif
> +
> +	combiner->parent_irq = platform_get_irq(pdev, 0);
> +	if (combiner->parent_irq <= 0) {
> +		dev_err(&pdev->dev, "Error getting IRQ resource\n");
> +		return -EINVAL;
> +	}
> +
> +	combiner->domain = irq_domain_create_linear(pdev->dev.fwnode, nirqs,
> +						    &domain_ops, combiner);
> +	if (!combiner->domain)
> +		/* Errors printed by irq_domain_create_linear */
> +		return -ENODEV;
> +
> +	irq_set_chained_handler_and_data(combiner->parent_irq,
> +					 combiner_handle_irq, combiner);
> +
> +	if (device_property_read_string(&pdev->dev, "qcom,combiner-name",
> +					&combiner->irq_chip.name) != 0)
> +		combiner->irq_chip.name = "qcom-irq-combiner";
> +
> +	dev_info(&pdev->dev, "Initialized with [p=%d,n=%d,r=%p]\n",
> +		 combiner->parent_irq, nirqs, combiner->regs[0].addr);
> +	return 0;
> +}
> +
> +static const struct acpi_device_id qcom_irq_combiner_acpi_match[] = {
> +	{ "QCOM80B1", },
> +	{ }
> +};
> +
> +static struct platform_driver qcom_irq_combiner_probe = {
> +	.driver = {
> +		.name = "qcom-irq-combiner",
> +		.owner = THIS_MODULE,
> +		.acpi_match_table = ACPI_PTR(qcom_irq_combiner_acpi_match),
> +	},
> +	.probe = combiner_probe,
> +};
> +
> +static int __init register_qcom_irq_combiner(void)
> +{
> +	return platform_driver_register(&qcom_irq_combiner_probe);
> +}
> +arch_initcall(register_qcom_irq_combiner);

That bloody ugly. We don't probe platform drivers from an
arch_initcall. Yes, probing it later will result in dependency
problems. You can either work around it by deferring the probe of other
drivers when they fail to get their interrupt, or help designing a
generic solution for this issue (mbigen is in the same boat).

> +
> +static int __init tcsr_probe(struct platform_device *pdev)
> +{
> +	struct resource *mr;
> +	void __iomem *tcsr_base;
> +
> +	mr = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (mr == NULL) {
> +		dev_err(&pdev->dev, "Error getting memory resource\n");
> +		return -EINVAL;
> +	}
> +
> +	tcsr_base = devm_ioremap_resource(&pdev->dev, mr);
> +	if (IS_ERR(tcsr_base)) {
> +		dev_err(&pdev->dev, "Error mapping memory resource\n");
> +		return PTR_ERR(tcsr_base);
> +	}
> +
> +	dev_info(&pdev->dev, "Initialized TCSR block @%pa\n", &mr->start);
> +	platform_set_drvdata(pdev, tcsr_base);
> +	return 0;
> +}
> +
> +static const struct acpi_device_id qcom_tcsr_acpi_match[] = {
> +	{ "QCOM80B0", },
> +	{ }
> +};
> +
> +static struct platform_driver qcom_tcsr_probe = {
> +	.driver = {
> +		.name = "qcom-tcsr",
> +		.owner = THIS_MODULE,
> +		.acpi_match_table = ACPI_PTR(qcom_tcsr_acpi_match),
> +	},
> +	.probe = tcsr_probe,
> +};
> +
> +static int __init register_qcom_tcsr(void)
> +{
> +	return platform_driver_register(&qcom_tcsr_probe);
> +}
> +arch_initcall(register_qcom_tcsr);


Same remark here. Also, if this irqchip is the only interesting thing
in the TCSR, why is it a separate device? If there is more to it, make
it a separate driver (a MFD or something similar...).

Thanks,

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

Re: [PATCH V3 0/2] irqchip: qcom: Add IRQ combiner driver

[agustinv]

Adding linux-arm-kernel and linux-acpi

On 2016-05-13 12:16, Agustin Vega-Frias wrote:
> Add support for IRQ combiners in the Top-level Control and Status
> Registers (TCSR) hardware block in Qualcomm Technologies chips.
> 
> The first patch adds support for ResourceSource/IRQ domain mapping
> when using Extended IRQ Resources with a specific ResourceSource.
> The core ACPI resource management code has been changed to lookup
> the IRQ domain when an IRQ resource indicates a ResourceSource,
> and register the IRQ on that domain, instead of a GSI.
> 
> The second patch takes advantage of the new capabilities to implement
> the driver for the IRQ combiners.
> 
> Changes V1 -> V2:
> * Remove use of GPIO library for the combiner
> * Refactor to use fwnode/ResourceSource to IRQ domain mapping
>   introduced in ACPI core
> 
> Changes V2 -> V3:
> * Removed parsing of _PRS to find IRQs
> * Removed acpi_irq_domain_create and acpi_irq_domain_remove
> 
> Agustin Vega-Frias (2):
>   ACPI: Add support for ResourceSource/IRQ domain mapping
>   irqchip: qcom: Add IRQ combiner driver
> 
>  drivers/acpi/Makefile               |   1 +
>  drivers/acpi/irqdomain.c            | 108 +++++++++++
>  drivers/acpi/resource.c             |  23 ++-
>  drivers/irqchip/Kconfig             |   8 +
>  drivers/irqchip/Makefile            |   1 +
>  drivers/irqchip/qcom-irq-combiner.c | 344 
> ++++++++++++++++++++++++++++++++++++
>  include/linux/acpi.h                |   6 +
>  7 files changed, 483 insertions(+), 8 deletions(-)
>  create mode 100644 drivers/acpi/irqdomain.c
>  create mode 100644 drivers/irqchip/qcom-irq-combiner.c
> 
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
> Forum,
> a Linux Foundation Collaborative Project.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,
a Linux Foundation Collaborative Project.