On Mon, Feb 8, 2021 at 5:14 PM Calvin Johnson
[off-list ref] wrote:

Define acpi_mdiobus_register() to Register mii_bus and create PHYs for
each ACPI child node.

...

+/**
+ * acpi_mdiobus_register - Register mii_bus and create PHYs from the ACPI ASL.

+ *

Redundant blank line.

+ * @mdio: pointer to mii_bus structure
+ * @fwnode: pointer to fwnode of MDIO bus.
+ *
+ * This function registers the mii_bus structure and registers a phy_device
+ * for each child node of @fwnode.
+ */
+int acpi_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode)
+{
+       struct fwnode_handle *child;
+       u32 addr;
+       int ret;
+
+       /* Mask out all PHYs from auto probing. */

+       mdio->phy_mask = ~0;

I would rather see GENMASK(31, 0) here because in case the type of the
variable is changed we will need to amend this anyway.

+       ret = mdiobus_register(mdio);
+       if (ret)
+               return ret;

+       mdio->dev.fwnode = fwnode;

Shouldn't it be rather ACPI_SET_COMPANION() as other bus / drivers do?

+/* Loop over the child nodes and register a phy_device for each PHY */

Indentation.

+       fwnode_for_each_child_node(fwnode, child) {
+               ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);

+               if ((ret) || addr >= PHY_MAX_ADDR)

Too many parentheses.

+                       continue;
+
+               ret = fwnode_mdiobus_register_phy(mdio, child, addr);
+               if (ret == -ENODEV)
+                       dev_err(&mdio->dev,
+                               "MDIO device at address %d is missing.\n",
+                               addr);
+       }
+       return 0;
+}

...

+/*
+ * ACPI helpers for the MDIO (Ethernet PHY) API
+ *
+ */

It's one line AFAICT!

...

+#include <linux/device.h>
+#include <linux/phy.h>

This seems a bit inconsistent with the below.
I see the user of mdiobus_register(). It's the only header should be
included. Everything else would be forward declared like

struct fwnode_handle;

+#if IS_ENABLED(CONFIG_ACPI_MDIO)
+int acpi_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode);
+#else /* CONFIG_ACPI_MDIO */
+static inline int acpi_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode)
+{
+       /*
+        * Fall back to mdiobus_register() function to register a bus.
+        * This way, we don't have to keep compat bits around in drivers.
+        */
+
+       return mdiobus_register(mdio);
+}
+#endif

-- 
With Best Regards,
Andy Shevchenko