diff mbox series

[RFC,net-next,v4,26/28] net: dsa: permit driver to provide custom phy_mii_mask for slave mdiobus

Message ID 20210508002920.19945-26-ansuelsmth@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [RFC,net-next,v4,01/28] net: mdio: ipq8064: clean whitespaces in define | expand

Checks

Context Check Description
netdev/apply fail Patch does not apply to net-next
netdev/tree_selection success Clearly marked for net-next

Commit Message

Christian Marangi May 8, 2021, 12:29 a.m. UTC
Some switch doesn't have a 1:1 map phy to port. Permit driver to provide
a custom phy_mii_mask so the internal mdiobus can correctly use the
provided phy reg as it can differ from the port reg.
The qca8k driver is provided as a first user of this function.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca8k.c | 30 ++++++++++++++++++++++++++++++
 include/net/dsa.h       |  7 +++++++
 net/dsa/dsa2.c          |  7 ++++++-
 3 files changed, 43 insertions(+), 1 deletion(-)

Comments

Florian Fainelli May 8, 2021, 3:52 p.m. UTC | #1
On 5/7/2021 5:29 PM, Ansuel Smith wrote:
> Some switch doesn't have a 1:1 map phy to port. Permit driver to provide
> a custom phy_mii_mask so the internal mdiobus can correctly use the
> provided phy reg as it can differ from the port reg.
> The qca8k driver is provided as a first user of this function.

Why not have qca8k be in charge of allocating its internal MDIO bus like
what mv88e6xxx or bcm_sf2 do? That would allow you to do all sorts of
customization there and you could skip having patches 23 and 24.
Christian Marangi May 8, 2021, 4:29 p.m. UTC | #2
On Sat, May 08, 2021 at 08:52:03AM -0700, Florian Fainelli wrote:
> 
> 
> On 5/7/2021 5:29 PM, Ansuel Smith wrote:
> > Some switch doesn't have a 1:1 map phy to port. Permit driver to provide
> > a custom phy_mii_mask so the internal mdiobus can correctly use the
> > provided phy reg as it can differ from the port reg.
> > The qca8k driver is provided as a first user of this function.
> 
> Why not have qca8k be in charge of allocating its internal MDIO bus like
> what mv88e6xxx or bcm_sf2 do? That would allow you to do all sorts of
> customization there and you could skip having patches 23 and 24.

Oh ok, I will implement the internal MDIO bus directly in the qca8k
driver. Was thinking... Should I keep the extra mdio node and move the
documentation to qca8k or should I handle all of that internally?
To me it looks cleaner the direct definition in the devicetree than the 
port_to_phy function in the driver but as you can see extra checks are
required to handle the new implementation and still supports the old
one.
diff mbox series

Patch

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 3d195fdd7ed5..3c3e05735b2d 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1685,7 +1685,37 @@  qca8k_get_tag_protocol(struct dsa_switch *ds, int port,
 	return DSA_TAG_PROTO_QCA;
 }
 
+static u32
+qca8k_get_phys_mii_mask(struct dsa_switch *ds)
+{
+	struct device_node *mdio, *phy;
+	u32 reg, phy_mii_mask = 0;
+	int err;
+
+	mdio = of_get_child_by_name(ds->dev->of_node, "mdio");
+	if (mdio) {
+		for_each_available_child_of_node(mdio, phy) {
+			err = of_property_read_u32(phy, "reg", &reg);
+			if (err) {
+				of_node_put(phy);
+				of_node_put(mdio);
+				return 0;
+			}
+
+			phy_mii_mask |= BIT(reg);
+		}
+
+		of_node_put(mdio);
+		return phy_mii_mask;
+	}
+
+	/* Fallback to the lagacy mapping if mdio node is not found */
+	dev_warn(ds->dev, "Using the legacy phys_mii_mapping. Consider updating the dts.");
+	return dsa_user_ports(ds);
+}
+
 static const struct dsa_switch_ops qca8k_switch_ops = {
+	.get_phys_mii_mask	= qca8k_get_phys_mii_mask,
 	.get_tag_protocol	= qca8k_get_tag_protocol,
 	.setup			= qca8k_setup,
 	.get_strings		= qca8k_get_strings,
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 83a933e563fe..4003ffc659a4 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -511,6 +511,13 @@  struct dsa_switch_ops {
 	void	(*teardown)(struct dsa_switch *ds);
 	u32	(*get_phy_flags)(struct dsa_switch *ds, int port);
 
+	/*
+	 * Provide a custom phys_mii_mask for the dsa slave mdiobus instead
+	 * of relying on the dsa_user_ports. Not every switch has a 1:1 map
+	 * port to PHY, hence the driver can provide their fixed mask.
+	 */
+	u32	(*get_phys_mii_mask)(struct dsa_switch *ds);
+
 	/*
 	 * Access to the switch's PHY registers.
 	 */
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 79adabe3e2a7..7eabd4d67849 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -682,8 +682,13 @@  static int dsa_switch_setup(struct dsa_switch *ds)
 	 * driver and before ops->setup() has run, since the switch drivers and
 	 * the slave MDIO bus driver rely on these values for probing PHY
 	 * devices or not
+	 * Driver can provide his on mask as some switch doesn't have a 1:1 map
+	 * phy to port.
 	 */
-	ds->phys_mii_mask |= dsa_user_ports(ds);
+	if (ds->ops->get_phys_mii_mask)
+		ds->phys_mii_mask = ds->ops->get_phys_mii_mask(ds);
+	else
+		ds->phys_mii_mask |= dsa_user_ports(ds);
 
 	/* Add the switch to devlink before calling setup, so that setup can
 	 * add dpipe tables