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

[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