[PATCH net-next 0/9] net: dsa: define port types

[PATCH net-next 0/9] net: dsa: define port types

[Vivien Didelot]

The DSA code currently has 3 bitmaps in the dsa_switch structure:
cpu_port_mask, dsa_port_mask and enabled_port_mask.

They are used to store the type of each switch port. This dates back
from when DSA didn't have a dsa_port structure to hold port-specific
data.

The dsa_switch structure is mainly used to communicate with DSA drivers
and must not contain such static data parsed from DTS or pdata, which
belongs the DSA core structures, such as dsa_switch_tree and dsa_port.

Also the enabled_port_mask is misleading, often misinterpreted as the
complement of disabled ports (thus including DSA and CPU ports), while
in fact it only masks the user ports.

A port can be of 3 types when it is not unused: "cpu" (interfacing with
a master device), "dsa" (interconnecting with another "dsa" port from
another switch chip), or "user" (user-facing port.)

This patchset first fixes the usage of DSA port type helpers, then
defines the DSA_PORT_TYPE_UNUSED, DSA_PORT_TYPE_CPU, DSA_PORT_TYPE_DSA,
and DSA_PORT_TYPE_USER port types, and finally removes the misleading
port bitmaps.

Vivien Didelot (9):
  net: dsa: add dsa_is_unused_port helper
  net: dsa: mv88e6xxx: skip unused ports
  net: dsa: fix dsa_is_normal_port helper
  net: dsa: rename dsa_is_normal_port helper
  net: dsa: use dsa_is_user_port everywhere
  net: dsa: introduce dsa_user_ports helper
  net: dsa: define port types
  net: dsa: use new port type in helpers
  net: dsa: remove port masks

 drivers/net/dsa/b53/b53_common.c |  2 +-
 drivers/net/dsa/bcm_sf2.c        |  9 ++++-----
 drivers/net/dsa/bcm_sf2_cfp.c    |  2 +-
 drivers/net/dsa/mt7530.c         |  6 +++---
 drivers/net/dsa/mv88e6060.c      |  5 ++---
 drivers/net/dsa/mv88e6xxx/chip.c |  5 ++++-
 drivers/net/dsa/qca8k.c          |  7 +++----
 include/net/dsa.h                | 39 ++++++++++++++++++++++++++++++---------
 net/dsa/dsa.c                    |  2 +-
 net/dsa/dsa2.c                   | 16 ++++------------
 net/dsa/legacy.c                 | 15 +++++++++------
 11 files changed, 62 insertions(+), 46 deletions(-)

-- 
2.14.3

[PATCH net-next 1/9] net: dsa: add dsa_is_unused_port helper

[Vivien Didelot]

As the comment above the chunk states, the b53 driver attempts to
disable the unused ports. But using ds->enabled_port_mask is misleading,
because this mask reports in fact the user ports.

To avoid confusion and fix this, this patch introduces an explicit
dsa_is_unused_port helper which ensures the corresponding bit is not
masked in any of the switch port masks.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/b53/b53_common.c | 2 +-
 include/net/dsa.h                | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index b48cf0487b43..c74a50112551 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -873,7 +873,7 @@ static int b53_setup(struct dsa_switch *ds)
 	for (port = 0; port < dev->num_ports; port++) {
 		if (dsa_is_cpu_port(ds, port))
 			b53_enable_cpu_port(dev, port);
-		else if (!(BIT(port) & ds->enabled_port_mask))
+		else if (dsa_is_unused_port(ds, port))
 			b53_disable_port(ds, port, NULL);
 	}
 
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 38961ef91d3d..6b1bc1c8f7e2 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -254,6 +254,13 @@ struct dsa_switch {
 	struct dsa_port ports[];
 };
 
+static inline bool dsa_is_unused_port(struct dsa_switch *ds, int p)
+{
+	u32 m = ds->enabled_port_mask | ds->dsa_port_mask | ds->cpu_port_mask;
+
+	return !(m & BIT(p));
+}
+
 static inline bool dsa_is_cpu_port(struct dsa_switch *ds, int p)
 {
 	return !!(ds->cpu_port_mask & (1 << p));
-- 
2.14.3

[PATCH net-next 2/9] net: dsa: mv88e6xxx: skip unused ports

[Vivien Didelot]

The unused ports are currently configured in normal mode. This does not
prevent the switch from being functional, but it is unnecessary. Skip
unused ports.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 677d6902807e..2d8cf66e8f74 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2005,6 +2005,9 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 
 	/* Setup Switch Port Registers */
 	for (i = 0; i < mv88e6xxx_num_ports(chip); i++) {
+		if (dsa_is_unused_port(ds, i))
+			continue;
+
 		err = mv88e6xxx_setup_port(chip, i);
 		if (err)
 			goto unlock;
-- 
2.14.3

[PATCH net-next 3/9] net: dsa: fix dsa_is_normal_port helper

[Vivien Didelot]

In order to know if a port is of type user, dsa_is_normal_port checks
that the given port is not of type DSA nor CPU. This is not enough
because a port can be unused.

Without the previous fix, this caused the unused mv88e6xxx ports to be
configured in normal mode.

The ds->enabled_port_mask reports the user ports, so check this instead.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 include/net/dsa.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 6b1bc1c8f7e2..4ad432ad2d40 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -273,7 +273,7 @@ static inline bool dsa_is_dsa_port(struct dsa_switch *ds, int p)
 
 static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
 {
-	return !dsa_is_cpu_port(ds, p) && !dsa_is_dsa_port(ds, p);
+	return !!(ds->enabled_port_mask & BIT(p));
 }
 
 static inline const struct dsa_port *dsa_to_port(struct dsa_switch *ds, int p)
-- 
2.14.3

[PATCH net-next 4/9] net: dsa: rename dsa_is_normal_port helper

[Vivien Didelot]

This patch renames dsa_is_normal_port to dsa_is_user_port because "user"
is the correct term in the DSA terminology, not "normal".

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
 include/net/dsa.h                | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 2d8cf66e8f74..09a66d4d9492 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1676,7 +1676,7 @@ static int mv88e6xxx_setup_port_mode(struct mv88e6xxx_chip *chip, int port)
 	if (dsa_is_dsa_port(chip->ds, port))
 		return mv88e6xxx_set_port_mode_dsa(chip, port);
 
-	if (dsa_is_normal_port(chip->ds, port))
+	if (dsa_is_user_port(chip->ds, port))
 		return mv88e6xxx_set_port_mode_normal(chip, port);
 
 	/* Setup CPU port mode depending on its supported tag format */
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 4ad432ad2d40..49701d958663 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -271,7 +271,7 @@ static inline bool dsa_is_dsa_port(struct dsa_switch *ds, int p)
 	return !!((ds->dsa_port_mask) & (1 << p));
 }
 
-static inline bool dsa_is_normal_port(struct dsa_switch *ds, int p)
+static inline bool dsa_is_user_port(struct dsa_switch *ds, int p)
 {
 	return !!(ds->enabled_port_mask & BIT(p));
 }
-- 
2.14.3

[PATCH net-next 5/9] net: dsa: use dsa_is_user_port everywhere

[Vivien Didelot]

Most of the DSA code still check ds->enabled_port_mask directly to
inspect a given port type instead of using the provided dsa_is_user_port
helper. Change this.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/bcm_sf2.c     | 9 ++++-----
 drivers/net/dsa/bcm_sf2_cfp.c | 2 +-
 drivers/net/dsa/mt7530.c      | 4 ++--
 drivers/net/dsa/qca8k.c       | 4 ++--
 net/dsa/dsa.c                 | 2 +-
 net/dsa/legacy.c              | 4 ++--
 6 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 2574a52ee161..7f47400e557e 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -652,8 +652,7 @@ static int bcm_sf2_sw_suspend(struct dsa_switch *ds)
 	 * bcm_sf2_sw_setup
 	 */
 	for (port = 0; port < DSA_MAX_PORTS; port++) {
-		if ((1 << port) & ds->enabled_port_mask ||
-		    dsa_is_cpu_port(ds, port))
+		if (dsa_is_user_port(ds, port) || dsa_is_cpu_port(ds, port))
 			bcm_sf2_port_disable(ds, port, NULL);
 	}
 
@@ -676,7 +675,7 @@ static int bcm_sf2_sw_resume(struct dsa_switch *ds)
 		bcm_sf2_gphy_enable_set(ds, true);
 
 	for (port = 0; port < DSA_MAX_PORTS; port++) {
-		if ((1 << port) & ds->enabled_port_mask)
+		if (dsa_is_user_port(ds, port))
 			bcm_sf2_port_setup(ds, port, NULL);
 		else if (dsa_is_cpu_port(ds, port))
 			bcm_sf2_imp_setup(ds, port);
@@ -771,7 +770,7 @@ static void bcm_sf2_sw_configure_vlan(struct dsa_switch *ds)
 	bcm_sf2_vlan_op(priv, ARLA_VTBL_CMD_CLEAR);
 
 	for (port = 0; port < priv->hw_params.num_ports; port++) {
-		if (!((1 << port) & ds->enabled_port_mask))
+		if (!dsa_is_user_port(ds, port))
 			continue;
 
 		core_writel(priv, 1, CORE_DEFAULT_1Q_TAG_P(port));
@@ -786,7 +785,7 @@ static int bcm_sf2_sw_setup(struct dsa_switch *ds)
 	/* Enable all valid ports and disable those unused */
 	for (port = 0; port < priv->hw_params.num_ports; port++) {
 		/* IMP port receives special treatment */
-		if ((1 << port) & ds->enabled_port_mask)
+		if (dsa_is_user_port(ds, port))
 			bcm_sf2_port_setup(ds, port, NULL);
 		else if (dsa_is_cpu_port(ds, port))
 			bcm_sf2_imp_setup(ds, port);
diff --git a/drivers/net/dsa/bcm_sf2_cfp.c b/drivers/net/dsa/bcm_sf2_cfp.c
index 4feb507eeee0..b721a2009b50 100644
--- a/drivers/net/dsa/bcm_sf2_cfp.c
+++ b/drivers/net/dsa/bcm_sf2_cfp.c
@@ -750,7 +750,7 @@ static int bcm_sf2_cfp_rule_set(struct dsa_switch *ds, int port,
 	port_num = fs->ring_cookie / SF2_NUM_EGRESS_QUEUES;
 
 	if (fs->ring_cookie == RX_CLS_FLOW_DISC ||
-	    !(BIT(port_num) & ds->enabled_port_mask) ||
+	    !dsa_is_user_port(ds, port_num) ||
 	    port_num >= priv->hw_params.num_ports)
 		return -EINVAL;
 	/*
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 21431be2831e..d1d4eea6a875 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -781,7 +781,7 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
 		 * same bridge. If the port is disabled, port matrix is kept
 		 * and not being setup until the port becomes enabled.
 		 */
-		if (ds->enabled_port_mask & BIT(i) && i != port) {
+		if (dsa_is_user_port(ds, i) && i != port) {
 			if (dsa_to_port(ds, i)->bridge_dev != bridge)
 				continue;
 			if (priv->ports[i].enable)
@@ -818,7 +818,7 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
 		 * in the same bridge. If the port is disabled, port matrix
 		 * is kept and not being setup until the port becomes enabled.
 		 */
-		if (ds->enabled_port_mask & BIT(i) && i != port) {
+		if (dsa_is_user_port(ds, i) && i != port) {
 			if (dsa_to_port(ds, i)->bridge_dev != bridge)
 				continue;
 			if (priv->ports[i].enable)
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index d1b0b1fb632f..37125858fe80 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -536,7 +536,7 @@ qca8k_setup(struct dsa_switch *ds)
 
 	/* Disable MAC by default on all user ports */
 	for (i = 1; i < QCA8K_NUM_PORTS; i++)
-		if (ds->enabled_port_mask & BIT(i))
+		if (dsa_is_user_port(ds, i))
 			qca8k_port_set_status(priv, i, 0);
 
 	/* Forward all unknown frames to CPU port for Linux processing */
@@ -556,7 +556,7 @@ qca8k_setup(struct dsa_switch *ds)
 		}
 
 		/* Invividual user ports get connected to CPU port only */
-		if (ds->enabled_port_mask & BIT(i)) {
+		if (dsa_is_user_port(ds, i)) {
 			int shift = 16 * (i % 2);
 
 			qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(i),
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index a3abf7a7b9a2..fe0081730305 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -201,7 +201,7 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
 #ifdef CONFIG_PM_SLEEP
 static bool dsa_is_port_initialized(struct dsa_switch *ds, int p)
 {
-	return ds->enabled_port_mask & (1 << p) && ds->ports[p].slave;
+	return dsa_is_user_port(ds, p) && ds->ports[p].slave;
 }
 
 int dsa_switch_suspend(struct dsa_switch *ds)
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index 93c1c43bcc58..0b79c6171d0d 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -190,7 +190,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds,
 		ds->ports[i].dn = cd->port_dn[i];
 		ds->ports[i].cpu_dp = dst->cpu_dp;
 
-		if (!(ds->enabled_port_mask & (1 << i)))
+		if (dsa_is_user_port(ds, i))
 			continue;
 
 		ret = dsa_slave_create(&ds->ports[i], cd->port_names[i]);
@@ -258,7 +258,7 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
 
 	/* Destroy network devices for physical switch ports. */
 	for (port = 0; port < ds->num_ports; port++) {
-		if (!(ds->enabled_port_mask & (1 << port)))
+		if (!dsa_is_user_port(ds, port))
 			continue;
 
 		if (!ds->ports[port].slave)
-- 
2.14.3

[PATCH net-next 6/9] net: dsa: introduce dsa_user_ports helper

[Vivien Didelot]

Introduce a dsa_user_ports() helper to return the ds->enabled_port_mask
mask which is more explicit. This will also minimize diffs when touching
this internal mask.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mt7530.c    | 2 +-
 drivers/net/dsa/mv88e6060.c | 5 ++---
 drivers/net/dsa/qca8k.c     | 3 +--
 include/net/dsa.h           | 5 +++++
 net/dsa/dsa2.c              | 2 +-
 net/dsa/legacy.c            | 2 +-
 6 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index d1d4eea6a875..627c039f12ca 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -688,7 +688,7 @@ mt7530_cpu_port_enable(struct mt7530_priv *priv,
 	 * the switch
 	 */
 	mt7530_write(priv, MT7530_PCR_P(port),
-		     PCR_MATRIX(priv->ds->enabled_port_mask));
+		     PCR_MATRIX(dsa_user_ports(priv->ds)));
 
 	return 0;
 }
diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
index f78b9e13be1c..45768e3c5bc5 100644
--- a/drivers/net/dsa/mv88e6060.c
+++ b/drivers/net/dsa/mv88e6060.c
@@ -175,9 +175,8 @@ static int mv88e6060_setup_port(struct dsa_switch *ds, int p)
 	 */
 	REG_WRITE(addr, PORT_VLAN_MAP,
 		  ((p & 0xf) << PORT_VLAN_MAP_DBNUM_SHIFT) |
-		   (dsa_is_cpu_port(ds, p) ?
-			ds->enabled_port_mask :
-			BIT(dsa_to_port(ds, p)->cpu_dp->index)));
+		   (dsa_is_cpu_port(ds, p) ? dsa_user_ports(ds) :
+		    BIT(dsa_to_port(ds, p)->cpu_dp->index)));
 
 	/* Port Association Vector: when learning source addresses
 	 * of packets, add the address to the address database using
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 37125858fe80..cf72e274275f 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -551,8 +551,7 @@ qca8k_setup(struct dsa_switch *ds)
 		/* CPU port gets connected to all user ports of the switch */
 		if (dsa_is_cpu_port(ds, i)) {
 			qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(QCA8K_CPU_PORT),
-				  QCA8K_PORT_LOOKUP_MEMBER,
-				  ds->enabled_port_mask);
+				  QCA8K_PORT_LOOKUP_MEMBER, dsa_user_ports(ds));
 		}
 
 		/* Invividual user ports get connected to CPU port only */
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 49701d958663..dc7728062396 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -276,6 +276,11 @@ static inline bool dsa_is_user_port(struct dsa_switch *ds, int p)
 	return !!(ds->enabled_port_mask & BIT(p));
 }
 
+static inline u32 dsa_user_ports(struct dsa_switch *ds)
+{
+	return ds->enabled_port_mask;
+}
+
 static inline const struct dsa_port *dsa_to_port(struct dsa_switch *ds, int p)
 {
 	return &ds->ports[p];
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 62485a57dbfc..d43c59c91058 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -312,7 +312,7 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 	 * the slave MDIO bus driver rely on these values for probing PHY
 	 * devices or not
 	 */
-	ds->phys_mii_mask = ds->enabled_port_mask;
+	ds->phys_mii_mask |= dsa_user_ports(ds);
 
 	/* Add the switch to devlink before calling setup, so that setup can
 	 * add dpipe tables
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index 0b79c6171d0d..fa543c4a6061 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -136,7 +136,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds,
 	/* Make the built-in MII bus mask match the number of ports,
 	 * switch drivers can override this later
 	 */
-	ds->phys_mii_mask = ds->enabled_port_mask;
+	ds->phys_mii_mask |= dsa_user_ports(ds);
 
 	/*
 	 * If the CPU connects to this switch, set the switch tree
-- 
2.14.3

[PATCH net-next 7/9] net: dsa: define port types

[Vivien Didelot]

Introduce an enumerated type for ports, which will be way more explicit
to identify a port type instead of digging into switch port masks.

A port can be of type CPU, DSA, user, or unused by default. This is a
static parsed information that cannot be changed at runtime.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 include/net/dsa.h | 7 +++++++
 net/dsa/dsa2.c    | 3 +++
 net/dsa/legacy.c  | 6 ++++++
 3 files changed, 16 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index dc7728062396..8da20c4a6552 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -180,6 +180,13 @@ struct dsa_port {
 	struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev,
 			       struct packet_type *pt);
 
+	enum {
+		DSA_PORT_TYPE_UNUSED = 0,
+		DSA_PORT_TYPE_CPU,
+		DSA_PORT_TYPE_DSA,
+		DSA_PORT_TYPE_USER,
+	} type;
+
 	struct dsa_switch	*ds;
 	unsigned int		index;
 	const char		*name;
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index d43c59c91058..dd6f35b92937 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -185,6 +185,7 @@ static int dsa_ds_complete(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 			return err;
 
 		ds->dsa_port_mask |= BIT(index);
+		port->type = DSA_PORT_TYPE_DSA;
 	}
 
 	return 0;
@@ -504,6 +505,7 @@ static int dsa_cpu_parse(struct dsa_port *port, u32 index,
 	 * net/dsa/dsa.c::dsa_switch_setup_one does.
 	 */
 	ds->cpu_port_mask |= BIT(index);
+	port->type = DSA_PORT_TYPE_CPU;
 
 	tag_protocol = ds->ops->get_tag_protocol(ds);
 	tag_ops = dsa_resolve_tag_protocol(tag_protocol);
@@ -543,6 +545,7 @@ static int dsa_ds_parse(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 			 * net/dsa/dsa.c::dsa_switch_setup_one does.
 			 */
 			ds->enabled_port_mask |= BIT(index);
+			port->type = DSA_PORT_TYPE_USER;
 		}
 
 	}
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index fa543c4a6061..9fd5b3adab1e 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -101,6 +101,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds,
 	struct dsa_chip_data *cd = ds->cd;
 	bool valid_name_found = false;
 	int index = ds->index;
+	struct dsa_port *dp;
 	int i, ret;
 
 	/*
@@ -109,6 +110,8 @@ static int dsa_switch_setup_one(struct dsa_switch *ds,
 	for (i = 0; i < ds->num_ports; i++) {
 		char *name;
 
+		dp = &ds->ports[i];
+
 		name = cd->port_names[i];
 		if (name == NULL)
 			continue;
@@ -122,10 +125,13 @@ static int dsa_switch_setup_one(struct dsa_switch *ds,
 			dst->cpu_dp = &ds->ports[i];
 			dst->cpu_dp->master = master;
 			ds->cpu_port_mask |= 1 << i;
+			dp->type = DSA_PORT_TYPE_CPU;
 		} else if (!strcmp(name, "dsa")) {
 			ds->dsa_port_mask |= 1 << i;
+			dp->type = DSA_PORT_TYPE_DSA;
 		} else {
 			ds->enabled_port_mask |= 1 << i;
+			dp->type = DSA_PORT_TYPE_USER;
 		}
 		valid_name_found = true;
 	}
-- 
2.14.3

[PATCH net-next 8/9] net: dsa: use new port type in helpers

[Vivien Didelot]

Now that DSA exposes an enumerated type for the ports, we can use them
directly instead of checking bitmaps, which is more consistent.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 include/net/dsa.h | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 8da20c4a6552..07dfbd7f4fd5 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -261,36 +261,41 @@ struct dsa_switch {
 	struct dsa_port ports[];
 };
 
+static inline const struct dsa_port *dsa_to_port(struct dsa_switch *ds, int p)
+{
+	return &ds->ports[p];
+}
+
 static inline bool dsa_is_unused_port(struct dsa_switch *ds, int p)
 {
-	u32 m = ds->enabled_port_mask | ds->dsa_port_mask | ds->cpu_port_mask;
-
-	return !(m & BIT(p));
+	return dsa_to_port(ds, p)->type == DSA_PORT_TYPE_UNUSED;
 }
 
 static inline bool dsa_is_cpu_port(struct dsa_switch *ds, int p)
 {
-	return !!(ds->cpu_port_mask & (1 << p));
+	return dsa_to_port(ds, p)->type == DSA_PORT_TYPE_CPU;
 }
 
 static inline bool dsa_is_dsa_port(struct dsa_switch *ds, int p)
 {
-	return !!((ds->dsa_port_mask) & (1 << p));
+	return dsa_to_port(ds, p)->type == DSA_PORT_TYPE_DSA;
 }
 
 static inline bool dsa_is_user_port(struct dsa_switch *ds, int p)
 {
-	return !!(ds->enabled_port_mask & BIT(p));
+	return dsa_to_port(ds, p)->type == DSA_PORT_TYPE_USER;
 }
 
 static inline u32 dsa_user_ports(struct dsa_switch *ds)
 {
-	return ds->enabled_port_mask;
-}
+	u32 mask = 0;
+	int p;
 
-static inline const struct dsa_port *dsa_to_port(struct dsa_switch *ds, int p)
-{
-	return &ds->ports[p];
+	for (p = 0; p < ds->num_ports; p++)
+		if (dsa_is_user_port(ds, p))
+			mask |= BIT(p);
+
+	return mask;
 }
 
 static inline u8 dsa_upstream_port(struct dsa_switch *ds)
-- 
2.14.3

[PATCH net-next 9/9] net: dsa: remove port masks

[Vivien Didelot]

Now that DSA core provides port types, there is no need to keep this
information at the switch level. This is a static information that is
part of a DSA core dsa_port structure. Remove them.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 include/net/dsa.h |  3 ---
 net/dsa/dsa2.c    | 11 -----------
 net/dsa/legacy.c  |  3 ---
 3 files changed, 17 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 07dfbd7f4fd5..50e276dc4c01 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -240,9 +240,6 @@ struct dsa_switch {
 	/*
 	 * Slave mii_bus and devices for the individual ports.
 	 */
-	u32			dsa_port_mask;
-	u32			cpu_port_mask;
-	u32			enabled_port_mask;
 	u32			phys_mii_mask;
 	struct mii_bus		*slave_mii_bus;
 
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index dd6f35b92937..ec58654a71cd 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -184,7 +184,6 @@ static int dsa_ds_complete(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 		if (err != 0)
 			return err;
 
-		ds->dsa_port_mask |= BIT(index);
 		port->type = DSA_PORT_TYPE_DSA;
 	}
 
@@ -500,11 +499,6 @@ static int dsa_cpu_parse(struct dsa_port *port, u32 index,
 		dst->cpu_dp->master = ethernet_dev;
 	}
 
-	/* Initialize cpu_port_mask now for drv->setup()
-	 * to have access to a correct value, just like what
-	 * net/dsa/dsa.c::dsa_switch_setup_one does.
-	 */
-	ds->cpu_port_mask |= BIT(index);
 	port->type = DSA_PORT_TYPE_CPU;
 
 	tag_protocol = ds->ops->get_tag_protocol(ds);
@@ -540,11 +534,6 @@ static int dsa_ds_parse(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 			if (err)
 				return err;
 		} else {
-			/* Initialize enabled_port_mask now for drv->setup()
-			 * to have access to a correct value, just like what
-			 * net/dsa/dsa.c::dsa_switch_setup_one does.
-			 */
-			ds->enabled_port_mask |= BIT(index);
 			port->type = DSA_PORT_TYPE_USER;
 		}
 
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index 9fd5b3adab1e..93e1b116ef83 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -124,13 +124,10 @@ static int dsa_switch_setup_one(struct dsa_switch *ds,
 			}
 			dst->cpu_dp = &ds->ports[i];
 			dst->cpu_dp->master = master;
-			ds->cpu_port_mask |= 1 << i;
 			dp->type = DSA_PORT_TYPE_CPU;
 		} else if (!strcmp(name, "dsa")) {
-			ds->dsa_port_mask |= 1 << i;
 			dp->type = DSA_PORT_TYPE_DSA;
 		} else {
-			ds->enabled_port_mask |= 1 << i;
 			dp->type = DSA_PORT_TYPE_USER;
 		}
 		valid_name_found = true;
-- 
2.14.3

Re: [PATCH net-next 7/9] net: dsa: define port types

[Jiri Pirko]

Thu, Oct 26, 2017 at 05:22:57PM CEST, vivien.didelot@savoirfairelinux.com wrote:
>Introduce an enumerated type for ports, which will be way more explicit
>to identify a port type instead of digging into switch port masks.
>
>A port can be of type CPU, DSA, user, or unused by default. This is a
>static parsed information that cannot be changed at runtime.
>
>Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>---
> include/net/dsa.h | 7 +++++++
> net/dsa/dsa2.c    | 3 +++
> net/dsa/legacy.c  | 6 ++++++
> 3 files changed, 16 insertions(+)
>
>diff --git a/include/net/dsa.h b/include/net/dsa.h
>index dc7728062396..8da20c4a6552 100644
>--- a/include/net/dsa.h
>+++ b/include/net/dsa.h
>@@ -180,6 +180,13 @@ struct dsa_port {
> 	struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev,
> 			       struct packet_type *pt);
> 
>+	enum {
>+		DSA_PORT_TYPE_UNUSED = 0,
>+		DSA_PORT_TYPE_CPU,
>+		DSA_PORT_TYPE_DSA,
>+		DSA_PORT_TYPE_USER,

Do you plan to expose this to userspace? How?

Re: [PATCH net-next 7/9] net: dsa: define port types

[Vivien Didelot]

Hi Jiri,

Jiri Pirko <jiri@resnulli.us> writes:

>>+	enum {
>>+		DSA_PORT_TYPE_UNUSED = 0,
>>+		DSA_PORT_TYPE_CPU,
>>+		DSA_PORT_TYPE_DSA,
>>+		DSA_PORT_TYPE_USER,
>
> Do you plan to expose this to userspace? How?

No. This is a DSA internal data for the moment.


Thanks,

        Vivien

Re: [PATCH net-next 1/9] net: dsa: add dsa_is_unused_port helper

[Florian Fainelli]

On 10/26/2017 08:22 AM, Vivien Didelot wrote:
> As the comment above the chunk states, the b53 driver attempts to
> disable the unused ports. But using ds->enabled_port_mask is misleading,
> because this mask reports in fact the user ports.
> 
> To avoid confusion and fix this, this patch introduces an explicit
> dsa_is_unused_port helper which ensures the corresponding bit is not
> masked in any of the switch port masks.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH net-next 2/9] net: dsa: mv88e6xxx: skip unused ports

[Florian Fainelli]

On 10/26/2017 08:22 AM, Vivien Didelot wrote:
> The unused ports are currently configured in normal mode. This does not
> prevent the switch from being functional, but it is unnecessary. Skip
> unused ports.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH net-next 3/9] net: dsa: fix dsa_is_normal_port helper

[Florian Fainelli]

On 10/26/2017 08:22 AM, Vivien Didelot wrote:
> In order to know if a port is of type user, dsa_is_normal_port checks
> that the given port is not of type DSA nor CPU. This is not enough
> because a port can be unused.
> 
> Without the previous fix, this caused the unused mv88e6xxx ports to be
> configured in normal mode.
> 
> The ds->enabled_port_mask reports the user ports, so check this instead.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH net-next 4/9] net: dsa: rename dsa_is_normal_port helper

[Florian Fainelli]

On 10/26/2017 08:22 AM, Vivien Didelot wrote:
> This patch renames dsa_is_normal_port to dsa_is_user_port because "user"
> is the correct term in the DSA terminology, not "normal".
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH net-next 5/9] net: dsa: use dsa_is_user_port everywhere

[Florian Fainelli]

On 10/26/2017 08:22 AM, Vivien Didelot wrote:
> Most of the DSA code still check ds->enabled_port_mask directly to
> inspect a given port type instead of using the provided dsa_is_user_port
> helper. Change this.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH net-next 0/9] net: dsa: define port types

[David Miller]

From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Thu, 26 Oct 2017 11:22:50 -0400

> The DSA code currently has 3 bitmaps in the dsa_switch structure:
> cpu_port_mask, dsa_port_mask and enabled_port_mask.
> 
> They are used to store the type of each switch port. This dates back
> from when DSA didn't have a dsa_port structure to hold port-specific
> data.
> 
> The dsa_switch structure is mainly used to communicate with DSA drivers
> and must not contain such static data parsed from DTS or pdata, which
> belongs the DSA core structures, such as dsa_switch_tree and dsa_port.
> 
> Also the enabled_port_mask is misleading, often misinterpreted as the
> complement of disabled ports (thus including DSA and CPU ports), while
> in fact it only masks the user ports.
> 
> A port can be of 3 types when it is not unused: "cpu" (interfacing with
> a master device), "dsa" (interconnecting with another "dsa" port from
> another switch chip), or "user" (user-facing port.)
> 
> This patchset first fixes the usage of DSA port type helpers, then
> defines the DSA_PORT_TYPE_UNUSED, DSA_PORT_TYPE_CPU, DSA_PORT_TYPE_DSA,
> and DSA_PORT_TYPE_USER port types, and finally removes the misleading
> port bitmaps.

Series applied, thanks Vivien.