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.
quoted hunk ↗ jump to hunk
+ * @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.
quoted hunk ↗ jump to hunk
+ 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.
quoted hunk ↗ jump to hunk
+ 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;
+}
...
quoted hunk ↗ jump to hunk
+/*
+ * 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;
quoted hunk ↗ jump to hunk
+#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