On Mon, Jul 11, 2016 at 09:56:18PM +0200, Maxime Ripard wrote: > From: Thierry Reding <treding@xxxxxxxxxx> > > The fdt_for_each_subnode() iterator macro provided by this patch can be > used to iterate over a device tree node's subnodes. At each iteration a > loop variable will be set to the next subnode. > > Signed-off-by: Thierry Reding <treding@xxxxxxxxxx> > Signed-off-by: Maxime Ripard <maxime.ripard@xxxxxxxxxxxxxxxxxx> Code looks ok, but there are some nits in the comments. > --- > libfdt/libfdt.h | 25 +++++++++++++++++++++++++ > tests/subnode_iterate.c | 8 ++------ > 2 files changed, 27 insertions(+), 6 deletions(-) > > diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h > index 36222fd4a6f4..0cf46872b54e 100644 > --- a/libfdt/libfdt.h > +++ b/libfdt/libfdt.h > @@ -168,6 +168,31 @@ int fdt_first_subnode(const void *fdt, int offset); > */ > int fdt_next_subnode(const void *fdt, int offset); > > +/** > + * fdt_for_each_subnode - iterate over all subnodes of a parent Parameter descriptions should go here. > + * > + * This is actually a wrapper around a for loop and would be used like so: > + * > + * fdt_for_each_subnode(fdt, node, parent) { Parameter order hasn't been updated for code revisions. > + * ... > + * use node > + * ... > + * } One gotcha with this is what happens if there's an error during the iteration. I suggest adding something like: if ((node < 0) && (node != -FDT_ERR_NOTFOUND)) { /* error handling */ } to the example, to remind users that they should check for this. > + * > + * Note that this is implemented as a macro and node is used as iterator in s/node/@node/ - makes it stand out as a parameter. > + * the loop. It should therefore be a locally allocated variable. The parent ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Remove for brevity. > + * variable on the other hand is never modified, so it can be constant or ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Remove for brevity. > + * even a literal. > + * > + * @node: child node (int) > + * @fdt: FDT blob (const void *) > + * @parent: parent node (int) > + */ > +#define fdt_for_each_subnode(node, fdt, parent) \ > + for (node = fdt_first_subnode(fdt, parent); \ > + node >= 0; \ > + node = fdt_next_subnode(fdt, node)) > + > /**********************************************************************/ > /* General functions */ > /**********************************************************************/ > diff --git a/tests/subnode_iterate.c b/tests/subnode_iterate.c > index b9f379d5963c..0fb5c901ebd7 100644 > --- a/tests/subnode_iterate.c > +++ b/tests/subnode_iterate.c > @@ -48,9 +48,7 @@ static void test_node(void *fdt, int parent_offset) > subnodes = cpu_to_fdt32(*prop); > > count = 0; > - for (offset = fdt_first_subnode(fdt, parent_offset); > - offset >= 0; > - offset = fdt_next_subnode(fdt, offset)) > + fdt_for_each_subnode(offset, fdt, parent_offset) > count++; > > if (count != subnodes) { > @@ -65,9 +63,7 @@ static void check_fdt_next_subnode(void *fdt) > int offset; > int count = 0; > > - for (offset = fdt_first_subnode(fdt, 0); > - offset >= 0; > - offset = fdt_next_subnode(fdt, offset)) { > + fdt_for_each_subnode(offset, fdt, 0) { > test_node(fdt, offset); > count++; > } -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature