linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] synquacer: implement ACPI gpio/interrupt support
@ 2019-05-27 11:27 Ard Biesheuvel
  2019-05-27 11:27 ` [PATCH v3 1/4] acpi/irq: implement helper to create hierachical domains Ard Biesheuvel
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2019-05-27 11:27 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-acpi, linux-gpio, Ard Biesheuvel, Masahisa Kojima,
	Linus Walleij, Marc Zyngier, Graeme Gregory, Lorenzo Pieralisi,
	Mika Westerberg, Rafael J. Wysocki, Len Brown

Wire up the existing GPIO and interrupt controller drivers to the ACPI
subsystem so they can be used on ACPI systems for ACPI event (power
button, hardware error notification etc)

Changes since v2:
- use helper to create hierarchical IRQ domains under ACPI instead of exposing
  the GSI domain's irqdomain pointer directly (#1)
- use has_acpi_companion() instead of ACPI_COMPANION() where possible (#4)
- add Mika's ack to #4

Changes since v1:
- Describe the EXIU controller as a separate device, which is a more accurate
  depiction of reality, and untangles the code a bit as well. Note that this
  requires the GPIO AML device to describe the EXIU interrupts explicitly.
- Add a patch to obtain the ACPI GSI irqdomain. The EXIU driver needs this
  to obtain the default parent domain, since the GIC is not modeled as an
  ACPI object in the namespace, and so the parent<->child link cannot be
  expressed in AML.
- Drop the Kconfig symbol for the GPIO controller. Just include the ACPI part
  when CONFIG_ACPI is defined.

Cc: Masahisa Kojima <masahisa.kojima@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Graeme Gregory <graeme.gregory@linaro.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>

Ard Biesheuvel (4):
  acpi/irq: implement helper to create hierachical domains
  irqchip/exiu: preparatory refactor for ACPI support
  irqchip/exiu: implement ACPI support
  gpio: mb86s7x: enable ACPI support

 drivers/acpi/irq.c             |  20 +++
 drivers/gpio/gpio-mb86s7x.c    |  51 ++++++-
 drivers/irqchip/irq-sni-exiu.c | 142 +++++++++++++++-----
 include/linux/acpi.h           |   7 +
 4 files changed, 182 insertions(+), 38 deletions(-)

-- 
2.20.1


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

* [PATCH v3 1/4] acpi/irq: implement helper to create hierachical domains
  2019-05-27 11:27 [PATCH v3 0/4] synquacer: implement ACPI gpio/interrupt support Ard Biesheuvel
@ 2019-05-27 11:27 ` Ard Biesheuvel
  2019-05-28 10:07   ` Mika Westerberg
                     ` (2 more replies)
  2019-05-27 11:27 ` [PATCH v3 2/4] irqchip/exiu: preparatory refactor for ACPI support Ard Biesheuvel
                   ` (3 subsequent siblings)
  4 siblings, 3 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2019-05-27 11:27 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-acpi, linux-gpio, Ard Biesheuvel, Masahisa Kojima,
	Linus Walleij, Marc Zyngier, Graeme Gregory, Lorenzo Pieralisi,
	Mika Westerberg, Rafael J. Wysocki, Len Brown

ACPI permits arbitrary producer->consumer interrupt links to be
described in AML, which means a topology such as the following
is perfectly legal:

  Device (EXIU) {
    Name (_HID, "SCX0008")
    Name (_UID, Zero)
    Name (_CRS, ResourceTemplate () {
      ...
    })
  }

  Device (GPIO) {
    Name (_HID, "SCX0007")
    Name (_UID, Zero)
    Name (_CRS, ResourceTemplate () {
      Memory32Fixed (ReadWrite, SYNQUACER_GPIO_BASE, SYNQUACER_GPIO_SIZE)
      Interrupt (ResourceConsumer, Edge, ActiveHigh, ExclusiveAndWake, 0, "\\_SB.EXIU") {
        7,
      }
    })
    ...
  }

The EXIU in this example is the external interrupt unit as can be found
on Socionext SynQuacer based platforms, which converts a block of 32 SPIs
from arbitrary polarity/trigger into level-high, with a separate set
of config/mask/unmask/clear controls.

The existing DT based driver in drivers/irqchip/irq-sni-exiu.c models
this as a hierarchical domain stacked on top of the GIC's irqdomain.
Since the GIC is modeled as a DT node as well, obtaining a reference
to this irqdomain is easily done by going through the parent link.

On ACPI systems, however, the GIC is not modeled as an object in the
namespace, and so device objects cannot refer to it directly. So in
order to obtain the irqdomain reference when driving the EXIU in ACPI
mode, we need a helper that implicitly grabs the default domain for
unqualified interrupts as the parent of the hierarchy.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/acpi/irq.c   | 20 ++++++++++++++++++++
 include/linux/acpi.h |  7 +++++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
index c3b2222e2129..39824a6bbcd5 100644
--- a/drivers/acpi/irq.c
+++ b/drivers/acpi/irq.c
@@ -295,3 +295,23 @@ void __init acpi_set_irq_model(enum acpi_irq_model_id model,
 	acpi_irq_model = model;
 	acpi_gsi_domain_id = fwnode;
 }
+
+/**
+ * acpi_irq_create_hierarchy - Create a hierarchical IRQ domain with the default
+ *                             GSI domain as its parent.
+ */
+struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
+					     unsigned int size,
+					     struct fwnode_handle *fwnode,
+					     const struct irq_domain_ops *ops,
+					     void *host_data)
+{
+	struct irq_domain *d = irq_find_matching_fwnode(acpi_gsi_domain_id,
+							DOMAIN_BUS_ANY);
+
+	if (!d)
+		return NULL;
+
+	return irq_domain_create_hierarchy(d, flags, size, fwnode, ops,
+					   host_data);
+}
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 98440df7fe42..70de4bc30cea 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -23,6 +23,7 @@
 
 #include <linux/errno.h>
 #include <linux/ioport.h>	/* for struct resource */
+#include <linux/irqdomain.h>
 #include <linux/resource_ext.h>
 #include <linux/device.h>
 #include <linux/property.h>
@@ -327,6 +328,12 @@ 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);
 
+struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
+					     unsigned int size,
+					     struct fwnode_handle *fwnode,
+					     const struct irq_domain_ops *ops,
+					     void *host_data);
+
 #ifdef CONFIG_X86_IO_APIC
 extern int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity);
 #else
-- 
2.20.1


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

* [PATCH v3 2/4] irqchip/exiu: preparatory refactor for ACPI support
  2019-05-27 11:27 [PATCH v3 0/4] synquacer: implement ACPI gpio/interrupt support Ard Biesheuvel
  2019-05-27 11:27 ` [PATCH v3 1/4] acpi/irq: implement helper to create hierachical domains Ard Biesheuvel
@ 2019-05-27 11:27 ` Ard Biesheuvel
  2019-05-27 11:27 ` [PATCH v3 3/4] irqchip/exiu: implement " Ard Biesheuvel
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2019-05-27 11:27 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-acpi, linux-gpio, Ard Biesheuvel, Masahisa Kojima,
	Linus Walleij, Marc Zyngier, Graeme Gregory, Lorenzo Pieralisi,
	Mika Westerberg, Rafael J. Wysocki, Len Brown

In preparation of adding support for EXIU controller devices described
via ACPI, split the DT init function in a DT specific and a generic part,
where the latter will be reused for ACPI support later.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/irqchip/irq-sni-exiu.c | 66 +++++++++++++-------
 1 file changed, 43 insertions(+), 23 deletions(-)

diff --git a/drivers/irqchip/irq-sni-exiu.c b/drivers/irqchip/irq-sni-exiu.c
index 1927b2f36ff6..fef7c2437dfb 100644
--- a/drivers/irqchip/irq-sni-exiu.c
+++ b/drivers/irqchip/irq-sni-exiu.c
@@ -1,7 +1,7 @@
 /*
  * Driver for Socionext External Interrupt Unit (EXIU)
  *
- * Copyright (c) 2017 Linaro, Ltd. <ard.biesheuvel@linaro.org>
+ * Copyright (c) 2017-2019 Linaro, Ltd. <ard.biesheuvel@linaro.org>
  *
  * Based on irq-tegra.c:
  *   Copyright (C) 2011 Google, Inc.
@@ -167,35 +167,23 @@ static const struct irq_domain_ops exiu_domain_ops = {
 	.free		= irq_domain_free_irqs_common,
 };
 
-static int __init exiu_init(struct device_node *node,
-			    struct device_node *parent)
+static struct exiu_irq_data *exiu_init(const struct fwnode_handle *fwnode,
+				       struct resource *res)
 {
-	struct irq_domain *parent_domain, *domain;
 	struct exiu_irq_data *data;
 	int err;
 
-	if (!parent) {
-		pr_err("%pOF: no parent, giving up\n", node);
-		return -ENODEV;
-	}
-
-	parent_domain = irq_find_host(parent);
-	if (!parent_domain) {
-		pr_err("%pOF: unable to obtain parent domain\n", node);
-		return -ENXIO;
-	}
-
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (!data)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
-	if (of_property_read_u32(node, "socionext,spi-base", &data->spi_base)) {
-		pr_err("%pOF: failed to parse 'spi-base' property\n", node);
+	if (fwnode_property_read_u32_array(fwnode, "socionext,spi-base",
+					   &data->spi_base, 1)) {
 		err = -ENODEV;
 		goto out_free;
 	}
 
-	data->base = of_iomap(node, 0);
+	data->base = ioremap(res->start, resource_size(res));
 	if (!data->base) {
 		err = -ENODEV;
 		goto out_free;
@@ -205,11 +193,44 @@ static int __init exiu_init(struct device_node *node,
 	writel_relaxed(0xFFFFFFFF, data->base + EIREQCLR);
 	writel_relaxed(0xFFFFFFFF, data->base + EIMASK);
 
+	return data;
+
+out_free:
+	kfree(data);
+	return ERR_PTR(err);
+}
+
+static int __init exiu_dt_init(struct device_node *node,
+			       struct device_node *parent)
+{
+	struct irq_domain *parent_domain, *domain;
+	struct exiu_irq_data *data;
+	struct resource res;
+
+	if (!parent) {
+		pr_err("%pOF: no parent, giving up\n", node);
+		return -ENODEV;
+	}
+
+	parent_domain = irq_find_host(parent);
+	if (!parent_domain) {
+		pr_err("%pOF: unable to obtain parent domain\n", node);
+		return -ENXIO;
+	}
+
+	if (of_address_to_resource(node, 0, &res)) {
+		pr_err("%pOF: failed to parse memory resource\n", node);
+		return -ENXIO;
+	}
+
+	data = exiu_init(of_node_to_fwnode(node), &res);
+	if (IS_ERR(data))
+		return PTR_ERR(data);
+
 	domain = irq_domain_add_hierarchy(parent_domain, 0, NUM_IRQS, node,
 					  &exiu_domain_ops, data);
 	if (!domain) {
 		pr_err("%pOF: failed to allocate domain\n", node);
-		err = -ENOMEM;
 		goto out_unmap;
 	}
 
@@ -220,8 +241,7 @@ static int __init exiu_init(struct device_node *node,
 
 out_unmap:
 	iounmap(data->base);
-out_free:
 	kfree(data);
-	return err;
+	return -ENOMEM;
 }
-IRQCHIP_DECLARE(exiu, "socionext,synquacer-exiu", exiu_init);
+IRQCHIP_DECLARE(exiu, "socionext,synquacer-exiu", exiu_dt_init);
-- 
2.20.1


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

* [PATCH v3 3/4] irqchip/exiu: implement ACPI support
  2019-05-27 11:27 [PATCH v3 0/4] synquacer: implement ACPI gpio/interrupt support Ard Biesheuvel
  2019-05-27 11:27 ` [PATCH v3 1/4] acpi/irq: implement helper to create hierachical domains Ard Biesheuvel
  2019-05-27 11:27 ` [PATCH v3 2/4] irqchip/exiu: preparatory refactor for ACPI support Ard Biesheuvel
@ 2019-05-27 11:27 ` Ard Biesheuvel
  2019-05-28 10:12   ` Mika Westerberg
  2019-05-27 11:27 ` [PATCH v3 4/4] gpio: mb86s7x: enable " Ard Biesheuvel
  2019-05-28 13:00 ` [PATCH v3 0/4] synquacer: implement ACPI gpio/interrupt support Marc Zyngier
  4 siblings, 1 reply; 14+ messages in thread
From: Ard Biesheuvel @ 2019-05-27 11:27 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-acpi, linux-gpio, Ard Biesheuvel, Masahisa Kojima,
	Linus Walleij, Marc Zyngier, Graeme Gregory, Lorenzo Pieralisi,
	Mika Westerberg, Rafael J. Wysocki, Len Brown

Expose the existing EXIU hierarchical irqchip domain code to permit
the interrupt controller to be used as the irqchip component of a
GPIO controller on ACPI systems, or as the target of ordinary
interrupt resources.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/irqchip/irq-sni-exiu.c | 76 +++++++++++++++++---
 1 file changed, 68 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-sni-exiu.c b/drivers/irqchip/irq-sni-exiu.c
index fef7c2437dfb..30a323a2b332 100644
--- a/drivers/irqchip/irq-sni-exiu.c
+++ b/drivers/irqchip/irq-sni-exiu.c
@@ -20,6 +20,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
+#include <linux/platform_device.h>
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 
@@ -134,9 +135,13 @@ static int exiu_domain_translate(struct irq_domain *domain,
 
 		*hwirq = fwspec->param[1] - info->spi_base;
 		*type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;
-		return 0;
+	} else {
+		if (fwspec->param_count != 2)
+			return -EINVAL;
+		*hwirq = fwspec->param[0];
+		*type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;
 	}
-	return -EINVAL;
+	return 0;
 }
 
 static int exiu_domain_alloc(struct irq_domain *dom, unsigned int virq,
@@ -147,16 +152,21 @@ static int exiu_domain_alloc(struct irq_domain *dom, unsigned int virq,
 	struct exiu_irq_data *info = dom->host_data;
 	irq_hw_number_t hwirq;
 
-	if (fwspec->param_count != 3)
-		return -EINVAL;	/* Not GIC compliant */
-	if (fwspec->param[0] != GIC_SPI)
-		return -EINVAL;	/* No PPI should point to this domain */
+	parent_fwspec = *fwspec;
+	if (is_of_node(dom->parent->fwnode)) {
+		if (fwspec->param_count != 3)
+			return -EINVAL;	/* Not GIC compliant */
+		if (fwspec->param[0] != GIC_SPI)
+			return -EINVAL;	/* No PPI should point to this domain */
 
+		hwirq = fwspec->param[1] - info->spi_base;
+	} else {
+		hwirq = fwspec->param[0];
+		parent_fwspec.param[0] = hwirq + info->spi_base + 32;
+	}
 	WARN_ON(nr_irqs != 1);
-	hwirq = fwspec->param[1] - info->spi_base;
 	irq_domain_set_hwirq_and_chip(dom, virq, hwirq, &exiu_irq_chip, info);
 
-	parent_fwspec = *fwspec;
 	parent_fwspec.fwnode = dom->parent->fwnode;
 	return irq_domain_alloc_irqs_parent(dom, virq, nr_irqs, &parent_fwspec);
 }
@@ -245,3 +255,53 @@ static int __init exiu_dt_init(struct device_node *node,
 	return -ENOMEM;
 }
 IRQCHIP_DECLARE(exiu, "socionext,synquacer-exiu", exiu_dt_init);
+
+#ifdef CONFIG_ACPI
+static int exiu_acpi_probe(struct platform_device *pdev)
+{
+	struct irq_domain *domain;
+	struct exiu_irq_data *data;
+	struct resource *res;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "failed to parse memory resource\n");
+		return -ENXIO;
+	}
+
+	data = exiu_init(dev_fwnode(&pdev->dev), res);
+	if (IS_ERR(data))
+		return PTR_ERR(data);
+
+	domain = acpi_irq_create_hierarchy(0, NUM_IRQS, dev_fwnode(&pdev->dev),
+					   &exiu_domain_ops, data);
+	if (!domain) {
+		dev_err(&pdev->dev, "failed to create IRQ domain\n");
+		goto out_unmap;
+	}
+
+	dev_info(&pdev->dev, "%d interrupts forwarded\n", NUM_IRQS);
+
+	return 0;
+
+out_unmap:
+	iounmap(data->base);
+	kfree(data);
+	return -ENOMEM;
+}
+
+static const struct acpi_device_id exiu_acpi_ids[] = {
+	{ "SCX0008" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(acpi, exiu_acpi_ids);
+
+static struct platform_driver exiu_driver = {
+	.driver = {
+		.name = "exiu",
+		.acpi_match_table = exiu_acpi_ids,
+	},
+	.probe = exiu_acpi_probe,
+};
+builtin_platform_driver(exiu_driver);
+#endif
-- 
2.20.1


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

* [PATCH v3 4/4] gpio: mb86s7x: enable ACPI support
  2019-05-27 11:27 [PATCH v3 0/4] synquacer: implement ACPI gpio/interrupt support Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2019-05-27 11:27 ` [PATCH v3 3/4] irqchip/exiu: implement " Ard Biesheuvel
@ 2019-05-27 11:27 ` Ard Biesheuvel
  2019-05-28  8:34   ` Linus Walleij
  2019-05-28 13:00 ` [PATCH v3 0/4] synquacer: implement ACPI gpio/interrupt support Marc Zyngier
  4 siblings, 1 reply; 14+ messages in thread
From: Ard Biesheuvel @ 2019-05-27 11:27 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-acpi, linux-gpio, Ard Biesheuvel, Masahisa Kojima,
	Linus Walleij, Marc Zyngier, Graeme Gregory, Lorenzo Pieralisi,
	Mika Westerberg, Rafael J. Wysocki, Len Brown

Make the mb86s7x GPIO block discoverable via ACPI. In addition, add
support for ACPI GPIO interrupts routed via platform interrupts, by
wiring the two together via the to_irq() gpiochip callback.

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/gpio/gpio-mb86s7x.c | 51 +++++++++++++++++---
 1 file changed, 44 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-mb86s7x.c b/drivers/gpio/gpio-mb86s7x.c
index 9308081e0a4a..64027f57a8aa 100644
--- a/drivers/gpio/gpio-mb86s7x.c
+++ b/drivers/gpio/gpio-mb86s7x.c
@@ -14,6 +14,7 @@
  *  GNU General Public License for more details.
  */
 
+#include <linux/acpi.h>
 #include <linux/io.h>
 #include <linux/init.h>
 #include <linux/clk.h>
@@ -27,6 +28,8 @@
 #include <linux/spinlock.h>
 #include <linux/slab.h>
 
+#include "gpiolib.h"
+
 /*
  * Only first 8bits of a register correspond to each pin,
  * so there are 4 registers for 32 pins.
@@ -143,6 +146,20 @@ static void mb86s70_gpio_set(struct gpio_chip *gc, unsigned gpio, int value)
 	spin_unlock_irqrestore(&gchip->lock, flags);
 }
 
+static int mb86s70_gpio_to_irq(struct gpio_chip *gc, unsigned int offset)
+{
+	int irq, index;
+
+	for (index = 0;; index++) {
+		irq = platform_get_irq(to_platform_device(gc->parent), index);
+		if (irq <= 0)
+			break;
+		if (irq_get_irq_data(irq)->hwirq == offset)
+			return irq;
+	}
+	return -EINVAL;
+}
+
 static int mb86s70_gpio_probe(struct platform_device *pdev)
 {
 	struct mb86s70_gpio_chip *gchip;
@@ -158,13 +175,15 @@ static int mb86s70_gpio_probe(struct platform_device *pdev)
 	if (IS_ERR(gchip->base))
 		return PTR_ERR(gchip->base);
 
-	gchip->clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(gchip->clk))
-		return PTR_ERR(gchip->clk);
+	if (!has_acpi_companion(&pdev->dev)) {
+		gchip->clk = devm_clk_get(&pdev->dev, NULL);
+		if (IS_ERR(gchip->clk))
+			return PTR_ERR(gchip->clk);
 
-	ret = clk_prepare_enable(gchip->clk);
-	if (ret)
-		return ret;
+		ret = clk_prepare_enable(gchip->clk);
+		if (ret)
+			return ret;
+	}
 
 	spin_lock_init(&gchip->lock);
 
@@ -180,19 +199,28 @@ static int mb86s70_gpio_probe(struct platform_device *pdev)
 	gchip->gc.parent = &pdev->dev;
 	gchip->gc.base = -1;
 
+	if (has_acpi_companion(&pdev->dev))
+		gchip->gc.to_irq = mb86s70_gpio_to_irq;
+
 	ret = gpiochip_add_data(&gchip->gc, gchip);
 	if (ret) {
 		dev_err(&pdev->dev, "couldn't register gpio driver\n");
 		clk_disable_unprepare(gchip->clk);
+		return ret;
 	}
 
-	return ret;
+	if (has_acpi_companion(&pdev->dev))
+		acpi_gpiochip_request_interrupts(&gchip->gc);
+
+	return 0;
 }
 
 static int mb86s70_gpio_remove(struct platform_device *pdev)
 {
 	struct mb86s70_gpio_chip *gchip = platform_get_drvdata(pdev);
 
+	if (has_acpi_companion(&pdev->dev))
+		acpi_gpiochip_free_interrupts(&gchip->gc);
 	gpiochip_remove(&gchip->gc);
 	clk_disable_unprepare(gchip->clk);
 
@@ -205,10 +233,19 @@ static const struct of_device_id mb86s70_gpio_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, mb86s70_gpio_dt_ids);
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id mb86s70_gpio_acpi_ids[] = {
+	{ "SCX0007" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(acpi, mb86s70_gpio_acpi_ids);
+#endif
+
 static struct platform_driver mb86s70_gpio_driver = {
 	.driver = {
 		.name = "mb86s70-gpio",
 		.of_match_table = mb86s70_gpio_dt_ids,
+		.acpi_match_table = ACPI_PTR(mb86s70_gpio_acpi_ids),
 	},
 	.probe = mb86s70_gpio_probe,
 	.remove = mb86s70_gpio_remove,
-- 
2.20.1


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

* Re: [PATCH v3 4/4] gpio: mb86s7x: enable ACPI support
  2019-05-27 11:27 ` [PATCH v3 4/4] gpio: mb86s7x: enable " Ard Biesheuvel
@ 2019-05-28  8:34   ` Linus Walleij
  2019-05-28 11:26     ` Ard Biesheuvel
  0 siblings, 1 reply; 14+ messages in thread
From: Linus Walleij @ 2019-05-28  8:34 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Linux ARM, ACPI Devel Maling List, open list:GPIO SUBSYSTEM,
	Masahisa Kojima, Marc Zyngier, Graeme Gregory, Lorenzo Pieralisi,
	Mika Westerberg, Rafael J. Wysocki, Len Brown

On Mon, May 27, 2019 at 1:27 PM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:

> Make the mb86s7x GPIO block discoverable via ACPI. In addition, add
> support for ACPI GPIO interrupts routed via platform interrupts, by
> wiring the two together via the to_irq() gpiochip callback.
>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

I assume you want to merge this through the IRQ tree or the ACPI
tree, so go ahead.

If you want me to queue the whole thing in the GPIO tree just tell
me (once we have the ACKs in place).

Yours,
Linus Walleij

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

* Re: [PATCH v3 1/4] acpi/irq: implement helper to create hierachical domains
  2019-05-27 11:27 ` [PATCH v3 1/4] acpi/irq: implement helper to create hierachical domains Ard Biesheuvel
@ 2019-05-28 10:07   ` Mika Westerberg
  2019-05-28 12:54   ` Marc Zyngier
  2019-05-28 13:02   ` Lorenzo Pieralisi
  2 siblings, 0 replies; 14+ messages in thread
From: Mika Westerberg @ 2019-05-28 10:07 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel, linux-acpi, linux-gpio, Masahisa Kojima,
	Linus Walleij, Marc Zyngier, Graeme Gregory, Lorenzo Pieralisi,
	Rafael J. Wysocki, Len Brown

On Mon, May 27, 2019 at 01:27:17PM +0200, Ard Biesheuvel wrote:
> +
> +/**
> + * acpi_irq_create_hierarchy - Create a hierarchical IRQ domain with the default
> + *                             GSI domain as its parent.

Please document the arguments as well.

> + */
> +struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
> +					     unsigned int size,
> +					     struct fwnode_handle *fwnode,
> +					     const struct irq_domain_ops *ops,
> +					     void *host_data)

Otherwise looks good to me.

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

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

* Re: [PATCH v3 3/4] irqchip/exiu: implement ACPI support
  2019-05-27 11:27 ` [PATCH v3 3/4] irqchip/exiu: implement " Ard Biesheuvel
@ 2019-05-28 10:12   ` Mika Westerberg
  0 siblings, 0 replies; 14+ messages in thread
From: Mika Westerberg @ 2019-05-28 10:12 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel, linux-acpi, linux-gpio, Masahisa Kojima,
	Linus Walleij, Marc Zyngier, Graeme Gregory, Lorenzo Pieralisi,
	Rafael J. Wysocki, Len Brown

On Mon, May 27, 2019 at 01:27:19PM +0200, Ard Biesheuvel wrote:
> Expose the existing EXIU hierarchical irqchip domain code to permit
> the interrupt controller to be used as the irqchip component of a
> GPIO controller on ACPI systems, or as the target of ordinary
> interrupt resources.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  drivers/irqchip/irq-sni-exiu.c | 76 +++++++++++++++++---
>  1 file changed, 68 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-sni-exiu.c b/drivers/irqchip/irq-sni-exiu.c
> index fef7c2437dfb..30a323a2b332 100644
> --- a/drivers/irqchip/irq-sni-exiu.c
> +++ b/drivers/irqchip/irq-sni-exiu.c
> @@ -20,6 +20,7 @@
>  #include <linux/of.h>
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
> +#include <linux/platform_device.h>
>  
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  
> @@ -134,9 +135,13 @@ static int exiu_domain_translate(struct irq_domain *domain,
>  
>  		*hwirq = fwspec->param[1] - info->spi_base;
>  		*type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;
> -		return 0;
> +	} else {
> +		if (fwspec->param_count != 2)
> +			return -EINVAL;
> +		*hwirq = fwspec->param[0];
> +		*type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;
>  	}
> -	return -EINVAL;
> +	return 0;
>  }
>  
>  static int exiu_domain_alloc(struct irq_domain *dom, unsigned int virq,
> @@ -147,16 +152,21 @@ static int exiu_domain_alloc(struct irq_domain *dom, unsigned int virq,
>  	struct exiu_irq_data *info = dom->host_data;
>  	irq_hw_number_t hwirq;
>  
> -	if (fwspec->param_count != 3)
> -		return -EINVAL;	/* Not GIC compliant */
> -	if (fwspec->param[0] != GIC_SPI)
> -		return -EINVAL;	/* No PPI should point to this domain */
> +	parent_fwspec = *fwspec;
> +	if (is_of_node(dom->parent->fwnode)) {
> +		if (fwspec->param_count != 3)
> +			return -EINVAL;	/* Not GIC compliant */
> +		if (fwspec->param[0] != GIC_SPI)
> +			return -EINVAL;	/* No PPI should point to this domain */
>  
> +		hwirq = fwspec->param[1] - info->spi_base;
> +	} else {
> +		hwirq = fwspec->param[0];
> +		parent_fwspec.param[0] = hwirq + info->spi_base + 32;
> +	}
>  	WARN_ON(nr_irqs != 1);
> -	hwirq = fwspec->param[1] - info->spi_base;
>  	irq_domain_set_hwirq_and_chip(dom, virq, hwirq, &exiu_irq_chip, info);
>  
> -	parent_fwspec = *fwspec;
>  	parent_fwspec.fwnode = dom->parent->fwnode;
>  	return irq_domain_alloc_irqs_parent(dom, virq, nr_irqs, &parent_fwspec);
>  }
> @@ -245,3 +255,53 @@ static int __init exiu_dt_init(struct device_node *node,
>  	return -ENOMEM;
>  }
>  IRQCHIP_DECLARE(exiu, "socionext,synquacer-exiu", exiu_dt_init);
> +
> +#ifdef CONFIG_ACPI
> +static int exiu_acpi_probe(struct platform_device *pdev)
> +{
> +	struct irq_domain *domain;
> +	struct exiu_irq_data *data;
> +	struct resource *res;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "failed to parse memory resource\n");
> +		return -ENXIO;
> +	}
> +
> +	data = exiu_init(dev_fwnode(&pdev->dev), res);
> +	if (IS_ERR(data))
> +		return PTR_ERR(data);
> +
> +	domain = acpi_irq_create_hierarchy(0, NUM_IRQS, dev_fwnode(&pdev->dev),
> +					   &exiu_domain_ops, data);
> +	if (!domain) {
> +		dev_err(&pdev->dev, "failed to create IRQ domain\n");
> +		goto out_unmap;
> +	}
> +
> +	dev_info(&pdev->dev, "%d interrupts forwarded\n", NUM_IRQS);

Not sure how useful this message is for the end user. Maybe dev_dbg()
instead.

Regardless,

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

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

* Re: [PATCH v3 4/4] gpio: mb86s7x: enable ACPI support
  2019-05-28  8:34   ` Linus Walleij
@ 2019-05-28 11:26     ` Ard Biesheuvel
  0 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2019-05-28 11:26 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Linux ARM, ACPI Devel Maling List, open list:GPIO SUBSYSTEM,
	Masahisa Kojima, Marc Zyngier, Graeme Gregory, Lorenzo Pieralisi,
	Mika Westerberg, Rafael J. Wysocki, Len Brown

On Tue, 28 May 2019 at 10:34, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Mon, May 27, 2019 at 1:27 PM Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>
> > Make the mb86s7x GPIO block discoverable via ACPI. In addition, add
> > support for ACPI GPIO interrupts routed via platform interrupts, by
> > wiring the two together via the to_irq() gpiochip callback.
> >
> > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>

Thanks.

> I assume you want to merge this through the IRQ tree or the ACPI
> tree, so go ahead.
>
> If you want me to queue the whole thing in the GPIO tree just tell
> me (once we have the ACKs in place).
>

Marc is willing to take the whole thing via the irqchip tree. I'll
need to apply some tweaks though, so I'll send out a v4 shortly.

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

* Re: [PATCH v3 1/4] acpi/irq: implement helper to create hierachical domains
  2019-05-27 11:27 ` [PATCH v3 1/4] acpi/irq: implement helper to create hierachical domains Ard Biesheuvel
  2019-05-28 10:07   ` Mika Westerberg
@ 2019-05-28 12:54   ` Marc Zyngier
  2019-05-28 13:12     ` Ard Biesheuvel
  2019-05-28 13:02   ` Lorenzo Pieralisi
  2 siblings, 1 reply; 14+ messages in thread
From: Marc Zyngier @ 2019-05-28 12:54 UTC (permalink / raw)
  To: Ard Biesheuvel, linux-arm-kernel
  Cc: linux-acpi, linux-gpio, Masahisa Kojima, Linus Walleij,
	Graeme Gregory, Lorenzo Pieralisi, Mika Westerberg,
	Rafael J. Wysocki, Len Brown

Hi Ard,

On 27/05/2019 12:27, Ard Biesheuvel wrote:
> ACPI permits arbitrary producer->consumer interrupt links to be
> described in AML, which means a topology such as the following
> is perfectly legal:
> 
>   Device (EXIU) {
>     Name (_HID, "SCX0008")
>     Name (_UID, Zero)
>     Name (_CRS, ResourceTemplate () {
>       ...
>     })
>   }
> 
>   Device (GPIO) {
>     Name (_HID, "SCX0007")
>     Name (_UID, Zero)
>     Name (_CRS, ResourceTemplate () {
>       Memory32Fixed (ReadWrite, SYNQUACER_GPIO_BASE, SYNQUACER_GPIO_SIZE)
>       Interrupt (ResourceConsumer, Edge, ActiveHigh, ExclusiveAndWake, 0, "\\_SB.EXIU") {
>         7,
>       }
>     })
>     ...
>   }
> 
> The EXIU in this example is the external interrupt unit as can be found
> on Socionext SynQuacer based platforms, which converts a block of 32 SPIs
> from arbitrary polarity/trigger into level-high, with a separate set
> of config/mask/unmask/clear controls.
> 
> The existing DT based driver in drivers/irqchip/irq-sni-exiu.c models
> this as a hierarchical domain stacked on top of the GIC's irqdomain.
> Since the GIC is modeled as a DT node as well, obtaining a reference
> to this irqdomain is easily done by going through the parent link.
> 
> On ACPI systems, however, the GIC is not modeled as an object in the
> namespace, and so device objects cannot refer to it directly. So in
> order to obtain the irqdomain reference when driving the EXIU in ACPI
> mode, we need a helper that implicitly grabs the default domain for
> unqualified interrupts as the parent of the hierarchy.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  drivers/acpi/irq.c   | 20 ++++++++++++++++++++
>  include/linux/acpi.h |  7 +++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
> index c3b2222e2129..39824a6bbcd5 100644
> --- a/drivers/acpi/irq.c
> +++ b/drivers/acpi/irq.c
> @@ -295,3 +295,23 @@ void __init acpi_set_irq_model(enum acpi_irq_model_id model,
>  	acpi_irq_model = model;
>  	acpi_gsi_domain_id = fwnode;
>  }
> +
> +/**
> + * acpi_irq_create_hierarchy - Create a hierarchical IRQ domain with the default
> + *                             GSI domain as its parent.
> + */
> +struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
> +					     unsigned int size,
> +					     struct fwnode_handle *fwnode,
> +					     const struct irq_domain_ops *ops,
> +					     void *host_data)
> +{
> +	struct irq_domain *d = irq_find_matching_fwnode(acpi_gsi_domain_id,
> +							DOMAIN_BUS_ANY);
> +
> +	if (!d)
> +		return NULL;
> +
> +	return irq_domain_create_hierarchy(d, flags, size, fwnode, ops,
> +					   host_data);
> +}
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 98440df7fe42..70de4bc30cea 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -23,6 +23,7 @@
>  
>  #include <linux/errno.h>
>  #include <linux/ioport.h>	/* for struct resource */
> +#include <linux/irqdomain.h>
>  #include <linux/resource_ext.h>
>  #include <linux/device.h>
>  #include <linux/property.h>
> @@ -327,6 +328,12 @@ 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);
>  
> +struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
> +					     unsigned int size,
> +					     struct fwnode_handle *fwnode,
> +					     const struct irq_domain_ops *ops,
> +					     void *host_data);
> +
>  #ifdef CONFIG_X86_IO_APIC
>  extern int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity);
>  #else
> 

Should we consider exporting this function to modules?

Otherwise (and with Mika's comments addressed), looks good to me.

Thanks,

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

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

* Re: [PATCH v3 0/4] synquacer: implement ACPI gpio/interrupt support
  2019-05-27 11:27 [PATCH v3 0/4] synquacer: implement ACPI gpio/interrupt support Ard Biesheuvel
                   ` (3 preceding siblings ...)
  2019-05-27 11:27 ` [PATCH v3 4/4] gpio: mb86s7x: enable " Ard Biesheuvel
@ 2019-05-28 13:00 ` Marc Zyngier
  4 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2019-05-28 13:00 UTC (permalink / raw)
  To: Ard Biesheuvel, linux-arm-kernel
  Cc: linux-acpi, linux-gpio, Masahisa Kojima, Linus Walleij,
	Graeme Gregory, Lorenzo Pieralisi, Mika Westerberg,
	Rafael J. Wysocki, Len Brown

Hi Ard,

On 27/05/2019 12:27, Ard Biesheuvel wrote:
> Wire up the existing GPIO and interrupt controller drivers to the ACPI
> subsystem so they can be used on ACPI systems for ACPI event (power
> button, hardware error notification etc)
> 
> Changes since v2:
> - use helper to create hierarchical IRQ domains under ACPI instead of exposing
>   the GSI domain's irqdomain pointer directly (#1)
> - use has_acpi_companion() instead of ACPI_COMPANION() where possible (#4)
> - add Mika's ack to #4
> 
> Changes since v1:
> - Describe the EXIU controller as a separate device, which is a more accurate
>   depiction of reality, and untangles the code a bit as well. Note that this
>   requires the GPIO AML device to describe the EXIU interrupts explicitly.
> - Add a patch to obtain the ACPI GSI irqdomain. The EXIU driver needs this
>   to obtain the default parent domain, since the GIC is not modeled as an
>   ACPI object in the namespace, and so the parent<->child link cannot be
>   expressed in AML.
> - Drop the Kconfig symbol for the GPIO controller. Just include the ACPI part
>   when CONFIG_ACPI is defined.

I'm quite happy with the way this is looking now. Once v4 is out, and in
the absence of any further objection, I'll queue it for 5.3.

Thanks,

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

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

* Re: [PATCH v3 1/4] acpi/irq: implement helper to create hierachical domains
  2019-05-27 11:27 ` [PATCH v3 1/4] acpi/irq: implement helper to create hierachical domains Ard Biesheuvel
  2019-05-28 10:07   ` Mika Westerberg
  2019-05-28 12:54   ` Marc Zyngier
@ 2019-05-28 13:02   ` Lorenzo Pieralisi
  2019-05-28 13:12     ` Ard Biesheuvel
  2 siblings, 1 reply; 14+ messages in thread
From: Lorenzo Pieralisi @ 2019-05-28 13:02 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel, linux-acpi, linux-gpio, Masahisa Kojima,
	Linus Walleij, Marc Zyngier, Graeme Gregory, Mika Westerberg,
	Rafael J. Wysocki, Len Brown

On Mon, May 27, 2019 at 01:27:17PM +0200, Ard Biesheuvel wrote:
> ACPI permits arbitrary producer->consumer interrupt links to be
> described in AML, which means a topology such as the following
> is perfectly legal:
> 
>   Device (EXIU) {
>     Name (_HID, "SCX0008")
>     Name (_UID, Zero)
>     Name (_CRS, ResourceTemplate () {
>       ...
>     })
>   }
> 
>   Device (GPIO) {
>     Name (_HID, "SCX0007")
>     Name (_UID, Zero)
>     Name (_CRS, ResourceTemplate () {
>       Memory32Fixed (ReadWrite, SYNQUACER_GPIO_BASE, SYNQUACER_GPIO_SIZE)
>       Interrupt (ResourceConsumer, Edge, ActiveHigh, ExclusiveAndWake, 0, "\\_SB.EXIU") {
>         7,
>       }
>     })
>     ...
>   }
> 
> The EXIU in this example is the external interrupt unit as can be found
> on Socionext SynQuacer based platforms, which converts a block of 32 SPIs
> from arbitrary polarity/trigger into level-high, with a separate set
> of config/mask/unmask/clear controls.
> 
> The existing DT based driver in drivers/irqchip/irq-sni-exiu.c models
> this as a hierarchical domain stacked on top of the GIC's irqdomain.
> Since the GIC is modeled as a DT node as well, obtaining a reference
> to this irqdomain is easily done by going through the parent link.
> 
> On ACPI systems, however, the GIC is not modeled as an object in the
> namespace, and so device objects cannot refer to it directly. So in
> order to obtain the irqdomain reference when driving the EXIU in ACPI
> mode, we need a helper that implicitly grabs the default domain for
> unqualified interrupts as the parent of the hierarchy.

Nit: I do not think they are "unqualified".

ACPI 6.3, table 6-237, Extended Interrupt Descriptor Definition:

"Resource Source: (Optional) If present, the device that uses this
descriptor consumes its resources from the resources produces by the
named device object. If not present, the device consumes its resources
out of a global pool."

Where the global pool I _assume_ is the GSI domain, so it is the default
expected behaviour (for once :))

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  drivers/acpi/irq.c   | 20 ++++++++++++++++++++
>  include/linux/acpi.h |  7 +++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
> index c3b2222e2129..39824a6bbcd5 100644
> --- a/drivers/acpi/irq.c
> +++ b/drivers/acpi/irq.c
> @@ -295,3 +295,23 @@ void __init acpi_set_irq_model(enum acpi_irq_model_id model,
>  	acpi_irq_model = model;
>  	acpi_gsi_domain_id = fwnode;
>  }
> +
> +/**
> + * acpi_irq_create_hierarchy - Create a hierarchical IRQ domain with the default
> + *                             GSI domain as its parent.

Yes please comment parameters even if it is just a wrapper around
the IRQ domain API.

> +struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
> +					     unsigned int size,
> +					     struct fwnode_handle *fwnode,
> +					     const struct irq_domain_ops *ops,
> +					     void *host_data)
> +{
> +	struct irq_domain *d = irq_find_matching_fwnode(acpi_gsi_domain_id,
> +							DOMAIN_BUS_ANY);
> +
> +	if (!d)
> +		return NULL;
> +
> +	return irq_domain_create_hierarchy(d, flags, size, fwnode, ops,
> +					   host_data);
> +}
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 98440df7fe42..70de4bc30cea 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -23,6 +23,7 @@
>  
>  #include <linux/errno.h>
>  #include <linux/ioport.h>	/* for struct resource */
> +#include <linux/irqdomain.h>
>  #include <linux/resource_ext.h>
>  #include <linux/device.h>
>  #include <linux/property.h>
> @@ -327,6 +328,12 @@ 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);
>  
> +struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
> +					     unsigned int size,
> +					     struct fwnode_handle *fwnode,
> +					     const struct irq_domain_ops *ops,
> +					     void *host_data);
> +
>  #ifdef CONFIG_X86_IO_APIC
>  extern int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity);
>  #else

Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

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

* Re: [PATCH v3 1/4] acpi/irq: implement helper to create hierachical domains
  2019-05-28 13:02   ` Lorenzo Pieralisi
@ 2019-05-28 13:12     ` Ard Biesheuvel
  0 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2019-05-28 13:12 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: linux-arm-kernel, ACPI Devel Maling List,
	open list:GPIO SUBSYSTEM, Masahisa Kojima, Linus Walleij,
	Marc Zyngier, Graeme Gregory, Mika Westerberg, Rafael J. Wysocki,
	Len Brown

On Tue, 28 May 2019 at 15:02, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> On Mon, May 27, 2019 at 01:27:17PM +0200, Ard Biesheuvel wrote:
> > ACPI permits arbitrary producer->consumer interrupt links to be
> > described in AML, which means a topology such as the following
> > is perfectly legal:
> >
> >   Device (EXIU) {
> >     Name (_HID, "SCX0008")
> >     Name (_UID, Zero)
> >     Name (_CRS, ResourceTemplate () {
> >       ...
> >     })
> >   }
> >
> >   Device (GPIO) {
> >     Name (_HID, "SCX0007")
> >     Name (_UID, Zero)
> >     Name (_CRS, ResourceTemplate () {
> >       Memory32Fixed (ReadWrite, SYNQUACER_GPIO_BASE, SYNQUACER_GPIO_SIZE)
> >       Interrupt (ResourceConsumer, Edge, ActiveHigh, ExclusiveAndWake, 0, "\\_SB.EXIU") {
> >         7,
> >       }
> >     })
> >     ...
> >   }
> >
> > The EXIU in this example is the external interrupt unit as can be found
> > on Socionext SynQuacer based platforms, which converts a block of 32 SPIs
> > from arbitrary polarity/trigger into level-high, with a separate set
> > of config/mask/unmask/clear controls.
> >
> > The existing DT based driver in drivers/irqchip/irq-sni-exiu.c models
> > this as a hierarchical domain stacked on top of the GIC's irqdomain.
> > Since the GIC is modeled as a DT node as well, obtaining a reference
> > to this irqdomain is easily done by going through the parent link.
> >
> > On ACPI systems, however, the GIC is not modeled as an object in the
> > namespace, and so device objects cannot refer to it directly. So in
> > order to obtain the irqdomain reference when driving the EXIU in ACPI
> > mode, we need a helper that implicitly grabs the default domain for
> > unqualified interrupts as the parent of the hierarchy.
>
> Nit: I do not think they are "unqualified".
>
> ACPI 6.3, table 6-237, Extended Interrupt Descriptor Definition:
>
> "Resource Source: (Optional) If present, the device that uses this
> descriptor consumes its resources from the resources produces by the
> named device object. If not present, the device consumes its resources
> out of a global pool."
>
> Where the global pool I _assume_ is the GSI domain, so it is the default
> expected behaviour (for once :))
>

By 'unqualified', I meant lacking an explicit description of the
producer of the resource. But I can change the wording if you prefer.

> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  drivers/acpi/irq.c   | 20 ++++++++++++++++++++
> >  include/linux/acpi.h |  7 +++++++
> >  2 files changed, 27 insertions(+)
> >
> > diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
> > index c3b2222e2129..39824a6bbcd5 100644
> > --- a/drivers/acpi/irq.c
> > +++ b/drivers/acpi/irq.c
> > @@ -295,3 +295,23 @@ void __init acpi_set_irq_model(enum acpi_irq_model_id model,
> >       acpi_irq_model = model;
> >       acpi_gsi_domain_id = fwnode;
> >  }
> > +
> > +/**
> > + * acpi_irq_create_hierarchy - Create a hierarchical IRQ domain with the default
> > + *                             GSI domain as its parent.
>
> Yes please comment parameters even if it is just a wrapper around
> the IRQ domain API.
>
> > +struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
> > +                                          unsigned int size,
> > +                                          struct fwnode_handle *fwnode,
> > +                                          const struct irq_domain_ops *ops,
> > +                                          void *host_data)
> > +{
> > +     struct irq_domain *d = irq_find_matching_fwnode(acpi_gsi_domain_id,
> > +                                                     DOMAIN_BUS_ANY);
> > +
> > +     if (!d)
> > +             return NULL;
> > +
> > +     return irq_domain_create_hierarchy(d, flags, size, fwnode, ops,
> > +                                        host_data);
> > +}
> > diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> > index 98440df7fe42..70de4bc30cea 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -23,6 +23,7 @@
> >
> >  #include <linux/errno.h>
> >  #include <linux/ioport.h>    /* for struct resource */
> > +#include <linux/irqdomain.h>
> >  #include <linux/resource_ext.h>
> >  #include <linux/device.h>
> >  #include <linux/property.h>
> > @@ -327,6 +328,12 @@ 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);
> >
> > +struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
> > +                                          unsigned int size,
> > +                                          struct fwnode_handle *fwnode,
> > +                                          const struct irq_domain_ops *ops,
> > +                                          void *host_data);
> > +
> >  #ifdef CONFIG_X86_IO_APIC
> >  extern int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity);
> >  #else
>
> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

Thanks,

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

* Re: [PATCH v3 1/4] acpi/irq: implement helper to create hierachical domains
  2019-05-28 12:54   ` Marc Zyngier
@ 2019-05-28 13:12     ` Ard Biesheuvel
  0 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2019-05-28 13:12 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-arm-kernel, ACPI Devel Maling List,
	open list:GPIO SUBSYSTEM, Masahisa Kojima, Linus Walleij,
	Graeme Gregory, Lorenzo Pieralisi, Mika Westerberg,
	Rafael J. Wysocki, Len Brown

On Tue, 28 May 2019 at 14:54, Marc Zyngier <marc.zyngier@arm.com> wrote:
>
> Hi Ard,
>
> On 27/05/2019 12:27, Ard Biesheuvel wrote:
> > ACPI permits arbitrary producer->consumer interrupt links to be
> > described in AML, which means a topology such as the following
> > is perfectly legal:
> >
> >   Device (EXIU) {
> >     Name (_HID, "SCX0008")
> >     Name (_UID, Zero)
> >     Name (_CRS, ResourceTemplate () {
> >       ...
> >     })
> >   }
> >
> >   Device (GPIO) {
> >     Name (_HID, "SCX0007")
> >     Name (_UID, Zero)
> >     Name (_CRS, ResourceTemplate () {
> >       Memory32Fixed (ReadWrite, SYNQUACER_GPIO_BASE, SYNQUACER_GPIO_SIZE)
> >       Interrupt (ResourceConsumer, Edge, ActiveHigh, ExclusiveAndWake, 0, "\\_SB.EXIU") {
> >         7,
> >       }
> >     })
> >     ...
> >   }
> >
> > The EXIU in this example is the external interrupt unit as can be found
> > on Socionext SynQuacer based platforms, which converts a block of 32 SPIs
> > from arbitrary polarity/trigger into level-high, with a separate set
> > of config/mask/unmask/clear controls.
> >
> > The existing DT based driver in drivers/irqchip/irq-sni-exiu.c models
> > this as a hierarchical domain stacked on top of the GIC's irqdomain.
> > Since the GIC is modeled as a DT node as well, obtaining a reference
> > to this irqdomain is easily done by going through the parent link.
> >
> > On ACPI systems, however, the GIC is not modeled as an object in the
> > namespace, and so device objects cannot refer to it directly. So in
> > order to obtain the irqdomain reference when driving the EXIU in ACPI
> > mode, we need a helper that implicitly grabs the default domain for
> > unqualified interrupts as the parent of the hierarchy.
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  drivers/acpi/irq.c   | 20 ++++++++++++++++++++
> >  include/linux/acpi.h |  7 +++++++
> >  2 files changed, 27 insertions(+)
> >
> > diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
> > index c3b2222e2129..39824a6bbcd5 100644
> > --- a/drivers/acpi/irq.c
> > +++ b/drivers/acpi/irq.c
> > @@ -295,3 +295,23 @@ void __init acpi_set_irq_model(enum acpi_irq_model_id model,
> >       acpi_irq_model = model;
> >       acpi_gsi_domain_id = fwnode;
> >  }
> > +
> > +/**
> > + * acpi_irq_create_hierarchy - Create a hierarchical IRQ domain with the default
> > + *                             GSI domain as its parent.
> > + */
> > +struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
> > +                                          unsigned int size,
> > +                                          struct fwnode_handle *fwnode,
> > +                                          const struct irq_domain_ops *ops,
> > +                                          void *host_data)
> > +{
> > +     struct irq_domain *d = irq_find_matching_fwnode(acpi_gsi_domain_id,
> > +                                                     DOMAIN_BUS_ANY);
> > +
> > +     if (!d)
> > +             return NULL;
> > +
> > +     return irq_domain_create_hierarchy(d, flags, size, fwnode, ops,
> > +                                        host_data);
> > +}
> > diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> > index 98440df7fe42..70de4bc30cea 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -23,6 +23,7 @@
> >
> >  #include <linux/errno.h>
> >  #include <linux/ioport.h>    /* for struct resource */
> > +#include <linux/irqdomain.h>
> >  #include <linux/resource_ext.h>
> >  #include <linux/device.h>
> >  #include <linux/property.h>
> > @@ -327,6 +328,12 @@ 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);
> >
> > +struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
> > +                                          unsigned int size,
> > +                                          struct fwnode_handle *fwnode,
> > +                                          const struct irq_domain_ops *ops,
> > +                                          void *host_data);
> > +
> >  #ifdef CONFIG_X86_IO_APIC
> >  extern int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity);
> >  #else
> >
>
> Should we consider exporting this function to modules?
>

Good point, we probably should.


> Otherwise (and with Mika's comments addressed), looks good to me.
>
> Thanks,
>
>         M.
> --
> Jazz is not dead. It just smells funny...

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

end of thread, other threads:[~2019-05-28 13:12 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-27 11:27 [PATCH v3 0/4] synquacer: implement ACPI gpio/interrupt support Ard Biesheuvel
2019-05-27 11:27 ` [PATCH v3 1/4] acpi/irq: implement helper to create hierachical domains Ard Biesheuvel
2019-05-28 10:07   ` Mika Westerberg
2019-05-28 12:54   ` Marc Zyngier
2019-05-28 13:12     ` Ard Biesheuvel
2019-05-28 13:02   ` Lorenzo Pieralisi
2019-05-28 13:12     ` Ard Biesheuvel
2019-05-27 11:27 ` [PATCH v3 2/4] irqchip/exiu: preparatory refactor for ACPI support Ard Biesheuvel
2019-05-27 11:27 ` [PATCH v3 3/4] irqchip/exiu: implement " Ard Biesheuvel
2019-05-28 10:12   ` Mika Westerberg
2019-05-27 11:27 ` [PATCH v3 4/4] gpio: mb86s7x: enable " Ard Biesheuvel
2019-05-28  8:34   ` Linus Walleij
2019-05-28 11:26     ` Ard Biesheuvel
2019-05-28 13:00 ` [PATCH v3 0/4] synquacer: implement ACPI gpio/interrupt support Marc Zyngier

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