diff mbox series

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

Message ID 20220325172234.1259667-3-clement.leger@bootlin.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series add fwnode based mdiobus registration | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 210 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Clément Léger March 25, 2022, 5:22 p.m. UTC
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(-)

Comments

Andrew Lunn March 25, 2022, 6:32 p.m. UTC | #1
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
Clément Léger March 28, 2022, 7:53 a.m. UTC | #2
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
diff mbox series

Patch

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);