On Wed, Sep 4, 2024 at 9:16 PM Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > On Wed, Sep 04, 2024 at 04:09:26PM +0300, Andy Shevchenko wrote: > > On Wed, Sep 04, 2024 at 05:00:05PM +0800, Chen-Yu Tsai wrote: > > > > +static struct device_node *of_get_child_regulator(struct device_node *parent, > > > + const char *prop_name) > > > +{ > > > + struct device_node *regnode = NULL; > > > > + struct device_node *child = NULL; > > Btw, redundant assignment here, as child will be assigned anyway AFAIR. > > > > + for_each_child_of_node(parent, child) { > > > > > + regnode = of_parse_phandle(child, prop_name, 0); > > > + if (!regnode) { > > > + regnode = of_get_child_regulator(child, prop_name); > > > + if (regnode) > > > + goto err_node_put; > > > + } else { > > > + goto err_node_put; > > > + } > > > > I know this is just a move of the existing code, but consider negating the > > conditional and have something like > > > > regnode = of_parse_phandle(child, prop_name, 0); > > if (regnode) > > goto err_node_put; > > > > regnode = of_get_child_regulator(child, prop_name); > > if (regnode) > > goto err_node_put; > > Looks like Mark already merged this one. I'll send extra patches to clean this up later. ChenYu > > > + } > > > + return NULL; > > > + > > > +err_node_put: > > > + of_node_put(child); > > > + return regnode; > > > +} > > -- > With Best Regards, > Andy Shevchenko > >