[PATCH v5 05/14] drivers: iommu: make iommu_fwspec OF agnostic

On Tue, Sep 13, 2016 at 02:38:35PM +0100, Robin Murphy wrote:

quoted

 static int arm_smmu_match_node(struct device *dev, void *data)
 {
-	return dev->of_node == data;
+	struct fwnode_handle *fwnode;
+
+	fwnode = dev->of_node ? &dev->of_node->fwnode : dev->fwnode;
+
+	return fwnode == data;
 }

Maybe we should hoist the dev_fwnode() helper from property.c up to
property.h so we can just have "return dev_fwnode(dev) == data;" here?

Yes, that's one way of doing it. The other would be initializing
dev->fwnode to &dev->of_node->fwnode in the DT probe path but first
I need to understand why that is not done in the first place.

[...]

quoted

+int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids)
+{
+	struct iommu_fwspec *fwspec = arch_get_iommu_fwspec(dev);
+	size_t size;
+
+	if (!fwspec)
+		return -EINVAL;
+
+	size = offsetof(struct iommu_fwspec, ids[fwspec->num_ids + 1]);
+	fwspec = krealloc(fwspec, size, GFP_KERNEL);
+	if (!fwspec)
+		return -ENOMEM;
+
+	while (num_ids--)
+		fwspec->ids[fwspec->num_ids++] = *ids++;

You've still got the +1 bug and incomprehensible loop from the old code
here, rather than the fixed version being removed below. Although now
that I've taken the plunge and done it properly in core code from the
outset, that should hopefully become moot.

Gah sorry, rebase mistake. I will wait for the dust to settle before
churning out a new series, it is hard to respin without a stable
base (hopefully your series will make this patch useless).

Thanks !
Lorenzo

Robin.

quoted

+
+	arch_set_iommu_fwspec(dev, fwspec);
+	return 0;
+}
+
+inline struct iommu_fwspec *dev_iommu_fwspec(struct device *dev)
+{
+	return arch_get_iommu_fwspec(dev);
+}

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 38669b8..ab3c069 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;

@@ -226,57 +187,3 @@ static int __init of_iommu_init(void)
 	return 0;
 }
 postcore_initcall_sync(of_iommu_init);
-
-int iommu_fwspec_init(struct device *dev, struct device_node *iommu_np)
-{
-	struct iommu_fwspec *fwspec = arch_get_iommu_fwspec(dev);
-
-	if (fwspec)
-		return 0;
-
-	fwspec = kzalloc(sizeof(*fwspec), GFP_KERNEL);
-	if (!fwspec)
-		return -ENOMEM;
-
-	fwspec->iommu_np = of_node_get(iommu_np);
-	fwspec->iommu_ops = of_iommu_get_ops(iommu_np);
-	arch_set_iommu_fwspec(dev, fwspec);
-	return 0;
-}
-
-void iommu_fwspec_free(struct device *dev)
-{
-	struct iommu_fwspec *fwspec = arch_get_iommu_fwspec(dev);
-
-	if (fwspec) {
-		of_node_put(fwspec->iommu_np);
-		kfree(fwspec);
-	}
-}
-
-int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids)
-{
-	struct iommu_fwspec *fwspec = arch_get_iommu_fwspec(dev);
-	size_t size;
-	int i;
-
-	if (!fwspec)
-		return -EINVAL;
-
-	size = offsetof(struct iommu_fwspec, ids[fwspec->num_ids + num_ids]);
-	fwspec = krealloc(fwspec, size, GFP_KERNEL);
-	if (!fwspec)
-		return -ENOMEM;
-
-	for (i = 0; i < num_ids; i++)
-		fwspec->ids[fwspec->num_ids + i] = ids[i];
-
-	fwspec->num_ids += num_ids;
-	arch_set_iommu_fwspec(dev, fwspec);
-	return 0;
-}
-
-inline struct iommu_fwspec *dev_iommu_fwspec(struct device *dev)
-{
-	return arch_get_iommu_fwspec(dev);
-}

diff --git a/include/linux/iommu-fwspec.h b/include/linux/iommu-fwspec.h
new file mode 100644
index 0000000..f88b635
--- /dev/null
+++ b/include/linux/iommu-fwspec.h

@@ -0,0 +1,70 @@
+#ifndef __IOMMU_FWSPEC_H
+#define __IOMMU_FWSPEC_H
+
+#include <linux/device.h>
+#include <linux/iommu.h>
+
+struct iommu_fwspec {
+	const struct iommu_ops	*iommu_ops;
+	struct fwnode_handle	*iommu_fwnode;
+	void			*iommu_priv;
+	unsigned int		num_ids;
+	u32			ids[];
+};
+
+#ifdef CONFIG_IOMMU_FWSPEC
+int iommu_fwspec_init(struct device *dev,
+		      struct fwnode_handle *iommu_fwnode);
+void iommu_fwspec_free(struct device *dev);
+int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids);
+struct iommu_fwspec *dev_iommu_fwspec(struct device *dev);
+
+void fwspec_iommu_set_ops(struct fwnode_handle *fwnode,
+			  const struct iommu_ops *ops);
+const struct iommu_ops *fwspec_iommu_get_ops(struct fwnode_handle *fwnode);
+
+#ifdef CONFIG_HAVE_IOMMU_FWSPEC
+#include <asm/iommu-fwspec.h>
+#else /* !CONFIG_HAVE_IOMMU_FWSPEC */
+static inline void arch_set_iommu_fwspec(struct device *dev,
+					 struct iommu_fwspec *fwspec) {}
+
+static inline struct iommu_fwspec *
+arch_get_iommu_fwspec(struct device *dev) { return NULL; }
+#endif
+#else /* CONFIG_IOMMU_FWSPEC */
+static inline int iommu_fwspec_init(struct device *dev,
+				    struct fwnode_handle *iommu_fwnode)
+{
+	return -ENODEV;
+}
+
+static inline void iommu_fwspec_free(struct device *dev)
+{
+}
+
+static inline int iommu_fwspec_add_ids(struct device *dev, u32 *ids,
+				       int num_ids)
+{
+	return -ENODEV;
+}
+
+static inline struct iommu_fwspec *dev_iommu_fwspec(struct device *dev)
+{
+	return NULL;
+}
+
+static inline void fwspec_iommu_set_ops(struct fwnode_handle *fwnode,
+					const struct iommu_ops *ops)
+{
+}
+
+static inline const struct iommu_ops *
+fwspec_iommu_get_ops(struct fwnode_handle *fwnode)
+{
+	return NULL;
+}
+
+#endif /* CONFIG_IOMMU_FWSPEC */
+
+#endif /* __IOMMU_FWSPEC_H */

diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index 358db49..4b02861 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h

@@ -3,6 +3,7 @@
 
 #include <linux/device.h>
 #include <linux/iommu.h>
+#include <linux/iommu-fwspec.h>
 #include <linux/of.h>
 
 #ifdef CONFIG_OF_IOMMU

@@ -14,14 +15,6 @@ extern int of_get_dma_window(struct device_node *dn, const char *prefix,
 extern const struct iommu_ops *of_iommu_configure(struct device *dev,
 					struct device_node *master_np);
 
-struct iommu_fwspec {
-	const struct iommu_ops	*iommu_ops;
-	struct device_node	*iommu_np;
-	void			*iommu_priv;
-	unsigned int		num_ids;
-	u32			ids[];
-};
-
 #else
 
 static inline int of_get_dma_window(struct device_node *dn, const char *prefix,

@@ -36,28 +29,19 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
 {
 	return NULL;
 }
-
-struct iommu_fwspec;
-
 #endif	/* CONFIG_OF_IOMMU */
 
-int iommu_fwspec_init(struct device *dev, struct device_node *iommu_np);
-void iommu_fwspec_free(struct device *dev);
-int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids);
-struct iommu_fwspec *dev_iommu_fwspec(struct device *dev);
-
-#ifdef CONFIG_HAVE_IOMMU_FWSPEC
-#include <asm/iommu-fwspec.h>
-#else /* !CONFIG_HAVE_IOMMU_FWSPEC */
-static inline void arch_set_iommu_fwspec(struct device *dev,
-					 struct iommu_fwspec *fwspec) {}
-
-static inline struct iommu_fwspec *
-arch_get_iommu_fwspec(struct device *dev) { return NULL; }
-#endif
+static inline void of_iommu_set_ops(struct device_node *np,
+				    const struct iommu_ops *ops)
+{
+	fwspec_iommu_set_ops(&np->fwnode, ops);
+}
 
-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 const struct iommu_ops *
+of_iommu_get_ops(struct device_node *np)
+{
+	return fwspec_iommu_get_ops(&np->fwnode);
+}
 
 extern struct of_device_id __iommu_of_table;