On Wed, Sep 4, 2024 at 9:16 PM Andy Shevchenko
[off-list ref] wrote:

On Wed, Sep 04, 2024 at 04:09:26PM +0300, Andy Shevchenko wrote:

quoted

On Wed, Sep 04, 2024 at 05:00:05PM +0800, Chen-Yu Tsai wrote:

quoted

quoted

+static struct device_node *of_get_child_regulator(struct device_node *parent,
+                                             const char *prop_name)
+{
+   struct device_node *regnode = NULL;

quoted

quoted

+   struct device_node *child = NULL;

Btw, redundant assignment here, as child will be assigned anyway AFAIR.

quoted

quoted

+   for_each_child_of_node(parent, child) {

quoted

+           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

quoted

quoted

+   }
+   return NULL;
+
+err_node_put:
+   of_node_put(child);
+   return regnode;
+}

--
With Best Regards,
Andy Shevchenko