On Thu, Jul 9, 2020 at 8:57 PM Calvin Johnson
[off-list ref] wrote:

Introduce device_mdiobus_register() to register mdiobus
in cases of either DT or ACPI.

...

+/**
+ * device_mdiobus_register - bring up all the PHYs on a given bus and
+ * attach them to bus. This handles both DT and ACPI methods.

This is usually one line summary and description goes...

+ * @bus: target mii_bus
+ * @dev: given MDIO device
+ *

...somewhere here.

+ * Returns 0 on success or < 0 on error.

This would be nicer to read as '...or negative error code' or alike.

+ */
+int device_mdiobus_register(struct mii_bus *bus,
+                           struct device *dev)
+{

+       if (dev->of_node) {
+               return of_mdiobus_register(bus, dev->of_node);
+       } else if (dev_fwnode(dev)) {
+               bus->dev.fwnode = dev_fwnode(dev);
+               return mdiobus_register(bus);

All these 'else' are redundant, but the main confusion here is the use
of dev_fwnode() vs. dev->of_node.

I would rather see something like

struct fwnode_handle *fwnode = dev_fwnode(dev);
...

if (is_of_node(fwnode))
  return ...(..., to_of_node(fwnode));
if (fwnode) {
  ...
  return ...
}
return -ENODEV;

(Okay, 'else':s may be left if you think it's better to read)

+       } else {
+               return -ENODEV;
+       }
+}

-- 
With Best Regards,
Andy Shevchenko