[PATCH 0/2 v2] Fix MAX_PHANDLE_ARGS limitations

[PATCH 0/2 v2] Fix MAX_PHANDLE_ARGS limitations

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Hi,

here is the second version of my patches to fix the
MAX_PHANDLE_ARGS limitation for the arm-smmu driver. On my
AMD Seattle system the max value of 16 is not enough to
initialize the SMMU, as the device tree node has an entry
with 25 phandle args (possible are up to 128). The result is
a WARNING and incorrect SMMU initialization.

On the other side MAX_PHANDLE_ARGS can't grow indefinitly,
as it causes 'struct of_phandle_args' to get bigger. This
struct is often allocated on-stack and should be small by
default.

So these patches introduce a way to make 'struct
of_phandle_args' bigger only when needed and change the
ARM-SMMU driver to make use of it.

The patches have been tested on my AMD Seattle system.

Please review.

Thanks,

	Joerg

Changes to v1:

	* Rebased to v4.2-rc2
	* Fixed leak in patch 2, as pointed out by Will
	  Deacon

Joerg Roedel (2):
  of: base: Allow more args than MAX_PHANDLE_ARGS if required
  arm/smmu: Make use of of_parse_phandle_with_var_args

 drivers/iommu/arm-smmu.c | 23 ++++++++++++++++-------
 drivers/of/base.c        | 43 ++++++++++++++++++++++++++++++++++++-------
 include/linux/of.h       |  7 +++++++
 3 files changed, 59 insertions(+), 14 deletions(-)

-- 
1.8.4.5

[PATCH 1/2] of: base: Allow more args than MAX_PHANDLE_ARGS if required

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

The main use of MAX_PHANDLE_ARGS is to define the number of
args elements in 'struct of_phandle_args'. This struct is
often declared on the stack and thus it is impractical to
increase MAX_PHANDLE_ARGS again and again.

To handle situations where more than MAX_PHANDLE_ARGS
elements may appear in a device-tree, introduce functions
to allocate/free 'struct of_phandle_args' with more than
MAX_PHANDLE_ARGS elements and provide the new function
of_parse_phandle_with_var_args(), which can handle those
variable-size structs.

This is necessary for the ARM-SMMU driver, where the number
of mmu-masters can be up to 128.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/of/base.c  | 43 ++++++++++++++++++++++++++++++++++++-------
 include/linux/of.h |  7 +++++++
 2 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 8b5a187..2b288db 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -54,6 +54,24 @@ DEFINE_MUTEX(of_mutex);
  */
 DEFINE_RAW_SPINLOCK(devtree_lock);
 
+struct of_phandle_args *of_alloc_phandle_args(int size)
+{
+	struct of_phandle_args *args;
+	int e = max(0, size - MAX_PHANDLE_ARGS);
+
+	args =  kzalloc(sizeof(struct of_phandle_args) + e * sizeof(uint32_t),
+			GFP_KERNEL);
+
+	return args;
+}
+EXPORT_SYMBOL(of_alloc_phandle_args);
+
+void of_free_phandle_args(struct of_phandle_args *args)
+{
+	kfree(args);
+}
+EXPORT_SYMBOL(of_free_phandle_args);
+
 int of_n_addr_cells(struct device_node *np)
 {
 	const __be32 *ip;
@@ -1446,7 +1464,8 @@ static int __of_parse_phandle_with_args(const struct device_node *np,
 					const char *list_name,
 					const char *cells_name,
 					int cell_count, int index,
-					struct of_phandle_args *out_args)
+					struct of_phandle_args *out_args,
+					int elems)
 {
 	const __be32 *list, *list_end;
 	int rc = 0, size, cur_index = 0;
@@ -1525,8 +1544,8 @@ static int __of_parse_phandle_with_args(const struct device_node *np,
 
 			if (out_args) {
 				int i;
-				if (WARN_ON(count > MAX_PHANDLE_ARGS))
-					count = MAX_PHANDLE_ARGS;
+				if (WARN_ON(count > elems))
+					count = elems;
 				out_args->np = node;
 				out_args->args_count = count;
 				for (i = 0; i < count; i++)
@@ -1577,7 +1596,7 @@ struct device_node *of_parse_phandle(const struct device_node *np,
 		return NULL;
 
 	if (__of_parse_phandle_with_args(np, phandle_name, NULL, 0,
-					 index, &args))
+					 index, &args, MAX_PHANDLE_ARGS))
 		return NULL;
 
 	return args.np;
@@ -1623,10 +1642,20 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
 	if (index < 0)
 		return -EINVAL;
 	return __of_parse_phandle_with_args(np, list_name, cells_name, 0,
-					    index, out_args);
+					    index, out_args, MAX_PHANDLE_ARGS);
 }
 EXPORT_SYMBOL(of_parse_phandle_with_args);
 
+int of_parse_phandle_with_var_args(const struct device_node *np, const char *list_name,
+				   const char *cells_name, int index,
+				   struct of_phandle_args *out_args, int elems)
+{
+	if (index < 0)
+		return -EINVAL;
+	return __of_parse_phandle_with_args(np, list_name, cells_name, 0,
+					    index, out_args, elems);
+}
+EXPORT_SYMBOL(of_parse_phandle_with_var_args);
 /**
  * of_parse_phandle_with_fixed_args() - Find a node pointed by phandle in a list
  * @np:		pointer to a device tree node containing a list
@@ -1664,7 +1693,7 @@ int of_parse_phandle_with_fixed_args(const struct device_node *np,
 	if (index < 0)
 		return -EINVAL;
 	return __of_parse_phandle_with_args(np, list_name, NULL, cell_count,
-					   index, out_args);
+					   index, out_args, MAX_PHANDLE_ARGS);
 }
 EXPORT_SYMBOL(of_parse_phandle_with_fixed_args);
 
@@ -1687,7 +1716,7 @@ int of_count_phandle_with_args(const struct device_node *np, const char *list_na
 				const char *cells_name)
 {
 	return __of_parse_phandle_with_args(np, list_name, cells_name, 0, -1,
-					    NULL);
+					    NULL, MAX_PHANDLE_ARGS);
 }
 EXPORT_SYMBOL(of_count_phandle_with_args);
 
diff --git a/include/linux/of.h b/include/linux/of.h
index edc068d..cd9f225 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -75,6 +75,10 @@ struct of_phandle_args {
 	uint32_t args[MAX_PHANDLE_ARGS];
 };
 
+/* Use these if you need more that MAX_PHANDLE_ARGS */
+extern struct of_phandle_args *of_alloc_phandle_args(int size);
+extern void of_free_phandle_args(struct of_phandle_args *args);
+
 struct of_reconfig_data {
 	struct device_node	*dn;
 	struct property		*prop;
@@ -327,6 +331,9 @@ extern struct device_node *of_parse_phandle(const struct device_node *np,
 extern int of_parse_phandle_with_args(const struct device_node *np,
 	const char *list_name, const char *cells_name, int index,
 	struct of_phandle_args *out_args);
+extern int of_parse_phandle_with_var_args(const struct device_node *np,
+	const char *list_name, const char *cells_name, int index,
+	struct of_phandle_args *out_args, int elems);
 extern int of_parse_phandle_with_fixed_args(const struct device_node *np,
 	const char *list_name, int cells_count, int index,
 	struct of_phandle_args *out_args);
-- 
1.8.4.5

[PATCH 2/2] arm/smmu: Make use of of_parse_phandle_with_var_args

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

The function of_parse_phandle_with_args() can only handle 16
args, but there are systems that require more (25 in my
case). So use the newly introduced function
of_parse_phandle_with_var_args() instead.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/arm-smmu.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 4cd0c29..1fe7f35 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -47,7 +47,7 @@
 #include "io-pgtable.h"
 
 /* Maximum number of stream IDs assigned to a single device */
-#define MAX_MASTER_STREAMIDS		MAX_PHANDLE_ARGS
+#define MAX_MASTER_STREAMIDS		128
 
 /* Maximum number of context banks per SMMU */
 #define ARM_SMMU_MAX_CBS		128
@@ -1698,7 +1698,7 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 	struct arm_smmu_device *smmu;
 	struct device *dev = &pdev->dev;
 	struct rb_node *node;
-	struct of_phandle_args masterspec;
+	struct of_phandle_args *masterspec;
 	int num_irqs, i, err;
 
 	smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
@@ -1757,15 +1757,19 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	masterspec = of_alloc_phandle_args(MAX_MASTER_STREAMIDS);
+	if (!masterspec)
+		return -ENOMEM;
+
 	i = 0;
 	smmu->masters = RB_ROOT;
-	while (!of_parse_phandle_with_args(dev->of_node, "mmu-masters",
-					   "#stream-id-cells", i,
-					   &masterspec)) {
-		err = register_smmu_master(smmu, dev, &masterspec);
+	while (!of_parse_phandle_with_var_args(dev->of_node, "mmu-masters",
+					   "#stream-id-cells", i, masterspec,
+					   MAX_MASTER_STREAMIDS)) {
+		err = register_smmu_master(smmu, dev, masterspec);
 		if (err) {
 			dev_err(dev, "failed to add master %s\n",
-				masterspec.np->name);
+				masterspec->np->name);
 			goto out_put_masters;
 		}
 
@@ -1797,6 +1801,8 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 		}
 	}
 
+	of_free_phandle_args(masterspec);
+
 	INIT_LIST_HEAD(&smmu->list);
 	spin_lock(&arm_smmu_devices_lock);
 	list_add(&smmu->list, &arm_smmu_devices);
@@ -1810,6 +1816,9 @@ out_free_irqs:
 		free_irq(smmu->irqs[i], smmu);
 
 out_put_masters:
+
+	of_free_phandle_args(masterspec);
+
 	for (node = rb_first(&smmu->masters); node; node = rb_next(node)) {
 		struct arm_smmu_master *master
 			= container_of(node, struct arm_smmu_master, node);
-- 
1.8.4.5

[PATCH 0/2 v2] Fix MAX_PHANDLE_ARGS limitations

[Joerg Roedel]

From: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>

Hi,

here is the second version of my patches to fix the
MAX_PHANDLE_ARGS limitation for the arm-smmu driver. On my
AMD Seattle system the max value of 16 is not enough to
initialize the SMMU, as the device tree node has an entry
with 25 phandle args (possible are up to 128). The result is
a WARNING and incorrect SMMU initialization.

On the other side MAX_PHANDLE_ARGS can't grow indefinitly,
as it causes 'struct of_phandle_args' to get bigger. This
struct is often allocated on-stack and should be small by
default.

So these patches introduce a way to make 'struct
of_phandle_args' bigger only when needed and change the
ARM-SMMU driver to make use of it.

The patches have been tested on my AMD Seattle system.

Please review.

Thanks,

	Joerg

Changes to v1:

	* Rebased to v4.2-rc2
	* Fixed leak in patch 2, as pointed out by Will
	  Deacon

Joerg Roedel (2):
  of: base: Allow more args than MAX_PHANDLE_ARGS if required
  arm/smmu: Make use of of_parse_phandle_with_var_args

 drivers/iommu/arm-smmu.c | 23 ++++++++++++++++-------
 drivers/of/base.c        | 43 ++++++++++++++++++++++++++++++++++++-------
 include/linux/of.h       |  7 +++++++
 3 files changed, 59 insertions(+), 14 deletions(-)

-- 
1.8.4.5

[PATCH 2/2] arm/smmu: Make use of of_parse_phandle_with_var_args

[Joerg Roedel]

From: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>

The function of_parse_phandle_with_args() can only handle 16
args, but there are systems that require more (25 in my
case). So use the newly introduced function
of_parse_phandle_with_var_args() instead.

Signed-off-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
---
 drivers/iommu/arm-smmu.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 4cd0c29..1fe7f35 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -47,7 +47,7 @@
 #include "io-pgtable.h"
 
 /* Maximum number of stream IDs assigned to a single device */
-#define MAX_MASTER_STREAMIDS		MAX_PHANDLE_ARGS
+#define MAX_MASTER_STREAMIDS		128
 
 /* Maximum number of context banks per SMMU */
 #define ARM_SMMU_MAX_CBS		128
@@ -1698,7 +1698,7 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 	struct arm_smmu_device *smmu;
 	struct device *dev = &pdev->dev;
 	struct rb_node *node;
-	struct of_phandle_args masterspec;
+	struct of_phandle_args *masterspec;
 	int num_irqs, i, err;
 
 	smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
@@ -1757,15 +1757,19 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	masterspec = of_alloc_phandle_args(MAX_MASTER_STREAMIDS);
+	if (!masterspec)
+		return -ENOMEM;
+
 	i = 0;
 	smmu->masters = RB_ROOT;
-	while (!of_parse_phandle_with_args(dev->of_node, "mmu-masters",
-					   "#stream-id-cells", i,
-					   &masterspec)) {
-		err = register_smmu_master(smmu, dev, &masterspec);
+	while (!of_parse_phandle_with_var_args(dev->of_node, "mmu-masters",
+					   "#stream-id-cells", i, masterspec,
+					   MAX_MASTER_STREAMIDS)) {
+		err = register_smmu_master(smmu, dev, masterspec);
 		if (err) {
 			dev_err(dev, "failed to add master %s\n",
-				masterspec.np->name);
+				masterspec->np->name);
 			goto out_put_masters;
 		}
 
@@ -1797,6 +1801,8 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 		}
 	}
 
+	of_free_phandle_args(masterspec);
+
 	INIT_LIST_HEAD(&smmu->list);
 	spin_lock(&arm_smmu_devices_lock);
 	list_add(&smmu->list, &arm_smmu_devices);
@@ -1810,6 +1816,9 @@ out_free_irqs:
 		free_irq(smmu->irqs[i], smmu);
 
 out_put_masters:
+
+	of_free_phandle_args(masterspec);
+
 	for (node = rb_first(&smmu->masters); node; node = rb_next(node)) {
 		struct arm_smmu_master *master
 			= container_of(node, struct arm_smmu_master, node);
-- 
1.9.1

Re: [PATCH 2/2] arm/smmu: Make use of of_parse_phandle_with_var_args

[Will Deacon]

Hi Joerg,

On Thu, Jul 16, 2015 at 09:30:44AM +0100, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
> 
> The function of_parse_phandle_with_args() can only handle 16
> args, but there are systems that require more (25 in my
> case). So use the newly introduced function
> of_parse_phandle_with_var_args() instead.

Looks good to me:

  Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>

Thanks!

Will
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH 1/2] of: base: Allow more args than MAX_PHANDLE_ARGS if required

[Joerg Roedel]

From: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>

The main use of MAX_PHANDLE_ARGS is to define the number of
args elements in 'struct of_phandle_args'. This struct is
often declared on the stack and thus it is impractical to
increase MAX_PHANDLE_ARGS again and again.

To handle situations where more than MAX_PHANDLE_ARGS
elements may appear in a device-tree, introduce functions
to allocate/free 'struct of_phandle_args' with more than
MAX_PHANDLE_ARGS elements and provide the new function
of_parse_phandle_with_var_args(), which can handle those
variable-size structs.

This is necessary for the ARM-SMMU driver, where the number
of mmu-masters can be up to 128.

Signed-off-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
---
 drivers/of/base.c  | 43 ++++++++++++++++++++++++++++++++++++-------
 include/linux/of.h |  7 +++++++
 2 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index f065026..1cba334 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -54,6 +54,24 @@ DEFINE_MUTEX(of_mutex);
  */
 DEFINE_RAW_SPINLOCK(devtree_lock);
 
+struct of_phandle_args *of_alloc_phandle_args(int size)
+{
+	struct of_phandle_args *args;
+	int e = max(0, size - MAX_PHANDLE_ARGS);
+
+	args =  kzalloc(sizeof(struct of_phandle_args) + e * sizeof(uint32_t),
+			GFP_KERNEL);
+
+	return args;
+}
+EXPORT_SYMBOL(of_alloc_phandle_args);
+
+void of_free_phandle_args(struct of_phandle_args *args)
+{
+	kfree(args);
+}
+EXPORT_SYMBOL(of_free_phandle_args);
+
 int of_n_addr_cells(struct device_node *np)
 {
 	const __be32 *ip;
@@ -1446,7 +1464,8 @@ static int __of_parse_phandle_with_args(const struct device_node *np,
 					const char *list_name,
 					const char *cells_name,
 					int cell_count, int index,
-					struct of_phandle_args *out_args)
+					struct of_phandle_args *out_args,
+					int elems)
 {
 	const __be32 *list, *list_end;
 	int rc = 0, size, cur_index = 0;
@@ -1525,8 +1544,8 @@ static int __of_parse_phandle_with_args(const struct device_node *np,
 
 			if (out_args) {
 				int i;
-				if (WARN_ON(count > MAX_PHANDLE_ARGS))
-					count = MAX_PHANDLE_ARGS;
+				if (WARN_ON(count > elems))
+					count = elems;
 				out_args->np = node;
 				out_args->args_count = count;
 				for (i = 0; i < count; i++)
@@ -1577,7 +1596,7 @@ struct device_node *of_parse_phandle(const struct device_node *np,
 		return NULL;
 
 	if (__of_parse_phandle_with_args(np, phandle_name, NULL, 0,
-					 index, &args))
+					 index, &args, MAX_PHANDLE_ARGS))
 		return NULL;
 
 	return args.np;
@@ -1623,10 +1642,20 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
 	if (index < 0)
 		return -EINVAL;
 	return __of_parse_phandle_with_args(np, list_name, cells_name, 0,
-					    index, out_args);
+					    index, out_args, MAX_PHANDLE_ARGS);
 }
 EXPORT_SYMBOL(of_parse_phandle_with_args);
 
+int of_parse_phandle_with_var_args(const struct device_node *np, const char *list_name,
+				   const char *cells_name, int index,
+				   struct of_phandle_args *out_args, int elems)
+{
+	if (index < 0)
+		return -EINVAL;
+	return __of_parse_phandle_with_args(np, list_name, cells_name, 0,
+					    index, out_args, elems);
+}
+EXPORT_SYMBOL(of_parse_phandle_with_var_args);
 /**
  * of_parse_phandle_with_fixed_args() - Find a node pointed by phandle in a list
  * @np:		pointer to a device tree node containing a list
@@ -1664,7 +1693,7 @@ int of_parse_phandle_with_fixed_args(const struct device_node *np,
 	if (index < 0)
 		return -EINVAL;
 	return __of_parse_phandle_with_args(np, list_name, NULL, cell_count,
-					   index, out_args);
+					   index, out_args, MAX_PHANDLE_ARGS);
 }
 EXPORT_SYMBOL(of_parse_phandle_with_fixed_args);
 
@@ -1687,7 +1716,7 @@ int of_count_phandle_with_args(const struct device_node *np, const char *list_na
 				const char *cells_name)
 {
 	return __of_parse_phandle_with_args(np, list_name, cells_name, 0, -1,
-					    NULL);
+					    NULL, MAX_PHANDLE_ARGS);
 }
 EXPORT_SYMBOL(of_count_phandle_with_args);
 
diff --git a/include/linux/of.h b/include/linux/of.h
index b871ff9..afb7893 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -75,6 +75,10 @@ struct of_phandle_args {
 	uint32_t args[MAX_PHANDLE_ARGS];
 };
 
+/* Use these if you need more that MAX_PHANDLE_ARGS */
+extern struct of_phandle_args *of_alloc_phandle_args(int size);
+extern void of_free_phandle_args(struct of_phandle_args *args);
+
 struct of_reconfig_data {
 	struct device_node	*dn;
 	struct property		*prop;
@@ -327,6 +331,9 @@ extern struct device_node *of_parse_phandle(const struct device_node *np,
 extern int of_parse_phandle_with_args(const struct device_node *np,
 	const char *list_name, const char *cells_name, int index,
 	struct of_phandle_args *out_args);
+extern int of_parse_phandle_with_var_args(const struct device_node *np,
+	const char *list_name, const char *cells_name, int index,
+	struct of_phandle_args *out_args, int elems);
 extern int of_parse_phandle_with_fixed_args(const struct device_node *np,
 	const char *list_name, int cells_count, int index,
 	struct of_phandle_args *out_args);
-- 
1.9.1

[PATCH 2/2] arm/smmu: Make use of of_parse_phandle_with_var_args

[Joerg Roedel]

From: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>

The function of_parse_phandle_with_args() can only handle 16
args, but there are systems that require more (25 in my
case). So use the newly introduced function
of_parse_phandle_with_var_args() instead.

Signed-off-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
---
 drivers/iommu/arm-smmu.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 66a803b..da4d7ac 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -47,7 +47,7 @@
 #include "io-pgtable.h"
 
 /* Maximum number of stream IDs assigned to a single device */
-#define MAX_MASTER_STREAMIDS		MAX_PHANDLE_ARGS
+#define MAX_MASTER_STREAMIDS		128
 
 /* Maximum number of context banks per SMMU */
 #define ARM_SMMU_MAX_CBS		128
@@ -1699,7 +1699,7 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 	struct arm_smmu_device *smmu;
 	struct device *dev = &pdev->dev;
 	struct rb_node *node;
-	struct of_phandle_args masterspec;
+	struct of_phandle_args *masterspec;
 	int num_irqs, i, err;
 
 	smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
@@ -1758,15 +1758,19 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	masterspec = of_alloc_phandle_args(MAX_MASTER_STREAMIDS);
+	if (!masterspec)
+		return -ENOMEM;
+
 	i = 0;
 	smmu->masters = RB_ROOT;
-	while (!of_parse_phandle_with_args(dev->of_node, "mmu-masters",
-					   "#stream-id-cells", i,
-					   &masterspec)) {
-		err = register_smmu_master(smmu, dev, &masterspec);
+	while (!of_parse_phandle_with_var_args(dev->of_node, "mmu-masters",
+					   "#stream-id-cells", i, masterspec,
+					   MAX_MASTER_STREAMIDS)) {
+		err = register_smmu_master(smmu, dev, masterspec);
 		if (err) {
 			dev_err(dev, "failed to add master %s\n",
-				masterspec.np->name);
+				masterspec->np->name);
 			goto out_put_masters;
 		}
 
@@ -1811,6 +1815,9 @@ out_free_irqs:
 		free_irq(smmu->irqs[i], smmu);
 
 out_put_masters:
+
+	of_free_phandle_args(masterspec);
+
 	for (node = rb_first(&smmu->masters); node; node = rb_next(node)) {
 		struct arm_smmu_master *master
 			= container_of(node, struct arm_smmu_master, node);
-- 
1.9.1

Re: [PATCH 2/2] arm/smmu: Make use of of_parse_phandle_with_var_args

[Will Deacon]

Hi Joerg,

Thanks for looking at this! I'm fine with the general idea, but obviously
the first patch needs an Ack from a devicetree person.

One comment on the code below...

On Thu, Jun 25, 2015 at 04:52:28PM +0100, Joerg Roedel wrote:
> The function of_parse_phandle_with_args() can only handle 16
> args, but there are systems that require more (25 in my
> case). So use the newly introduced function
> of_parse_phandle_with_var_args() instead.

[...]

> +	masterspec = of_alloc_phandle_args(MAX_MASTER_STREAMIDS);
> +	if (!masterspec)
> +		return -ENOMEM;
> +
>  	i = 0;
>  	smmu->masters = RB_ROOT;
> -	while (!of_parse_phandle_with_args(dev->of_node, "mmu-masters",
> -					   "#stream-id-cells", i,
> -					   &masterspec)) {
> -		err = register_smmu_master(smmu, dev, &masterspec);
> +	while (!of_parse_phandle_with_var_args(dev->of_node, "mmu-masters",
> +					   "#stream-id-cells", i, masterspec,
> +					   MAX_MASTER_STREAMIDS)) {
> +		err = register_smmu_master(smmu, dev, masterspec);
>  		if (err) {
>  			dev_err(dev, "failed to add master %s\n",
> -				masterspec.np->name);
> +				masterspec->np->name);
>  			goto out_put_masters;
>  		}
>  
> @@ -1811,6 +1815,9 @@ out_free_irqs:
>  		free_irq(smmu->irqs[i], smmu);
>  
>  out_put_masters:
> +
> +	of_free_phandle_args(masterspec);

Shouldn't we also free the masterspec on success?

Will
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] arm/smmu: Make use of of_parse_phandle_with_var_args

[Joerg Roedel]

Hi Will,

On Fri, Jun 26, 2015 at 12:10:47PM +0100, Will Deacon wrote:
> Thanks for looking at this! I'm fine with the general idea, but obviously
> the first patch needs an Ack from a devicetree person.

Yeah, sure. I was looking into a simple solution that does not require
to rewrite all the callers. Lets see what the device-tree maintainers
think about it :)

> >  out_put_masters:
> > +
> > +	of_free_phandle_args(masterspec);
> 
> Shouldn't we also free the masterspec on success?

Right, thanks. This code is only the error path. I'll add this in the
next version.


	Joerg