diff mbox series

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

Message ID 20200711065600.9448-3-calvin.johnson@oss.nxp.com
State Changes Requested
Delegated to: David Miller
Headers show
Series [net-next,v6,1/6] Documentation: ACPI: DSD: Document MDIO PHY | expand

Commit Message

Calvin Johnson July 11, 2020, 6:55 a.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 v6:
- change device_mdiobus_register() parameter position
- improve documentation

Changes in v5:
- add description
- clean up if else

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

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

Comments

Florian Fainelli July 11, 2020, 9:36 p.m. UTC | #1
On 7/10/2020 11:55 PM, Calvin Johnson wrote:
> 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 v6:
> - change device_mdiobus_register() parameter position
> - improve documentation
> 
> Changes in v5:
> - add description
> - clean up if else
> 
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/net/phy/mdio_bus.c | 26 ++++++++++++++++++++++++++
>  include/linux/mdio.h       |  1 +
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index 46b33701ad4b..8610f938f81f 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -501,6 +501,32 @@ static int mdiobus_create_device(struct mii_bus *bus,
>  	return ret;
>  }
>  
> +/**
> + * device_mdiobus_register - register mdiobus for either DT or ACPI
> + * @bus: target mii_bus
> + * @dev: given MDIO device
> + *
> + * Description: Given an MDIO device and target mii bus, this function
> + * calls of_mdiobus_register() for DT node and mdiobus_register() in
> + * case of ACPI.
> + *
> + * Returns 0 on success or negative error code on failure.
> + */
> +int device_mdiobus_register(struct device *dev,
> +			    struct mii_bus *bus)
> +{
> +	struct fwnode_handle *fwnode = dev_fwnode(dev);
> +
> +	if (is_of_node(fwnode))
> +		return of_mdiobus_register(bus, to_of_node(fwnode));
> +	if (fwnode) {
> +		bus->dev.fwnode = fwnode;
> +		return mdiobus_register(bus);
> +	}
> +	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..f454c5435101 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 device *dev, struct mii_bus *bus);

Humm, this header file does not have any of the mii_bus registration
functions declared, and it typically pertains to mdio_device instances
which are devices *on* the mii_bus. phy.h may be more appropriate here
until we break it up into phy_device proper, mii_bus, etc.

>  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);
>
Calvin Johnson July 13, 2020, 5:56 a.m. UTC | #2
On Sat, Jul 11, 2020 at 02:36:20PM -0700, Florian Fainelli wrote:
> 
> 
> On 7/10/2020 11:55 PM, Calvin Johnson wrote:
> > 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 v6:
> > - change device_mdiobus_register() parameter position
> > - improve documentation
> > 
> > Changes in v5:
> > - add description
> > - clean up if else
> > 
> > Changes in v4: None
> > Changes in v3: None
> > Changes in v2: None
> > 
> >  drivers/net/phy/mdio_bus.c | 26 ++++++++++++++++++++++++++
> >  include/linux/mdio.h       |  1 +
> >  2 files changed, 27 insertions(+)
> > 
> > diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> > index 46b33701ad4b..8610f938f81f 100644
> > --- a/drivers/net/phy/mdio_bus.c
> > +++ b/drivers/net/phy/mdio_bus.c
> > @@ -501,6 +501,32 @@ static int mdiobus_create_device(struct mii_bus *bus,
> >  	return ret;
> >  }
> >  
> > +/**
> > + * device_mdiobus_register - register mdiobus for either DT or ACPI
> > + * @bus: target mii_bus
> > + * @dev: given MDIO device
> > + *
> > + * Description: Given an MDIO device and target mii bus, this function
> > + * calls of_mdiobus_register() for DT node and mdiobus_register() in
> > + * case of ACPI.
> > + *
> > + * Returns 0 on success or negative error code on failure.
> > + */
> > +int device_mdiobus_register(struct device *dev,
> > +			    struct mii_bus *bus)
> > +{
> > +	struct fwnode_handle *fwnode = dev_fwnode(dev);
> > +
> > +	if (is_of_node(fwnode))
> > +		return of_mdiobus_register(bus, to_of_node(fwnode));
> > +	if (fwnode) {
> > +		bus->dev.fwnode = fwnode;
> > +		return mdiobus_register(bus);
> > +	}
> > +	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..f454c5435101 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 device *dev, struct mii_bus *bus);
> 
> Humm, this header file does not have any of the mii_bus registration
> functions declared, and it typically pertains to mdio_device instances
> which are devices *on* the mii_bus. phy.h may be more appropriate here
> until we break it up into phy_device proper, mii_bus, etc.
> 
> >  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);

Although some mii_bus registration functions are declared in phy.h, IMO, it
doesn't seem to be the right place. If we look plainly, phy related things
would be expected in phy.h and mdio related in mdio.h. Maybe as you said
we should consider breaking into phy_device proper, mii_bus, etc. We can take it
up later.

Please let me know if you still want device_mdiobus_register() in phy.h.

Thanks
Calvin
diff mbox series

Patch

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 46b33701ad4b..8610f938f81f 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -501,6 +501,32 @@  static int mdiobus_create_device(struct mii_bus *bus,
 	return ret;
 }
 
+/**
+ * device_mdiobus_register - register mdiobus for either DT or ACPI
+ * @bus: target mii_bus
+ * @dev: given MDIO device
+ *
+ * Description: Given an MDIO device and target mii bus, this function
+ * calls of_mdiobus_register() for DT node and mdiobus_register() in
+ * case of ACPI.
+ *
+ * Returns 0 on success or negative error code on failure.
+ */
+int device_mdiobus_register(struct device *dev,
+			    struct mii_bus *bus)
+{
+	struct fwnode_handle *fwnode = dev_fwnode(dev);
+
+	if (is_of_node(fwnode))
+		return of_mdiobus_register(bus, to_of_node(fwnode));
+	if (fwnode) {
+		bus->dev.fwnode = fwnode;
+		return mdiobus_register(bus);
+	}
+	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..f454c5435101 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 device *dev, struct mii_bus *bus);
 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);