linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/14] ACPI platform MSI, interrupt producer/consumer and its example mbi-gen
@ 2016-10-25 15:09 Hanjun Guo
  2016-10-25 15:09 ` [PATCH v3 01/14] ACPI: ARM64: IORT: minor cleanup for iort_match_node_callback() Hanjun Guo
                   ` (13 more replies)
  0 siblings, 14 replies; 20+ messages in thread
From: Hanjun Guo @ 2016-10-25 15:09 UTC (permalink / raw)
  To: Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi
  Cc: linux-acpi, linux-arm-kernel, linux-kernel, Thomas Gleixner,
	Greg KH, Tomasz Nowicki, Ma Jun, Kefeng Wang, Agustin Vega-Frias,
	Sinan Kaya, G Gregory, Charles Garcia-Tobin, huxinwei, yimin,
	linuxarm, Hanjun Guo

From: Hanjun Guo <hanjun.guo@linaro.org>

This patch series is v3 of a previous posting,

https://lkml.org/lkml/2016/9/14/510

v2 -> v3:
	- Drop RFC tag
	- Rebase against v4.9-rc2 and Lorenzo's v6 of ACPI IORT ARM SMMU support [1]
	- Add 3 cleanup patches (patch 1, 2, 3)
	- Drop arch_init call patch from last version
	- Introduce a callback for platform device to set msi domain
	- Introduce a new API to get paltform device's domain instead of
	  reusing the PCI one in previous version
	- Add a patch to rework iort_node_get_id()

[1]: http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1251993.html

v2 -> v1:
	- Fix the bug of if multi Interrupt() resoures in single _PRS,
          we need to calculate all the irq numbers (I missed it in previous
          version);
	- Rebased on Marc's irq/irqchip-4.9 branch and Lorenzo's v5
	  SMMU patches (also Robin's SMMu patches)
	- Add patch irqchip: mbigen: promote mbigen init.

With platform msi support landed in the kernel, and the introduction
of IORT for GICv3 ITS (PCI MSI) [1], the framework for platform msi
is ready, this patch set add few patches to enable the ACPI platform
msi support.

For platform device connecting to ITS on arm platform, we have IORT
table with the named componant node to describe the mappings of paltform
device and ITS, so we can retrieve the dev id and find its parent
irqdomain (ITS) from IORT table (simlar with the ACPI ITS support).

With acpi platform msi supported, we add the ACPI support for irqchip
mbi-gen, which use this framework to form its stacked irqdomain, below
is the mbi-gen's topology in the system:


  | ---------------|                  |------------------|
  |                |       MSI        |                  |     wired interrupt(s)
  |       ITS      |<-----------------|   MBI-GEN        |<----------------------------- IO device(s)
  |                |                  |                  |<----------------------------- IO device(s)
  ------------------                  -------------------

So with ACPI platform MSI support, we can build the stacked domain for
mbi-gen which represented its mappings with the named componant in IORT,
but we still missing the connectings of devices to mbi-gen as in ACPI
world devices connect to main interrupt controller in MADT in default.

In ACPI 6.1, section 19.6.62, Interrupt Resource Descriptor Macro,

    Interrupt (ResourceUsage, EdgeLevel, ActiveLevel, Shared,
    ResourceSourceIndex, ResourceSource, DescriptorName)
    { InterruptList } => Buffer
    
For the arguement ResourceUsage and DescriptorName, which means:
    
ResourceUsage describes whether the device consumes the specified
interrupt ( ResourceConsumer ) or produces it for use by a child
device ( ResourceProducer ).
If nothing is specified, then ResourceConsumer is assumed.
    
DescriptorName evaluates to a name string which refers to the
entire resource descriptor.
    
So it can be used for devices connecting to a specific interrupt
prodcucer instead of the main interrupt controller in MADT, we can
define:

Interrupt(ResourceConsumer,..., "\_SB.IRQP") {12,14,....},

then get the interrupt producer with the full path name "\_SB.IRQP".

Thanks
Hanjun

Hanjun Guo (12):
  ACPI: ARM64: IORT: minor cleanup for iort_match_node_callback()
  irqchip: gic-v3-its: keep the head file include in alphabetic order
  ACPI: ARM64: IORT: add missing comment for iort_dev_find_its_id()
  irqchip: gicv3-its: platform-msi: refactor its_pmsi_prepare()
  ACPI: platform-msi: retrieve dev id from IORT
  irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare
    for ACPI
  irqchip: gicv3-its: platform-msi: scan MADT to create platform msi
    domain
  ACPI: ARM64: IORT: rework iort_node_get_id()
  ACPI: platform: setup MSI domain for ACPI based platform device
  msi: platform: make platform_msi_create_device_domain() ACPI aware
  ACPI: irq: introduce interrupt producer
  irqchip: mbigen: Add ACPI support

Kefeng Wang (2):
  irqchip: mbigen: drop module owner
  irqchip: mbigen: introduce mbigen_of_create_domain()

 drivers/acpi/acpi_platform.c                  |  11 +++
 drivers/acpi/arm64/iort.c                     |  83 ++++++++++++++++++--
 drivers/acpi/gsi.c                            |  10 ++-
 drivers/acpi/resource.c                       |  85 ++++++++++++++------
 drivers/base/platform-msi.c                   |   3 +-
 drivers/base/platform.c                       |   3 +
 drivers/irqchip/irq-gic-v3-its-platform-msi.c | 106 +++++++++++++++++++------
 drivers/irqchip/irq-gic-v3-its.c              |   3 +-
 drivers/irqchip/irq-mbigen.c                  | 109 ++++++++++++++++++++++----
 include/acpi/acpi_bus.h                       |   1 +
 include/linux/acpi_iort.h                     |  11 +++
 include/linux/platform_device.h               |   3 +
 12 files changed, 346 insertions(+), 82 deletions(-)

-- 
1.7.12.4

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

* [PATCH v3 01/14] ACPI: ARM64: IORT: minor cleanup for iort_match_node_callback()
  2016-10-25 15:09 [PATCH v3 00/14] ACPI platform MSI, interrupt producer/consumer and its example mbi-gen Hanjun Guo
@ 2016-10-25 15:09 ` Hanjun Guo
  2016-10-25 15:09 ` [PATCH v3 02/14] irqchip: gic-v3-its: keep the head file include in alphabetic order Hanjun Guo
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Hanjun Guo @ 2016-10-25 15:09 UTC (permalink / raw)
  To: Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi
  Cc: linux-acpi, linux-arm-kernel, linux-kernel, Thomas Gleixner,
	Greg KH, Tomasz Nowicki, Ma Jun, Kefeng Wang, Agustin Vega-Frias,
	Sinan Kaya, G Gregory, Charles Garcia-Tobin, huxinwei, yimin,
	linuxarm, Hanjun Guo

From: Hanjun Guo <hanjun.guo@linaro.org>

Cleanup iort_match_node_callback() a little bit to reduce
some lines of code, aslo fix the indentation in iort_scan_node().

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Tomasz Nowicki <tn@semihalf.com>
---
 drivers/acpi/arm64/iort.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 5741bfd..ee68cfc 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -225,7 +225,7 @@ static struct acpi_iort_node *iort_scan_node(enum acpi_iort_node_type type,
 
 		if (iort_node->type == type &&
 		    ACPI_SUCCESS(callback(iort_node, context)))
-				return iort_node;
+			return iort_node;
 
 		iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
 					 iort_node->length);
@@ -253,17 +253,15 @@ static acpi_status iort_match_node_callback(struct acpi_iort_node *node,
 					    void *context)
 {
 	struct device *dev = context;
-	acpi_status status;
+	acpi_status status = AE_NOT_FOUND;
 
 	if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT) {
 		struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
 		struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
 		struct acpi_iort_named_component *ncomp;
 
-		if (!adev) {
-			status = AE_NOT_FOUND;
+		if (!adev)
 			goto out;
-		}
 
 		status = acpi_get_name(adev->handle, ACPI_FULL_PATHNAME, &buf);
 		if (ACPI_FAILURE(status)) {
@@ -289,8 +287,6 @@ static acpi_status iort_match_node_callback(struct acpi_iort_node *node,
 		 */
 		status = pci_rc->pci_segment_number == pci_domain_nr(bus) ?
 							AE_OK : AE_NOT_FOUND;
-	} else {
-		status = AE_NOT_FOUND;
 	}
 out:
 	return status;
-- 
1.7.12.4

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

* [PATCH v3 02/14] irqchip: gic-v3-its: keep the head file include in alphabetic order
  2016-10-25 15:09 [PATCH v3 00/14] ACPI platform MSI, interrupt producer/consumer and its example mbi-gen Hanjun Guo
  2016-10-25 15:09 ` [PATCH v3 01/14] ACPI: ARM64: IORT: minor cleanup for iort_match_node_callback() Hanjun Guo
@ 2016-10-25 15:09 ` Hanjun Guo
  2016-10-25 15:09 ` [PATCH v3 03/14] ACPI: ARM64: IORT: add missing comment for iort_dev_find_its_id() Hanjun Guo
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Hanjun Guo @ 2016-10-25 15:09 UTC (permalink / raw)
  To: Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi
  Cc: linux-acpi, linux-arm-kernel, linux-kernel, Thomas Gleixner,
	Greg KH, Tomasz Nowicki, Ma Jun, Kefeng Wang, Agustin Vega-Frias,
	Sinan Kaya, G Gregory, Charles Garcia-Tobin, huxinwei, yimin,
	linuxarm, Hanjun Guo

From: Hanjun Guo <hanjun.guo@linaro.org>

The head file is strictly in alphabetic order now, so let's
be the rule breaker. As acpi_iort.h includes acpi.h so remove
the duplidate acpi.h inclusion as well.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Tomasz Nowicki <tn@semihalf.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index c5dee30..1508bf4 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -15,14 +15,13 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <linux/acpi.h>
+#include <linux/acpi_iort.h>
 #include <linux/bitmap.h>
 #include <linux/cpu.h>
 #include <linux/delay.h>
 #include <linux/dma-iommu.h>
 #include <linux/interrupt.h>
 #include <linux/irqdomain.h>
-#include <linux/acpi_iort.h>
 #include <linux/log2.h>
 #include <linux/mm.h>
 #include <linux/msi.h>
-- 
1.7.12.4

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

* [PATCH v3 03/14] ACPI: ARM64: IORT: add missing comment for iort_dev_find_its_id()
  2016-10-25 15:09 [PATCH v3 00/14] ACPI platform MSI, interrupt producer/consumer and its example mbi-gen Hanjun Guo
  2016-10-25 15:09 ` [PATCH v3 01/14] ACPI: ARM64: IORT: minor cleanup for iort_match_node_callback() Hanjun Guo
  2016-10-25 15:09 ` [PATCH v3 02/14] irqchip: gic-v3-its: keep the head file include in alphabetic order Hanjun Guo
@ 2016-10-25 15:09 ` Hanjun Guo
  2016-10-25 15:09 ` [PATCH v3 04/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_prepare() Hanjun Guo
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Hanjun Guo @ 2016-10-25 15:09 UTC (permalink / raw)
  To: Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi
  Cc: linux-acpi, linux-arm-kernel, linux-kernel, Thomas Gleixner,
	Greg KH, Tomasz Nowicki, Ma Jun, Kefeng Wang, Agustin Vega-Frias,
	Sinan Kaya, G Gregory, Charles Garcia-Tobin, huxinwei, yimin,
	linuxarm, Hanjun Guo

From: Hanjun Guo <hanjun.guo@linaro.org>

We are missing req_id's comment for iort_dev_find_its_id(),
add it back.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Tomasz Nowicki <tn@semihalf.com>
---
 drivers/acpi/arm64/iort.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index ee68cfc..03bae2c 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -446,6 +446,7 @@ u32 iort_msi_map_rid(struct device *dev, u32 req_id)
 /**
  * iort_dev_find_its_id() - Find the ITS identifier for a device
  * @dev: The device.
+ * @req_id: Device's Requster ID
  * @idx: Index of the ITS identifier list.
  * @its_id: ITS identifier.
  *
-- 
1.7.12.4

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

* [PATCH v3 04/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_prepare()
  2016-10-25 15:09 [PATCH v3 00/14] ACPI platform MSI, interrupt producer/consumer and its example mbi-gen Hanjun Guo
                   ` (2 preceding siblings ...)
  2016-10-25 15:09 ` [PATCH v3 03/14] ACPI: ARM64: IORT: add missing comment for iort_dev_find_its_id() Hanjun Guo
@ 2016-10-25 15:09 ` Hanjun Guo
  2016-10-25 15:09 ` [PATCH v3 05/14] ACPI: platform-msi: retrieve dev id from IORT Hanjun Guo
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Hanjun Guo @ 2016-10-25 15:09 UTC (permalink / raw)
  To: Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi
  Cc: linux-acpi, linux-arm-kernel, linux-kernel, Thomas Gleixner,
	Greg KH, Tomasz Nowicki, Ma Jun, Kefeng Wang, Agustin Vega-Frias,
	Sinan Kaya, G Gregory, Charles Garcia-Tobin, huxinwei, yimin,
	linuxarm, Hanjun Guo

From: Hanjun Guo <hanjun.guo@linaro.org>

Adding ACPI support for platform MSI, we need to retrieve the
dev id in ACPI way instead of device tree, we already have
a well formed function its_pmsi_prepare() to get the dev id
but it's OF dependent, so collect OF related code and put them
into a single function to make its_pmsi_prepare() more friendly
to ACPI later.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Sinan Kaya <okaya@codeaurora.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Tomasz Nowicki <tn@semihalf.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/irq-gic-v3-its-platform-msi.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its-platform-msi.c b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
index 470b4aa..3c94278 100644
--- a/drivers/irqchip/irq-gic-v3-its-platform-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
@@ -24,15 +24,11 @@
 	.name			= "ITS-pMSI",
 };
 
-static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
-			    int nvec, msi_alloc_info_t *info)
+static int of_pmsi_get_dev_id(struct irq_domain *domain, struct device *dev,
+				  u32 *dev_id)
 {
-	struct msi_domain_info *msi_info;
-	u32 dev_id;
 	int ret, index = 0;
 
-	msi_info = msi_get_domain_info(domain->parent);
-
 	/* Suck the DeviceID out of the msi-parent property */
 	do {
 		struct of_phandle_args args;
@@ -43,11 +39,24 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
 		if (args.np == irq_domain_get_of_node(domain)) {
 			if (WARN_ON(args.args_count != 1))
 				return -EINVAL;
-			dev_id = args.args[0];
+			*dev_id = args.args[0];
 			break;
 		}
 	} while (!ret);
 
+	return ret;
+}
+
+static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
+			    int nvec, msi_alloc_info_t *info)
+{
+	struct msi_domain_info *msi_info;
+	u32 dev_id;
+	int ret;
+
+	msi_info = msi_get_domain_info(domain->parent);
+
+	ret = of_pmsi_get_dev_id(domain, dev, &dev_id);
 	if (ret)
 		return ret;
 
-- 
1.7.12.4

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

* [PATCH v3 05/14] ACPI: platform-msi: retrieve dev id from IORT
  2016-10-25 15:09 [PATCH v3 00/14] ACPI platform MSI, interrupt producer/consumer and its example mbi-gen Hanjun Guo
                   ` (3 preceding siblings ...)
  2016-10-25 15:09 ` [PATCH v3 04/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_prepare() Hanjun Guo
@ 2016-10-25 15:09 ` Hanjun Guo
  2016-10-25 15:09 ` [PATCH v3 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI Hanjun Guo
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Hanjun Guo @ 2016-10-25 15:09 UTC (permalink / raw)
  To: Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi
  Cc: linux-acpi, linux-arm-kernel, linux-kernel, Thomas Gleixner,
	Greg KH, Tomasz Nowicki, Ma Jun, Kefeng Wang, Agustin Vega-Frias,
	Sinan Kaya, G Gregory, Charles Garcia-Tobin, huxinwei, yimin,
	linuxarm, Hanjun Guo

From: Hanjun Guo <hanjun.guo@linaro.org>

For devices connecting to ITS, it needs dev id to identify
itself, and this dev id is represented in the IORT table in
named componant node [1] for platform devices, so in this
patch we will scan the IORT to retrieve device's dev id.

Introduce iort_pmsi_get_dev_id() with pointer dev passed
in for that purpose.

[1]: https://static.docs.arm.com/den0049/b/DEN0049B_IO_Remapping_Table.pdf

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Sinan Kaya <okaya@codeaurora.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Tomasz Nowicki <tn@semihalf.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/acpi/arm64/iort.c                     | 26 ++++++++++++++++++++++++++
 drivers/irqchip/irq-gic-v3-its-platform-msi.c |  4 +++-
 include/linux/acpi_iort.h                     |  8 ++++++++
 3 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 03bae2c..7d543aa 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -444,6 +444,32 @@ u32 iort_msi_map_rid(struct device *dev, u32 req_id)
 }
 
 /**
+ * iort_pmsi_get_dev_id() - Get the device id for a device
+ * @dev: The device for which the mapping is to be done.
+ * @dev_id: The device ID found.
+ *
+ * Returns: 0 for successful find a dev id, errors otherwise
+ */
+int iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id)
+{
+	struct acpi_iort_node *node;
+
+	if (!iort_table)
+		return -ENODEV;
+
+	node = iort_find_dev_node(dev);
+	if (!node) {
+		dev_err(dev, "can't find related IORT node\n");
+		return -ENODEV;
+	}
+
+	if(!iort_node_get_id(node, dev_id, IORT_MSI_TYPE, 0))
+		return -ENODEV;
+
+	return 0;
+}
+
+/**
  * iort_dev_find_its_id() - Find the ITS identifier for a device
  * @dev: The device.
  * @req_id: Device's Requster ID
diff --git a/drivers/irqchip/irq-gic-v3-its-platform-msi.c b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
index 3c94278..16587a9 100644
--- a/drivers/irqchip/irq-gic-v3-its-platform-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
@@ -15,6 +15,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/acpi_iort.h>
 #include <linux/device.h>
 #include <linux/msi.h>
 #include <linux/of.h>
@@ -56,7 +57,8 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
 
 	msi_info = msi_get_domain_info(domain->parent);
 
-	ret = of_pmsi_get_dev_id(domain, dev, &dev_id);
+	ret = dev->of_node ? of_pmsi_get_dev_id(domain, dev, &dev_id) :
+		iort_pmsi_get_dev_id(dev, &dev_id);
 	if (ret)
 		return ret;
 
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index dcb2b60..3f717cc 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -33,6 +33,7 @@
 void acpi_iort_init(void);
 bool iort_node_match(u8 type);
 u32 iort_msi_map_rid(struct device *dev, u32 req_id);
+int iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id);
 struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id);
 /* IOMMU interface */
 const struct iommu_ops *iort_iommu_configure(struct device *dev);
@@ -41,9 +42,16 @@ static inline void acpi_iort_init(void) { }
 static inline bool iort_node_match(u8 type) { return false; }
 static inline u32 iort_msi_map_rid(struct device *dev, u32 req_id)
 { return req_id; }
+
 static inline struct irq_domain *iort_get_device_domain(struct device *dev,
 							u32 req_id)
 { return NULL; }
+
+static inline int iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id)
+{
+	return -ENODEV;
+}
+
 /* IOMMU interface */
 static inline
 const struct iommu_ops *iort_iommu_configure(struct device *dev)
-- 
1.7.12.4

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

* [PATCH v3 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI
  2016-10-25 15:09 [PATCH v3 00/14] ACPI platform MSI, interrupt producer/consumer and its example mbi-gen Hanjun Guo
                   ` (4 preceding siblings ...)
  2016-10-25 15:09 ` [PATCH v3 05/14] ACPI: platform-msi: retrieve dev id from IORT Hanjun Guo
@ 2016-10-25 15:09 ` Hanjun Guo
  2016-10-25 15:09 ` [PATCH v3 07/14] irqchip: gicv3-its: platform-msi: scan MADT to create platform msi domain Hanjun Guo
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Hanjun Guo @ 2016-10-25 15:09 UTC (permalink / raw)
  To: Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi
  Cc: linux-acpi, linux-arm-kernel, linux-kernel, Thomas Gleixner,
	Greg KH, Tomasz Nowicki, Ma Jun, Kefeng Wang, Agustin Vega-Frias,
	Sinan Kaya, G Gregory, Charles Garcia-Tobin, huxinwei, yimin,
	linuxarm, Hanjun Guo

From: Hanjun Guo <hanjun.guo@linaro.org>

Introduce its_pmsi_init_one() to refactor the code to isolate
ACPI&DT common code to prepare for ACPI later.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Sinan Kaya <okaya@codeaurora.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Tomasz Nowicki <tn@semihalf.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/irq-gic-v3-its-platform-msi.c | 45 ++++++++++++++++-----------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its-platform-msi.c b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
index 16587a9..ff72704 100644
--- a/drivers/irqchip/irq-gic-v3-its-platform-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
@@ -84,34 +84,43 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
 	{},
 };
 
-static int __init its_pmsi_init(void)
+static int __init its_pmsi_init_one(struct fwnode_handle *fwnode,
+				const char *name)
 {
-	struct device_node *np;
 	struct irq_domain *parent;
 
+	parent = irq_find_matching_fwnode(fwnode, DOMAIN_BUS_NEXUS);
+	if (!parent || !msi_get_domain_info(parent)) {
+		pr_err("%s: unable to locate ITS domain\n", name);
+		return -ENXIO;
+	}
+
+	if (!platform_msi_create_irq_domain(fwnode, &its_pmsi_domain_info,
+					    parent)) {
+		pr_err("%s: unable to create platform domain\n", name);
+		return -ENXIO;
+	}
+
+	pr_info("Platform MSI: %s domain created\n", name);
+	return 0;
+}
+
+static void __init its_pmsi_of_init(void)
+{
+	struct device_node *np;
+
 	for (np = of_find_matching_node(NULL, its_device_id); np;
 	     np = of_find_matching_node(np, its_device_id)) {
 		if (!of_property_read_bool(np, "msi-controller"))
 			continue;
 
-		parent = irq_find_matching_host(np, DOMAIN_BUS_NEXUS);
-		if (!parent || !msi_get_domain_info(parent)) {
-			pr_err("%s: unable to locate ITS domain\n",
-			       np->full_name);
-			continue;
-		}
-
-		if (!platform_msi_create_irq_domain(of_node_to_fwnode(np),
-						    &its_pmsi_domain_info,
-						    parent)) {
-			pr_err("%s: unable to create platform domain\n",
-			       np->full_name);
-			continue;
-		}
-
-		pr_info("Platform MSI: %s domain created\n", np->full_name);
+		its_pmsi_init_one(of_node_to_fwnode(np), np->full_name);
 	}
+}
 
+static int __init its_pmsi_init(void)
+{
+	its_pmsi_of_init();
 	return 0;
 }
 early_initcall(its_pmsi_init);
-- 
1.7.12.4

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

* [PATCH v3 07/14] irqchip: gicv3-its: platform-msi: scan MADT to create platform msi domain
  2016-10-25 15:09 [PATCH v3 00/14] ACPI platform MSI, interrupt producer/consumer and its example mbi-gen Hanjun Guo
                   ` (5 preceding siblings ...)
  2016-10-25 15:09 ` [PATCH v3 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI Hanjun Guo
@ 2016-10-25 15:09 ` Hanjun Guo
  2016-10-25 15:09 ` [PATCH v3 08/14] ACPI: ARM64: IORT: rework iort_node_get_id() Hanjun Guo
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Hanjun Guo @ 2016-10-25 15:09 UTC (permalink / raw)
  To: Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi
  Cc: linux-acpi, linux-arm-kernel, linux-kernel, Thomas Gleixner,
	Greg KH, Tomasz Nowicki, Ma Jun, Kefeng Wang, Agustin Vega-Frias,
	Sinan Kaya, G Gregory, Charles Garcia-Tobin, huxinwei, yimin,
	linuxarm, Hanjun Guo

From: Hanjun Guo <hanjun.guo@linaro.org>

With the introduction of its_pmsi_init_one(), we can add some code
on top for ACPI support of platform MSI.

We are scanning the MADT table to get the ITS entry(ies), then use
the information to create the platform msi domain for devices connect
to it, just like the PCI MSI for ITS did.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Sinan Kaya <okaya@codeaurora.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Tomasz Nowicki <tn@semihalf.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/irq-gic-v3-its-platform-msi.c | 36 +++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3-its-platform-msi.c b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
index ff72704..0be0437 100644
--- a/drivers/irqchip/irq-gic-v3-its-platform-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
@@ -105,6 +105,41 @@ static int __init its_pmsi_init_one(struct fwnode_handle *fwnode,
 	return 0;
 }
 
+#ifdef CONFIG_ACPI
+static int __init
+its_pmsi_parse_madt(struct acpi_subtable_header *header,
+			const unsigned long end)
+{
+	struct acpi_madt_generic_translator *its_entry;
+	struct fwnode_handle *domain_handle;
+	const char *node_name;
+	int err = -ENXIO;
+
+	its_entry = (struct acpi_madt_generic_translator *)header;
+	node_name = kasprintf(GFP_KERNEL, "ITS@0x%lx",
+			      (long)its_entry->base_address);
+	domain_handle = iort_find_domain_token(its_entry->translation_id);
+	if (!domain_handle) {
+		pr_err("%s: Unable to locate ITS domain handle\n", node_name);
+		goto out;
+	}
+
+	err = its_pmsi_init_one(domain_handle, node_name);
+
+out:
+	kfree(node_name);
+	return err;
+}
+
+static void __init its_acpi_pmsi_init(void)
+{
+	acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
+			      its_pmsi_parse_madt, 0);
+}
+#else
+static inline void its_acpi_pmsi_init(void) { }
+#endif
+
 static void __init its_pmsi_of_init(void)
 {
 	struct device_node *np;
@@ -121,6 +156,7 @@ static void __init its_pmsi_of_init(void)
 static int __init its_pmsi_init(void)
 {
 	its_pmsi_of_init();
+	its_acpi_pmsi_init();
 	return 0;
 }
 early_initcall(its_pmsi_init);
-- 
1.7.12.4

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

* [PATCH v3 08/14] ACPI: ARM64: IORT: rework iort_node_get_id()
  2016-10-25 15:09 [PATCH v3 00/14] ACPI platform MSI, interrupt producer/consumer and its example mbi-gen Hanjun Guo
                   ` (6 preceding siblings ...)
  2016-10-25 15:09 ` [PATCH v3 07/14] irqchip: gicv3-its: platform-msi: scan MADT to create platform msi domain Hanjun Guo
@ 2016-10-25 15:09 ` Hanjun Guo
  2016-10-25 15:09 ` [PATCH v3 09/14] ACPI: platform: setup MSI domain for ACPI based platform device Hanjun Guo
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Hanjun Guo @ 2016-10-25 15:09 UTC (permalink / raw)
  To: Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi
  Cc: linux-acpi, linux-arm-kernel, linux-kernel, Thomas Gleixner,
	Greg KH, Tomasz Nowicki, Ma Jun, Kefeng Wang, Agustin Vega-Frias,
	Sinan Kaya, G Gregory, Charles Garcia-Tobin, huxinwei, yimin,
	linuxarm, Hanjun Guo

From: Hanjun Guo <hanjun.guo@linaro.org>

iort_node_get_id() has two output, one is the mapped ids,
the other is the referenced parent node which is returned
from the function.

For now we need a API just return its parent node for
single mapping, so just update this function slightly then
reuse it later.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/acpi/arm64/iort.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 7d543aa..2881108 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -347,7 +347,8 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
 	if (map[index].flags & ACPI_IORT_ID_SINGLE_MAPPING) {
 		if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT ||
 		    node->type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
-			*id_out = map[index].output_base;
+			if (id_out)
+				*id_out = map[index].output_base;
 			return parent;
 		}
 	}
-- 
1.7.12.4

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

* [PATCH v3 09/14] ACPI: platform: setup MSI domain for ACPI based platform device
  2016-10-25 15:09 [PATCH v3 00/14] ACPI platform MSI, interrupt producer/consumer and its example mbi-gen Hanjun Guo
                   ` (7 preceding siblings ...)
  2016-10-25 15:09 ` [PATCH v3 08/14] ACPI: ARM64: IORT: rework iort_node_get_id() Hanjun Guo
@ 2016-10-25 15:09 ` Hanjun Guo
  2016-10-25 15:09 ` [PATCH v3 10/14] msi: platform: make platform_msi_create_device_domain() ACPI aware Hanjun Guo
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Hanjun Guo @ 2016-10-25 15:09 UTC (permalink / raw)
  To: Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi
  Cc: linux-acpi, linux-arm-kernel, linux-kernel, Thomas Gleixner,
	Greg KH, Tomasz Nowicki, Ma Jun, Kefeng Wang, Agustin Vega-Frias,
	Sinan Kaya, G Gregory, Charles Garcia-Tobin, huxinwei, yimin,
	linuxarm, Hanjun Guo

From: Hanjun Guo <hanjun.guo@linaro.org>

With the platform msi domain created, we can set up the msi domain
for a platform device when it's probed.

In order to do that, we need to get the domain that the platform
device connecting to, so the iort_get_platform_device_domain() is
introduced to retrieve the domain from iort.

After the domain is retrieved, we need a proper way to set the
domain to paltform device, as some platform devices such as an
irqchip needs the msi irqdomain to be the interrupt parent domain,
we need to get irqdomain before platform device is probed but after
the platform device is allocated, so introduce a callback (pre_add_cb)
in pdevinfo to prepare firmware related information which is needed
for device probe, then set the msi domain in that callback.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/acpi/acpi_platform.c    | 11 +++++++++++
 drivers/acpi/arm64/iort.c       | 43 +++++++++++++++++++++++++++++++++++++++++
 drivers/base/platform.c         |  3 +++
 include/linux/acpi_iort.h       |  3 +++
 include/linux/platform_device.h |  3 +++
 5 files changed, 63 insertions(+)

diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
index b200ae1..13d0bf8 100644
--- a/drivers/acpi/acpi_platform.c
+++ b/drivers/acpi/acpi_platform.c
@@ -12,6 +12,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/acpi_iort.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/kernel.h>
@@ -48,6 +49,15 @@ static void acpi_platform_fill_resource(struct acpi_device *adev,
 }
 
 /**
+ * acpi_platform_pre_add_cb - callback before platform device is added, to
+ * prepare firmware related information which is needed for device probe
+ */
+static void acpi_platform_pre_add_cb(struct device *dev)
+{
+	acpi_configure_pmsi_domain(dev);
+}
+
+/**
  * acpi_create_platform_device - Create platform device for ACPI device node
  * @adev: ACPI device node to create a platform device for.
  *
@@ -106,6 +116,7 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev)
 	pdevinfo.res = resources;
 	pdevinfo.num_res = count;
 	pdevinfo.fwnode = acpi_fwnode_handle(adev);
+	pdevinfo.pre_add_cb = acpi_platform_pre_add_cb;
 
 	if (acpi_dma_supported(adev))
 		pdevinfo.dma_mask = DMA_BIT_MASK(32);
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 2881108..fd6eb65 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -527,6 +527,49 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id)
 	return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
 }
 
+/**
+ * iort_get_platform_device_domain() - Find MSI domain related to a
+ * platform device
+ * @dev: the dev pointer associated with the platform device
+ *
+ * Returns: the MSI domain for this device, NULL otherwise
+ */
+static struct irq_domain *iort_get_platform_device_domain(struct device *dev)
+{
+	struct acpi_iort_node *node, *msi_parent;
+	struct fwnode_handle *iort_fwnode;
+	struct acpi_iort_its_group *its;
+
+	/* find its associated iort node */
+	node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
+			      iort_match_node_callback, dev);
+	if (!node)
+		return NULL;
+
+	/* then find its msi parent node */
+	msi_parent = iort_node_get_id(node, NULL, IORT_MSI_TYPE, 0);
+	if (!msi_parent)
+		return NULL;
+
+	/* Move to ITS specific data */
+	its = (struct acpi_iort_its_group *)msi_parent->node_data;
+
+	iort_fwnode = iort_find_domain_token(its->identifiers[0]);
+	if (!iort_fwnode)
+		return NULL;
+
+	return irq_find_matching_fwnode(iort_fwnode, DOMAIN_BUS_PLATFORM_MSI);
+}
+
+void acpi_configure_pmsi_domain(struct device *dev)
+{
+	struct irq_domain *msi_domain;
+
+	msi_domain = iort_get_platform_device_domain(dev);
+	if (msi_domain)
+		dev_set_msi_domain(dev, msi_domain);
+}
+
 static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
 {
 	u32 *rid = data;
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index c4af003..3e68f31 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -537,6 +537,9 @@ struct platform_device *platform_device_register_full(
 			goto err;
 	}
 
+	if (pdevinfo->pre_add_cb)
+		pdevinfo->pre_add_cb(&pdev->dev);
+
 	ret = platform_device_add(pdev);
 	if (ret) {
 err:
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index 3f717cc..21e33b6 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -37,6 +37,7 @@
 struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id);
 /* IOMMU interface */
 const struct iommu_ops *iort_iommu_configure(struct device *dev);
+void acpi_configure_pmsi_domain(struct device *dev);
 #else
 static inline void acpi_iort_init(void) { }
 static inline bool iort_node_match(u8 type) { return false; }
@@ -56,6 +57,8 @@ static inline int iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id)
 static inline
 const struct iommu_ops *iort_iommu_configure(struct device *dev)
 { return NULL; }
+
+static inline void acpi_configure_pmsi_domain(struct device *dev) { }
 #endif
 
 #define IORT_ACPI_DECLARE(name, table_id, fn)		\
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 98c2a7c..280d366fb 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -74,6 +74,9 @@ struct platform_device_info {
 		u64 dma_mask;
 
 		struct property_entry *properties;
+
+		/* preparation callback before the platform device is added */
+		void (*pre_add_cb)(struct device *);
 };
 extern struct platform_device *platform_device_register_full(
 		const struct platform_device_info *pdevinfo);
-- 
1.7.12.4

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

* [PATCH v3 10/14] msi: platform: make platform_msi_create_device_domain() ACPI aware
  2016-10-25 15:09 [PATCH v3 00/14] ACPI platform MSI, interrupt producer/consumer and its example mbi-gen Hanjun Guo
                   ` (8 preceding siblings ...)
  2016-10-25 15:09 ` [PATCH v3 09/14] ACPI: platform: setup MSI domain for ACPI based platform device Hanjun Guo
@ 2016-10-25 15:09 ` Hanjun Guo
  2016-10-25 15:09 ` [PATCH v3 11/14] ACPI: irq: introduce interrupt producer Hanjun Guo
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Hanjun Guo @ 2016-10-25 15:09 UTC (permalink / raw)
  To: Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi
  Cc: linux-acpi, linux-arm-kernel, linux-kernel, Thomas Gleixner,
	Greg KH, Tomasz Nowicki, Ma Jun, Kefeng Wang, Agustin Vega-Frias,
	Sinan Kaya, G Gregory, Charles Garcia-Tobin, huxinwei, yimin,
	linuxarm, Hanjun Guo

From: Hanjun Guo <hanjun.guo@linaro.org>

With the platform msi domain created for ITS, irqchip such as
mbi-gen connecting ITS, which needs ctreate its own irqdomain.

Fortunately with the platform msi support upstreamed by Marc,
we just need to add minor code to make it run properly.

platform_msi_create_device_domain() is almost ready for ACPI use
except of_node_to_fwnode() is for dt only, make it ACPI aware then
things will work in both DTS and ACPI.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Greg KH <gregkh@linuxfoundation.org>
---
 drivers/base/platform-msi.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
index be6a599..035ca3b 100644
--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -345,8 +345,7 @@ struct irq_domain *
 
 	data->host_data = host_data;
 	domain = irq_domain_create_hierarchy(dev->msi_domain, 0, nvec,
-					     of_node_to_fwnode(dev->of_node),
-					     ops, data);
+					     dev->fwnode, ops, data);
 	if (!domain)
 		goto free_priv;
 
-- 
1.7.12.4

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

* [PATCH v3 11/14] ACPI: irq: introduce interrupt producer
  2016-10-25 15:09 [PATCH v3 00/14] ACPI platform MSI, interrupt producer/consumer and its example mbi-gen Hanjun Guo
                   ` (9 preceding siblings ...)
  2016-10-25 15:09 ` [PATCH v3 10/14] msi: platform: make platform_msi_create_device_domain() ACPI aware Hanjun Guo
@ 2016-10-25 15:09 ` Hanjun Guo
  2016-10-28 19:32   ` agustinv
                     ` (2 more replies)
  2016-10-25 15:09 ` [PATCH v3 12/14] irqchip: mbigen: drop module owner Hanjun Guo
                   ` (2 subsequent siblings)
  13 siblings, 3 replies; 20+ messages in thread
From: Hanjun Guo @ 2016-10-25 15:09 UTC (permalink / raw)
  To: Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi
  Cc: linux-acpi, linux-arm-kernel, linux-kernel, Thomas Gleixner,
	Greg KH, Tomasz Nowicki, Ma Jun, Kefeng Wang, Agustin Vega-Frias,
	Sinan Kaya, G Gregory, Charles Garcia-Tobin, huxinwei, yimin,
	linuxarm, Hanjun Guo

From: Hanjun Guo <hanjun.guo@linaro.org>

In ACPI 6.1 spec, section 19.6.62, Interrupt Resource Descriptor Macro,

Interrupt (ResourceUsage, EdgeLevel, ActiveLevel, Shared,
ResourceSourceIndex, ResourceSource, DescriptorName)
{ InterruptList } => Buffer

For the arguement ResourceUsage and DescriptorName, which means:

ResourceUsage describes whether the device consumes the specified
interrupt ( ResourceConsumer ) or produces it for use by a child
device ( ResourceProducer ).
If nothing is specified, then ResourceConsumer is assumed.

DescriptorName evaluates to a name string which refers to the
entire resource descriptor.

So it can be used for devices connecting to a specific interrupt
prodcucer instead of the main interrupt controller in MADT. In the
real world, we have irqchip such as mbi-gen which connecting to
a group of wired interrupts and then issue msi to the ITS, devices
connecting to such interrupt controller fit this scope.

For now the irq for ACPI only pointer to the main interrupt
controller's irqdomain, for devices not connecting to those
irqdomains, which need to present its irq parent, we can use
following ASL code to represent it:

Interrupt(ResourceConsumer,..., "\_SB.IRQP") {12,14,....}

then we can parse the interrupt producer with the full
path name "\_SB.IRQP".

In order to do that, we introduce a pointer interrupt_producer
in struct acpi_device, and fill it when scanning irq resources
for acpi device if it specifies the interrupt producer.

But for now when parsing the resources for acpi devices, we don't
pass the acpi device for acpi_walk_resoures() in drivers/acpi/resource.c,
so introduce a adev in struct res_proc_context to pass it as a context
to scan the interrupt resources, then finally pass to acpi_register_gsi()
to find its interrupt producer to get the virq from diffrent domains.

With steps above ready, rework acpi_register_gsi() to get other
interrupt producer if devices not connecting to main interrupt
controller.

Since we often pass NULL to acpi_register_gsi() and there is no interrupt
producer for devices connect to gicd on ARM or io-apic on X86, so it will
use the default irqdomain for those deivces and no functional changes to
those devices.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Agustin Vega-Frias <agustinv@codeaurora.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 drivers/acpi/gsi.c      | 10 ++++--
 drivers/acpi/resource.c | 85 ++++++++++++++++++++++++++++++++++---------------
 include/acpi/acpi_bus.h |  1 +
 3 files changed, 68 insertions(+), 28 deletions(-)

diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
index ee9e0f2..29ee547 100644
--- a/drivers/acpi/gsi.c
+++ b/drivers/acpi/gsi.c
@@ -55,13 +55,19 @@ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
 		      int polarity)
 {
 	struct irq_fwspec fwspec;
+	struct acpi_device *adev = dev ? to_acpi_device(dev) : NULL;
 
-	if (WARN_ON(!acpi_gsi_domain_id)) {
+	if (adev && &adev->fwnode && adev->interrupt_producer)
+		/* devices in DSDT connecting to spefic interrupt producer */
+		fwspec.fwnode = adev->interrupt_producer;
+	else if (acpi_gsi_domain_id)
+		/* devices connecting to gicd in default */
+		fwspec.fwnode = acpi_gsi_domain_id;
+	else {
 		pr_warn("GSI: No registered irqchip, giving up\n");
 		return -EINVAL;
 	}
 
-	fwspec.fwnode = acpi_gsi_domain_id;
 	fwspec.param[0] = gsi;
 	fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
 	fwspec.param_count = 2;
diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 56241eb..f1371cf 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -381,7 +381,7 @@ static void acpi_dev_irqresource_disabled(struct resource *res, u32 gsi)
 	res->flags = IORESOURCE_IRQ | IORESOURCE_DISABLED | IORESOURCE_UNSET;
 }
 
-static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
+static void acpi_dev_get_irqresource(struct acpi_device *adev, struct resource *res, u32 gsi,
 				     u8 triggering, u8 polarity, u8 shareable,
 				     bool legacy)
 {
@@ -415,7 +415,7 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
 	}
 
 	res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
-	irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
+	irq = acpi_register_gsi(&adev->dev, gsi, triggering, polarity);
 	if (irq >= 0) {
 		res->start = irq;
 		res->end = irq;
@@ -424,27 +424,9 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
 	}
 }
 
-/**
- * acpi_dev_resource_interrupt - Extract ACPI interrupt resource information.
- * @ares: Input ACPI resource object.
- * @index: Index into the array of GSIs represented by the resource.
- * @res: Output generic resource object.
- *
- * Check if the given ACPI resource object represents an interrupt resource
- * and @index does not exceed the resource's interrupt count (true is returned
- * in that case regardless of the results of the other checks)).  If that's the
- * case, register the GSI corresponding to @index from the array of interrupts
- * represented by the resource and populate the generic resource object pointed
- * to by @res accordingly.  If the registration of the GSI is not successful,
- * IORESOURCE_DISABLED will be set it that object's flags.
- *
- * Return:
- * 1) false with res->flags setting to zero: not the expected resource type
- * 2) false with IORESOURCE_DISABLED in res->flags: valid unassigned resource
- * 3) true: valid assigned resource
- */
-bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
-				 struct resource *res)
+static bool __acpi_dev_resource_interrupt(struct acpi_device *adev,
+					  struct acpi_resource *ares, int index,
+					  struct resource *res)
 {
 	struct acpi_resource_irq *irq;
 	struct acpi_resource_extended_irq *ext_irq;
@@ -460,7 +442,7 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
 			acpi_dev_irqresource_disabled(res, 0);
 			return false;
 		}
-		acpi_dev_get_irqresource(res, irq->interrupts[index],
+		acpi_dev_get_irqresource(adev, res, irq->interrupts[index],
 					 irq->triggering, irq->polarity,
 					 irq->sharable, true);
 		break;
@@ -470,7 +452,31 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
 			acpi_dev_irqresource_disabled(res, 0);
 			return false;
 		}
-		acpi_dev_get_irqresource(res, ext_irq->interrupts[index],
+
+		/*
+		 * It's a interrupt consumer device and connecting to specfic
+		 * interrupt controller. For now, we only support connecting
+		 * interrupts to one irq controller for a single device
+		 */
+		if (ext_irq->producer_consumer == ACPI_CONSUMER
+		    && ext_irq->resource_source.string_length != 0
+		    && !adev->interrupt_producer) {
+			acpi_status status;
+			acpi_handle handle;
+			struct acpi_device *device;
+
+			status = acpi_get_handle(NULL, ext_irq->resource_source.string_ptr, &handle);
+			if (ACPI_FAILURE(status))
+				return false;
+
+			device = acpi_bus_get_acpi_device(handle);
+			if (!device)
+				return false;
+
+			adev->interrupt_producer = &device->fwnode;
+		}
+
+		acpi_dev_get_irqresource(adev, res, ext_irq->interrupts[index],
 					 ext_irq->triggering, ext_irq->polarity,
 					 ext_irq->sharable, false);
 		break;
@@ -481,6 +487,31 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
 
 	return true;
 }
+
+/**
+ * acpi_dev_resource_interrupt - Extract ACPI interrupt resource information.
+ * @ares: Input ACPI resource object.
+ * @index: Index into the array of GSIs represented by the resource.
+ * @res: Output generic resource object.
+ *
+ * Check if the given ACPI resource object represents an interrupt resource
+ * and @index does not exceed the resource's interrupt count (true is returned
+ * in that case regardless of the results of the other checks)).  If that's the
+ * case, register the GSI corresponding to @index from the array of interrupts
+ * represented by the resource and populate the generic resource object pointed
+ * to by @res accordingly.  If the registration of the GSI is not successful,
+ * IORESOURCE_DISABLED will be set it that object's flags.
+ *
+ * Return:
+ * 1) false with res->flags setting to zero: not the expected resource type
+ * 2) false with IORESOURCE_DISABLED in res->flags: valid unassigned resource
+ * 3) true: valid assigned resource
+ */
+bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
+				 struct resource *res)
+{
+	return __acpi_dev_resource_interrupt(NULL, ares, index, res);
+}
 EXPORT_SYMBOL_GPL(acpi_dev_resource_interrupt);
 
 /**
@@ -499,6 +530,7 @@ struct res_proc_context {
 	void *preproc_data;
 	int count;
 	int error;
+	struct acpi_device *adev;
 };
 
 static acpi_status acpi_dev_new_resource_entry(struct resource_win *win,
@@ -546,7 +578,7 @@ static acpi_status acpi_dev_process_resource(struct acpi_resource *ares,
 	    || acpi_dev_resource_ext_address_space(ares, &win))
 		return acpi_dev_new_resource_entry(&win, c);
 
-	for (i = 0; acpi_dev_resource_interrupt(ares, i, res); i++) {
+	for (i = 0; __acpi_dev_resource_interrupt(c->adev, ares, i, res); i++) {
 		acpi_status status;
 
 		status = acpi_dev_new_resource_entry(&win, c);
@@ -599,6 +631,7 @@ int acpi_dev_get_resources(struct acpi_device *adev, struct list_head *list,
 	c.preproc_data = preproc_data;
 	c.count = 0;
 	c.error = 0;
+	c.adev = adev;
 	status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
 				     acpi_dev_process_resource, &c);
 	if (ACPI_FAILURE(status)) {
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 4242c31..5410d3b 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -355,6 +355,7 @@ struct acpi_device {
 	int device_type;
 	acpi_handle handle;		/* no handle for fixed hardware */
 	struct fwnode_handle fwnode;
+	struct fwnode_handle *interrupt_producer;
 	struct acpi_device *parent;
 	struct list_head children;
 	struct list_head node;
-- 
1.7.12.4

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

* [PATCH v3 12/14] irqchip: mbigen: drop module owner
  2016-10-25 15:09 [PATCH v3 00/14] ACPI platform MSI, interrupt producer/consumer and its example mbi-gen Hanjun Guo
                   ` (10 preceding siblings ...)
  2016-10-25 15:09 ` [PATCH v3 11/14] ACPI: irq: introduce interrupt producer Hanjun Guo
@ 2016-10-25 15:09 ` Hanjun Guo
  2016-10-25 15:09 ` [PATCH v3 13/14] irqchip: mbigen: introduce mbigen_of_create_domain() Hanjun Guo
  2016-10-25 15:09 ` [PATCH v3 14/14] irqchip: mbigen: Add ACPI support Hanjun Guo
  13 siblings, 0 replies; 20+ messages in thread
From: Hanjun Guo @ 2016-10-25 15:09 UTC (permalink / raw)
  To: Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi
  Cc: linux-acpi, linux-arm-kernel, linux-kernel, Thomas Gleixner,
	Greg KH, Tomasz Nowicki, Ma Jun, Kefeng Wang, Agustin Vega-Frias,
	Sinan Kaya, G Gregory, Charles Garcia-Tobin, huxinwei, yimin,
	linuxarm, Hanjun Guo

From: Kefeng Wang <wangkefeng.wang@huawei.com>

Module owner will be set by driver core, so drop it.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ma Jun <majun258@huawei.com>
---
 drivers/irqchip/irq-mbigen.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index 03b79b0..c01ab41 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -293,7 +293,6 @@ static int mbigen_device_probe(struct platform_device *pdev)
 static struct platform_driver mbigen_platform_driver = {
 	.driver = {
 		.name		= "Hisilicon MBIGEN-V2",
-		.owner		= THIS_MODULE,
 		.of_match_table	= mbigen_of_match,
 	},
 	.probe			= mbigen_device_probe,
-- 
1.7.12.4

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

* [PATCH v3 13/14] irqchip: mbigen: introduce mbigen_of_create_domain()
  2016-10-25 15:09 [PATCH v3 00/14] ACPI platform MSI, interrupt producer/consumer and its example mbi-gen Hanjun Guo
                   ` (11 preceding siblings ...)
  2016-10-25 15:09 ` [PATCH v3 12/14] irqchip: mbigen: drop module owner Hanjun Guo
@ 2016-10-25 15:09 ` Hanjun Guo
  2016-10-25 15:09 ` [PATCH v3 14/14] irqchip: mbigen: Add ACPI support Hanjun Guo
  13 siblings, 0 replies; 20+ messages in thread
From: Hanjun Guo @ 2016-10-25 15:09 UTC (permalink / raw)
  To: Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi
  Cc: linux-acpi, linux-arm-kernel, linux-kernel, Thomas Gleixner,
	Greg KH, Tomasz Nowicki, Ma Jun, Kefeng Wang, Agustin Vega-Frias,
	Sinan Kaya, G Gregory, Charles Garcia-Tobin, huxinwei, yimin,
	linuxarm, Hanjun Guo

From: Kefeng Wang <wangkefeng.wang@huawei.com>

Introduce mbigen_of_create_domain() to consolidate OF related
code and prepare for ACPI later.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ma Jun <majun258@huawei.com>
---
 drivers/irqchip/irq-mbigen.c | 42 +++++++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index c01ab41..9484ea0 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -236,27 +236,15 @@ static int mbigen_irq_domain_alloc(struct irq_domain *domain,
 	.free		= irq_domain_free_irqs_common,
 };
 
-static int mbigen_device_probe(struct platform_device *pdev)
+static int mbigen_of_create_domain(struct platform_device *pdev,
+				   struct mbigen_device *mgn_chip)
 {
-	struct mbigen_device *mgn_chip;
+	struct device *parent;
 	struct platform_device *child;
 	struct irq_domain *domain;
 	struct device_node *np;
-	struct device *parent;
-	struct resource *res;
 	u32 num_pins;
 
-	mgn_chip = devm_kzalloc(&pdev->dev, sizeof(*mgn_chip), GFP_KERNEL);
-	if (!mgn_chip)
-		return -ENOMEM;
-
-	mgn_chip->pdev = pdev;
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	mgn_chip->base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(mgn_chip->base))
-		return PTR_ERR(mgn_chip->base);
-
 	for_each_child_of_node(pdev->dev.of_node, np) {
 		if (!of_property_read_bool(np, "interrupt-controller"))
 			continue;
@@ -280,6 +268,30 @@ static int mbigen_device_probe(struct platform_device *pdev)
 			return -ENOMEM;
 	}
 
+	return 0;
+}
+
+static int mbigen_device_probe(struct platform_device *pdev)
+{
+	struct mbigen_device *mgn_chip;
+	struct resource *res;
+	int err;
+
+	mgn_chip = devm_kzalloc(&pdev->dev, sizeof(*mgn_chip), GFP_KERNEL);
+	if (!mgn_chip)
+		return -ENOMEM;
+
+	mgn_chip->pdev = pdev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mgn_chip->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(mgn_chip->base))
+		return PTR_ERR(mgn_chip->base);
+
+	err = mbigen_of_create_domain(pdev, mgn_chip);
+	if (err)
+		return err;
+
 	platform_set_drvdata(pdev, mgn_chip);
 	return 0;
 }
-- 
1.7.12.4

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

* [PATCH v3 14/14] irqchip: mbigen: Add ACPI support
  2016-10-25 15:09 [PATCH v3 00/14] ACPI platform MSI, interrupt producer/consumer and its example mbi-gen Hanjun Guo
                   ` (12 preceding siblings ...)
  2016-10-25 15:09 ` [PATCH v3 13/14] irqchip: mbigen: introduce mbigen_of_create_domain() Hanjun Guo
@ 2016-10-25 15:09 ` Hanjun Guo
  13 siblings, 0 replies; 20+ messages in thread
From: Hanjun Guo @ 2016-10-25 15:09 UTC (permalink / raw)
  To: Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi
  Cc: linux-acpi, linux-arm-kernel, linux-kernel, Thomas Gleixner,
	Greg KH, Tomasz Nowicki, Ma Jun, Kefeng Wang, Agustin Vega-Frias,
	Sinan Kaya, G Gregory, Charles Garcia-Tobin, huxinwei, yimin,
	linuxarm, Hanjun Guo

From: Hanjun Guo <hanjun.guo@linaro.org>

With the preparation of platform msi support and interrupt producer
in DSDT, we can add mbigen ACPI support now.

We are using _PRS methd to indicate number of irq pins instead
of num_pins in DT to avoid _DSD usage in this case.

For mbi-gen,
    Device(MBI0) {
          Name(_HID, "HISI0152")
          Name(_UID, Zero)
          Name(_CRS, ResourceTemplate() {
                  Memory32Fixed(ReadWrite, 0xa0080000, 0x10000)
          })

          Name (_PRS, ResourceTemplate() {
		  Interrupt(ResourceProducer,...) {12,14,....}
          })
    }

For devices,

   Device(COM0) {
          Name(_HID, "ACPIIDxx")
          Name(_UID, Zero)
          Name(_CRS, ResourceTemplate() {
                 Memory32Fixed(ReadWrite, 0xb0030000, 0x10000)
		 Interrupt(ResourceConsumer,..., "\_SB.MBI0") {12}
          })
    }

With the helpe of platform msi and interrupt producer, then devices
will get the virq from mbi-gen's irqdomain.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ma Jun <majun258@huawei.com>
---
 drivers/irqchip/irq-mbigen.c | 70 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 67 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index 9484ea0..ca6add1 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -16,6 +16,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/acpi.h>
 #include <linux/interrupt.h>
 #include <linux/irqchip.h>
 #include <linux/module.h>
@@ -180,7 +181,7 @@ static int mbigen_domain_translate(struct irq_domain *d,
 				    unsigned long *hwirq,
 				    unsigned int *type)
 {
-	if (is_of_node(fwspec->fwnode)) {
+	if (is_of_node(fwspec->fwnode) || is_acpi_device_node(fwspec->fwnode)) {
 		if (fwspec->param_count != 2)
 			return -EINVAL;
 
@@ -271,6 +272,54 @@ static int mbigen_of_create_domain(struct platform_device *pdev,
 	return 0;
 }
 
+#ifdef CONFIG_ACPI
+static acpi_status mbigen_acpi_process_resource(struct acpi_resource *ares,
+					     void *context)
+{
+	struct acpi_resource_extended_irq *ext_irq;
+	u32 *num_irqs = context;
+
+	switch (ares->type) {
+	case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
+		ext_irq = &ares->data.extended_irq;
+		*num_irqs += ext_irq->interrupt_count;
+		break;
+	default:
+		break;
+	}
+
+	return AE_OK;
+}
+
+static int mbigen_acpi_create_domain(struct platform_device *pdev,
+				     struct mbigen_device *mgn_chip)
+{
+	struct irq_domain *domain;
+	u32 num_msis = 0;
+	acpi_status status;
+
+	status = acpi_walk_resources(ACPI_HANDLE(&pdev->dev), METHOD_NAME__PRS,
+				     mbigen_acpi_process_resource, &num_msis);
+        if (ACPI_FAILURE(status) || num_msis == 0)
+		return -EINVAL;
+
+	domain = platform_msi_create_device_domain(&pdev->dev, num_msis,
+						   mbigen_write_msg,
+						   &mbigen_domain_ops,
+						   mgn_chip);
+	if (!domain)
+		return -ENOMEM;
+
+	return 0;
+}
+#else
+static int mbigen_acpi_create_domain(struct platform_device *pdev,
+				     struct mbigen_device *mgn_chip)
+{
+	return -ENODEV;
+}
+#endif
+
 static int mbigen_device_probe(struct platform_device *pdev)
 {
 	struct mbigen_device *mgn_chip;
@@ -288,9 +337,17 @@ static int mbigen_device_probe(struct platform_device *pdev)
 	if (IS_ERR(mgn_chip->base))
 		return PTR_ERR(mgn_chip->base);
 
-	err = mbigen_of_create_domain(pdev, mgn_chip);
-	if (err)
+	if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node)
+		err = mbigen_of_create_domain(pdev, mgn_chip);
+	else if (ACPI_COMPANION(&pdev->dev))
+		err = mbigen_acpi_create_domain(pdev, mgn_chip);
+	else
+		err = -EINVAL;
+
+	if (err) {
+		dev_err(&pdev->dev, "Failed to create mbi-gen@%p irqdomain", mgn_chip->base);
 		return err;
+	}
 
 	platform_set_drvdata(pdev, mgn_chip);
 	return 0;
@@ -302,10 +359,17 @@ static int mbigen_device_probe(struct platform_device *pdev)
 };
 MODULE_DEVICE_TABLE(of, mbigen_of_match);
 
+static const struct acpi_device_id mbigen_acpi_match[] = {
+        { "HISI0152", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, mbigen_acpi_match);
+
 static struct platform_driver mbigen_platform_driver = {
 	.driver = {
 		.name		= "Hisilicon MBIGEN-V2",
 		.of_match_table	= mbigen_of_match,
+		.acpi_match_table = ACPI_PTR(mbigen_acpi_match),
 	},
 	.probe			= mbigen_device_probe,
 };
-- 
1.7.12.4

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

* Re: [PATCH v3 11/14] ACPI: irq: introduce interrupt producer
  2016-10-25 15:09 ` [PATCH v3 11/14] ACPI: irq: introduce interrupt producer Hanjun Guo
@ 2016-10-28 19:32   ` agustinv
  2016-11-02 21:09     ` Hanjun Guo
  2016-12-01 11:12   ` Aleksey Makarov
  2016-12-03 12:18   ` G Gregory
  2 siblings, 1 reply; 20+ messages in thread
From: agustinv @ 2016-10-28 19:32 UTC (permalink / raw)
  To: Hanjun Guo
  Cc: Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi, linux-acpi,
	linux-arm-kernel, linux-kernel, Thomas Gleixner, Greg KH,
	Tomasz Nowicki, Ma Jun, Kefeng Wang, Sinan Kaya, G Gregory,
	Charles Garcia-Tobin, huxinwei, yimin, linuxarm, Hanjun Guo

Hey Hanjun,

On 2016-10-25 11:09, Hanjun Guo wrote:
> From: Hanjun Guo <hanjun.guo@linaro.org>
> 
> In ACPI 6.1 spec, section 19.6.62, Interrupt Resource Descriptor Macro,
> 
> Interrupt (ResourceUsage, EdgeLevel, ActiveLevel, Shared,
> ResourceSourceIndex, ResourceSource, DescriptorName)
> { InterruptList } => Buffer
> 
> For the arguement ResourceUsage and DescriptorName, which means:
> 
> ResourceUsage describes whether the device consumes the specified
> interrupt ( ResourceConsumer ) or produces it for use by a child
> device ( ResourceProducer ).
> If nothing is specified, then ResourceConsumer is assumed.
> 
> DescriptorName evaluates to a name string which refers to the
> entire resource descriptor.
> 
> So it can be used for devices connecting to a specific interrupt
> prodcucer instead of the main interrupt controller in MADT. In the
> real world, we have irqchip such as mbi-gen which connecting to
> a group of wired interrupts and then issue msi to the ITS, devices
> connecting to such interrupt controller fit this scope.
> 
> For now the irq for ACPI only pointer to the main interrupt
> controller's irqdomain, for devices not connecting to those
> irqdomains, which need to present its irq parent, we can use
> following ASL code to represent it:
> 
> Interrupt(ResourceConsumer,..., "\_SB.IRQP") {12,14,....}
> 
> then we can parse the interrupt producer with the full
> path name "\_SB.IRQP".
> 
> In order to do that, we introduce a pointer interrupt_producer
> in struct acpi_device, and fill it when scanning irq resources
> for acpi device if it specifies the interrupt producer.
> 
> But for now when parsing the resources for acpi devices, we don't
> pass the acpi device for acpi_walk_resoures() in 
> drivers/acpi/resource.c,
> so introduce a adev in struct res_proc_context to pass it as a context
> to scan the interrupt resources, then finally pass to 
> acpi_register_gsi()
> to find its interrupt producer to get the virq from diffrent domains.
> 
> With steps above ready, rework acpi_register_gsi() to get other
> interrupt producer if devices not connecting to main interrupt
> controller.
> 
> Since we often pass NULL to acpi_register_gsi() and there is no 
> interrupt
> producer for devices connect to gicd on ARM or io-apic on X86, so it 
> will
> use the default irqdomain for those deivces and no functional changes 
> to
> those devices.
> 
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Agustin Vega-Frias <agustinv@codeaurora.org>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> ---
>  drivers/acpi/gsi.c      | 10 ++++--
>  drivers/acpi/resource.c | 85 
> ++++++++++++++++++++++++++++++++++---------------
>  include/acpi/acpi_bus.h |  1 +
>  3 files changed, 68 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
> index ee9e0f2..29ee547 100644
> --- a/drivers/acpi/gsi.c
> +++ b/drivers/acpi/gsi.c
> @@ -55,13 +55,19 @@ int acpi_register_gsi(struct device *dev, u32 gsi,
> int trigger,
>  		      int polarity)
>  {
>  	struct irq_fwspec fwspec;
> +	struct acpi_device *adev = dev ? to_acpi_device(dev) : NULL;
> 
> -	if (WARN_ON(!acpi_gsi_domain_id)) {
> +	if (adev && &adev->fwnode && adev->interrupt_producer)
> +		/* devices in DSDT connecting to spefic interrupt producer */
> +		fwspec.fwnode = adev->interrupt_producer;
> +	else if (acpi_gsi_domain_id)
> +		/* devices connecting to gicd in default */
> +		fwspec.fwnode = acpi_gsi_domain_id;
> +	else {
>  		pr_warn("GSI: No registered irqchip, giving up\n");
>  		return -EINVAL;
>  	}
> 
> -	fwspec.fwnode = acpi_gsi_domain_id;
>  	fwspec.param[0] = gsi;
>  	fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
>  	fwspec.param_count = 2;
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 56241eb..f1371cf 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -381,7 +381,7 @@ static void acpi_dev_irqresource_disabled(struct
> resource *res, u32 gsi)
>  	res->flags = IORESOURCE_IRQ | IORESOURCE_DISABLED | IORESOURCE_UNSET;
>  }
> 
> -static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
> +static void acpi_dev_get_irqresource(struct acpi_device *adev, struct
> resource *res, u32 gsi,
>  				     u8 triggering, u8 polarity, u8 shareable,
>  				     bool legacy)
>  {
> @@ -415,7 +415,7 @@ static void acpi_dev_get_irqresource(struct
> resource *res, u32 gsi,
>  	}
> 
>  	res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
> -	irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
> +	irq = acpi_register_gsi(&adev->dev, gsi, triggering, polarity);
>  	if (irq >= 0) {
>  		res->start = irq;
>  		res->end = irq;
> @@ -424,27 +424,9 @@ static void acpi_dev_get_irqresource(struct
> resource *res, u32 gsi,
>  	}
>  }
> 
> -/**
> - * acpi_dev_resource_interrupt - Extract ACPI interrupt resource 
> information.
> - * @ares: Input ACPI resource object.
> - * @index: Index into the array of GSIs represented by the resource.
> - * @res: Output generic resource object.
> - *
> - * Check if the given ACPI resource object represents an interrupt 
> resource
> - * and @index does not exceed the resource's interrupt count (true is 
> returned
> - * in that case regardless of the results of the other checks)).  If 
> that's the
> - * case, register the GSI corresponding to @index from the array of 
> interrupts
> - * represented by the resource and populate the generic resource 
> object pointed
> - * to by @res accordingly.  If the registration of the GSI is not 
> successful,
> - * IORESOURCE_DISABLED will be set it that object's flags.
> - *
> - * Return:
> - * 1) false with res->flags setting to zero: not the expected resource 
> type
> - * 2) false with IORESOURCE_DISABLED in res->flags: valid unassigned 
> resource
> - * 3) true: valid assigned resource
> - */
> -bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int 
> index,
> -				 struct resource *res)
> +static bool __acpi_dev_resource_interrupt(struct acpi_device *adev,
> +					  struct acpi_resource *ares, int index,
> +					  struct resource *res)
>  {
>  	struct acpi_resource_irq *irq;
>  	struct acpi_resource_extended_irq *ext_irq;
> @@ -460,7 +442,7 @@ bool acpi_dev_resource_interrupt(struct
> acpi_resource *ares, int index,
>  			acpi_dev_irqresource_disabled(res, 0);
>  			return false;
>  		}
> -		acpi_dev_get_irqresource(res, irq->interrupts[index],
> +		acpi_dev_get_irqresource(adev, res, irq->interrupts[index],
>  					 irq->triggering, irq->polarity,
>  					 irq->sharable, true);
>  		break;
> @@ -470,7 +452,31 @@ bool acpi_dev_resource_interrupt(struct
> acpi_resource *ares, int index,
>  			acpi_dev_irqresource_disabled(res, 0);
>  			return false;
>  		}
> -		acpi_dev_get_irqresource(res, ext_irq->interrupts[index],
> +
> +		/*
> +		 * It's a interrupt consumer device and connecting to specfic
> +		 * interrupt controller. For now, we only support connecting
> +		 * interrupts to one irq controller for a single device
> +		 */
> +		if (ext_irq->producer_consumer == ACPI_CONSUMER
> +		    && ext_irq->resource_source.string_length != 0
> +		    && !adev->interrupt_producer) {
> +			acpi_status status;
> +			acpi_handle handle;
> +			struct acpi_device *device;
> +
> +			status = acpi_get_handle(NULL,
> ext_irq->resource_source.string_ptr, &handle);
> +			if (ACPI_FAILURE(status))
> +				return false;
> +
> +			device = acpi_bus_get_acpi_device(handle);
> +			if (!device)
> +				return false;
> +
> +			adev->interrupt_producer = &device->fwnode;

You are missing an 'acpi_bus_put_acpi_device(device)' call here.

Besides that, this approach will not work in the case where a device
wants to consume interrupts from multiple controllers since you are
forcing adev->interrupt_producer to be the first resource_source
found.

That's the reason I was advocating dynamic lookup (see [1]).

I am about to submit V6 of my series where I also address the probe
ordering issues by enabling re-initialization of platform_device
resources when the resource was marked disabled due to the domain
nor being there during ACPI bus scan.

Thanks,
Agustin

[1] https://lkml.org/lkml/2016/10/18/592

> +		}
> +
> +		acpi_dev_get_irqresource(adev, res, ext_irq->interrupts[index],
>  					 ext_irq->triggering, ext_irq->polarity,
>  					 ext_irq->sharable, false);
>  		break;
> @@ -481,6 +487,31 @@ bool acpi_dev_resource_interrupt(struct
> acpi_resource *ares, int index,
> 
>  	return true;
>  }
> +
> +/**
> + * acpi_dev_resource_interrupt - Extract ACPI interrupt resource 
> information.
> + * @ares: Input ACPI resource object.
> + * @index: Index into the array of GSIs represented by the resource.
> + * @res: Output generic resource object.
> + *
> + * Check if the given ACPI resource object represents an interrupt 
> resource
> + * and @index does not exceed the resource's interrupt count (true is 
> returned
> + * in that case regardless of the results of the other checks)).  If 
> that's the
> + * case, register the GSI corresponding to @index from the array of 
> interrupts
> + * represented by the resource and populate the generic resource 
> object pointed
> + * to by @res accordingly.  If the registration of the GSI is not 
> successful,
> + * IORESOURCE_DISABLED will be set it that object's flags.
> + *
> + * Return:
> + * 1) false with res->flags setting to zero: not the expected resource 
> type
> + * 2) false with IORESOURCE_DISABLED in res->flags: valid unassigned 
> resource
> + * 3) true: valid assigned resource
> + */
> +bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int 
> index,
> +				 struct resource *res)
> +{
> +	return __acpi_dev_resource_interrupt(NULL, ares, index, res);
> +}
>  EXPORT_SYMBOL_GPL(acpi_dev_resource_interrupt);
> 
>  /**
> @@ -499,6 +530,7 @@ struct res_proc_context {
>  	void *preproc_data;
>  	int count;
>  	int error;
> +	struct acpi_device *adev;
>  };
> 
>  static acpi_status acpi_dev_new_resource_entry(struct resource_win 
> *win,
> @@ -546,7 +578,7 @@ static acpi_status
> acpi_dev_process_resource(struct acpi_resource *ares,
>  	    || acpi_dev_resource_ext_address_space(ares, &win))
>  		return acpi_dev_new_resource_entry(&win, c);
> 
> -	for (i = 0; acpi_dev_resource_interrupt(ares, i, res); i++) {
> +	for (i = 0; __acpi_dev_resource_interrupt(c->adev, ares, i, res); 
> i++) {
>  		acpi_status status;
> 
>  		status = acpi_dev_new_resource_entry(&win, c);
> @@ -599,6 +631,7 @@ int acpi_dev_get_resources(struct acpi_device
> *adev, struct list_head *list,
>  	c.preproc_data = preproc_data;
>  	c.count = 0;
>  	c.error = 0;
> +	c.adev = adev;
>  	status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
>  				     acpi_dev_process_resource, &c);
>  	if (ACPI_FAILURE(status)) {
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 4242c31..5410d3b 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -355,6 +355,7 @@ struct acpi_device {
>  	int device_type;
>  	acpi_handle handle;		/* no handle for fixed hardware */
>  	struct fwnode_handle fwnode;
> +	struct fwnode_handle *interrupt_producer;
>  	struct acpi_device *parent;
>  	struct list_head children;
>  	struct list_head node;
> --
> 1.7.12.4

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

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

* Re: [PATCH v3 11/14] ACPI: irq: introduce interrupt producer
  2016-10-28 19:32   ` agustinv
@ 2016-11-02 21:09     ` Hanjun Guo
  0 siblings, 0 replies; 20+ messages in thread
From: Hanjun Guo @ 2016-11-02 21:09 UTC (permalink / raw)
  To: agustinv, Hanjun Guo
  Cc: Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi, linux-acpi,
	linux-arm-kernel, linux-kernel, Thomas Gleixner, Greg KH,
	Tomasz Nowicki, Ma Jun, Kefeng Wang, Sinan Kaya, G Gregory,
	Charles Garcia-Tobin, huxinwei, yimin, linuxarm

Hi Agustin,

Sorry for the late reply, on travailing for now.

On 10/29/2016 03:32 AM, agustinv@codeaurora.org wrote:
> Hey Hanjun,
>
> On 2016-10-25 11:09, Hanjun Guo wrote:
>> From: Hanjun Guo <hanjun.guo@linaro.org>
>>
>> In ACPI 6.1 spec, section 19.6.62, Interrupt Resource Descriptor Macro,
>>
>> Interrupt (ResourceUsage, EdgeLevel, ActiveLevel, Shared,
>> ResourceSourceIndex, ResourceSource, DescriptorName)
>> { InterruptList } => Buffer
>>
>> For the arguement ResourceUsage and DescriptorName, which means:
>>
>> ResourceUsage describes whether the device consumes the specified
>> interrupt ( ResourceConsumer ) or produces it for use by a child
>> device ( ResourceProducer ).
>> If nothing is specified, then ResourceConsumer is assumed.
>>
>> DescriptorName evaluates to a name string which refers to the
>> entire resource descriptor.
>>
>> So it can be used for devices connecting to a specific interrupt
>> prodcucer instead of the main interrupt controller in MADT. In the
>> real world, we have irqchip such as mbi-gen which connecting to
>> a group of wired interrupts and then issue msi to the ITS, devices
>> connecting to such interrupt controller fit this scope.
>>
>> For now the irq for ACPI only pointer to the main interrupt
>> controller's irqdomain, for devices not connecting to those
>> irqdomains, which need to present its irq parent, we can use
>> following ASL code to represent it:
>>
>> Interrupt(ResourceConsumer,..., "\_SB.IRQP") {12,14,....}
>>
>> then we can parse the interrupt producer with the full
>> path name "\_SB.IRQP".
>>
>> In order to do that, we introduce a pointer interrupt_producer
>> in struct acpi_device, and fill it when scanning irq resources
>> for acpi device if it specifies the interrupt producer.
>>
>> But for now when parsing the resources for acpi devices, we don't
>> pass the acpi device for acpi_walk_resoures() in drivers/acpi/resource.c,
>> so introduce a adev in struct res_proc_context to pass it as a context
>> to scan the interrupt resources, then finally pass to acpi_register_gsi()
>> to find its interrupt producer to get the virq from diffrent domains.
>>
>> With steps above ready, rework acpi_register_gsi() to get other
>> interrupt producer if devices not connecting to main interrupt
>> controller.
>>
>> Since we often pass NULL to acpi_register_gsi() and there is no interrupt
>> producer for devices connect to gicd on ARM or io-apic on X86, so it will
>> use the default irqdomain for those deivces and no functional changes to
>> those devices.
>>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>> Cc: Agustin Vega-Frias <agustinv@codeaurora.org>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> ---
>>  drivers/acpi/gsi.c      | 10 ++++--
>>  drivers/acpi/resource.c | 85
>> ++++++++++++++++++++++++++++++++++---------------
>>  include/acpi/acpi_bus.h |  1 +
>>  3 files changed, 68 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
>> index ee9e0f2..29ee547 100644
>> --- a/drivers/acpi/gsi.c
>> +++ b/drivers/acpi/gsi.c
>> @@ -55,13 +55,19 @@ int acpi_register_gsi(struct device *dev, u32 gsi,
>> int trigger,
>>                int polarity)
>>  {
>>      struct irq_fwspec fwspec;
>> +    struct acpi_device *adev = dev ? to_acpi_device(dev) : NULL;
>>
>> -    if (WARN_ON(!acpi_gsi_domain_id)) {
>> +    if (adev && &adev->fwnode && adev->interrupt_producer)
>> +        /* devices in DSDT connecting to spefic interrupt producer */
>> +        fwspec.fwnode = adev->interrupt_producer;
>> +    else if (acpi_gsi_domain_id)
>> +        /* devices connecting to gicd in default */
>> +        fwspec.fwnode = acpi_gsi_domain_id;
>> +    else {
>>          pr_warn("GSI: No registered irqchip, giving up\n");
>>          return -EINVAL;
>>      }
>>
>> -    fwspec.fwnode = acpi_gsi_domain_id;
>>      fwspec.param[0] = gsi;
>>      fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
>>      fwspec.param_count = 2;
>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
>> index 56241eb..f1371cf 100644
>> --- a/drivers/acpi/resource.c
>> +++ b/drivers/acpi/resource.c
>> @@ -381,7 +381,7 @@ static void acpi_dev_irqresource_disabled(struct
>> resource *res, u32 gsi)
>>      res->flags = IORESOURCE_IRQ | IORESOURCE_DISABLED |
>> IORESOURCE_UNSET;
>>  }
>>
>> -static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>> +static void acpi_dev_get_irqresource(struct acpi_device *adev, struct
>> resource *res, u32 gsi,
>>                       u8 triggering, u8 polarity, u8 shareable,
>>                       bool legacy)
>>  {
>> @@ -415,7 +415,7 @@ static void acpi_dev_get_irqresource(struct
>> resource *res, u32 gsi,
>>      }
>>
>>      res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
>> -    irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
>> +    irq = acpi_register_gsi(&adev->dev, gsi, triggering, polarity);
>>      if (irq >= 0) {
>>          res->start = irq;
>>          res->end = irq;
>> @@ -424,27 +424,9 @@ static void acpi_dev_get_irqresource(struct
>> resource *res, u32 gsi,
>>      }
>>  }
>>
>> -/**
>> - * acpi_dev_resource_interrupt - Extract ACPI interrupt resource
>> information.
>> - * @ares: Input ACPI resource object.
>> - * @index: Index into the array of GSIs represented by the resource.
>> - * @res: Output generic resource object.
>> - *
>> - * Check if the given ACPI resource object represents an interrupt
>> resource
>> - * and @index does not exceed the resource's interrupt count (true is
>> returned
>> - * in that case regardless of the results of the other checks)).  If
>> that's the
>> - * case, register the GSI corresponding to @index from the array of
>> interrupts
>> - * represented by the resource and populate the generic resource
>> object pointed
>> - * to by @res accordingly.  If the registration of the GSI is not
>> successful,
>> - * IORESOURCE_DISABLED will be set it that object's flags.
>> - *
>> - * Return:
>> - * 1) false with res->flags setting to zero: not the expected
>> resource type
>> - * 2) false with IORESOURCE_DISABLED in res->flags: valid unassigned
>> resource
>> - * 3) true: valid assigned resource
>> - */
>> -bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>> -                 struct resource *res)
>> +static bool __acpi_dev_resource_interrupt(struct acpi_device *adev,
>> +                      struct acpi_resource *ares, int index,
>> +                      struct resource *res)
>>  {
>>      struct acpi_resource_irq *irq;
>>      struct acpi_resource_extended_irq *ext_irq;
>> @@ -460,7 +442,7 @@ bool acpi_dev_resource_interrupt(struct
>> acpi_resource *ares, int index,
>>              acpi_dev_irqresource_disabled(res, 0);
>>              return false;
>>          }
>> -        acpi_dev_get_irqresource(res, irq->interrupts[index],
>> +        acpi_dev_get_irqresource(adev, res, irq->interrupts[index],
>>                       irq->triggering, irq->polarity,
>>                       irq->sharable, true);
>>          break;
>> @@ -470,7 +452,31 @@ bool acpi_dev_resource_interrupt(struct
>> acpi_resource *ares, int index,
>>              acpi_dev_irqresource_disabled(res, 0);
>>              return false;
>>          }
>> -        acpi_dev_get_irqresource(res, ext_irq->interrupts[index],
>> +
>> +        /*
>> +         * It's a interrupt consumer device and connecting to specfic
>> +         * interrupt controller. For now, we only support connecting
>> +         * interrupts to one irq controller for a single device
>> +         */
>> +        if (ext_irq->producer_consumer == ACPI_CONSUMER
>> +            && ext_irq->resource_source.string_length != 0
>> +            && !adev->interrupt_producer) {
>> +            acpi_status status;
>> +            acpi_handle handle;
>> +            struct acpi_device *device;
>> +
>> +            status = acpi_get_handle(NULL,
>> ext_irq->resource_source.string_ptr, &handle);
>> +            if (ACPI_FAILURE(status))
>> +                return false;
>> +
>> +            device = acpi_bus_get_acpi_device(handle);
>> +            if (!device)
>> +                return false;
>> +
>> +            adev->interrupt_producer = &device->fwnode;
>
> You are missing an 'acpi_bus_put_acpi_device(device)' call here.

good catch!

>
> Besides that, this approach will not work in the case where a device
> wants to consume interrupts from multiple controllers since you are
> forcing adev->interrupt_producer to be the first resource_source
> found.

Yes, you are right, and it's in the comment of the code, we
can fix that to add some extra code.

>
> That's the reason I was advocating dynamic lookup (see [1]).
>
> I am about to submit V6 of my series where I also address the probe
> ordering issues by enabling re-initialization of platform_device
> resources when the resource was marked disabled due to the domain
> nor being there during ACPI bus scan.

I will take a deep look for your v6 patch set, probably we can
work together to get things down.

Thanks
Hanjun

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

* Re: [PATCH v3 11/14] ACPI: irq: introduce interrupt producer
  2016-10-25 15:09 ` [PATCH v3 11/14] ACPI: irq: introduce interrupt producer Hanjun Guo
  2016-10-28 19:32   ` agustinv
@ 2016-12-01 11:12   ` Aleksey Makarov
  2016-12-02 10:07     ` Hanjun Guo
  2016-12-03 12:18   ` G Gregory
  2 siblings, 1 reply; 20+ messages in thread
From: Aleksey Makarov @ 2016-12-01 11:12 UTC (permalink / raw)
  To: Hanjun Guo, Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi
  Cc: linux-acpi, linux-arm-kernel, linux-kernel, Thomas Gleixner,
	Greg KH, Tomasz Nowicki, Ma Jun, Kefeng Wang, Agustin Vega-Frias,
	Sinan Kaya, G Gregory, Charles Garcia-Tobin, huxinwei, yimin,
	linuxarm, Hanjun Guo


Hi Hanjun,

On 10/25/2016 09:09 PM, Hanjun Guo wrote:
> From: Hanjun Guo <hanjun.guo@linaro.org>
>
> In ACPI 6.1 spec, section 19.6.62, Interrupt Resource Descriptor Macro,

[ ... ]

> ---
>  drivers/acpi/gsi.c      | 10 ++++--
>  drivers/acpi/resource.c | 85 ++++++++++++++++++++++++++++++++++---------------
>  include/acpi/acpi_bus.h |  1 +
>  3 files changed, 68 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
> index ee9e0f2..29ee547 100644
> --- a/drivers/acpi/gsi.c
> +++ b/drivers/acpi/gsi.c
> @@ -55,13 +55,19 @@ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
>  		      int polarity)
>  {
>  	struct irq_fwspec fwspec;
> +	struct acpi_device *adev = dev ? to_acpi_device(dev) : NULL;

Why are you sure dev is always an acpi device?
Look for example at drivers/acpi/pci_irq.c:377 where this function
is called for a PCI device

>
> -	if (WARN_ON(!acpi_gsi_domain_id)) {
> +	if (adev && &adev->fwnode && adev->interrupt_producer)

&adev->fwnode is always true

> +		/* devices in DSDT connecting to spefic interrupt producer */
> +		fwspec.fwnode = adev->interrupt_producer;
> +	else if (acpi_gsi_domain_id)
> +		/* devices connecting to gicd in default */
> +		fwspec.fwnode = acpi_gsi_domain_id;
> +	else {
>  		pr_warn("GSI: No registered irqchip, giving up\n");
>  		return -EINVAL;
>  	}

[ ... ]

All the best
Aleksey Makarov

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

* Re: [PATCH v3 11/14] ACPI: irq: introduce interrupt producer
  2016-12-01 11:12   ` Aleksey Makarov
@ 2016-12-02 10:07     ` Hanjun Guo
  0 siblings, 0 replies; 20+ messages in thread
From: Hanjun Guo @ 2016-12-02 10:07 UTC (permalink / raw)
  To: Aleksey Makarov, Hanjun Guo, Marc Zyngier, Rafael J. Wysocki,
	Lorenzo Pieralisi
  Cc: linux-acpi, linux-arm-kernel, linux-kernel, Thomas Gleixner,
	Greg KH, Tomasz Nowicki, Ma Jun, Kefeng Wang, Agustin Vega-Frias,
	Sinan Kaya, G Gregory, Charles Garcia-Tobin, huxinwei, yimin,
	linuxarm

Hi Aleksey,

On 2016/12/1 19:12, Aleksey Makarov wrote:
>
> Hi Hanjun,
>
> On 10/25/2016 09:09 PM, Hanjun Guo wrote:
>> From: Hanjun Guo <hanjun.guo@linaro.org>
>>
>> In ACPI 6.1 spec, section 19.6.62, Interrupt Resource Descriptor Macro,
>
> [ ... ]
>
>> ---
>>  drivers/acpi/gsi.c      | 10 ++++--
>>  drivers/acpi/resource.c | 85
>> ++++++++++++++++++++++++++++++++++---------------
>>  include/acpi/acpi_bus.h |  1 +
>>  3 files changed, 68 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
>> index ee9e0f2..29ee547 100644
>> --- a/drivers/acpi/gsi.c
>> +++ b/drivers/acpi/gsi.c
>> @@ -55,13 +55,19 @@ int acpi_register_gsi(struct device *dev, u32 gsi,
>> int trigger,
>>                int polarity)
>>  {
>>      struct irq_fwspec fwspec;
>> +    struct acpi_device *adev = dev ? to_acpi_device(dev) : NULL;
>
> Why are you sure dev is always an acpi device?
> Look for example at drivers/acpi/pci_irq.c:377 where this function
> is called for a PCI device

Good catch, but I will drop this patch and use Agustin's one [1].

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

Thanks
Hanjun

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

* Re: [PATCH v3 11/14] ACPI: irq: introduce interrupt producer
  2016-10-25 15:09 ` [PATCH v3 11/14] ACPI: irq: introduce interrupt producer Hanjun Guo
  2016-10-28 19:32   ` agustinv
  2016-12-01 11:12   ` Aleksey Makarov
@ 2016-12-03 12:18   ` G Gregory
  2 siblings, 0 replies; 20+ messages in thread
From: G Gregory @ 2016-12-03 12:18 UTC (permalink / raw)
  To: Hanjun Guo
  Cc: Marc Zyngier, Rafael J. Wysocki, Lorenzo Pieralisi, linux-acpi,
	linux-arm-kernel, linux-kernel, Thomas Gleixner, Greg KH,
	Tomasz Nowicki, Ma Jun, Kefeng Wang, Agustin Vega-Frias,
	Sinan Kaya, Charles Garcia-Tobin, huxinwei, yimin, linuxarm,
	Hanjun Guo

On Tue, Oct 25, 2016 at 11:09:26PM +0800, Hanjun Guo wrote:
> From: Hanjun Guo <hanjun.guo@linaro.org>
> 
> In ACPI 6.1 spec, section 19.6.62, Interrupt Resource Descriptor Macro,
> 
> Interrupt (ResourceUsage, EdgeLevel, ActiveLevel, Shared,
> ResourceSourceIndex, ResourceSource, DescriptorName)
> { InterruptList } => Buffer
> 
> For the arguement ResourceUsage and DescriptorName, which means:
> 
> ResourceUsage describes whether the device consumes the specified
> interrupt ( ResourceConsumer ) or produces it for use by a child
> device ( ResourceProducer ).
> If nothing is specified, then ResourceConsumer is assumed.
> 
> DescriptorName evaluates to a name string which refers to the
> entire resource descriptor.
> 
> So it can be used for devices connecting to a specific interrupt
> prodcucer instead of the main interrupt controller in MADT. In the
> real world, we have irqchip such as mbi-gen which connecting to
> a group of wired interrupts and then issue msi to the ITS, devices
> connecting to such interrupt controller fit this scope.
> 
> For now the irq for ACPI only pointer to the main interrupt
> controller's irqdomain, for devices not connecting to those
> irqdomains, which need to present its irq parent, we can use
> following ASL code to represent it:
> 
> Interrupt(ResourceConsumer,..., "\_SB.IRQP") {12,14,....}
> 
> then we can parse the interrupt producer with the full
> path name "\_SB.IRQP".
> 
> In order to do that, we introduce a pointer interrupt_producer
> in struct acpi_device, and fill it when scanning irq resources
> for acpi device if it specifies the interrupt producer.
> 
> But for now when parsing the resources for acpi devices, we don't
> pass the acpi device for acpi_walk_resoures() in drivers/acpi/resource.c,
> so introduce a adev in struct res_proc_context to pass it as a context
> to scan the interrupt resources, then finally pass to acpi_register_gsi()
> to find its interrupt producer to get the virq from diffrent domains.
> 
> With steps above ready, rework acpi_register_gsi() to get other
> interrupt producer if devices not connecting to main interrupt
> controller.
> 
> Since we often pass NULL to acpi_register_gsi() and there is no interrupt
> producer for devices connect to gicd on ARM or io-apic on X86, so it will
> use the default irqdomain for those deivces and no functional changes to
> those devices.
> 
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Agustin Vega-Frias <agustinv@codeaurora.org>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> ---
>  drivers/acpi/gsi.c      | 10 ++++--
>  drivers/acpi/resource.c | 85 ++++++++++++++++++++++++++++++++++---------------
>  include/acpi/acpi_bus.h |  1 +
>  3 files changed, 68 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
> index ee9e0f2..29ee547 100644
> --- a/drivers/acpi/gsi.c
> +++ b/drivers/acpi/gsi.c
> @@ -55,13 +55,19 @@ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
>  		      int polarity)
>  {
>  	struct irq_fwspec fwspec;
> +	struct acpi_device *adev = dev ? to_acpi_device(dev) : NULL;
>  
> -	if (WARN_ON(!acpi_gsi_domain_id)) {
> +	if (adev && &adev->fwnode && adev->interrupt_producer)
> +		/* devices in DSDT connecting to spefic interrupt producer */
> +		fwspec.fwnode = adev->interrupt_producer;
> +	else if (acpi_gsi_domain_id)
> +		/* devices connecting to gicd in default */
> +		fwspec.fwnode = acpi_gsi_domain_id;
> +	else {
>  		pr_warn("GSI: No registered irqchip, giving up\n");
>  		return -EINVAL;
>  	}
>  
> -	fwspec.fwnode = acpi_gsi_domain_id;
>  	fwspec.param[0] = gsi;
>  	fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
>  	fwspec.param_count = 2;
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 56241eb..f1371cf 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -381,7 +381,7 @@ static void acpi_dev_irqresource_disabled(struct resource *res, u32 gsi)
>  	res->flags = IORESOURCE_IRQ | IORESOURCE_DISABLED | IORESOURCE_UNSET;
>  }
>  
> -static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
> +static void acpi_dev_get_irqresource(struct acpi_device *adev, struct resource *res, u32 gsi,
>  				     u8 triggering, u8 polarity, u8 shareable,
>  				     bool legacy)
>  {
> @@ -415,7 +415,7 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>  	}
>  
>  	res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
> -	irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
> +	irq = acpi_register_gsi(&adev->dev, gsi, triggering, polarity);
>  	if (irq >= 0) {
>  		res->start = irq;
>  		res->end = irq;
> @@ -424,27 +424,9 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>  	}
>  }
>  
> -/**
> - * acpi_dev_resource_interrupt - Extract ACPI interrupt resource information.
> - * @ares: Input ACPI resource object.
> - * @index: Index into the array of GSIs represented by the resource.
> - * @res: Output generic resource object.
> - *
> - * Check if the given ACPI resource object represents an interrupt resource
> - * and @index does not exceed the resource's interrupt count (true is returned
> - * in that case regardless of the results of the other checks)).  If that's the
> - * case, register the GSI corresponding to @index from the array of interrupts
> - * represented by the resource and populate the generic resource object pointed
> - * to by @res accordingly.  If the registration of the GSI is not successful,
> - * IORESOURCE_DISABLED will be set it that object's flags.
> - *
> - * Return:
> - * 1) false with res->flags setting to zero: not the expected resource type
> - * 2) false with IORESOURCE_DISABLED in res->flags: valid unassigned resource
> - * 3) true: valid assigned resource
> - */
> -bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
> -				 struct resource *res)
> +static bool __acpi_dev_resource_interrupt(struct acpi_device *adev,
> +					  struct acpi_resource *ares, int index,
> +					  struct resource *res)
>  {
>  	struct acpi_resource_irq *irq;
>  	struct acpi_resource_extended_irq *ext_irq;
> @@ -460,7 +442,7 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>  			acpi_dev_irqresource_disabled(res, 0);
>  			return false;
>  		}
> -		acpi_dev_get_irqresource(res, irq->interrupts[index],
> +		acpi_dev_get_irqresource(adev, res, irq->interrupts[index],
>  					 irq->triggering, irq->polarity,
>  					 irq->sharable, true);
>  		break;
> @@ -470,7 +452,31 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>  			acpi_dev_irqresource_disabled(res, 0);
>  			return false;
>  		}
> -		acpi_dev_get_irqresource(res, ext_irq->interrupts[index],
> +
> +		/*
> +		 * It's a interrupt consumer device and connecting to specfic
> +		 * interrupt controller. For now, we only support connecting
> +		 * interrupts to one irq controller for a single device
> +		 */
> +		if (ext_irq->producer_consumer == ACPI_CONSUMER
> +		    && ext_irq->resource_source.string_length != 0
> +		    && !adev->interrupt_producer) {
> +			acpi_status status;
> +			acpi_handle handle;
> +			struct acpi_device *device;
> +
> +			status = acpi_get_handle(NULL, ext_irq->resource_source.string_ptr, &handle);
> +			if (ACPI_FAILURE(status))
> +				return false;
> +
> +			device = acpi_bus_get_acpi_device(handle);
> +			if (!device)
> +				return false;
> +
> +			adev->interrupt_producer = &device->fwnode;
> +		}
> +
> +		acpi_dev_get_irqresource(adev, res, ext_irq->interrupts[index],
>  					 ext_irq->triggering, ext_irq->polarity,
>  					 ext_irq->sharable, false);
>  		break;
> @@ -481,6 +487,31 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>  
>  	return true;
>  }
> +
> +/**
> + * acpi_dev_resource_interrupt - Extract ACPI interrupt resource information.
> + * @ares: Input ACPI resource object.
> + * @index: Index into the array of GSIs represented by the resource.
> + * @res: Output generic resource object.
> + *
> + * Check if the given ACPI resource object represents an interrupt resource
> + * and @index does not exceed the resource's interrupt count (true is returned
> + * in that case regardless of the results of the other checks)).  If that's the
> + * case, register the GSI corresponding to @index from the array of interrupts
> + * represented by the resource and populate the generic resource object pointed
> + * to by @res accordingly.  If the registration of the GSI is not successful,
> + * IORESOURCE_DISABLED will be set it that object's flags.
> + *
> + * Return:
> + * 1) false with res->flags setting to zero: not the expected resource type
> + * 2) false with IORESOURCE_DISABLED in res->flags: valid unassigned resource
> + * 3) true: valid assigned resource
> + */
> +bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
> +				 struct resource *res)
> +{
> +	return __acpi_dev_resource_interrupt(NULL, ares, index, res);
> +}
>  EXPORT_SYMBOL_GPL(acpi_dev_resource_interrupt);

You cannot do this as &adev->dev is deferenced in acpi_dev_get_irqresource
without a check that adev is NULL, and so passes an offset from NULL to
acpi_register_gsi() which breaks everything.

Graeme

>  
>  /**
> @@ -499,6 +530,7 @@ struct res_proc_context {
>  	void *preproc_data;
>  	int count;
>  	int error;
> +	struct acpi_device *adev;
>  };
>  
>  static acpi_status acpi_dev_new_resource_entry(struct resource_win *win,
> @@ -546,7 +578,7 @@ static acpi_status acpi_dev_process_resource(struct acpi_resource *ares,
>  	    || acpi_dev_resource_ext_address_space(ares, &win))
>  		return acpi_dev_new_resource_entry(&win, c);
>  
> -	for (i = 0; acpi_dev_resource_interrupt(ares, i, res); i++) {
> +	for (i = 0; __acpi_dev_resource_interrupt(c->adev, ares, i, res); i++) {
>  		acpi_status status;
>  
>  		status = acpi_dev_new_resource_entry(&win, c);
> @@ -599,6 +631,7 @@ int acpi_dev_get_resources(struct acpi_device *adev, struct list_head *list,
>  	c.preproc_data = preproc_data;
>  	c.count = 0;
>  	c.error = 0;
> +	c.adev = adev;
>  	status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
>  				     acpi_dev_process_resource, &c);
>  	if (ACPI_FAILURE(status)) {
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 4242c31..5410d3b 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -355,6 +355,7 @@ struct acpi_device {
>  	int device_type;
>  	acpi_handle handle;		/* no handle for fixed hardware */
>  	struct fwnode_handle fwnode;
> +	struct fwnode_handle *interrupt_producer;
>  	struct acpi_device *parent;
>  	struct list_head children;
>  	struct list_head node;
> -- 
> 1.7.12.4
> 

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

end of thread, other threads:[~2016-12-03 12:18 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-25 15:09 [PATCH v3 00/14] ACPI platform MSI, interrupt producer/consumer and its example mbi-gen Hanjun Guo
2016-10-25 15:09 ` [PATCH v3 01/14] ACPI: ARM64: IORT: minor cleanup for iort_match_node_callback() Hanjun Guo
2016-10-25 15:09 ` [PATCH v3 02/14] irqchip: gic-v3-its: keep the head file include in alphabetic order Hanjun Guo
2016-10-25 15:09 ` [PATCH v3 03/14] ACPI: ARM64: IORT: add missing comment for iort_dev_find_its_id() Hanjun Guo
2016-10-25 15:09 ` [PATCH v3 04/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_prepare() Hanjun Guo
2016-10-25 15:09 ` [PATCH v3 05/14] ACPI: platform-msi: retrieve dev id from IORT Hanjun Guo
2016-10-25 15:09 ` [PATCH v3 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI Hanjun Guo
2016-10-25 15:09 ` [PATCH v3 07/14] irqchip: gicv3-its: platform-msi: scan MADT to create platform msi domain Hanjun Guo
2016-10-25 15:09 ` [PATCH v3 08/14] ACPI: ARM64: IORT: rework iort_node_get_id() Hanjun Guo
2016-10-25 15:09 ` [PATCH v3 09/14] ACPI: platform: setup MSI domain for ACPI based platform device Hanjun Guo
2016-10-25 15:09 ` [PATCH v3 10/14] msi: platform: make platform_msi_create_device_domain() ACPI aware Hanjun Guo
2016-10-25 15:09 ` [PATCH v3 11/14] ACPI: irq: introduce interrupt producer Hanjun Guo
2016-10-28 19:32   ` agustinv
2016-11-02 21:09     ` Hanjun Guo
2016-12-01 11:12   ` Aleksey Makarov
2016-12-02 10:07     ` Hanjun Guo
2016-12-03 12:18   ` G Gregory
2016-10-25 15:09 ` [PATCH v3 12/14] irqchip: mbigen: drop module owner Hanjun Guo
2016-10-25 15:09 ` [PATCH v3 13/14] irqchip: mbigen: introduce mbigen_of_create_domain() Hanjun Guo
2016-10-25 15:09 ` [PATCH v3 14/14] irqchip: mbigen: Add ACPI support Hanjun Guo

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