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; > > > + } > > + return NULL; > > + > > +err_node_put: > > + of_node_put(child); > > + return regnode; > > +} -- With Best Regards, Andy Shevchenko