| Message ID | 20201217234337.1983732-6-djrscally@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add functionality to ipu3-cio2 driver allowing software_node connections to sensors on platforms designed for Windows | expand |
On Thu, Dec 17, 2020 at 11:43:30PM +0000, Daniel Scally wrote: > To maintain consistency with software_node_unregister_nodes(), reverse > the order in which the software_node_unregister_node_group() function > unregisters nodes. ... > + * Unregister multiple software nodes at once. The array will be unwound in > + * reverse order (I.E. last entry first) and thus if any member of the array A nit: I.E. -> i.e. > + * has its .parent member set then they should appear later in the array such > + * that they are unregistered first.
Hi Daniel, On Thu, Dec 17, 2020 at 11:43:30PM +0000, Daniel Scally wrote: > To maintain consistency with software_node_unregister_nodes(), reverse > the order in which the software_node_unregister_node_group() function > unregisters nodes. > > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Daniel Scally <djrscally@gmail.com> > --- > Changes in v2: > > - Initialised i properly > > drivers/base/swnode.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c > index cfd1faea48a7..2b90d380039b 100644 > --- a/drivers/base/swnode.c > +++ b/drivers/base/swnode.c > @@ -778,16 +778,22 @@ EXPORT_SYMBOL_GPL(software_node_register_node_group); > * software_node_unregister_node_group - Unregister a group of software nodes > * @node_group: NULL terminated array of software node pointers to be unregistered > * > - * Unregister multiple software nodes at once. > + * Unregister multiple software nodes at once. The array will be unwound in > + * reverse order (I.E. last entry first) and thus if any member of the array > + * has its .parent member set then they should appear later in the array such > + * that they are unregistered first. > */ > void software_node_unregister_node_group(const struct software_node **node_group) > { > - unsigned int i; > + unsigned int i = 0; > > if (!node_group) > return; > > - for (i = 0; node_group[i]; i++) > + while (node_group[i]->name) Why is this change made? node_group is a NULL-terminated array, and the above accesses the name pointer on each entry before checking the entry is non-NULL. Or do I miss something here? > + i++; > + > + while (i--) > software_node_unregister(node_group[i]); > } > EXPORT_SYMBOL_GPL(software_node_unregister_node_group); -- Regards, Sakari Ailus
On Mon, Dec 21, 2020 at 11:21:16AM +0200, Sakari Ailus wrote: > On Thu, Dec 17, 2020 at 11:43:30PM +0000, Daniel Scally wrote: > > To maintain consistency with software_node_unregister_nodes(), reverse > > the order in which the software_node_unregister_node_group() function > > unregisters nodes. ... > > void software_node_unregister_node_group(const struct software_node **node_group) > > { > > - unsigned int i; > > + unsigned int i = 0; > > > > if (!node_group) > > return; > > > > - for (i = 0; node_group[i]; i++) > > + while (node_group[i]->name) > > Why is this change made? node_group is a NULL-terminated array, and the > above accesses the name pointer on each entry before checking the entry is > non-NULL. Or do I miss something here? I believe it's a copy'n'paste typo. > > + i++; > > + > > + while (i--) > > software_node_unregister(node_group[i]); > > } -- With Best Regards, Andy Shevchenko
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c index cfd1faea48a7..2b90d380039b 100644 --- a/drivers/base/swnode.c +++ b/drivers/base/swnode.c @@ -778,16 +778,22 @@ EXPORT_SYMBOL_GPL(software_node_register_node_group); * software_node_unregister_node_group - Unregister a group of software nodes * @node_group: NULL terminated array of software node pointers to be unregistered * - * Unregister multiple software nodes at once. + * Unregister multiple software nodes at once. The array will be unwound in + * reverse order (I.E. last entry first) and thus if any member of the array + * has its .parent member set then they should appear later in the array such + * that they are unregistered first. */ void software_node_unregister_node_group(const struct software_node **node_group) { - unsigned int i; + unsigned int i = 0; if (!node_group) return; - for (i = 0; node_group[i]; i++) + while (node_group[i]->name) + i++; + + while (i--) software_node_unregister(node_group[i]); } EXPORT_SYMBOL_GPL(software_node_unregister_node_group);