Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1362884 > unrolled thread

[PATCH 0/6] of: Implement iterator for phandles

Started byJoerg Roedel <joro@8bytes.org>
First post2016-03-22 19:00 +0100
Last post2016-03-23 21:40 +0100
Articles 16 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/6] of: Implement iterator for phandles Joerg Roedel <joro@8bytes.org> - 2016-03-22 19:00 +0100
    [PATCH 3/6] of: Remove counting special case from __of_parse_phandle_with_args() Joerg Roedel <joro@8bytes.org> - 2016-03-22 19:00 +0100
    [PATCH 4/6] of: Introduce of_for_each_phandle() helper macro Joerg Roedel <joro@8bytes.org> - 2016-03-22 19:00 +0100
    [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in device-tree parsing Joerg Roedel <joro@8bytes.org> - 2016-03-22 19:00 +0100
      Re: [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in  device-tree parsing Rob Herring <robh+dt@kernel.org> - 2016-03-22 19:40 +0100
        [PATCH v2] iommu/arm-smmu: Make use of phandle iterators in  device-tree device-tree parsing Joerg Roedel <joro@8bytes.org> - 2016-03-23 12:50 +0100
          Re: [PATCH v2] iommu/arm-smmu: Make use of phandle iterators in  device-tree device-tree parsing kbuild test robot <lkp@intel.com> - 2016-03-23 16:20 +0100
      Re: [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in  device-tree parsing Robin Murphy <robin.murphy@arm.com> - 2016-03-22 20:00 +0100
        Re: [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in  device-tree parsing Joerg Roedel <joro@8bytes.org> - 2016-03-23 13:00 +0100
          Re: [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in  device-tree parsing Will Deacon <will.deacon@arm.com> - 2016-03-29 19:30 +0200
      Re: [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in  device-tree parsing Will Deacon <will.deacon@arm.com> - 2016-03-29 19:30 +0200
    [PATCH 2/6] of: Move phandle walking to of_phandle_iterator_next() Joerg Roedel <joro@8bytes.org> - 2016-03-22 19:10 +0100
    [PATCH 1/6] of: Introduce struct of_phandle_iterator Joerg Roedel <joro@8bytes.org> - 2016-03-22 19:10 +0100
    Re: [PATCH 0/6] of: Implement iterator for phandles Rob Herring <robh+dt@kernel.org> - 2016-03-22 19:50 +0100
      Re: [PATCH 0/6] of: Implement iterator for phandles Joerg Roedel <joro@8bytes.org> - 2016-03-23 13:00 +0100
        Re: [PATCH 0/6] of: Implement iterator for phandles Rob Herring <robh+dt@kernel.org> - 2016-03-23 21:40 +0100

#1362884 — [PATCH 0/6] of: Implement iterator for phandles

FromJoerg Roedel <joro@8bytes.org>
Date2016-03-22 19:00 +0100
Subject[PATCH 0/6] of: Implement iterator for phandles
Message-ID<rfyKZ-88A-7@gated-at.bofh.it>
Hi,

here is an implementation of the iterator over phandles
concept which Rob Herring suggested to me some time ago. My
approach is a little bit different from what the diff showed
back then, but it gets rid of the allocation and 'struct
'struct of_phandle_args' misuse.

I also converted the arm-smmu driver to make use of the
iterator. The resulting kernel boots on my AMD Seattle
system and fixes the warning triggered there. The patches
now also pass all dt-unittests in my kvm environment.

Please review. Patches are based on v4.5.

Thanks,

	Joerg

Changes since RFC post:

* Reordered members of 'struct of_phandle_iterator' and did
  some renaming

* Removed index counting from the iterator

* Split up iterator implementation into multiple patches

* Fixed the code to survive all dt-unittests, tested with each
  patch in this series

* Re-added and updated some comments which got lost during the
  conversion.

* Added of_for_each_phandle macro for easier handling

* Moved the counting special-case from __of_parse_phandle_with_args
  directly to of_count_phandle_with_args for code
  simplification

* Removed some iterator helper functions

* Formatting and style changes

Joerg Roedel (6):
  of: Introduce struct of_phandle_iterator
  of: Move phandle walking to of_phandle_iterator_next()
  of: Remove counting special case from __of_parse_phandle_with_args()
  of: Introduce of_for_each_phandle() helper macro
  of: Introduce of_phandle_iterator_args()
  iommu/arm-smmu: Make use of phandle iterators in device-tree parsing

 drivers/iommu/arm-smmu.c |  28 +++++--
 drivers/of/base.c        | 206 ++++++++++++++++++++++++++++++-----------------
 include/linux/of.h       |  56 +++++++++++++
 3 files changed, 211 insertions(+), 79 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]


#1362887 — [PATCH 3/6] of: Remove counting special case from __of_parse_phandle_with_args()

FromJoerg Roedel <joro@8bytes.org>
Date2016-03-22 19:00 +0100
Subject[PATCH 3/6] of: Remove counting special case from __of_parse_phandle_with_args()
Message-ID<rfyL0-88A-21@gated-at.bofh.it>
In reply to#1362884
From: Joerg Roedel <jroedel@suse.de>

The index = -1 case in __of_parse_phandle_with_args() is
used to just return the number of phandles. That special
case needs extra handling, so move it to the place where it
is needed: of_count_phandle_with_args().

This allows to further simplify __of_parse_phandle_with_args()
later on.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/of/base.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 4036512..15593cd 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1583,10 +1583,7 @@ static int __of_parse_phandle_with_args(const struct device_node *np,
 	 * Unlock node before returning result; will be one of:
 	 * -ENOENT : index is for empty phandle
 	 * -EINVAL : parsing error on data
-	 * [1..n]  : Number of phandle (count mode; when index = -1)
 	 */
-	if (rc == -ENOENT && index < 0)
-		rc = cur_index;
 
  err:
 	if (it.node)
@@ -1722,8 +1719,20 @@ EXPORT_SYMBOL(of_parse_phandle_with_fixed_args);
 int of_count_phandle_with_args(const struct device_node *np, const char *list_name,
 				const char *cells_name)
 {
-	return __of_parse_phandle_with_args(np, list_name, cells_name, 0, -1,
-					    NULL);
+	struct of_phandle_iterator it;
+	int rc, cur_index = 0;
+
+	rc = of_phandle_iterator_init(&it, np, list_name, cells_name, 0);
+	if (rc)
+		return rc;
+
+	while ((rc = of_phandle_iterator_next(&it)) == 0)
+		cur_index += 1;
+
+	if (rc != -ENOENT)
+		return rc;
+
+	return cur_index;
 }
 EXPORT_SYMBOL(of_count_phandle_with_args);
 
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]


#1362891 — [PATCH 4/6] of: Introduce of_for_each_phandle() helper macro

FromJoerg Roedel <joro@8bytes.org>
Date2016-03-22 19:00 +0100
Subject[PATCH 4/6] of: Introduce of_for_each_phandle() helper macro
Message-ID<rfyL0-88A-23@gated-at.bofh.it>
In reply to#1362884
From: Joerg Roedel <jroedel@suse.de>

With this macro any user can easily iterate over a list of
phandles. The patch also converts __of_parse_phandle_with_args()
to make use of the macro.

The of_count_phandle_with_args() function is not converted,
because the macro hides the return value of of_phandle_iterator_init(),
which is needed in there.

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

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 15593cd..471d3d9 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1542,13 +1542,8 @@ static int __of_parse_phandle_with_args(const struct device_node *np,
 	struct of_phandle_iterator it;
 	int rc, cur_index = 0;
 
-	rc = of_phandle_iterator_init(&it, np, list_name,
-				      cells_name, cell_count);
-	if (rc)
-		return rc;
-
 	/* Loop over the phandles until all the requested entry is found */
-	while ((rc = of_phandle_iterator_next(&it)) == 0) {
+	of_for_each_phandle(&it, rc, np, list_name, cells_name, cell_count) {
 		/*
 		 * All of the error cases bail out of the loop, so at
 		 * this point, the parsing is successful. If the requested
diff --git a/include/linux/of.h b/include/linux/of.h
index d94388b0..05e38ed 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -908,6 +908,12 @@ static inline int of_property_read_s32(const struct device_node *np,
 	return of_property_read_u32(np, propname, (u32*) out_value);
 }
 
+#define of_for_each_phandle(it, err, np, ln, cn, cc)			\
+	for (of_phandle_iterator_init((it), (np), (ln), (cn), (cc)),	\
+	     err = of_phandle_iterator_next(it);			\
+	     err == 0;							\
+	     err = of_phandle_iterator_next(it))
+
 #define of_property_for_each_u32(np, propname, prop, p, u)	\
 	for (prop = of_find_property(np, propname, NULL),	\
 		p = of_prop_next_u32(prop, NULL, &u);		\
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]


#1362894 — [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in device-tree parsing

FromJoerg Roedel <joro@8bytes.org>
Date2016-03-22 19:00 +0100
Subject[PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in device-tree parsing
Message-ID<rfyL0-88A-35@gated-at.bofh.it>
In reply to#1362884
From: Joerg Roedel <jroedel@suse.de>

Remove the usage of of_parse_phandle_with_args() and replace
it by the phandle-iterator implementation so that we can
parse out all of the potentially present 128 stream-ids.

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

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 59ee4b8..413bd64 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -48,7 +48,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
@@ -349,6 +349,12 @@ struct arm_smmu_domain {
 	struct iommu_domain		domain;
 };
 
+struct arm_smmu_phandle_args {
+	struct device_node *np;
+	int args_count;
+	uint32_t args[MAX_MASTER_STREAMIDS];
+};
+
 static struct iommu_ops arm_smmu_ops;
 
 static DEFINE_SPINLOCK(arm_smmu_devices_lock);
@@ -458,7 +464,7 @@ static int insert_smmu_master(struct arm_smmu_device *smmu,
 
 static int register_smmu_master(struct arm_smmu_device *smmu,
 				struct device *dev,
-				struct of_phandle_args *masterspec)
+				struct arm_smmu_phandle_args *masterspec)
 {
 	int i;
 	struct arm_smmu_master *master;
@@ -1716,7 +1722,8 @@ 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_iterator it;
+	struct arm_smmu_phandle_args masterspec;
 	int num_irqs, i, err;
 
 	smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
@@ -1777,9 +1784,14 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 
 	i = 0;
 	smmu->masters = RB_ROOT;
-	while (!of_parse_phandle_with_args(dev->of_node, "mmu-masters",
-					   "#stream-id-cells", i,
-					   &masterspec)) {
+
+	of_for_each_phandle(&it, err, dev->of_node,
+			    "mmu-masters", "#stream-id-cells", 0) {
+		int count = of_phandle_iterator_args(&it, masterspec.args,
+						     MAX_MASTER_STREAMIDS);
+		masterspec.np		= of_node_get(it.node);
+		masterspec.args_count	= count;
+
 		err = register_smmu_master(smmu, dev, &masterspec);
 		if (err) {
 			dev_err(dev, "failed to add master %s\n",
@@ -1789,6 +1801,10 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 
 		i++;
 	}
+
+	if (i == 0)
+		goto out_put_masters;
+
 	dev_notice(dev, "registered %d master devices\n", i);
 
 	parse_driver_options(smmu);
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]


#1362908 — Re: [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in device-tree parsing

FromRob Herring <robh+dt@kernel.org>
Date2016-03-22 19:40 +0100
SubjectRe: [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in device-tree parsing
Message-ID<rfznI-af-9@gated-at.bofh.it>
In reply to#1362894
On Tue, Mar 22, 2016 at 12:58 PM, Joerg Roedel <joro@8bytes.org> wrote:
> From: Joerg Roedel <jroedel@suse.de>
>
> Remove the usage of of_parse_phandle_with_args() and replace
> it by the phandle-iterator implementation so that we can
> parse out all of the potentially present 128 stream-ids.
>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/iommu/arm-smmu.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 59ee4b8..413bd64 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -48,7 +48,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
> @@ -349,6 +349,12 @@ struct arm_smmu_domain {
>         struct iommu_domain             domain;
>  };
>
> +struct arm_smmu_phandle_args {
> +       struct device_node *np;
> +       int args_count;
> +       uint32_t args[MAX_MASTER_STREAMIDS];
> +};
> +
>  static struct iommu_ops arm_smmu_ops;
>
>  static DEFINE_SPINLOCK(arm_smmu_devices_lock);
> @@ -458,7 +464,7 @@ static int insert_smmu_master(struct arm_smmu_device *smmu,
>
>  static int register_smmu_master(struct arm_smmu_device *smmu,
>                                 struct device *dev,
> -                               struct of_phandle_args *masterspec)
> +                               struct arm_smmu_phandle_args *masterspec)
>  {
>         int i;
>         struct arm_smmu_master *master;
> @@ -1716,7 +1722,8 @@ 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_iterator it;
> +       struct arm_smmu_phandle_args masterspec;

Isn't this a bit big to put on the stack being ~512 bytes?

>         int num_irqs, i, err;
>
>         smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);

[toc] | [prev] | [next] | [standalone]


#1363370 — [PATCH v2] iommu/arm-smmu: Make use of phandle iterators in device-tree device-tree parsing

FromJoerg Roedel <joro@8bytes.org>
Date2016-03-23 12:50 +0100
Subject[PATCH v2] iommu/arm-smmu: Make use of phandle iterators in device-tree device-tree parsing
Message-ID<rfPsu-2Nq-11@gated-at.bofh.it>
In reply to#1362908
On Tue, Mar 22, 2016 at 01:38:06PM -0500, Rob Herring wrote:
> > +       struct of_phandle_iterator it;
> > +       struct arm_smmu_phandle_args masterspec;
> 
> Isn't this a bit big to put on the stack being ~512 bytes?

Yeah, you might be right. I havn't seen any problems booting with this
being allocated on the stack, but to be on the safe side I changed the
patch to allocate the masterspec with kmalloc, patch below.


	Joerg

From 85995d1edea7f61aae3c31e0fbd3258622d0b5ae Mon Sep 17 00:00:00 2001
From: Joerg Roedel <jroedel@suse.de>
Date: Wed, 16 Mar 2016 17:10:10 +0100
Subject: [PATCH] iommu/arm-smmu: Make use of phandle iterators in device-tree
 parsing

Remove the usage of of_parse_phandle_with_args() and replace
it by the phandle-iterator implementation so that we can
parse out all of the potentially present 128 stream-ids.

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

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 59ee4b8..fa9b98c 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -48,7 +48,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
@@ -349,6 +349,12 @@ struct arm_smmu_domain {
 	struct iommu_domain		domain;
 };
 
+struct arm_smmu_phandle_args {
+	struct device_node *np;
+	int args_count;
+	uint32_t args[MAX_MASTER_STREAMIDS];
+};
+
 static struct iommu_ops arm_smmu_ops;
 
 static DEFINE_SPINLOCK(arm_smmu_devices_lock);
@@ -458,7 +464,7 @@ static int insert_smmu_master(struct arm_smmu_device *smmu,
 
 static int register_smmu_master(struct arm_smmu_device *smmu,
 				struct device *dev,
-				struct of_phandle_args *masterspec)
+				struct arm_smmu_phandle_args *masterspec)
 {
 	int i;
 	struct arm_smmu_master *master;
@@ -1716,7 +1722,8 @@ 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_iterator it;
+	struct arm_smmu_phandle_args *masterspec;
 	int num_irqs, i, err;
 
 	smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
@@ -1777,20 +1784,38 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 
 	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);
+
+	err = -ENOMEM;
+	/* No need to zero the memory for masterspec */
+	masterspec = kmalloc(sizeof(*masterspec), GFP_KERNEL);
+	if (!masterspec)
+		goto out_put_masters;
+
+	of_for_each_phandle(&it, err, dev->of_node,
+			    "mmu-masters", "#stream-id-cells", 0) {
+		int count = of_phandle_iterator_args(&it, masterspec->args,
+						     MAX_MASTER_STREAMIDS);
+		masterspec->np		= of_node_get(it.node);
+		masterspec->args_count	= count;
+
+		err = register_smmu_master(smmu, dev, masterspec);
 		if (err) {
 			dev_err(dev, "failed to add master %s\n",
-				masterspec.np->name);
+				masterspec->np->name);
+			kfree(masterspec);
 			goto out_put_masters;
 		}
 
 		i++;
 	}
+
+	if (i == 0)
+		goto out_put_masters;
+
 	dev_notice(dev, "registered %d master devices\n", i);
 
+	kfree(masterspec);
+
 	parse_driver_options(smmu);
 
 	if (smmu->version > ARM_SMMU_V1 &&
-- 
2.4.3

[toc] | [prev] | [next] | [standalone]


#1363489 — Re: [PATCH v2] iommu/arm-smmu: Make use of phandle iterators in device-tree device-tree parsing

Fromkbuild test robot <lkp@intel.com>
Date2016-03-23 16:20 +0100
SubjectRe: [PATCH v2] iommu/arm-smmu: Make use of phandle iterators in device-tree device-tree parsing
Message-ID<rfSJI-5g0-13@gated-at.bofh.it>
In reply to#1363370

[Multipart message — attachments visible in raw view] — view raw

Hi Joerg,

[auto build test ERROR on iommu/next]
[also build test ERROR on v4.5 next-20160323]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Joerg-Roedel/iommu-arm-smmu-Make-use-of-phandle-iterators-in-device-tree-device-tree-parsing/20160323-194824
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: arm64-defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   drivers/iommu/arm-smmu.c: In function 'arm_smmu_device_dt_probe':
   drivers/iommu/arm-smmu.c:1746:29: error: storage size of 'it' isn't known
     struct of_phandle_iterator it;
                                ^
>> drivers/iommu/arm-smmu.c:1815:2: error: implicit declaration of function 'of_for_each_phandle' [-Werror=implicit-function-declaration]
     of_for_each_phandle(&it, err, dev->of_node,
     ^
>> drivers/iommu/arm-smmu.c:1816:46: error: expected ';' before '{' token
           "mmu-masters", "#stream-id-cells", 0) {
                                                 ^
   drivers/iommu/arm-smmu.c:1746:29: warning: unused variable 'it' [-Wunused-variable]
     struct of_phandle_iterator it;
                                ^
   drivers/iommu/arm-smmu.c: At top level:
   drivers/iommu/arm-smmu.c:473:12: warning: 'register_smmu_master' defined but not used [-Wunused-function]
    static int register_smmu_master(struct arm_smmu_device *smmu,
               ^
   cc1: some warnings being treated as errors

vim +/of_for_each_phandle +1815 drivers/iommu/arm-smmu.c

  1740	{
  1741		const struct of_device_id *of_id;
  1742		struct resource *res;
  1743		struct arm_smmu_device *smmu;
  1744		struct device *dev = &pdev->dev;
  1745		struct rb_node *node;
> 1746		struct of_phandle_iterator it;
  1747		struct arm_smmu_phandle_args *masterspec;
  1748		int num_irqs, i, err;
  1749	
  1750		smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
  1751		if (!smmu) {
  1752			dev_err(dev, "failed to allocate arm_smmu_device\n");
  1753			return -ENOMEM;
  1754		}
  1755		smmu->dev = dev;
  1756	
  1757		of_id = of_match_node(arm_smmu_of_match, dev->of_node);
  1758		smmu->version = (enum arm_smmu_arch_version)of_id->data;
  1759	
  1760		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
  1761		smmu->base = devm_ioremap_resource(dev, res);
  1762		if (IS_ERR(smmu->base))
  1763			return PTR_ERR(smmu->base);
  1764		smmu->size = resource_size(res);
  1765	
  1766		if (of_property_read_u32(dev->of_node, "#global-interrupts",
  1767					 &smmu->num_global_irqs)) {
  1768			dev_err(dev, "missing #global-interrupts property\n");
  1769			return -ENODEV;
  1770		}
  1771	
  1772		num_irqs = 0;
  1773		while ((res = platform_get_resource(pdev, IORESOURCE_IRQ, num_irqs))) {
  1774			num_irqs++;
  1775			if (num_irqs > smmu->num_global_irqs)
  1776				smmu->num_context_irqs++;
  1777		}
  1778	
  1779		if (!smmu->num_context_irqs) {
  1780			dev_err(dev, "found %d interrupts but expected at least %d\n",
  1781				num_irqs, smmu->num_global_irqs + 1);
  1782			return -ENODEV;
  1783		}
  1784	
  1785		smmu->irqs = devm_kzalloc(dev, sizeof(*smmu->irqs) * num_irqs,
  1786					  GFP_KERNEL);
  1787		if (!smmu->irqs) {
  1788			dev_err(dev, "failed to allocate %d irqs\n", num_irqs);
  1789			return -ENOMEM;
  1790		}
  1791	
  1792		for (i = 0; i < num_irqs; ++i) {
  1793			int irq = platform_get_irq(pdev, i);
  1794	
  1795			if (irq < 0) {
  1796				dev_err(dev, "failed to get irq index %d\n", i);
  1797				return -ENODEV;
  1798			}
  1799			smmu->irqs[i] = irq;
  1800		}
  1801	
  1802		err = arm_smmu_device_cfg_probe(smmu);
  1803		if (err)
  1804			return err;
  1805	
  1806		i = 0;
  1807		smmu->masters = RB_ROOT;
  1808	
  1809		err = -ENOMEM;
  1810		/* No need to zero the memory for masterspec */
  1811		masterspec = kmalloc(sizeof(*masterspec), GFP_KERNEL);
  1812		if (!masterspec)
  1813			goto out_put_masters;
  1814	
> 1815		of_for_each_phandle(&it, err, dev->of_node,
> 1816				    "mmu-masters", "#stream-id-cells", 0) {
  1817			int count = of_phandle_iterator_args(&it, masterspec->args,
  1818							     MAX_MASTER_STREAMIDS);
  1819			masterspec->np		= of_node_get(it.node);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]


#1362912 — Re: [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in device-tree parsing

FromRobin Murphy <robin.murphy@arm.com>
Date2016-03-22 20:00 +0100
SubjectRe: [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in device-tree parsing
Message-ID<rfzH4-h4-1@gated-at.bofh.it>
In reply to#1362894
Hi Joerg,

On 22/03/16 17:58, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
>
> Remove the usage of of_parse_phandle_with_args() and replace
> it by the phandle-iterator implementation so that we can
> parse out all of the potentially present 128 stream-ids.

In a stream-matching implementation, a device may quite legitimately own 
anything up to _all_ of the stream IDs (32768, or 65536 if we ever 
implement support for the SMMUv2 EXID extension), so this is only a 
genuine limit for stream indexing (and if anyone ever actually made one 
of those, I don't think they're running mainline on it).

Alternatively, how straightforward is it to change the DT on your 
machine? I'll be getting a v2 of [1] out in a couple of weeks (after 
imminent holidays), which already gets rid of MAX_MASTER_STREAMIDS 
altogether, and might also have grown proper SMR support by then.

Robin.

[1]:http://thread.gmane.org/gmane.linux.kernel.iommu/12454

> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>   drivers/iommu/arm-smmu.c | 28 ++++++++++++++++++++++------
>   1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 59ee4b8..413bd64 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -48,7 +48,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
> @@ -349,6 +349,12 @@ struct arm_smmu_domain {
>   	struct iommu_domain		domain;
>   };
>
> +struct arm_smmu_phandle_args {
> +	struct device_node *np;
> +	int args_count;
> +	uint32_t args[MAX_MASTER_STREAMIDS];
> +};
> +
>   static struct iommu_ops arm_smmu_ops;
>
>   static DEFINE_SPINLOCK(arm_smmu_devices_lock);
> @@ -458,7 +464,7 @@ static int insert_smmu_master(struct arm_smmu_device *smmu,
>
>   static int register_smmu_master(struct arm_smmu_device *smmu,
>   				struct device *dev,
> -				struct of_phandle_args *masterspec)
> +				struct arm_smmu_phandle_args *masterspec)
>   {
>   	int i;
>   	struct arm_smmu_master *master;
> @@ -1716,7 +1722,8 @@ 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_iterator it;
> +	struct arm_smmu_phandle_args masterspec;
>   	int num_irqs, i, err;
>
>   	smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
> @@ -1777,9 +1784,14 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
>
>   	i = 0;
>   	smmu->masters = RB_ROOT;
> -	while (!of_parse_phandle_with_args(dev->of_node, "mmu-masters",
> -					   "#stream-id-cells", i,
> -					   &masterspec)) {
> +
> +	of_for_each_phandle(&it, err, dev->of_node,
> +			    "mmu-masters", "#stream-id-cells", 0) {
> +		int count = of_phandle_iterator_args(&it, masterspec.args,
> +						     MAX_MASTER_STREAMIDS);
> +		masterspec.np		= of_node_get(it.node);
> +		masterspec.args_count	= count;
> +
>   		err = register_smmu_master(smmu, dev, &masterspec);
>   		if (err) {
>   			dev_err(dev, "failed to add master %s\n",
> @@ -1789,6 +1801,10 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
>
>   		i++;
>   	}
> +
> +	if (i == 0)
> +		goto out_put_masters;
> +
>   	dev_notice(dev, "registered %d master devices\n", i);
>
>   	parse_driver_options(smmu);
>

[toc] | [prev] | [next] | [standalone]


#1363378 — Re: [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in device-tree parsing

FromJoerg Roedel <joro@8bytes.org>
Date2016-03-23 13:00 +0100
SubjectRe: [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in device-tree parsing
Message-ID<rfPCa-2QN-13@gated-at.bofh.it>
In reply to#1362912
Hi Robin,

On Tue, Mar 22, 2016 at 06:53:48PM +0000, Robin Murphy wrote:
> In a stream-matching implementation, a device may quite legitimately
> own anything up to _all_ of the stream IDs (32768, or 65536 if we
> ever implement support for the SMMUv2 EXID extension), so this is
> only a genuine limit for stream indexing (and if anyone ever
> actually made one of those, I don't think they're running mainline
> on it).

Do you mean we might see a lot more than the currently 128 supported
stream-ids for an smmu?

> Alternatively, how straightforward is it to change the DT on your
> machine? I'll be getting a v2 of [1] out in a couple of weeks (after
> imminent holidays), which already gets rid of MAX_MASTER_STREAMIDS
> altogether, and might also have grown proper SMR support by then.

Hmm, I don't know how to change the DT of this Seattle machine. I think
it is provided by the ACPI BIOS. At least it boots with grub2 and not
u-boot and there is no DT in /boot.


	Joerg

[toc] | [prev] | [next] | [standalone]


#1366504 — Re: [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in device-tree parsing

FromWill Deacon <will.deacon@arm.com>
Date2016-03-29 19:30 +0200
SubjectRe: [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in device-tree parsing
Message-ID<ri5CP-10E-29@gated-at.bofh.it>
In reply to#1363378
On Wed, Mar 23, 2016 at 12:51:28PM +0100, Joerg Roedel wrote:
> On Tue, Mar 22, 2016 at 06:53:48PM +0000, Robin Murphy wrote:
> > In a stream-matching implementation, a device may quite legitimately
> > own anything up to _all_ of the stream IDs (32768, or 65536 if we
> > ever implement support for the SMMUv2 EXID extension), so this is
> > only a genuine limit for stream indexing (and if anyone ever
> > actually made one of those, I don't think they're running mainline
> > on it).
> 
> Do you mean we might see a lot more than the currently 128 supported
> stream-ids for an smmu?

We might, but this patch is still an improvement for now.

Will

[toc] | [prev] | [next] | [standalone]


#1366502 — Re: [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in device-tree parsing

FromWill Deacon <will.deacon@arm.com>
Date2016-03-29 19:30 +0200
SubjectRe: [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in device-tree parsing
Message-ID<ri5CO-10E-21@gated-at.bofh.it>
In reply to#1362894
Hi Joerg,

On Tue, Mar 22, 2016 at 06:58:29PM +0100, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> Remove the usage of of_parse_phandle_with_args() and replace
> it by the phandle-iterator implementation so that we can
> parse out all of the potentially present 128 stream-ids.
> 
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/iommu/arm-smmu.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 59ee4b8..413bd64 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -48,7 +48,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
> @@ -349,6 +349,12 @@ struct arm_smmu_domain {
>  	struct iommu_domain		domain;
>  };
>  
> +struct arm_smmu_phandle_args {
> +	struct device_node *np;
> +	int args_count;
> +	uint32_t args[MAX_MASTER_STREAMIDS];
> +};
> +
>  static struct iommu_ops arm_smmu_ops;
>  
>  static DEFINE_SPINLOCK(arm_smmu_devices_lock);
> @@ -458,7 +464,7 @@ static int insert_smmu_master(struct arm_smmu_device *smmu,
>  
>  static int register_smmu_master(struct arm_smmu_device *smmu,
>  				struct device *dev,
> -				struct of_phandle_args *masterspec)
> +				struct arm_smmu_phandle_args *masterspec)
>  {
>  	int i;
>  	struct arm_smmu_master *master;
> @@ -1716,7 +1722,8 @@ 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_iterator it;
> +	struct arm_smmu_phandle_args masterspec;
>  	int num_irqs, i, err;
>  
>  	smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
> @@ -1777,9 +1784,14 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
>  
>  	i = 0;
>  	smmu->masters = RB_ROOT;
> -	while (!of_parse_phandle_with_args(dev->of_node, "mmu-masters",
> -					   "#stream-id-cells", i,
> -					   &masterspec)) {
> +
> +	of_for_each_phandle(&it, err, dev->of_node,
> +			    "mmu-masters", "#stream-id-cells", 0) {
> +		int count = of_phandle_iterator_args(&it, masterspec.args,
> +						     MAX_MASTER_STREAMIDS);
> +		masterspec.np		= of_node_get(it.node);
> +		masterspec.args_count	= count;
> +
>  		err = register_smmu_master(smmu, dev, &masterspec);
>  		if (err) {
>  			dev_err(dev, "failed to add master %s\n",
> @@ -1789,6 +1801,10 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
>  
>  		i++;
>  	}
> +
> +	if (i == 0)
> +		goto out_put_masters;

I'm confused by this hunk. If i == 0, then we shouldn't have registered
any masters and therefore out_put_masters won't have anything to do.

In fact, I'm not completely clear on how the of_node refcounting interacts
with your iterators. Does the iterator put the node after you call the
"next" function, or does it increment each thing exactly once?

Will

[toc] | [prev] | [next] | [standalone]


#1362896 — [PATCH 2/6] of: Move phandle walking to of_phandle_iterator_next()

FromJoerg Roedel <joro@8bytes.org>
Date2016-03-22 19:10 +0100
Subject[PATCH 2/6] of: Move phandle walking to of_phandle_iterator_next()
Message-ID<rfyUF-8st-1@gated-at.bofh.it>
In reply to#1362884
From: Joerg Roedel <jroedel@suse.de>

Move the code to walk over the phandles out of the loop in
__of_parse_phandle_with_args() to a separate function that
just works with the iterator handle: of_phandle_iterator_next().

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

diff --git a/drivers/of/base.c b/drivers/of/base.c
index bfdb09b..4036512 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1464,6 +1464,75 @@ int of_phandle_iterator_init(struct of_phandle_iterator *it,
 	return 0;
 }
 
+int of_phandle_iterator_next(struct of_phandle_iterator *it)
+{
+	uint32_t count = 0;
+
+	if (it->node) {
+		of_node_put(it->node);
+		it->node = NULL;
+	}
+
+	if (!it->cur || it->phandle_end >= it->list_end)
+		return -ENOENT;
+
+	it->cur = it->phandle_end;
+
+	/* If phandle is 0, then it is an empty entry with no arguments. */
+	it->phandle = be32_to_cpup(it->cur++);
+
+	if (it->phandle) {
+
+		/*
+		 * Find the provider node and parse the #*-cells property to
+		 * determine the argument length.
+		 */
+		it->node = of_find_node_by_phandle(it->phandle);
+
+		if (it->cells_name) {
+			if (!it->node) {
+				pr_err("%s: could not find phandle\n",
+				       it->parent->full_name);
+				goto err;
+			}
+
+			if (of_property_read_u32(it->node, it->cells_name,
+						 &count)) {
+				pr_err("%s: could not get %s for %s\n",
+				       it->parent->full_name,
+				       it->cells_name,
+				       it->node->full_name);
+				goto err;
+			}
+		} else {
+			count = it->cell_count;
+		}
+
+		/*
+		 * Make sure that the arguments actually fit in the remaining
+		 * property data length
+		 */
+		if (it->cur + count > it->list_end) {
+			pr_err("%s: arguments longer than property\n",
+			       it->parent->full_name);
+			goto err;
+		}
+	}
+
+	it->phandle_end = it->cur + count;
+	it->cur_count = count;
+
+	return 0;
+
+err:
+	if (it->node) {
+		of_node_put(it->node);
+		it->node = NULL;
+	}
+
+	return -EINVAL;
+}
+
 static int __of_parse_phandle_with_args(const struct device_node *np,
 					const char *list_name,
 					const char *cells_name,
@@ -1479,59 +1548,9 @@ static int __of_parse_phandle_with_args(const struct device_node *np,
 		return rc;
 
 	/* Loop over the phandles until all the requested entry is found */
-	while (it.cur < it.list_end) {
-		rc = -EINVAL;
-		it.cur_count = 0;
-
-		/*
-		 * If phandle is 0, then it is an empty entry with no
-		 * arguments.  Skip forward to the next entry.
-		 */
-		it.phandle = be32_to_cpup(it.cur++);
-		if (it.phandle) {
-			/*
-			 * Find the provider node and parse the #*-cells
-			 * property to determine the argument length.
-			 *
-			 * This is not needed if the cell count is hard-coded
-			 * (i.e. cells_name not set, but cell_count is set),
-			 * except when we're going to return the found node
-			 * below.
-			 */
-			if (it.cells_name || cur_index == index) {
-				it.node = of_find_node_by_phandle(it.phandle);
-				if (!it.node) {
-					pr_err("%s: could not find phandle\n",
-						it.parent->full_name);
-					goto err;
-				}
-			}
-
-			if (it.cells_name) {
-				if (of_property_read_u32(it.node, it.cells_name,
-							 &it.cur_count)) {
-					pr_err("%s: could not get %s for %s\n",
-						it.parent->full_name, it.cells_name,
-						it.node->full_name);
-					goto err;
-				}
-			} else {
-				it.cur_count = it.cell_count;
-			}
-
-			/*
-			 * Make sure that the arguments actually fit in the
-			 * remaining property data length
-			 */
-			if (it.cur + it.cur_count > it.list_end) {
-				pr_err("%s: arguments longer than property\n",
-					 it.parent->full_name);
-				goto err;
-			}
-		}
-
+	while ((rc = of_phandle_iterator_next(&it)) == 0) {
 		/*
-		 * All of the error cases above bail out of the loop, so at
+		 * All of the error cases bail out of the loop, so at
 		 * this point, the parsing is successful. If the requested
 		 * index matches, then fill the out_args structure and return,
 		 * or return -ENOENT for an empty entry.
@@ -1557,9 +1576,6 @@ static int __of_parse_phandle_with_args(const struct device_node *np,
 			return 0;
 		}
 
-		of_node_put(it.node);
-		it.node = NULL;
-		it.cur += it.cur_count;
 		cur_index++;
 	}
 
@@ -1569,7 +1585,9 @@ static int __of_parse_phandle_with_args(const struct device_node *np,
 	 * -EINVAL : parsing error on data
 	 * [1..n]  : Number of phandle (count mode; when index = -1)
 	 */
-	rc = index < 0 ? cur_index : -ENOENT;
+	if (rc == -ENOENT && index < 0)
+		rc = cur_index;
+
  err:
 	if (it.node)
 		of_node_put(it.node);
diff --git a/include/linux/of.h b/include/linux/of.h
index 70c2bdb..d94388b0 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -358,6 +358,8 @@ extern int of_phandle_iterator_init(struct of_phandle_iterator *it,
 				    const char *cells_name,
 				    int cell_count);
 
+extern int of_phandle_iterator_next(struct of_phandle_iterator *it);
+
 extern void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align));
 extern int of_alias_get_id(struct device_node *np, const char *stem);
 extern int of_alias_get_highest_id(const char *stem);
@@ -641,6 +643,11 @@ static inline int of_phandle_iterator_init(struct of_phandle_iterator *it,
 	return -ENOSYS;
 }
 
+static inline int of_phandle_iterator_next(struct of_phandle_iterator *it)
+{
+	return -ENOSYS;
+}
+
 static inline int of_alias_get_id(struct device_node *np, const char *stem)
 {
 	return -ENOSYS;
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]


#1362899 — [PATCH 1/6] of: Introduce struct of_phandle_iterator

FromJoerg Roedel <joro@8bytes.org>
Date2016-03-22 19:10 +0100
Subject[PATCH 1/6] of: Introduce struct of_phandle_iterator
Message-ID<rfyUG-8st-13@gated-at.bofh.it>
In reply to#1362884
From: Joerg Roedel <jroedel@suse.de>

This struct carrys all necessary information to iterate over
a list of phandles and extract the arguments. Add an
init-function for the iterator and make use of it in
__of_parse_phandle_with_args().

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

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 017dd94..bfdb09b 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1439,35 +1439,56 @@ void of_print_phandle_args(const char *msg, const struct of_phandle_args *args)
 	printk("\n");
 }
 
+int of_phandle_iterator_init(struct of_phandle_iterator *it,
+		const struct device_node *np,
+		const char *list_name,
+		const char *cells_name,
+		int cell_count)
+{
+	const __be32 *list;
+	int size;
+
+	memset(it, 0, sizeof(*it));
+
+	list = of_get_property(np, list_name, &size);
+	if (!list)
+		return -ENOENT;
+
+	it->cells_name = cells_name;
+	it->cell_count = cell_count;
+	it->parent = np;
+	it->list_end = list + size / sizeof(*list);
+	it->phandle_end = list;
+	it->cur = list;
+
+	return 0;
+}
+
 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)
 {
-	const __be32 *list, *list_end;
-	int rc = 0, size, cur_index = 0;
-	uint32_t count = 0;
-	struct device_node *node = NULL;
-	phandle phandle;
+	struct of_phandle_iterator it;
+	int rc, cur_index = 0;
 
-	/* Retrieve the phandle list property */
-	list = of_get_property(np, list_name, &size);
-	if (!list)
-		return -ENOENT;
-	list_end = list + size / sizeof(*list);
+	rc = of_phandle_iterator_init(&it, np, list_name,
+				      cells_name, cell_count);
+	if (rc)
+		return rc;
 
 	/* Loop over the phandles until all the requested entry is found */
-	while (list < list_end) {
+	while (it.cur < it.list_end) {
 		rc = -EINVAL;
-		count = 0;
+		it.cur_count = 0;
 
 		/*
 		 * If phandle is 0, then it is an empty entry with no
 		 * arguments.  Skip forward to the next entry.
 		 */
-		phandle = be32_to_cpup(list++);
-		if (phandle) {
+		it.phandle = be32_to_cpup(it.cur++);
+		if (it.phandle) {
 			/*
 			 * Find the provider node and parse the #*-cells
 			 * property to determine the argument length.
@@ -1477,34 +1498,34 @@ static int __of_parse_phandle_with_args(const struct device_node *np,
 			 * except when we're going to return the found node
 			 * below.
 			 */
-			if (cells_name || cur_index == index) {
-				node = of_find_node_by_phandle(phandle);
-				if (!node) {
+			if (it.cells_name || cur_index == index) {
+				it.node = of_find_node_by_phandle(it.phandle);
+				if (!it.node) {
 					pr_err("%s: could not find phandle\n",
-						np->full_name);
+						it.parent->full_name);
 					goto err;
 				}
 			}
 
-			if (cells_name) {
-				if (of_property_read_u32(node, cells_name,
-							 &count)) {
+			if (it.cells_name) {
+				if (of_property_read_u32(it.node, it.cells_name,
+							 &it.cur_count)) {
 					pr_err("%s: could not get %s for %s\n",
-						np->full_name, cells_name,
-						node->full_name);
+						it.parent->full_name, it.cells_name,
+						it.node->full_name);
 					goto err;
 				}
 			} else {
-				count = cell_count;
+				it.cur_count = it.cell_count;
 			}
 
 			/*
 			 * Make sure that the arguments actually fit in the
 			 * remaining property data length
 			 */
-			if (list + count > list_end) {
+			if (it.cur + it.cur_count > it.list_end) {
 				pr_err("%s: arguments longer than property\n",
-					 np->full_name);
+					 it.parent->full_name);
 				goto err;
 			}
 		}
@@ -1517,28 +1538,28 @@ static int __of_parse_phandle_with_args(const struct device_node *np,
 		 */
 		rc = -ENOENT;
 		if (cur_index == index) {
-			if (!phandle)
+			if (!it.phandle)
 				goto err;
 
 			if (out_args) {
 				int i;
-				if (WARN_ON(count > MAX_PHANDLE_ARGS))
-					count = MAX_PHANDLE_ARGS;
-				out_args->np = node;
-				out_args->args_count = count;
-				for (i = 0; i < count; i++)
-					out_args->args[i] = be32_to_cpup(list++);
+				if (WARN_ON(it.cur_count > MAX_PHANDLE_ARGS))
+					it.cur_count = MAX_PHANDLE_ARGS;
+				out_args->np = it.node;
+				out_args->args_count = it.cur_count;
+				for (i = 0; i < it.cur_count; i++)
+					out_args->args[i] = be32_to_cpup(it.cur++);
 			} else {
-				of_node_put(node);
+				of_node_put(it.node);
 			}
 
 			/* Found it! return success */
 			return 0;
 		}
 
-		of_node_put(node);
-		node = NULL;
-		list += count;
+		of_node_put(it.node);
+		it.node = NULL;
+		it.cur += it.cur_count;
 		cur_index++;
 	}
 
@@ -1550,8 +1571,8 @@ static int __of_parse_phandle_with_args(const struct device_node *np,
 	 */
 	rc = index < 0 ? cur_index : -ENOENT;
  err:
-	if (node)
-		of_node_put(node);
+	if (it.node)
+		of_node_put(it.node);
 	return rc;
 }
 
diff --git a/include/linux/of.h b/include/linux/of.h
index dc6e396..70c2bdb 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -75,6 +75,23 @@ struct of_phandle_args {
 	uint32_t args[MAX_PHANDLE_ARGS];
 };
 
+struct of_phandle_iterator {
+	/* Common iterator information */
+	const char *cells_name;
+	int cell_count;
+	const struct device_node *parent;
+
+	/* List size information */
+	const __be32 *list_end;
+	const __be32 *phandle_end;
+
+	/* Current position state */
+	const __be32 *cur;
+	uint32_t cur_count;
+	phandle phandle;
+	struct device_node *node;
+};
+
 struct of_reconfig_data {
 	struct device_node	*dn;
 	struct property		*prop;
@@ -334,6 +351,13 @@ extern int of_parse_phandle_with_fixed_args(const struct device_node *np,
 extern int of_count_phandle_with_args(const struct device_node *np,
 	const char *list_name, const char *cells_name);
 
+/* phandle iterator functions */
+extern int of_phandle_iterator_init(struct of_phandle_iterator *it,
+				    const struct device_node *np,
+				    const char *list_name,
+				    const char *cells_name,
+				    int cell_count);
+
 extern void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align));
 extern int of_alias_get_id(struct device_node *np, const char *stem);
 extern int of_alias_get_highest_id(const char *stem);
@@ -608,6 +632,15 @@ static inline int of_count_phandle_with_args(struct device_node *np,
 	return -ENOSYS;
 }
 
+static inline int of_phandle_iterator_init(struct of_phandle_iterator *it,
+					   const struct device_node *np,
+					   const char *list_name,
+					   const char *cells_name,
+					   int cell_count)
+{
+	return -ENOSYS;
+}
+
 static inline int of_alias_get_id(struct device_node *np, const char *stem)
 {
 	return -ENOSYS;
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]


#1362910

FromRob Herring <robh+dt@kernel.org>
Date2016-03-22 19:50 +0100
Message-ID<rfzxo-dz-3@gated-at.bofh.it>
In reply to#1362884
On Tue, Mar 22, 2016 at 12:58 PM, Joerg Roedel <joro@8bytes.org> wrote:
> Hi,
>
> here is an implementation of the iterator over phandles
> concept which Rob Herring suggested to me some time ago. My
> approach is a little bit different from what the diff showed
> back then, but it gets rid of the allocation and 'struct
> 'struct of_phandle_args' misuse.
>
> I also converted the arm-smmu driver to make use of the
> iterator. The resulting kernel boots on my AMD Seattle
> system and fixes the warning triggered there. The patches
> now also pass all dt-unittests in my kvm environment.
>
> Please review. Patches are based on v4.5.

Other than my one comment, this looks good to me. For the series:

Acked-by: Rob Herring <robh@kernel.org>

[toc] | [prev] | [next] | [standalone]


#1363377

FromJoerg Roedel <joro@8bytes.org>
Date2016-03-23 13:00 +0100
Message-ID<rfPCa-2QN-11@gated-at.bofh.it>
In reply to#1362910
Hi Rob,

On Tue, Mar 22, 2016 at 01:45:41PM -0500, Rob Herring wrote:
> On Tue, Mar 22, 2016 at 12:58 PM, Joerg Roedel <joro@8bytes.org> wrote:
> > Please review. Patches are based on v4.5.
> 
> Other than my one comment, this looks good to me. For the series:
> 
> Acked-by: Rob Herring <robh@kernel.org>

Thanks a lot for your fast reply! I guess these patches will go through
the DT tree, or should I carry them in the IOMMU tree? The DT tree
probably makes more sense.



	Joerg

[toc] | [prev] | [next] | [standalone]


#1363670

FromRob Herring <robh+dt@kernel.org>
Date2016-03-23 21:40 +0100
Message-ID<rfXJo-n8-7@gated-at.bofh.it>
In reply to#1363377
On Wed, Mar 23, 2016 at 6:54 AM, Joerg Roedel <joro@8bytes.org> wrote:
> Hi Rob,
>
> On Tue, Mar 22, 2016 at 01:45:41PM -0500, Rob Herring wrote:
>> On Tue, Mar 22, 2016 at 12:58 PM, Joerg Roedel <joro@8bytes.org> wrote:
>> > Please review. Patches are based on v4.5.
>>
>> Other than my one comment, this looks good to me. For the series:
>>
>> Acked-by: Rob Herring <robh@kernel.org>
>
> Thanks a lot for your fast reply! I guess these patches will go through
> the DT tree, or should I carry them in the IOMMU tree? The DT tree
> probably makes more sense.

Sure, I can take them.

Rob

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web