Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v9 02/16] drivers: acpi: iort: introduce linker section for IORT entries probing
From: Lorenzo Pieralisi @ 2016-11-21 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121100148.24769-1-lorenzo.pieralisi@arm.com>

Since commit e647b532275b ("ACPI: Add early device probing
infrastructure") the kernel has gained the infrastructure that allows
adding linker script section entries to execute ACPI driver callbacks
(ie probe routines) for all subsystems that register a table entry
in the respective kernel section (eg clocksource, irqchip).

Since ARM IOMMU devices data is described through IORT tables when
booting with ACPI, the ARM IOMMU drivers must be made able to hook ACPI
callback routines that are called to probe IORT entries and initialize
the respective IOMMU devices.

To avoid adding driver specific hooks into IORT table initialization
code (breaking therefore code modularity - ie ACPI IORT code must be made
aware of ARM SMMU drivers ACPI init callbacks), this patch adds code
that allows ARM SMMU drivers to take advantage of the ACPI early probing
infrastructure, so that they can add linker script section entries
containing drivers callback to be executed on IORT tables detection.

Since IORT nodes are differentiated by a type, the callback routines
can easily parse the IORT table entries, check the IORT nodes and
carry out some actions whenever the IORT node type associated with
the driver specific callback is matched.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
Reviewed-by: Tomasz Nowicki <tn@semihalf.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Tomasz Nowicki <tn@semihalf.com>
Cc: Tomasz Nowicki <tn@semihalf.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/acpi/arm64/iort.c         | 13 ++++++++++---
 include/asm-generic/vmlinux.lds.h |  1 +
 include/linux/acpi_iort.h         |  3 +++
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 6b81746..2c46ebc 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -361,8 +361,15 @@ void __init acpi_iort_init(void)
 	acpi_status status;
 
 	status = acpi_get_table(ACPI_SIG_IORT, 0, &iort_table);
-	if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
-		const char *msg = acpi_format_exception(status);
-		pr_err("Failed to get table, %s\n", msg);
+	if (ACPI_FAILURE(status)) {
+		if (status != AE_NOT_FOUND) {
+			const char *msg = acpi_format_exception(status);
+
+			pr_err("Failed to get table, %s\n", msg);
+		}
+
+		return;
 	}
+
+	acpi_probe_device_table(iort);
 }
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 31e1d63..9e3aa34 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -566,6 +566,7 @@
 	IRQCHIP_OF_MATCH_TABLE()					\
 	ACPI_PROBE_TABLE(irqchip)					\
 	ACPI_PROBE_TABLE(clksrc)					\
+	ACPI_PROBE_TABLE(iort)						\
 	EARLYCON_TABLE()
 
 #define INIT_TEXT							\
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index 0e32dac..d16fdda 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -39,4 +39,7 @@ static inline struct irq_domain *iort_get_device_domain(struct device *dev,
 { return NULL; }
 #endif
 
+#define IORT_ACPI_DECLARE(name, table_id, fn)		\
+	ACPI_DECLARE_PROBE_ENTRY(iort, name, table_id, 0, NULL, 0, fn)
+
 #endif /* __ACPI_IORT_H__ */
-- 
2.10.0

^ permalink raw reply related

* [PATCH v9 03/16] drivers: acpi: iort: add support for IOMMU fwnode registration
From: Lorenzo Pieralisi @ 2016-11-21 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121100148.24769-1-lorenzo.pieralisi@arm.com>

The ACPI IORT table provide entries for IOMMU (aka SMMU in ARM world)
components that allow creating the kernel data structures required to
probe and initialize the IOMMU devices.

This patch provides support in the IORT kernel code to register IOMMU
components and their respective fwnode.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
Reviewed-by: Tomasz Nowicki <tn@semihalf.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Tomasz Nowicki <tn@semihalf.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Tomasz Nowicki <tn@semihalf.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
---
 drivers/acpi/arm64/iort.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 2c46ebc..1ac2720 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -20,7 +20,9 @@
 
 #include <linux/acpi_iort.h>
 #include <linux/kernel.h>
+#include <linux/list.h>
 #include <linux/pci.h>
+#include <linux/slab.h>
 
 struct iort_its_msi_chip {
 	struct list_head	list;
@@ -28,6 +30,90 @@ struct iort_its_msi_chip {
 	u32			translation_id;
 };
 
+struct iort_fwnode {
+	struct list_head list;
+	struct acpi_iort_node *iort_node;
+	struct fwnode_handle *fwnode;
+};
+static LIST_HEAD(iort_fwnode_list);
+static DEFINE_SPINLOCK(iort_fwnode_lock);
+
+/**
+ * iort_set_fwnode() - Create iort_fwnode and use it to register
+ *		       iommu data in the iort_fwnode_list
+ *
+ * @node: IORT table node associated with the IOMMU
+ * @fwnode: fwnode associated with the IORT node
+ *
+ * Returns: 0 on success
+ *          <0 on failure
+ */
+static inline int iort_set_fwnode(struct acpi_iort_node *iort_node,
+				  struct fwnode_handle *fwnode)
+{
+	struct iort_fwnode *np;
+
+	np = kzalloc(sizeof(struct iort_fwnode), GFP_ATOMIC);
+
+	if (WARN_ON(!np))
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&np->list);
+	np->iort_node = iort_node;
+	np->fwnode = fwnode;
+
+	spin_lock(&iort_fwnode_lock);
+	list_add_tail(&np->list, &iort_fwnode_list);
+	spin_unlock(&iort_fwnode_lock);
+
+	return 0;
+}
+
+/**
+ * iort_get_fwnode() - Retrieve fwnode associated with an IORT node
+ *
+ * @node: IORT table node to be looked-up
+ *
+ * Returns: fwnode_handle pointer on success, NULL on failure
+ */
+static inline
+struct fwnode_handle *iort_get_fwnode(struct acpi_iort_node *node)
+{
+	struct iort_fwnode *curr;
+	struct fwnode_handle *fwnode = NULL;
+
+	spin_lock(&iort_fwnode_lock);
+	list_for_each_entry(curr, &iort_fwnode_list, list) {
+		if (curr->iort_node == node) {
+			fwnode = curr->fwnode;
+			break;
+		}
+	}
+	spin_unlock(&iort_fwnode_lock);
+
+	return fwnode;
+}
+
+/**
+ * iort_delete_fwnode() - Delete fwnode associated with an IORT node
+ *
+ * @node: IORT table node associated with fwnode to delete
+ */
+static inline void iort_delete_fwnode(struct acpi_iort_node *node)
+{
+	struct iort_fwnode *curr, *tmp;
+
+	spin_lock(&iort_fwnode_lock);
+	list_for_each_entry_safe(curr, tmp, &iort_fwnode_list, list) {
+		if (curr->iort_node == node) {
+			list_del(&curr->list);
+			kfree(curr);
+			break;
+		}
+	}
+	spin_unlock(&iort_fwnode_lock);
+}
+
 typedef acpi_status (*iort_find_node_callback)
 	(struct acpi_iort_node *node, void *context);
 
-- 
2.10.0

^ permalink raw reply related

* [PATCH v9 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic
From: Lorenzo Pieralisi @ 2016-11-21 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121100148.24769-1-lorenzo.pieralisi@arm.com>

The of_iommu_{set/get}_ops() API is used to associate a device
tree node with a specific set of IOMMU operations. The same
kernel interface is required on systems booting with ACPI, where
devices are not associated with a device tree node, therefore
the interface requires generalization.

The struct device fwnode member represents the fwnode token associated
with the device and the struct it points at is firmware specific;
regardless, it is initialized on both ACPI and DT systems and makes an
ideal candidate to use it to associate a set of IOMMU operations to a
given device, through its struct device.fwnode member pointer, paving
the way for representing per-device iommu_ops (ie an iommu instance
associated with a device).

Convert the DT specific of_iommu_{set/get}_ops() interface to
use struct device.fwnode as a look-up token, making the interface
usable on ACPI systems and rename the data structures and the
registration API so that they are made to represent their usage
more clearly.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Reviewed-by: Tomasz Nowicki <tn@semihalf.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Tomasz Nowicki <tn@semihalf.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Joerg Roedel <joro@8bytes.org>
---
 drivers/iommu/iommu.c    | 40 ++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/of_iommu.c | 39 ---------------------------------------
 include/linux/iommu.h    | 14 ++++++++++++++
 include/linux/of_iommu.h | 12 ++++++++++--
 4 files changed, 64 insertions(+), 41 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 9a2f196..8d3e847 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1615,6 +1615,46 @@ int iommu_request_dm_for_dev(struct device *dev)
 	return ret;
 }
 
+struct iommu_instance {
+	struct list_head list;
+	struct fwnode_handle *fwnode;
+	const struct iommu_ops *ops;
+};
+static LIST_HEAD(iommu_instance_list);
+static DEFINE_SPINLOCK(iommu_instance_lock);
+
+void iommu_register_instance(struct fwnode_handle *fwnode,
+			     const struct iommu_ops *ops)
+{
+	struct iommu_instance *iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
+
+	if (WARN_ON(!iommu))
+		return;
+
+	of_node_get(to_of_node(fwnode));
+	INIT_LIST_HEAD(&iommu->list);
+	iommu->fwnode = fwnode;
+	iommu->ops = ops;
+	spin_lock(&iommu_instance_lock);
+	list_add_tail(&iommu->list, &iommu_instance_list);
+	spin_unlock(&iommu_instance_lock);
+}
+
+const struct iommu_ops *iommu_get_instance(struct fwnode_handle *fwnode)
+{
+	struct iommu_instance *instance;
+	const struct iommu_ops *ops = NULL;
+
+	spin_lock(&iommu_instance_lock);
+	list_for_each_entry(instance, &iommu_instance_list, list)
+		if (instance->fwnode == fwnode) {
+			ops = instance->ops;
+			break;
+		}
+	spin_unlock(&iommu_instance_lock);
+	return ops;
+}
+
 int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
 		      const struct iommu_ops *ops)
 {
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 5b82862..0f57ddc 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -96,45 +96,6 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
 }
 EXPORT_SYMBOL_GPL(of_get_dma_window);
 
-struct of_iommu_node {
-	struct list_head list;
-	struct device_node *np;
-	const struct iommu_ops *ops;
-};
-static LIST_HEAD(of_iommu_list);
-static DEFINE_SPINLOCK(of_iommu_lock);
-
-void of_iommu_set_ops(struct device_node *np, const struct iommu_ops *ops)
-{
-	struct of_iommu_node *iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
-
-	if (WARN_ON(!iommu))
-		return;
-
-	of_node_get(np);
-	INIT_LIST_HEAD(&iommu->list);
-	iommu->np = np;
-	iommu->ops = ops;
-	spin_lock(&of_iommu_lock);
-	list_add_tail(&iommu->list, &of_iommu_list);
-	spin_unlock(&of_iommu_lock);
-}
-
-const struct iommu_ops *of_iommu_get_ops(struct device_node *np)
-{
-	struct of_iommu_node *node;
-	const struct iommu_ops *ops = NULL;
-
-	spin_lock(&of_iommu_lock);
-	list_for_each_entry(node, &of_iommu_list, list)
-		if (node->np == np) {
-			ops = node->ops;
-			break;
-		}
-	spin_unlock(&of_iommu_lock);
-	return ops;
-}
-
 static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
 {
 	struct of_phandle_args *iommu_spec = data;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 436dc21..f2960e4 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -351,6 +351,9 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
 		      const struct iommu_ops *ops);
 void iommu_fwspec_free(struct device *dev);
 int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids);
+void iommu_register_instance(struct fwnode_handle *fwnode,
+			     const struct iommu_ops *ops);
+const struct iommu_ops *iommu_get_instance(struct fwnode_handle *fwnode);
 
 #else /* CONFIG_IOMMU_API */
 
@@ -580,6 +583,17 @@ static inline int iommu_fwspec_add_ids(struct device *dev, u32 *ids,
 	return -ENODEV;
 }
 
+static inline void iommu_register_instance(struct fwnode_handle *fwnode,
+					   const struct iommu_ops *ops)
+{
+}
+
+static inline
+const struct iommu_ops *iommu_get_instance(struct fwnode_handle *fwnode)
+{
+	return NULL;
+}
+
 #endif /* CONFIG_IOMMU_API */
 
 #endif /* __LINUX_IOMMU_H */
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index e80b9c7..6a7fc50 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -31,8 +31,16 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
 
 #endif	/* CONFIG_OF_IOMMU */
 
-void of_iommu_set_ops(struct device_node *np, const struct iommu_ops *ops);
-const struct iommu_ops *of_iommu_get_ops(struct device_node *np);
+static inline void of_iommu_set_ops(struct device_node *np,
+				    const struct iommu_ops *ops)
+{
+	iommu_register_instance(&np->fwnode, ops);
+}
+
+static inline const struct iommu_ops *of_iommu_get_ops(struct device_node *np)
+{
+	return iommu_get_instance(&np->fwnode);
+}
 
 extern struct of_device_id __iommu_of_table;
 
-- 
2.10.0

^ permalink raw reply related

* [PATCH v9 05/16] drivers: iommu: arm-smmu: convert struct device of_node to fwnode usage
From: Lorenzo Pieralisi @ 2016-11-21 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121100148.24769-1-lorenzo.pieralisi@arm.com>

Current ARM SMMU driver rely on the struct device.of_node pointer for
device look-up and iommu_ops retrieval.

In preparation for ACPI probing enablement, convert the driver to use
the struct device.fwnode member for device and iommu_ops look-up so that
the driver infrastructure can be used also on systems that do not
associate an of_node pointer to a struct device (eg ACPI), making the
device look-up and iommu_ops retrieval firmware agnostic.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Reviewed-by: Tomasz Nowicki <tn@semihalf.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Tomasz Nowicki <tn@semihalf.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/arm-smmu.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 8f72814..339a8d3 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1379,13 +1379,14 @@ static bool arm_smmu_capable(enum iommu_cap cap)
 
 static int arm_smmu_match_node(struct device *dev, void *data)
 {
-	return dev->of_node == data;
+	return dev->fwnode == data;
 }
 
-static struct arm_smmu_device *arm_smmu_get_by_node(struct device_node *np)
+static
+struct arm_smmu_device *arm_smmu_get_by_fwnode(struct fwnode_handle *fwnode)
 {
 	struct device *dev = driver_find_device(&arm_smmu_driver.driver, NULL,
-						np, arm_smmu_match_node);
+						fwnode, arm_smmu_match_node);
 	put_device(dev);
 	return dev ? dev_get_drvdata(dev) : NULL;
 }
@@ -1403,7 +1404,7 @@ static int arm_smmu_add_device(struct device *dev)
 		if (ret)
 			goto out_free;
 	} else if (fwspec && fwspec->ops == &arm_smmu_ops) {
-		smmu = arm_smmu_get_by_node(to_of_node(fwspec->iommu_fwnode));
+		smmu = arm_smmu_get_by_fwnode(fwspec->iommu_fwnode);
 	} else {
 		return -ENODEV;
 	}
@@ -2007,7 +2008,7 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 		}
 	}
 
-	of_iommu_set_ops(dev->of_node, &arm_smmu_ops);
+	iommu_register_instance(dev->fwnode, &arm_smmu_ops);
 	platform_set_drvdata(pdev, smmu);
 	arm_smmu_device_reset(smmu);
 
-- 
2.10.0

^ permalink raw reply related

* [PATCH v9 06/16] drivers: iommu: arm-smmu-v3: convert struct device of_node to fwnode usage
From: Lorenzo Pieralisi @ 2016-11-21 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121100148.24769-1-lorenzo.pieralisi@arm.com>

Current ARM SMMU v3 driver rely on the struct device.of_node pointer for
device look-up and iommu_ops retrieval.

In preparation for ACPI probing enablement, convert the driver to use
the struct device.fwnode member for device and iommu_ops look-up so that
the driver infrastructure can be used also on systems that do not
associate an of_node pointer to a struct device (eg ACPI), making the
device look-up and iommu_ops retrieval firmware agnostic.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Reviewed-by: Tomasz Nowicki <tn@semihalf.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Tomasz Nowicki <tn@semihalf.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/arm-smmu-v3.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index e6f9b2d..e6e1c87 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1723,13 +1723,14 @@ static struct platform_driver arm_smmu_driver;
 
 static int arm_smmu_match_node(struct device *dev, void *data)
 {
-	return dev->of_node == data;
+	return dev->fwnode == data;
 }
 
-static struct arm_smmu_device *arm_smmu_get_by_node(struct device_node *np)
+static
+struct arm_smmu_device *arm_smmu_get_by_fwnode(struct fwnode_handle *fwnode)
 {
 	struct device *dev = driver_find_device(&arm_smmu_driver.driver, NULL,
-						np, arm_smmu_match_node);
+						fwnode, arm_smmu_match_node);
 	put_device(dev);
 	return dev ? dev_get_drvdata(dev) : NULL;
 }
@@ -1765,7 +1766,7 @@ static int arm_smmu_add_device(struct device *dev)
 		master = fwspec->iommu_priv;
 		smmu = master->smmu;
 	} else {
-		smmu = arm_smmu_get_by_node(to_of_node(fwspec->iommu_fwnode));
+		smmu = arm_smmu_get_by_fwnode(fwspec->iommu_fwnode);
 		if (!smmu)
 			return -ENODEV;
 		master = kzalloc(sizeof(*master), GFP_KERNEL);
@@ -2634,7 +2635,8 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 		return ret;
 
 	/* And we're up. Go go go! */
-	of_iommu_set_ops(dev->of_node, &arm_smmu_ops);
+	iommu_register_instance(dev->fwnode, &arm_smmu_ops);
+
 #ifdef CONFIG_PCI
 	if (pci_bus_type.iommu_ops != &arm_smmu_ops) {
 		pci_request_acs();
-- 
2.10.0

^ permalink raw reply related

* [PATCH v9 07/16] drivers: acpi: implement acpi_dma_configure
From: Lorenzo Pieralisi @ 2016-11-21 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121100148.24769-1-lorenzo.pieralisi@arm.com>

On DT based systems, the of_dma_configure() API implements DMA
configuration for a given device. On ACPI systems an API equivalent to
of_dma_configure() is missing which implies that it is currently not
possible to set-up DMA operations for devices through the ACPI generic
kernel layer.

This patch fills the gap by introducing acpi_dma_configure/deconfigure()
calls that for now are just wrappers around arch_setup_dma_ops() and
arch_teardown_dma_ops() and also updates ACPI and PCI core code to use
the newly introduced acpi_dma_configure/acpi_dma_deconfigure functions.

Since acpi_dma_configure() is used to configure DMA operations, the
function initializes the dma/coherent_dma masks to sane default values
if the current masks are uninitialized (also to keep the default values
consistent with DT systems) to make sure the device has a complete
default DMA set-up.

The DMA range size passed to arch_setup_dma_ops() is sized according
to the device coherent_dma_mask (starting at address 0x0), mirroring the
DT probing path behaviour when a dma-ranges property is not provided
for the device being probed; this changes the current arch_setup_dma_ops()
call parameters in the ACPI probing case, but since arch_setup_dma_ops()
is a NOP on all architectures but ARM/ARM64 this patch does not change
the current kernel behaviour on them.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com> [pci]
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Tomasz Nowicki <tn@semihalf.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Tomasz Nowicki <tn@semihalf.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Tomasz Nowicki <tn@semihalf.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
---
 drivers/acpi/glue.c     |  4 ++--
 drivers/acpi/scan.c     | 40 ++++++++++++++++++++++++++++++++++++++++
 drivers/pci/probe.c     |  3 +--
 include/acpi/acpi_bus.h |  2 ++
 include/linux/acpi.h    |  5 +++++
 5 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 5ea5dc2..f8d6564 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -227,8 +227,7 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
 
 	attr = acpi_get_dma_attr(acpi_dev);
 	if (attr != DEV_DMA_NOT_SUPPORTED)
-		arch_setup_dma_ops(dev, 0, 0, NULL,
-				   attr == DEV_DMA_COHERENT);
+		acpi_dma_configure(dev, attr);
 
 	acpi_physnode_link_name(physical_node_name, node_id);
 	retval = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
@@ -251,6 +250,7 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
 	return 0;
 
  err:
+	acpi_dma_deconfigure(dev);
 	ACPI_COMPANION_SET(dev, NULL);
 	put_device(dev);
 	put_device(&acpi_dev->dev);
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 3d1856f..45b5710 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1370,6 +1370,46 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
 		return DEV_DMA_NON_COHERENT;
 }
 
+/**
+ * acpi_dma_configure - Set-up DMA configuration for the device.
+ * @dev: The pointer to the device
+ * @attr: device dma attributes
+ */
+void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
+{
+	/*
+	 * Set default coherent_dma_mask to 32 bit.  Drivers are expected to
+	 * setup the correct supported mask.
+	 */
+	if (!dev->coherent_dma_mask)
+		dev->coherent_dma_mask = DMA_BIT_MASK(32);
+
+	/*
+	 * Set it to coherent_dma_mask by default if the architecture
+	 * code has not set it.
+	 */
+	if (!dev->dma_mask)
+		dev->dma_mask = &dev->coherent_dma_mask;
+
+	/*
+	 * Assume dma valid range starts at 0 and covers the whole
+	 * coherent_dma_mask.
+	 */
+	arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, NULL,
+			   attr == DEV_DMA_COHERENT);
+}
+EXPORT_SYMBOL_GPL(acpi_dma_configure);
+
+/**
+ * acpi_dma_deconfigure - Tear-down DMA configuration for the device.
+ * @dev: The pointer to the device
+ */
+void acpi_dma_deconfigure(struct device *dev)
+{
+	arch_teardown_dma_ops(dev);
+}
+EXPORT_SYMBOL_GPL(acpi_dma_deconfigure);
+
 static void acpi_init_coherency(struct acpi_device *adev)
 {
 	unsigned long long cca = 0;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ab00267..c29e07a 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1738,8 +1738,7 @@ static void pci_dma_configure(struct pci_dev *dev)
 		if (attr == DEV_DMA_NOT_SUPPORTED)
 			dev_warn(&dev->dev, "DMA not supported.\n");
 		else
-			arch_setup_dma_ops(&dev->dev, 0, 0, NULL,
-					   attr == DEV_DMA_COHERENT);
+			acpi_dma_configure(&dev->dev, attr);
 	}
 
 	pci_put_host_bridge_device(bridge);
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index c1a524d..4242c31 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -573,6 +573,8 @@ struct acpi_pci_root {
 
 bool acpi_dma_supported(struct acpi_device *adev);
 enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev);
+void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr);
+void acpi_dma_deconfigure(struct device *dev);
 
 struct acpi_device *acpi_find_child_device(struct acpi_device *parent,
 					   u64 address, bool check_children);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 996a29c..8d15fc5 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -765,6 +765,11 @@ static inline enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
 	return DEV_DMA_NOT_SUPPORTED;
 }
 
+static inline void acpi_dma_configure(struct device *dev,
+				      enum dev_dma_attr attr) { }
+
+static inline void acpi_dma_deconfigure(struct device *dev) { }
+
 #define ACPI_PTR(_ptr)	(NULL)
 
 static inline void acpi_device_set_enumerated(struct acpi_device *adev)
-- 
2.10.0

^ permalink raw reply related

* [PATCH v9 08/16] drivers: acpi: iort: add node match function
From: Lorenzo Pieralisi @ 2016-11-21 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121100148.24769-1-lorenzo.pieralisi@arm.com>

Device drivers (eg ARM SMMU) need to know if a specific component
is part of the IORT table, so that kernel data structures are not
initialized at initcalls time if the respective component is not
part of the IORT table.

To this end, this patch adds a trivial function that allows detecting
if a given IORT node type is present or not in the ACPI table, providing
an ACPI IORT equivalent for of_find_matching_node().

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Tomasz Nowicki <tn@semihalf.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Tomasz Nowicki <tn@semihalf.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Tomasz Nowicki <tn@semihalf.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
---
 drivers/acpi/arm64/iort.c | 15 +++++++++++++++
 include/linux/acpi_iort.h |  2 ++
 2 files changed, 17 insertions(+)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 1ac2720..4bb6acb 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -227,6 +227,21 @@ static struct acpi_iort_node *iort_scan_node(enum acpi_iort_node_type type,
 	return NULL;
 }
 
+static acpi_status
+iort_match_type_callback(struct acpi_iort_node *node, void *context)
+{
+	return AE_OK;
+}
+
+bool iort_node_match(u8 type)
+{
+	struct acpi_iort_node *node;
+
+	node = iort_scan_node(type, iort_match_type_callback, NULL);
+
+	return node != NULL;
+}
+
 static acpi_status iort_match_node_callback(struct acpi_iort_node *node,
 					    void *context)
 {
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index d16fdda..17bb078 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -28,10 +28,12 @@ void iort_deregister_domain_token(int trans_id);
 struct fwnode_handle *iort_find_domain_token(int trans_id);
 #ifdef CONFIG_ACPI_IORT
 void acpi_iort_init(void);
+bool iort_node_match(u8 type);
 u32 iort_msi_map_rid(struct device *dev, u32 req_id);
 struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id);
 #else
 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,
-- 
2.10.0

^ permalink raw reply related

* [PATCH v9 09/16] drivers: acpi: iort: add support for ARM SMMU platform devices creation
From: Lorenzo Pieralisi @ 2016-11-21 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121100148.24769-1-lorenzo.pieralisi@arm.com>

In ARM ACPI systems, IOMMU components are specified through static
IORT table entries. In order to create platform devices for the
corresponding ARM SMMU components, IORT kernel code should be made
able to parse IORT table entries and create platform devices
dynamically.

This patch adds the generic IORT infrastructure required to create
platform devices for ARM SMMUs.

ARM SMMU versions have different resources requirement therefore this
patch also introduces an IORT specific structure (ie iort_iommu_config)
that contains hooks (to be defined when the corresponding ARM SMMU
driver support is added to the kernel) to be used to define the
platform devices names, init the IOMMUs, count their resources and
finally initialize them.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Tomasz Nowicki <tn@semihalf.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Tomasz Nowicki <tn@semihalf.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Tomasz Nowicki <tn@semihalf.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
---
 drivers/acpi/arm64/iort.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 151 insertions(+)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 4bb6acb..ddf83b5 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -19,9 +19,11 @@
 #define pr_fmt(fmt)	"ACPI: IORT: " fmt
 
 #include <linux/acpi_iort.h>
+#include <linux/iommu.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/pci.h>
+#include <linux/platform_device.h>
 #include <linux/slab.h>
 
 struct iort_its_msi_chip {
@@ -457,6 +459,153 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id)
 	return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
 }
 
+struct iort_iommu_config {
+	const char *name;
+	int (*iommu_init)(struct acpi_iort_node *node);
+	bool (*iommu_is_coherent)(struct acpi_iort_node *node);
+	int (*iommu_count_resources)(struct acpi_iort_node *node);
+	void (*iommu_init_resources)(struct resource *res,
+				     struct acpi_iort_node *node);
+};
+
+static __init
+const struct iort_iommu_config *iort_get_iommu_cfg(struct acpi_iort_node *node)
+{
+	return NULL;
+}
+
+/**
+ * iort_add_smmu_platform_device() - Allocate a platform device for SMMU
+ * @node: Pointer to SMMU ACPI IORT node
+ *
+ * Returns: 0 on success, <0 failure
+ */
+static int __init iort_add_smmu_platform_device(struct acpi_iort_node *node)
+{
+	struct fwnode_handle *fwnode;
+	struct platform_device *pdev;
+	struct resource *r;
+	enum dev_dma_attr attr;
+	int ret, count;
+	const struct iort_iommu_config *ops = iort_get_iommu_cfg(node);
+
+	if (!ops)
+		return -ENODEV;
+
+	pdev = platform_device_alloc(ops->name, PLATFORM_DEVID_AUTO);
+	if (!pdev)
+		return PTR_ERR(pdev);
+
+	count = ops->iommu_count_resources(node);
+
+	r = kcalloc(count, sizeof(*r), GFP_KERNEL);
+	if (!r) {
+		ret = -ENOMEM;
+		goto dev_put;
+	}
+
+	ops->iommu_init_resources(r, node);
+
+	ret = platform_device_add_resources(pdev, r, count);
+	/*
+	 * Resources are duplicated in platform_device_add_resources,
+	 * free their allocated memory
+	 */
+	kfree(r);
+
+	if (ret)
+		goto dev_put;
+
+	/*
+	 * Add a copy of IORT node pointer to platform_data to
+	 * be used to retrieve IORT data information.
+	 */
+	ret = platform_device_add_data(pdev, &node, sizeof(node));
+	if (ret)
+		goto dev_put;
+
+	/*
+	 * We expect the dma masks to be equivalent for
+	 * all SMMUs set-ups
+	 */
+	pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
+
+	fwnode = iort_get_fwnode(node);
+
+	if (!fwnode) {
+		ret = -ENODEV;
+		goto dev_put;
+	}
+
+	pdev->dev.fwnode = fwnode;
+
+	attr = ops->iommu_is_coherent(node) ?
+			     DEV_DMA_COHERENT : DEV_DMA_NON_COHERENT;
+
+	/* Configure DMA for the page table walker */
+	acpi_dma_configure(&pdev->dev, attr);
+
+	ret = platform_device_add(pdev);
+	if (ret)
+		goto dma_deconfigure;
+
+	return 0;
+
+dma_deconfigure:
+	acpi_dma_deconfigure(&pdev->dev);
+dev_put:
+	platform_device_put(pdev);
+
+	return ret;
+}
+
+static void __init iort_init_platform_devices(void)
+{
+	struct acpi_iort_node *iort_node, *iort_end;
+	struct acpi_table_iort *iort;
+	struct fwnode_handle *fwnode;
+	int i, ret;
+
+	/*
+	 * iort_table and iort both point to the start of IORT table, but
+	 * have different struct types
+	 */
+	iort = (struct acpi_table_iort *)iort_table;
+
+	/* Get the first IORT node */
+	iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort,
+				 iort->node_offset);
+	iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort,
+				iort_table->length);
+
+	for (i = 0; i < iort->node_count; i++) {
+		if (iort_node >= iort_end) {
+			pr_err("iort node pointer overflows, bad table\n");
+			return;
+		}
+
+		if ((iort_node->type == ACPI_IORT_NODE_SMMU) ||
+			(iort_node->type == ACPI_IORT_NODE_SMMU_V3)) {
+
+			fwnode = acpi_alloc_fwnode_static();
+			if (!fwnode)
+				return;
+
+			iort_set_fwnode(iort_node, fwnode);
+
+			ret = iort_add_smmu_platform_device(iort_node);
+			if (ret) {
+				iort_delete_fwnode(iort_node);
+				acpi_free_fwnode_static(fwnode);
+				return;
+			}
+		}
+
+		iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
+					 iort_node->length);
+	}
+}
+
 void __init acpi_iort_init(void)
 {
 	acpi_status status;
@@ -472,5 +621,7 @@ void __init acpi_iort_init(void)
 		return;
 	}
 
+	iort_init_platform_devices();
+
 	acpi_probe_device_table(iort);
 }
-- 
2.10.0

^ permalink raw reply related

* [PATCH v9 10/16] drivers: iommu: arm-smmu-v3: split probe functions into DT/generic portions
From: Lorenzo Pieralisi @ 2016-11-21 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121100148.24769-1-lorenzo.pieralisi@arm.com>

Current ARM SMMUv3 probe functions intermingle HW and DT probing in the
initialization functions to detect and programme the ARM SMMU v3 driver
features. In order to allow probing the ARM SMMUv3 with other firmwares
than DT, this patch splits the ARM SMMUv3 init functions into DT and HW
specific portions so that other FW interfaces (ie ACPI) can reuse the HW
probing functions and skip the DT portion accordingly.

This patch implements no functional change, only code reshuffling.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Tomasz Nowicki <tn@semihalf.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Tomasz Nowicki <tn@semihalf.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Joerg Roedel <joro@8bytes.org>
---
 drivers/iommu/arm-smmu-v3.c | 43 +++++++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index e6e1c87..2b3f9ac 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2381,10 +2381,10 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)
 	return 0;
 }
 
-static int arm_smmu_device_probe(struct arm_smmu_device *smmu)
+static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
 {
 	u32 reg;
-	bool coherent;
+	bool coherent = smmu->features & ARM_SMMU_FEAT_COHERENCY;
 
 	/* IDR0 */
 	reg = readl_relaxed(smmu->base + ARM_SMMU_IDR0);
@@ -2436,13 +2436,9 @@ static int arm_smmu_device_probe(struct arm_smmu_device *smmu)
 		smmu->features |= ARM_SMMU_FEAT_HYP;
 
 	/*
-	 * The dma-coherent property is used in preference to the ID
+	 * The coherency feature as set by FW is used in preference to the ID
 	 * register, but warn on mismatch.
 	 */
-	coherent = of_dma_is_coherent(smmu->dev->of_node);
-	if (coherent)
-		smmu->features |= ARM_SMMU_FEAT_COHERENCY;
-
 	if (!!(reg & IDR0_COHACC) != coherent)
 		dev_warn(smmu->dev, "IDR0.COHACC overridden by dma-coherent property (%s)\n",
 			 coherent ? "true" : "false");
@@ -2563,21 +2559,35 @@ static int arm_smmu_device_probe(struct arm_smmu_device *smmu)
 	return 0;
 }
 
-static int arm_smmu_device_dt_probe(struct platform_device *pdev)
+static int arm_smmu_device_dt_probe(struct platform_device *pdev,
+				    struct arm_smmu_device *smmu)
 {
-	int irq, ret;
-	struct resource *res;
-	struct arm_smmu_device *smmu;
 	struct device *dev = &pdev->dev;
-	bool bypass = true;
 	u32 cells;
+	int ret = -EINVAL;
 
 	if (of_property_read_u32(dev->of_node, "#iommu-cells", &cells))
 		dev_err(dev, "missing #iommu-cells property\n");
 	else if (cells != 1)
 		dev_err(dev, "invalid #iommu-cells value (%d)\n", cells);
 	else
-		bypass = false;
+		ret = 0;
+
+	parse_driver_options(smmu);
+
+	if (of_dma_is_coherent(dev->of_node))
+		smmu->features |= ARM_SMMU_FEAT_COHERENCY;
+
+	return ret;
+}
+
+static int arm_smmu_device_probe(struct platform_device *pdev)
+{
+	int irq, ret;
+	struct resource *res;
+	struct arm_smmu_device *smmu;
+	struct device *dev = &pdev->dev;
+	bool bypass;
 
 	smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
 	if (!smmu) {
@@ -2614,10 +2624,11 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 	if (irq > 0)
 		smmu->gerr_irq = irq;
 
-	parse_driver_options(smmu);
+	/* Set bypass mode according to firmware probing result */
+	bypass = !!arm_smmu_device_dt_probe(pdev, smmu);
 
 	/* Probe the h/w */
-	ret = arm_smmu_device_probe(smmu);
+	ret = arm_smmu_device_hw_probe(smmu);
 	if (ret)
 		return ret;
 
@@ -2679,7 +2690,7 @@ static struct platform_driver arm_smmu_driver = {
 		.name		= "arm-smmu-v3",
 		.of_match_table	= of_match_ptr(arm_smmu_of_match),
 	},
-	.probe	= arm_smmu_device_dt_probe,
+	.probe	= arm_smmu_device_probe,
 	.remove	= arm_smmu_device_remove,
 };
 
-- 
2.10.0

^ permalink raw reply related

* [PATCH v9 11/16] drivers: iommu: arm-smmu-v3: add IORT configuration
From: Lorenzo Pieralisi @ 2016-11-21 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121100148.24769-1-lorenzo.pieralisi@arm.com>

In ACPI bases systems, in order to be able to create platform
devices and initialize them for ARM SMMU v3 components, the IORT
kernel implementation requires a set of static functions to be
used by the IORT kernel layer to configure platform devices for
ARM SMMU v3 components.

Add static configuration functions to the IORT kernel layer for
the ARM SMMU v3 components, so that the ARM SMMU v3 driver can
initialize its respective platform device by relying on the IORT
kernel infrastructure and by adding a corresponding ACPI device
early probe section entry.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Tomasz Nowicki <tn@semihalf.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Tomasz Nowicki <tn@semihalf.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Joerg Roedel <joro@8bytes.org>
---
 drivers/acpi/arm64/iort.c   | 103 +++++++++++++++++++++++++++++++++++++++++++-
 drivers/iommu/arm-smmu-v3.c |  49 ++++++++++++++++++++-
 2 files changed, 150 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index ddf83b5..fd52e4c 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -459,6 +459,95 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id)
 	return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
 }
 
+static void __init acpi_iort_register_irq(int hwirq, const char *name,
+					  int trigger,
+					  struct resource *res)
+{
+	int irq = acpi_register_gsi(NULL, hwirq, trigger,
+				    ACPI_ACTIVE_HIGH);
+
+	if (irq <= 0) {
+		pr_err("could not register gsi hwirq %d name [%s]\n", hwirq,
+								      name);
+		return;
+	}
+
+	res->start = irq;
+	res->end = irq;
+	res->flags = IORESOURCE_IRQ;
+	res->name = name;
+}
+
+static int __init arm_smmu_v3_count_resources(struct acpi_iort_node *node)
+{
+	struct acpi_iort_smmu_v3 *smmu;
+	/* Always present mem resource */
+	int num_res = 1;
+
+	/* Retrieve SMMUv3 specific data */
+	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
+
+	if (smmu->event_gsiv)
+		num_res++;
+
+	if (smmu->pri_gsiv)
+		num_res++;
+
+	if (smmu->gerr_gsiv)
+		num_res++;
+
+	if (smmu->sync_gsiv)
+		num_res++;
+
+	return num_res;
+}
+
+static void __init arm_smmu_v3_init_resources(struct resource *res,
+					      struct acpi_iort_node *node)
+{
+	struct acpi_iort_smmu_v3 *smmu;
+	int num_res = 0;
+
+	/* Retrieve SMMUv3 specific data */
+	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
+
+	res[num_res].start = smmu->base_address;
+	res[num_res].end = smmu->base_address + SZ_128K - 1;
+	res[num_res].flags = IORESOURCE_MEM;
+
+	num_res++;
+
+	if (smmu->event_gsiv)
+		acpi_iort_register_irq(smmu->event_gsiv, "eventq",
+				       ACPI_EDGE_SENSITIVE,
+				       &res[num_res++]);
+
+	if (smmu->pri_gsiv)
+		acpi_iort_register_irq(smmu->pri_gsiv, "priq",
+				       ACPI_EDGE_SENSITIVE,
+				       &res[num_res++]);
+
+	if (smmu->gerr_gsiv)
+		acpi_iort_register_irq(smmu->gerr_gsiv, "gerror",
+				       ACPI_EDGE_SENSITIVE,
+				       &res[num_res++]);
+
+	if (smmu->sync_gsiv)
+		acpi_iort_register_irq(smmu->sync_gsiv, "cmdq-sync",
+				       ACPI_EDGE_SENSITIVE,
+				       &res[num_res++]);
+}
+
+static bool __init arm_smmu_v3_is_coherent(struct acpi_iort_node *node)
+{
+	struct acpi_iort_smmu_v3 *smmu;
+
+	/* Retrieve SMMUv3 specific data */
+	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
+
+	return smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE;
+}
+
 struct iort_iommu_config {
 	const char *name;
 	int (*iommu_init)(struct acpi_iort_node *node);
@@ -468,10 +557,22 @@ struct iort_iommu_config {
 				     struct acpi_iort_node *node);
 };
 
+static const struct iort_iommu_config iort_arm_smmu_v3_cfg __initconst = {
+	.name = "arm-smmu-v3",
+	.iommu_is_coherent = arm_smmu_v3_is_coherent,
+	.iommu_count_resources = arm_smmu_v3_count_resources,
+	.iommu_init_resources = arm_smmu_v3_init_resources
+};
+
 static __init
 const struct iort_iommu_config *iort_get_iommu_cfg(struct acpi_iort_node *node)
 {
-	return NULL;
+	switch (node->type) {
+	case ACPI_IORT_NODE_SMMU_V3:
+		return &iort_arm_smmu_v3_cfg;
+	default:
+		return NULL;
+	}
 }
 
 /**
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 2b3f9ac..d22c428 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -20,6 +20,8 @@
  * This driver is powered by bad coffee and bombay mix.
  */
 
+#include <linux/acpi.h>
+#include <linux/acpi_iort.h>
 #include <linux/delay.h>
 #include <linux/dma-iommu.h>
 #include <linux/err.h>
@@ -2559,6 +2561,32 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
 	return 0;
 }
 
+#ifdef CONFIG_ACPI
+static int arm_smmu_device_acpi_probe(struct platform_device *pdev,
+				      struct arm_smmu_device *smmu)
+{
+	struct acpi_iort_smmu_v3 *iort_smmu;
+	struct device *dev = smmu->dev;
+	struct acpi_iort_node *node;
+
+	node = *(struct acpi_iort_node **)dev_get_platdata(dev);
+
+	/* Retrieve SMMUv3 specific data */
+	iort_smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
+
+	if (iort_smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE)
+		smmu->features |= ARM_SMMU_FEAT_COHERENCY;
+
+	return 0;
+}
+#else
+static inline int arm_smmu_device_acpi_probe(struct platform_device *pdev,
+					     struct arm_smmu_device *smmu)
+{
+	return -ENODEV;
+}
+#endif
+
 static int arm_smmu_device_dt_probe(struct platform_device *pdev,
 				    struct arm_smmu_device *smmu)
 {
@@ -2624,8 +2652,16 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
 	if (irq > 0)
 		smmu->gerr_irq = irq;
 
+	if (dev->of_node) {
+		ret = arm_smmu_device_dt_probe(pdev, smmu);
+	} else {
+		ret = arm_smmu_device_acpi_probe(pdev, smmu);
+		if (ret == -ENODEV)
+			return ret;
+	}
+
 	/* Set bypass mode according to firmware probing result */
-	bypass = !!arm_smmu_device_dt_probe(pdev, smmu);
+	bypass = !!ret;
 
 	/* Probe the h/w */
 	ret = arm_smmu_device_hw_probe(smmu);
@@ -2728,6 +2764,17 @@ static int __init arm_smmu_of_init(struct device_node *np)
 }
 IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3", arm_smmu_of_init);
 
+#ifdef CONFIG_ACPI
+static int __init acpi_smmu_v3_init(struct acpi_table_header *table)
+{
+	if (iort_node_match(ACPI_IORT_NODE_SMMU_V3))
+		return arm_smmu_init();
+
+	return 0;
+}
+IORT_ACPI_DECLARE(arm_smmu_v3, ACPI_SIG_IORT, acpi_smmu_v3_init);
+#endif
+
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
 MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
 MODULE_LICENSE("GPL v2");
-- 
2.10.0

^ permalink raw reply related

* [PATCH v9 12/16] drivers: iommu: arm-smmu: split probe functions into DT/generic portions
From: Lorenzo Pieralisi @ 2016-11-21 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121100148.24769-1-lorenzo.pieralisi@arm.com>

Current ARM SMMU probe functions intermingle HW and DT probing
in the initialization functions to detect and programme the ARM SMMU
driver features. In order to allow probing the ARM SMMU with other
firmwares than DT, this patch splits the ARM SMMU init functions into
DT and HW specific portions so that other FW interfaces (ie ACPI) can
reuse the HW probing functions and skip the DT portion accordingly.

This patch implements no functional change, only code reshuffling.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Reviewed-by: Tomasz Nowicki <tn@semihalf.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Tomasz Nowicki <tn@semihalf.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/arm-smmu.c | 62 +++++++++++++++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 25 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 339a8d3..573b2b6 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1668,7 +1668,7 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
 	unsigned long size;
 	void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
 	u32 id;
-	bool cttw_dt, cttw_reg;
+	bool cttw_reg, cttw_fw = smmu->features & ARM_SMMU_FEAT_COHERENT_WALK;
 	int i;
 
 	dev_notice(smmu->dev, "probing hardware configuration...\n");
@@ -1713,20 +1713,17 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
 
 	/*
 	 * In order for DMA API calls to work properly, we must defer to what
-	 * the DT says about coherency, regardless of what the hardware claims.
+	 * the FW says about coherency, regardless of what the hardware claims.
 	 * Fortunately, this also opens up a workaround for systems where the
 	 * ID register value has ended up configured incorrectly.
 	 */
-	cttw_dt = of_dma_is_coherent(smmu->dev->of_node);
 	cttw_reg = !!(id & ID0_CTTW);
-	if (cttw_dt)
-		smmu->features |= ARM_SMMU_FEAT_COHERENT_WALK;
-	if (cttw_dt || cttw_reg)
+	if (cttw_fw || cttw_reg)
 		dev_notice(smmu->dev, "\t%scoherent table walk\n",
-			   cttw_dt ? "" : "non-");
-	if (cttw_dt != cttw_reg)
+			   cttw_fw ? "" : "non-");
+	if (cttw_fw != cttw_reg)
 		dev_notice(smmu->dev,
-			   "\t(IDR0.CTTW overridden by dma-coherent property)\n");
+			   "\t(IDR0.CTTW overridden by FW configuration)\n");
 
 	/* Max. number of entries we have for stream matching/indexing */
 	size = 1 << ((id >> ID0_NUMSIDB_SHIFT) & ID0_NUMSIDB_MASK);
@@ -1907,15 +1904,25 @@ static const struct of_device_id arm_smmu_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
 
-static int arm_smmu_device_dt_probe(struct platform_device *pdev)
+static int arm_smmu_device_dt_probe(struct platform_device *pdev,
+				    struct arm_smmu_device *smmu)
 {
 	const struct arm_smmu_match_data *data;
-	struct resource *res;
-	struct arm_smmu_device *smmu;
 	struct device *dev = &pdev->dev;
-	int num_irqs, i, err;
 	bool legacy_binding;
 
+	if (of_property_read_u32(dev->of_node, "#global-interrupts",
+				 &smmu->num_global_irqs)) {
+		dev_err(dev, "missing #global-interrupts property\n");
+		return -ENODEV;
+	}
+
+	data = of_device_get_match_data(dev);
+	smmu->version = data->version;
+	smmu->model = data->model;
+
+	parse_driver_options(smmu);
+
 	legacy_binding = of_find_property(dev->of_node, "mmu-masters", NULL);
 	if (legacy_binding && !using_generic_binding) {
 		if (!using_legacy_binding)
@@ -1928,6 +1935,19 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	if (of_dma_is_coherent(dev->of_node))
+		smmu->features |= ARM_SMMU_FEAT_COHERENT_WALK;
+
+	return 0;
+}
+
+static int arm_smmu_device_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct arm_smmu_device *smmu;
+	struct device *dev = &pdev->dev;
+	int num_irqs, i, err;
+
 	smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
 	if (!smmu) {
 		dev_err(dev, "failed to allocate arm_smmu_device\n");
@@ -1935,9 +1955,9 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 	}
 	smmu->dev = dev;
 
-	data = of_device_get_match_data(dev);
-	smmu->version = data->version;
-	smmu->model = data->model;
+	err = arm_smmu_device_dt_probe(pdev, smmu);
+	if (err)
+		return err;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	smmu->base = devm_ioremap_resource(dev, res);
@@ -1945,12 +1965,6 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 		return PTR_ERR(smmu->base);
 	smmu->size = resource_size(res);
 
-	if (of_property_read_u32(dev->of_node, "#global-interrupts",
-				 &smmu->num_global_irqs)) {
-		dev_err(dev, "missing #global-interrupts property\n");
-		return -ENODEV;
-	}
-
 	num_irqs = 0;
 	while ((res = platform_get_resource(pdev, IORESOURCE_IRQ, num_irqs))) {
 		num_irqs++;
@@ -1985,8 +1999,6 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
-	parse_driver_options(smmu);
-
 	if (smmu->version == ARM_SMMU_V2 &&
 	    smmu->num_context_banks != smmu->num_context_irqs) {
 		dev_err(dev,
@@ -2048,7 +2060,7 @@ static struct platform_driver arm_smmu_driver = {
 		.name		= "arm-smmu",
 		.of_match_table	= of_match_ptr(arm_smmu_of_match),
 	},
-	.probe	= arm_smmu_device_dt_probe,
+	.probe	= arm_smmu_device_probe,
 	.remove	= arm_smmu_device_remove,
 };
 
-- 
2.10.0

^ permalink raw reply related

* [PATCH v9 13/16] drivers: iommu: arm-smmu: add IORT configuration
From: Lorenzo Pieralisi @ 2016-11-21 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121100148.24769-1-lorenzo.pieralisi@arm.com>

In ACPI based systems, in order to be able to create platform
devices and initialize them for ARM SMMU components, the IORT
kernel implementation requires a set of static functions to be
used by the IORT kernel layer to configure platform devices for
ARM SMMU components.

Add static configuration functions to the IORT kernel layer for
the ARM SMMU components, so that the ARM SMMU driver can
initialize its respective platform device by relying on the IORT
kernel infrastructure and by adding a corresponding ACPI device
early probe section entry.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Tomasz Nowicki <tn@semihalf.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Tomasz Nowicki <tn@semihalf.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Joerg Roedel <joro@8bytes.org>
---
 drivers/acpi/arm64/iort.c | 71 +++++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/arm-smmu.c  | 77 ++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/acpi_iort.h |  3 ++
 3 files changed, 150 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index fd52e4c..8a8ae5e 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -548,6 +548,68 @@ static bool __init arm_smmu_v3_is_coherent(struct acpi_iort_node *node)
 	return smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE;
 }
 
+static int __init arm_smmu_count_resources(struct acpi_iort_node *node)
+{
+	struct acpi_iort_smmu *smmu;
+
+	/* Retrieve SMMU specific data */
+	smmu = (struct acpi_iort_smmu *)node->node_data;
+
+	/*
+	 * Only consider the global fault interrupt and ignore the
+	 * configuration access interrupt.
+	 *
+	 * MMIO address and global fault interrupt resources are always
+	 * present so add them to the context interrupt count as a static
+	 * value.
+	 */
+	return smmu->context_interrupt_count + 2;
+}
+
+static void __init arm_smmu_init_resources(struct resource *res,
+					   struct acpi_iort_node *node)
+{
+	struct acpi_iort_smmu *smmu;
+	int i, hw_irq, trigger, num_res = 0;
+	u64 *ctx_irq, *glb_irq;
+
+	/* Retrieve SMMU specific data */
+	smmu = (struct acpi_iort_smmu *)node->node_data;
+
+	res[num_res].start = smmu->base_address;
+	res[num_res].end = smmu->base_address + smmu->span - 1;
+	res[num_res].flags = IORESOURCE_MEM;
+	num_res++;
+
+	glb_irq = ACPI_ADD_PTR(u64, node, smmu->global_interrupt_offset);
+	/* Global IRQs */
+	hw_irq = IORT_IRQ_MASK(glb_irq[0]);
+	trigger = IORT_IRQ_TRIGGER_MASK(glb_irq[0]);
+
+	acpi_iort_register_irq(hw_irq, "arm-smmu-global", trigger,
+				     &res[num_res++]);
+
+	/* Context IRQs */
+	ctx_irq = ACPI_ADD_PTR(u64, node, smmu->context_interrupt_offset);
+	for (i = 0; i < smmu->context_interrupt_count; i++) {
+		hw_irq = IORT_IRQ_MASK(ctx_irq[i]);
+		trigger = IORT_IRQ_TRIGGER_MASK(ctx_irq[i]);
+
+		acpi_iort_register_irq(hw_irq, "arm-smmu-context", trigger,
+				       &res[num_res++]);
+	}
+}
+
+static bool __init arm_smmu_is_coherent(struct acpi_iort_node *node)
+{
+	struct acpi_iort_smmu *smmu;
+
+	/* Retrieve SMMU specific data */
+	smmu = (struct acpi_iort_smmu *)node->node_data;
+
+	return smmu->flags & ACPI_IORT_SMMU_COHERENT_WALK;
+}
+
 struct iort_iommu_config {
 	const char *name;
 	int (*iommu_init)(struct acpi_iort_node *node);
@@ -564,12 +626,21 @@ static const struct iort_iommu_config iort_arm_smmu_v3_cfg __initconst = {
 	.iommu_init_resources = arm_smmu_v3_init_resources
 };
 
+static const struct iort_iommu_config iort_arm_smmu_cfg __initconst = {
+	.name = "arm-smmu",
+	.iommu_is_coherent = arm_smmu_is_coherent,
+	.iommu_count_resources = arm_smmu_count_resources,
+	.iommu_init_resources = arm_smmu_init_resources
+};
+
 static __init
 const struct iort_iommu_config *iort_get_iommu_cfg(struct acpi_iort_node *node)
 {
 	switch (node->type) {
 	case ACPI_IORT_NODE_SMMU_V3:
 		return &iort_arm_smmu_v3_cfg;
+	case ACPI_IORT_NODE_SMMU:
+		return &iort_arm_smmu_cfg;
 	default:
 		return NULL;
 	}
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 573b2b6..8e66b16 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -28,6 +28,8 @@
 
 #define pr_fmt(fmt) "arm-smmu: " fmt
 
+#include <linux/acpi.h>
+#include <linux/acpi_iort.h>
 #include <linux/atomic.h>
 #include <linux/delay.h>
 #include <linux/dma-iommu.h>
@@ -1904,6 +1906,64 @@ static const struct of_device_id arm_smmu_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
 
+#ifdef CONFIG_ACPI
+static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
+{
+	int ret = 0;
+
+	switch (model) {
+	case ACPI_IORT_SMMU_V1:
+	case ACPI_IORT_SMMU_CORELINK_MMU400:
+		smmu->version = ARM_SMMU_V1;
+		smmu->model = GENERIC_SMMU;
+		break;
+	case ACPI_IORT_SMMU_V2:
+		smmu->version = ARM_SMMU_V2;
+		smmu->model = GENERIC_SMMU;
+		break;
+	case ACPI_IORT_SMMU_CORELINK_MMU500:
+		smmu->version = ARM_SMMU_V2;
+		smmu->model = ARM_MMU500;
+		break;
+	default:
+		ret = -ENODEV;
+	}
+
+	return ret;
+}
+
+static int arm_smmu_device_acpi_probe(struct platform_device *pdev,
+				      struct arm_smmu_device *smmu)
+{
+	struct device *dev = smmu->dev;
+	struct acpi_iort_node *node =
+		*(struct acpi_iort_node **)dev_get_platdata(dev);
+	struct acpi_iort_smmu *iort_smmu;
+	int ret;
+
+	/* Retrieve SMMU1/2 specific data */
+	iort_smmu = (struct acpi_iort_smmu *)node->node_data;
+
+	ret = acpi_smmu_get_data(iort_smmu->model, smmu);
+	if (ret < 0)
+		return ret;
+
+	/* Ignore the configuration access interrupt */
+	smmu->num_global_irqs = 1;
+
+	if (iort_smmu->flags & ACPI_IORT_SMMU_COHERENT_WALK)
+		smmu->features |= ARM_SMMU_FEAT_COHERENT_WALK;
+
+	return 0;
+}
+#else
+static inline int arm_smmu_device_acpi_probe(struct platform_device *pdev,
+					     struct arm_smmu_device *smmu)
+{
+	return -ENODEV;
+}
+#endif
+
 static int arm_smmu_device_dt_probe(struct platform_device *pdev,
 				    struct arm_smmu_device *smmu)
 {
@@ -1955,7 +2015,11 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
 	}
 	smmu->dev = dev;
 
-	err = arm_smmu_device_dt_probe(pdev, smmu);
+	if (dev->of_node)
+		err = arm_smmu_device_dt_probe(pdev, smmu);
+	else
+		err = arm_smmu_device_acpi_probe(pdev, smmu);
+
 	if (err)
 		return err;
 
@@ -2103,6 +2167,17 @@ IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401", arm_smmu_of_init);
 IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500", arm_smmu_of_init);
 IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2", arm_smmu_of_init);
 
+#ifdef CONFIG_ACPI
+static int __init arm_smmu_acpi_init(struct acpi_table_header *table)
+{
+	if (iort_node_match(ACPI_IORT_NODE_SMMU))
+		return arm_smmu_init();
+
+	return 0;
+}
+IORT_ACPI_DECLARE(arm_smmu, ACPI_SIG_IORT, arm_smmu_acpi_init);
+#endif
+
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
 MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
 MODULE_LICENSE("GPL v2");
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index 17bb078..79ba1bb 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -23,6 +23,9 @@
 #include <linux/fwnode.h>
 #include <linux/irqdomain.h>
 
+#define IORT_IRQ_MASK(irq)		(irq & 0xffffffffULL)
+#define IORT_IRQ_TRIGGER_MASK(irq)	((irq >> 32) & 0xffffffffULL)
+
 int iort_register_domain_token(int trans_id, struct fwnode_handle *fw_node);
 void iort_deregister_domain_token(int trans_id);
 struct fwnode_handle *iort_find_domain_token(int trans_id);
-- 
2.10.0

^ permalink raw reply related

* [PATCH v9 14/16] drivers: acpi: iort: replace rid map type with type mask
From: Lorenzo Pieralisi @ 2016-11-21 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121100148.24769-1-lorenzo.pieralisi@arm.com>

IORT tables provide data that allow the kernel to carry out
device ID mappings between endpoints and system components
(eg interrupt controllers, IOMMUs). When the mapping for a
given device ID is carried out, the translation mechanism
is done on a per-subsystem basis rather than a component
subtype (ie the IOMMU kernel layer will look for mappings
from a device to all IORT node types corresponding to IOMMU
components), therefore the corresponding mapping API should
work on a range (ie mask) of IORT node types corresponding
to a common set of components (eg IOMMUs) rather than a
specific node type.

Upgrade the IORT iort_node_map_rid() API to work with a
type mask instead of a single node type so that it can
be used for mappings that span multiple components types
(ie IOMMUs).

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Tomasz Nowicki <tn@semihalf.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Tomasz Nowicki <tn@semihalf.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Tomasz Nowicki <tn@semihalf.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
---
 drivers/acpi/arm64/iort.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 8a8ae5e..f3bbef8 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -26,6 +26,9 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
+#define IORT_TYPE_MASK(type)	(1 << (type))
+#define IORT_MSI_TYPE		(1 << ACPI_IORT_NODE_ITS_GROUP)
+
 struct iort_its_msi_chip {
 	struct list_head	list;
 	struct fwnode_handle	*fw_node;
@@ -317,7 +320,7 @@ static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in,
 
 static struct acpi_iort_node *iort_node_map_rid(struct acpi_iort_node *node,
 						u32 rid_in, u32 *rid_out,
-						u8 type)
+						u8 type_mask)
 {
 	u32 rid = rid_in;
 
@@ -326,7 +329,7 @@ static struct acpi_iort_node *iort_node_map_rid(struct acpi_iort_node *node,
 		struct acpi_iort_id_mapping *map;
 		int i;
 
-		if (node->type == type) {
+		if (IORT_TYPE_MASK(node->type) & type_mask) {
 			if (rid_out)
 				*rid_out = rid;
 			return node;
@@ -399,7 +402,7 @@ u32 iort_msi_map_rid(struct device *dev, u32 req_id)
 	if (!node)
 		return req_id;
 
-	iort_node_map_rid(node, req_id, &dev_id, ACPI_IORT_NODE_ITS_GROUP);
+	iort_node_map_rid(node, req_id, &dev_id, IORT_MSI_TYPE);
 	return dev_id;
 }
 
@@ -421,7 +424,7 @@ static int iort_dev_find_its_id(struct device *dev, u32 req_id,
 	if (!node)
 		return -ENXIO;
 
-	node = iort_node_map_rid(node, req_id, NULL, ACPI_IORT_NODE_ITS_GROUP);
+	node = iort_node_map_rid(node, req_id, NULL, IORT_MSI_TYPE);
 	if (!node)
 		return -ENXIO;
 
-- 
2.10.0

^ permalink raw reply related

* [PATCH v9 15/16] drivers: acpi: iort: add single mapping function
From: Lorenzo Pieralisi @ 2016-11-21 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121100148.24769-1-lorenzo.pieralisi@arm.com>

The current IORT id mapping API requires components to provide
an input requester ID (a Bus-Device-Function (BDF) identifier for
PCI devices) to translate an input identifier to an output
identifier through an IORT range mapping.

Named components do not have an identifiable source ID therefore
their respective input/output mapping can only be defined in
IORT tables through single mappings, that provide a translation
that does not require any input identifier.

Current IORT interface for requester id mappings (iort_node_map_rid())
is not suitable for components that do not provide a requester id,
so it cannot be used for IORT named components.

Add an interface to the IORT API to enable retrieval of id
by allowing an indexed walk of the single mappings array for
a given component, therefore completing the IORT mapping API.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Tomasz Nowicki <tn@semihalf.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Tomasz Nowicki <tn@semihalf.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Tomasz Nowicki <tn@semihalf.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
---
 drivers/acpi/arm64/iort.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index f3bbef8..6aae49c 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -318,6 +318,45 @@ static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in,
 	return 0;
 }
 
+static
+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;
+
+	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);
+
+	/* 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;
+	}
+
+	parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
+			       map->output_reference);
+
+	if (!(IORT_TYPE_MASK(parent->type) & type_mask))
+		return NULL;
+
+	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;
+			return parent;
+		}
+	}
+
+	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)
-- 
2.10.0

^ permalink raw reply related

* [PATCH v9 16/16] drivers: acpi: iort: introduce iort_iommu_configure
From: Lorenzo Pieralisi @ 2016-11-21 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121100148.24769-1-lorenzo.pieralisi@arm.com>

DT based systems have a generic kernel API to configure IOMMUs
for devices (ie of_iommu_configure()).

On ARM based ACPI systems, the of_iommu_configure() equivalent can
be implemented atop ACPI IORT kernel API, with the corresponding
functions to map device identifiers to IOMMUs and retrieve the
corresponding IOMMU operations necessary for DMA operations set-up.

By relying on the iommu_fwspec generic kernel infrastructure,
implement the IORT based IOMMU configuration for ARM ACPI systems
and hook it up in the ACPI kernel layer that implements DMA
configuration for a device.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> [ACPI core]
Reviewed-by: Tomasz Nowicki <tn@semihalf.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Tomasz Nowicki <tn@semihalf.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Tomasz Nowicki <tn@semihalf.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
---
 drivers/acpi/arm64/iort.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/scan.c       |  7 +++-
 include/linux/acpi_iort.h |  6 +++
 3 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 6aae49c..47bace8 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -28,6 +28,8 @@
 
 #define IORT_TYPE_MASK(type)	(1 << (type))
 #define IORT_MSI_TYPE		(1 << ACPI_IORT_NODE_ITS_GROUP)
+#define IORT_IOMMU_TYPE		((1 << ACPI_IORT_NODE_SMMU) |	\
+				(1 << ACPI_IORT_NODE_SMMU_V3))
 
 struct iort_its_msi_chip {
 	struct list_head	list;
@@ -501,6 +503,102 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id)
 	return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
 }
 
+static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
+{
+	u32 *rid = data;
+
+	*rid = alias;
+	return 0;
+}
+
+static int arm_smmu_iort_xlate(struct device *dev, u32 streamid,
+			       struct fwnode_handle *fwnode,
+			       const struct iommu_ops *ops)
+{
+	int ret = iommu_fwspec_init(dev, fwnode, ops);
+
+	if (!ret)
+		ret = iommu_fwspec_add_ids(dev, &streamid, 1);
+
+	return ret;
+}
+
+static const struct iommu_ops *iort_iommu_xlate(struct device *dev,
+					struct acpi_iort_node *node,
+					u32 streamid)
+{
+	const struct iommu_ops *ops = NULL;
+	int ret = -ENODEV;
+	struct fwnode_handle *iort_fwnode;
+
+	if (node) {
+		iort_fwnode = iort_get_fwnode(node);
+		if (!iort_fwnode)
+			return NULL;
+
+		ops = iommu_get_instance(iort_fwnode);
+		if (!ops)
+			return NULL;
+
+		ret = arm_smmu_iort_xlate(dev, streamid, iort_fwnode, ops);
+	}
+
+	return ret ? NULL : ops;
+}
+
+/**
+ * iort_iommu_configure - Set-up IOMMU configuration for a device.
+ *
+ * @dev: device to configure
+ *
+ * Returns: iommu_ops pointer on configuration success
+ *          NULL on configuration failure
+ */
+const struct iommu_ops *iort_iommu_configure(struct device *dev)
+{
+	struct acpi_iort_node *node, *parent;
+	const struct iommu_ops *ops = NULL;
+	u32 streamid = 0;
+
+	if (dev_is_pci(dev)) {
+		struct pci_bus *bus = to_pci_dev(dev)->bus;
+		u32 rid;
+
+		pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
+				       &rid);
+
+		node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
+				      iort_match_node_callback, &bus->dev);
+		if (!node)
+			return NULL;
+
+		parent = iort_node_map_rid(node, rid, &streamid,
+					   IORT_IOMMU_TYPE);
+
+		ops = iort_iommu_xlate(dev, parent, streamid);
+
+	} else {
+		int i = 0;
+
+		node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
+				      iort_match_node_callback, dev);
+		if (!node)
+			return NULL;
+
+		parent = iort_node_get_id(node, &streamid,
+					  IORT_IOMMU_TYPE, i++);
+
+		while (parent) {
+			ops = iort_iommu_xlate(dev, parent, streamid);
+
+			parent = iort_node_get_id(node, &streamid,
+						  IORT_IOMMU_TYPE, i++);
+		}
+	}
+
+	return ops;
+}
+
 static void __init acpi_iort_register_irq(int hwirq, const char *name,
 					  int trigger,
 					  struct resource *res)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 45b5710..80698d3 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -7,6 +7,7 @@
 #include <linux/slab.h>
 #include <linux/kernel.h>
 #include <linux/acpi.h>
+#include <linux/acpi_iort.h>
 #include <linux/signal.h>
 #include <linux/kthread.h>
 #include <linux/dmi.h>
@@ -1377,6 +1378,8 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
  */
 void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
 {
+	const struct iommu_ops *iommu;
+
 	/*
 	 * Set default coherent_dma_mask to 32 bit.  Drivers are expected to
 	 * setup the correct supported mask.
@@ -1391,11 +1394,13 @@ void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
 	if (!dev->dma_mask)
 		dev->dma_mask = &dev->coherent_dma_mask;
 
+	iommu = iort_iommu_configure(dev);
+
 	/*
 	 * Assume dma valid range starts at 0 and covers the whole
 	 * coherent_dma_mask.
 	 */
-	arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, NULL,
+	arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, iommu,
 			   attr == DEV_DMA_COHERENT);
 }
 EXPORT_SYMBOL_GPL(acpi_dma_configure);
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index 79ba1bb..dcb2b60 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -34,6 +34,8 @@ void acpi_iort_init(void);
 bool iort_node_match(u8 type);
 u32 iort_msi_map_rid(struct device *dev, u32 req_id);
 struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id);
+/* IOMMU interface */
+const struct iommu_ops *iort_iommu_configure(struct device *dev);
 #else
 static inline void acpi_iort_init(void) { }
 static inline bool iort_node_match(u8 type) { return false; }
@@ -42,6 +44,10 @@ static inline u32 iort_msi_map_rid(struct device *dev, u32 req_id)
 static inline struct irq_domain *iort_get_device_domain(struct device *dev,
 							u32 req_id)
 { return NULL; }
+/* IOMMU interface */
+static inline
+const struct iommu_ops *iort_iommu_configure(struct device *dev)
+{ return NULL; }
 #endif
 
 #define IORT_ACPI_DECLARE(name, table_id, fn)		\
-- 
2.10.0

^ permalink raw reply related

* [PATCH] mfd: twl-core: export twl_get_regmap
From: Nicolae Rosia @ 2016-11-21 10:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121093144.GE23750@n2100.armlinux.org.uk>

On Mon, Nov 21, 2016 at 11:31 AM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> Passing data between drivers using *_set_drvdata() is a layering
> violation:
>
> 1. Driver data is supposed to be driver private data associated with
>    the currently bound driver.
> 2. The driver data pointer is NULL'd when the driver unbinds from the
>    device.  See __device_release_driver() and the
>    dev_set_drvdata(dev, NULL).
> 3. It will break with CONFIG_DEBUG_TEST_DRIVER_REMOVE enabled for a
>    similar reason to (2).
>
> So, do not pass data between drivers using *_set_drvdata() - any
> examples in the kernel already are founded on bad practice, are
> fragile, and are already broken for some kernel configurations.
After inspecting mfd_add_device, it seems that it creates a
platform_device which has the parent set to the driver calling the
function.
Isn't module unloading forbidden if there is a parent->child
relationship in place and you're removing the parent?
What should be the best practice to share data between drivers?
Reference counted data?
In the case of TWL, the twl-core is just a simple container for
regmaps - all other "sub devices" are using those regmaps to access
the I2C device's registers, it makes no sense to remove the parent
driver since it does *nothing*.

^ permalink raw reply

* [PATCH 8/9] mfd: wm97xx-core: core support for wm97xx Codec
From: Lee Jones @ 2016-11-21 10:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <87twb3r94s.fsf@belgarion.home>

Mark, please see below:

On Sat, 19 Nov 2016, Robert Jarzmik wrote:
> Lee Jones <lee.jones@linaro.org> writes:
> 
> >> +#define WM9705_VENDOR_ID 0x574d4c05
> >> +#define WM9712_VENDOR_ID 0x574d4c12
> >> +#define WM9713_VENDOR_ID 0x574d4c13
> >> +#define WM97xx_VENDOR_ID_MASK 0xffffffff
> >
> > These are probably better represented as enums.
> These are ids, just as devicetree ids, or PCI ids, I don't think an enum will
> fit.

That's fine.  We can use enums to simply group items of a similar
type.  Representing these as enums would only serve to benefit code
cleanliness.  What makes you think they won't fit?

> >> +struct wm97xx_priv {
> >> +	struct regmap *regmap;
> >> +	struct snd_ac97 *ac97;
> >> +	struct device *dev;
> >> +};
> >
> > Replace _priv with _ddata.
> Ok, will do, would you care to explain if this is something specific to your mfd
> tree, or is it a kernel overall recommendation ?

It's personal preference.  But these aren't necessarily private, so
priv doesn't really make a great deal of sense.  These types of saved
pointers are better described as device data (ddata).


[...]

> >> +static const struct reg_default wm97xx_reg_defaults[] = {
> >> +};
> >
> > What's the point in this?
> Ah, that's a reminder I have still more work on this patch ... Either I remove
> support for wm9705 and wm9712 in the first version, or I complete it and add the
> tables :
>  - wm9705_reg_defaults
>  - wm9712_reg_defaults

Please don't add redundant code.  I suggest you remove it for this
submission.

> >> +static int wm9705_register(struct wm97xx_priv *wm97xx)
> >> +{
> >> +	return 0;
> >> +}
> >> +
> >> +static int wm9712_register(struct wm97xx_priv *wm97xx)
> >> +{
> >> +	return 0;
> >> +}
> >
> > I don't get it?
> >
> > Either populate or don't provide.
> Same story as above, either I complete wm9705 and wm9712 support or I remove it.

Remove it please.

> >> +static int wm9713_register(struct wm97xx_priv *wm97xx,
> >> +			   struct wm97xx_pdata *pdata)
> >> +{
> >
> > What are you lining this up with?
> Emacs ... I don't see your point though, seeing it not aligned in the diff chunk
> doesn't mean it's not properly aligned.

That is true.  So the two "scruct"s are line up in the source file,
right?

[...]

> >> +	codec_pdata.ac97 = wm97xx->ac97;
> >> +	codec_pdata.regmap = devm_regmap_init_ac97(wm97xx->ac97,
> >> +						   &wm9713_regmap_config);
> >> +	codec_pdata.batt_pdata = pdata->batt_pdata;
> >> +	if (IS_ERR(codec_pdata.regmap))
> >> +		return PTR_ERR(codec_pdata.regmap);
> >
> > This doesn't look like pdata.  You can get rid of all of this hoop
> > jumping if you add it to ddata and set it as such.
> You mean I should pass ddata to mfd sub-cells, right ?

Correct.

[...]

> >> +static int wm97xx_ac97_probe(struct ac97_codec_device *adev)
> >
> > This looks sound specific.
> >
> > Why isn't this a plane platform driver?
> That's the whole purpose of the patch serie.
> 
> This serie transforms the wm97xx drivers from a platform driver model to an ac97
> model, where the ac97 devices are automatically discovered. The long explanation
> is in patch 0/9, on how it started and what is the global purpose.
> 
> The short story is : switch to the new AC97 bus driver model.
> 
> As for the "sound specific part", it's because AC97 bus is mainly used in sound
> oriented drivers, but still the codec IPs provide more than just sound, as the
> Wolfson codecs for instance.

I'd like to get Mark Brown's opinion on this.

> >> +{
> >> +	struct wm97xx_priv *wm97xx;
> >> +	int ret;
> >> +	void *pdata = snd_ac97_codec_get_platdata(adev);
> >> +
> >> +	wm97xx = devm_kzalloc(ac97_codec_dev2dev(adev),
> >> +			      sizeof(*wm97xx), GFP_KERNEL);
> >> +	if (!wm97xx)
> >> +		return -ENOMEM;
> >> +
> >> +	wm97xx->dev = ac97_codec_dev2dev(adev);
> >> +	wm97xx->ac97 = snd_ac97_compat_alloc(adev);
> >> +	if (IS_ERR(wm97xx->ac97))
> >> +		return PTR_ERR(wm97xx->ac97);
> >> +
> >> +
> >> +	ac97_set_drvdata(adev, wm97xx);
> >> +	dev_info(wm97xx->dev, "wm97xx core found, id=0x%x\n",
> >> +		 adev->vendor_id);
> >
> > All of this ac97/sound stuff should be done in the ac97/sound driver.
> 
> Nope, it's not sound adherence you're seeing here, it's ac97 bus and driver
> model adherence you're seeing. Would the bus be in drivers/ac97 instead of
> sound/ac97, the code would remain the same, would be bus be i2c you would see
> the same kind of calls but with i2c_xxx and not ac97_xxx.
> 
> The wm97xx needs an ac97 bus to interact with the hardware, to provide sound,
> gpio, adc, etc ... functions. That's what is expressed here, and the fact that
> this ac97 access has to shared amongst the mfd sub-cells, and that these cells
> require :
>  - wm97xx->ac97 : this one is for drivers/input/touchscreen/wm97xx-core.c
> 
> So the requirement in this case is not for ac97/sound but input/touchscreen.
> 
> >> diff --git a/include/linux/mfd/wm97xx.h b/include/linux/mfd/wm97xx.h
> >> new file mode 100644
> >> index 000000000000..627322f14d48
> >> --- /dev/null
> >> +++ b/include/linux/mfd/wm97xx.h
> >> @@ -0,0 +1,31 @@
> >> +/*
> >> + * wm97xx client interface
> >> + *
> >> + * Copyright (C) 2016 Robert Jarzmik
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License as published by
> >> + * the Free Software Foundation; either version 2 of the License, or
> >> + * (at your option) any later version.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >> + * GNU General Public License for more details.
> >> + */
> >> +
> >> +#ifndef __LINUX_MFD_WM97XX_H
> >> +#define __LINUX_MFD_WM97XX_H
> >> +
> >> +struct regmap;
> >> +struct wm97xx_batt_pdata;
> >> +struct snd_ac97;
> >
> > Why can't you just add the include files?
> I could, but I don't need to, do I ?
> Moreover, if a mfd sub-cell doesn't use regmap for example, I won't include a
> useless define.
> 
> Thanks for the review, Lee. This will iterate, I'll split out mfd patch(es) to
> follow up the review with you and Mark to lessen the burden on your mailbox.
> 
> Cheers.
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [GIT PULL] Second Round of Renesas ARM Based SoC Drivers Updates for v4.10
From: Geert Uytterhoeven @ 2016-11-21 10:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161119013551.GB2543@localhost>

Hi Olof,

On Sat, Nov 19, 2016 at 2:35 AM, Olof Johansson <olof@lixom.net> wrote:
> On Thu, Nov 17, 2016 at 03:04:35PM +0100, Simon Horman wrote:
>> Please consider these second round of Renesas ARM based SoC drivers updates for v4.10.
>>
>> This pull request is based on a merge of:
>>
>> * The previous round of such requests, tagged as renesas-drivers-for-v4.10,
>>   which you have already pulled.
>> * The soc-device-match-tag1 tag of Geert Uytterhoeven's renesas-driver's tree.
>>   This is included to provide core soc_device_match() infrastructure which
>>   is a dependency of identifying SoC and registering with SoC bus.
>>
>>
>> The following changes since commit 437c4eeb0bd4c1d68817be997716f52b8c22a9c3:
>>
>>   Merge tag 'soc-device-match-tag1' of https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers into HEAD (2016-11-15 14:12:57 +0100)
>>
>> are available in the git repository at:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git tags/renesas-drivers2-for-v4.10
>>
>> for you to fetch changes up to 63ee9e2ba47dbdb42156c9b940515cfd49e78c91:
>>
>>   soc: renesas: Identify SoC and register with the SoC bus (2016-11-17 14:37:20 +0100)
>>
>> ----------------------------------------------------------------
>> Second Round of Renesas ARM Based SoC Drivers Updates for v4.10
>>
>> * Identify SoC and register with the SoC bus
>> * Add support for the r8a7745 SoC to rcar-sysc
>>
>> ----------------------------------------------------------------
>> Geert Uytterhoeven (2):
>>       ARM: shmobile: Document DT bindings for Product Register
>>       soc: renesas: Identify SoC and register with the SoC bus
>>
>> Sergei Shtylyov (2):
>>       ARM: shmobile: r8a7745: add power domain index macros
>>       soc: renesas: rcar-sysc: add R8A7745 support
>>
>
> So, this pull request contains 8 patches, not 4. Seems like your pull
> request doesn't show any of the code from Geert's branch, didn't mention
> it in the tag and only in the email text above. Furthermore, Geert's
> branch modifies driver core code, so it's extra important to make sure
> it's clear that it's an unusual pull request.

Please accept our apologies for failing to make this clearer.

> Given that this modifies driver core, please either merge that code
> through Greg first, or get an ack from him. If you merge through him,
> make sure it's on a standalone topic branch that we can share.

I provided my soc-device-match branch (tag soc-device-match-tag1) as an
immutable base, to be included by all interested parties that need the
soc_device_match() infrastructure (Freescale/NXP, Samsung, Renesas).
Its core parts have been acked by Greg, and the fixes by Arnd and/or Greg
(the last fix only received an informal ack, that's why I hadn't added the
ack).
This branch has already been pulled by Ulf, and is present in mmc/next, as
a dependency for a Freescale/NXP driver update.

I hope my explanation helps. If anything is still unclear, please ask.
Thanks for reconsidering!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH 1/6] ASoC: samsung: Remove non-existing MACH dependencies
From: Sylwester Nawrocki @ 2016-11-21 10:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479566911-5580-2-git-send-email-krzk@kernel.org>

On 11/19/2016 03:48 PM, Krzysztof Kozlowski wrote:
> MACH_SMDKC100 was removed in commit b8529ec1c1b0 ("ARM: S5PC100: no more
> support S5PC100 SoC"). MACH_SMDKV210 and MACH_SMDKC110 in commit
> 28c8331d386 ("ARM: S5PV210: Remove support for board files").
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

^ permalink raw reply

* [PATCH v8 6/7] arm/arm64: vgic: Implement VGICv3 CPU interface access
From: Christoffer Dall @ 2016-11-21 10:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CALicx6uia5_H5Zg_x6GP0nKgZi_513w97igCuDdezySOUNQtBw@mail.gmail.com>

On Fri, Nov 18, 2016 at 10:28:34PM +0530, Vijay Kilari wrote:
> On Thu, Nov 17, 2016 at 9:39 PM, Christoffer Dall
> <christoffer.dall@linaro.org> wrote:
> > On Thu, Nov 17, 2016 at 09:25:59PM +0530, Vijay Kilari wrote:
> >> On Thu, Nov 17, 2016 at 12:22 AM, Christoffer Dall
> >> <christoffer.dall@linaro.org> wrote:
> >> > On Fri, Nov 04, 2016 at 04:43:32PM +0530, vijay.kilari at gmail.com wrote:
> >> >> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> >> >>
> >> >> VGICv3 CPU interface registers are accessed using
> >> >> KVM_DEV_ARM_VGIC_CPU_SYSREGS ioctl. These registers are accessed
> >> >> as 64-bit. The cpu MPIDR value is passed along with register id.
> >> >> is used to identify the cpu for registers access.
> >> >>
> >> >> The version of VGIC v3 specification is define here
> >> >> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/445611.html
> >> >>
> >> >> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> >> >> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> >> >> ---
> >> >>  arch/arm64/include/uapi/asm/kvm.h   |   3 +
> >> >>  arch/arm64/kvm/Makefile             |   1 +
> >> >>  include/kvm/arm_vgic.h              |   9 +
> >> >>  virt/kvm/arm/vgic/vgic-kvm-device.c |  27 +++
> >> >>  virt/kvm/arm/vgic/vgic-mmio-v3.c    |  19 +++
> >> >>  virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 324 ++++++++++++++++++++++++++++++++++++
> >> >>  virt/kvm/arm/vgic/vgic-v3.c         |   8 +
> >> >>  virt/kvm/arm/vgic/vgic.h            |   4 +
> >> >>  8 files changed, 395 insertions(+)
> >> >>
> >> >> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> >> >> index 56dc08d..91c7137 100644
> >> >> --- a/arch/arm64/include/uapi/asm/kvm.h
> >> >> +++ b/arch/arm64/include/uapi/asm/kvm.h
> >> >> @@ -206,9 +206,12 @@ struct kvm_arch_memory_slot {
> >> >>                       (0xffffffffULL << KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT)
> >> >>  #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT      0
> >> >>  #define   KVM_DEV_ARM_VGIC_OFFSET_MASK       (0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
> >> >> +#define   KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK (0xffff)
> >> >>  #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS 3
> >> >>  #define KVM_DEV_ARM_VGIC_GRP_CTRL    4
> >> >>  #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
> >> >> +#define KVM_DEV_ARM_VGIC_CPU_SYSREGS    6
> >> >> +
> >> >>  #define   KVM_DEV_ARM_VGIC_CTRL_INIT 0
> >> >>
> >> >>  /* Device Control API on vcpu fd */
> >> >> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> >> >> index d50a82a..1a14e29 100644
> >> >> --- a/arch/arm64/kvm/Makefile
> >> >> +++ b/arch/arm64/kvm/Makefile
> >> >> @@ -32,5 +32,6 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v3.o
> >> >>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-kvm-device.o
> >> >>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-its.o
> >> >>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o
> >> >> +kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-sys-reg-v3.o
> >> >
> >> > Thi is making me wonder:  Are we properly handling GICv3 save/restore
> >> > for AArch32 now that we have GICv3 support for AArch32?  By properly I
> >> > mean that either it is clearly only supported on AArch64 systems or it's
> >> > supported on both AArch64 and AArch32, but it shouldn't break randomly
> >> > on AArch32.
> >>
> >> It supports both AArch64 and AArch64 in handling of system registers
> >> save/restore.
> >> All system registers that we save/restore are 32-bit for both aarch64
> >> and aarch32.
> >> Though opcode op0 should be zero for aarch32, the remaining Op and CRn codes
> >> are same. However the codes sent by qemu is matched and register
> >> are handled properly irrespective of AArch32 or AArch64.
> >>
> >> I don't have platform which support AArch32 guests to verify.
> >
> > Actually this is not about the guest, it's about an ARMv8 AArch32 host
> > that has a GICv3.
> >
> > I just tried to do a v7 compile with your patches, and it results in an
> > epic failure, so there's something for you to look at.
> >
> >> >
> >> >>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o
> >> >>  kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o
> >> >> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> >> >> index 002f092..730a18a 100644
> >> >> --- a/include/kvm/arm_vgic.h
> >> >> +++ b/include/kvm/arm_vgic.h
> >> >> @@ -71,6 +71,9 @@ struct vgic_global {
> >> >>
> >> >>       /* GIC system register CPU interface */
> >> >>       struct static_key_false gicv3_cpuif;
> >> >> +
> >> >> +     /* Cache ICH_VTR_EL2 reg value */
> >> >> +     u32                     ich_vtr_el2;
> >> >>  };
> >> >>
> >> >>  extern struct vgic_global kvm_vgic_global_state;
> >> >> @@ -269,6 +272,12 @@ struct vgic_cpu {
> >> >>       u64 pendbaser;
> >> >>
> >> >>       bool lpis_enabled;
> >> >> +
> >> >> +     /* Cache guest priority bits */
> >> >> +     u32 num_pri_bits;
> >> >> +
> >> >> +     /* Cache guest interrupt ID bits */
> >> >> +     u32 num_id_bits;
> >> >>  };
> >> >>
> >> >>  extern struct static_key_false vgic_v2_cpuif_trap;
> >> >> diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
> >> >> index 6c7d30c..da532d1 100644
> >> >> --- a/virt/kvm/arm/vgic/vgic-kvm-device.c
> >> >> +++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
> >> >> @@ -504,6 +504,14 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
> >> >>               if (!is_write)
> >> >>                       *reg = tmp32;
> >> >>               break;
> >> >> +     case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
> >> >> +             u64 regid;
> >> >> +
> >> >> +             regid = (attr->attr & KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK);
> >> >> +             ret = vgic_v3_cpu_sysregs_uaccess(vcpu, is_write,
> >> >> +                                               regid, reg);
> >> >> +             break;
> >> >> +     }
> >> >>       default:
> >> >>               ret = -EINVAL;
> >> >>               break;
> >> >> @@ -537,6 +545,15 @@ static int vgic_v3_set_attr(struct kvm_device *dev,
> >> >>               reg = tmp32;
> >> >>               return vgic_v3_attr_regs_access(dev, attr, &reg, true);
> >> >>       }
> >> >> +     case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
> >> >> +             u64 __user *uaddr = (u64 __user *)(long)attr->addr;
> >> >> +             u64 reg;
> >> >> +
> >> >> +             if (get_user(reg, uaddr))
> >> >> +                     return -EFAULT;
> >> >> +
> >> >> +             return vgic_v3_attr_regs_access(dev, attr, &reg, true);
> >> >> +     }
> >> >>       }
> >> >>       return -ENXIO;
> >> >>  }
> >> >> @@ -563,6 +580,15 @@ static int vgic_v3_get_attr(struct kvm_device *dev,
> >> >>               tmp32 = reg;
> >> >>               return put_user(tmp32, uaddr);
> >> >>       }
> >> >> +     case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
> >> >> +             u64 __user *uaddr = (u64 __user *)(long)attr->addr;
> >> >> +             u64 reg;
> >> >> +
> >> >> +             ret = vgic_v3_attr_regs_access(dev, attr, &reg, false);
> >> >> +             if (ret)
> >> >> +                     return ret;
> >> >> +             return put_user(reg, uaddr);
> >> >> +     }
> >> >>       }
> >> >>
> >> >>       return -ENXIO;
> >> >> @@ -581,6 +607,7 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
> >> >>               break;
> >> >>       case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
> >> >>       case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS:
> >> >> +     case KVM_DEV_ARM_VGIC_CPU_SYSREGS:
> >> >>               return vgic_v3_has_attr_regs(dev, attr);
> >> >>       case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
> >> >>               return 0;
> >> >> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> >> >> index b35fb83..519b919 100644
> >> >> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
> >> >> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> >> >> @@ -23,6 +23,7 @@
> >> >>
> >> >>  #include "vgic.h"
> >> >>  #include "vgic-mmio.h"
> >> >> +#include "sys_regs.h"
> >> >>
> >> >>  /* extract @num bytes at @offset bytes offset in data */
> >> >>  unsigned long extract_bytes(u64 data, unsigned int offset,
> >> >> @@ -639,6 +640,24 @@ int vgic_v3_has_attr_regs(struct kvm_device *dev, struct kvm_device_attr *attr)
> >> >>               nr_regions = ARRAY_SIZE(vgic_v3_rdbase_registers);
> >> >>               break;
> >> >>       }
> >> >> +     case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
> >> >> +             u64 reg, id;
> >> >> +             unsigned long vgic_mpidr, mpidr_reg;
> >> >> +             struct kvm_vcpu *vcpu;
> >> >> +
> >> >> +             vgic_mpidr = (attr->attr & KVM_DEV_ARM_VGIC_V3_MPIDR_MASK) >>
> >> >> +                           KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT;
> >> >> +
> >> >> +             /* Convert plain mpidr value to MPIDR reg format */
> >> >> +             mpidr_reg = VGIC_TO_MPIDR(vgic_mpidr);
> >> >> +
> >> >> +             vcpu = kvm_mpidr_to_vcpu(dev->kvm, mpidr_reg);
> >> >> +             if (!vcpu)
> >> >> +                     return -EINVAL;
> >> >> +
> >> >> +             id = (attr->attr & KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK);
> >> >> +             return vgic_v3_has_cpu_sysregs_attr(vcpu, 0, id, &reg);
> >> >> +     }
> >> >>       default:
> >> >>               return -ENXIO;
> >> >>       }
> >> >> diff --git a/virt/kvm/arm/vgic/vgic-sys-reg-v3.c b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
> >> >> new file mode 100644
> >> >> index 0000000..69d8597
> >> >> --- /dev/null
> >> >> +++ b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
> >> >
> >> > Shouldn't we have a GPL header here?
> >> >
> >> >> @@ -0,0 +1,324 @@
> >> >> +#include <linux/irqchip/arm-gic-v3.h>
> >> >> +#include <linux/kvm.h>
> >> >> +#include <linux/kvm_host.h>
> >> >> +#include <kvm/iodev.h>
> >> >> +#include <kvm/arm_vgic.h>
> >> >> +#include <asm/kvm_emulate.h>
> >> >> +#include <asm/kvm_arm.h>
> >> >> +#include <asm/kvm_mmu.h>
> >> >> +
> >> >> +#include "vgic.h"
> >> >> +#include "vgic-mmio.h"
> >> >> +#include "sys_regs.h"
> >> >> +
> >> >> +static bool access_gic_ctlr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> >> >> +                         const struct sys_reg_desc *r)
> >> >> +{
> >> >> +     struct vgic_cpu *vgic_v3_cpu = &vcpu->arch.vgic_cpu;
> >> >> +     struct vgic_vmcr vmcr;
> >> >> +     u64 val;
> >> >> +     u32 num_pri_bits, num_id_bits;
> >> >> +
> >> >> +     vgic_get_vmcr(vcpu, &vmcr);
> >> >> +     if (p->is_write) {
> >> >> +             val = p->regval;
> >> >> +
> >> >> +             /*
> >> >> +              * Does not allow update of ICC_CTLR_EL1 if HW does not support
> >> >> +              * guest programmed ID and PRI bits
> >> >> +              */
> >> >
> >> > I would suggest rewording this comment:
> >> > Disallow restoring VM state not supported by this hardware.
> >> >
> >> >> +             num_pri_bits = ((val & ICC_CTLR_EL1_PRI_BITS_MASK) >>
> >> >> +                             ICC_CTLR_EL1_PRI_BITS_SHIFT) + 1;
> >> >> +             if (num_pri_bits > vgic_v3_cpu->num_pri_bits)
> >> >> +                     return false;
> >> >> +
> >> >> +             vgic_v3_cpu->num_pri_bits = num_pri_bits;
> >> >
> >> > hmmm, this looks weird to me, because vgic_v3_cpu->num_pri_bits I don't
> >> > understand which effect this is intended to have?
> >> >
> >> > Sure, it may limit what you do with other registers later, but since
> >> > there's no ordering requirement that the ctlr be restored first, I'm not
> >> > sure it makes sense.
> >> >
> >> > Also, since this field is RO in the ICH_VTR, we'll have a strange
> >> > situation during runtime after a GICv3 restore where the
> >> > vgic_v3_cpu->num_pri_its differs from the hardware's ICH_VTR_EL2 field,
> >> > which is never the case if you didn't do a save/restore.
> >>
> >> Yes, but in any case, vgic_v3_cpu->num_pri_bits will be always less
> >> than HW supported
> >> value.
> >>
> >
> > So answer my question:  What is the intended effect of writing this
> > value?  Is it just so that if you migrate this platform back again, then
> > you're checking compatibility with what the guest would potentially do,
> 
> Yes 

Then add a comment explaining that

> and also to limit the valid aprn registers access as you said above.
> But that has ordering restriction. Which I think we should follow.
> 

I'm sorry, now I'm confused.  Is there an ordering requirement in the
API, or how should we follow this?

> > or should you maintain the num_pri_bits limitation during runtime
> > somehow?
> Once after checking compatibility, at runtime it is not updated
> and this value is not used at all in VGIC further
> >
> >> >
> >> > Finally, should we somehow ensure that this field is set to the same
> >> > value across VCPUs or is that not an architectural requirement?
> >> >
> >>    Yes it is nice to have it same across VCPUs. But should be ok as
> >> we are ensuring value is not greater than HW supported value.
> >
> > Does the architecture allow having a different number of priority bits
> > supported across CPUs?  If not, you shouldn't allow a VM programming
> > things that way either.
> 
> AFAIK, architecturally it is not mentioned any where in the spec that priority
> bits should be same across CPUs.

ok

> >
> >> There is no single point of place where we can make such a check
> >>
> >> >> +
> >> >> +             num_id_bits = (val & ICC_CTLR_EL1_ID_BITS_MASK) >>
> >> >> +                            ICC_CTLR_EL1_ID_BITS_SHIFT;
> >> >> +             if (num_id_bits > vgic_v3_cpu->num_id_bits)
> >> >> +                     return false;
> >> >> +
> >> >> +             vgic_v3_cpu->num_id_bits = num_id_bits;
> >> >
> >> > same questions
> >> >
> >> >> +
> >> >> +             vmcr.ctlr &= ~(ICH_VMCR_CBPR_MASK | ICH_VMCR_EOIM_MASK);
> >> >> +             vmcr.ctlr |= ((val & ICC_CTLR_EL1_CBPR_MASK) >>
> >> >> +                           ICC_CTLR_EL1_CBPR_SHIFT) << ICH_VMCR_CBPR_SHIFT;
> >> >> +             vmcr.ctlr |= ((val & ICC_CTLR_EL1_EOImode_MASK) >>
> >> >> +                           ICC_CTLR_EL1_EOImode_SHIFT) <<
> >> >> +                           ICH_VMCR_EOIM_SHIFT;
> >> >
> >> > I'm really confused here.  Is the vmcr.ctlr field in the ICC_CTLR_EL1
> >> > format or in the VMCR format?  I would assume the former, since
> >> > otherwise I don't get the point with this indirection, and for GICv2
> >> > vmcr.ctlr captures the GICC_CTLR value and git_set_vmcr transforms this
> >> > into VMCR values.
> >> >
> >> > Having a line that says "ctlr &= ~ICH_VMCR" should make some alarm bells
> >> > ring.
> >>
> >> I will check and fix it.
> >> >
> >> >> +             vgic_set_vmcr(vcpu, &vmcr);
> >> >
> >> > Should we check compatibility between the source and destination for the
> >> > SEIS and A3V support here?
> >>
> >> Can be checked. But I feel A3V check makes more sense than checking for
> >> SEIS.
> >>
> >
> > Please argue the *why* for whatever you end up doing with respect to
> > both bits in the commit message of your next patch revision.
> >
> >> >
> >> >> +     } else {
> >> >> +             val = 0;
> >> >> +             val |= (vgic_v3_cpu->num_pri_bits - 1) <<
> >> >> +                     ICC_CTLR_EL1_PRI_BITS_SHIFT;
> >> >> +             val |= vgic_v3_cpu->num_id_bits <<
> >> >> +                     ICC_CTLR_EL1_ID_BITS_SHIFT;
> >> >> +             val |= ((kvm_vgic_global_state.ich_vtr_el2 &
> >> >> +                     ICH_VTR_SEIS_MASK) >> ICH_VTR_SEIS_SHIFT) <<
> >> >> +                     ICC_CTLR_EL1_SEIS_SHIFT;
> >> >> +             val |= ((kvm_vgic_global_state.ich_vtr_el2 &
> >> >> +                     ICH_VTR_A3V_MASK) >> ICH_VTR_A3V_SHIFT) <<
> >> >> +                     ICC_CTLR_EL1_A3V_SHIFT;
> >> >> +             val |= ((vmcr.ctlr & ICH_VMCR_CBPR_MASK) >>
> >> >> +                     ICH_VMCR_CBPR_SHIFT) << ICC_CTLR_EL1_CBPR_SHIFT;
> >> >> +             val |= ((vmcr.ctlr & ICH_VMCR_EOIM_MASK) >>
> >> >> +                     ICH_VMCR_EOIM_SHIFT) << ICC_CTLR_EL1_EOImode_SHIFT;
> >> >
> >> > again, these last two look weird to me.
> >> >
> >> >> +
> >> >> +             p->regval = val;
> >> >> +     }
> >> >> +
> >> >> +     return true;
> >> >> +}
> >> >> +
> >> >> +static bool access_gic_pmr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> >> >> +                        const struct sys_reg_desc *r)
> >> >> +{
> >> >> +     struct vgic_vmcr vmcr;
> >> >> +
> >> >> +     vgic_get_vmcr(vcpu, &vmcr);
> >> >> +     if (p->is_write) {
> >> >> +             vmcr.pmr = (p->regval & ICC_PMR_EL1_MASK) >> ICC_PMR_EL1_SHIFT;
> >> >> +             vgic_set_vmcr(vcpu, &vmcr);
> >> >> +     } else {
> >> >> +             p->regval = (vmcr.pmr << ICC_PMR_EL1_SHIFT) & ICC_PMR_EL1_MASK;
> >> >> +     }
> >> >> +
> >> >> +     return true;
> >> >> +}
> >> >> +
> >> >> +static bool access_gic_bpr0(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> >> >> +                         const struct sys_reg_desc *r)
> >> >> +{
> >> >> +     struct vgic_vmcr vmcr;
> >> >> +
> >> >> +     vgic_get_vmcr(vcpu, &vmcr);
> >> >> +     if (p->is_write) {
> >> >> +             vmcr.bpr = (p->regval & ICC_BPR0_EL1_MASK) >>
> >> >> +                         ICC_BPR0_EL1_SHIFT;
> >> >> +             vgic_set_vmcr(vcpu, &vmcr);
> >> >> +     } else {
> >> >> +             p->regval = (vmcr.bpr << ICC_BPR0_EL1_SHIFT) &
> >> >> +                          ICC_BPR0_EL1_MASK;
> >> >> +     }
> >> >> +
> >> >> +     return true;
> >> >> +}
> >> >> +
> >> >> +static bool access_gic_bpr1(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> >> >> +                         const struct sys_reg_desc *r)
> >> >> +{
> >> >> +     struct vgic_vmcr vmcr;
> >> >> +
> >> >> +     if (!p->is_write)
> >> >> +             p->regval = 0;
> >> >> +
> >> >> +     vgic_get_vmcr(vcpu, &vmcr);
> >> >> +     if (!((vmcr.ctlr & ICH_VMCR_CBPR_MASK) >> ICH_VMCR_CBPR_SHIFT)) {
> >> >> +             if (p->is_write) {
> >> >> +                     vmcr.abpr = (p->regval & ICC_BPR1_EL1_MASK) >>
> >> >> +                                  ICC_BPR1_EL1_SHIFT;
> >> >> +                     vgic_set_vmcr(vcpu, &vmcr);
> >> >> +             } else {
> >> >> +                     p->regval = (vmcr.abpr << ICC_BPR1_EL1_SHIFT) &
> >> >> +                                  ICC_BPR1_EL1_MASK;
> >> >> +             }
> >> >> +     } else {
> >> >> +             if (!p->is_write)
> >> >> +                     p->regval = min((vmcr.bpr + 1), 7U);
> >> >> +     }
> >> >> +
> >> >> +     return true;
> >> >> +}
> >> >> +
> >> >> +static bool access_gic_grpen0(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> >> >> +                           const struct sys_reg_desc *r)
> >> >> +{
> >> >> +     struct vgic_vmcr vmcr;
> >> >> +
> >> >> +     vgic_get_vmcr(vcpu, &vmcr);
> >> >> +     if (p->is_write) {
> >> >> +             vmcr.grpen0 = (p->regval & ICC_IGRPEN0_EL1_MASK) >>
> >> >> +                                   ICC_IGRPEN0_EL1_SHIFT;
> >> >> +             vgic_set_vmcr(vcpu, &vmcr);
> >> >> +     } else {
> >> >> +             p->regval = (vmcr.grpen0 << ICC_IGRPEN0_EL1_SHIFT) &
> >> >> +                          ICC_IGRPEN0_EL1_MASK;
> >> >> +     }
> >> >> +
> >> >> +     return true;
> >> >> +}
> >> >> +
> >> >> +static bool access_gic_grpen1(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> >> >> +                           const struct sys_reg_desc *r)
> >> >> +{
> >> >> +     struct vgic_vmcr vmcr;
> >> >> +
> >> >> +     vgic_get_vmcr(vcpu, &vmcr);
> >> >> +     if (p->is_write) {
> >> >> +             vmcr.grpen1 = (p->regval & ICC_IGRPEN1_EL1_MASK) >>
> >> >> +                                   ICC_IGRPEN1_EL1_SHIFT;
> >> >> +             vgic_set_vmcr(vcpu, &vmcr);
> >> >> +     } else {
> >> >> +             p->regval = (vmcr.grpen1 << ICC_IGRPEN1_EL1_SHIFT) &
> >> >> +                          ICC_IGRPEN1_EL1_MASK;
> >> >> +     }
> >> >> +
> >> >> +     return true;
> >> >> +}
> >> >> +
> >> >> +static void vgic_v3_access_apr_reg(struct kvm_vcpu *vcpu,
> >> >> +                                struct sys_reg_params *p, u8 apr, u8 idx)
> >> >> +{
> >> >> +     struct vgic_v3_cpu_if *vgicv3 = &vcpu->arch.vgic_cpu.vgic_v3;
> >> >> +     uint32_t *ap_reg;
> >> >> +
> >> >> +     if (apr)
> >> >> +             ap_reg = &vgicv3->vgic_ap1r[idx];
> >> >> +     else
> >> >> +             ap_reg = &vgicv3->vgic_ap0r[idx];
> >> >> +
> >> >> +     if (p->is_write)
> >> >> +             *ap_reg = p->regval;
> >> >> +     else
> >> >> +             p->regval = *ap_reg;
> >> >> +}
> >> >> +
> >> >> +static void access_gic_aprn(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> >> >> +                         const struct sys_reg_desc *r, u8 apr)
> >> >> +{
> >> >> +     struct vgic_cpu *vgic_v3_cpu = &vcpu->arch.vgic_cpu;
> >> >> +     u8 idx = r->Op2 & 3;
> >> >> +
> >> >> +     switch (vgic_v3_cpu->num_pri_bits) {
> >> >> +     case 7:
> >> >> +             if (idx > 3)
> >> >> +                     goto err;
> >> >
> >> > idx cannot be higher than three given the mask above, right?
> >> >
> >> >> +             vgic_v3_access_apr_reg(vcpu, p, apr, idx);
> >> >> +             break;
> >> >> +     case 6:
> >> >> +             if (idx > 1)
> >> >> +                     goto err;
> >> >> +             vgic_v3_access_apr_reg(vcpu, p, apr, idx);
> >> >> +             break;
> >> >> +     default:
> >> >> +             if (idx > 0)
> >> >> +                     goto err;
> >> >> +             vgic_v3_access_apr_reg(vcpu, p, apr, idx);
> >> >> +     }
> >> >
> >> > what's the rationale behind ignoring the case where userspace is using
> >> > unsupported priorities?  Is it that this will be checked during
> >> > save/restore of the ctlr?
> >> >
> >> > This sort of thing just looks like the case that's impossible to debug,
> >> > because userspace could be scratching its head trying to understand why
> >> > the value it wrote isn't recorded anywhere...
> >> >
> >> > If there's a good rationale for doing it this way, then could we have a
> >> > comment to that effect?
> >>
> >> Accessing umplemented priority registers raised UNDEF exception.
> >> So userspace accesing should be ignored instead of recording unsupported
> >> values.
> >>
> >
> > That's not what I asked.
> >
> > I asked why it's silently ignored as opposed to raising an error visible
> > to user space?
> >
> >> >
> >> >> +
> >> >> +     return;
> >> >> +err:
> >> >> +     if (!p->is_write)
> >> >> +             p->regval = 0;
> >> >> +}
> >> >> +
> >> >> +static bool access_gic_ap0r(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> >> >> +                         const struct sys_reg_desc *r)
> >> >> +{
> >> >> +     access_gic_aprn(vcpu, p, r, 0);
> >> >> +
> >> >> +     return true;
> >> >> +}
> >> >> +
> >> >> +static bool access_gic_ap1r(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> >> >> +                         const struct sys_reg_desc *r)
> >> >> +{
> >> >> +     access_gic_aprn(vcpu, p, r, 1);
> >> >> +
> >> >> +     return true;
> >> >> +}
> >> >> +
> >> >> +static bool access_gic_sre(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> >> >> +                        const struct sys_reg_desc *r)
> >> >> +{
> >> >> +     struct vgic_v3_cpu_if *vgicv3 = &vcpu->arch.vgic_cpu.vgic_v3;
> >> >> +
> >> >> +     /* Validate SRE bit */
> >> >> +     if (p->is_write) {
> >> >> +             if (!(p->regval & ICC_SRE_EL1_SRE))
> >> >> +                     return false;
> >> >> +     } else {
> >> >> +             p->regval = vgicv3->vgic_sre;
> >> >> +     }
> >> >> +
> >> >> +     return true;
> >> >> +}
> >> >> +
> >> >> +static const struct sys_reg_desc gic_v3_icc_reg_descs[] = {
> >> >> +     /* ICC_PMR_EL1 */
> >> >> +     { Op0(3), Op1(0), CRn(4), CRm(6), Op2(0), access_gic_pmr },
> >> >> +     /* ICC_BPR0_EL1 */
> >> >> +     { Op0(3), Op1(0), CRn(12), CRm(8), Op2(3), access_gic_bpr0 },
> >> >> +     /* ICC_AP0R0_EL1 */
> >> >> +     { Op0(3), Op1(0), CRn(12), CRm(8), Op2(4), access_gic_ap0r },
> >> >> +     /* ICC_AP0R1_EL1 */
> >> >> +     { Op0(3), Op1(0), CRn(12), CRm(8), Op2(5), access_gic_ap0r },
> >> >> +     /* ICC_AP0R2_EL1 */
> >> >> +     { Op0(3), Op1(0), CRn(12), CRm(8), Op2(6), access_gic_ap0r },
> >> >> +     /* ICC_AP0R3_EL1 */
> >> >> +     { Op0(3), Op1(0), CRn(12), CRm(8), Op2(7), access_gic_ap0r },
> >> >> +     /* ICC_AP1R0_EL1 */
> >> >> +     { Op0(3), Op1(0), CRn(12), CRm(9), Op2(0), access_gic_ap1r },
> >> >> +     /* ICC_AP1R1_EL1 */
> >> >> +     { Op0(3), Op1(0), CRn(12), CRm(9), Op2(1), access_gic_ap1r },
> >> >> +     /* ICC_AP1R2_EL1 */
> >> >> +     { Op0(3), Op1(0), CRn(12), CRm(9), Op2(2), access_gic_ap1r },
> >> >> +     /* ICC_AP1R3_EL1 */
> >> >> +     { Op0(3), Op1(0), CRn(12), CRm(9), Op2(3), access_gic_ap1r },
> >> >> +     /* ICC_BPR1_EL1 */
> >> >> +     { Op0(3), Op1(0), CRn(12), CRm(12), Op2(3), access_gic_bpr1 },
> >> >> +     /* ICC_CTLR_EL1 */
> >> >> +     { Op0(3), Op1(0), CRn(12), CRm(12), Op2(4), access_gic_ctlr },
> >> >> +     /* ICC_SRE_EL1 */
> >> >> +     { Op0(3), Op1(0), CRn(12), CRm(12), Op2(5), access_gic_sre },
> >> >> +     /* ICC_IGRPEN0_EL1 */
> >> >> +     { Op0(3), Op1(0), CRn(12), CRm(12), Op2(6), access_gic_grpen0 },
> >> >> +     /* ICC_GRPEN1_EL1 */
> >> >> +     { Op0(3), Op1(0), CRn(12), CRm(12), Op2(7), access_gic_grpen1 },
> >> >> +};
> >> >> +
> >> >> +int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id,
> >> >> +                             u64 *reg)
> >> >> +{
> >> >> +     struct sys_reg_params params;
> >> >> +     u64 sysreg = (id & KVM_DEV_ARM_VGIC_SYSREG_MASK) | KVM_REG_SIZE_U64;
> >> >> +
> >> >> +     params.regval = *reg;
> >> >> +     params.is_write = is_write;
> >> >> +     params.is_aarch32 = false;
> >> >> +     params.is_32bit = false;
> >> >> +
> >> >> +     if (find_reg_by_id(sysreg, &params, gic_v3_icc_reg_descs,
> >> >> +                           ARRAY_SIZE(gic_v3_icc_reg_descs)))
> >> >> +             return 0;
> >> >> +
> >> >> +     return -ENXIO;
> >> >> +}
> >> >> +
> >> >> +int vgic_v3_cpu_sysregs_uaccess(struct kvm_vcpu *vcpu, bool is_write, u64 id,
> >> >> +                             u64 *reg)
> >> >> +{
> >> >> +     struct sys_reg_params params;
> >> >> +     const struct sys_reg_desc *r;
> >> >> +     u64 sysreg = (id & KVM_DEV_ARM_VGIC_SYSREG_MASK) | KVM_REG_SIZE_U64;
> >> >> +
> >> >> +     if (is_write)
> >> >> +             params.regval = *reg;
> >> >> +     params.is_write = is_write;
> >> >> +     params.is_aarch32 = false;
> >> >> +     params.is_32bit = false;
> >> >> +
> >> >> +     r = find_reg_by_id(sysreg, &params, gic_v3_icc_reg_descs,
> >> >> +                        ARRAY_SIZE(gic_v3_icc_reg_descs));
> >> >> +     if (!r)
> >> >> +             return -ENXIO;
> >> >> +
> >> >> +     if (!r->access(vcpu, &params, r))
> >> >> +             return -EINVAL;
> >> >
> >> > According to the API, EINVAL meansinvalid mpidr.  Should we expand on
> >> > how it can be used or allocate a new error code?
> >> How abt EACCES error code?
> >>
> >
> > That would mean permission denied, which is a bit weird.
> Yes I agree, but you can suggest.

You could expand the meaning in the API doc for gicv3 and use EINVAL, or
you could expand on what ENXIO means.


Thanks,
-Christoffer

^ permalink raw reply

* [GIT PULL v2 9/10] arm64: tegra: Device tree changes for v4.10-rc1
From: Thierry Reding @ 2016-11-21 10:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161118161719.24153-9-thierry.reding@gmail.com>

Hi ARM SoC maintainers,

The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:

  Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git tags/tegra-for-4.10-arm64-dt-numeric-ids

for you to fetch changes up to 99575bceebd60b572f0ccf9a900fdb970922ca49:

  arm64: tegra: Add NVIDIA P2771 board support (2016-11-21 10:43:42 +0100)

The only change since the original pull request is the replacement of
symbolic names in DTS files by their numerical equivalents to remove the
dependency on the driver branchs, as requested by Olof.

I have three patches to reintroduce these symbols that I can resend after
v4.10-rc1.

Thanks,
Thierry

----------------------------------------------------------------
arm64: tegra: Device tree changes for v4.10-rc1

This adds initial support for Tegra186, the P3310 processor module as
well as the P2771 development board. Not much is functional, but there
is enough to boot to an initial ramdisk with debug serial output.

----------------------------------------------------------------
Joseph Lo (3):
      arm64: tegra: Add Tegra186 support
      arm64: tegra: Add NVIDIA P3310 processor module support
      arm64: tegra: Add NVIDIA P2771 board support

Thierry Reding (6):
      arm64: tegra: Add CPU nodes for Tegra186
      arm64: tegra: Add serial ports on Tegra186
      arm64: tegra: Add I2C controllers on Tegra186
      arm64: tegra: Add SDHCI controllers on Tegra186
      arm64: tegra: Add GPIO controllers on Tegra186
      arm64: tegra: Enable PSCI on P3310

 arch/arm64/boot/dts/nvidia/Makefile                |   1 +
 arch/arm64/boot/dts/nvidia/tegra186-p2771-0000.dts |   8 +
 arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi     |  64 ++++
 arch/arm64/boot/dts/nvidia/tegra186.dtsi           | 398 +++++++++++++++++++++
 4 files changed, 471 insertions(+)
 create mode 100644 arch/arm64/boot/dts/nvidia/tegra186-p2771-0000.dts
 create mode 100644 arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi
 create mode 100644 arch/arm64/boot/dts/nvidia/tegra186.dtsi

^ permalink raw reply

* [PATCH v3 10/10] ARM: dts: da850: add usb device node
From: Axel Haslam @ 2016-11-21 10:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <48d0c158-f7c8-0d1e-f06e-b7b28d6f2b93@lechnology.com>

On Mon, Nov 21, 2016 at 3:42 AM, David Lechner <david@lechnology.com> wrote:
> On 11/07/2016 02:39 PM, Axel Haslam wrote:
>>
>> This adds the ohci device node for the da850 soc.
>> It also enables it for the omapl138 hawk board.
>>
>> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
>> ---
>>  arch/arm/boot/dts/da850-lcdk.dts | 8 ++++++++
>>  arch/arm/boot/dts/da850.dtsi     | 8 ++++++++
>>  2 files changed, 16 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/da850-lcdk.dts
>> b/arch/arm/boot/dts/da850-lcdk.dts
>> index 7b8ab21..aaf533e 100644
>> --- a/arch/arm/boot/dts/da850-lcdk.dts
>> +++ b/arch/arm/boot/dts/da850-lcdk.dts
>> @@ -86,6 +86,14 @@
>>         };
>>  };
>>
>> +&usb_phy {
>> +       status = "okay";
>> +};
>> +
>> +&ohci {
>> +       status = "okay";
>> +};
>> +
>>  &serial2 {
>>         pinctrl-names = "default";
>>         pinctrl-0 = <&serial2_rxtx_pins>;
>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>> index 2534aab..50e86da 100644
>> --- a/arch/arm/boot/dts/da850.dtsi
>> +++ b/arch/arm/boot/dts/da850.dtsi
>> @@ -405,6 +405,14 @@
>>                                         >;
>>                         status = "disabled";
>>                 };
>> +               ohci: usb at 0225000 {
>
>
> In commit 2957e36e76c836b167e5e0c1edb578d8a9bd7af6 in the linux-davinci
> tree, the alias for the musb device is usb0. So, I think we should use usb1
> here instead of ohci - or change the usb0 alias to musb.
>
> https://git.kernel.org/cgit/linux/kernel/git/nsekhar/linux-davinci.git/commit/?h=v4.10/dt&id=2957e36e76c836b167e5e0c1edb578d8a9bd7af6

ok, i will change to usb1, since i will be resubmiting this.

>
>> +                       compatible = "ti,da830-ohci";
>> +                       reg = <0x225000 0x1000>;
>> +                       interrupts = <59>;
>> +                       phys = <&usb_phy 1>;
>> +                       phy-names = "usb-phy";
>> +                       status = "disabled";
>> +               };
>>                 gpio: gpio at 226000 {
>>                         compatible = "ti,dm6441-gpio";
>>                         gpio-controller;
>>
>

^ permalink raw reply

* [BUG] i2c-designware silently fails on long transfers
From: Mika Westerberg @ 2016-11-21 10:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161118193542.GO1041@n2100.armlinux.org.uk>

On Fri, Nov 18, 2016 at 07:35:42PM +0000, Russell King - ARM Linux wrote:
> With reference to this commit:
> 
> commit d39f77b06a712fcba6185a20bb209e357923d980
> Author: Andrew Jackson <Andrew.Jackson@arm.com>
> Date:   Fri Nov 7 12:10:44 2014 +0000
> 
>     i2c: designware: prevent early stop on TX FIFO empty
> 
>     If the Designware core is configured with IC_EMPTYFIFO_HOLD_MASTER_EN
>     set to zero, allowing the TX FIFO to become empty causes a STOP
>     condition to be generated on the I2C bus. If the transmit FIFO
>     threshold is set too high, an erroneous STOP condition can be
>     generated on long transfers - particularly where the interrupt
>     latency is extended.
> 
>     Signed-off-by: Andrew Jackson <Andrew.Jackson@arm.com>
>     Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
>     Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>     Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> 
> The TDA998x driver issues long I2C transfers to read the EDID from the
> device - and userspace can also issue large transfers too.  However,
> if a DW core is configured with IC_EMPTYFIFO_HOLD_MASTER_EN set as
> zero, the above commit doesn't seem to solve the problem.  During
> boot, with the patch below, I see:
> 
> [    1.736549] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x10
> [    1.736564] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x510
> [    1.736608] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x504
> [    1.736799] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x514
> [    1.736819] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x510
> ...
> [    1.737986] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x504
> [    1.738010] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x514
> [    1.738034] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x504
> [    1.738039] random: fast init done
> [    1.740120] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x714
> [    1.740231] i2c_dw_xfer: ffffffc97657b770:1 -> ffffffc97657b770:1 (0:0) [0 0 3 0] 8 [tx:ffffffc976682380:47] [rx:ffffffc9766823c9:55]
> [    1.740249] [drm:drm_edid_block_valid] *ERROR* EDID checksum is invalid, remainder is 93
> [    1.746979] Raw EDID:
> [    1.747934]          00 ff ff ff ff ff ff 00 34 a9 96 a2 01 01 01 01
> [    1.752342]          00 17 01 03 80 80 48 78 0a da ff a3 58 4a a2 29
> [    1.756748]          17 49 4b 21 08 00 31 40 45 40 61 40 81 80 01 01
> [    1.761153]          01 01 01 01 01 01 02 3a 80 d0 72 38 2d 40 10 2c
> [    1.765555]          45 80 ba 88 21 00 00 1e 02 00 d0 4e 30 09 12 54
> [    1.769958]          01 08 02 00 23 36 01 40 01 05 00 80 a1 4c 4b 49
> [    1.774361]          22 00 00 40 03 00 28 00 23 01 20 00 01 88 00 01
> [    1.778762]          08 00 00 40 00 02 03 04 0a 00 80 00 02 00 00 40
> 
> The significant thing is the "i2c_dw_xfer" line, where I add a print of
> the current state.  Here, we can see that the transfer is mid-way, but
> a stop condition has been generated by the hardware, leaving 55 bytes
> to be received.
> 
> Unfortunately, the i2c-designware driver ignores this, and believes that
> the transfer completed both fully and successfully, but returns bogus
> data to userspace or the kernel driver.  That's really _bad_ behaviour
> by the driver - it should at least return an error.

Totally agree.

> This problem is _soo_ bad that on my Juno, I can't run Xorg (it hits
> this every time we try to read the EDID) nor can I boot with the TV
> connected (it hits this every boot as well.)
> 
> I'd go as far as to say that the i2c-designware hardware, when
> configured with this option set to zero, is fundamentally broken for OS
> which do not provide any guarantee for interrupt latency, such as Linux.
> 
> The commit above tries to mitigate this by reducing the Tx FIFO
> threshold, so the interrupt is raised sooner, but that's clearly not
> enough for reliable operation.
> 
> Another mitigation would be to lower the I2C bus frequency on Juno from
> 400kHz to 100kHz, so that there's 4x longer IRQ latency possible.
> However, even that isn't going to be reliable - even going to 100kHz
> isn't going to allow the above case to be solved - the interrupt is
> delayed by around 2ms, and it takes about 1.4ms to send/receive 16 bytes
> at 100kHz.  (9 * 16 / (100*10^3)).
> 
> So, I think all hope is lost for i2c-designware on Juno to cope with
> reading the EDID from TDA998x reliably.

:-(

I wonder if we can get it work more reliably by using DMA (provided that
there are DMA channels available for I2C in Juno)? That would allow the
hardware to perform longer reads without relying on how fast the
interrupt handler is able to empty the Rx FIFO.

^ permalink raw reply

* [RFT v2 2/5] ASoC: samsung: smdk_wm8580: Remove old platforms and drop mach-types usage
From: Sylwester Nawrocki @ 2016-11-21 10:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479669895-19124-3-git-send-email-krzk@kernel.org>

On 11/20/2016 08:24 PM, Krzysztof Kozlowski wrote:
> MACH_SMDKC100, MACH_SMDKV210 and MACH_SMDKC110 are no longer supported
> so we can drop the dead code.  After this the driver no longer
> differentiates between machines (S3C24xx machines are not supported by
> it) so there is no need to override I2S device id in cpu_dai_name and
> SEC_PLAYBACK dai_link can be removed as well.
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
> 
> Not tested. The driver did not override .platform_name which looks
> suspicious to me. However I did not want to add changes which could have
> some visible impact on output code.

The patch looks good to me. However the existing smdk64xx sound support
less so. I don't have smdk6410 set up for testing yet, possibly I get
around that next week.
Indeed it's strange .platform_name is not also "samsung-i2s.2".

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

^ permalink raw reply

* [PATCH 1/2] i2c: designware: report short transfers
From: Mika Westerberg @ 2016-11-21 10:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <E1c7p12-0000R0-S8@rmk-PC.armlinux.org.uk>

On Fri, Nov 18, 2016 at 07:40:04PM +0000, Russell King wrote:
> Rather than reporting success for a short transfer due to interrupt
> latency, report an error both to the caller, as well as to the kernel
> log.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

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

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox