diff mbox series

[net-next,v4,2/6] net: phy: introduce device_mdiobus_register()

Message ID 20200709175722.5228-3-calvin.johnson@oss.nxp.com (mailing list archive)
State Superseded, archived
Headers show
Series [net-next,v4,1/6] Documentation: ACPI: DSD: Document MDIO PHY | expand

Commit Message

Calvin Johnson July 9, 2020, 5:57 p.m. UTC
Introduce device_mdiobus_register() to register mdiobus
in cases of either DT or ACPI.

Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com>
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/net/phy/mdio_bus.c | 22 ++++++++++++++++++++++
 include/linux/mdio.h       |  1 +
 2 files changed, 23 insertions(+)

Comments

Andy Shevchenko July 9, 2020, 8:39 p.m. UTC | #1
On Thu, Jul 9, 2020 at 8:57 PM Calvin Johnson
<calvin.johnson@oss.nxp.com> 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;
> +       }
> +}
diff mbox series

Patch

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 46b33701ad4b..3c2749e84f74 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -501,6 +501,28 @@  static int mdiobus_create_device(struct mii_bus *bus,
 	return ret;
 }
 
+/**
+ * device_mdiobus_register - bring up all the PHYs on a given bus and
+ * attach them to bus. This handles both DT and ACPI methods.
+ * @bus: target mii_bus
+ * @dev: given MDIO device
+ *
+ * Returns 0 on success or < 0 on error.
+ */
+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);
+	} else {
+		return -ENODEV;
+	}
+}
+EXPORT_SYMBOL(device_mdiobus_register);
+
 /**
  * __mdiobus_register - bring up all the PHYs on a given bus and attach them to bus
  * @bus: target mii_bus
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index 898cbf00332a..f78c6a7f8eb7 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -358,6 +358,7 @@  static inline int mdiobus_c45_read(struct mii_bus *bus, int prtad, int devad,
 	return mdiobus_read(bus, prtad, mdiobus_c45_addr(devad, regnum));
 }
 
+int device_mdiobus_register(struct mii_bus *bus, struct device *dev);
 int mdiobus_register_device(struct mdio_device *mdiodev);
 int mdiobus_unregister_device(struct mdio_device *mdiodev);
 bool mdiobus_is_registered_device(struct mii_bus *bus, int addr);