[net-next,v2,5/7] net: mdiobus: search for PSE nodes by parsing PHY nodes.

Commit Message

Oleksij Rempel Aug. 25, 2022, 1:02 p.m. UTC

Some PHYs can be linked with PSE (Power Sourcing Equipment), so search
for related nodes and attach it to the phydev.

Signed-off-by: Oleksij Rempel <[e-mail]>
---
 drivers/net/mdio/fwnode_mdio.c | 37 ++++++++++++++++++++++++++++++++--
 drivers/net/phy/phy_device.c   |  2 ++
 include/linux/phy.h            |  2 ++
 3 files changed, 39 insertions(+), 2 deletions(-)

Comments

Russell King (Oracle) Aug. 25, 2022, 1:11 p.m. UTC | #1

On Thu, Aug 25, 2022 at 03:02:09PM +0200, Oleksij Rempel wrote:
> +static struct pse_control *
> +fwnode_find_pse_control(struct fwnode_handle *fwnode)
> +{
> +	struct pse_control *psec;
> +	struct device_node *np;
> +
> +	if (is_acpi_node(fwnode))
> +		return NULL;
> +
> +	np = to_of_node(fwnode);
> +	if (!np)
> +		return NULL;

Doesn't to_of_node() confirm that the fwnode is a DT node? In other
words, isn't the "is_acpi_node()" entirely redundant?

> +
> +	psec = of_pse_control_get(np);
> +	if (IS_ERR_OR_NULL(psec))
> +		return NULL;
> +
> +	return psec;
> +}

So fwnode_find_pse_control() returns NULL on error.

> +	psec = fwnode_find_pse_control(child);
> +	if (IS_ERR(psec))
> +		return PTR_ERR(psec);

This usage expects it to return an error-pointer.

Clearly, there is some disagreement about what fwnode_find_pse_control()
returns on error.

Patch

diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index e78ad55c0e091..1e775c449f5db 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -10,10 +10,31 @@ 
 #include <linux/fwnode_mdio.h>
 #include <linux/of.h>
 #include <linux/phy.h>
+#include <linux/pse-pd/pse.h>
 
 MODULE_AUTHOR("Calvin Johnson <[e-mail]>");
 MODULE_LICENSE("GPL");
 
+static struct pse_control *
+fwnode_find_pse_control(struct fwnode_handle *fwnode)
+{
+	struct pse_control *psec;
+	struct device_node *np;
+
+	if (is_acpi_node(fwnode))
+		return NULL;
+
+	np = to_of_node(fwnode);
+	if (!np)
+		return NULL;
+
+	psec = of_pse_control_get(np);
+	if (IS_ERR_OR_NULL(psec))
+		return NULL;
+
+	return psec;
+}
+
 static struct mii_timestamper *
 fwnode_find_mii_timestamper(struct fwnode_handle *fwnode)
 {
@@ -89,14 +110,21 @@  int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 				struct fwnode_handle *child, u32 addr)
 {
 	struct mii_timestamper *mii_ts = NULL;
+	struct pse_control *psec = NULL;
 	struct phy_device *phy;
 	bool is_c45 = false;
 	u32 phy_id;
 	int rc;
 
+	psec = fwnode_find_pse_control(child);
+	if (IS_ERR(psec))
+		return PTR_ERR(psec);
+
 	mii_ts = fwnode_find_mii_timestamper(child);
-	if (IS_ERR(mii_ts))
-		return PTR_ERR(mii_ts);
+	if (IS_ERR(mii_ts)) {
+		rc = PTR_ERR(mii_ts);
+		goto clean_pse;
+	}
 
 	rc = fwnode_property_match_string(child, "compatible",
 					  "ethernet-phy-ieee802.3-c45");
@@ -132,18 +160,23 @@  int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 			goto clean_phy;
 	}
 
+	phy->psec = psec;
+
 	/* phy->mii_ts may already be defined by the PHY driver. A
 	 * mii_timestamper probed via the device tree will still have
 	 * precedence.
 	 */
 	if (mii_ts)
 		phy->mii_ts = mii_ts;
+
 	return 0;
 
 clean_phy:
 	phy_device_free(phy);
 clean_mii_ts:
 	unregister_mii_timestamper(mii_ts);
+clean_pse:
+	pse_control_put(psec);
 
 	return rc;
 }
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 0c6efd7926907..221bc872ee2fb 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -26,6 +26,7 @@ 
 #include <linux/netdevice.h>
 #include <linux/phy.h>
 #include <linux/phy_led_triggers.h>
+#include <linux/pse-pd/pse.h>
 #include <linux/property.h>
 #include <linux/sfp.h>
 #include <linux/skbuff.h>
@@ -988,6 +989,7 @@  EXPORT_SYMBOL(phy_device_register);
  */
 void phy_device_remove(struct phy_device *phydev)
 {
+	pse_control_put(phydev->psec);
 	unregister_mii_timestamper(phydev->mii_ts);
 
 	device_del(&phydev->mdio.dev);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 87638c55d8442..0c91870b82582 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -588,6 +588,7 @@  struct macsec_ops;
  * @master_slave_get: Current master/slave advertisement
  * @master_slave_state: Current master/slave configuration
  * @mii_ts: Pointer to time stamper callbacks
+ * @psec: Pointer to Power Sourcing Equipment control struct
  * @lock:  Mutex for serialization access to PHY
  * @state_queue: Work queue for state machine
  * @shared: Pointer to private data shared by phys in one package
@@ -701,6 +702,7 @@  struct phy_device {
 	struct phylink *phylink;
 	struct net_device *attached_dev;
 	struct mii_timestamper *mii_ts;
+	struct pse_control *psec;
 
 	u8 mdix;
 	u8 mdix_ctrl;