[net-next: PATCH v3 0/8] DSA: switch to fwnode_/device_

[net-next: PATCH v3 0/8] DSA: switch to fwnode_/device_

[Marcin Wojtas]

Hi,

This is a re-spin of the DSA migration to fwnode_/device_ API.
It addresses all comments from the previous iteration - the
details are summarized in the changelog section below.

This time the patchset is based and tested on top of
pure net-next/main branch. Each commit was checked on:
* On EspressoBIN
* On SolidRun CN913x CEx7 Eval Board 

Any comments or remarks will be appreciated.

Best regards,
Marcin

Changelog v2 -> v3:
1/8:
  * Replace forward declaration s/device_node/fwnode_handle/ in
    include/linux/phy_fixed.h
  * Add Florian's RB

* 3/8:
  * Extend lines width in the commit message.
  * While dropping dp->dn fields in the drivers, switch to
    fwnode_ API in the updated places.

* 5/8:
  * Update routine name to fwnode_find_parent_dev_match()
  * Improve comment section
  * Move the definition adjacent to a group of fwnode
    APIs operating on parents

Changelog v1 -> v2:
1/8
  * Drop unnecessary check in fixed_phy_get_gpiod()
  * Improve line breaking
  * Use device_set_node & dev_fwnode

2/8
  * Switch to fwnode_property_count_u32 and fix comparison
    in if statement.

3/8
  * Drop dn usage entirely and use dp->fwnode only. Update
    all dependent drivers to use to_of_node.
  * Use device_set_node, dev_fwnode & device_get_named_child_node
  * Replace '_of' routines suffix with '_fw'

4/8
  * Use device_set_node

5/8
  * New patch

6/8 
  * Use device_match_fwnode
  * Restore EXPORT_SYMBOL()

7/8
  * Get rid of of_mdiobus_register_device 

8/8
  * Use dev_fwnode in mv88e6xxx_probe 
  * Simplify condition checks in mv88e6xxx_probe as suggested by Andy

Marcin Wojtas (8):
  net: phy: fixed_phy: switch to fwnode_ API
  net: mdio: switch fixed-link PHYs API to fwnode_
  net: dsa: switch to device_/fwnode_ APIs
  net: mvpp2: initialize port fwnode pointer
  device property: introduce fwnode_find_parent_dev_match
  net: core: switch to fwnode_find_net_device_by_node()
  net: mdio: introduce fwnode_mdiobus_register_device()
  net: dsa: mv88e6xxx: switch to device_/fwnode_ APIs

 include/linux/etherdevice.h                     |   1 +
 include/linux/fwnode_mdio.h                     |  22 ++++
 include/linux/of_net.h                          |   6 -
 include/linux/phy_fixed.h                       |   6 +-
 include/linux/property.h                        |   1 +
 include/net/dsa.h                               |   2 +-
 net/dsa/dsa_priv.h                              |   4 +-
 drivers/base/property.c                         |  23 ++++
 drivers/net/dsa/mt7530.c                        |   6 +-
 drivers/net/dsa/mv88e6xxx/chip.c                |  57 ++++-----
 drivers/net/dsa/qca/qca8k.c                     |   2 +-
 drivers/net/dsa/realtek/rtl8365mb.c             |   2 +-
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c |   2 +-
 drivers/net/mdio/fwnode_mdio.c                  | 129 ++++++++++++++++++++
 drivers/net/mdio/of_mdio.c                      | 111 +----------------
 drivers/net/phy/fixed_phy.c                     |  39 +++---
 net/core/net-sysfs.c                            |  25 ++--
 net/dsa/dsa2.c                                  | 101 ++++++++-------
 net/dsa/port.c                                  |  68 +++++------
 net/dsa/slave.c                                 |   7 +-
 20 files changed, 329 insertions(+), 285 deletions(-)

-- 
2.29.0

[net-next: PATCH v3 1/8] net: phy: fixed_phy: switch to fwnode_ API

[Marcin Wojtas]

This patch allows to use fixed_phy driver and its helper
functions without Device Tree dependency, by swtiching from
of_ to fwnode_ API.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 include/linux/phy_fixed.h   |  6 +--
 drivers/net/mdio/of_mdio.c  |  2 +-
 drivers/net/phy/fixed_phy.c | 39 +++++++-------------
 3 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h
index 52bc8e487ef7..e3a03494f218 100644
--- a/include/linux/phy_fixed.h
+++ b/include/linux/phy_fixed.h
@@ -10,7 +10,7 @@ struct fixed_phy_status {
 	int asym_pause;
 };
 
-struct device_node;
+struct fwnode_handle;
 struct gpio_desc;
 
 #if IS_ENABLED(CONFIG_FIXED_PHY)
@@ -19,7 +19,7 @@ extern int fixed_phy_add(unsigned int irq, int phy_id,
 			 struct fixed_phy_status *status);
 extern struct phy_device *fixed_phy_register(unsigned int irq,
 					     struct fixed_phy_status *status,
-					     struct device_node *np);
+					     struct fwnode_handle *fwnode);
 
 extern struct phy_device *
 fixed_phy_register_with_gpiod(unsigned int irq,
@@ -38,7 +38,7 @@ static inline int fixed_phy_add(unsigned int irq, int phy_id,
 }
 static inline struct phy_device *fixed_phy_register(unsigned int irq,
 						struct fixed_phy_status *status,
-						struct device_node *np)
+						struct fwnode_handle *fwnode)
 {
 	return ERR_PTR(-ENODEV);
 }
diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index 9e3c815a070f..d755fe1ecdda 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -421,7 +421,7 @@ int of_phy_register_fixed_link(struct device_node *np)
 	return -ENODEV;
 
 register_phy:
-	return PTR_ERR_OR_ZERO(fixed_phy_register(PHY_POLL, &status, np));
+	return PTR_ERR_OR_ZERO(fixed_phy_register(PHY_POLL, &status, of_fwnode_handle(np)));
 }
 EXPORT_SYMBOL(of_phy_register_fixed_link);
 
diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index aef739c20ac4..e59d186f78e6 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -15,9 +15,9 @@
 #include <linux/mii.h>
 #include <linux/phy.h>
 #include <linux/phy_fixed.h>
+#include <linux/property.h>
 #include <linux/err.h>
 #include <linux/slab.h>
-#include <linux/of.h>
 #include <linux/gpio/consumer.h>
 #include <linux/idr.h>
 #include <linux/netdevice.h>
@@ -186,16 +186,12 @@ static void fixed_phy_del(int phy_addr)
 	}
 }
 
-#ifdef CONFIG_OF_GPIO
-static struct gpio_desc *fixed_phy_get_gpiod(struct device_node *np)
+static struct gpio_desc *fixed_phy_get_gpiod(struct fwnode_handle *fwnode)
 {
-	struct device_node *fixed_link_node;
+	struct fwnode_handle *fixed_link_node;
 	struct gpio_desc *gpiod;
 
-	if (!np)
-		return NULL;
-
-	fixed_link_node = of_get_child_by_name(np, "fixed-link");
+	fixed_link_node = fwnode_get_named_child_node(fwnode, "fixed-link");
 	if (!fixed_link_node)
 		return NULL;
 
@@ -204,28 +200,21 @@ static struct gpio_desc *fixed_phy_get_gpiod(struct device_node *np)
 	 * Linux device associated with it, we simply have obtain
 	 * the GPIO descriptor from the device tree like this.
 	 */
-	gpiod = fwnode_gpiod_get_index(of_fwnode_handle(fixed_link_node),
-				       "link", 0, GPIOD_IN, "mdio");
+	gpiod = fwnode_gpiod_get_index(fixed_link_node, "link", 0, GPIOD_IN, "mdio");
 	if (IS_ERR(gpiod) && PTR_ERR(gpiod) != -EPROBE_DEFER) {
 		if (PTR_ERR(gpiod) != -ENOENT)
 			pr_err("error getting GPIO for fixed link %pOF, proceed without\n",
 			       fixed_link_node);
 		gpiod = NULL;
 	}
-	of_node_put(fixed_link_node);
+	fwnode_handle_put(fixed_link_node);
 
 	return gpiod;
 }
-#else
-static struct gpio_desc *fixed_phy_get_gpiod(struct device_node *np)
-{
-	return NULL;
-}
-#endif
 
 static struct phy_device *__fixed_phy_register(unsigned int irq,
 					       struct fixed_phy_status *status,
-					       struct device_node *np,
+					       struct fwnode_handle *fwnode,
 					       struct gpio_desc *gpiod)
 {
 	struct fixed_mdio_bus *fmb = &platform_fmb;
@@ -238,7 +227,7 @@ static struct phy_device *__fixed_phy_register(unsigned int irq,
 
 	/* Check if we have a GPIO associated with this fixed phy */
 	if (!gpiod) {
-		gpiod = fixed_phy_get_gpiod(np);
+		gpiod = fixed_phy_get_gpiod(fwnode);
 		if (IS_ERR(gpiod))
 			return ERR_CAST(gpiod);
 	}
@@ -269,8 +258,8 @@ static struct phy_device *__fixed_phy_register(unsigned int irq,
 		phy->asym_pause = status->asym_pause;
 	}
 
-	of_node_get(np);
-	phy->mdio.dev.of_node = np;
+	fwnode_handle_get(fwnode);
+	device_set_node(&phy->mdio.dev, fwnode);
 	phy->is_pseudo_fixed_link = true;
 
 	switch (status->speed) {
@@ -299,7 +288,7 @@ static struct phy_device *__fixed_phy_register(unsigned int irq,
 	ret = phy_device_register(phy);
 	if (ret) {
 		phy_device_free(phy);
-		of_node_put(np);
+		fwnode_handle_put(fwnode);
 		fixed_phy_del(phy_addr);
 		return ERR_PTR(ret);
 	}
@@ -309,9 +298,9 @@ static struct phy_device *__fixed_phy_register(unsigned int irq,
 
 struct phy_device *fixed_phy_register(unsigned int irq,
 				      struct fixed_phy_status *status,
-				      struct device_node *np)
+				      struct fwnode_handle *fwnode)
 {
-	return __fixed_phy_register(irq, status, np, NULL);
+	return __fixed_phy_register(irq, status, fwnode, NULL);
 }
 EXPORT_SYMBOL_GPL(fixed_phy_register);
 
@@ -327,7 +316,7 @@ EXPORT_SYMBOL_GPL(fixed_phy_register_with_gpiod);
 void fixed_phy_unregister(struct phy_device *phy)
 {
 	phy_device_remove(phy);
-	of_node_put(phy->mdio.dev.of_node);
+	fwnode_handle_put(dev_fwnode(&phy->mdio.dev));
 	fixed_phy_del(phy->mdio.addr);
 }
 EXPORT_SYMBOL_GPL(fixed_phy_unregister);
-- 
2.29.0

Re: [net-next: PATCH v3 1/8] net: phy: fixed_phy: switch to fwnode_ API

[Andy Shevchenko]

On Wed, Jul 27, 2022 at 08:43:14AM +0200, Marcin Wojtas wrote:
> This patch allows to use fixed_phy driver and its helper

Check "Submitting Patches" for "This patch..." and fix your message
accordingly.

> functions without Device Tree dependency, by swtiching from
> of_ to fwnode_ API.

-- 
With Best Regards,
Andy Shevchenko

[net-next: PATCH v3 2/8] net: mdio: switch fixed-link PHYs API to fwnode_

[Marcin Wojtas]

fixed-link PHYs API is used by DSA and a number of drivers
and was depending on of_. Switch to fwnode_ so to make it
hardware description agnostic and allow to be used in ACPI
world as well.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 include/linux/fwnode_mdio.h    |  19 ++++
 drivers/net/mdio/fwnode_mdio.c | 100 ++++++++++++++++++++
 drivers/net/mdio/of_mdio.c     |  79 +---------------
 3 files changed, 122 insertions(+), 76 deletions(-)

diff --git a/include/linux/fwnode_mdio.h b/include/linux/fwnode_mdio.h
index faf603c48c86..98755b8c6c8a 100644
--- a/include/linux/fwnode_mdio.h
+++ b/include/linux/fwnode_mdio.h
@@ -16,6 +16,11 @@ 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_phy_register_fixed_link(struct fwnode_handle *fwnode);
+
+void fwnode_phy_deregister_fixed_link(struct fwnode_handle *fwnode);
+
+bool fwnode_phy_is_fixed_link(struct fwnode_handle *fwnode);
 #else /* CONFIG_FWNODE_MDIO */
 int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
 				       struct phy_device *phy,
@@ -30,6 +35,20 @@ static inline int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 {
 	return -EINVAL;
 }
+
+static inline int fwnode_phy_register_fixed_link(struct fwnode_handle *fwnode)
+{
+	return -ENODEV;
+}
+
+static inline void fwnode_phy_deregister_fixed_link(struct fwnode_handle *fwnode)
+{
+}
+
+static inline bool fwnode_phy_is_fixed_link(struct fwnode_handle *fwnode)
+{
+	return false;
+}
 #endif
 
 #endif /* __LINUX_FWNODE_MDIO_H */
diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index 1c1584fca632..454fdae24150 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -10,6 +10,7 @@
 #include <linux/fwnode_mdio.h>
 #include <linux/of.h>
 #include <linux/phy.h>
+#include <linux/phy_fixed.h>
 
 MODULE_AUTHOR("Calvin Johnson <calvin.johnson@oss.nxp.com>");
 MODULE_LICENSE("GPL");
@@ -147,3 +148,102 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 	return 0;
 }
 EXPORT_SYMBOL(fwnode_mdiobus_register_phy);
+
+/*
+ * fwnode_phy_is_fixed_link() and fwnode_phy_register_fixed_link() must
+ * support two bindings:
+ * - the old binding, where 'fixed-link' was a property with 5
+ *   cells encoding various information about the fixed PHY
+ * - the new binding, where 'fixed-link' is a sub-node of the
+ *   Ethernet device.
+ */
+bool fwnode_phy_is_fixed_link(struct fwnode_handle *fwnode)
+{
+	struct fwnode_handle *fixed_link_node;
+	const char *managed;
+	int len;
+
+	/* New binding */
+	fixed_link_node = fwnode_get_named_child_node(fwnode, "fixed-link");
+	if (fixed_link_node) {
+		fwnode_handle_put(fixed_link_node);
+		return true;
+	}
+
+	if (fwnode_property_read_string(fwnode, "managed", &managed) == 0 &&
+	    strcmp(managed, "auto") != 0)
+		return true;
+
+	/* Old binding */
+	len = fwnode_property_count_u32(fwnode, "fixed-link");
+	if (len == 5)
+		return true;
+
+	return false;
+}
+EXPORT_SYMBOL(fwnode_phy_is_fixed_link);
+
+int fwnode_phy_register_fixed_link(struct fwnode_handle *fwnode)
+{
+	struct fixed_phy_status status = {};
+	struct fwnode_handle *fixed_link_node;
+	u32 fixed_link_prop[5];
+	const char *managed;
+
+	if (fwnode_property_read_string(fwnode, "managed", &managed) == 0 &&
+	    strcmp(managed, "in-band-status") == 0) {
+		/* status is zeroed, namely its .link member */
+		goto register_phy;
+	}
+
+	/* New binding */
+	fixed_link_node = fwnode_get_named_child_node(fwnode, "fixed-link");
+	if (fixed_link_node) {
+		status.link = 1;
+		status.duplex = fwnode_property_present(fixed_link_node,
+							"full-duplex");
+		if (fwnode_property_read_u32(fixed_link_node, "speed",
+					     &status.speed)) {
+			fwnode_handle_put(fixed_link_node);
+			return -EINVAL;
+		}
+		status.pause = fwnode_property_present(fixed_link_node, "pause");
+		status.asym_pause = fwnode_property_present(fixed_link_node,
+							    "asym-pause");
+		fwnode_handle_put(fixed_link_node);
+
+		goto register_phy;
+	}
+
+	/* Old binding */
+	if (fwnode_property_read_u32_array(fwnode, "fixed-link", fixed_link_prop,
+					   ARRAY_SIZE(fixed_link_prop)) == 0) {
+		status.link = 1;
+		status.duplex = fixed_link_prop[1];
+		status.speed  = fixed_link_prop[2];
+		status.pause  = fixed_link_prop[3];
+		status.asym_pause = fixed_link_prop[4];
+		goto register_phy;
+	}
+
+	return -ENODEV;
+
+register_phy:
+	return PTR_ERR_OR_ZERO(fixed_phy_register(PHY_POLL, &status, fwnode));
+}
+EXPORT_SYMBOL(fwnode_phy_register_fixed_link);
+
+void fwnode_phy_deregister_fixed_link(struct fwnode_handle *fwnode)
+{
+	struct phy_device *phydev;
+
+	phydev = fwnode_phy_find_device(fwnode);
+	if (!phydev)
+		return;
+
+	fixed_phy_unregister(phydev);
+
+	put_device(&phydev->mdio.dev);	/* fwnode_phy_find_device() */
+	phy_device_free(phydev);	/* fixed_phy_register() */
+}
+EXPORT_SYMBOL(fwnode_phy_deregister_fixed_link);
diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index d755fe1ecdda..409da6e92f7d 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -351,91 +351,18 @@ EXPORT_SYMBOL(of_phy_get_and_connect);
  */
 bool of_phy_is_fixed_link(struct device_node *np)
 {
-	struct device_node *dn;
-	int len, err;
-	const char *managed;
-
-	/* New binding */
-	dn = of_get_child_by_name(np, "fixed-link");
-	if (dn) {
-		of_node_put(dn);
-		return true;
-	}
-
-	err = of_property_read_string(np, "managed", &managed);
-	if (err == 0 && strcmp(managed, "auto") != 0)
-		return true;
-
-	/* Old binding */
-	if (of_get_property(np, "fixed-link", &len) &&
-	    len == (5 * sizeof(__be32)))
-		return true;
-
-	return false;
+	return fwnode_phy_is_fixed_link(of_fwnode_handle(np));
 }
 EXPORT_SYMBOL(of_phy_is_fixed_link);
 
 int of_phy_register_fixed_link(struct device_node *np)
 {
-	struct fixed_phy_status status = {};
-	struct device_node *fixed_link_node;
-	u32 fixed_link_prop[5];
-	const char *managed;
-
-	if (of_property_read_string(np, "managed", &managed) == 0 &&
-	    strcmp(managed, "in-band-status") == 0) {
-		/* status is zeroed, namely its .link member */
-		goto register_phy;
-	}
-
-	/* New binding */
-	fixed_link_node = of_get_child_by_name(np, "fixed-link");
-	if (fixed_link_node) {
-		status.link = 1;
-		status.duplex = of_property_read_bool(fixed_link_node,
-						      "full-duplex");
-		if (of_property_read_u32(fixed_link_node, "speed",
-					 &status.speed)) {
-			of_node_put(fixed_link_node);
-			return -EINVAL;
-		}
-		status.pause = of_property_read_bool(fixed_link_node, "pause");
-		status.asym_pause = of_property_read_bool(fixed_link_node,
-							  "asym-pause");
-		of_node_put(fixed_link_node);
-
-		goto register_phy;
-	}
-
-	/* Old binding */
-	if (of_property_read_u32_array(np, "fixed-link", fixed_link_prop,
-				       ARRAY_SIZE(fixed_link_prop)) == 0) {
-		status.link = 1;
-		status.duplex = fixed_link_prop[1];
-		status.speed  = fixed_link_prop[2];
-		status.pause  = fixed_link_prop[3];
-		status.asym_pause = fixed_link_prop[4];
-		goto register_phy;
-	}
-
-	return -ENODEV;
-
-register_phy:
-	return PTR_ERR_OR_ZERO(fixed_phy_register(PHY_POLL, &status, of_fwnode_handle(np)));
+	return fwnode_phy_register_fixed_link(of_fwnode_handle(np));
 }
 EXPORT_SYMBOL(of_phy_register_fixed_link);
 
 void of_phy_deregister_fixed_link(struct device_node *np)
 {
-	struct phy_device *phydev;
-
-	phydev = of_phy_find_device(np);
-	if (!phydev)
-		return;
-
-	fixed_phy_unregister(phydev);
-
-	put_device(&phydev->mdio.dev);	/* of_phy_find_device() */
-	phy_device_free(phydev);	/* fixed_phy_register() */
+	fwnode_phy_deregister_fixed_link(of_fwnode_handle(np));
 }
 EXPORT_SYMBOL(of_phy_deregister_fixed_link);
-- 
2.29.0

Re: [net-next: PATCH v3 2/8] net: mdio: switch fixed-link PHYs API to fwnode_

[Andy Shevchenko]

On Wed, Jul 27, 2022 at 08:43:15AM +0200, Marcin Wojtas wrote:
> fixed-link PHYs API is used by DSA and a number of drivers
> and was depending on of_. Switch to fwnode_ so to make it
> hardware description agnostic and allow to be used in ACPI
> world as well.

...

> +	/* Old binding */
> +	len = fwnode_property_count_u32(fwnode, "fixed-link");
> +	if (len == 5)
> +		return true;
> +
> +	return false;

Can be

	return len == 5;

or

	return fwnode_...(...) == 5;

Original also good, so up to you,

...

> +		if (fwnode_property_read_u32(fixed_link_node, "speed",
> +					     &status.speed)) {
> +			fwnode_handle_put(fixed_link_node);
> +			return -EINVAL;
> +		}

Why shadowing actual error code?

Either

	ret = fwnode_...(...);
	if (ret) {
		...
		return ret;
	}

or add a comment explaining the above magic transformations.

...

> +	/* Old binding */
> +	if (fwnode_property_read_u32_array(fwnode, "fixed-link", fixed_link_prop,
> +					   ARRAY_SIZE(fixed_link_prop)) == 0) {
> +		status.link = 1;
> +		status.duplex = fixed_link_prop[1];
> +		status.speed  = fixed_link_prop[2];
> +		status.pause  = fixed_link_prop[3];
> +		status.asym_pause = fixed_link_prop[4];
> +		goto register_phy;
> +	}
> +
> +	return -ENODEV;

Ditto.

-- 
With Best Regards,
Andy Shevchenko

[net-next: PATCH v3 3/8] net: dsa: switch to device_/fwnode_ APIs

[Marcin Wojtas]

In order to support both DT and ACPI in future, modify the generic DSA
code to use device_/fwnode_ equivalent routines. Drop using port's 'dn'
field and use only fwnode - update all dependent drivers.

Because support for more generic fwnode is added, replace '_of' suffix
with '_fw' in related routines. No functional change is introduced by
this patch.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 include/net/dsa.h                   |   2 +-
 net/dsa/dsa_priv.h                  |   4 +-
 drivers/net/dsa/mt7530.c            |   6 +-
 drivers/net/dsa/mv88e6xxx/chip.c    |  14 +--
 drivers/net/dsa/qca/qca8k.c         |   2 +-
 drivers/net/dsa/realtek/rtl8365mb.c |   2 +-
 net/dsa/dsa2.c                      | 100 +++++++++++---------
 net/dsa/port.c                      |  68 +++++++------
 net/dsa/slave.c                     |   7 +-
 9 files changed, 103 insertions(+), 102 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index b902b31bebce..0a328c0073ec 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -302,7 +302,7 @@ struct dsa_port {
 
 	u8			setup:1;
 
-	struct device_node	*dn;
+	struct fwnode_handle    *fwnode;
 	unsigned int		ageing_time;
 
 	struct dsa_bridge	*bridge;
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index d9722e49864b..2c0034a915ee 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -285,8 +285,8 @@ int dsa_port_mrp_add_ring_role(const struct dsa_port *dp,
 int dsa_port_mrp_del_ring_role(const struct dsa_port *dp,
 			       const struct switchdev_obj_ring_role_mrp *mrp);
 int dsa_port_phylink_create(struct dsa_port *dp);
-int dsa_port_link_register_of(struct dsa_port *dp);
-void dsa_port_link_unregister_of(struct dsa_port *dp);
+int dsa_port_link_register_fw(struct dsa_port *dp);
+void dsa_port_link_unregister_fw(struct dsa_port *dp);
 int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr);
 void dsa_port_hsr_leave(struct dsa_port *dp, struct net_device *hsr);
 int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, bool broadcast);
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 835807911be0..427b66342493 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2109,8 +2109,8 @@ mt7530_setup(struct dsa_switch *ds)
 	struct device_node *phy_node;
 	struct device_node *mac_np;
 	struct mt7530_dummy_poll p;
-	phy_interface_t interface;
 	struct dsa_port *cpu_dp;
+	int interface;
 	u32 id, val;
 	int ret, i;
 
@@ -2232,8 +2232,8 @@ mt7530_setup(struct dsa_switch *ds)
 
 	if (!dsa_is_unused_port(ds, 5)) {
 		priv->p5_intf_sel = P5_INTF_SEL_GMAC5;
-		ret = of_get_phy_mode(dsa_to_port(ds, 5)->dn, &interface);
-		if (ret && ret != -ENODEV)
+		interface = fwnode_get_phy_mode(dsa_to_port(ds, 5)->fwnode);
+		if (interface < 0)
 			return ret;
 	} else {
 		/* Scan the ethernet nodes. look for GMAC1, lookup used phy */
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 07e9a4da924c..a46ebdfba1c3 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3275,7 +3275,7 @@ static int mv88e6xxx_setup_upstream_port(struct mv88e6xxx_chip *chip, int port)
 
 static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
 {
-	struct device_node *phy_handle = NULL;
+	struct fwnode_handle *phy_handle = NULL;
 	struct dsa_switch *ds = chip->ds;
 	phy_interface_t mode;
 	struct dsa_port *dp;
@@ -3499,15 +3499,15 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
 
 	if (chip->info->ops->serdes_set_tx_amplitude) {
 		if (dp)
-			phy_handle = of_parse_phandle(dp->dn, "phy-handle", 0);
+			phy_handle = fwnode_find_reference(dp->fwnode, "phy-handle", 0);
 
-		if (phy_handle && !of_property_read_u32(phy_handle,
-							"tx-p2p-microvolt",
-							&tx_amp))
+		if (!IS_ERR(phy_handle) && !fwnode_property_read_u32(phy_handle,
+								     "tx-p2p-microvolt",
+								     &tx_amp))
 			err = chip->info->ops->serdes_set_tx_amplitude(chip,
 								port, tx_amp);
-		if (phy_handle) {
-			of_node_put(phy_handle);
+		if (!IS_ERR(phy_handle)) {
+			fwnode_handle_put(phy_handle);
 			if (err)
 				return err;
 		}
diff --git a/drivers/net/dsa/qca/qca8k.c b/drivers/net/dsa/qca/qca8k.c
index 1cbb05b0323f..77b14ade0828 100644
--- a/drivers/net/dsa/qca/qca8k.c
+++ b/drivers/net/dsa/qca/qca8k.c
@@ -1517,7 +1517,7 @@ qca8k_parse_port_config(struct qca8k_priv *priv)
 			continue;
 
 		dp = dsa_to_port(priv->ds, port);
-		port_dn = dp->dn;
+		port_dn = to_of_node(dp->fwnode);
 		cpu_port_index++;
 
 		if (!of_device_is_available(port_dn))
diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c
index da31d8b839ac..d61da012451f 100644
--- a/drivers/net/dsa/realtek/rtl8365mb.c
+++ b/drivers/net/dsa/realtek/rtl8365mb.c
@@ -887,7 +887,7 @@ static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port,
 		return -ENODEV;
 
 	dp = dsa_to_port(priv->ds, port);
-	dn = dp->dn;
+	dn = to_of_node(dp->fwnode);
 
 	/* Set the RGMII TX/RX delay
 	 *
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index cac48a741f27..82fb3b009fb4 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -296,12 +296,12 @@ static void dsa_tree_put(struct dsa_switch_tree *dst)
 }
 
 static struct dsa_port *dsa_tree_find_port_by_node(struct dsa_switch_tree *dst,
-						   struct device_node *dn)
+						   struct fwnode_handle *fwnode)
 {
 	struct dsa_port *dp;
 
 	list_for_each_entry(dp, &dst->ports, list)
-		if (dp->dn == dn)
+		if (dp->fwnode == fwnode)
 			return dp;
 
 	return NULL;
@@ -337,14 +337,13 @@ static bool dsa_port_setup_routing_table(struct dsa_port *dp)
 {
 	struct dsa_switch *ds = dp->ds;
 	struct dsa_switch_tree *dst = ds->dst;
-	struct device_node *dn = dp->dn;
 	struct of_phandle_iterator it;
 	struct dsa_port *link_dp;
 	struct dsa_link *dl;
 	int err;
 
-	of_for_each_phandle(&it, err, dn, "link", NULL, 0) {
-		link_dp = dsa_tree_find_port_by_node(dst, it.node);
+	of_for_each_phandle(&it, err, to_of_node(dp->fwnode), "link", NULL, 0) {
+		link_dp = dsa_tree_find_port_by_node(dst, of_fwnode_handle(it.node));
 		if (!link_dp) {
 			of_node_put(it.node);
 			return false;
@@ -469,7 +468,7 @@ static int dsa_port_setup(struct dsa_port *dp)
 		dsa_port_disable(dp);
 		break;
 	case DSA_PORT_TYPE_CPU:
-		err = dsa_port_link_register_of(dp);
+		err = dsa_port_link_register_fw(dp);
 		if (err)
 			break;
 		dsa_port_link_registered = true;
@@ -481,7 +480,7 @@ static int dsa_port_setup(struct dsa_port *dp)
 
 		break;
 	case DSA_PORT_TYPE_DSA:
-		err = dsa_port_link_register_of(dp);
+		err = dsa_port_link_register_fw(dp);
 		if (err)
 			break;
 		dsa_port_link_registered = true;
@@ -493,7 +492,7 @@ static int dsa_port_setup(struct dsa_port *dp)
 
 		break;
 	case DSA_PORT_TYPE_USER:
-		of_get_mac_address(dp->dn, dp->mac);
+		fwnode_get_mac_address(dp->fwnode, dp->mac);
 		err = dsa_slave_create(dp);
 		if (err)
 			break;
@@ -505,7 +504,7 @@ static int dsa_port_setup(struct dsa_port *dp)
 	if (err && dsa_port_enabled)
 		dsa_port_disable(dp);
 	if (err && dsa_port_link_registered)
-		dsa_port_link_unregister_of(dp);
+		dsa_port_link_unregister_fw(dp);
 	if (err) {
 		if (ds->ops->port_teardown)
 			ds->ops->port_teardown(ds, dp->index);
@@ -577,11 +576,11 @@ static void dsa_port_teardown(struct dsa_port *dp)
 		break;
 	case DSA_PORT_TYPE_CPU:
 		dsa_port_disable(dp);
-		dsa_port_link_unregister_of(dp);
+		dsa_port_link_unregister_fw(dp);
 		break;
 	case DSA_PORT_TYPE_DSA:
 		dsa_port_disable(dp);
-		dsa_port_link_unregister_of(dp);
+		dsa_port_link_unregister_fw(dp);
 		break;
 	case DSA_PORT_TYPE_USER:
 		if (dp->slave) {
@@ -853,7 +852,7 @@ static int dsa_switch_setup_tag_protocol(struct dsa_switch *ds)
 static int dsa_switch_setup(struct dsa_switch *ds)
 {
 	struct dsa_devlink_priv *dl_priv;
-	struct device_node *dn;
+	struct fwnode_handle *fwnode;
 	struct dsa_port *dp;
 	int err;
 
@@ -909,10 +908,10 @@ static int dsa_switch_setup(struct dsa_switch *ds)
 
 		dsa_slave_mii_bus_init(ds);
 
-		dn = of_get_child_by_name(ds->dev->of_node, "mdio");
+		fwnode = device_get_named_child_node(ds->dev, "mdio");
 
-		err = of_mdiobus_register(ds->slave_mii_bus, dn);
-		of_node_put(dn);
+		err = of_mdiobus_register(ds->slave_mii_bus, to_of_node(fwnode));
+		fwnode_handle_put(fwnode);
 		if (err < 0)
 			goto free_slave_mii_bus;
 	}
@@ -1482,24 +1481,33 @@ static int dsa_port_parse_cpu(struct dsa_port *dp, struct net_device *master,
 	return 0;
 }
 
-static int dsa_port_parse_of(struct dsa_port *dp, struct device_node *dn)
+static int dsa_port_parse_fw(struct dsa_port *dp, struct fwnode_handle *fwnode)
 {
-	struct device_node *ethernet = of_parse_phandle(dn, "ethernet", 0);
-	const char *name = of_get_property(dn, "label", NULL);
-	bool link = of_property_read_bool(dn, "link");
+	struct fwnode_handle *ethernet = fwnode_find_reference(fwnode, "ethernet", 0);
+	bool link = fwnode_property_present(fwnode, "link");
+	const char *name;
+	int ret;
+
+	ret = fwnode_property_read_string(fwnode, "label", &name);
+	if (ret)
+		return ret;
 
-	dp->dn = dn;
+	dp->fwnode = fwnode;
 
-	if (ethernet) {
+	if (!IS_ERR(ethernet)) {
 		struct net_device *master;
 		const char *user_protocol;
 
-		master = of_find_net_device_by_node(ethernet);
-		of_node_put(ethernet);
+		master = of_find_net_device_by_node(to_of_node(ethernet));
+		fwnode_handle_put(ethernet);
 		if (!master)
 			return -EPROBE_DEFER;
 
-		user_protocol = of_get_property(dn, "dsa-tag-protocol", NULL);
+		ret = fwnode_property_read_string(fwnode, "dsa-tag-protocol",
+						  &user_protocol);
+		if (ret)
+			user_protocol = NULL;
+
 		return dsa_port_parse_cpu(dp, master, user_protocol);
 	}
 
@@ -1509,61 +1517,61 @@ static int dsa_port_parse_of(struct dsa_port *dp, struct device_node *dn)
 	return dsa_port_parse_user(dp, name);
 }
 
-static int dsa_switch_parse_ports_of(struct dsa_switch *ds,
-				     struct device_node *dn)
+static int dsa_switch_parse_ports_fw(struct dsa_switch *ds,
+				     struct fwnode_handle *fwnode)
 {
-	struct device_node *ports, *port;
+	struct fwnode_handle *ports, *port;
 	struct dsa_port *dp;
 	int err = 0;
 	u32 reg;
 
-	ports = of_get_child_by_name(dn, "ports");
+	ports = fwnode_get_named_child_node(fwnode, "ports");
 	if (!ports) {
 		/* The second possibility is "ethernet-ports" */
-		ports = of_get_child_by_name(dn, "ethernet-ports");
+		ports = fwnode_get_named_child_node(fwnode, "ethernet-ports");
 		if (!ports) {
 			dev_err(ds->dev, "no ports child node found\n");
 			return -EINVAL;
 		}
 	}
 
-	for_each_available_child_of_node(ports, port) {
-		err = of_property_read_u32(port, "reg", &reg);
+	fwnode_for_each_available_child_node(ports, port) {
+		err = fwnode_property_read_u32(port, "reg", &reg);
 		if (err) {
-			of_node_put(port);
+			fwnode_handle_put(port);
 			goto out_put_node;
 		}
 
 		if (reg >= ds->num_ports) {
 			dev_err(ds->dev, "port %pOF index %u exceeds num_ports (%u)\n",
 				port, reg, ds->num_ports);
-			of_node_put(port);
+			fwnode_handle_put(port);
 			err = -EINVAL;
 			goto out_put_node;
 		}
 
 		dp = dsa_to_port(ds, reg);
 
-		err = dsa_port_parse_of(dp, port);
+		err = dsa_port_parse_fw(dp, port);
 		if (err) {
-			of_node_put(port);
+			fwnode_handle_put(port);
 			goto out_put_node;
 		}
 	}
 
 out_put_node:
-	of_node_put(ports);
+	fwnode_handle_put(ports);
 	return err;
 }
 
-static int dsa_switch_parse_member_of(struct dsa_switch *ds,
-				      struct device_node *dn)
+static int dsa_switch_parse_member_fw(struct dsa_switch *ds,
+				      struct fwnode_handle *fwnode)
 {
 	u32 m[2] = { 0, 0 };
 	int sz;
 
 	/* Don't error out if this optional property isn't found */
-	sz = of_property_read_variable_u32_array(dn, "dsa,member", m, 2, 2);
+	sz = fwnode_property_read_u32_array(fwnode, "dsa,member", m, 2);
 	if (sz < 0 && sz != -EINVAL)
 		return sz;
 
@@ -1600,11 +1608,11 @@ static int dsa_switch_touch_ports(struct dsa_switch *ds)
 	return 0;
 }
 
-static int dsa_switch_parse_of(struct dsa_switch *ds, struct device_node *dn)
+static int dsa_switch_parse_fw(struct dsa_switch *ds, struct fwnode_handle *fwnode)
 {
 	int err;
 
-	err = dsa_switch_parse_member_of(ds, dn);
+	err = dsa_switch_parse_member_fw(ds, fwnode);
 	if (err)
 		return err;
 
@@ -1612,7 +1620,7 @@ static int dsa_switch_parse_of(struct dsa_switch *ds, struct device_node *dn)
 	if (err)
 		return err;
 
-	return dsa_switch_parse_ports_of(ds, dn);
+	return dsa_switch_parse_ports_fw(ds, fwnode);
 }
 
 static int dsa_port_parse(struct dsa_port *dp, const char *name,
@@ -1705,20 +1713,20 @@ static int dsa_switch_probe(struct dsa_switch *ds)
 {
 	struct dsa_switch_tree *dst;
 	struct dsa_chip_data *pdata;
-	struct device_node *np;
+	struct fwnode_handle *fwnode;
 	int err;
 
 	if (!ds->dev)
 		return -ENODEV;
 
 	pdata = ds->dev->platform_data;
-	np = ds->dev->of_node;
+	fwnode = dev_fwnode(ds->dev);
 
 	if (!ds->num_ports)
 		return -EINVAL;
 
-	if (np) {
-		err = dsa_switch_parse_of(ds, np);
+	if (fwnode) {
+		err = dsa_switch_parse_fw(ds, fwnode);
 		if (err)
 			dsa_switch_release_ports(ds);
 	} else if (pdata) {
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 2dd76eb1621c..40c7d1d9b488 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -6,10 +6,9 @@
  *	Vivien Didelot <vivien.didelot@savoirfairelinux.com>
  */
 
+#include <linux/fwnode_mdio.h>
 #include <linux/if_bridge.h>
 #include <linux/notifier.h>
-#include <linux/of_mdio.h>
-#include <linux/of_net.h>
 
 #include "dsa_priv.h"
 
@@ -1380,20 +1379,20 @@ void dsa_port_set_tag_protocol(struct dsa_port *cpu_dp,
 
 static struct phy_device *dsa_port_get_phy_device(struct dsa_port *dp)
 {
-	struct device_node *phy_dn;
+	struct fwnode_handle *phy_handle;
 	struct phy_device *phydev;
 
-	phy_dn = of_parse_phandle(dp->dn, "phy-handle", 0);
-	if (!phy_dn)
+	phy_handle = fwnode_find_reference(dp->fwnode, "phy-handle", 0);
+	if (IS_ERR(phy_handle))
 		return NULL;
 
-	phydev = of_phy_find_device(phy_dn);
+	phydev = fwnode_phy_find_device(phy_handle);
 	if (!phydev) {
-		of_node_put(phy_dn);
+		fwnode_handle_put(phy_handle);
 		return ERR_PTR(-EPROBE_DEFER);
 	}
 
-	of_node_put(phy_dn);
+	fwnode_handle_put(phy_handle);
 	return phydev;
 }
 
@@ -1525,11 +1524,10 @@ static const struct phylink_mac_ops dsa_port_phylink_mac_ops = {
 int dsa_port_phylink_create(struct dsa_port *dp)
 {
 	struct dsa_switch *ds = dp->ds;
-	phy_interface_t mode;
-	int err;
+	int mode;
 
-	err = of_get_phy_mode(dp->dn, &mode);
-	if (err)
+	mode = fwnode_get_phy_mode(dp->fwnode);
+	if (mode < 0)
 		mode = PHY_INTERFACE_MODE_NA;
 
 	/* Presence of phylink_mac_link_state or phylink_mac_an_restart is
@@ -1542,7 +1540,7 @@ int dsa_port_phylink_create(struct dsa_port *dp)
 	if (ds->ops->phylink_get_caps)
 		ds->ops->phylink_get_caps(ds, dp->index, &dp->pl_config);
 
-	dp->pl = phylink_create(&dp->pl_config, of_fwnode_handle(dp->dn),
+	dp->pl = phylink_create(&dp->pl_config, dp->fwnode,
 				mode, &dsa_port_phylink_mac_ops);
 	if (IS_ERR(dp->pl)) {
 		pr_err("error creating PHYLINK: %ld\n", PTR_ERR(dp->pl));
@@ -1552,7 +1550,7 @@ int dsa_port_phylink_create(struct dsa_port *dp)
 	return 0;
 }
 
-static int dsa_port_setup_phy_of(struct dsa_port *dp, bool enable)
+static int dsa_port_setup_phy_fw(struct dsa_port *dp, bool enable)
 {
 	struct dsa_switch *ds = dp->ds;
 	struct phy_device *phydev;
@@ -1590,16 +1588,15 @@ static int dsa_port_setup_phy_of(struct dsa_port *dp, bool enable)
 	return err;
 }
 
-static int dsa_port_fixed_link_register_of(struct dsa_port *dp)
+static int dsa_port_fixed_link_register_fw(struct dsa_port *dp)
 {
-	struct device_node *dn = dp->dn;
 	struct dsa_switch *ds = dp->ds;
 	struct phy_device *phydev;
 	int port = dp->index;
-	phy_interface_t mode;
+	int mode;
 	int err;
 
-	err = of_phy_register_fixed_link(dn);
+	err = fwnode_phy_register_fixed_link(dp->fwnode);
 	if (err) {
 		dev_err(ds->dev,
 			"failed to register the fixed PHY of port %d\n",
@@ -1607,10 +1604,10 @@ static int dsa_port_fixed_link_register_of(struct dsa_port *dp)
 		return err;
 	}
 
-	phydev = of_phy_find_device(dn);
+	phydev = fwnode_phy_find_device(dp->fwnode);
 
-	err = of_get_phy_mode(dn, &mode);
-	if (err)
+	mode = fwnode_get_phy_mode(dp->fwnode);
+	if (mode < 0)
 		mode = PHY_INTERFACE_MODE_NA;
 	phydev->interface = mode;
 
@@ -1627,7 +1624,6 @@ static int dsa_port_fixed_link_register_of(struct dsa_port *dp)
 static int dsa_port_phylink_register(struct dsa_port *dp)
 {
 	struct dsa_switch *ds = dp->ds;
-	struct device_node *port_dn = dp->dn;
 	int err;
 
 	dp->pl_config.dev = ds->dev;
@@ -1637,7 +1633,7 @@ static int dsa_port_phylink_register(struct dsa_port *dp)
 	if (err)
 		return err;
 
-	err = phylink_of_phy_connect(dp->pl, port_dn, 0);
+	err = phylink_fwnode_phy_connect(dp->pl, dp->fwnode, 0);
 	if (err && err != -ENODEV) {
 		pr_err("could not attach to PHY: %d\n", err);
 		goto err_phy_connect;
@@ -1650,35 +1646,35 @@ static int dsa_port_phylink_register(struct dsa_port *dp)
 	return err;
 }
 
-int dsa_port_link_register_of(struct dsa_port *dp)
+int dsa_port_link_register_fw(struct dsa_port *dp)
 {
+	struct fwnode_handle *phy_handle;
 	struct dsa_switch *ds = dp->ds;
-	struct device_node *phy_np;
 	int port = dp->index;
 
 	if (!ds->ops->adjust_link) {
-		phy_np = of_parse_phandle(dp->dn, "phy-handle", 0);
-		if (of_phy_is_fixed_link(dp->dn) || phy_np) {
+		phy_handle = fwnode_find_reference(dp->fwnode, "phy-handle", 0);
+		if (fwnode_phy_is_fixed_link(dp->fwnode) || !IS_ERR(phy_handle)) {
 			if (ds->ops->phylink_mac_link_down)
 				ds->ops->phylink_mac_link_down(ds, port,
 					MLO_AN_FIXED, PHY_INTERFACE_MODE_NA);
-			of_node_put(phy_np);
+			fwnode_handle_put(dp->fwnode);
 			return dsa_port_phylink_register(dp);
 		}
-		of_node_put(phy_np);
+		fwnode_handle_put(dp->fwnode);
 		return 0;
 	}
 
 	dev_warn(ds->dev,
 		 "Using legacy PHYLIB callbacks. Please migrate to PHYLINK!\n");
 
-	if (of_phy_is_fixed_link(dp->dn))
-		return dsa_port_fixed_link_register_of(dp);
+	if (fwnode_phy_is_fixed_link(dp->fwnode))
+		return dsa_port_fixed_link_register_fw(dp);
 	else
-		return dsa_port_setup_phy_of(dp, true);
+		return dsa_port_setup_phy_fw(dp, true);
 }
 
-void dsa_port_link_unregister_of(struct dsa_port *dp)
+void dsa_port_link_unregister_fw(struct dsa_port *dp)
 {
 	struct dsa_switch *ds = dp->ds;
 
@@ -1691,10 +1687,10 @@ void dsa_port_link_unregister_of(struct dsa_port *dp)
 		return;
 	}
 
-	if (of_phy_is_fixed_link(dp->dn))
-		of_phy_deregister_fixed_link(dp->dn);
+	if (fwnode_phy_is_fixed_link(dp->fwnode))
+		fwnode_phy_deregister_fixed_link(dp->fwnode);
 	else
-		dsa_port_setup_phy_of(dp, false);
+		dsa_port_setup_phy_fw(dp, false);
 }
 
 int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index ad6a6663feeb..209e24cb1477 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -10,8 +10,6 @@
 #include <linux/phy.h>
 #include <linux/phy_fixed.h>
 #include <linux/phylink.h>
-#include <linux/of_net.h>
-#include <linux/of_mdio.h>
 #include <linux/mdio.h>
 #include <net/rtnetlink.h>
 #include <net/pkt_cls.h>
@@ -2228,7 +2226,6 @@ static int dsa_slave_phy_connect(struct net_device *slave_dev, int addr,
 static int dsa_slave_phy_setup(struct net_device *slave_dev)
 {
 	struct dsa_port *dp = dsa_slave_to_port(slave_dev);
-	struct device_node *port_dn = dp->dn;
 	struct dsa_switch *ds = dp->ds;
 	u32 phy_flags = 0;
 	int ret;
@@ -2252,7 +2249,7 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev)
 	if (ds->ops->get_phy_flags)
 		phy_flags = ds->ops->get_phy_flags(ds, dp->index);
 
-	ret = phylink_of_phy_connect(dp->pl, port_dn, phy_flags);
+	ret = phylink_fwnode_phy_connect(dp->pl, dp->fwnode, phy_flags);
 	if (ret == -ENODEV && ds->slave_mii_bus) {
 		/* We could not connect to a designated PHY or SFP, so try to
 		 * use the switch internal MDIO bus instead
@@ -2364,7 +2361,7 @@ int dsa_slave_create(struct dsa_port *port)
 	SET_NETDEV_DEVTYPE(slave_dev, &dsa_type);
 
 	SET_NETDEV_DEV(slave_dev, port->ds->dev);
-	slave_dev->dev.of_node = port->dn;
+	device_set_node(&slave_dev->dev, port->fwnode);
 	slave_dev->vlan_features = master->vlan_features;
 
 	p = netdev_priv(slave_dev);
-- 
2.29.0

Re: [net-next: PATCH v3 3/8] net: dsa: switch to device_/fwnode_ APIs

[Andy Shevchenko]

On Wed, Jul 27, 2022 at 08:43:16AM +0200, Marcin Wojtas wrote:
> In order to support both DT and ACPI in future, modify the generic DSA
> code to use device_/fwnode_ equivalent routines. Drop using port's 'dn'
> field and use only fwnode - update all dependent drivers.
> 
> Because support for more generic fwnode is added, replace '_of' suffix
> with '_fw' in related routines. No functional change is introduced by
> this patch.

...

>  static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
>  {
> -	struct device_node *phy_handle = NULL;
> +	struct fwnode_handle *phy_handle = NULL;
>  	struct dsa_switch *ds = chip->ds;
>  	phy_interface_t mode;
>  	struct dsa_port *dp;
> @@ -3499,15 +3499,15 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
>  
>  	if (chip->info->ops->serdes_set_tx_amplitude) {
>  		if (dp)
> -			phy_handle = of_parse_phandle(dp->dn, "phy-handle", 0);
> +			phy_handle = fwnode_find_reference(dp->fwnode, "phy-handle", 0);
>  
> -		if (phy_handle && !of_property_read_u32(phy_handle,
> -							"tx-p2p-microvolt",
> -							&tx_amp))
> +		if (!IS_ERR(phy_handle) && !fwnode_property_read_u32(phy_handle,
> +								     "tx-p2p-microvolt",
> +								     &tx_amp))
>  			err = chip->info->ops->serdes_set_tx_amplitude(chip,
>  								port, tx_amp);
> -		if (phy_handle) {
> -			of_node_put(phy_handle);
> +		if (!IS_ERR(phy_handle)) {
> +			fwnode_handle_put(phy_handle);
>  			if (err)
>  				return err;
>  		}

I believe after 002752af7b89 ("device property: Allow error pointer to be
passed to fwnode APIs") you may simplify above like:

		if (!fwnode_property_read_u32(phy_handle, "tx-p2p-microvolt",
					      &tx_amp))
			err = chip->info->ops->serdes_set_tx_amplitude(chip,
								port, tx_amp);
		else
			err = 0;
		fwnode_handle_put(phy_handle);
		if (err)
			return err;

It also possible you can do refactoring before/after this one.

-- 
With Best Regards,
Andy Shevchenko

[net-next: PATCH v3 4/8] net: mvpp2: initialize port fwnode pointer

[Marcin Wojtas]

As a preparation to switch the DSA subsystem from using
of_find_net_device_by_node() to its more generic fwnode_
equivalent, the port's device structure should be updated
with its fwnode pointer, similarly to of_node - see analogous
commit c4053ef32208 ("net: mvpp2: initialize port of_node pointer").

This patch is required to prevent a regression before updating
the DSA API on boards that connect the mvpp2 port to switch,
such as Clearfog GT-8K or CN913x CEx7 Evaluation Board.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index b84128b549b4..03d5ff649c47 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -6868,7 +6868,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	dev->min_mtu = ETH_MIN_MTU;
 	/* 9704 == 9728 - 20 and rounding to 8 */
 	dev->max_mtu = MVPP2_BM_JUMBO_PKT_SIZE;
-	dev->dev.of_node = port_node;
+	device_set_node(&dev->dev, port_fwnode);
 
 	port->pcs_gmac.ops = &mvpp2_phylink_gmac_pcs_ops;
 	port->pcs_xlg.ops = &mvpp2_phylink_xlg_pcs_ops;
-- 
2.29.0

[net-next: PATCH v3 5/8] device property: introduce fwnode_find_parent_dev_match

[Marcin Wojtas]

This patch adds a new generic routine fwnode_find_parent_dev_match
that can be used e.g. as a callback for class_find_device().
It searches for the struct device corresponding to a
struct fwnode_handle by iterating over device and
its parents.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 include/linux/property.h |  1 +
 drivers/base/property.c  | 23 ++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/linux/property.h b/include/linux/property.h
index a5b429d623f6..829254a5ae63 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -95,6 +95,7 @@ struct device *fwnode_get_next_parent_dev(struct fwnode_handle *fwnode);
 unsigned int fwnode_count_parents(const struct fwnode_handle *fwn);
 struct fwnode_handle *fwnode_get_nth_parent(struct fwnode_handle *fwn,
 					    unsigned int depth);
+int fwnode_find_parent_dev_match(struct device *dev, const void *data);
 bool fwnode_is_ancestor_of(struct fwnode_handle *ancestor, struct fwnode_handle *child);
 struct fwnode_handle *fwnode_get_next_child_node(
 	const struct fwnode_handle *fwnode, struct fwnode_handle *child);
diff --git a/drivers/base/property.c b/drivers/base/property.c
index ed6f449f8e5c..ee6a8144f103 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -685,6 +685,29 @@ struct fwnode_handle *fwnode_get_nth_parent(struct fwnode_handle *fwnode,
 }
 EXPORT_SYMBOL_GPL(fwnode_get_nth_parent);
 
+/**
+ * fwnode_find_parent_dev_match - look for a device matching the struct fwnode_handle
+ * @dev: the struct device to initiate the search
+ * @data: pointer passed for comparison
+ *
+ * Looks up the device structure corresponding with the fwnode by iterating
+ * over @dev and its parents.
+ * The routine can be used e.g. as a callback for class_find_device().
+ *
+ * Returns: %1 - match is found
+ *          %0 - match not found
+ */
+int fwnode_find_parent_dev_match(struct device *dev, const void *data)
+{
+	for (; dev; dev = dev->parent) {
+		if (device_match_fwnode(dev, data))
+			return 1;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fwnode_find_parent_dev_match);
+
 /**
  * fwnode_is_ancestor_of - Test if @ancestor is ancestor of @child
  * @ancestor: Firmware which is tested for being an ancestor
-- 
2.29.0

Re: [net-next: PATCH v3 5/8] device property: introduce fwnode_find_parent_dev_match

[Andy Shevchenko]

On Wed, Jul 27, 2022 at 08:43:18AM +0200, Marcin Wojtas wrote:
> This patch adds a new generic routine fwnode_find_parent_dev_match

"This patch..."

> that can be used e.g. as a callback for class_find_device().
> It searches for the struct device corresponding to a
> struct fwnode_handle by iterating over device and
> its parents.

-- 
With Best Regards,
Andy Shevchenko

[net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Marcin Wojtas]

A helper function which allows getting the struct net_device pointer
associated with a given device tree node can be more generic and
also support alternative hardware description. Switch to fwnode_
and update the only existing caller in DSA subsystem.
For that purpose use newly added fwnode_dev_node_match helper routine.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 include/linux/etherdevice.h |  1 +
 include/linux/of_net.h      |  6 -----
 net/core/net-sysfs.c        | 25 ++++++--------------
 net/dsa/dsa2.c              |  3 ++-
 4 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 92b10e67d5f8..a335775af244 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -35,6 +35,7 @@ int nvmem_get_mac_address(struct device *dev, void *addrbuf);
 int device_get_mac_address(struct device *dev, char *addr);
 int device_get_ethdev_address(struct device *dev, struct net_device *netdev);
 int fwnode_get_mac_address(struct fwnode_handle *fwnode, char *addr);
+struct net_device *fwnode_find_net_device_by_node(struct fwnode_handle *fwnode);
 
 u32 eth_get_headlen(const struct net_device *dev, const void *data, u32 len);
 __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev);
diff --git a/include/linux/of_net.h b/include/linux/of_net.h
index 0484b613ca64..f672f831292d 100644
--- a/include/linux/of_net.h
+++ b/include/linux/of_net.h
@@ -15,7 +15,6 @@ struct net_device;
 extern int of_get_phy_mode(struct device_node *np, phy_interface_t *interface);
 extern int of_get_mac_address(struct device_node *np, u8 *mac);
 int of_get_ethdev_address(struct device_node *np, struct net_device *dev);
-extern struct net_device *of_find_net_device_by_node(struct device_node *np);
 #else
 static inline int of_get_phy_mode(struct device_node *np,
 				  phy_interface_t *interface)
@@ -32,11 +31,6 @@ static inline int of_get_ethdev_address(struct device_node *np, struct net_devic
 {
 	return -ENODEV;
 }
-
-static inline struct net_device *of_find_net_device_by_node(struct device_node *np)
-{
-	return NULL;
-}
 #endif
 
 #endif /* __LINUX_OF_NET_H */
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index d61afd21aab5..fc972545aaea 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/capability.h>
+#include <linux/etherdevice.h>
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
 #include <linux/if_arp.h>
@@ -1935,38 +1936,26 @@ static struct class net_class __ro_after_init = {
 	.get_ownership = net_get_ownership,
 };
 
-#ifdef CONFIG_OF
-static int of_dev_node_match(struct device *dev, const void *data)
-{
-	for (; dev; dev = dev->parent) {
-		if (dev->of_node == data)
-			return 1;
-	}
-
-	return 0;
-}
-
 /*
- * of_find_net_device_by_node - lookup the net device for the device node
- * @np: OF device node
+ * fwnode_find_net_device_by_node - lookup the net device for the device fwnode
+ * @fwnode: firmware node
  *
- * Looks up the net_device structure corresponding with the device node.
+ * Looks up the net_device structure corresponding with the fwnode.
  * If successful, returns a pointer to the net_device with the embedded
  * struct device refcount incremented by one, or NULL on failure. The
  * refcount must be dropped when done with the net_device.
  */
-struct net_device *of_find_net_device_by_node(struct device_node *np)
+struct net_device *fwnode_find_net_device_by_node(struct fwnode_handle *fwnode)
 {
 	struct device *dev;
 
-	dev = class_find_device(&net_class, NULL, np, of_dev_node_match);
+	dev = class_find_device(&net_class, NULL, fwnode, fwnode_find_parent_dev_match);
 	if (!dev)
 		return NULL;
 
 	return to_net_dev(dev);
 }
-EXPORT_SYMBOL(of_find_net_device_by_node);
-#endif
+EXPORT_SYMBOL(fwnode_find_net_device_by_node);
 
 /* Delete sysfs entries but hold kobject reference until after all
  * netdev references are gone.
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 82fb3b009fb4..bba416eba9c2 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -7,6 +7,7 @@
  */
 
 #include <linux/device.h>
+#include <linux/etherdevice.h>
 #include <linux/err.h>
 #include <linux/list.h>
 #include <linux/netdevice.h>
@@ -1498,7 +1499,7 @@ static int dsa_port_parse_fw(struct dsa_port *dp, struct fwnode_handle *fwnode)
 		struct net_device *master;
 		const char *user_protocol;
 
-		master = of_find_net_device_by_node(to_of_node(ethernet));
+		master = fwnode_find_net_device_by_node(ethernet);
 		fwnode_handle_put(ethernet);
 		if (!master)
 			return -EPROBE_DEFER;
-- 
2.29.0

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Vladimir Oltean]

On Wed, Jul 27, 2022 at 08:43:19AM +0200, Marcin Wojtas wrote:
> A helper function which allows getting the struct net_device pointer
> associated with a given device tree node can be more generic and
> also support alternative hardware description. Switch to fwnode_
> and update the only existing caller in DSA subsystem.
> For that purpose use newly added fwnode_dev_node_match helper routine.
> 
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
> -struct net_device *of_find_net_device_by_node(struct device_node *np)
> +struct net_device *fwnode_find_net_device_by_node(struct fwnode_handle *fwnode)
>  {
>  	struct device *dev;
>  
> -	dev = class_find_device(&net_class, NULL, np, of_dev_node_match);
> +	dev = class_find_device(&net_class, NULL, fwnode, fwnode_find_parent_dev_match);

This needs to maintain compatibility with DSA masters that have
dev->of_node but don't have dev->fwnode populated.

>  	if (!dev)
>  		return NULL;
>  
>  	return to_net_dev(dev);
>  }
> -EXPORT_SYMBOL(of_find_net_device_by_node);
> -#endif
> +EXPORT_SYMBOL(fwnode_find_net_device_by_node);

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Marcin Wojtas]

śr., 27 lip 2022 o 16:31 Vladimir Oltean <olteanv@gmail.com> napisał(a):
>
> On Wed, Jul 27, 2022 at 08:43:19AM +0200, Marcin Wojtas wrote:
> > A helper function which allows getting the struct net_device pointer
> > associated with a given device tree node can be more generic and
> > also support alternative hardware description. Switch to fwnode_
> > and update the only existing caller in DSA subsystem.
> > For that purpose use newly added fwnode_dev_node_match helper routine.
> >
> > Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> > ---
> > -struct net_device *of_find_net_device_by_node(struct device_node *np)
> > +struct net_device *fwnode_find_net_device_by_node(struct fwnode_handle *fwnode)
> >  {
> >       struct device *dev;
> >
> > -     dev = class_find_device(&net_class, NULL, np, of_dev_node_match);
> > +     dev = class_find_device(&net_class, NULL, fwnode, fwnode_find_parent_dev_match);
>
> This needs to maintain compatibility with DSA masters that have
> dev->of_node but don't have dev->fwnode populated.
>

Do you mean a situation analogous to what I addressed in:
[net-next: PATCH v3 4/8] net: mvpp2: initialize port fwnode pointer
?

I found indeed a couple of drivers that may require a similar change
(e.g. dpaa2).

IMO we have 2 options:
- update these drivers
- add some kind of fallback? If yes, I am wondering about an elegant
solution - maybe add an extra check inside
fwnode_find_parent_dev_match?

What would you suggest?

Best regards,
Marcin

> >       if (!dev)
> >               return NULL;
> >
> >       return to_net_dev(dev);
> >  }
> > -EXPORT_SYMBOL(of_find_net_device_by_node);
> > -#endif
> > +EXPORT_SYMBOL(fwnode_find_net_device_by_node);

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Vladimir Oltean]

On Wed, Jul 27, 2022 at 05:18:16PM +0200, Marcin Wojtas wrote:
> Do you mean a situation analogous to what I addressed in:
> [net-next: PATCH v3 4/8] net: mvpp2: initialize port fwnode pointer
> ?

Not sure if "analogous" is the right word. My estimation is that the
overwhelmingly vast majority of DSA masters can be found by DSA simply
due to the SET_NETDEV_DEV() call that the Ethernet drivers need to make
anyway.  I see that mvpp2 also needed commit c4053ef32208 ("net: mvpp2:
initialize port of_node pointer"), but that isn't needed in general, and
I can't tell you exactly why it is needed there, I don't know enough
about the mvpp2 driver.

> I found indeed a couple of drivers that may require a similar change
> (e.g. dpaa2).

There I can tell you why the dpaa2-mac code mangles with net_dev->dev.of_node,
but I'd rather not go into an explanation that essentially doesn't matter.
The point is that you'd be mistaken to think that only the drivers which
touch the net device's ->dev->of_node are the ones that need updating
for your series to not cause regressions.

> IMO we have 2 options:
> - update these drivers
> - add some kind of fallback? If yes, I am wondering about an elegant
> solution - maybe add an extra check inside
> fwnode_find_parent_dev_match?
> 
> What would you suggest?

Fixing fwnode_find_parent_dev_match(), of course. This change broke DSA
on my LS1028A system (master in drivers/net/ethernet/freescale/enetc/)
and LS1021A (master in drivers/net/ethernet/freescale/gianfar.c).

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Marcin Wojtas]

śr., 27 lip 2022 o 18:38 Vladimir Oltean <olteanv@gmail.com> napisał(a):
>
> On Wed, Jul 27, 2022 at 05:18:16PM +0200, Marcin Wojtas wrote:
> > Do you mean a situation analogous to what I addressed in:
> > [net-next: PATCH v3 4/8] net: mvpp2: initialize port fwnode pointer
> > ?
>
> Not sure if "analogous" is the right word. My estimation is that the
> overwhelmingly vast majority of DSA masters can be found by DSA simply
> due to the SET_NETDEV_DEV() call that the Ethernet drivers need to make
> anyway.  I see that mvpp2 also needed commit c4053ef32208 ("net: mvpp2:
> initialize port of_node pointer"), but that isn't needed in general, and
> I can't tell you exactly why it is needed there, I don't know enough
> about the mvpp2 driver.

SET_NETDEV_DEV() fills net_device->dev.parent with &pdev->dev
and in most cases it is sufficient apparently it is sufficient for
fwnode_find_parent_dev_match (at least tests with mvneta case proves
it's fine).

We have some corner cases though:
* mvpp2 -> single controller can handle up to 3 net_devices and
therefore we need device_set_node() to make this work. I think dpaa2
is a similar case
* PCIE drivers with extra DT description (I think that's the case of enetc).

>
> > I found indeed a couple of drivers that may require a similar change
> > (e.g. dpaa2).
>
> There I can tell you why the dpaa2-mac code mangles with net_dev->dev.of_node,
> but I'd rather not go into an explanation that essentially doesn't matter.
> The point is that you'd be mistaken to think that only the drivers which
> touch the net device's ->dev->of_node are the ones that need updating
> for your series to not cause regressions.

As above - SET_NETDEV_DEV() should be fine in most cases, but we can
never be 100% sure untils it's verified.

>
> > IMO we have 2 options:
> > - update these drivers
> > - add some kind of fallback? If yes, I am wondering about an elegant
> > solution - maybe add an extra check inside
> > fwnode_find_parent_dev_match?
> >
> > What would you suggest?
>
> Fixing fwnode_find_parent_dev_match(), of course. This change broke DSA
> on my LS1028A system (master in drivers/net/ethernet/freescale/enetc/)
> and LS1021A (master in drivers/net/ethernet/freescale/gianfar.c).

Can you please check applying following diff:

--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -695,20 +695,22 @@ EXPORT_SYMBOL_GPL(fwnode_get_nth_parent);
  * The routine can be used e.g. as a callback for class_find_device().
  *
  * Returns: %1 - match is found
  *          %0 - match not found
  */
 int fwnode_find_parent_dev_match(struct device *dev, const void *data)
 {
        for (; dev; dev = dev->parent) {
                if (device_match_fwnode(dev, data))
                        return 1;
+               else if (device_match_of_node(dev, to_of_node(data))
+                       return 1;
        }

        return 0;
 }
 EXPORT_SYMBOL_GPL(fwnode_find_parent_dev_match);

Thanks for the review and test.
Marcin

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Vladimir Oltean]

On Wed, Jul 27, 2022 at 07:40:00PM +0200, Marcin Wojtas wrote:
> SET_NETDEV_DEV() fills net_device->dev.parent with &pdev->dev
> and in most cases it is sufficient apparently it is sufficient for
> fwnode_find_parent_dev_match (at least tests with mvneta case proves
> it's fine).

Indeed, mvneta works, which is a plain old platform device that hasn't
even been converted to fwnode, so why don't the others?

Well, as it turns out, it's one of the cases where I spoke to soon,
thinking I knew what was the problem why probing failed, before actually
debugging.

I thought there was no dmesg output from DSA at all, which would have
indicated an eternal -EPROBE_DEFER situation. But there's one tiny line
I had overlooked:

[    5.094013] mscc_felix 0000:00:00.5: error -EINVAL: Failed to register DSA switch

This comes from here:

static int dsa_port_parse_fw(struct dsa_port *dp, struct fwnode_handle *fwnode)
{
	struct fwnode_handle *ethernet = fwnode_find_reference(fwnode, "ethernet", 0);
	bool link = fwnode_property_present(fwnode, "link");
	const char *name = NULL;
	int ret;

	ret = fwnode_property_read_string(fwnode, "label", &name);
//	if (ret)
//		return ret;

	dp->fwnode = fwnode;

The 'label' property of a port was optional, you've made it mandatory by accident.
It is used only by DSA drivers that register using platform data.

(side note, I can't believe you actually have a 'label' property for the
CPU port and how many people are in the same situation as you; you know
it isn't used for anything, right? how do we stop the cargo cult?)

> Can you please check applying following diff:
> 
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -695,20 +695,22 @@ EXPORT_SYMBOL_GPL(fwnode_get_nth_parent);
>   * The routine can be used e.g. as a callback for class_find_device().
>   *
>   * Returns: %1 - match is found
>   *          %0 - match not found
>   */
>  int fwnode_find_parent_dev_match(struct device *dev, const void *data)
>  {
>         for (; dev; dev = dev->parent) {
>                 if (device_match_fwnode(dev, data))
>                         return 1;
> +               else if (device_match_of_node(dev, to_of_node(data))
> +                       return 1;
>         }
> 
>         return 0;
>  }
>  EXPORT_SYMBOL_GPL(fwnode_find_parent_dev_match);

So, nothing to do with device_match_fwnode() failing, that would have
been strange, come to think about it. Sorry for the noise here.

But looking at the deeper implementation of dev_fwnode() as:

struct fwnode_handle *dev_fwnode(struct device *dev)
{
	return IS_ENABLED(CONFIG_OF) && dev->of_node ?
		of_fwnode_handle(dev->of_node) : dev->fwnode;
}

I wonder, why did you have to modify mvpp2? It looks at dev->of_node
prior to returning dev->fwnode. It should work.

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Vladimir Oltean]

On Thu, Jul 28, 2022 at 12:11:12AM +0300, Vladimir Oltean wrote:
> The 'label' property of a port was optional, you've made it mandatory by accident.
> It is used only by DSA drivers that register using platform data.

I didn't mean to say exactly this, the second phrase was an addition
through which I tried to make it clear that the "cpu" label *is* used,
but only by the drivers using platform data. With OF it's not, neither
that nor label = "dsa". We have other means of recognizing a CPU or DSA
port, by the 'ethernet' and/or 'dsa' phandles.

Additionally, the label is optional even for user port. One can have
udev rules that assign names to Ethernet ports. I think that is even
encouraged; some of the things in DSA predate the establishment of some
best practices.

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Marcin Wojtas]

śr., 27 lip 2022 o 23:11 Vladimir Oltean <olteanv@gmail.com> napisał(a):
>
> On Wed, Jul 27, 2022 at 07:40:00PM +0200, Marcin Wojtas wrote:
> > SET_NETDEV_DEV() fills net_device->dev.parent with &pdev->dev
> > and in most cases it is sufficient apparently it is sufficient for
> > fwnode_find_parent_dev_match (at least tests with mvneta case proves
> > it's fine).
>
> Indeed, mvneta works, which is a plain old platform device that hasn't
> even been converted to fwnode, so why don't the others?
>
> Well, as it turns out, it's one of the cases where I spoke to soon,
> thinking I knew what was the problem why probing failed, before actually
> debugging.
>
> I thought there was no dmesg output from DSA at all, which would have
> indicated an eternal -EPROBE_DEFER situation. But there's one tiny line
> I had overlooked:
>
> [    5.094013] mscc_felix 0000:00:00.5: error -EINVAL: Failed to register DSA switch
>
> This comes from here:
>
> static int dsa_port_parse_fw(struct dsa_port *dp, struct fwnode_handle *fwnode)
> {
>         struct fwnode_handle *ethernet = fwnode_find_reference(fwnode, "ethernet", 0);
>         bool link = fwnode_property_present(fwnode, "link");
>         const char *name = NULL;
>         int ret;
>
>         ret = fwnode_property_read_string(fwnode, "label", &name);
> //      if (ret)
> //              return ret;
>
>         dp->fwnode = fwnode;
>
> The 'label' property of a port was optional, you've made it mandatory by accident.
> It is used only by DSA drivers that register using platform data.

Thanks for spotting that, I will make it optional again.

>
> (side note, I can't believe you actually have a 'label' property for the
> CPU port and how many people are in the same situation as you; you know
> it isn't used for anything, right? how do we stop the cargo cult?)
>

Well, given these results:
~/git/linux : git grep 'label = \"cpu\"' arch/arm/boot/dts/ | wc -l
79
~/git/linux : git grep 'label = \"cpu\"' arch/arm64/boot/dts/ | wc -l
14

It's not a surprise I also have it defined in the platforms I test. I
agree it doesn't serve any purpose in terms of creating the devices in
DSA with DT, but it IMO increases readability of the description at
least.

> > Can you please check applying following diff:
> >
> > --- a/drivers/base/property.c
> > +++ b/drivers/base/property.c
> > @@ -695,20 +695,22 @@ EXPORT_SYMBOL_GPL(fwnode_get_nth_parent);
> >   * The routine can be used e.g. as a callback for class_find_device().
> >   *
> >   * Returns: %1 - match is found
> >   *          %0 - match not found
> >   */
> >  int fwnode_find_parent_dev_match(struct device *dev, const void *data)
> >  {
> >         for (; dev; dev = dev->parent) {
> >                 if (device_match_fwnode(dev, data))
> >                         return 1;
> > +               else if (device_match_of_node(dev, to_of_node(data))
> > +                       return 1;
> >         }
> >
> >         return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(fwnode_find_parent_dev_match);
>
> So, nothing to do with device_match_fwnode() failing, that would have
> been strange, come to think about it. Sorry for the noise here.
>

Great, thank you for confirmation.

> But looking at the deeper implementation of dev_fwnode() as:
>
> struct fwnode_handle *dev_fwnode(struct device *dev)
> {
>         return IS_ENABLED(CONFIG_OF) && dev->of_node ?
>                 of_fwnode_handle(dev->of_node) : dev->fwnode;
> }
>
> I wonder, why did you have to modify mvpp2? It looks at dev->of_node
> prior to returning dev->fwnode. It should work.

When I boot with ACPI, the dev->of_node becomes NULL. With DT it's fine as-is.

Best regards,
Marcin

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Vladimir Oltean]

On Thu, Jul 28, 2022 at 08:47:58AM +0200, Marcin Wojtas wrote:
> > The 'label' property of a port was optional, you've made it mandatory by accident.
> > It is used only by DSA drivers that register using platform data.
> 
> Thanks for spotting that, I will make it optional again.
> 
> > (side note, I can't believe you actually have a 'label' property for the
> > CPU port and how many people are in the same situation as you; you know
> > it isn't used for anything, right? how do we stop the cargo cult?)
> 
> Well, given these results:
> ~/git/linux : git grep 'label = \"cpu\"' arch/arm/boot/dts/ | wc -l
> 79
> ~/git/linux : git grep 'label = \"cpu\"' arch/arm64/boot/dts/ | wc -l
> 14
> 
> It's not a surprise I also have it defined in the platforms I test. I
> agree it doesn't serve any purpose in terms of creating the devices in
> DSA with DT, but it IMO increases readability of the description at
> least.

We've glided over this way too easily, so I'll repeat this thing I've said:

| One can have udev rules that assign names to Ethernet ports. I think
| that is even encouraged; some of the things in DSA predate the
| establishment of some best practices.

I know I'm not exactly "upfront" by saying this at v3 rather than earlier,
but I haven't had the time and I still don't have as much as I'd like.
Sorry for that.

Please don't jump to sending v4 just yet, and please don't expect that
this patch set will make it for the upcoming 5.20 release candidates.

ACPI is a whole new world and I don't think we want to mass-migrate each
and every OF binding that DSA has to the generic fwnode form, at least
not without having a serious discussion about it.

The 'label' thing is actually one of the things that I'm seriously
considering skipping parsing if this is an ACPI system, simply because
best practices are different today than they were when the OF bindings
were created.

There's also the change that validates that phylink has the fwnode
properties it needs to work properly:
https://patchwork.kernel.org/project/netdevbpf/patch/20220723164635.1621911-1-vladimir.oltean@nxp.com/

Please don't even think that the DSA fwnode conversion will be merged
before the validation patch (sorry, I'm not saying this to block you,
I'm saying this because I don't want DSA to start with zero-day baggage
on ACPI).

And even when the validation patch gets merged, you'll need to adapt it
to fwnode because that's what will be required syntactically, but we'll
only go through the motions of calling of_device_compatible_match() for
the OF case. With ACPI, every driver will opt into strict validation,
that's non negotiable.

And then there are some other issues we've learned about, with the DT
bindings that specific drivers such as mv88e6xxx and realtek-smi have.
I'll give you more details once we get to the actual mv88e6xxx
conversion to ACPI; currently my memory lacks some of the precise
details of how come mv88e6xxx came to not observe the issue but
realtek-smi did. Anyway, the issue was that fw_devlink causes the
internal PHYs to probe with the generic rather than the specific PHY
driver, if interrupts are being used (and provided by the switch as
'interrupt-controller').

It can be debated what exactly is at fault there, although one
interpretation can be that the DT bindings themselves are to blame,
for describing a circular dependency between a parent and a child.
I've been suggesting the authors of new drivers to take an alternative
approach and describe the switch chip as a MFD, with only the actual
switching component being probed by DSA and the rest being separate
drivers:
https://lore.kernel.org/all/20211204022037.dkipkk42qet4u7go@skbuf/T/

You'll say, ok but don't we have to keep maintaining mv8e6xxx OF bindings
functional with fw_devlink too anyway, so what benefit would there be if
for ACPI we'd split the exact same monolithic driver into a MFD?
And maybe you have a point, I don't know, I haven't actually tried to
look at the code and see if it could be restructured cleanly to probe
and work in both cases.

The bottom line is that if you haven't received too much review for your
series until now, I suspect it's because none of the DSA maintainers
cropped a large enough chunk of time yet to actually clarify things for
themselves. I don't consider the things I've pointed out here to be a
'review' in the proper sense, they're just cases I'm thinking about in
the back of my mind where we should learn from past mistakes.
I'll revisit when I will have come to some conclusions.

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Andrew Lunn]

> The 'label' thing is actually one of the things that I'm seriously
> considering skipping parsing if this is an ACPI system, simply because
> best practices are different today than they were when the OF bindings
> were created.

Agreed. We want the ACPI binding to learn from what has worked and not
worked in DT. We should clean up some of the historical mess. And
enforce things we don't in DT simply because there is too much
history.

So a straight one to one conversion is not going to happen.

> It can be debated what exactly is at fault there, although one
> interpretation can be that the DT bindings themselves are to blame,
> for describing a circular dependency between a parent and a child.

DT describes hardware. I'm not sure hardware can have a circular
dependency. It is more about how software make use of that hardware
description that ends up in circular dependencies.

	    Andrew

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Marcin Wojtas]

czw., 28 lip 2022 o 22:18 Andrew Lunn <andrew@lunn.ch> napisał(a):
>
> > The 'label' thing is actually one of the things that I'm seriously
> > considering skipping parsing if this is an ACPI system, simply because
> > best practices are different today than they were when the OF bindings
> > were created.
>
> Agreed. We want the ACPI binding to learn from what has worked and not
> worked in DT. We should clean up some of the historical mess. And
> enforce things we don't in DT simply because there is too much
> history.
>
> So a straight one to one conversion is not going to happen.

I understand your standpoint - there is a long history, possible
clean-ups, backward compatibility considerations, etc. that should not
be zero-day baggage of ACPI. Otoh, we don't need to be worried about
the ACPI binding too much now - as agreed it was removed from this
series, beginning from v2. IMO it may be better to return to that once
the ACPI Spec is updated with the MDIOSerialBus and the patches are
resubmitted on whatever shape of the DSA subsystem is established
within the next weeks/months from now.

In v1 we discussed also the resubmission of the non-ACPI-related
patches, which would pave the way to dropping the explicit OF_
dependency in the DSA and moving to a generic hardware description
kernel API - without any functional change. Modifying DT bindings and
clean-ups could be done on top this patchset as well. Of course, it is
the subsystems' Maintainers call and I'll adjust accordingly - if you
wish me to wait and rebase after the 'validation patch' lands in
net-next, I'll do that.

A side note: I was of course aware that making it for the v5.20 would
be extremely hard, but I decided to give it a try anyway - I had to
wait for some time, as this series was gated by fate of the eventually
abandoned phylink-related changes.

Thanks,
Marcin

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Andrew Lunn]

On Thu, Jul 28, 2022 at 11:23:31PM +0200, Marcin Wojtas wrote:
> czw., 28 lip 2022 o 22:18 Andrew Lunn <andrew@lunn.ch> napisał(a):
> >
> > > The 'label' thing is actually one of the things that I'm seriously
> > > considering skipping parsing if this is an ACPI system, simply because
> > > best practices are different today than they were when the OF bindings
> > > were created.
> >
> > Agreed. We want the ACPI binding to learn from what has worked and not
> > worked in DT. We should clean up some of the historical mess. And
> > enforce things we don't in DT simply because there is too much
> > history.
> >
> > So a straight one to one conversion is not going to happen.
> 
> I understand your standpoint - there is a long history, possible
> clean-ups, backward compatibility considerations, etc. that should not
> be zero-day baggage of ACPI. Otoh, we don't need to be worried about
> the ACPI binding too much now - as agreed it was removed from this
> series, beginning from v2. IMO it may be better to return to that once
> the ACPI Spec is updated with the MDIOSerialBus and the patches are
> resubmitted on whatever shape of the DSA subsystem is established
> within the next weeks/months from now.
> 
> In v1 we discussed also the resubmission of the non-ACPI-related
> patches, which would pave the way to dropping the explicit OF_
> dependency in the DSA and moving to a generic hardware description
> kernel API - without any functional change.

Ideally, we want to keep all the ugly DT stuff in DT. We want to
ensure that any "generic hardware description kernel API" does not
inherit all the ugly DT stuff.

ACPI and DT are different things, so i don't see why they need to
share code.

      Andrew

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Andy Shevchenko]

On Fri, Jul 29, 2022 at 12:29 AM Andrew Lunn <andrew@lunn.ch> wrote:
> On Thu, Jul 28, 2022 at 11:23:31PM +0200, Marcin Wojtas wrote:

...

> ACPI and DT are different things, so i don't see why they need to
> share code.

Yes and no.

ACPI _DSD extension allows us to share a lot of code when it comes to
specific device properties (that are not standardized anyhow by ACPI
specification, and for the record many of them even may not, but some
are, like MIPI camera). Maybe you want to have something like a
property standard for DSA that can be published as MIPI or so and then
that part of the code of course may very well be shared. Discussed
MDIOSerialBus() resource type is pure ACPI thingy if going to be
standardized and indeed, that part can't be shared.

Entire exercise with fwnodes allows to make drivers (most of or most
of the parts of) to be shared between different resource providers.
And this is a cool part about it.

-- 
With Best Regards,
Andy Shevchenko

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Marcin Wojtas]

śr., 27 lip 2022 o 23:15 Andy Shevchenko <andy.shevchenko@gmail.com> napisał(a):
>
>
>
> On Wednesday, July 27, 2022, Marcin Wojtas <mw@semihalf.com> wrote:
>>
>> śr., 27 lip 2022 o 18:38 Vladimir Oltean <olteanv@gmail.com> napisał(a):
>> >
>> > On Wed, Jul 27, 2022 at 05:18:16PM +0200, Marcin Wojtas wrote:
>> > > Do you mean a situation analogous to what I addressed in:
>> > > [net-next: PATCH v3 4/8] net: mvpp2: initialize port fwnode pointer
>> > > ?
>> >
>> > Not sure if "analogous" is the right word. My estimation is that the
>> > overwhelmingly vast majority of DSA masters can be found by DSA simply
>> > due to the SET_NETDEV_DEV() call that the Ethernet drivers need to make
>> > anyway.  I see that mvpp2 also needed commit c4053ef32208 ("net: mvpp2:
>> > initialize port of_node pointer"), but that isn't needed in general, and
>> > I can't tell you exactly why it is needed there, I don't know enough
>> > about the mvpp2 driver.
>>
>> SET_NETDEV_DEV() fills net_device->dev.parent with &pdev->dev
>> and in most cases it is sufficient apparently it is sufficient for
>> fwnode_find_parent_dev_match (at least tests with mvneta case proves
>> it's fine).
>>
>> We have some corner cases though:
>> * mvpp2 -> single controller can handle up to 3 net_devices and
>> therefore we need device_set_node() to make this work. I think dpaa2
>> is a similar case
>> * PCIE drivers with extra DT description (I think that's the case of enetc).
>>
>> >
>> > > I found indeed a couple of drivers that may require a similar change
>> > > (e.g. dpaa2).
>> >
>> > There I can tell you why the dpaa2-mac code mangles with net_dev->dev.of_node,
>> > but I'd rather not go into an explanation that essentially doesn't matter.
>> > The point is that you'd be mistaken to think that only the drivers which
>> > touch the net device's ->dev->of_node are the ones that need updating
>> > for your series to not cause regressions.
>>
>> As above - SET_NETDEV_DEV() should be fine in most cases, but we can
>> never be 100% sure untils it's verified.
>>
>> >
>> > > IMO we have 2 options:
>> > > - update these drivers
>> > > - add some kind of fallback? If yes, I am wondering about an elegant
>> > > solution - maybe add an extra check inside
>> > > fwnode_find_parent_dev_match?
>> > >
>> > > What would you suggest?
>> >
>> > Fixing fwnode_find_parent_dev_match(), of course.
>
>
>
> Fixing how?!
>
>
>>
>>
>> This change broke DSA
>> > on my LS1028A system (master in drivers/net/ethernet/freescale/enetc/)
>> > and LS1021A (master in drivers/net/ethernet/freescale/gianfar.c).
>>
>> Can you please check applying following diff:
>>
>> --- a/drivers/base/property.c
>> +++ b/drivers/base/property.c
>> @@ -695,20 +695,22 @@ EXPORT_SYMBOL_GPL(fwnode_get_nth_parent);
>>   * The routine can be used e.g. as a callback for class_find_device().
>>   *
>>   * Returns: %1 - match is found
>>   *          %0 - match not found
>>   */
>>  int fwnode_find_parent_dev_match(struct device *dev, const void *data)
>>  {
>>         for (; dev; dev = dev->parent) {
>>                 if (device_match_fwnode(dev, data))
>>                         return 1;
>> +               else if (device_match_of_node(dev, to_of_node(data))
>> +                       return 1;
>>         }
>>
>
> This adds a piece of dead code. device_match_fwnode() covers this already.
>

Yes, indeed. After recent update, I think we can assume the current
implementation of fwnode_find_parent_dev_match should work fine with
all existing cases.

Thank you for all remarks and comments, I'll address them in v4 later today.

Best regards,
Marcin

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Vladimir Oltean]

On Thu, Jul 28, 2022 at 08:52:04AM +0200, Marcin Wojtas wrote:
> Yes, indeed. After recent update, I think we can assume the current
> implementation of fwnode_find_parent_dev_match should work fine with
> all existing cases.

What you should really be fixing is the commit message of patch 4,
that's what threw me off:

| As a preparation to switch the DSA subsystem from using
| of_find_net_device_by_node() to its more generic fwnode_
| equivalent, the port's device structure should be updated
| with its fwnode pointer, similarly to of_node - see analogous
| commit c4053ef32208 ("net: mvpp2: initialize port of_node pointer").
| 
| This patch is required to prevent a regression before updating
| the DSA API on boards that connect the mvpp2 port to switch,
| such as Clearfog GT-8K or CN913x CEx7 Evaluation Board.

There's no regression to speak of. DSA didn't work with ACPI before, and
fwnode_find_net_device_by_node() still works with the plain dev->of_node
assignment.

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Marcin Wojtas]

czw., 28 lip 2022 o 11:16 Vladimir Oltean <olteanv@gmail.com> napisał(a):
>
> On Thu, Jul 28, 2022 at 08:52:04AM +0200, Marcin Wojtas wrote:
> > Yes, indeed. After recent update, I think we can assume the current
> > implementation of fwnode_find_parent_dev_match should work fine with
> > all existing cases.
>
> What you should really be fixing is the commit message of patch 4,
> that's what threw me off:
>
> | As a preparation to switch the DSA subsystem from using
> | of_find_net_device_by_node() to its more generic fwnode_
> | equivalent, the port's device structure should be updated
> | with its fwnode pointer, similarly to of_node - see analogous
> | commit c4053ef32208 ("net: mvpp2: initialize port of_node pointer").
> |
> | This patch is required to prevent a regression before updating
> | the DSA API on boards that connect the mvpp2 port to switch,
> | such as Clearfog GT-8K or CN913x CEx7 Evaluation Board.
>
> There's no regression to speak of. DSA didn't work with ACPI before, and
> fwnode_find_net_device_by_node() still works with the plain dev->of_node
> assignment.

There was a regression even for OF in v1, but after switching to
device_match_fwnode() it works indeed. Anyway patch v4 is imo useful,
I'll only reword the commit message.

Thanks,
Marcin

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Vladimir Oltean]

On Thu, Jul 28, 2022 at 06:56:48PM +0200, Marcin Wojtas wrote:
> There was a regression even for OF in v1, but after switching to
> device_match_fwnode() it works indeed. Anyway patch v4 is imo useful,
> I'll only reword the commit message.

Do you mean patch 4 or patch v4? If patch 4, of course it's useful, but
not for avoiding a regression with OF (case in which I drop all my claims
made earlier about fw_find_net_device_by_node), but rather to actually
get something working with actual ACPI (although perhaps not in this
series, you'll need to add ACPI IDs in the mv88e6xxx driver some time
later as well, maybe you could focus this series just on converting DSA
to play nice with fwnodes). If you're already thinking about the v4 of
this patch set, I'll respond to that in a separate email shortly.

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Marcin Wojtas]

czw., 28 lip 2022 o 21:16 Vladimir Oltean <olteanv@gmail.com> napisał(a):
>
> On Thu, Jul 28, 2022 at 06:56:48PM +0200, Marcin Wojtas wrote:
> > There was a regression even for OF in v1, but after switching to
> > device_match_fwnode() it works indeed. Anyway patch v4 is imo useful,
> > I'll only reword the commit message.
>
> Do you mean patch 4 or patch v4? If patch 4, of course it's useful, but

Patch 4/8 in v4 :) I'm working on it right now to submit asap.

> not for avoiding a regression with OF (case in which I drop all my claims
> made earlier about fw_find_net_device_by_node), but rather to actually

Change in the mvpp2 driver:
-       dev->dev.of_node = port_node;
+       device_set_node(&dev->dev, port_fwnode);
is desired and correct anyway, so as a low-cost change I think it can
be included in this series (which is in fact preparation-to-ACPI
support). I will update the commit message. accordingly.

> get something working with actual ACPI (although perhaps not in this
> series, you'll need to add ACPI IDs in the mv88e6xxx driver some time

v1 added all of this, but we agreed that ACPI-specific bits should be
sent separately later, after extending the ACPI Specification.

> later as well, maybe you could focus this series just on converting DSA
> to play nice with fwnodes). If you're already thinking about the v4 of
> this patch set, I'll respond to that in a separate email shortly.

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Andy Shevchenko]

On Wed, Jul 27, 2022 at 5:24 PM Marcin Wojtas <mw@semihalf.com> wrote:
> śr., 27 lip 2022 o 16:31 Vladimir Oltean <olteanv@gmail.com> napisał(a):
> > On Wed, Jul 27, 2022 at 08:43:19AM +0200, Marcin Wojtas wrote:

...

> > > +     dev = class_find_device(&net_class, NULL, fwnode, fwnode_find_parent_dev_match);
> >
> > This needs to maintain compatibility with DSA masters that have
> > dev->of_node but don't have dev->fwnode populated.
>
> Do you mean a situation analogous to what I addressed in:
> [net-next: PATCH v3 4/8] net: mvpp2: initialize port fwnode pointer
> ?
>
> I found indeed a couple of drivers that may require a similar change
> (e.g. dpaa2).
>
> IMO we have 2 options:
> - update these drivers

Not Vladimir here, but my 2cents that update is best and elegant, it
can be done even before this series.

-- 
With Best Regards,
Andy Shevchenko

Re: [net-next: PATCH v3 6/8] net: core: switch to fwnode_find_net_device_by_node()

[Marcin Wojtas]

śr., 27 lip 2022 o 19:00 Andy Shevchenko <andy.shevchenko@gmail.com> napisał(a):
>
> On Wed, Jul 27, 2022 at 5:24 PM Marcin Wojtas <mw@semihalf.com> wrote:
> > śr., 27 lip 2022 o 16:31 Vladimir Oltean <olteanv@gmail.com> napisał(a):
> > > On Wed, Jul 27, 2022 at 08:43:19AM +0200, Marcin Wojtas wrote:
>
> ...
>
> > > > +     dev = class_find_device(&net_class, NULL, fwnode, fwnode_find_parent_dev_match);
> > >
> > > This needs to maintain compatibility with DSA masters that have
> > > dev->of_node but don't have dev->fwnode populated.
> >
> > Do you mean a situation analogous to what I addressed in:
> > [net-next: PATCH v3 4/8] net: mvpp2: initialize port fwnode pointer
> > ?
> >
> > I found indeed a couple of drivers that may require a similar change
> > (e.g. dpaa2).
> >
> > IMO we have 2 options:
> > - update these drivers
>
> Not Vladimir here, but my 2cents that update is best and elegant, it
> can be done even before this series.
>

In general I agree it's desired, but I'm not sure if we can catch all
cases just by reading code or rather base on regression reports
later...

Best regards,
Marcin

[net-next: PATCH v3 7/8] net: mdio: introduce fwnode_mdiobus_register_device()

[Marcin Wojtas]

As a preparation patch to extend MDIO capabilities in the ACPI world,
introduce fwnode_mdiobus_register_device() to register non-PHY
devices on the mdiobus.

Use the newly introduced routine instead of of_mdiobus_register_device().

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 include/linux/fwnode_mdio.h    |  3 ++
 drivers/net/mdio/fwnode_mdio.c | 29 ++++++++++++++++++
 drivers/net/mdio/of_mdio.c     | 32 +-------------------
 3 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/include/linux/fwnode_mdio.h b/include/linux/fwnode_mdio.h
index 98755b8c6c8a..39d74c5d1bb0 100644
--- a/include/linux/fwnode_mdio.h
+++ b/include/linux/fwnode_mdio.h
@@ -16,6 +16,9 @@ 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_device(struct mii_bus *mdio,
+				   struct fwnode_handle *child, u32 addr);
+
 int fwnode_phy_register_fixed_link(struct fwnode_handle *fwnode);
 
 void fwnode_phy_deregister_fixed_link(struct fwnode_handle *fwnode);
diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index 454fdae24150..3743f34e7c2d 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -149,6 +149,35 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 }
 EXPORT_SYMBOL(fwnode_mdiobus_register_phy);
 
+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);
+
+	/* Associate the fwnode with the device structure so it
+	 * can be looked up later.
+	 */
+	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 %p fwnode at address %i\n",
+		child, addr);
+	return 0;
+}
+EXPORT_SYMBOL(fwnode_mdiobus_register_device);
+
 /*
  * fwnode_phy_is_fixed_link() and fwnode_phy_register_fixed_link() must
  * support two bindings:
diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index 409da6e92f7d..bd941da030bb 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -48,36 +48,6 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio,
 	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
@@ -186,7 +156,7 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
 		if (of_mdiobus_child_is_phy(child))
 			rc = of_mdiobus_register_phy(mdio, child, addr);
 		else
-			rc = of_mdiobus_register_device(mdio, child, addr);
+			rc = fwnode_mdiobus_register_device(mdio, of_fwnode_handle(child), addr);
 
 		if (rc == -ENODEV)
 			dev_err(&mdio->dev,
-- 
2.29.0

Re: [net-next: PATCH v3 7/8] net: mdio: introduce fwnode_mdiobus_register_device()

[Andy Shevchenko]

On Wed, Jul 27, 2022 at 08:43:20AM +0200, Marcin Wojtas wrote:
> As a preparation patch to extend MDIO capabilities in the ACPI world,
> introduce fwnode_mdiobus_register_device() to register non-PHY
> devices on the mdiobus.

...

> +	dev_dbg(&mdio->dev, "registered mdio device %p fwnode at address %i\n",
> +		child, addr);

"%p" makes a little sense (and we have hashed pointers). I think the "%pfw"
would be much better for the user to see.

...

> -	dev_dbg(&mdio->dev, "registered mdio device %pOFn at address %i\n",
> -		child, addr);

Exactly my point above.

-- 
With Best Regards,
Andy Shevchenko

[net-next: PATCH v3 8/8] net: dsa: mv88e6xxx: switch to device_/fwnode_ APIs

[Marcin Wojtas]

In order to support both DT and ACPI in future, modify the
mv88e6xx driver  code to use device_/fwnode_ equivalent routines.
No functional change is introduced by this patch.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 43 +++++++++-----------
 1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index a46ebdfba1c3..e7848b56316f 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3891,10 +3891,11 @@ static int mv88e6xxx_mdio_write(struct mii_bus *bus, int phy, int reg, u16 val)
 }
 
 static int mv88e6xxx_mdio_register(struct mv88e6xxx_chip *chip,
-				   struct device_node *np,
+				   struct fwnode_handle *fwnode,
 				   bool external)
 {
 	static int index;
+	struct device_node *np = to_of_node(fwnode);
 	struct mv88e6xxx_mdio_bus *mdio_bus;
 	struct mii_bus *bus;
 	int err;
@@ -3973,18 +3974,18 @@ static void mv88e6xxx_mdios_unregister(struct mv88e6xxx_chip *chip)
 }
 
 static int mv88e6xxx_mdios_register(struct mv88e6xxx_chip *chip,
-				    struct device_node *np)
+				    struct fwnode_handle *fwnode)
 {
-	struct device_node *child;
+	struct fwnode_handle *child;
 	int err;
 
 	/* Always register one mdio bus for the internal/default mdio
 	 * bus. This maybe represented in the device tree, but is
 	 * optional.
 	 */
-	child = of_get_child_by_name(np, "mdio");
+	child = fwnode_get_named_child_node(fwnode, "mdio");
 	err = mv88e6xxx_mdio_register(chip, child, false);
-	of_node_put(child);
+	fwnode_handle_put(child);
 	if (err)
 		return err;
 
@@ -3992,13 +3993,13 @@ static int mv88e6xxx_mdios_register(struct mv88e6xxx_chip *chip,
 	 * which say they are compatible with the external mdio
 	 * bus.
 	 */
-	for_each_available_child_of_node(np, child) {
-		if (of_device_is_compatible(
-			    child, "marvell,mv88e6xxx-mdio-external")) {
+	fwnode_for_each_available_child_node(fwnode, child) {
+		if (fwnode_property_match_string(child, "compatible",
+						 "marvell,mv88e6xxx-mdio-external") == 0) {
 			err = mv88e6xxx_mdio_register(chip, child, true);
 			if (err) {
 				mv88e6xxx_mdios_unregister(chip);
-				of_node_put(child);
+				fwnode_handle_put(child);
 				return err;
 			}
 		}
@@ -6984,20 +6985,16 @@ static SIMPLE_DEV_PM_OPS(mv88e6xxx_pm_ops, mv88e6xxx_suspend, mv88e6xxx_resume);
 static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 {
 	struct dsa_mv88e6xxx_pdata *pdata = mdiodev->dev.platform_data;
+	struct fwnode_handle *fwnode = dev_fwnode(&mdiodev->dev);
 	const struct mv88e6xxx_info *compat_info = NULL;
 	struct device *dev = &mdiodev->dev;
-	struct device_node *np = dev->of_node;
 	struct mv88e6xxx_chip *chip;
 	int port;
 	int err;
 
-	if (!np && !pdata)
-		return -EINVAL;
-
-	if (np)
-		compat_info = of_device_get_match_data(dev);
-
-	if (pdata) {
+	if (fwnode)
+		compat_info = device_get_match_data(dev);
+	else if (pdata) {
 		compat_info = pdata_device_get_match_data(dev);
 
 		if (!pdata->netdev)
@@ -7054,9 +7051,9 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	mv88e6xxx_phy_init(chip);
 
 	if (chip->info->ops->get_eeprom) {
-		if (np)
-			of_property_read_u32(np, "eeprom-length",
-					     &chip->eeprom_len);
+		if (fwnode)
+			device_property_read_u32(dev, "eeprom-length",
+						 &chip->eeprom_len);
 		else
 			chip->eeprom_len = pdata->eeprom_len;
 	}
@@ -7067,8 +7064,8 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	if (err)
 		goto out;
 
-	if (np) {
-		chip->irq = of_irq_get(np, 0);
+	if (fwnode) {
+		chip->irq = fwnode_irq_get(fwnode, 0);
 		if (chip->irq == -EPROBE_DEFER) {
 			err = chip->irq;
 			goto out;
@@ -7106,7 +7103,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	if (err)
 		goto out_g1_atu_prob_irq;
 
-	err = mv88e6xxx_mdios_register(chip, np);
+	err = mv88e6xxx_mdios_register(chip, fwnode);
 	if (err)
 		goto out_g1_vtu_prob_irq;
 
-- 
2.29.0

Re: [net-next: PATCH v3 8/8] net: dsa: mv88e6xxx: switch to device_/fwnode_ APIs

[Andy Shevchenko]

On Wed, Jul 27, 2022 at 08:43:21AM +0200, Marcin Wojtas wrote:
> In order to support both DT and ACPI in future, modify the
> mv88e6xx driver  code to use device_/fwnode_ equivalent routines.
> No functional change is introduced by this patch.

...

>  static int mv88e6xxx_probe(struct mdio_device *mdiodev)
>  {
>  	struct dsa_mv88e6xxx_pdata *pdata = mdiodev->dev.platform_data;

> +	struct fwnode_handle *fwnode = dev_fwnode(&mdiodev->dev);

Move this...

>  	const struct mv88e6xxx_info *compat_info = NULL;
>  	struct device *dev = &mdiodev->dev;
> -	struct device_node *np = dev->of_node;

...here as

	struct fwnode_handle *fwnode = dev_fwnode(dev);

>  	struct mv88e6xxx_chip *chip;
>  	int port;
>  	int err;

> +	if (fwnode)

Redundant check.

> +		compat_info = device_get_match_data(dev);
> +	else if (pdata) {
>  		compat_info = pdata_device_get_match_data(dev);

	compat_info - device_get_match_data(dev);
	if (!compat_info && pdata)
		compat_info = pdata_device_get_match_data(dev);

...

> +		if (fwnode)
> +			device_property_read_u32(dev, "eeprom-length",
> +						 &chip->eeprom_len);
>  		else
>  			chip->eeprom_len = pdata->eeprom_len;

Can be done w/o conditional

		chip->eeprom_len = pdata->eeprom_len;
		device_property_read_u32(dev, "eeprom-length", &chip->eeprom_len);

-- 
With Best Regards,
Andy Shevchenko