[PATCH v6 00/14] ACPI platform MSI support and its example mbigen

[PATCH v6 00/14] ACPI platform MSI support and its example mbigen

[Hanjun Guo]

v5 -> v6:
	- Call acpi_configure_pmsi_domain() for platform devices in
	  acpi_platform_notify() as it's cleaner (suggested by Rafael)
	- Remove the "u8 type" for iort_id_map() because it's unused
	- Rebase on top of 4.10-rc2
	- Collect test and review tags

v4 -> v5:
	- Add mbigen support back with tested on with Agustin's patchset,
	  and it's a good example of how ACPI platform MSI works
	- rebased on top of lastest Linus tree (commit 52bce91 splice: reinstate SIGPIPE/EPIPE handling)

v3 -> v4:
        - Drop mbi-gen patches to just submit platform msi support because
          will rebase mbi-gen patches on top of Agustin's patchset, and discusion
          is going there.
        - Add a patch to support device topology such as NC(named componant, paltform device)
          ->SMMU->ITS which suggested by Lorenzo;
        - rebased on top of Lorenzo's v9 of ACPI IORT ARM SMMU support;
        - rebased on top of 4.9-rc7

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

v1 -> v2:
        - 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) and SMMU, 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).

The fisrt 3 patches are cleanups;

Patch 4,5 are refactoring its_pmsi_prepare() for both DT and ACPI
then retrieve the dev id from iort;

Patch 6,7 to create platform msi domain to ACPI case which scanned
the MADT table;

Patch 8,9,10,11 to setup the msi domain for platform device based
on IORT table.

Patch 12,13,14 convert DT based mbigen driver to support ACPI/DT.

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
  ACPI: ARM64: IORT: rework iort_node_get_id() for NC->SMMU->ITS case
  msi: platform: make platform_msi_create_device_domain() ACPI aware
  irqchip: mbigen: Add ACPI support

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

 drivers/acpi/arm64/iort.c                     | 140 ++++++++++++++++++++------
 drivers/acpi/glue.c                           |   6 ++
 drivers/base/platform-msi.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/linux/acpi_iort.h                     |  11 ++
 7 files changed, 299 insertions(+), 79 deletions(-)

-- 
1.9.1

[PATCH v6 01/14] ACPI: ARM64: IORT: minor cleanup for iort_match_node_callback()

[Hanjun Guo]

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>
Tested-by: Majun <majun258@huawei.com>
Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
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 e0d2e6e..46e2d82 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.9.1

[PATCH v6 02/14] irqchip: gic-v3-its: keep the head file include in alphabetic order

[Hanjun Guo]

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>
Tested-by: Majun <majun258@huawei.com>
Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
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 69b040f..f471939 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.9.1

[PATCH v6 03/14] ACPI: ARM64: IORT: add missing comment for iort_dev_find_its_id()

[Hanjun Guo]

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>
Tested-by: Majun <majun258@huawei.com>
Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
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 46e2d82..174e983 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.9.1

[PATCH v6 04/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_prepare()

[Hanjun Guo]

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>
Tested-by: Majun <majun258@huawei.com>
Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
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.9.1

[PATCH v6 05/14] ACPI: platform-msi: retrieve dev id from IORT

[Hanjun Guo]

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>
Tested-by: Majun <majun258@huawei.com>
Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
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 174e983..ab7bae7 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 77e0809..ef99fd52 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 */
 void iort_set_dma_mask(struct device *dev);
@@ -42,9 +43,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 void iort_set_dma_mask(struct device *dev) { }
 static inline
-- 
1.9.1

[PATCH v6 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI

[Hanjun Guo]

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>
Tested-by: Majun <majun258@huawei.com>
Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
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.9.1

[PATCH v6 07/14] irqchip: gicv3-its: platform-msi: scan MADT to create platform msi domain

[Hanjun Guo]

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>
Tested-by: Majun <majun258@huawei.com>
Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
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.9.1

[PATCH v6 08/14] ACPI: ARM64: IORT: rework iort_node_get_id()

[Hanjun Guo]

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>
Tested-by: Majun <majun258@huawei.com>
Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
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 ab7bae7..bc68d93 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.9.1

[PATCH v6 09/14] ACPI: platform: setup MSI domain for ACPI based platform device

[Hanjun Guo]

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 (the time slot of setting the
msi domain also works for other cases). So simply call
acpi_configure_pmsi_domain() in acpi_platform_notify() for
platform devices will work.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 drivers/acpi/arm64/iort.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/glue.c       |  6 ++++++
 include/linux/acpi_iort.h |  3 +++
 3 files changed, 52 insertions(+)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index bc68d93..6b72fcb 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/acpi/glue.c b/drivers/acpi/glue.c
index f8d6564..4a73f27 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -6,6 +6,8 @@
  *
  * This file is released under the GPLv2.
  */
+
+#include <linux/acpi_iort.h>
 #include <linux/export.h>
 #include <linux/init.h>
 #include <linux/list.h>
@@ -14,6 +16,7 @@
 #include <linux/rwsem.h>
 #include <linux/acpi.h>
 #include <linux/dma-mapping.h>
+#include <linux/platform_device.h>
 
 #include "internal.h"
 
@@ -315,6 +318,9 @@ static int acpi_platform_notify(struct device *dev)
 	if (!adev)
 		goto out;
 
+	if (dev->bus == &platform_bus_type)
+		acpi_configure_pmsi_domain(dev);
+
 	if (type && type->setup)
 		type->setup(dev);
 	else if (adev->handler && adev->handler->bind)
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index ef99fd52..33f5ac3 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -38,6 +38,7 @@
 /* IOMMU interface */
 void iort_set_dma_mask(struct device *dev);
 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; }
@@ -58,6 +59,8 @@ static inline void iort_set_dma_mask(struct device *dev) { }
 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)		\
-- 
1.9.1

[PATCH v6 10/14] ACPI: ARM64: IORT: rework iort_node_get_id() for NC->SMMU->ITS case

[Hanjun Guo]

iort_node_get_id() for now only support NC(named componant)->SMMU
or NC->ITS cases, we also have other device topology such NC->
SMMU->ITS, so rework iort_node_get_id() for those cases.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Majun <majun258@huawei.com>
Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 drivers/acpi/arm64/iort.c | 61 ++++++++++++++++++++++++++---------------------
 1 file changed, 34 insertions(+), 27 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 6b72fcb..99f079b 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -292,22 +292,28 @@ static acpi_status iort_match_node_callback(struct acpi_iort_node *node,
 	return status;
 }
 
-static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in,
-		       u32 *rid_out)
+static int iort_id_single_map(struct acpi_iort_id_mapping *map, u8 type,
+			      u32 *rid_out)
 {
 	/* Single mapping does not care for input id */
 	if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
 		if (type == ACPI_IORT_NODE_NAMED_COMPONENT ||
 		    type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
-			*rid_out = map->output_base;
+			if (rid_out)
+				*rid_out = map->output_base;
 			return 0;
 		}
 
 		pr_warn(FW_BUG "[map %p] SINGLE MAPPING flag not allowed for node type %d, skipping ID map\n",
 			map, type);
-		return -ENXIO;
 	}
 
+	return -ENXIO;
+}
+
+static int iort_id_map(struct acpi_iort_id_mapping *map, u32 rid_in,
+		       u32 *rid_out)
+{
 	if (rid_in < map->input_base ||
 	    (rid_in >= map->input_base + map->id_count))
 		return -ENXIO;
@@ -324,33 +330,34 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
 	struct acpi_iort_node *parent;
 	struct acpi_iort_id_mapping *map;
 
-	if (!node->mapping_offset || !node->mapping_count ||
-				     index >= node->mapping_count)
-		return NULL;
-
-	map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
-			   node->mapping_offset);
+	while (node) {
+		if (!node->mapping_offset || !node->mapping_count ||
+					     index >= node->mapping_count)
+			return NULL;
 
-	/* Firmware bug! */
-	if (!map->output_reference) {
-		pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n",
-		       node, node->type);
-		return NULL;
-	}
+		map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
+				   node->mapping_offset);
 
-	parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
-			       map->output_reference);
+		/* Firmware bug! */
+		if (!map->output_reference) {
+			pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n",
+			       node, node->type);
+			return NULL;
+		}
 
-	if (!(IORT_TYPE_MASK(parent->type) & type_mask))
-		return NULL;
+		parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
+				      map->output_reference);
 
-	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) {
-			if (id_out)
-				*id_out = map[index].output_base;
-			return parent;
+		/* go upstream to find its parent */
+		if (!(IORT_TYPE_MASK(parent->type) & type_mask)) {
+			node = parent;
+			continue;
 		}
+
+		if (iort_id_single_map(&map[index], node->type, id_out))
+			break;
+
+		return parent;
 	}
 
 	return NULL;
@@ -388,7 +395,7 @@ static struct acpi_iort_node *iort_node_map_rid(struct acpi_iort_node *node,
 
 		/* Do the RID translation */
 		for (i = 0; i < node->mapping_count; i++, map++) {
-			if (!iort_id_map(map, node->type, rid, &rid))
+			if (!iort_id_map(map, rid, &rid))
 				break;
 		}
 
-- 
1.9.1

[PATCH v6 11/14] msi: platform: make platform_msi_create_device_domain() ACPI aware

[Hanjun Guo]

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>
Tested-by: Majun <majun258@huawei.com>
Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 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.9.1

[PATCH v6 12/14] irqchip: mbigen: drop module owner

[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>
Reviewed-by: Majun <majun258@huawei.com>
Tested-by: Majun <majun258@huawei.com>
Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
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.9.1

[PATCH v6 13/14] irqchip: mbigen: introduce mbigen_of_create_domain()

[Hanjun Guo]

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

Introduce mbigen_of_create_domain() to consolidate OF related
code and prepare for ACPI later, no funtional change.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Reviewed-by: Majun <majun258@huawei.com>
Tested-by: Majun <majun258@huawei.com>
Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 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..4e11da5 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(&pdev->dev, res->start, resource_size(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.9.1

[PATCH v6 14/14] irqchip: mbigen: Add ACPI support

[Hanjun Guo]

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>
Reviewed-by: Majun <majun258@huawei.com>
Tested-by: Majun <majun258@huawei.com>
Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 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 4e11da5..17d35fa 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.9.1

Re: [PATCH v6 09/14] ACPI: platform: setup MSI domain for ACPI based platform device

[Rafael J. Wysocki]

On Mon, Jan 2, 2017 at 2:31 PM, Hanjun Guo <hanjun.guo@linaro.org> wrote:
> 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 (the time slot of setting the
> msi domain also works for other cases). So simply call
> acpi_configure_pmsi_domain() in acpi_platform_notify() for
> platform devices will work.
>
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

ACK for the glue.c part.

> ---
>  drivers/acpi/arm64/iort.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/glue.c       |  6 ++++++
>  include/linux/acpi_iort.h |  3 +++
>  3 files changed, 52 insertions(+)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index bc68d93..6b72fcb 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/acpi/glue.c b/drivers/acpi/glue.c
> index f8d6564..4a73f27 100644
> --- a/drivers/acpi/glue.c
> +++ b/drivers/acpi/glue.c
> @@ -6,6 +6,8 @@
>   *
>   * This file is released under the GPLv2.
>   */
> +
> +#include <linux/acpi_iort.h>
>  #include <linux/export.h>
>  #include <linux/init.h>
>  #include <linux/list.h>
> @@ -14,6 +16,7 @@
>  #include <linux/rwsem.h>
>  #include <linux/acpi.h>
>  #include <linux/dma-mapping.h>
> +#include <linux/platform_device.h>
>
>  #include "internal.h"
>
> @@ -315,6 +318,9 @@ static int acpi_platform_notify(struct device *dev)
>         if (!adev)
>                 goto out;
>
> +       if (dev->bus == &platform_bus_type)
> +               acpi_configure_pmsi_domain(dev);
> +
>         if (type && type->setup)
>                 type->setup(dev);
>         else if (adev->handler && adev->handler->bind)
> diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
> index ef99fd52..33f5ac3 100644
> --- a/include/linux/acpi_iort.h
> +++ b/include/linux/acpi_iort.h
> @@ -38,6 +38,7 @@
>  /* IOMMU interface */
>  void iort_set_dma_mask(struct device *dev);
>  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; }
> @@ -58,6 +59,8 @@ static inline void iort_set_dma_mask(struct device *dev) { }
>  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)          \
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 10/14] ACPI: ARM64: IORT: rework iort_node_get_id() for NC->SMMU->ITS case

[Sinan Kaya]

[-- Attachment #1: Type: text/plain, Size: 5008 bytes --]

Hi Hanjun,

On 1/2/2017 8:31 AM, Hanjun Guo wrote:
> iort_node_get_id() for now only support NC(named componant)->SMMU
> or NC->ITS cases, we also have other device topology such NC->
> SMMU->ITS, so rework iort_node_get_id() for those cases.
> 
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> Tested-by: Majun <majun258@huawei.com>
> Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> ---
>  drivers/acpi/arm64/iort.c | 61 ++++++++++++++++++++++++++---------------------
>  1 file changed, 34 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 6b72fcb..99f079b 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -292,22 +292,28 @@ static acpi_status iort_match_node_callback(struct acpi_iort_node *node,
>  	return status;
>  }
>  
> -static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in,
> -		       u32 *rid_out)
> +static int iort_id_single_map(struct acpi_iort_id_mapping *map, u8 type,
> +			      u32 *rid_out)
>  {
>  	/* Single mapping does not care for input id */
>  	if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
>  		if (type == ACPI_IORT_NODE_NAMED_COMPONENT ||
>  		    type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
> -			*rid_out = map->output_base;
> +			if (rid_out)
> +				*rid_out = map->output_base;
>  			return 0;
>  		}
>  
>  		pr_warn(FW_BUG "[map %p] SINGLE MAPPING flag not allowed for node type %d, skipping ID map\n",
>  			map, type);
> -		return -ENXIO;
>  	}
>  
> +	return -ENXIO;
> +}
> +
> +static int iort_id_map(struct acpi_iort_id_mapping *map, u32 rid_in,
> +		       u32 *rid_out)
> +{
>  	if (rid_in < map->input_base ||
>  	    (rid_in >= map->input_base + map->id_count))
>  		return -ENXIO;
> @@ -324,33 +330,34 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
>  	struct acpi_iort_node *parent;
>  	struct acpi_iort_id_mapping *map;
>  
> -	if (!node->mapping_offset || !node->mapping_count ||
> -				     index >= node->mapping_count)
> -		return NULL;
> -
> -	map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
> -			   node->mapping_offset);
> +	while (node) {
> +		if (!node->mapping_offset || !node->mapping_count ||
> +					     index >= node->mapping_count)
> +			return NULL;
>  
> -	/* Firmware bug! */
> -	if (!map->output_reference) {
> -		pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n",
> -		       node, node->type);
> -		return NULL;
> -	}
> +		map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
> +				   node->mapping_offset);
>  
> -	parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> -			       map->output_reference);
> +		/* Firmware bug! */
> +		if (!map->output_reference) {
> +			pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n",
> +			       node, node->type);
> +			return NULL;
> +		}
>  
> -	if (!(IORT_TYPE_MASK(parent->type) & type_mask))
> -		return NULL;
> +		parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> +				      map->output_reference);
>  
> -	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) {
> -			if (id_out)
> -				*id_out = map[index].output_base;
> -			return parent;
> +		/* go upstream to find its parent */
> +		if (!(IORT_TYPE_MASK(parent->type) & type_mask)) {
> +			node = parent;
> +			continue;
>  		}
> +
> +		if (iort_id_single_map(&map[index], node->type, id_out))
> +			break;
> +
> +		return parent;
>  	}
>  
>  	return NULL;
> @@ -388,7 +395,7 @@ static struct acpi_iort_node *iort_node_map_rid(struct acpi_iort_node *node,
>  
>  		/* Do the RID translation */
>  		for (i = 0; i < node->mapping_count; i++, map++) {
> -			if (!iort_id_map(map, node->type, rid, &rid))
> +			if (!iort_id_map(map, rid, &rid))
>  				break;
>  		}
>  
> 

I wanted to follow up on your note for NC->SMMU->ITS case as I do have this use case on the 
Qualcomm QDF2400 server and HIDMA DMA Engine. HIDMA is capable of sending MSI interrupts
towards the GIC ITS. 

I don't know if this patch is supposed to fix the NC->SMMU->ITS case as it suggests in the commit
message but it doesn't seems to be working for me. Maybe, it was a to do for you. It wasn't quite
clear from the commit. 

I debugged the code and came up with the following patch. Feel free to incorporate/rework with
your existing patch.

A named node can have an output ID of 0x20 and SMMU can have an output
parameter of 0x80000. The device ID needs to be 0x80000+0x20 for this
use case. 

With the addition of this patch on top of the first 11 patches, I'm also providing my tested by here
for the first 11 patches.

Tested-by: Sinan Kaya <okaya@codeaurora.org>

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

[-- Attachment #2: 0001-ACPI-ARM64-IORT-rework-iort_node_get_id-for-NC-SMMU-.patch --]
[-- Type: text/plain, Size: 3302 bytes --]

>From c5ab7172a400bfd5b460374e70394fe78c260603 Mon Sep 17 00:00:00 2001
From: Sinan Kaya <okaya@codeaurora.org>
Date: Mon, 2 Jan 2017 17:16:45 -0500
Subject: [PATCH] ACPI: ARM64: IORT: rework iort_node_get_id() for
 NC->SMMU->ITS case part #2

Code won't collect the output ID as it traverses NC->SMMU->ITS path.
Adding support for this use case.

A named node can have an output ID of 0x20 and SMMU can have an output
parameter of 0x80000. The device ID needs to be 0x80000+0x20 for this
use case.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/acpi/arm64/iort.c | 58 ++++++++++++++++++++++++++---------------------
 1 file changed, 32 insertions(+), 26 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 882e624..19cb97a 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -296,18 +296,16 @@ static int iort_id_single_map(struct acpi_iort_id_mapping *map, u8 type,
 			      u32 *rid_out)
 {
 	/* Single mapping does not care for input id */
-	if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
-		if (type == ACPI_IORT_NODE_NAMED_COMPONENT ||
-		    type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
-			if (rid_out)
-				*rid_out = map->output_base;
-			return 0;
-		}
-
-		pr_warn(FW_BUG "[map %p] SINGLE MAPPING flag not allowed for node type %d, skipping ID map\n",
-			map, type);
+	if (type == ACPI_IORT_NODE_NAMED_COMPONENT ||
+	    type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
+		if (rid_out)
+			*rid_out = map->output_base;
+		return 0;
 	}

+	pr_warn(FW_BUG "[map %p] SINGLE MAPPING flag not allowed for node type %d, skipping ID map\n",
+		map, type);
+
 	return -ENXIO;
 }

@@ -327,13 +325,20 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
 					u32 *id_out, u8 type_mask,
 					int index)
 {
-	struct acpi_iort_node *parent;
-	struct acpi_iort_id_mapping *map;
+	u32 id = 0;

 	while (node) {
-		if (!node->mapping_offset || !node->mapping_count ||
-					     index >= node->mapping_count)
-			return NULL;
+		struct acpi_iort_id_mapping *map;
+
+		if (IORT_TYPE_MASK(node->type) & type_mask) {
+			if (id_out)
+				*id_out = id;
+
+			return node;
+		}
+
+		if (!node->mapping_offset || !node->mapping_count)
+			goto fail_map;

 		map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
 				   node->mapping_offset);
@@ -342,24 +347,25 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
 		if (!map->output_reference) {
 			pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n",
 			       node, node->type);
-			return NULL;
+			goto fail_map;
 		}

-		parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
-				      map->output_reference);
-
-		/* go upstream to find its parent */
-		if (!(IORT_TYPE_MASK(parent->type) & type_mask)) {
-			node = parent;
-			continue;
+		if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
+			if (iort_id_single_map(&map[index], node->type, &id))
+				goto fail_map;
+		} else {
+			if (iort_id_map(map, id, &id))
+				goto fail_map;
 		}

-		if (iort_id_single_map(&map[index], node->type, id_out))
-			break;
+		if (index == node->mapping_count)
+			goto fail_map;

-		return parent;
+		node = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
+				      map->output_reference);
 	}

+fail_map:
 	return NULL;
 }

--
1.9.1

Re: [PATCH v6 10/14] ACPI: ARM64: IORT: rework iort_node_get_id() for NC->SMMU->ITS case

[Hanjun Guo]

Hi Sinan,

On 01/03/2017 06:30 AM, Sinan Kaya wrote:
> Hi Hanjun,
>
> On 1/2/2017 8:31 AM, Hanjun Guo wrote:
>> iort_node_get_id() for now only support NC(named componant)->SMMU
>> or NC->ITS cases, we also have other device topology such NC->
>> SMMU->ITS, so rework iort_node_get_id() for those cases.
>>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>> Tested-by: Majun <majun258@huawei.com>
>> Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> ---
>>   drivers/acpi/arm64/iort.c | 61 ++++++++++++++++++++++++++---------------------
>>   1 file changed, 34 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>> index 6b72fcb..99f079b 100644
>> --- a/drivers/acpi/arm64/iort.c
>> +++ b/drivers/acpi/arm64/iort.c
>> @@ -292,22 +292,28 @@ static acpi_status iort_match_node_callback(struct acpi_iort_node *node,
>>   	return status;
>>   }
>>
>> -static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in,
>> -		       u32 *rid_out)
>> +static int iort_id_single_map(struct acpi_iort_id_mapping *map, u8 type,
>> +			      u32 *rid_out)
>>   {
>>   	/* Single mapping does not care for input id */
>>   	if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
>>   		if (type == ACPI_IORT_NODE_NAMED_COMPONENT ||
>>   		    type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
>> -			*rid_out = map->output_base;
>> +			if (rid_out)
>> +				*rid_out = map->output_base;
>>   			return 0;
>>   		}
>>
>>   		pr_warn(FW_BUG "[map %p] SINGLE MAPPING flag not allowed for node type %d, skipping ID map\n",
>>   			map, type);
>> -		return -ENXIO;
>>   	}
>>
>> +	return -ENXIO;
>> +}
>> +
>> +static int iort_id_map(struct acpi_iort_id_mapping *map, u32 rid_in,
>> +		       u32 *rid_out)
>> +{
>>   	if (rid_in < map->input_base ||
>>   	    (rid_in >= map->input_base + map->id_count))
>>   		return -ENXIO;
>> @@ -324,33 +330,34 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
>>   	struct acpi_iort_node *parent;
>>   	struct acpi_iort_id_mapping *map;
>>
>> -	if (!node->mapping_offset || !node->mapping_count ||
>> -				     index >= node->mapping_count)
>> -		return NULL;
>> -
>> -	map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
>> -			   node->mapping_offset);
>> +	while (node) {
>> +		if (!node->mapping_offset || !node->mapping_count ||
>> +					     index >= node->mapping_count)
>> +			return NULL;
>>
>> -	/* Firmware bug! */
>> -	if (!map->output_reference) {
>> -		pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n",
>> -		       node, node->type);
>> -		return NULL;
>> -	}
>> +		map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
>> +				   node->mapping_offset);
>>
>> -	parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
>> -			       map->output_reference);
>> +		/* Firmware bug! */
>> +		if (!map->output_reference) {
>> +			pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n",
>> +			       node, node->type);
>> +			return NULL;
>> +		}
>>
>> -	if (!(IORT_TYPE_MASK(parent->type) & type_mask))
>> -		return NULL;
>> +		parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
>> +				      map->output_reference);
>>
>> -	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) {
>> -			if (id_out)
>> -				*id_out = map[index].output_base;
>> -			return parent;
>> +		/* go upstream to find its parent */
>> +		if (!(IORT_TYPE_MASK(parent->type) & type_mask)) {
>> +			node = parent;
>> +			continue;
>>   		}
>> +
>> +		if (iort_id_single_map(&map[index], node->type, id_out))
>> +			break;
>> +
>> +		return parent;
>>   	}
>>
>>   	return NULL;
>> @@ -388,7 +395,7 @@ static struct acpi_iort_node *iort_node_map_rid(struct acpi_iort_node *node,
>>
>>   		/* Do the RID translation */
>>   		for (i = 0; i < node->mapping_count; i++, map++) {
>> -			if (!iort_id_map(map, node->type, rid, &rid))
>> +			if (!iort_id_map(map, rid, &rid))
>>   				break;
>>   		}
>>
>>
>
> I wanted to follow up on your note for NC->SMMU->ITS case as I do have this use case on the
> Qualcomm QDF2400 server and HIDMA DMA Engine. HIDMA is capable of sending MSI interrupts
> towards the GIC ITS.
>
> I don't know if this patch is supposed to fix the NC->SMMU->ITS case as it suggests in the commit
> message but it doesn't seems to be working for me. Maybe, it was a to do for you. It wasn't quite
> clear from the commit.

I noticed this issue too after I sent out this patch set, sorry :(

>
> I debugged the code and came up with the following patch. Feel free to incorporate/rework with
> your existing patch.
>
> A named node can have an output ID of 0x20 and SMMU can have an output
> parameter of 0x80000. The device ID needs to be 0x80000+0x20 for this
> use case.

I think in your case, there are muti input IDs with multi output IDs,
such as:

                stream id                               request id
NC (0x00~0x30) --------> SMMU (0x80000~0x80000+0x30)  ------------> ITS

In my patch, I just think named component is single mapping only, and
multi ID mappings for PCI RC, that's the wrong assumption, I will
incorporate your patch to fix the problem in next version.

>
> With the addition of this patch on top of the first 11 patches, I'm also providing my tested by here
> for the first 11 patches.
>
> Tested-by: Sinan Kaya <okaya@codeaurora.org>

Thank you very much :)

Hanjun

Re: [PATCH v6 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI

[Tomasz Nowicki]

Hi,

Can we merge patch 4 & 6 into one patch so that we keep refactoring part 
as one piece ? I do not see a reason to keep them separate or have patch 
5 in between. You can refactor what needs to be refactored, add 
necessary functions to iort.c and then support ACPI for 
irq-gic-v3-its-platform-msi.c

Thanks,
Tomasz

On 02.01.2017 14:31, Hanjun Guo wrote:
> 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>
> Tested-by: Majun <majun258@huawei.com>
> Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> 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);
>

Re: [PATCH v6 05/14] ACPI: platform-msi: retrieve dev id from IORT

[Tomasz Nowicki]

On 02.01.2017 14:31, Hanjun Guo wrote:
> 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>
> Tested-by: Majun <majun258@huawei.com>
> Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> 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 174e983..ab7bae7 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;
> +}
> +
> +/**
Giving that you are extending this to NC->
SMMU->ITS case in later patch, we can use existing helpers from iort.c, 
like that:

+/**
+ * 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;
+
+	node = iort_find_dev_node(dev);
+	if (!node)
+		return -ENODEV;
+
+	if (!iort_node_map_rid(node, 0, dev_id, IORT_MSI_TYPE))
+		return -ENODEV;
+
+	return 0;
+}

Correct me if I am wrong.

Tomasz

Re: [PATCH v6 05/14] ACPI: platform-msi: retrieve dev id from IORT

[Tomasz Nowicki]

On 03.01.2017 09:43, Tomasz Nowicki wrote:
> On 02.01.2017 14:31, Hanjun Guo wrote:
>> 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>
>> Tested-by: Majun <majun258@huawei.com>
>> Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
>> 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 174e983..ab7bae7 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;
>> +}
>> +
>> +/**
> Giving that you are extending this to NC->
> SMMU->ITS case in later patch, we can use existing helpers from iort.c,
> like that:
>
> +/**
> + * 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;
> +
> +    node = iort_find_dev_node(dev);
> +    if (!node)
> +        return -ENODEV;
> +
> +    if (!iort_node_map_rid(node, 0, dev_id, IORT_MSI_TYPE))
> +        return -ENODEV;
> +
> +    return 0;
> +}
>
> Correct me if I am wrong.
>

"0" as rid_in for iort_node_map_rid() isn't good idea, sorry...

Tomasz

Re: [PATCH v6 05/14] ACPI: platform-msi: retrieve dev id from IORT

[Tomasz Nowicki]

On 03.01.2017 10:37, Tomasz Nowicki wrote:
> On 03.01.2017 09:43, Tomasz Nowicki wrote:
>> On 02.01.2017 14:31, Hanjun Guo wrote:
>>> 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>
>>> Tested-by: Majun <majun258@huawei.com>
>>> Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
>>> 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 174e983..ab7bae7 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;
>>> +}
>>> +
>>> +/**
>> Giving that you are extending this to NC->
>> SMMU->ITS case in later patch, we can use existing helpers from iort.c,
>> like that:
>>
>> +/**
>> + * 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;
>> +
>> +    node = iort_find_dev_node(dev);
>> +    if (!node)
>> +        return -ENODEV;
>> +
>> +    if (!iort_node_map_rid(node, 0, dev_id, IORT_MSI_TYPE))
>> +        return -ENODEV;
>> +
>> +    return 0;
>> +}
>>
>> Correct me if I am wrong.
>>
>
> "0" as rid_in for iort_node_map_rid() isn't good idea, sorry...
>

I refactored iort_node_map_rid() and added new 
iort_node_map_single_rid() which should works for you. Below patch bases 
on v4.10-rc2:

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index e0d2e6e..2526b89 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -297,8 +297,17 @@ static acpi_status iort_match_node_callback(struct 
acpi_iort_node *node,
  }

  static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 
rid_in,
-		       u32 *rid_out)
+		       u32 *rid_out, bool single)
  {
+	if (!single) {
+		if (rid_in < map->input_base ||
+		    (rid_in >= map->input_base + map->id_count))
+			return -ENXIO;
+
+		*rid_out = map->output_base + (rid_in - map->input_base);
+		return 0;
+	}
+
  	/* Single mapping does not care for input id */
  	if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
  		if (type == ACPI_IORT_NODE_NAMED_COMPONENT ||
@@ -309,15 +318,9 @@ static int iort_id_map(struct acpi_iort_id_mapping 
*map, u8 type, u32 rid_in,

  		pr_warn(FW_BUG "[map %p] SINGLE MAPPING flag not allowed for node 
type %d, skipping ID map\n",
  			map, type);
-		return -ENXIO;
  	}

-	if (rid_in < map->input_base ||
-	    (rid_in >= map->input_base + map->id_count))
-		return -ENXIO;
-
-	*rid_out = map->output_base + (rid_in - map->input_base);
-	return 0;
+	return -ENXIO;
  }

  static
@@ -359,6 +362,38 @@ struct acpi_iort_node *iort_node_get_id(struct 
acpi_iort_node *node,
  	return NULL;
  }

+static struct acpi_iort_node *iort_node_map_rid_one(struct 
acpi_iort_node *node,
+						    u32 *rid, bool single)
+{
+	struct acpi_iort_id_mapping *map;
+	struct acpi_iort_node *next;
+	int i;
+
+	if (!node->mapping_offset || !node->mapping_count)
+		return NULL;
+
+	map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
+			   node->mapping_offset);
+
+	/* Firmware bug! */
+	if (!map->output_reference) {
+		pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n",
+		       node, node->type);
+		return NULL;
+	}
+
+	next = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
+			    map->output_reference);
+
+	/* Do the RID translation */
+	for (i = 0; i < node->mapping_count; i++, map++) {
+		if (!iort_id_map(map, node->type, *rid, rid, single))
+			return next;
+	}
+
+	return NULL;
+}
+
  static struct acpi_iort_node *iort_node_map_rid(struct acpi_iort_node 
*node,
  						u32 rid_in, u32 *rid_out,
  						u8 type_mask)
@@ -367,42 +402,15 @@ static struct acpi_iort_node 
*iort_node_map_rid(struct acpi_iort_node *node,

  	/* Parse the ID mapping tree to find specified node type */
  	while (node) {
-		struct acpi_iort_id_mapping *map;
-		int i;
-
  		if (IORT_TYPE_MASK(node->type) & type_mask) {
  			if (rid_out)
  				*rid_out = rid;
  			return node;
  		}

-		if (!node->mapping_offset || !node->mapping_count)
-			goto fail_map;
-
-		map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
-				   node->mapping_offset);
-
-		/* Firmware bug! */
-		if (!map->output_reference) {
-			pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n",
-			       node, node->type);
-			goto fail_map;
-		}
-
-		/* Do the RID translation */
-		for (i = 0; i < node->mapping_count; i++, map++) {
-			if (!iort_id_map(map, node->type, rid, &rid))
-				break;
-		}
-
-		if (i == node->mapping_count)
-			goto fail_map;
-
-		node = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
-				    map->output_reference);
+		node = iort_node_map_rid_one(node, &rid, false);
  	}

-fail_map:
  	/* Map input RID to output RID unchanged on mapping failure*/
  	if (rid_out)
  		*rid_out = rid_in;
@@ -410,6 +418,20 @@ static struct acpi_iort_node 
*iort_node_map_rid(struct acpi_iort_node *node,
  	return NULL;
  }

+static struct acpi_iort_node *iort_node_map_single_rid(
+						struct acpi_iort_node *node,
+						u32 *rid_out, u8 type_mask)
+{
+	u32 rid;
+
+	/* Get the initial rid */
+	node = iort_node_map_rid_one(node, &rid, true);
+	if (!node)
+		return NULL;
+
+	return iort_node_map_rid(node, rid, rid_out, type_mask);
+}
+
  static struct acpi_iort_node *iort_find_dev_node(struct device *dev)
  {
  	struct pci_bus *pbus;

Re: [PATCH v6 01/14] ACPI: ARM64: IORT: minor cleanup for iort_match_node_callback()

[Lorenzo Pieralisi]

On Mon, Jan 02, 2017 at 09:31:32PM +0800, Hanjun Guo wrote:
> Cleanup iort_match_node_callback() a little bit to reduce
> some lines of code, aslo fix the indentation in iort_scan_node().

s/aslo/also

"Also" in a commit log is a sign a patch should be split and that's what
you should do even though I know it is tempting to merge all trivial
changes into one single patch.

Make it two patches please.

Thanks,
Lorenzo

> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> Tested-by: Majun <majun258@huawei.com>
> Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> 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 e0d2e6e..46e2d82 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.9.1
> 

Re: [PATCH v6 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI

[Hanjun Guo]

Hi Tomasz,

On 2017/1/3 15:41, Tomasz Nowicki wrote:
> Hi,
>
> Can we merge patch 4 & 6 into one patch so that we keep refactoring part
> as one piece ? I do not see a reason to keep them separate or have patch
> 5 in between. You can refactor what needs to be refactored, add
> necessary functions to iort.c and then support ACPI for
> irq-gic-v3-its-platform-msi.c

There are two functions here,
  - retrieve the dev id from IORT which was DT based only;

  - init the platform msi domain from MADT;

For each of them split it into two steps,
  - refactor the code for ACPI later and it's easy for review
    because wen can easily to figure out it has functional
    change or not

  - add ACPI functionality

Does it make sense?

Thanks
Hanjun

Re: [PATCH v6 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI

[Tomasz Nowicki]

On 04.01.2017 08:02, Hanjun Guo wrote:
> Hi Tomasz,
>
> On 2017/1/3 15:41, Tomasz Nowicki wrote:
>> Hi,
>>
>> Can we merge patch 4 & 6 into one patch so that we keep refactoring part
>> as one piece ? I do not see a reason to keep them separate or have patch
>> 5 in between. You can refactor what needs to be refactored, add
>> necessary functions to iort.c and then support ACPI for
>> irq-gic-v3-its-platform-msi.c
>
> There are two functions here,
>  - retrieve the dev id from IORT which was DT based only;
>
>  - init the platform msi domain from MADT;
>
> For each of them split it into two steps,
>  - refactor the code for ACPI later and it's easy for review
>    because wen can easily to figure out it has functional
>    change or not
>
>  - add ACPI functionality
>
> Does it make sense?

It is up to Marc, but personally I prefer:
1. Refactor dev id retrieving and init function in one patch and 
highlight no functional changes in changelog
2. Crate necessary infrastructure in iort.c
3. Then add ACPI support to irq-gic-v3-its-platform-msi.c

Thanks,
Tomasz

Re: [PATCH v6 01/14] ACPI: ARM64: IORT: minor cleanup for iort_match_node_callback()

[Hanjun Guo]

Hi Lorenzo,

On 2017/1/3 22:08, Lorenzo Pieralisi wrote:
> On Mon, Jan 02, 2017 at 09:31:32PM +0800, Hanjun Guo wrote:
>> Cleanup iort_match_node_callback() a little bit to reduce
>> some lines of code, aslo fix the indentation in iort_scan_node().
>
> s/aslo/also
>
> "Also" in a commit log is a sign a patch should be split and that's what
> you should do even though I know it is tempting to merge all trivial
> changes into one single patch.
>
> Make it two patches please.

Will do, thanks!

Do you have more comments regarding this patch set? I will
incorporate all the comments and send out a new version.

Thanks
Hanjun

Re: [PATCH v6 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI

[Hanjun Guo]

On 2017/1/4 15:29, Tomasz Nowicki wrote:
> On 04.01.2017 08:02, Hanjun Guo wrote:
>> Hi Tomasz,
>>
>> On 2017/1/3 15:41, Tomasz Nowicki wrote:
>>> Hi,
>>>
>>> Can we merge patch 4 & 6 into one patch so that we keep refactoring part
>>> as one piece ? I do not see a reason to keep them separate or have patch
>>> 5 in between. You can refactor what needs to be refactored, add
>>> necessary functions to iort.c and then support ACPI for
>>> irq-gic-v3-its-platform-msi.c
>>
>> There are two functions here,
>>  - retrieve the dev id from IORT which was DT based only;
>>
>>  - init the platform msi domain from MADT;
>>
>> For each of them split it into two steps,
>>  - refactor the code for ACPI later and it's easy for review
>>    because wen can easily to figure out it has functional
>>    change or not
>>
>>  - add ACPI functionality
>>
>> Does it make sense?
>
> It is up to Marc, but personally I prefer:
> 1. Refactor dev id retrieving and init function in one patch and
> highlight no functional changes in changelog
> 2. Crate necessary infrastructure in iort.c
> 3. Then add ACPI support to irq-gic-v3-its-platform-msi.c

I have no strong preferences, and it's easy to do so as just
need to squash/reorder the patches.

Marc, Lorenzo, could you give some suggestions here?

Thanks
Hanjun

Re: [PATCH v6 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI

[Marc Zyngier]

On 04/01/17 08:25, Hanjun Guo wrote:
> On 2017/1/4 15:29, Tomasz Nowicki wrote:
>> On 04.01.2017 08:02, Hanjun Guo wrote:
>>> Hi Tomasz,
>>>
>>> On 2017/1/3 15:41, Tomasz Nowicki wrote:
>>>> Hi,
>>>>
>>>> Can we merge patch 4 & 6 into one patch so that we keep refactoring part
>>>> as one piece ? I do not see a reason to keep them separate or have patch
>>>> 5 in between. You can refactor what needs to be refactored, add
>>>> necessary functions to iort.c and then support ACPI for
>>>> irq-gic-v3-its-platform-msi.c
>>>
>>> There are two functions here,
>>>  - retrieve the dev id from IORT which was DT based only;
>>>
>>>  - init the platform msi domain from MADT;
>>>
>>> For each of them split it into two steps,
>>>  - refactor the code for ACPI later and it's easy for review
>>>    because wen can easily to figure out it has functional
>>>    change or not
>>>
>>>  - add ACPI functionality
>>>
>>> Does it make sense?
>>
>> It is up to Marc, but personally I prefer:
>> 1. Refactor dev id retrieving and init function in one patch and
>> highlight no functional changes in changelog
>> 2. Crate necessary infrastructure in iort.c
>> 3. Then add ACPI support to irq-gic-v3-its-platform-msi.c
> 
> I have no strong preferences, and it's easy to do so as just
> need to squash/reorder the patches.
> 
> Marc, Lorenzo, could you give some suggestions here?

I think it'd make the reviewing easier to have patches that are
semantically grouped together (all the ACPI IORT together, for example).

It would help understanding where you're aiming at instead of jumping
from irqchip to ACPI and back every other patch...

Thanks,

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

Re: [PATCH v6 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI

[Hanjun Guo]

On 2017/1/4 17:02, Marc Zyngier wrote:
> On 04/01/17 08:25, Hanjun Guo wrote:
>> On 2017/1/4 15:29, Tomasz Nowicki wrote:
>>> On 04.01.2017 08:02, Hanjun Guo wrote:
>>>> Hi Tomasz,
>>>>
>>>> On 2017/1/3 15:41, Tomasz Nowicki wrote:
>>>>> Hi,
>>>>>
>>>>> Can we merge patch 4 & 6 into one patch so that we keep refactoring part
>>>>> as one piece ? I do not see a reason to keep them separate or have patch
>>>>> 5 in between. You can refactor what needs to be refactored, add
>>>>> necessary functions to iort.c and then support ACPI for
>>>>> irq-gic-v3-its-platform-msi.c
>>>>
>>>> There are two functions here,
>>>>  - retrieve the dev id from IORT which was DT based only;
>>>>
>>>>  - init the platform msi domain from MADT;
>>>>
>>>> For each of them split it into two steps,
>>>>  - refactor the code for ACPI later and it's easy for review
>>>>    because wen can easily to figure out it has functional
>>>>    change or not
>>>>
>>>>  - add ACPI functionality
>>>>
>>>> Does it make sense?
>>>
>>> It is up to Marc, but personally I prefer:
>>> 1. Refactor dev id retrieving and init function in one patch and
>>> highlight no functional changes in changelog
>>> 2. Crate necessary infrastructure in iort.c
>>> 3. Then add ACPI support to irq-gic-v3-its-platform-msi.c
>>
>> I have no strong preferences, and it's easy to do so as just
>> need to squash/reorder the patches.
>>
>> Marc, Lorenzo, could you give some suggestions here?
>
> I think it'd make the reviewing easier to have patches that are
> semantically grouped together (all the ACPI IORT together, for example).
>
> It would help understanding where you're aiming at instead of jumping
> from irqchip to ACPI and back every other patch...

OK, I will reorder the patches and address the comments, then post
a new version.

Thanks
Hanjun

Re: [PATCH v6 03/14] ACPI: ARM64: IORT: add missing comment for iort_dev_find_its_id()

[Lorenzo Pieralisi]

On Mon, Jan 02, 2017 at 09:31:34PM +0800, Hanjun Guo wrote:
> We are missing req_id's comment for iort_dev_find_its_id(),
> add it back.

"Add missing req_id parameter to the iort_dev_find_its_id() function
kernel-doc comment."

> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> Tested-by: Majun <majun258@huawei.com>
> Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> 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 46e2d82..174e983 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

s/Requster/Requester

We can send it upstream independently along with some other patches
in this series but I will have a look at the whole series first.

Lorenzo

>   * @idx: Index of the ITS identifier list.
>   * @its_id: ITS identifier.
>   *
> -- 
> 1.9.1
> 

Re: [PATCH v6 11/14] msi: platform: make platform_msi_create_device_domain() ACPI aware

[Lorenzo Pieralisi]

On Mon, Jan 02, 2017 at 09:31:42PM +0800, Hanjun Guo wrote:
> With the platform msi domain created for ITS, irqchip such as
> mbi-gen connecting ITS, which needs ctreate its own irqdomain.

This patch touches generic platform-msi code, there is nothing ITS
and mbi-gen specific that has to be known here.

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

Do you really think that anyone reading this log can easily
make use of this statement ?

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

This commit log is unreadable and the readable bits do not contain
information that can be used for the purpose a commit log is made
for.

Please rewrite it in a way that can be used in the future to understand
what this patch does and why you want it in the kernel, thanks.

Lorenzo

> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> Tested-by: Majun <majun258@huawei.com>
> Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Greg KH <gregkh@linuxfoundation.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> ---
>  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.9.1
> 

Re: [PATCH v6 08/14] ACPI: ARM64: IORT: rework iort_node_get_id()

[Lorenzo Pieralisi]

On Mon, Jan 02, 2017 at 09:31:39PM +0800, Hanjun Guo wrote:
> 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.

I think we need to fix iort_node_get_id() first though, I am referring
to the index usage in relation to acpi_iort_id_mapping.output_reference
and related parent pointer retrieval as you reported to me, I am happy
to send it upstream independently.

As for this patch it is ok even though we can create an API that
just retrieve a node parent without fiddling about with passing
a NULL pointer for the id_out to achieve the same.

Thanks,
Lorenzo

> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> Tested-by: Majun <majun258@huawei.com>
> Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> 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 ab7bae7..bc68d93 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.9.1
> 

Re: [PATCH v6 05/14] ACPI: platform-msi: retrieve dev id from IORT

[Lorenzo Pieralisi]

On Mon, Jan 02, 2017 at 09:31:36PM +0800, Hanjun Guo wrote:
> 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>
> Tested-by: Majun <majun258@huawei.com>
> Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> 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 174e983..ab7bae7 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))

I disagree with this approach. For named components we know that
there are always two steps involved (second optional):

(1) Retrieve the initial id (this may well provide the final mapping)
(2) Map the id (optional if (1) represents the map type we need)

That's the reason why I kept iort_node_get_id() and iort_node_map_rid()
separated.

Now, what we can do is to create an iort_node_map_id() function that is
PCI agnostic (ie rename rid to id :)), whose rid_in is either a PCI RID
or the outcome of a previous call to iort_node_get_id() for named
components, that's in my opinion cleaner.

It would be even cleaner if you passed a type_mask (or write a
wrapper function for that) that is:

(IORT_MSI_TYPE | IORT_IOMMU_TYPE)

and we just use the returned parent pointer to check if the mapping
providing the initial id correspond to the type we are looking for (eg
ITS) or we need to map the retrieved initial id any further, with
iort_node_map_id(), to get to the final identifier.

Thoughts ?

Thanks,
Lorenzo

> +		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 77e0809..ef99fd52 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 */
>  void iort_set_dma_mask(struct device *dev);
> @@ -42,9 +43,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 void iort_set_dma_mask(struct device *dev) { }
>  static inline
> -- 
> 1.9.1
> 

Re: [PATCH v6 03/14] ACPI: ARM64: IORT: add missing comment for iort_dev_find_its_id()

[Hanjun Guo]

On 2017/1/4 22:34, Lorenzo Pieralisi wrote:
> On Mon, Jan 02, 2017 at 09:31:34PM +0800, Hanjun Guo wrote:
>> We are missing req_id's comment for iort_dev_find_its_id(),
>> add it back.
>
> "Add missing req_id parameter to the iort_dev_find_its_id() function
> kernel-doc comment."
>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>> Tested-by: Majun <majun258@huawei.com>
>> Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
>> 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 46e2d82..174e983 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
>
> s/Requster/Requester
>
> We can send it upstream independently along with some other patches
> in this series but I will have a look at the whole series first.

Do you mean go to 4.10-rcx?

Thanks
Hanjun

Re: [PATCH v6 08/14] ACPI: ARM64: IORT: rework iort_node_get_id()

[Hanjun Guo]

Hi Lorenzo,

On 2017/1/5 1:58, Lorenzo Pieralisi wrote:
> On Mon, Jan 02, 2017 at 09:31:39PM +0800, Hanjun Guo wrote:
>> 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.
>
> I think we need to fix iort_node_get_id() first though, I am referring
> to the index usage in relation to acpi_iort_id_mapping.output_reference
> and related parent pointer retrieval as you reported to me, I am happy
> to send it upstream independently.

Sure, please.

>
> As for this patch it is ok even though we can create an API that
> just retrieve a node parent without fiddling about with passing
> a NULL pointer for the id_out to achieve the same.

Since you commented "[PATCH v6 05/14] ACPI: platform-msi: retrieve dev
id from IORT" which also refer to this API, I will reply in that
email.

Thanks
Hanjun

Re: [PATCH v6 03/14] ACPI: ARM64: IORT: add missing comment for iort_dev_find_its_id()

[Lorenzo Pieralisi]

On Thu, Jan 05, 2017 at 02:05:06PM +0800, Hanjun Guo wrote:
> On 2017/1/4 22:34, Lorenzo Pieralisi wrote:
> >On Mon, Jan 02, 2017 at 09:31:34PM +0800, Hanjun Guo wrote:
> >>We are missing req_id's comment for iort_dev_find_its_id(),
> >>add it back.
> >
> >"Add missing req_id parameter to the iort_dev_find_its_id() function
> >kernel-doc comment."
> >
> >>Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> >>Tested-by: Majun <majun258@huawei.com>
> >>Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> >>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 46e2d82..174e983 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
> >
> >s/Requster/Requester
> >
> >We can send it upstream independently along with some other patches
> >in this series but I will have a look at the whole series first.
> 
> Do you mean go to 4.10-rcx?

Yes, technically it is a fix, not urgent at all though.

Lorenzo

Re: [PATCH v6 05/14] ACPI: platform-msi: retrieve dev id from IORT

[Hanjun Guo]

Hi Lorenzo,

On 2017/1/5 3:18, Lorenzo Pieralisi wrote:
> On Mon, Jan 02, 2017 at 09:31:36PM +0800, Hanjun Guo wrote:
>> 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>
>> Tested-by: Majun <majun258@huawei.com>
>> Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
>> 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 174e983..ab7bae7 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))
>
> I disagree with this approach. For named components we know that
> there are always two steps involved (second optional):
>
> (1) Retrieve the initial id (this may well provide the final mapping)
> (2) Map the id (optional if (1) represents the map type we need)
>
> That's the reason why I kept iort_node_get_id() and iort_node_map_rid()
> separated.
>
> Now, what we can do is to create an iort_node_map_id() function that is
> PCI agnostic (ie rename rid to id :)), whose rid_in is either a PCI RID
> or the outcome of a previous call to iort_node_get_id() for named
> components, that's in my opinion cleaner.

iort_node_map_rid() was designed for that purpose, and we can use it
for platform device, the issue that we need to pass a req id
unconditionally which is not needed for platform device, Tomasz
proposed a similar solution to rework iort_node_map_rid(), and
I think it makes sense.

>
> It would be even cleaner if you passed a type_mask (or write a
> wrapper function for that) that is:
>
> (IORT_MSI_TYPE | IORT_IOMMU_TYPE)

Sorry, I got little lost here, could you explain it in detail?

>
> and we just use the returned parent pointer to check if the mapping
> providing the initial id correspond to the type we are looking for (eg
> ITS) or we need to map the retrieved initial id any further, with
> iort_node_map_id(), to get to the final identifier.
>
> Thoughts ?

I think rework iort_node_map_rid() and not extend iort_node_get_id()
is the right direction, could you explain a bit more then I can demo
the code?

Thanks
Hanjun

Re: [PATCH v6 05/14] ACPI: platform-msi: retrieve dev id from IORT

[Lorenzo Pieralisi]

On Thu, Jan 05, 2017 at 08:45:37PM +0800, Hanjun Guo wrote:
> Hi Lorenzo,
> 
> On 2017/1/5 3:18, Lorenzo Pieralisi wrote:
> >On Mon, Jan 02, 2017 at 09:31:36PM +0800, Hanjun Guo wrote:
> >>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>
> >>Tested-by: Majun <majun258@huawei.com>
> >>Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> >>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 174e983..ab7bae7 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))
> >
> >I disagree with this approach. For named components we know that
> >there are always two steps involved (second optional):
> >
> >(1) Retrieve the initial id (this may well provide the final mapping)
> >(2) Map the id (optional if (1) represents the map type we need)
> >
> >That's the reason why I kept iort_node_get_id() and iort_node_map_rid()
> >separated.
> >
> >Now, what we can do is to create an iort_node_map_id() function that is
> >PCI agnostic (ie rename rid to id :)), whose rid_in is either a PCI RID
> >or the outcome of a previous call to iort_node_get_id() for named
> >components, that's in my opinion cleaner.
> 
> iort_node_map_rid() was designed for that purpose, and we can use it
> for platform device, the issue that we need to pass a req id
> unconditionally which is not needed for platform device, Tomasz
> proposed a similar solution to rework iort_node_map_rid(), and
> I think it makes sense.
> 
> >
> >It would be even cleaner if you passed a type_mask (or write a
> >wrapper function for that) that is:
> >
> >(IORT_MSI_TYPE | IORT_IOMMU_TYPE)
> 
> Sorry, I got little lost here, could you explain it in detail?

Yes sorry I was not clear. What I wanted to say is, for named
components, that do not have an intrinsic id, we have to call
iort_node_get_id() regardless of the type mask, we have to have
a way to get the "source/initial id", so basically the type_mask
is not important at all, it becomes important when it comes to
understanding what type of id the value returned from
iort_node_get_id() is.

So basically, passing:

#define IORT_TYPE_ANY (IORT_MSI_TYPE | IORT_IOMMU_TYPE)

as type_mask to iort_node_get_id() means "retrieve any kind of
initial id", that's what I wanted to say.

In iort_iommu_configure() iort_node_get_id() is a bit different because
we want only a type of id, ie a streamid, therefore the mask that we
pass in is IORT_IOMMU_TYPE.

> >and we just use the returned parent pointer to check if the mapping
> >providing the initial id correspond to the type we are looking for (eg
> >ITS) or we need to map the retrieved initial id any further, with
> >iort_node_map_id(), to get to the final identifier.
> >
> >Thoughts ?
> 
> I think rework iort_node_map_rid() and not extend iort_node_get_id()
> is the right direction, could you explain a bit more then I can demo
> the code?

What you can do is create a wrapper, say iort_node_map_platform_id()
(whose signature is equivalent to iort_node_map_rid() minus rid_in)
that carries out the two steps outlined above.

To do that I suggest the following:

(1) I send a patch to "fix" iort_node_get_id() (ie index issue you
    reported)
(2) We remove type_mask handling from iort_node_get_id()
(3) We create iort_node_map_platform_id() that (pseudo-code, I can
    write the patch if it is clearer):

struct acpi_iort_node *iort_node_map_platform_id(u8 type_mask, int index,
						 ...)
{
	u32 id, id_out;
	struct acpi_iort_node *parent = iort_node_get_id(&id, index);

	if (!parent)
		return NULL;

	/* we should probably rename iort_node_map_rid() too */
	if (!(IORT_TYPE_MASK(parent->type) & type_mask)
		parent = iort_node_map_rid(parent, id, &id_out, type_mask);

	return parent;
}

(4) we update current iort_node_get_id() users and move them over
    to iort_node_map_platform_id()

Let me know if that's clear so that we can agree on a way forward.

Thanks,
Lorenzo

Re: [PATCH v6 05/14] ACPI: platform-msi: retrieve dev id from IORT

[Hanjun Guo]

Hi Lorenzo,

On 2017/1/5 23:15, Lorenzo Pieralisi wrote:
> On Thu, Jan 05, 2017 at 08:45:37PM +0800, Hanjun Guo wrote:
>> Hi Lorenzo,
>>
>> On 2017/1/5 3:18, Lorenzo Pieralisi wrote:
>>> On Mon, Jan 02, 2017 at 09:31:36PM +0800, Hanjun Guo wrote:
>>>> 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>
>>>> Tested-by: Majun <majun258@huawei.com>
>>>> Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
>>>> 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 174e983..ab7bae7 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))
>>>
>>> I disagree with this approach. For named components we know that
>>> there are always two steps involved (second optional):
>>>
>>> (1) Retrieve the initial id (this may well provide the final mapping)
>>> (2) Map the id (optional if (1) represents the map type we need)
>>>
>>> That's the reason why I kept iort_node_get_id() and iort_node_map_rid()
>>> separated.
>>>
>>> Now, what we can do is to create an iort_node_map_id() function that is
>>> PCI agnostic (ie rename rid to id :)), whose rid_in is either a PCI RID
>>> or the outcome of a previous call to iort_node_get_id() for named
>>> components, that's in my opinion cleaner.
>>
>> iort_node_map_rid() was designed for that purpose, and we can use it
>> for platform device, the issue that we need to pass a req id
>> unconditionally which is not needed for platform device, Tomasz
>> proposed a similar solution to rework iort_node_map_rid(), and
>> I think it makes sense.
>>
>>>
>>> It would be even cleaner if you passed a type_mask (or write a
>>> wrapper function for that) that is:
>>>
>>> (IORT_MSI_TYPE | IORT_IOMMU_TYPE)
>>
>> Sorry, I got little lost here, could you explain it in detail?
>
> Yes sorry I was not clear. What I wanted to say is, for named
> components, that do not have an intrinsic id, we have to call
> iort_node_get_id() regardless of the type mask, we have to have
> a way to get the "source/initial id", so basically the type_mask
> is not important at all, it becomes important when it comes to
> understanding what type of id the value returned from
> iort_node_get_id() is.
>
> So basically, passing:
>
> #define IORT_TYPE_ANY (IORT_MSI_TYPE | IORT_IOMMU_TYPE)
>
> as type_mask to iort_node_get_id() means "retrieve any kind of
> initial id", that's what I wanted to say.

Thanks for the clarify, I'm working on this to demo the code as you
suggested.

>
> In iort_iommu_configure() iort_node_get_id() is a bit different because
> we want only a type of id, ie a streamid, therefore the mask that we
> pass in is IORT_IOMMU_TYPE.
>
>>> and we just use the returned parent pointer to check if the mapping
>>> providing the initial id correspond to the type we are looking for (eg
>>> ITS) or we need to map the retrieved initial id any further, with
>>> iort_node_map_id(), to get to the final identifier.
>>>
>>> Thoughts ?
>>
>> I think rework iort_node_map_rid() and not extend iort_node_get_id()
>> is the right direction, could you explain a bit more then I can demo
>> the code?
>
> What you can do is create a wrapper, say iort_node_map_platform_id()
> (whose signature is equivalent to iort_node_map_rid() minus rid_in)
> that carries out the two steps outlined above.
>
> To do that I suggest the following:
>
> (1) I send a patch to "fix" iort_node_get_id() (ie index issue you
>     reported)

I prepared two simple patches, one is for fix the indentation and
the other is adding the missing kernel-doc comment, how about
sending the out for 4.10-rcx?

> (2) We remove type_mask handling from iort_node_get_id()

iort_node_get_id() for now only supports id single mappings,
Do we need to extend it for multi id mappings? seems Sinan's
platform have such cases.

> (3) We create iort_node_map_platform_id() that (pseudo-code, I can
>     write the patch if it is clearer):
>
> struct acpi_iort_node *iort_node_map_platform_id(u8 type_mask, int index,
> 						 ...)
> {
> 	u32 id, id_out;
> 	struct acpi_iort_node *parent = iort_node_get_id(&id, index);
>
> 	if (!parent)
> 		return NULL;
>
> 	/* we should probably rename iort_node_map_rid() too */
> 	if (!(IORT_TYPE_MASK(parent->type) & type_mask)
> 		parent = iort_node_map_rid(parent, id, &id_out, type_mask);
>
> 	return parent;
> }
>
> (4) we update current iort_node_get_id() users and move them over
>     to iort_node_map_platform_id()

I think we need to prepare one patch for the above steps, or it
have functional changes for iort_node_get_id(), for example we
removed the type_mask handling from iort_node_get_id() and it
will break the case for SMMU if we only have requester id entries.

>
> Let me know if that's clear so that we can agree on a way forward.

Much clearer, the direction is clear and we need to discuss the details.

Thanks
Hanjun

Re: [PATCH v6 05/14] ACPI: platform-msi: retrieve dev id from IORT

[Lorenzo Pieralisi]

On Tue, Jan 10, 2017 at 09:39:39PM +0800, Hanjun Guo wrote:

[...]

> >What you can do is create a wrapper, say iort_node_map_platform_id()
> >(whose signature is equivalent to iort_node_map_rid() minus rid_in)
> >that carries out the two steps outlined above.
> >
> >To do that I suggest the following:
> >
> >(1) I send a patch to "fix" iort_node_get_id() (ie index issue you
> >    reported)
> 
> I prepared two simple patches, one is for fix the indentation and
> the other is adding the missing kernel-doc comment, how about
> sending the out for 4.10-rcx?

For me it is fine depending on how Rafael wants to handle them,
ie if he can batch those with the eg iort_node_get_id() fix I have
just sent:

https://patchwork.kernel.org/patch/9507041/

> >(2) We remove type_mask handling from iort_node_get_id()
> 
> iort_node_get_id() for now only supports id single mappings,
> Do we need to extend it for multi id mappings? seems Sinan's
> platform have such cases.

I am not really sure I understand what you mean here.

> >(3) We create iort_node_map_platform_id() that (pseudo-code, I can
> >    write the patch if it is clearer):
> >
> >struct acpi_iort_node *iort_node_map_platform_id(u8 type_mask, int index,
> >						 ...)
> >{
> >	u32 id, id_out;
> >	struct acpi_iort_node *parent = iort_node_get_id(&id, index);
> >
> >	if (!parent)
> >		return NULL;
> >
> >	/* we should probably rename iort_node_map_rid() too */
> >	if (!(IORT_TYPE_MASK(parent->type) & type_mask)
> >		parent = iort_node_map_rid(parent, id, &id_out, type_mask);
> >
> >	return parent;
> >}
> >
> >(4) we update current iort_node_get_id() users and move them over
> >    to iort_node_map_platform_id()
> 
> I think we need to prepare one patch for the above steps, or it
> have functional changes for iort_node_get_id(), for example we
> removed the type_mask handling from iort_node_get_id() and it
> will break the case for SMMU if we only have requester id entries.

If the question is "should we apply this change as a single logical
patch" the answer is yes, it looks a simple one to me (basically
it implies writing the function above and update the iort_node_get_id()
existing callers with it). Does this answer your question ?

Thanks !
Lorenzo

Re: [PATCH v6 02/14] irqchip: gic-v3-its: keep the head file include in alphabetic order

[Matthias Brugger]

On 02/01/17 14:31, Hanjun Guo wrote:
> 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.
>

Sounds strange, maybe someting like:
Rearrange header file includes to alphabetic order. As acpi_iort.h...

Regards,
Matthias

> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> Tested-by: Majun <majun258@huawei.com>
> Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> 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 69b040f..f471939 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>
>

Re: [PATCH v6 02/14] irqchip: gic-v3-its: keep the head file include in alphabetic order

[Matthias Brugger]

On 02/01/17 14:31, Hanjun Guo wrote:
> 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.
>

Sounds strange, maybe someting like:
Rearrange header file includes to alphabetic order. As acpi_iort.h...

Regards,
Matthias

> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> Tested-by: Majun <majun258@huawei.com>
> Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> 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 69b040f..f471939 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>
>

Re: [PATCH v6 04/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_prepare()

[Matthias Brugger]

On 02/01/17 14:31, Hanjun Guo wrote:
> 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>
> Tested-by: Majun <majun258@huawei.com>
> Tested-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> 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(-)
>

Reviewed-by: Matthias Brugger <mbrugger@suse.com>

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

Re: [PATCH v6 05/14] ACPI: platform-msi: retrieve dev id from IORT

[Hanjun Guo]

On 01/10/2017 10:57 PM, Lorenzo Pieralisi wrote:
> On Tue, Jan 10, 2017 at 09:39:39PM +0800, Hanjun Guo wrote:
>
> [...]
>
>>> What you can do is create a wrapper, say iort_node_map_platform_id()
>>> (whose signature is equivalent to iort_node_map_rid() minus rid_in)
>>> that carries out the two steps outlined above.
>>>
>>> To do that I suggest the following:
>>>
>>> (1) I send a patch to "fix" iort_node_get_id() (ie index issue you
>>>     reported)
>>
>> I prepared two simple patches, one is for fix the indentation and
>> the other is adding the missing kernel-doc comment, how about
>> sending the out for 4.10-rcx?
>
> For me it is fine depending on how Rafael wants to handle them,
> ie if he can batch those with the eg iort_node_get_id() fix I have
> just sent:
>
> https://patchwork.kernel.org/patch/9507041/
>
>>> (2) We remove type_mask handling from iort_node_get_id()
>>
>> iort_node_get_id() for now only supports id single mappings,
>> Do we need to extend it for multi id mappings? seems Sinan's
>> platform have such cases.
>
> I am not really sure I understand what you mean here.

Sorry for not clear, I was thinking if we want to support
ID mapping entries with multi IDs like BDFs for RC,

>
>>> (3) We create iort_node_map_platform_id() that (pseudo-code, I can
>>>     write the patch if it is clearer):
>>>
>>> struct acpi_iort_node *iort_node_map_platform_id(u8 type_mask, int index,
>>> 						 ...)
>>> {
>>> 	u32 id, id_out;
>>> 	struct acpi_iort_node *parent = iort_node_get_id(&id, index);
>>>
>>> 	if (!parent)
>>> 		return NULL;
>>>
>>> 	/* we should probably rename iort_node_map_rid() too */
>>> 	if (!(IORT_TYPE_MASK(parent->type) & type_mask)
>>> 		parent = iort_node_map_rid(parent, id, &id_out, type_mask);
>>>
>>> 	return parent;
>>> }
>>>
>>> (4) we update current iort_node_get_id() users and move them over
>>>     to iort_node_map_platform_id()
>>
>> I think we need to prepare one patch for the above steps, or it
>> have functional changes for iort_node_get_id(), for example we
>> removed the type_mask handling from iort_node_get_id() and it
>> will break the case for SMMU if we only have requester id entries.
>
> If the question is "should we apply this change as a single logical
> patch" the answer is yes, it looks a simple one to me (basically
> it implies writing the function above and update the iort_node_get_id()
> existing callers with it). Does this answer your question ?

Yes, thank you for your patience :)

When I was preparing patches, I split them into three patches, hope it
makes the review easier, will send out the patch set soon.

Thanks
Hanjun

Re: [PATCH v6 02/14] irqchip: gic-v3-its: keep the head file include in alphabetic order

[Hanjun Guo]

On 01/11/2017 06:20 PM, Matthias Brugger wrote:
>
>
> On 02/01/17 14:31, Hanjun Guo wrote:
>> 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.
>>
>
> Sounds strange, maybe someting like:
> Rearrange header file includes to alphabetic order. As acpi_iort.h...

It's better, will update the patch.

Thanks
Hanjun