[PATCH V6 0/3] irqchip: qcom: Add IRQ combiner driver

[PATCH V6 0/3] 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 fixes probe deferral by allowing platform_device
IRQ resources to be re-initialized if the ACPI core failed to find
the IRQ domain during ACPI bus scan.

The second 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 third patch takes advantage of the new capabilities to implement
the driver for the IRQ combiners.

Changes V5 -> V6:
* Drop probe table and on-demand probing based on resource_source
* Add patch to fix probe deferral
* Change back combiner driver to use the platform_device/platform_driver
  APIs

Changes V4 -> V5:
* Fix issues flagged by the 0-DAY build bot
* Fix some minor style issues raised by Timur

Changes V3 -> V4:
* Add a DSDT device probe table that is used to probe DSDT IRQ chips
  as necessary when converting HW IRQs to Linux IRQs
* Describe IRQ combiner registers as ACPI Register resources

Agustin Vega-Frias (3):
  ACPI: Retry IRQ conversion if it failed previously
  ACPI: Add support for ResourceSource/IRQ domain mapping
  irqchip: qcom: Add IRQ combiner driver

 drivers/acpi/Makefile               |   1 +
 drivers/acpi/irqdomain.c            | 119 +++++++++++++
 drivers/acpi/resource.c             |  80 ++++++++-
 drivers/base/platform.c             |   9 +-
 drivers/irqchip/Kconfig             |   8 +
 drivers/irqchip/Makefile            |   1 +
 drivers/irqchip/qcom-irq-combiner.c | 337 ++++++++++++++++++++++++++++++++++++
 include/linux/acpi.h                |  32 ++++
 8 files changed, 577 insertions(+), 10 deletions(-)
 create mode 100644 drivers/acpi/irqdomain.c
 create mode 100644 drivers/irqchip/qcom-irq-combiner.c

-- 
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH V6 1/3] ACPI: Retry IRQ conversion if it failed previously

[Agustin Vega-Frias]

This allows probe deferral to work properly when a dependent device
fails to get a valid IRQ because the IRQ domain was not registered
at the time the resources were added to the platform_device.

Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
---
 drivers/acpi/resource.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/base/platform.c |  9 +++++++-
 include/linux/acpi.h    |  7 ++++++
 3 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 56241eb..4beda15 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -664,3 +664,62 @@ int acpi_dev_filter_resource_type(struct acpi_resource *ares,
 	return (type & types) ? 0 : 1;
 }
 EXPORT_SYMBOL_GPL(acpi_dev_filter_resource_type);
+
+struct acpi_irq_get_ctx {
+	unsigned int index;
+	struct resource *res;
+};
+
+static acpi_status acpi_irq_get_cb(struct acpi_resource *ares, void *context)
+{
+	struct acpi_irq_get_ctx *ctx = context;
+	struct acpi_resource_irq *irq;
+	struct acpi_resource_extended_irq *ext_irq;
+
+	switch (ares->type) {
+	case ACPI_RESOURCE_TYPE_IRQ:
+		irq = &ares->data.irq;
+		if (ctx->index < irq->interrupt_count) {
+			acpi_dev_resource_interrupt(ares, ctx->index, ctx->res);
+			return AE_CTRL_TERMINATE;
+		}
+		ctx->index -= irq->interrupt_count;
+		break;
+	case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
+		ext_irq = &ares->data.extended_irq;
+		if (ctx->index < ext_irq->interrupt_count) {
+			acpi_dev_resource_interrupt(ares, ctx->index, ctx->res);
+			return AE_CTRL_TERMINATE;
+		}
+		ctx->index -= ext_irq->interrupt_count;
+		break;
+	}
+
+	return AE_OK;
+}
+
+/**
+ * acpi_irq_get - Look for the ACPI IRQ resource with the given index and
+ *                use it to initialize the given Linux IRQ resource.
+ * @handle ACPI device handle
+ * @index  ACPI IRQ resource index to lookup
+ * @res    Linux IRQ resource to initialize
+ *
+ * Return: 0 on success
+ *         -EINVAL if an error occurs
+ *         -EPROBE_DEFER if the IRQ lookup/conversion failed
+ */
+int acpi_irq_get(acpi_handle handle, unsigned int index, struct resource *res)
+{
+	struct acpi_irq_get_ctx ctx = { index, res };
+	acpi_status status;
+
+	status = acpi_walk_resources(handle, METHOD_NAME__CRS,
+				     acpi_irq_get_cb, &ctx);
+	if (ACPI_FAILURE(status))
+		return -EINVAL;
+	if (res->flags & IORESOURCE_DISABLED)
+		return -EPROBE_DEFER;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(acpi_irq_get);
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index c4af003..61423d2 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -102,6 +102,14 @@ int platform_get_irq(struct platform_device *dev, unsigned int num)
 	}
 
 	r = platform_get_resource(dev, IORESOURCE_IRQ, num);
+	if (r && r->flags & IORESOURCE_DISABLED && ACPI_COMPANION(&dev->dev)) {
+		int ret;
+
+		ret = acpi_irq_get(ACPI_HANDLE(&dev->dev), num, r);
+		if (ret)
+			return ret;
+	}
+
 	/*
 	 * The resources may pass trigger flags to the irqs that need
 	 * to be set up. It so happens that the trigger flags for
@@ -1450,4 +1458,3 @@ void __init early_platform_cleanup(void)
 		memset(&pd->dev.devres_head, 0, sizeof(pd->dev.devres_head));
 	}
 }
-
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index ddbeda6..40213c4 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -405,6 +405,7 @@ bool acpi_dev_resource_ext_address_space(struct acpi_resource *ares,
 unsigned int acpi_dev_get_irq_type(int triggering, int polarity);
 bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
 				 struct resource *res);
+int acpi_irq_get(acpi_handle handle, unsigned int index, struct resource *res);
 
 void acpi_dev_free_resource_list(struct list_head *list);
 int acpi_dev_get_resources(struct acpi_device *adev, struct list_head *list,
@@ -762,6 +763,12 @@ static inline int acpi_reconfig_notifier_unregister(struct notifier_block *nb)
 	return -EINVAL;
 }
 
+static inline int acpi_irq_get(acpi_handle handle, unsigned int index,
+			       struct resource *res)
+{
+	return -EINVAL;
+}
+
 #endif	/* !CONFIG_ACPI */
 
 #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
-- 
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH V6 2/3] 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 | 119 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/resource.c  |  21 +++++----
 include/linux/acpi.h     |  25 ++++++++++
 4 files changed, 157 insertions(+), 9 deletions(-)
 create mode 100644 drivers/acpi/irqdomain.c

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 9ed0878..880401b 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -57,6 +57,7 @@ acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
 acpi-y				+= acpi_lpat.o
 acpi-$(CONFIG_ACPI_GENERIC_GSI) += gsi.o
 acpi-$(CONFIG_ACPI_WATCHDOG)	+= acpi_watchdog.o
+acpi-$(CONFIG_IRQ_DOMAIN)	+= 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..11d3658
--- /dev/null
+++ b/drivers/acpi/irqdomain.c
@@ -0,0 +1,119 @@
+/*
+ * 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
+ * @hwirq: Hardware 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(const struct acpi_resource_source *source,
+				 u32 hwirq, int trigger, int polarity)
+{
+	struct irq_fwspec fwspec;
+	struct acpi_device *device;
+	acpi_handle handle;
+	acpi_status status;
+	int ret;
+
+	/* An empty acpi_resource_source means it is a GSI */
+	if (!source->string_length)
+		return acpi_register_gsi(NULL, hwirq, trigger, polarity);
+
+	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;
+
+	if (irq_find_matching_fwnode(&device->fwnode, DOMAIN_BUS_ANY) == NULL) {
+		ret = -EPROBE_DEFER;
+		goto out_put_device;
+	}
+
+	fwspec.fwnode = &device->fwnode;
+	fwspec.param[0] = hwirq;
+	fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
+	fwspec.param_count = 2;
+
+	ret = irq_create_fwspec_mapping(&fwspec);
+
+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
+ * @hwirq: Hardware 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(const struct acpi_resource_source *source,
+				   u32 hwirq)
+{
+	struct irq_domain *domain;
+	struct acpi_device *device;
+	acpi_handle handle;
+	acpi_status status;
+	int ret = 0;
+
+	if (!source->string_length) {
+		acpi_unregister_gsi(hwirq);
+		return 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, hwirq));
+
+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 4beda15..ccb6f4f 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 hwirq,
+				     const 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(hwirq)) {
+		acpi_dev_irqresource_disabled(res, hwirq);
 		return;
 	}
 
@@ -402,25 +403,25 @@ 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(hwirq, &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,
-				   t ? "level" : "edge", p ? "low" : "high");
+			pr_warn("ACPI: IRQ %d override to %s, %s\n", hwirq,
+				t ? "level" : "edge", p ? "low" : "high");
 			triggering = trig;
 			polarity = pol;
 		}
 	}
 
 	res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
-	irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
+	irq = acpi_irq_domain_register_irq(source, hwirq, triggering, polarity);
 	if (irq >= 0) {
 		res->start = irq;
 		res->end = irq;
 	} else {
-		acpi_dev_irqresource_disabled(res, gsi);
+		acpi_dev_irqresource_disabled(res, hwirq);
 	}
 }
 
@@ -446,6 +447,7 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
 bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
 				 struct resource *res)
 {
+	const struct acpi_resource_source dummy = { 0, 0, NULL };
 	struct acpi_resource_irq *irq;
 	struct acpi_resource_extended_irq *ext_irq;
 
@@ -460,7 +462,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 +473,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 40213c4..ce30a12 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -321,6 +321,31 @@ void acpi_set_irq_model(enum acpi_irq_model_id model,
  */
 void acpi_unregister_gsi (u32 gsi);
 
+#ifdef CONFIG_IRQ_DOMAIN
+
+int acpi_irq_domain_register_irq(const struct acpi_resource_source *source,
+				 u32 hwirq, int trigger, int polarity);
+int acpi_irq_domain_unregister_irq(const struct acpi_resource_source *source,
+				   u32 hwirq);
+
+#else
+
+static inline int acpi_irq_domain_register_irq(
+	const struct acpi_resource_source *source, u32 hwirq, int trigger,
+	int polarity)
+{
+	return acpi_register_gsi(NULL, hwirq, trigger, polarity);
+}
+
+static inline int acpi_irq_domain_unregister_irq(
+	const struct acpi_resource_source *source,  u32 hwirq)
+{
+	acpi_unregister_gsi(hwirq);
+	return 0;
+}
+
+#endif /* CONFIG_IRQ_DOMAIN */
+
 struct pci_dev;
 
 int acpi_pci_irq_enable (struct pci_dev *dev);
-- 
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH V6 3/3] 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 | 337 ++++++++++++++++++++++++++++++++++++
 3 files changed, 346 insertions(+)
 create mode 100644 drivers/irqchip/qcom-irq-combiner.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index bc0af33..610f902 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -279,3 +279,11 @@ config EZNPS_GIC
 config STM32_EXTI
 	bool
 	select 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 e4dbfc8..1818a0b 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -74,3 +74,4 @@ obj-$(CONFIG_LS_SCFG_MSI)		+= irq-ls-scfg-msi.o
 obj-$(CONFIG_EZNPS_GIC)			+= irq-eznps.o
 obj-$(CONFIG_ARCH_ASPEED)		+= irq-aspeed-vic.o
 obj-$(CONFIG_STM32_EXTI) 		+= irq-stm32-exti.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..fc25251
--- /dev/null
+++ b/drivers/irqchip/qcom-irq-combiner.c
@@ -0,0 +1,337 @@
+/* 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/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/platform_device.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);
+}
+
+/*
+ * irq_domain_ops 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);
+}
+
+#ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
+static int combiner_irq_translate(struct irq_domain *d, struct irq_fwspec *fws,
+				  unsigned long *hwirq, unsigned int *type)
+{
+	if (is_acpi_node(fws->fwnode)) {
+		if (fws->param_count != 2)
+			return -EINVAL;
+
+		*hwirq = fws->param[0];
+		*type = fws->param[1];
+		return 0;
+	}
+
+	return -EINVAL;
+}
+#endif
+
+static const struct irq_domain_ops domain_ops = {
+	.map = combiner_irq_map,
+	.unmap = combiner_irq_unmap,
+#ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
+	.translate = combiner_irq_translate
+#endif
+};
+
+/*
+ * Device probing
+ */
+
+#ifdef CONFIG_ACPI
+
+static acpi_status count_registers_cb(struct acpi_resource *ares, void *context)
+{
+	int *count = context;
+
+	if (ares->type == ACPI_RESOURCE_TYPE_GENERIC_REGISTER)
+		++(*count);
+	return AE_OK;
+}
+
+static int count_registers(struct platform_device *pdev)
+{
+	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
+	acpi_status status;
+	int count = 0;
+
+	if (!acpi_has_method(adev->handle, METHOD_NAME__CRS))
+		return -EINVAL;
+
+	status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
+				     count_registers_cb, &count);
+	if (ACPI_FAILURE(status))
+		return -EINVAL;
+	return count;
+}
+
+struct get_registers_context {
+	struct device *dev;
+	struct combiner *combiner;
+	int err;
+};
+
+static acpi_status get_registers_cb(struct acpi_resource *ares, void *context)
+{
+	struct get_registers_context *ctx = context;
+	struct acpi_resource_generic_register *reg;
+	phys_addr_t paddr;
+	void __iomem *vaddr;
+
+	if (ares->type != ACPI_RESOURCE_TYPE_GENERIC_REGISTER)
+		return AE_OK;
+
+	reg = &ares->data.generic_reg;
+	paddr = reg->address;
+	if ((reg->space_id != ACPI_SPACE_MEM) ||
+	    (reg->bit_offset != 0) ||
+	    (reg->bit_width > REG_SIZE)) {
+		dev_err(ctx->dev, "Bad register resource @%pa\n", &paddr);
+		ctx->err = -EINVAL;
+		return AE_ERROR;
+	}
+
+	vaddr = devm_ioremap(ctx->dev, reg->address, REG_SIZE);
+	if (IS_ERR(vaddr)) {
+		dev_err(ctx->dev, "Can't map register @%pa\n", &paddr);
+		ctx->err = PTR_ERR(vaddr);
+		return AE_ERROR;
+	}
+
+	ctx->combiner->regs[ctx->combiner->nregs].addr = vaddr;
+	ctx->combiner->nirqs += reg->bit_width;
+	ctx->combiner->nregs++;
+	return AE_OK;
+}
+
+static int get_registers(struct platform_device *pdev, struct combiner *comb)
+{
+	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
+	acpi_status status;
+	struct get_registers_context ctx;
+
+	if (!acpi_has_method(adev->handle, METHOD_NAME__CRS))
+		return -EINVAL;
+
+	ctx.dev = &pdev->dev;
+	ctx.combiner = comb;
+	ctx.err = 0;
+
+	status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
+				     get_registers_cb, &ctx);
+	if (ACPI_FAILURE(status))
+		return ctx.err;
+	return 0;
+}
+
+#else /* !CONFIG_ACPI */
+
+static int count_registers(struct platform_device *pdev)
+{
+	return -EINVAL;
+}
+
+static int get_registers(struct platform_device *pdev, struct combiner *comb)
+{
+	return -EINVAL;
+}
+
+#endif
+
+static int __init combiner_probe(struct platform_device *pdev)
+{
+	struct combiner *combiner;
+	size_t alloc_sz;
+	u32 nregs;
+	int err;
+
+	nregs = count_registers(pdev);
+	if (nregs <= 0) {
+		dev_err(&pdev->dev, "Error reading register resources\n");
+		return -EINVAL;
+	}
+
+	alloc_sz = sizeof(*combiner) + sizeof(struct combiner_reg) * nregs;
+	combiner = devm_kzalloc(&pdev->dev, alloc_sz, GFP_KERNEL);
+	if (!combiner)
+		return -ENOMEM;
+
+	err = get_registers(pdev, combiner);
+	if (err < 0)
+		return err;
+
+	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, combiner->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);
+	combiner->irq_chip.irq_mask = combiner_irq_chip_mask_irq;
+	combiner->irq_chip.irq_unmask = combiner_irq_chip_unmask_irq;
+	combiner->irq_chip.name = pdev->name;
+
+	dev_info(&pdev->dev, "Initialized with [p=%d,n=%d,r=%p]\n",
+		 combiner->parent_irq, combiner->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);
+}
+device_initcall(register_qcom_irq_combiner);
-- 
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH V6 2/3] ACPI: Add support for ResourceSource/IRQ domain mapping

[Hanjun Guo]

Hi Marc, Rafael, Lorenzo,

Since we agreed to add a probe deferral if we failed to get irq
resources which mirroring the DT does (patch 1 in this patch set),
I think the last blocker to make things work both for Agustin and
me [1] is this patch, which makes the interrupt producer and consumer
work in ACPI, we have two different solution for one thing, we'd happy
to work together for one solution, could you give some suggestions
please?

[1]: https://mail-archive.com/linux-kernel@vger.kernel.org/msg1257419.html

Agustin, I have some comments below.

On 2016/10/29 4:48, 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 | 119 +++++++++++++++++++++++++++++++++++++++++++++++

Could we just reuse the gsi.c and not introduce a new
file, probably we can change the gsi.c to irqdomain.c
or something similar, then reuse the code in gsi.c.

Thanks
Hanjun

>  drivers/acpi/resource.c  |  21 +++++----
>  include/linux/acpi.h     |  25 ++++++++++
>  4 files changed, 157 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/acpi/irqdomain.c
>
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 9ed0878..880401b 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -57,6 +57,7 @@ acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
>  acpi-y				+= acpi_lpat.o
>  acpi-$(CONFIG_ACPI_GENERIC_GSI) += gsi.o
>  acpi-$(CONFIG_ACPI_WATCHDOG)	+= acpi_watchdog.o
> +acpi-$(CONFIG_IRQ_DOMAIN)	+= 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..11d3658
> --- /dev/null
> +++ b/drivers/acpi/irqdomain.c
> @@ -0,0 +1,119 @@
> +/*
> + * 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
> + * @hwirq: Hardware 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(const struct acpi_resource_source *source,
> +				 u32 hwirq, int trigger, int polarity)
> +{
> +	struct irq_fwspec fwspec;
> +	struct acpi_device *device;
> +	acpi_handle handle;
> +	acpi_status status;
> +	int ret;
> +
> +	/* An empty acpi_resource_source means it is a GSI */
> +	if (!source->string_length)
> +		return acpi_register_gsi(NULL, hwirq, trigger, polarity);
> +
> +	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;
> +
> +	if (irq_find_matching_fwnode(&device->fwnode, DOMAIN_BUS_ANY) == NULL) {
> +		ret = -EPROBE_DEFER;
> +		goto out_put_device;
> +	}
> +
> +	fwspec.fwnode = &device->fwnode;
> +	fwspec.param[0] = hwirq;
> +	fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
> +	fwspec.param_count = 2;
> +
> +	ret = irq_create_fwspec_mapping(&fwspec);
> +
> +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
> + * @hwirq: Hardware 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(const struct acpi_resource_source *source,
> +				   u32 hwirq)
> +{
> +	struct irq_domain *domain;
> +	struct acpi_device *device;
> +	acpi_handle handle;
> +	acpi_status status;
> +	int ret = 0;
> +
> +	if (!source->string_length) {
> +		acpi_unregister_gsi(hwirq);
> +		return 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, hwirq));
> +
> +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 4beda15..ccb6f4f 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 hwirq,
> +				     const 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(hwirq)) {
> +		acpi_dev_irqresource_disabled(res, hwirq);
>  		return;
>  	}
>
> @@ -402,25 +403,25 @@ 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(hwirq, &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,
> -				   t ? "level" : "edge", p ? "low" : "high");
> +			pr_warn("ACPI: IRQ %d override to %s, %s\n", hwirq,
> +				t ? "level" : "edge", p ? "low" : "high");
>  			triggering = trig;
>  			polarity = pol;
>  		}
>  	}
>
>  	res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
> -	irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
> +	irq = acpi_irq_domain_register_irq(source, hwirq, triggering, polarity);
>  	if (irq >= 0) {
>  		res->start = irq;
>  		res->end = irq;
>  	} else {
> -		acpi_dev_irqresource_disabled(res, gsi);
> +		acpi_dev_irqresource_disabled(res, hwirq);
>  	}
>  }
>
> @@ -446,6 +447,7 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>  bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>  				 struct resource *res)
>  {
> +	const struct acpi_resource_source dummy = { 0, 0, NULL };
>  	struct acpi_resource_irq *irq;
>  	struct acpi_resource_extended_irq *ext_irq;
>
> @@ -460,7 +462,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 +473,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 40213c4..ce30a12 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -321,6 +321,31 @@ void acpi_set_irq_model(enum acpi_irq_model_id model,
>   */
>  void acpi_unregister_gsi (u32 gsi);
>
> +#ifdef CONFIG_IRQ_DOMAIN
> +
> +int acpi_irq_domain_register_irq(const struct acpi_resource_source *source,
> +				 u32 hwirq, int trigger, int polarity);
> +int acpi_irq_domain_unregister_irq(const struct acpi_resource_source *source,
> +				   u32 hwirq);
> +
> +#else
> +
> +static inline int acpi_irq_domain_register_irq(
> +	const struct acpi_resource_source *source, u32 hwirq, int trigger,
> +	int polarity)
> +{
> +	return acpi_register_gsi(NULL, hwirq, trigger, polarity);
> +}
> +
> +static inline int acpi_irq_domain_unregister_irq(
> +	const struct acpi_resource_source *source,  u32 hwirq)
> +{
> +	acpi_unregister_gsi(hwirq);
> +	return 0;
> +}
> +
> +#endif /* CONFIG_IRQ_DOMAIN */
> +
>  struct pci_dev;
>
>  int acpi_pci_irq_enable (struct pci_dev *dev);
>

Re: [PATCH V6 2/3] ACPI: Add support for ResourceSource/IRQ domain mapping

[agustinv]

Hey Hanjun,

On 2016-11-09 21:36, Hanjun Guo wrote:
> Hi Marc, Rafael, Lorenzo,
> 
> Since we agreed to add a probe deferral if we failed to get irq
> resources which mirroring the DT does (patch 1 in this patch set),
> I think the last blocker to make things work both for Agustin and
> me [1] is this patch, which makes the interrupt producer and consumer
> work in ACPI, we have two different solution for one thing, we'd happy
> to work together for one solution, could you give some suggestions
> please?
> 
> [1]: 
> https://mail-archive.com/linux-kernel@vger.kernel.org/msg1257419.html
> 
> Agustin, I have some comments below.
> 
> On 2016/10/29 4:48, 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 | 119 
>> +++++++++++++++++++++++++++++++++++++++++++++++
> 
> Could we just reuse the gsi.c and not introduce a new
> file, probably we can change the gsi.c to irqdomain.c
> or something similar, then reuse the code in gsi.c.

I was thinking just that after we chatted off-list.
I might revisit and see what I come up with given that we already have
a device argument and we could pass the IRQ source there.

Thanks

> 
> Thanks
> Hanjun
> 
>>  drivers/acpi/resource.c  |  21 +++++----
>>  include/linux/acpi.h     |  25 ++++++++++
>>  4 files changed, 157 insertions(+), 9 deletions(-)
>>  create mode 100644 drivers/acpi/irqdomain.c
>> 
>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>> index 9ed0878..880401b 100644
>> --- a/drivers/acpi/Makefile
>> +++ b/drivers/acpi/Makefile
>> @@ -57,6 +57,7 @@ acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
>>  acpi-y				+= acpi_lpat.o
>>  acpi-$(CONFIG_ACPI_GENERIC_GSI) += gsi.o
>>  acpi-$(CONFIG_ACPI_WATCHDOG)	+= acpi_watchdog.o
>> +acpi-$(CONFIG_IRQ_DOMAIN)	+= 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..11d3658
>> --- /dev/null
>> +++ b/drivers/acpi/irqdomain.c
>> @@ -0,0 +1,119 @@
>> +/*
>> + * 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
>> + * @hwirq: Hardware 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(const struct acpi_resource_source 
>> *source,
>> +				 u32 hwirq, int trigger, int polarity)
>> +{
>> +	struct irq_fwspec fwspec;
>> +	struct acpi_device *device;
>> +	acpi_handle handle;
>> +	acpi_status status;
>> +	int ret;
>> +
>> +	/* An empty acpi_resource_source means it is a GSI */
>> +	if (!source->string_length)
>> +		return acpi_register_gsi(NULL, hwirq, trigger, polarity);
>> +
>> +	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;
>> +
>> +	if (irq_find_matching_fwnode(&device->fwnode, DOMAIN_BUS_ANY) == 
>> NULL) {
>> +		ret = -EPROBE_DEFER;
>> +		goto out_put_device;
>> +	}
>> +
>> +	fwspec.fwnode = &device->fwnode;
>> +	fwspec.param[0] = hwirq;
>> +	fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
>> +	fwspec.param_count = 2;
>> +
>> +	ret = irq_create_fwspec_mapping(&fwspec);
>> +
>> +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
>> + * @hwirq: Hardware 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(const struct acpi_resource_source 
>> *source,
>> +				   u32 hwirq)
>> +{
>> +	struct irq_domain *domain;
>> +	struct acpi_device *device;
>> +	acpi_handle handle;
>> +	acpi_status status;
>> +	int ret = 0;
>> +
>> +	if (!source->string_length) {
>> +		acpi_unregister_gsi(hwirq);
>> +		return 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, hwirq));
>> +
>> +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 4beda15..ccb6f4f 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 hwirq,
>> +				     const 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(hwirq)) {
>> +		acpi_dev_irqresource_disabled(res, hwirq);
>>  		return;
>>  	}
>> 
>> @@ -402,25 +403,25 @@ 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(hwirq, &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,
>> -				   t ? "level" : "edge", p ? "low" : "high");
>> +			pr_warn("ACPI: IRQ %d override to %s, %s\n", hwirq,
>> +				t ? "level" : "edge", p ? "low" : "high");
>>  			triggering = trig;
>>  			polarity = pol;
>>  		}
>>  	}
>> 
>>  	res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
>> -	irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
>> +	irq = acpi_irq_domain_register_irq(source, hwirq, triggering, 
>> polarity);
>>  	if (irq >= 0) {
>>  		res->start = irq;
>>  		res->end = irq;
>>  	} else {
>> -		acpi_dev_irqresource_disabled(res, gsi);
>> +		acpi_dev_irqresource_disabled(res, hwirq);
>>  	}
>>  }
>> 
>> @@ -446,6 +447,7 @@ static void acpi_dev_get_irqresource(struct 
>> resource *res, u32 gsi,
>>  bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int 
>> index,
>>  				 struct resource *res)
>>  {
>> +	const struct acpi_resource_source dummy = { 0, 0, NULL };
>>  	struct acpi_resource_irq *irq;
>>  	struct acpi_resource_extended_irq *ext_irq;
>> 
>> @@ -460,7 +462,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 +473,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 40213c4..ce30a12 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -321,6 +321,31 @@ void acpi_set_irq_model(enum acpi_irq_model_id 
>> model,
>>   */
>>  void acpi_unregister_gsi (u32 gsi);
>> 
>> +#ifdef CONFIG_IRQ_DOMAIN
>> +
>> +int acpi_irq_domain_register_irq(const struct acpi_resource_source 
>> *source,
>> +				 u32 hwirq, int trigger, int polarity);
>> +int acpi_irq_domain_unregister_irq(const struct acpi_resource_source 
>> *source,
>> +				   u32 hwirq);
>> +
>> +#else
>> +
>> +static inline int acpi_irq_domain_register_irq(
>> +	const struct acpi_resource_source *source, u32 hwirq, int trigger,
>> +	int polarity)
>> +{
>> +	return acpi_register_gsi(NULL, hwirq, trigger, polarity);
>> +}
>> +
>> +static inline int acpi_irq_domain_unregister_irq(
>> +	const struct acpi_resource_source *source,  u32 hwirq)
>> +{
>> +	acpi_unregister_gsi(hwirq);
>> +	return 0;
>> +}
>> +
>> +#endif /* CONFIG_IRQ_DOMAIN */
>> +
>>  struct pci_dev;
>> 
>>  int acpi_pci_irq_enable (struct pci_dev *dev);
>> 

-- 
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a 
Linux Foundation Collaborative Project.

Re: [PATCH V6 2/3] ACPI: Add support for ResourceSource/IRQ domain mapping

[Lorenzo Pieralisi]

On Thu, Nov 10, 2016 at 10:02:35AM -0500, agustinv@codeaurora.org wrote:
> Hey Hanjun,
> 
> On 2016-11-09 21:36, Hanjun Guo wrote:
> >Hi Marc, Rafael, Lorenzo,
> >
> >Since we agreed to add a probe deferral if we failed to get irq
> >resources which mirroring the DT does (patch 1 in this patch set),
> >I think the last blocker to make things work both for Agustin and
> >me [1] is this patch, which makes the interrupt producer and consumer
> >work in ACPI, we have two different solution for one thing, we'd happy
> >to work together for one solution, could you give some suggestions
> >please?
> >
> >[1]: https://mail-archive.com/linux-kernel@vger.kernel.org/msg1257419.html
> >
> >Agustin, I have some comments below.
> >
> >On 2016/10/29 4:48, 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 | 119
> >>+++++++++++++++++++++++++++++++++++++++++++++++
> >
> >Could we just reuse the gsi.c and not introduce a new
> >file, probably we can change the gsi.c to irqdomain.c
> >or something similar, then reuse the code in gsi.c.
> 
> I was thinking just that after we chatted off-list.

Yes, that's a fair point.

> I might revisit and see what I come up with given that we already have
> a device argument and we could pass the IRQ source there.

I agree with the approach taken by this patch, I do not like much
passing around struct acpi_resource_source *source (in particular
the dummy struct) I do not think it is needed, I will comment on
the code.

Hopefully there is not any buggy FW out there that does use the
resource source inappropriately otherwise we will notice on x86/ia64
(ie you can't blame FW if it breaks the kernel) but I suspect the
only way to find out is by trying, the patch has to go through Rafael's
review anyway before getting there so it is fine.

Lorenzo

> Thanks
> 
> >
> >Thanks
> >Hanjun
> >
> >> drivers/acpi/resource.c  |  21 +++++----
> >> include/linux/acpi.h     |  25 ++++++++++
> >> 4 files changed, 157 insertions(+), 9 deletions(-)
> >> create mode 100644 drivers/acpi/irqdomain.c
> >>
> >>diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> >>index 9ed0878..880401b 100644
> >>--- a/drivers/acpi/Makefile
> >>+++ b/drivers/acpi/Makefile
> >>@@ -57,6 +57,7 @@ acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
> >> acpi-y				+= acpi_lpat.o
> >> acpi-$(CONFIG_ACPI_GENERIC_GSI) += gsi.o
> >> acpi-$(CONFIG_ACPI_WATCHDOG)	+= acpi_watchdog.o
> >>+acpi-$(CONFIG_IRQ_DOMAIN)	+= 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..11d3658
> >>--- /dev/null
> >>+++ b/drivers/acpi/irqdomain.c
> >>@@ -0,0 +1,119 @@
> >>+/*
> >>+ * 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
> >>+ * @hwirq: Hardware 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(const struct
> >>acpi_resource_source *source,
> >>+				 u32 hwirq, int trigger, int polarity)
> >>+{
> >>+	struct irq_fwspec fwspec;
> >>+	struct acpi_device *device;
> >>+	acpi_handle handle;
> >>+	acpi_status status;
> >>+	int ret;
> >>+
> >>+	/* An empty acpi_resource_source means it is a GSI */
> >>+	if (!source->string_length)
> >>+		return acpi_register_gsi(NULL, hwirq, trigger, polarity);
> >>+
> >>+	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;
> >>+
> >>+	if (irq_find_matching_fwnode(&device->fwnode, DOMAIN_BUS_ANY)
> >>== NULL) {
> >>+		ret = -EPROBE_DEFER;
> >>+		goto out_put_device;
> >>+	}
> >>+
> >>+	fwspec.fwnode = &device->fwnode;
> >>+	fwspec.param[0] = hwirq;
> >>+	fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
> >>+	fwspec.param_count = 2;
> >>+
> >>+	ret = irq_create_fwspec_mapping(&fwspec);
> >>+
> >>+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
> >>+ * @hwirq: Hardware 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(const struct
> >>acpi_resource_source *source,
> >>+				   u32 hwirq)
> >>+{
> >>+	struct irq_domain *domain;
> >>+	struct acpi_device *device;
> >>+	acpi_handle handle;
> >>+	acpi_status status;
> >>+	int ret = 0;
> >>+
> >>+	if (!source->string_length) {
> >>+		acpi_unregister_gsi(hwirq);
> >>+		return 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, hwirq));
> >>+
> >>+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 4beda15..ccb6f4f 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 hwirq,
> >>+				     const 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(hwirq)) {
> >>+		acpi_dev_irqresource_disabled(res, hwirq);
> >> 		return;
> >> 	}
> >>
> >>@@ -402,25 +403,25 @@ 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(hwirq, &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,
> >>-				   t ? "level" : "edge", p ? "low" : "high");
> >>+			pr_warn("ACPI: IRQ %d override to %s, %s\n", hwirq,
> >>+				t ? "level" : "edge", p ? "low" : "high");
> >> 			triggering = trig;
> >> 			polarity = pol;
> >> 		}
> >> 	}
> >>
> >> 	res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
> >>-	irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
> >>+	irq = acpi_irq_domain_register_irq(source, hwirq, triggering,
> >>polarity);
> >> 	if (irq >= 0) {
> >> 		res->start = irq;
> >> 		res->end = irq;
> >> 	} else {
> >>-		acpi_dev_irqresource_disabled(res, gsi);
> >>+		acpi_dev_irqresource_disabled(res, hwirq);
> >> 	}
> >> }
> >>
> >>@@ -446,6 +447,7 @@ static void acpi_dev_get_irqresource(struct
> >>resource *res, u32 gsi,
> >> bool acpi_dev_resource_interrupt(struct acpi_resource *ares,
> >>int index,
> >> 				 struct resource *res)
> >> {
> >>+	const struct acpi_resource_source dummy = { 0, 0, NULL };
> >> 	struct acpi_resource_irq *irq;
> >> 	struct acpi_resource_extended_irq *ext_irq;
> >>
> >>@@ -460,7 +462,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 +473,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 40213c4..ce30a12 100644
> >>--- a/include/linux/acpi.h
> >>+++ b/include/linux/acpi.h
> >>@@ -321,6 +321,31 @@ void acpi_set_irq_model(enum
> >>acpi_irq_model_id model,
> >>  */
> >> void acpi_unregister_gsi (u32 gsi);
> >>
> >>+#ifdef CONFIG_IRQ_DOMAIN
> >>+
> >>+int acpi_irq_domain_register_irq(const struct
> >>acpi_resource_source *source,
> >>+				 u32 hwirq, int trigger, int polarity);
> >>+int acpi_irq_domain_unregister_irq(const struct
> >>acpi_resource_source *source,
> >>+				   u32 hwirq);
> >>+
> >>+#else
> >>+
> >>+static inline int acpi_irq_domain_register_irq(
> >>+	const struct acpi_resource_source *source, u32 hwirq, int trigger,
> >>+	int polarity)
> >>+{
> >>+	return acpi_register_gsi(NULL, hwirq, trigger, polarity);
> >>+}
> >>+
> >>+static inline int acpi_irq_domain_unregister_irq(
> >>+	const struct acpi_resource_source *source,  u32 hwirq)
> >>+{
> >>+	acpi_unregister_gsi(hwirq);
> >>+	return 0;
> >>+}
> >>+
> >>+#endif /* CONFIG_IRQ_DOMAIN */
> >>+
> >> struct pci_dev;
> >>
> >> int acpi_pci_irq_enable (struct pci_dev *dev);
> >>
> 
> -- 
> Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm
> Technologies, Inc.
> Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a
> Linux Foundation Collaborative Project.

Re: [PATCH V6 2/3] ACPI: Add support for ResourceSource/IRQ domain mapping

[Hanjun Guo]

On 11/10/2016 11:02 PM, agustinv@codeaurora.org wrote:
> Hey Hanjun,
>
> On 2016-11-09 21:36, Hanjun Guo wrote:
>> Hi Marc, Rafael, Lorenzo,
>>
>> Since we agreed to add a probe deferral if we failed to get irq
>> resources which mirroring the DT does (patch 1 in this patch set),
>> I think the last blocker to make things work both for Agustin and
>> me [1] is this patch, which makes the interrupt producer and consumer
>> work in ACPI, we have two different solution for one thing, we'd happy
>> to work together for one solution, could you give some suggestions
>> please?
>>
>> [1]:
>> https://mail-archive.com/linux-kernel@vger.kernel.org/msg1257419.html
>>
>> Agustin, I have some comments below.
>>
>> On 2016/10/29 4:48, 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 | 119
>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>
>> Could we just reuse the gsi.c and not introduce a new
>> file, probably we can change the gsi.c to irqdomain.c
>> or something similar, then reuse the code in gsi.c.
>
> I was thinking just that after we chatted off-list.

Great.

> I might revisit and see what I come up with given that we already have
> a device argument and we could pass the IRQ source there.

Sorry, I'm little confused here, why we "already have a device
argument"? in drivers/acpi/resource.c, it just use NULL as device
and we can't pass dev directly for the API is using now, could
you explain more?

Thanks
Hanjun

Re: [PATCH V6 2/3] ACPI: Add support for ResourceSource/IRQ domain mapping

[Hanjun Guo]

Hi Lorenzo,

On 11/11/2016 01:58 AM, Lorenzo Pieralisi wrote:
> On Thu, Nov 10, 2016 at 10:02:35AM -0500, agustinv@codeaurora.org wrote:
>> Hey Hanjun,
>>
>> On 2016-11-09 21:36, Hanjun Guo wrote:
>>> Hi Marc, Rafael, Lorenzo,
>>>
>>> Since we agreed to add a probe deferral if we failed to get irq
>>> resources which mirroring the DT does (patch 1 in this patch set),
>>> I think the last blocker to make things work both for Agustin and
>>> me [1] is this patch, which makes the interrupt producer and consumer
>>> work in ACPI, we have two different solution for one thing, we'd happy
>>> to work together for one solution, could you give some suggestions
>>> please?
>>>
>>> [1]: https://mail-archive.com/linux-kernel@vger.kernel.org/msg1257419.html
>>>
>>> Agustin, I have some comments below.
>>>
>>> On 2016/10/29 4:48, 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 | 119
>>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>
>>> Could we just reuse the gsi.c and not introduce a new
>>> file, probably we can change the gsi.c to irqdomain.c
>>> or something similar, then reuse the code in gsi.c.
>>
>> I was thinking just that after we chatted off-list.
>
> Yes, that's a fair point.
>
>> I might revisit and see what I come up with given that we already have
>> a device argument and we could pass the IRQ source there.
>
> I agree with the approach taken by this patch, I do not like much
> passing around struct acpi_resource_source *source (in particular
> the dummy struct) I do not think it is needed, I will comment on
> the code.

thanks for your time to have a look:)

>
> Hopefully there is not any buggy FW out there that does use the
> resource source inappropriately otherwise we will notice on x86/ia64
> (ie you can't blame FW if it breaks the kernel) but I suspect the
> only way to find out is by trying, the patch has to go through Rafael's
> review anyway before getting there so it is fine.

I think we can avoid that by not touching the logic that x86/ia64
already used, but only adding interrupt producer/consumer function.

Thanks
Hanjun

Re: [PATCH V6 2/3] ACPI: Add support for ResourceSource/IRQ domain mapping

[agustinv]

Hey Lorenzo, Hanjun,

On 2016-11-11 08:33, Hanjun Guo wrote:
> Hi Lorenzo,
> 
> On 11/11/2016 01:58 AM, Lorenzo Pieralisi wrote:
>> On Thu, Nov 10, 2016 at 10:02:35AM -0500, agustinv@codeaurora.org 
>> wrote:
>>> Hey Hanjun,
>>> 
>>> On 2016-11-09 21:36, Hanjun Guo wrote:
>>>> Hi Marc, Rafael, Lorenzo,
>>>> 
>>>> Since we agreed to add a probe deferral if we failed to get irq
>>>> resources which mirroring the DT does (patch 1 in this patch set),
>>>> I think the last blocker to make things work both for Agustin and
>>>> me [1] is this patch, which makes the interrupt producer and 
>>>> consumer
>>>> work in ACPI, we have two different solution for one thing, we'd 
>>>> happy
>>>> to work together for one solution, could you give some suggestions
>>>> please?
>>>> 
>>>> [1]: 
>>>> https://mail-archive.com/linux-kernel@vger.kernel.org/msg1257419.html
>>>> 
>>>> Agustin, I have some comments below.
>>>> 
>>>> On 2016/10/29 4:48, 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 | 119
>>>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>> 
>>>> Could we just reuse the gsi.c and not introduce a new
>>>> file, probably we can change the gsi.c to irqdomain.c
>>>> or something similar, then reuse the code in gsi.c.
>>> 
>>> I was thinking just that after we chatted off-list.
>> 
>> Yes, that's a fair point.
>> 
>>> I might revisit and see what I come up with given that we already 
>>> have
>>> a device argument and we could pass the IRQ source there.
>> 
>> I agree with the approach taken by this patch, I do not like much
>> passing around struct acpi_resource_source *source (in particular
>> the dummy struct) I do not think it is needed, I will comment on
>> the code.
> 
> thanks for your time to have a look:)
> 
>> 
>> Hopefully there is not any buggy FW out there that does use the
>> resource source inappropriately otherwise we will notice on x86/ia64
>> (ie you can't blame FW if it breaks the kernel) but I suspect the
>> only way to find out is by trying, the patch has to go through 
>> Rafael's
>> review anyway before getting there so it is fine.
> 
> I think we can avoid that by not touching the logic that x86/ia64
> already used, but only adding interrupt producer/consumer function.

I looked at this more today and implemented a new patch that I plan to
test over the weekend, but I wanted to let you know the approach I am
pursuing.

On the new patch use of ResourceSource when parsing ACPI Extended IRQ
Resources is conditional on CONFIG_ACPI_GENERIC_GSI. The reason for this
is two fold:

1. Since we wanted to reduce duplication and place the new APIs on the
    same source file as acpi_register_gsi, which is already under that
    config flag.
2. So the patch does not have effect on platforms not using the generic
    GSI support, including x86/ia64.

If support for this is needed outside platforms using the generic GSI
implementation, we can move these APIs out to their own source file
and eliminate the CONFIG_ACPI_GENERIC_GSI conditionality.

I'll send the new patch, hopefully some time tomorrow, but please let
me know if you have concerns with this approach.

Thanks,
Agustin

-- 
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a 
Linux Foundation Collaborative Project.

Re: [PATCH V6 2/3] ACPI: Add support for ResourceSource/IRQ domain mapping

[Hanjun Guo]

On 11/12/2016 11:01 AM, agustinv@codeaurora.org wrote:
> Hey Lorenzo, Hanjun,
>
> On 2016-11-11 08:33, Hanjun Guo wrote:
>> Hi Lorenzo,
>>
>> On 11/11/2016 01:58 AM, Lorenzo Pieralisi wrote:
>>> On Thu, Nov 10, 2016 at 10:02:35AM -0500, agustinv@codeaurora.org wrote:
>>>> Hey Hanjun,
>>>>
>>>> On 2016-11-09 21:36, Hanjun Guo wrote:
>>>>> Hi Marc, Rafael, Lorenzo,
>>>>>
>>>>> Since we agreed to add a probe deferral if we failed to get irq
>>>>> resources which mirroring the DT does (patch 1 in this patch set),
>>>>> I think the last blocker to make things work both for Agustin and
>>>>> me [1] is this patch, which makes the interrupt producer and consumer
>>>>> work in ACPI, we have two different solution for one thing, we'd happy
>>>>> to work together for one solution, could you give some suggestions
>>>>> please?
>>>>>
>>>>> [1]:
>>>>> https://mail-archive.com/linux-kernel@vger.kernel.org/msg1257419.html
>>>>>
>>>>> Agustin, I have some comments below.
>>>>>
>>>>> On 2016/10/29 4:48, 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 | 119
>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>>>
>>>>> Could we just reuse the gsi.c and not introduce a new
>>>>> file, probably we can change the gsi.c to irqdomain.c
>>>>> or something similar, then reuse the code in gsi.c.
>>>>
>>>> I was thinking just that after we chatted off-list.
>>>
>>> Yes, that's a fair point.
>>>
>>>> I might revisit and see what I come up with given that we already have
>>>> a device argument and we could pass the IRQ source there.
>>>
>>> I agree with the approach taken by this patch, I do not like much
>>> passing around struct acpi_resource_source *source (in particular
>>> the dummy struct) I do not think it is needed, I will comment on
>>> the code.
>>
>> thanks for your time to have a look:)
>>
>>>
>>> Hopefully there is not any buggy FW out there that does use the
>>> resource source inappropriately otherwise we will notice on x86/ia64
>>> (ie you can't blame FW if it breaks the kernel) but I suspect the
>>> only way to find out is by trying, the patch has to go through Rafael's
>>> review anyway before getting there so it is fine.
>>
>> I think we can avoid that by not touching the logic that x86/ia64
>> already used, but only adding interrupt producer/consumer function.
>
> I looked at this more today and implemented a new patch that I plan to
> test over the weekend, but I wanted to let you know the approach I am
> pursuing.
>
> On the new patch use of ResourceSource when parsing ACPI Extended IRQ
> Resources is conditional on CONFIG_ACPI_GENERIC_GSI. The reason for this
> is two fold:
>
> 1. Since we wanted to reduce duplication and place the new APIs on the
>     same source file as acpi_register_gsi, which is already under that
>     config flag.
> 2. So the patch does not have effect on platforms not using the generic
>     GSI support, including x86/ia64.
>
> If support for this is needed outside platforms using the generic GSI
> implementation, we can move these APIs out to their own source file
> and eliminate the CONFIG_ACPI_GENERIC_GSI conditionality.

I think is fine because ACPI_GENERIC_GSI is not for x86 at now, please
send out the patch then we can discuss.

Thanks
Hanjun

>
> I'll send the new patch, hopefully some time tomorrow, but please let
> me know if you have concerns with this approach.
>
> Thanks,
> Agustin
>