diff mbox series

[net-next:,03/12] net: dsa: switch to device_/fwnode_ APIs

Message ID 20220620150225.1307946-4-mw@semihalf.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series ACPI support for DSA | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 18 this patch: 18
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/build_clang success Errors and warnings before: 14 this patch: 14
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 18 this patch: 18
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns WARNING: line length of 82 exceeds 80 columns WARNING: line length of 83 exceeds 80 columns WARNING: line length of 86 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Marcin Wojtas June 20, 2022, 3:02 p.m. UTC 
In order to support both ACPI and DT, modify the generic
DSA code to use device_/fwnode_ equivalent routines.
No functional change is introduced by this patch.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 include/net/dsa.h |  1 +
 net/dsa/dsa2.c    | 76 +++++++++++---------
 net/dsa/port.c    | 54 +++++++-------
 net/dsa/slave.c   |  6 +-
 4 files changed, 71 insertions(+), 66 deletions(-)

Comments

Andy Shevchenko June 20, 2022, 5:41 p.m. UTC | #1 
On Mon, Jun 20, 2022 at 05:02:16PM +0200, Marcin Wojtas wrote:
> In order to support both ACPI and DT, modify the generic
> DSA code to use device_/fwnode_ equivalent routines.
> No functional change is introduced by this patch.

...

>  	struct device_node	*dn;

What prevents us from removing this?

> +	struct fwnode_handle    *fwnode;

...

> -		dn = of_get_child_by_name(ds->dev->of_node, "mdio");
> +		fwnode = fwnode_get_named_child_node(ds->dev->fwnode, "mdio");

The rule of thumb is avoid dereferencing fwnode from struct device. So
dev_fwnode(), but here it would be achieved by device_get_named_child_node().

...

> -static int dsa_switch_parse_of(struct dsa_switch *ds, struct device_node *dn)
> +static int dsa_switch_parse_of(struct dsa_switch *ds, struct fwnode_handle *fwnode)

Shouldn't _of suffix be replaced by, let's say, _fw?

...

> -	return dsa_switch_parse_ports_of(ds, dn);
> +	return dsa_switch_parse_ports_of(ds, fwnode);

Ditto.

...

> +	fwnode = ds->dev->fwnode;

dev_fwnode() or corresponding device_property_ API.

...

>  	slave_dev->dev.of_node = port->dn;
> +	slave_dev->dev.fwnode = port->fwnode;

device_set_node()
Marcin Wojtas June 21, 2022, 9:27 a.m. UTC | #2 
pon., 20 cze 2022 o 19:41 Andy Shevchenko
<andriy.shevchenko@linux.intel.com> napisał(a):
>
> On Mon, Jun 20, 2022 at 05:02:16PM +0200, Marcin Wojtas wrote:
> > In order to support both ACPI and DT, modify the generic
> > DSA code to use device_/fwnode_ equivalent routines.
> > No functional change is introduced by this patch.
>
> ...
>
> >       struct device_node      *dn;
>
> What prevents us from removing this?

I left it to satisfy possible issues with backward compatibility - I
migrated mv88e6xxx, other DSA drivers still rely on of_* and may use
this field.

>
> > +     struct fwnode_handle    *fwnode;
>
> ...
>
> > -             dn = of_get_child_by_name(ds->dev->of_node, "mdio");
> > +             fwnode = fwnode_get_named_child_node(ds->dev->fwnode, "mdio");
>
> The rule of thumb is avoid dereferencing fwnode from struct device. So
> dev_fwnode(), but here it would be achieved by device_get_named_child_node().
>

Ok, thanks - will do for all occurences.

> ...
>
> > -static int dsa_switch_parse_of(struct dsa_switch *ds, struct device_node *dn)
> > +static int dsa_switch_parse_of(struct dsa_switch *ds, struct fwnode_handle *fwnode)
>
> Shouldn't _of suffix be replaced by, let's say, _fw?
>

I thought about it and can perform such naming update in next iteration.

> ...
>
> > -     return dsa_switch_parse_ports_of(ds, dn);
> > +     return dsa_switch_parse_ports_of(ds, fwnode);
>
> Ditto.
>
> ...
>
> > +     fwnode = ds->dev->fwnode;
>
> dev_fwnode() or corresponding device_property_ API.
>

OK.

> ...
>
> >       slave_dev->dev.of_node = port->dn;
> > +     slave_dev->dev.fwnode = port->fwnode;
>
> device_set_node()
>

OK.

Thanks,
Marcin
Andy Shevchenko June 21, 2022, 11:02 a.m. UTC | #3 
On Tue, Jun 21, 2022 at 11:27:43AM +0200, Marcin Wojtas wrote:
> pon., 20 cze 2022 o 19:41 Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> napisał(a):
> > On Mon, Jun 20, 2022 at 05:02:16PM +0200, Marcin Wojtas wrote:

...

> > >       struct device_node      *dn;
> >
> > What prevents us from removing this?
> 
> I left it to satisfy possible issues with backward compatibility - I
> migrated mv88e6xxx, other DSA drivers still rely on of_* and may use
> this field.

If it is so, it's a way to get into troubles of desynchronized dn and fwnode.

> > > +     struct fwnode_handle    *fwnode;
Florian Fainelli June 22, 2022, 4:09 p.m. UTC | #4 
On 6/21/22 04:02, Andy Shevchenko wrote:
> On Tue, Jun 21, 2022 at 11:27:43AM +0200, Marcin Wojtas wrote:
>> pon., 20 cze 2022 o 19:41 Andy Shevchenko
>> <andriy.shevchenko@linux.intel.com> napisał(a):
>>> On Mon, Jun 20, 2022 at 05:02:16PM +0200, Marcin Wojtas wrote:
> 
> ...
> 
>>>>        struct device_node      *dn;
>>>
>>> What prevents us from removing this?
>>
>> I left it to satisfy possible issues with backward compatibility - I
>> migrated mv88e6xxx, other DSA drivers still rely on of_* and may use
>> this field.
> 
> If it is so, it's a way to get into troubles of desynchronized dn and fwnode.

Agreed, we can take it in baby steps if you prefer, but ultimately we 
should move to using a fwnode reference rather than a device_node 
reference and if there are drivers needing the device_node we can always 
extract it from the fwnode.
diff mbox series

Patch

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 14f07275852b..692c1dddc5f8 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -299,6 +299,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/dsa2.c b/net/dsa/dsa2.c
index cac48a741f27..039022bf914b 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -493,7 +493,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;
@@ -853,7 +853,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 +909,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 = fwnode_get_named_child_node(ds->dev->fwnode, "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 +1482,34 @@  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_of(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;
+	dp->dn = to_of_node(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);
 	}
 
@@ -1510,34 +1520,34 @@  static int dsa_port_parse_of(struct dsa_port *dp, struct device_node *dn)
 }
 
 static int dsa_switch_parse_ports_of(struct dsa_switch *ds,
-				     struct device_node *dn)
+				     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;
 		}
@@ -1546,24 +1556,24 @@  static int dsa_switch_parse_ports_of(struct dsa_switch *ds,
 
 		err = dsa_port_parse_of(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)
+				      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 +1610,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_of(struct dsa_switch *ds, struct fwnode_handle *fwnode)
 {
 	int err;
 
-	err = dsa_switch_parse_member_of(ds, dn);
+	err = dsa_switch_parse_member_of(ds, fwnode);
 	if (err)
 		return err;
 
@@ -1612,7 +1622,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_of(ds, fwnode);
 }
 
 static int dsa_port_parse(struct dsa_port *dp, const char *name,
@@ -1705,20 +1715,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 = ds->dev->fwnode;
 
 	if (!ds->num_ports)
 		return -EINVAL;
 
-	if (np) {
-		err = dsa_switch_parse_of(ds, np);
+	if (fwnode) {
+		err = dsa_switch_parse_of(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 3738f2d40a0b..a0e46e276de0 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"
 
@@ -1379,20 +1378,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;
 }
 
@@ -1524,11 +1523,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
@@ -1541,7 +1539,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));
@@ -1591,14 +1589,13 @@  static int dsa_port_setup_phy_of(struct dsa_port *dp, bool enable)
 
 static int dsa_port_fixed_link_register_of(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",
@@ -1606,10 +1603,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;
 
@@ -1626,7 +1623,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;
@@ -1636,7 +1632,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;
@@ -1651,27 +1647,27 @@  static int dsa_port_phylink_register(struct dsa_port *dp)
 
 int dsa_port_link_register_of(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))
+	if (fwnode_phy_is_fixed_link(dp->fwnode))
 		return dsa_port_fixed_link_register_of(dp);
 	else
 		return dsa_port_setup_phy_of(dp, true);
@@ -1690,8 +1686,8 @@  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);
 }
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 2e1ac638d135..b801795d73a6 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>
@@ -2204,7 +2202,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;
@@ -2228,7 +2225,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
@@ -2341,6 +2338,7 @@  int dsa_slave_create(struct dsa_port *port)
 
 	SET_NETDEV_DEV(slave_dev, port->ds->dev);
 	slave_dev->dev.of_node = port->dn;
+	slave_dev->dev.fwnode = port->fwnode;
 	slave_dev->vlan_features = master->vlan_features;
 
 	p = netdev_priv(slave_dev);