[net-next 0/5] add fwnode based mdiobus registration

[net-next 0/5] add fwnode based mdiobus registration

[Clément Léger]

In order to allow the mdiobus to be used completely with fwnode and
continue to add fwnode support. This series adds
fwnode_mdiobus_register() which allows to register a MDIO bus with a
fwnode_handle. This support also works with device-tree and thus allows
to integrate of_mdobus_register on top of fwnode support.

ACPI acpi_mdiobus_register() function seems similar enough with
fwnode_mdiobus_register() to be integrated into that later one and thus
remove ACPI specific registration, keeping only the fwnode one for all
types of node. I'm not able to test that specific part so I did not do
it in this series.

This series is a subset of the one that was first submitted as a larger
series to add swnode support [1]. In this one, it will be focused on
fwnode support only since it seems to have reach a consensus that
adding fwnode to subsystems makes sense.

Additional information:

The device I'm trying to support is a PCIe card that uses a lan9662
SoC. This card is meant to be used an ethernet switch with 2 x RJ45
ports and 2 x 10G SFPs. The lan966x SoCs can be used in two different
ways:

 - It can run Linux by itself, on ARM64 cores included in the SoC. This
   use-case of the lan966x is currently being upstreamed, using a
   traditional Device Tree representation of the lan996x HW blocks [1]
   A number of drivers for the different IPs of the SoC have already
   been merged in upstream Linux.

 - It can be used as a PCIe endpoint, connected to a separate platform
   that acts as the PCIe root complex. In this case, all the devices
   that are embedded on this SoC are exposed through PCIe BARs and the
   ARM64 cores of the SoC are not used. Since this is a PCIe card, it
   can be plugged on any platform, of any architecture supporting PCIe.

The goal if this work is to allow OF based drivers to be reused with
software nodes by supporting fwnode in multiple subsystems.

[1] https://lore.kernel.org/netdev/YhPSkz8+BIcdb72R@smile.fi.intel.com/T/

fwnode_i2c_only

Clément Léger (5):
  net: mdio: fwnode: add fwnode_mdiobus_register()
  net: mdio: of: use fwnode_mdiobus_* functions
  net: mdiobus: fwnode: avoid calling of_* functions with non OF nodes
  net: mdiobus: fwnode: allow phy device registration with non OF nodes
  net: mdio: mscc-miim: use fwnode_mdiobus_register()

 drivers/net/mdio/fwnode_mdio.c    | 214 +++++++++++++++++++++++++++++-
 drivers/net/mdio/mdio-mscc-miim.c |   4 +-
 drivers/net/mdio/of_mdio.c        | 187 +-------------------------
 include/linux/fwnode_mdio.h       |  13 ++
 4 files changed, 229 insertions(+), 189 deletions(-)

-- 
2.34.1

[net-next 1/5] net: mdio: fwnode: add fwnode_mdiobus_register()

[Clément Léger]

In order to support software node description transparently, add fwnode
support with fwnode_mdiobus_register(). This function behaves exactly
like of_mdiobus_register() function but using the fwnode node agnostic
API. This support might also be used to merge ACPI mdiobus support
which is quite similar to the fwnode one.

Some part such as the whitelist matching are kept exclusively for OF
nodes since it uses an of_device_id struct and seems tightly coupled
with OF. Other parts are generic and will allow to move the existing
OF support on top of this fwnode version.

Signed-off-by: Clément Léger <clement.leger@bootlin.com>
---
 drivers/net/mdio/fwnode_mdio.c | 210 +++++++++++++++++++++++++++++++++
 include/linux/fwnode_mdio.h    |  13 ++
 2 files changed, 223 insertions(+)

diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index 1becb1a731f6..f9ec3818041a 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -11,6 +11,8 @@
 #include <linux/of.h>
 #include <linux/phy.h>
 
+#define DEFAULT_GPIO_RESET_DELAY	10	/* in microseconds */
+
 MODULE_AUTHOR("Calvin Johnson <calvin.johnson@oss.nxp.com>");
 MODULE_LICENSE("GPL");
 
@@ -142,3 +144,211 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 	return 0;
 }
 EXPORT_SYMBOL(fwnode_mdiobus_register_phy);
+
+/* The following is a list of PHY compatible strings which appear in
+ * some DTBs. The compatible string is never matched against a PHY
+ * driver, so is pointless. We only expect devices which are not PHYs
+ * to have a compatible string, so they can be matched to an MDIO
+ * driver.  Encourage users to upgrade their DT blobs to remove these.
+ */
+static const struct of_device_id whitelist_phys[] = {
+	{ .compatible = "brcm,40nm-ephy" },
+	{ .compatible = "broadcom,bcm5241" },
+	{ .compatible = "marvell,88E1111", },
+	{ .compatible = "marvell,88e1116", },
+	{ .compatible = "marvell,88e1118", },
+	{ .compatible = "marvell,88e1145", },
+	{ .compatible = "marvell,88e1149r", },
+	{ .compatible = "marvell,88e1310", },
+	{ .compatible = "marvell,88E1510", },
+	{ .compatible = "marvell,88E1514", },
+	{ .compatible = "moxa,moxart-rtl8201cp", },
+	{}
+};
+
+/* Return true if the child node is for a phy. It must either:
+ * o Compatible string of "ethernet-phy-idX.X"
+ * o Compatible string of "ethernet-phy-ieee802.3-c45"
+ * o Compatible string of "ethernet-phy-ieee802.3-c22"
+ * o No compatibility string
+ *
+ * A device which is not a phy is expected to have a compatible string
+ * indicating what sort of device it is.
+ */
+bool fwnode_mdiobus_child_is_phy(struct fwnode_handle *child)
+{
+	u32 phy_id;
+
+	if (fwnode_get_phy_id(child, &phy_id) != -EINVAL)
+		return true;
+
+	if (fwnode_property_match_string(child, "compatible",
+					 "ethernet-phy-ieee802.3-c45") >= 0)
+		return true;
+
+	if (fwnode_property_match_string(child, "compatible",
+					 "ethernet-phy-ieee802.3-c22") >= 0)
+		return true;
+
+	if (is_of_node(child) &&
+	    of_match_node(whitelist_phys, to_of_node(child))) {
+		pr_warn(FW_WARN
+			"%s: Whitelisted compatible string. Please remove\n",
+			fwnode_get_name(child));
+		return true;
+	}
+
+	if (!fwnode_property_present(child, "compatible"))
+		return true;
+
+	return false;
+}
+EXPORT_SYMBOL(fwnode_mdiobus_child_is_phy);
+
+static int fwnode_mdiobus_register_device(struct mii_bus *mdio,
+					  struct fwnode_handle *child,
+					  u32 addr)
+{
+	struct mdio_device *mdiodev;
+	int rc;
+
+	mdiodev = mdio_device_create(mdio, addr);
+	if (IS_ERR(mdiodev))
+		return PTR_ERR(mdiodev);
+
+	fwnode_handle_get(child);
+	device_set_node(&mdiodev->dev, child);
+
+	/* All data is now stored in the mdiodev struct; register it. */
+	rc = mdio_device_register(mdiodev);
+	if (rc) {
+		mdio_device_free(mdiodev);
+		fwnode_handle_put(child);
+		return rc;
+	}
+
+	dev_dbg(&mdio->dev, "registered mdio device %s at address %i\n",
+		fwnode_get_name(child), addr);
+	return 0;
+}
+
+static inline int fwnode_mdio_parse_addr(struct device *dev,
+					 const struct fwnode_handle *fwnode)
+{
+	u32 addr;
+	int ret;
+
+	ret = fwnode_property_read_u32(fwnode, "reg", &addr);
+	if (ret < 0) {
+		dev_err(dev, "%s has invalid PHY address\n",
+			fwnode_get_name(fwnode));
+		return ret;
+	}
+
+	/* A PHY must have a reg property in the range [0-31] */
+	if (addr >= PHY_MAX_ADDR) {
+		dev_err(dev, "%s PHY address %i is too large\n",
+			fwnode_get_name(fwnode), addr);
+		return -EINVAL;
+	}
+
+	return addr;
+}
+
+/**
+ * fwnode_mdiobus_register - Register mii_bus and create PHYs from fwnode
+ * @mdio: pointer to mii_bus structure
+ * @fwnode: pointer to fwnode of MDIO bus.
+ *
+ */
+int fwnode_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode)
+{
+	struct fwnode_handle *child;
+	bool scanphys = false;
+	int addr, rc;
+
+	if (!fwnode)
+		return mdiobus_register(mdio);
+
+	if (!fwnode_device_is_available(fwnode))
+		return -ENODEV;
+
+	/* Mask out all PHYs from auto probing.  Instead the PHYs listed in
+	 * the device tree are populated after the bus has been registered
+	 */
+	mdio->phy_mask = ~0;
+
+	device_set_node(&mdio->dev, fwnode);
+
+	/* Get bus level PHY reset GPIO details */
+	mdio->reset_delay_us = DEFAULT_GPIO_RESET_DELAY;
+	fwnode_property_read_u32(fwnode, "reset-delay-us",
+				 &mdio->reset_delay_us);
+	mdio->reset_post_delay_us = 0;
+	fwnode_property_read_u32(fwnode, "reset-post-delay-us",
+				 &mdio->reset_post_delay_us);
+
+	/* Register the MDIO bus */
+	rc = mdiobus_register(mdio);
+	if (rc)
+		return rc;
+
+	/* Loop over the child nodes and register a phy_device for each phy */
+	fwnode_for_each_available_child_node(fwnode, child) {
+		addr = fwnode_mdio_parse_addr(&mdio->dev, child);
+		if (addr < 0) {
+			scanphys = true;
+			continue;
+		}
+
+		if (fwnode_mdiobus_child_is_phy(child))
+			rc = fwnode_mdiobus_register_phy(mdio, child, addr);
+		else
+			rc = fwnode_mdiobus_register_device(mdio, child, addr);
+
+		if (rc == -ENODEV)
+			dev_err(&mdio->dev,
+				"MDIO device at address %d is missing.\n",
+				addr);
+		else if (rc)
+			goto unregister;
+	}
+
+	if (!scanphys)
+		return 0;
+
+	/* auto scan for PHYs with empty reg property */
+	fwnode_for_each_available_child_node(fwnode, child) {
+		/* Skip PHYs with reg property set */
+		if (fwnode_property_present(child, "reg"))
+			continue;
+
+		for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
+			/* skip already registered PHYs */
+			if (mdiobus_is_registered_device(mdio, addr))
+				continue;
+
+			/* be noisy to encourage people to set reg property */
+			dev_info(&mdio->dev, "scan phy %s at address %i\n",
+				 fwnode_get_name(child), addr);
+
+			if (fwnode_mdiobus_child_is_phy(child)) {
+				/* -ENODEV is the return code that PHYLIB has
+				 * standardized on to indicate that bus
+				 * scanning should continue.
+				 */
+				rc = fwnode_mdiobus_register_phy(mdio, child,
+								 addr);
+				if (!rc)
+					break;
+				if (rc != -ENODEV)
+					goto unregister;
+			}
+		}
+	}
+
+unregister:
+	mdiobus_unregister(mdio);
+	return rc;
+}
+EXPORT_SYMBOL(fwnode_mdiobus_register);
diff --git a/include/linux/fwnode_mdio.h b/include/linux/fwnode_mdio.h
index faf603c48c86..ca380050056d 100644
--- a/include/linux/fwnode_mdio.h
+++ b/include/linux/fwnode_mdio.h
@@ -9,6 +9,7 @@
 #include <linux/phy.h>
 
 #if IS_ENABLED(CONFIG_FWNODE_MDIO)
+bool fwnode_mdiobus_child_is_phy(struct fwnode_handle *child);
 int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
 				       struct phy_device *phy,
 				       struct fwnode_handle *child, u32 addr);
@@ -16,7 +17,13 @@ int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
 int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 				struct fwnode_handle *child, u32 addr);
 
+int fwnode_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode);
 #else /* CONFIG_FWNODE_MDIO */
+static inline bool fwnode_mdiobus_child_is_phy(struct fwnode_handle *child)
+{
+	return false;
+}
+
 int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
 				       struct phy_device *phy,
 				       struct fwnode_handle *child, u32 addr)
@@ -30,6 +37,12 @@ static inline int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 {
 	return -EINVAL;
 }
+
+static int fwnode_mdiobus_register(struct mii_bus *mdio,
+				   struct fwnode_handle *fwnode)
+{
+	return -EINVAL;
+}
 #endif
 
 #endif /* __LINUX_FWNODE_MDIO_H */
-- 
2.34.1

[net-next 2/5] net: mdio: of: use fwnode_mdiobus_* functions

[Clément Léger]

Now that fwnode support has been added and implements the same behavior
expected by device-tree parsing, directly call fwnode_mdiobus_*
functions in of_mdio.c. Eventually, the same will be doable for ACPI in
order to use a single parsing method based on fwnode API.

Signed-off-by: Clément Léger <clement.leger@bootlin.com>
---
 drivers/net/mdio/of_mdio.c | 187 +------------------------------------
 1 file changed, 2 insertions(+), 185 deletions(-)

diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index 9e3c815a070f..b53dab9fc78e 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -21,18 +21,9 @@
 #include <linux/phy.h>
 #include <linux/phy_fixed.h>
 
-#define DEFAULT_GPIO_RESET_DELAY	10	/* in microseconds */
-
 MODULE_AUTHOR("Grant Likely <grant.likely@secretlab.ca>");
 MODULE_LICENSE("GPL");
 
-/* Extract the clause 22 phy ID from the compatible string of the form
- * ethernet-phy-idAAAA.BBBB */
-static int of_get_phy_id(struct device_node *device, u32 *phy_id)
-{
-	return fwnode_get_phy_id(of_fwnode_handle(device), phy_id);
-}
-
 int of_mdiobus_phy_device_register(struct mii_bus *mdio, struct phy_device *phy,
 				   struct device_node *child, u32 addr)
 {
@@ -42,98 +33,9 @@ int of_mdiobus_phy_device_register(struct mii_bus *mdio, struct phy_device *phy,
 }
 EXPORT_SYMBOL(of_mdiobus_phy_device_register);
 
-static int of_mdiobus_register_phy(struct mii_bus *mdio,
-				    struct device_node *child, u32 addr)
-{
-	return fwnode_mdiobus_register_phy(mdio, of_fwnode_handle(child), addr);
-}
-
-static int of_mdiobus_register_device(struct mii_bus *mdio,
-				      struct device_node *child, u32 addr)
-{
-	struct fwnode_handle *fwnode = of_fwnode_handle(child);
-	struct mdio_device *mdiodev;
-	int rc;
-
-	mdiodev = mdio_device_create(mdio, addr);
-	if (IS_ERR(mdiodev))
-		return PTR_ERR(mdiodev);
-
-	/* Associate the OF node with the device structure so it
-	 * can be looked up later.
-	 */
-	fwnode_handle_get(fwnode);
-	device_set_node(&mdiodev->dev, fwnode);
-
-	/* All data is now stored in the mdiodev struct; register it. */
-	rc = mdio_device_register(mdiodev);
-	if (rc) {
-		mdio_device_free(mdiodev);
-		of_node_put(child);
-		return rc;
-	}
-
-	dev_dbg(&mdio->dev, "registered mdio device %pOFn at address %i\n",
-		child, addr);
-	return 0;
-}
-
-/* The following is a list of PHY compatible strings which appear in
- * some DTBs. The compatible string is never matched against a PHY
- * driver, so is pointless. We only expect devices which are not PHYs
- * to have a compatible string, so they can be matched to an MDIO
- * driver.  Encourage users to upgrade their DT blobs to remove these.
- */
-static const struct of_device_id whitelist_phys[] = {
-	{ .compatible = "brcm,40nm-ephy" },
-	{ .compatible = "broadcom,bcm5241" },
-	{ .compatible = "marvell,88E1111", },
-	{ .compatible = "marvell,88e1116", },
-	{ .compatible = "marvell,88e1118", },
-	{ .compatible = "marvell,88e1145", },
-	{ .compatible = "marvell,88e1149r", },
-	{ .compatible = "marvell,88e1310", },
-	{ .compatible = "marvell,88E1510", },
-	{ .compatible = "marvell,88E1514", },
-	{ .compatible = "moxa,moxart-rtl8201cp", },
-	{}
-};
-
-/*
- * Return true if the child node is for a phy. It must either:
- * o Compatible string of "ethernet-phy-idX.X"
- * o Compatible string of "ethernet-phy-ieee802.3-c45"
- * o Compatible string of "ethernet-phy-ieee802.3-c22"
- * o In the white list above (and issue a warning)
- * o No compatibility string
- *
- * A device which is not a phy is expected to have a compatible string
- * indicating what sort of device it is.
- */
 bool of_mdiobus_child_is_phy(struct device_node *child)
 {
-	u32 phy_id;
-
-	if (of_get_phy_id(child, &phy_id) != -EINVAL)
-		return true;
-
-	if (of_device_is_compatible(child, "ethernet-phy-ieee802.3-c45"))
-		return true;
-
-	if (of_device_is_compatible(child, "ethernet-phy-ieee802.3-c22"))
-		return true;
-
-	if (of_match_node(whitelist_phys, child)) {
-		pr_warn(FW_WARN
-			"%pOF: Whitelisted compatible string. Please remove\n",
-			child);
-		return true;
-	}
-
-	if (!of_find_property(child, "compatible", NULL))
-		return true;
-
-	return false;
+	return fwnode_mdiobus_child_is_phy(of_fwnode_handle(child));
 }
 EXPORT_SYMBOL(of_mdiobus_child_is_phy);
 
@@ -147,92 +49,7 @@ EXPORT_SYMBOL(of_mdiobus_child_is_phy);
  */
 int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
 {
-	struct device_node *child;
-	bool scanphys = false;
-	int addr, rc;
-
-	if (!np)
-		return mdiobus_register(mdio);
-
-	/* Do not continue if the node is disabled */
-	if (!of_device_is_available(np))
-		return -ENODEV;
-
-	/* Mask out all PHYs from auto probing.  Instead the PHYs listed in
-	 * the device tree are populated after the bus has been registered */
-	mdio->phy_mask = ~0;
-
-	device_set_node(&mdio->dev, of_fwnode_handle(np));
-
-	/* Get bus level PHY reset GPIO details */
-	mdio->reset_delay_us = DEFAULT_GPIO_RESET_DELAY;
-	of_property_read_u32(np, "reset-delay-us", &mdio->reset_delay_us);
-	mdio->reset_post_delay_us = 0;
-	of_property_read_u32(np, "reset-post-delay-us", &mdio->reset_post_delay_us);
-
-	/* Register the MDIO bus */
-	rc = mdiobus_register(mdio);
-	if (rc)
-		return rc;
-
-	/* Loop over the child nodes and register a phy_device for each phy */
-	for_each_available_child_of_node(np, child) {
-		addr = of_mdio_parse_addr(&mdio->dev, child);
-		if (addr < 0) {
-			scanphys = true;
-			continue;
-		}
-
-		if (of_mdiobus_child_is_phy(child))
-			rc = of_mdiobus_register_phy(mdio, child, addr);
-		else
-			rc = of_mdiobus_register_device(mdio, child, addr);
-
-		if (rc == -ENODEV)
-			dev_err(&mdio->dev,
-				"MDIO device at address %d is missing.\n",
-				addr);
-		else if (rc)
-			goto unregister;
-	}
-
-	if (!scanphys)
-		return 0;
-
-	/* auto scan for PHYs with empty reg property */
-	for_each_available_child_of_node(np, child) {
-		/* Skip PHYs with reg property set */
-		if (of_find_property(child, "reg", NULL))
-			continue;
-
-		for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
-			/* skip already registered PHYs */
-			if (mdiobus_is_registered_device(mdio, addr))
-				continue;
-
-			/* be noisy to encourage people to set reg property */
-			dev_info(&mdio->dev, "scan phy %pOFn at address %i\n",
-				 child, addr);
-
-			if (of_mdiobus_child_is_phy(child)) {
-				/* -ENODEV is the return code that PHYLIB has
-				 * standardized on to indicate that bus
-				 * scanning should continue.
-				 */
-				rc = of_mdiobus_register_phy(mdio, child, addr);
-				if (!rc)
-					break;
-				if (rc != -ENODEV)
-					goto unregister;
-			}
-		}
-	}
-
-	return 0;
-
-unregister:
-	mdiobus_unregister(mdio);
-	return rc;
+	return fwnode_mdiobus_register(mdio, of_fwnode_handle(np));
 }
 EXPORT_SYMBOL(of_mdiobus_register);
 
-- 
2.34.1

[net-next 3/5] net: mdiobus: fwnode: avoid calling of_* functions with non OF nodes

[Clément Léger]

Without this check, of_parse_phandle_with_fixed_args() will be called
with whatever the type of the node. Use !is_of_node() which will work
for all node types supported by the fwnode API (ACPI, software nodes).

Signed-off-by: Clément Léger <clement.leger@bootlin.com>
---
 drivers/net/mdio/fwnode_mdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index f9ec3818041a..7f71c0700c55 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -22,7 +22,7 @@ fwnode_find_mii_timestamper(struct fwnode_handle *fwnode)
 	struct of_phandle_args arg;
 	int err;
 
-	if (is_acpi_node(fwnode))
+	if (!is_of_node(fwnode))
 		return NULL;
 
 	err = of_parse_phandle_with_fixed_args(to_of_node(fwnode),
-- 
2.34.1

[net-next 4/5] net: mdiobus: fwnode: allow phy device registration with non OF nodes

[Clément Léger]

When using a software node, we also want to call
fwnode_mdiobus_phy_device_register() which support all nodes type.
Remove the is_of_node() check to allow that.

Signed-off-by: Clément Léger <clement.leger@bootlin.com>
---
 drivers/net/mdio/fwnode_mdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index 7f71c0700c55..9f9dc56f03aa 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -126,7 +126,7 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 			fwnode_handle_put(phy->mdio.dev.fwnode);
 			return rc;
 		}
-	} else if (is_of_node(child)) {
+	} else {
 		rc = fwnode_mdiobus_phy_device_register(bus, phy, child, addr);
 		if (rc) {
 			unregister_mii_timestamper(mii_ts);
-- 
2.34.1

[net-next 5/5] net: mdio: mscc-miim: use fwnode_mdiobus_register()

[Clément Léger]

Use fwnode_mdiobus_register() to be compatible with devices described
with fwnode.

Signed-off-by: Clément Léger <clement.leger@bootlin.com>
---
 drivers/net/mdio/mdio-mscc-miim.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mdio/mdio-mscc-miim.c b/drivers/net/mdio/mdio-mscc-miim.c
index c483ba67c21f..ea79421fcfd4 100644
--- a/drivers/net/mdio/mdio-mscc-miim.c
+++ b/drivers/net/mdio/mdio-mscc-miim.c
@@ -7,12 +7,12 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/fwnode_mdio.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
 #include <linux/mdio/mdio-mscc-miim.h>
 #include <linux/module.h>
-#include <linux/of_mdio.h>
 #include <linux/phy.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
@@ -288,7 +288,7 @@ static int mscc_miim_probe(struct platform_device *pdev)
 	if (!miim->info)
 		return -EINVAL;
 
-	ret = of_mdiobus_register(bus, pdev->dev.of_node);
+	ret = fwnode_mdiobus_register(bus, dev_fwnode(&pdev->dev));
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
 		return ret;
-- 
2.34.1

Re: [net-next 2/5] net: mdio: of: use fwnode_mdiobus_* functions

[Andrew Lunn]

On Fri, Mar 25, 2022 at 06:22:31PM +0100, Clément Léger wrote:
> Now that fwnode support has been added and implements the same behavior
> expected by device-tree parsing

The problem is, we cannot actually see that. There is no side by side
comparison which makes it clear it has the same behaviour.

Please see if something like this will work:

1/4: copy drivers/net/mdio/of_mdio.c to drivers/net/mdio/fwnode_mdio.c
2/4: Delete from fwnode_mdio.c the bits you don't need, like the whitelist
3/4: modify what is left of fwnode_mdio.c to actually use fwnode.
4/4: Rework of_mdio.c to use the code in fwnode_mdio.c

The 3/4 should make it clear it has the same behaviour, because we can
see what you have actually changed.

FYI: net-next is closed at the moment, so you need to post RFC
patches.

    Andrew

Re: [net-next 1/5] net: mdio: fwnode: add fwnode_mdiobus_register()

[Andrew Lunn]

On Fri, Mar 25, 2022 at 06:22:30PM +0100, Clément Léger wrote:
> In order to support software node description transparently, add fwnode
> support with fwnode_mdiobus_register(). This function behaves exactly
> like of_mdiobus_register() function but using the fwnode node agnostic
> API. This support might also be used to merge ACPI mdiobus support
> which is quite similar to the fwnode one.
> 
> Some part such as the whitelist matching are kept exclusively for OF
> nodes since it uses an of_device_id struct and seems tightly coupled
> with OF. Other parts are generic and will allow to move the existing
> OF support on top of this fwnode version.

Does fwnode have any documentation? How does a developer know what
properties can be passed? Should you be adding a

Documentation/fwnode/bindings/net/mdio.yaml ?

	Andrew

Re: [net-next 5/5] net: mdio: mscc-miim: use fwnode_mdiobus_register()

[kernel test robot]

Hi "Clément,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Cl-ment-L-ger/net-mdio-fwnode-add-fwnode_mdiobus_register/20220326-040146
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 89695196f0ba78a17453f9616355f2ca6b293402
config: riscv-randconfig-r003-20220325 (https://download.01.org/0day-ci/archive/20220326/202203261006.uZruvRKb-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 0f6d9501cf49ce02937099350d08f20c4af86f3d)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/24673ffe7514a73903a3bc49585846bce1de8748
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Cl-ment-L-ger/net-mdio-fwnode-add-fwnode_mdiobus_register/20220326-040146
        git checkout 24673ffe7514a73903a3bc49585846bce1de8748
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/net/mdio/ fs/gfs2/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/net/mdio/mdio-mscc-miim.c:10:
>> include/linux/fwnode_mdio.h:27:5: warning: no previous prototype for function 'fwnode_mdiobus_phy_device_register' [-Wmissing-prototypes]
   int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
       ^
   include/linux/fwnode_mdio.h:27:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
   ^
   static 
   1 warning generated.


vim +/fwnode_mdiobus_phy_device_register +27 include/linux/fwnode_mdio.h

8ed21cbab4f71b Clément Léger  2022-03-25  26  
bc1bee3b87ee48 Calvin Johnson 2021-06-11 @27  int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
bc1bee3b87ee48 Calvin Johnson 2021-06-11  28  				       struct phy_device *phy,
bc1bee3b87ee48 Calvin Johnson 2021-06-11  29  				       struct fwnode_handle *child, u32 addr)
bc1bee3b87ee48 Calvin Johnson 2021-06-11  30  {
bc1bee3b87ee48 Calvin Johnson 2021-06-11  31  	return -EINVAL;
bc1bee3b87ee48 Calvin Johnson 2021-06-11  32  }
bc1bee3b87ee48 Calvin Johnson 2021-06-11  33  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [net-next 1/5] net: mdio: fwnode: add fwnode_mdiobus_register()

[kernel test robot]

Hi "Clément,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Cl-ment-L-ger/net-mdio-fwnode-add-fwnode_mdiobus_register/20220326-040146
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 89695196f0ba78a17453f9616355f2ca6b293402
config: x86_64-randconfig-c007 (https://download.01.org/0day-ci/archive/20220326/202203261037.wkZLCKMl-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 0f6d9501cf49ce02937099350d08f20c4af86f3d)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/8ed21cbab4f71b382b70d22da18e9331bd9b714f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Cl-ment-L-ger/net-mdio-fwnode-add-fwnode_mdiobus_register/20220326-040146
        git checkout 8ed21cbab4f71b382b70d22da18e9331bd9b714f
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/net/mdio/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/net/mdio/fwnode_mdio.c:154:34: warning: unused variable 'whitelist_phys' [-Wunused-const-variable]
   static const struct of_device_id whitelist_phys[] = {
                                    ^
   1 warning generated.


vim +/whitelist_phys +154 drivers/net/mdio/fwnode_mdio.c

   147	
   148	/* The following is a list of PHY compatible strings which appear in
   149	 * some DTBs. The compatible string is never matched against a PHY
   150	 * driver, so is pointless. We only expect devices which are not PHYs
   151	 * to have a compatible string, so they can be matched to an MDIO
   152	 * driver.  Encourage users to upgrade their DT blobs to remove these.
   153	 */
 > 154	static const struct of_device_id whitelist_phys[] = {
   155		{ .compatible = "brcm,40nm-ephy" },
   156		{ .compatible = "broadcom,bcm5241" },
   157		{ .compatible = "marvell,88E1111", },
   158		{ .compatible = "marvell,88e1116", },
   159		{ .compatible = "marvell,88e1118", },
   160		{ .compatible = "marvell,88e1145", },
   161		{ .compatible = "marvell,88e1149r", },
   162		{ .compatible = "marvell,88e1310", },
   163		{ .compatible = "marvell,88E1510", },
   164		{ .compatible = "marvell,88E1514", },
   165		{ .compatible = "moxa,moxart-rtl8201cp", },
   166		{}
   167	};
   168	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [net-next 1/5] net: mdio: fwnode: add fwnode_mdiobus_register()

[kernel test robot]

Hi "Clément,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Cl-ment-L-ger/net-mdio-fwnode-add-fwnode_mdiobus_register/20220326-040146
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 89695196f0ba78a17453f9616355f2ca6b293402
config: x86_64-randconfig-a005 (https://download.01.org/0day-ci/archive/20220326/202203261007.nhIuHNPd-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 0f6d9501cf49ce02937099350d08f20c4af86f3d)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/8ed21cbab4f71b382b70d22da18e9331bd9b714f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Cl-ment-L-ger/net-mdio-fwnode-add-fwnode_mdiobus_register/20220326-040146
        git checkout 8ed21cbab4f71b382b70d22da18e9331bd9b714f
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/net/mdio/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/net/mdio/fwnode_mdio.c:154:34: warning: unused variable 'whitelist_phys' [-Wunused-const-variable]
   static const struct of_device_id whitelist_phys[] = {
                                    ^
   1 warning generated.


vim +/whitelist_phys +154 drivers/net/mdio/fwnode_mdio.c

   147	
   148	/* The following is a list of PHY compatible strings which appear in
   149	 * some DTBs. The compatible string is never matched against a PHY
   150	 * driver, so is pointless. We only expect devices which are not PHYs
   151	 * to have a compatible string, so they can be matched to an MDIO
   152	 * driver.  Encourage users to upgrade their DT blobs to remove these.
   153	 */
 > 154	static const struct of_device_id whitelist_phys[] = {
   155		{ .compatible = "brcm,40nm-ephy" },
   156		{ .compatible = "broadcom,bcm5241" },
   157		{ .compatible = "marvell,88E1111", },
   158		{ .compatible = "marvell,88e1116", },
   159		{ .compatible = "marvell,88e1118", },
   160		{ .compatible = "marvell,88e1145", },
   161		{ .compatible = "marvell,88e1149r", },
   162		{ .compatible = "marvell,88e1310", },
   163		{ .compatible = "marvell,88E1510", },
   164		{ .compatible = "marvell,88E1514", },
   165		{ .compatible = "moxa,moxart-rtl8201cp", },
   166		{}
   167	};
   168	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [net-next 1/5] net: mdio: fwnode: add fwnode_mdiobus_register()

[Clément Léger]

Le Fri, 25 Mar 2022 19:38:24 +0100,
Andrew Lunn <andrew@lunn.ch> a écrit :

> On Fri, Mar 25, 2022 at 06:22:30PM +0100, Clément Léger wrote:
> > In order to support software node description transparently, add fwnode
> > support with fwnode_mdiobus_register(). This function behaves exactly
> > like of_mdiobus_register() function but using the fwnode node agnostic
> > API. This support might also be used to merge ACPI mdiobus support
> > which is quite similar to the fwnode one.
> > 
> > Some part such as the whitelist matching are kept exclusively for OF
> > nodes since it uses an of_device_id struct and seems tightly coupled
> > with OF. Other parts are generic and will allow to move the existing
> > OF support on top of this fwnode version.  
> 
> Does fwnode have any documentation? How does a developer know what
> properties can be passed? Should you be adding a
> 
> Documentation/fwnode/bindings/net/mdio.yaml ?
> 
> 	Andrew

Hi Andrew,

Actually, fwnode is an abstraction for various firmware nodes such as
ACPI, device-tree and software nodes. It allows to access properties,
child and other attributes transparently from these various nodes but
does not actually defines how they should describe the hardware. If
there is specific hanling to be done, node type can be checked using
is_acpi_node(), is_of_node() and so on.

I think it is still needed to document the bindings for each node type.

-- 
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com

Re: [net-next 2/5] net: mdio: of: use fwnode_mdiobus_* functions

[Clément Léger]

Le Fri, 25 Mar 2022 19:32:45 +0100,
Andrew Lunn <andrew@lunn.ch> a écrit :

> On Fri, Mar 25, 2022 at 06:22:31PM +0100, Clément Léger wrote:
> > Now that fwnode support has been added and implements the same behavior
> > expected by device-tree parsing  
> 
> The problem is, we cannot actually see that. There is no side by side
> comparison which makes it clear it has the same behaviour.
> 
> Please see if something like this will work:
> 
> 1/4: copy drivers/net/mdio/of_mdio.c to drivers/net/mdio/fwnode_mdio.c
> 2/4: Delete from fwnode_mdio.c the bits you don't need, like the whitelist
> 3/4: modify what is left of fwnode_mdio.c to actually use fwnode.
> 4/4: Rework of_mdio.c to use the code in fwnode_mdio.c
> 
> The 3/4 should make it clear it has the same behaviour, because we can
> see what you have actually changed.

Indeed, that would be more clear to provide this as separate patches
that shows clearly the conversion. Will do that.

> 
> FYI: net-next is closed at the moment, so you need to post RFC
> patches.

Ok, I refered to http://vger.kernel.org/~davem/net-next.html which
seems wrong actually

> 
>     Andrew

Thanks

-- 
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com

Re: [net-next 1/5] net: mdio: fwnode: add fwnode_mdiobus_register()

[Andrew Lunn]

On Mon, Mar 28, 2022 at 08:26:42AM +0200, Clément Léger wrote:
> Le Fri, 25 Mar 2022 19:38:24 +0100,
> Andrew Lunn <andrew@lunn.ch> a écrit :
> 
> > On Fri, Mar 25, 2022 at 06:22:30PM +0100, Clément Léger wrote:
> > > In order to support software node description transparently, add fwnode
> > > support with fwnode_mdiobus_register(). This function behaves exactly
> > > like of_mdiobus_register() function but using the fwnode node agnostic
> > > API. This support might also be used to merge ACPI mdiobus support
> > > which is quite similar to the fwnode one.
> > > 
> > > Some part such as the whitelist matching are kept exclusively for OF
> > > nodes since it uses an of_device_id struct and seems tightly coupled
> > > with OF. Other parts are generic and will allow to move the existing
> > > OF support on top of this fwnode version.  
> > 
> > Does fwnode have any documentation? How does a developer know what
> > properties can be passed? Should you be adding a
> > 
> > Documentation/fwnode/bindings/net/mdio.yaml ?
> > 
> > 	Andrew
> 
> Hi Andrew,
> 
> Actually, fwnode is an abstraction for various firmware nodes such as
> ACPI, device-tree and software nodes. It allows to access properties,
> child and other attributes transparently from these various nodes but
> does not actually defines how they should describe the hardware. If
> there is specific hanling to be done, node type can be checked using
> is_acpi_node(), is_of_node() and so on.
> 
> I think it is still needed to document the bindings for each node type.

But you seem to be implementing a subset of what each node type
supports. So maybe it would be good to document which parts of the OF
binding can be used, which parts of the ACPI binding can be used, etc.

	Andrew

Re: [net-next 1/5] net: mdio: fwnode: add fwnode_mdiobus_register()

[Clément Léger]

Le Mon, 28 Mar 2022 15:03:16 +0200,
Andrew Lunn <andrew@lunn.ch> a écrit :

> > > 
> > > Does fwnode have any documentation? How does a developer know what
> > > properties can be passed? Should you be adding a
> > > 
> > > Documentation/fwnode/bindings/net/mdio.yaml ?
> > > 
> > > 	Andrew  
> > 
> > Hi Andrew,
> > 
> > Actually, fwnode is an abstraction for various firmware nodes such as
> > ACPI, device-tree and software nodes. It allows to access properties,
> > child and other attributes transparently from these various nodes but
> > does not actually defines how they should describe the hardware. If
> > there is specific hanling to be done, node type can be checked using
> > is_acpi_node(), is_of_node() and so on.
> > 
> > I think it is still needed to document the bindings for each node type.  
> 
> But you seem to be implementing a subset of what each node type
> supports. So maybe it would be good to document which parts of the OF
> binding can be used, which parts of the ACPI binding can be used, etc.

With this series, fwnode_mdiobus_register() supports exactly the same
subset that is supported by of_mdiobus_register(). This is not true for
ACPI though, but I could easily add this support providing that someone
could test it. Or I can left it as is and document that ACPI is not
supported and add some checks to avoid fwnode_mdiobus_register being
called with an ACPI node. What would you prefer ?

The goal in the end is to be able to use only fwnode_mdiobus_register()
to register mdiobus device whatever the node type.

> 
> 	Andrew


-- 
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com

Re: [net-next 1/5] net: mdio: fwnode: add fwnode_mdiobus_register()

[Andrew Lunn]

> With this series, fwnode_mdiobus_register() supports exactly the same
> subset that is supported by of_mdiobus_register().

I need to see the side-by-side conversion. But it looked to me you did
not support everything in DT.

And another question is, should it support everything in DT. The DT
binding has things which are deprecated. We have to support them in
DT, they are ABI. But anything new should not be using them.

    Andrew

Re: [net-next 1/5] net: mdio: fwnode: add fwnode_mdiobus_register()

[Clément Léger]

Le Mon, 28 Mar 2022 16:12:33 +0200,
Andrew Lunn <andrew@lunn.ch> a écrit :

> > With this series, fwnode_mdiobus_register() supports exactly the same
> > subset that is supported by of_mdiobus_register().  
> 
> I need to see the side-by-side conversion. But it looked to me you did
> not support everything in DT.

I splat the conversion as you requested and it make it clear that it's
a 1:1 conversion. But indeed, it will be better by looking at patches.

> 
> And another question is, should it support everything in DT. The DT
> binding has things which are deprecated. We have to support them in
> DT, they are ABI. But anything new should not be using them.

Ok, so maybe, the fwnode support should stop supporting these
deprecated features. From what I can see, there is at least the
following two things:

- Whitelist id table (seems to check legacy compatible strings)
- Scanning of child nodes that don't have a reg property. reg is
  specified as required in the bindings so it's probably not a good
  thing to keep that.

So maybe these features can be removed from the fwnode support.

Thanks,

-- 
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com