[PATCH net-next 0/9] net: dsa: misc improvements

[PATCH net-next 0/9] net: dsa: misc improvements

[Florian Fainelli]

Hi all,

This patch series builds on top of Andrew's "New DSA bind, switches as devices"
patch set and does the following:

- add support for the old DSA binding with the new dsa_register_switch() API
  which is needed by some platforms where the Device Tree is pretty much frozen

- add a few helper functions/goodies for net/dsa/dsa2.c to be as close as possible
  from net/dsa/dsa.c in terms of what drivers can expect, in particular the slave
  MDIO bus and the enabled_port_mask and phy_mii_mask

- fix the CPU port ethtools ops to work in a multiple tree setup since we can
  no longer assume a single tree is supported

- finally conver the bcm_sf2 driver to be a true platform device driver and
  slightly rework its internal vs. external MDIO bus indirect read/writes to
  be cleaner

Florian Fainelli (9):
  net: dsa: Prepare to support legacy DT binding
  net: dsa: Add support for parsing the old binding
  net: dsa: Provide unique DSA slave MII bus names
  net: dsa: Initialize ds->enabled_port_mask and ds->phys_mii_mask
  net: dsa: Export suspend/resume functions
  net: dsa: Add initialization helper for CPU port ethtool_ops
  net: dsa: Initialize CPU port ethtool ops per tree
  net: dsa: bcm_sf2: Make it a real platform device driver
  net: dsa: bcm_sf2: Register our slave MDIO bus

 drivers/net/dsa/bcm_sf2.c | 390 +++++++++++++++++++++++++++++-----------------
 drivers/net/dsa/bcm_sf2.h |   6 +
 include/net/dsa.h         |  14 ++
 net/dsa/dsa.c             |  35 ++++-
 net/dsa/dsa2.c            | 199 ++++++++++++++++++++---
 net/dsa/dsa_priv.h        |   3 +
 net/dsa/slave.c           |  25 ++-
 7 files changed, 491 insertions(+), 181 deletions(-)

-- 
2.7.4

[PATCH net-next 1/9] net: dsa: Prepare to support legacy DT binding

[Florian Fainelli]

In preparation for supporting the legacy DT binding, call
dsa_get_ports() early on to allow two different parsing code paths to be
called.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/dsa2.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 4e5051bed643..b5640d8ffbae 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -552,7 +552,7 @@ static struct device_node *dsa_get_ports(struct dsa_switch *ds,
 	return ports;
 }
 
-static int _dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
+static int __dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
 {
 	struct device_node *ports = dsa_get_ports(ds, np);
 	struct dsa_switch_tree *dst;
@@ -622,6 +622,16 @@ out:
 	return err;
 }
 
+static int _dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
+{
+	struct device_node *ports = dsa_get_ports(ds, np);
+
+	if (IS_ERR(ports))
+		return PTR_ERR(ports);
+
+	return __dsa_register_switch(ds, np);
+}
+
 int dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
 {
 	int err;
-- 
2.7.4

[PATCH net-next] net: dsa: Provide CPU port statistics to master netdev

[Florian Fainelli]

This patch overloads the DSA master netdev, aka CPU Ethernet MAC to also
include switch-side statistics, which is useful for debugging purposes,
when the switch is not properly connected to the Ethernet MAC (duplex
mismatch, (RG)MII electrical issues etc.).

We accomplish this by retaining the original copy of the master netdev's
ethtool_ops, and just overload the 3 operations we care about:
get_sset_count, get_strings and get_ethtool_stats so as to intercept
these calls and call into the original master_netdev ethtool_ops, plus
our own.

We take this approach as opposed to providing a set of DSA helper
functions that would retrive the CPU port's statistics, because the
entire purpose of DSA is to allow unmodified Ethernet MAC drivers to be
used as CPU conduit interfaces, therefore, statistics overlay in such
drivers would simply not scale.

The new ethtool -S <iface> output would therefore look like this now:
<iface> statistics
p<2 digits cpu port number>_<switch MIB counter names>

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes from RFC:

- prepend the CPU port as a prefix to make it clear what the stats are
  about, master netdev interface stats are unchanged

 include/net/dsa.h |  5 ++++
 net/dsa/slave.c   | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 2d280aba97e2..8e86af87c84f 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -111,6 +111,11 @@ struct dsa_switch_tree {
 	enum dsa_tag_protocol	tag_protocol;
 
 	/*
+	 * Original copy of the master netdev ethtool_ops
+	 */
+	struct ethtool_ops	master_ethtool_ops;
+
+	/*
 	 * The switch and port to which the CPU is attached.
 	 */
 	s8			cpu_switch;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 3b6750f5e68b..5ea8a40c8d33 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -666,6 +666,78 @@ static void dsa_slave_get_strings(struct net_device *dev,
 	}
 }
 
+static void dsa_cpu_port_get_ethtool_stats(struct net_device *dev,
+					   struct ethtool_stats *stats,
+					   uint64_t *data)
+{
+	struct dsa_switch_tree *dst = dev->dsa_ptr;
+	struct dsa_switch *ds = dst->ds[0];
+	s8 cpu_port = dst->cpu_port;
+	int count = 0;
+
+	if (dst->master_ethtool_ops.get_sset_count) {
+		count = dst->master_ethtool_ops.get_sset_count(dev,
+							       ETH_SS_STATS);
+		dst->master_ethtool_ops.get_ethtool_stats(dev, stats, data);
+	}
+
+	if (ds->drv->get_ethtool_stats)
+		ds->drv->get_ethtool_stats(ds, cpu_port, data + count);
+}
+
+static int dsa_cpu_port_get_sset_count(struct net_device *dev, int sset)
+{
+	struct dsa_switch_tree *dst = dev->dsa_ptr;
+	struct dsa_switch *ds = dst->ds[0];
+	int count = 0;
+
+	if (dst->master_ethtool_ops.get_sset_count)
+		count += dst->master_ethtool_ops.get_sset_count(dev, sset);
+
+	if (sset == ETH_SS_STATS && ds->drv->get_sset_count)
+		count += ds->drv->get_sset_count(ds);
+
+	return count;
+}
+
+static void dsa_cpu_port_get_strings(struct net_device *dev,
+				     uint32_t stringset, uint8_t *data)
+{
+	struct dsa_switch_tree *dst = dev->dsa_ptr;
+	struct dsa_switch *ds = dst->ds[0];
+	s8 cpu_port = dst->cpu_port;
+	int len = ETH_GSTRING_LEN;
+	int mcount = 0, count;
+	unsigned int i;
+	uint8_t pfx[4];
+	uint8_t *ndata;
+
+	snprintf(pfx, sizeof(pfx), "p%.2d", cpu_port);
+	/* We do not want to be NULL-terminated, since this is a prefix */
+	pfx[sizeof(pfx) - 1] = '_';
+
+	if (dst->master_ethtool_ops.get_sset_count) {
+		mcount = dst->master_ethtool_ops.get_sset_count(dev,
+								ETH_SS_STATS);
+		dst->master_ethtool_ops.get_strings(dev, stringset, data);
+	}
+
+	if (stringset == ETH_SS_STATS && ds->drv->get_strings) {
+		ndata = data + mcount * len;
+		/* This function copies ETH_GSTRINGS_LEN bytes, we will mangle
+		 * the output after to prepend our CPU port prefix we
+		 * constructed earlier
+		 */
+		ds->drv->get_strings(ds, cpu_port, ndata);
+		count = ds->drv->get_sset_count(ds);
+		for (i = 0; i < count; i++) {
+			memmove(ndata + (i * len + sizeof(pfx)),
+				ndata + i * len, len - sizeof(pfx));
+			memcpy(ndata + i * len, pfx, sizeof(pfx));
+		}
+	}
+}
+
 static void dsa_slave_get_ethtool_stats(struct net_device *dev,
 					struct ethtool_stats *stats,
 					uint64_t *data)
@@ -821,6 +893,8 @@ static const struct ethtool_ops dsa_slave_ethtool_ops = {
 	.get_eee		= dsa_slave_get_eee,
 };
 
+static struct ethtool_ops dsa_cpu_port_ethtool_ops;
+
 static const struct net_device_ops dsa_slave_netdev_ops = {
 	.ndo_open	 	= dsa_slave_open,
 	.ndo_stop		= dsa_slave_close,
@@ -1038,6 +1112,7 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
 		     int port, char *name)
 {
 	struct net_device *master = ds->dst->master_netdev;
+	struct dsa_switch_tree *dst = ds->dst;
 	struct net_device *slave_dev;
 	struct dsa_slave_priv *p;
 	int ret;
@@ -1049,6 +1124,19 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
 
 	slave_dev->features = master->vlan_features;
 	slave_dev->ethtool_ops = &dsa_slave_ethtool_ops;
+	if (master->ethtool_ops != &dsa_cpu_port_ethtool_ops) {
+		memcpy(&dst->master_ethtool_ops, master->ethtool_ops,
+		       sizeof(struct ethtool_ops));
+		memcpy(&dsa_cpu_port_ethtool_ops, &dst->master_ethtool_ops,
+		       sizeof(struct ethtool_ops));
+		dsa_cpu_port_ethtool_ops.get_sset_count =
+					dsa_cpu_port_get_sset_count;
+		dsa_cpu_port_ethtool_ops.get_ethtool_stats =
+					dsa_cpu_port_get_ethtool_stats;
+		dsa_cpu_port_ethtool_ops.get_strings =
+					dsa_cpu_port_get_strings;
+		master->ethtool_ops = &dsa_cpu_port_ethtool_ops;
+	}
 	eth_hw_addr_inherit(slave_dev, master);
 	slave_dev->priv_flags |= IFF_NO_QUEUE;
 	slave_dev->netdev_ops = &dsa_slave_netdev_ops;
-- 
2.1.0

Re: [PATCH net-next] net: dsa: Provide CPU port statistics to master netdev

[Florian Fainelli]

On 06/03/2016 05:05 PM, Florian Fainelli wrote:
> This patch overloads the DSA master netdev, aka CPU Ethernet MAC to also
> include switch-side statistics, which is useful for debugging purposes,
> when the switch is not properly connected to the Ethernet MAC (duplex
> mismatch, (RG)MII electrical issues etc.).
> 
> We accomplish this by retaining the original copy of the master netdev's
> ethtool_ops, and just overload the 3 operations we care about:
> get_sset_count, get_strings and get_ethtool_stats so as to intercept
> these calls and call into the original master_netdev ethtool_ops, plus
> our own.
> 
> We take this approach as opposed to providing a set of DSA helper
> functions that would retrive the CPU port's statistics, because the
> entire purpose of DSA is to allow unmodified Ethernet MAC drivers to be
> used as CPU conduit interfaces, therefore, statistics overlay in such
> drivers would simply not scale.

Disregard this patch, it was left in the patches folder...
-- 
Florian

[PATCH net-next] net: dsa: bcm_sf2: Implement FDB operations

[Florian Fainelli]

Add support for the FDB add, delete, and dump operations. The add and
delete operations are implemented using directed ARL operations using
the specified MAC address and consist in a read operation, write and
readback operation.

The dump operation consists in using the ARL search and software
filtering entries which are not for the desired port.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/bcm_sf2.c      | 236 +++++++++++++++++++++++++++++++++++++++++
 drivers/net/dsa/bcm_sf2.h      |  56 ++++++++++
 drivers/net/dsa/bcm_sf2_regs.h |  43 ++++++++
 3 files changed, 335 insertions(+)

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 9d56515f4c4d..4f32b8a530bf 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -25,6 +25,8 @@
 #include <linux/ethtool.h>
 #include <linux/if_bridge.h>
 #include <linux/brcmphy.h>
+#include <linux/etherdevice.h>
+#include <net/switchdev.h>
 
 #include "bcm_sf2.h"
 #include "bcm_sf2_regs.h"
@@ -555,6 +557,236 @@ static int bcm_sf2_sw_br_set_stp_state(struct dsa_switch *ds, int port,
 	return 0;
 }
 
+/* Address Resolution Logic routines */
+static int bcm_sf2_arl_op_wait(struct bcm_sf2_priv *priv)
+{
+	unsigned int timeout = 10;
+	u32 reg;
+
+	do {
+		reg = core_readl(priv, CORE_ARLA_RWCTL);
+		if (!(reg & ARL_STRTDN))
+			return 0;
+
+		usleep_range(1000, 2000);
+	} while (timeout--);
+
+	return -ETIMEDOUT;
+}
+
+static int bcm_sf2_arl_rw_op(struct bcm_sf2_priv *priv, unsigned int op)
+{
+	u32 cmd;
+
+	if (op > ARL_RW)
+		return -EINVAL;
+
+	cmd = core_readl(priv, CORE_ARLA_RWCTL);
+	cmd &= ~IVL_SVL_SELECT;
+	cmd |= ARL_STRTDN;
+	if (op)
+		cmd |= ARL_RW;
+	else
+		cmd &= ~ARL_RW;
+	core_writel(priv, cmd, CORE_ARLA_RWCTL);
+
+	return bcm_sf2_arl_op_wait(priv);
+}
+
+static int bcm_sf2_arl_read(struct bcm_sf2_priv *priv, u64 mac,
+			    u16 vid, struct bcm_sf2_arl_entry *ent, u8 *idx,
+			    bool is_valid)
+{
+	unsigned int i;
+	int ret;
+
+	ret = bcm_sf2_arl_op_wait(priv);
+	if (ret)
+		return ret;
+
+	/* Read the 4 bins */
+	for (i = 0; i < 4; i++) {
+		u64 mac_vid;
+		u32 fwd_entry;
+
+		mac_vid = core_readq(priv, CORE_ARLA_MACVID_ENTRY(i));
+		fwd_entry = core_readl(priv, CORE_ARLA_FWD_ENTRY(i));
+		bcm_sf2_arl_to_entry(ent, mac_vid, fwd_entry);
+
+		if (ent->is_valid && is_valid) {
+			*idx = i;
+			return 0;
+		}
+
+		/* This is the MAC we just deleted */
+		if (!is_valid && (mac_vid & mac))
+			return 0;
+	}
+
+	return -ENOENT;
+}
+
+static int bcm_sf2_arl_op(struct bcm_sf2_priv *priv, int op, int port,
+			  const unsigned char *addr, u16 vid, bool is_valid)
+{
+	struct bcm_sf2_arl_entry ent;
+	u32 fwd_entry;
+	u64 mac, mac_vid = 0;
+	u8 idx = 0;
+	int ret;
+
+	/* Convert the array into a 64-bit MAC */
+	mac = bcm_sf2_mac_to_u64(addr);
+
+	/* Perform a read for the given MAC and VID */
+	core_writeq(priv, mac, CORE_ARLA_MAC);
+	core_writel(priv, vid, CORE_ARLA_VID);
+
+	/* Issue a read operation for this MAC */
+	ret = bcm_sf2_arl_rw_op(priv, 1);
+	if (ret)
+		return ret;
+
+	ret = bcm_sf2_arl_read(priv, mac, vid, &ent, &idx, is_valid);
+	/* If this is a read, just finish now */
+	if (op)
+		return ret;
+
+	/* We could not find a matching MAC, so reset to a new entry */
+	if (ret) {
+		fwd_entry = 0;
+		idx = 0;
+	}
+
+	memset(&ent, 0, sizeof(ent));
+	ent.port = port;
+	ent.is_valid = is_valid;
+	ent.vid = vid;
+	ent.is_static = true;
+	memcpy(ent.mac, addr, ETH_ALEN);
+	bcm_sf2_arl_from_entry(&mac_vid, &fwd_entry, &ent);
+
+	core_writeq(priv, mac_vid, CORE_ARLA_MACVID_ENTRY(idx));
+	core_writel(priv, fwd_entry, CORE_ARLA_FWD_ENTRY(idx));
+
+	ret = bcm_sf2_arl_rw_op(priv, 0);
+	if (ret)
+		return ret;
+
+	/* Re-read the entry to check */
+	return bcm_sf2_arl_read(priv, mac, vid, &ent, &idx, is_valid);
+}
+
+static int bcm_sf2_sw_fdb_prepare(struct dsa_switch *ds, int port,
+				  const struct switchdev_obj_port_fdb *fdb,
+				  struct switchdev_trans *trans)
+{
+	/* We do not need to do anything specific here yet */
+	return 0;
+}
+
+static int bcm_sf2_sw_fdb_add(struct dsa_switch *ds, int port,
+			      const struct switchdev_obj_port_fdb *fdb,
+			      struct switchdev_trans *trans)
+{
+	struct bcm_sf2_priv *priv = ds_to_priv(ds);
+
+	return bcm_sf2_arl_op(priv, 0, port, fdb->addr, fdb->vid, true);
+}
+
+static int bcm_sf2_sw_fdb_del(struct dsa_switch *ds, int port,
+			      const struct switchdev_obj_port_fdb *fdb)
+{
+	struct bcm_sf2_priv *priv = ds_to_priv(ds);
+
+	return bcm_sf2_arl_op(priv, 0, port, fdb->addr, fdb->vid, false);
+}
+
+static int bcm_sf2_arl_search_wait(struct bcm_sf2_priv *priv)
+{
+	unsigned timeout = 1000;
+	u32 reg;
+
+	do {
+		reg = core_readl(priv, CORE_ARLA_SRCH_CTL);
+		if (!(reg & ARLA_SRCH_STDN))
+			return 0;
+
+		if (reg & ARLA_SRCH_VLID)
+			return 0;
+
+		usleep_range(1000, 2000);
+	} while (timeout--);
+
+	return -ETIMEDOUT;
+}
+
+static void bcm_sf2_arl_search_rd(struct bcm_sf2_priv *priv, u8 idx,
+				  struct bcm_sf2_arl_entry *ent)
+{
+	u64 mac_vid;
+	u32 fwd_entry;
+
+	mac_vid = core_readq(priv, CORE_ARLA_SRCH_RSLT_MACVID(idx));
+	fwd_entry = core_readl(priv, CORE_ARLA_SRCH_RSLT(idx));
+	bcm_sf2_arl_to_entry(ent, mac_vid, fwd_entry);
+}
+
+static int bcm_sf2_sw_fdb_copy(struct net_device *dev, int port,
+			       const struct bcm_sf2_arl_entry *ent,
+			       struct switchdev_obj_port_fdb *fdb,
+			       int (*cb)(struct switchdev_obj *obj))
+{
+	if (!ent->is_valid)
+		return 0;
+
+	if (port != ent->port)
+		return 0;
+
+	ether_addr_copy(fdb->addr, ent->mac);
+	fdb->vid = ent->vid;
+	fdb->ndm_state = ent->is_static ? NUD_NOARP : NUD_REACHABLE;
+
+	return cb(&fdb->obj);
+}
+
+static int bcm_sf2_sw_fdb_dump(struct dsa_switch *ds, int port,
+			       struct switchdev_obj_port_fdb *fdb,
+			       int (*cb)(struct switchdev_obj *obj))
+{
+	struct bcm_sf2_priv *priv = ds_to_priv(ds);
+	struct net_device *dev = ds->ports[port];
+	struct bcm_sf2_arl_entry results[2];
+	unsigned int count = 0;
+	int ret;
+
+	/* Start search operation */
+	core_writel(priv, ARLA_SRCH_STDN, CORE_ARLA_SRCH_CTL);
+
+	do {
+		ret = bcm_sf2_arl_search_wait(priv);
+		if (ret)
+			return ret;
+
+		/* Read both entries, then return their values back */
+		bcm_sf2_arl_search_rd(priv, 0, &results[0]);
+		ret = bcm_sf2_sw_fdb_copy(dev, port, &results[0], fdb, cb);
+		if (ret)
+			return ret;
+
+		bcm_sf2_arl_search_rd(priv, 1, &results[1]);
+		ret = bcm_sf2_sw_fdb_copy(dev, port, &results[1], fdb, cb);
+		if (ret)
+			return ret;
+
+		if (!results[0].is_valid && !results[1].is_valid)
+			break;
+
+	} while (count++ < CORE_ARLA_NUM_ENTRIES);
+
+	return 0;
+}
+
 static irqreturn_t bcm_sf2_switch_0_isr(int irq, void *dev_id)
 {
 	struct bcm_sf2_priv *priv = dev_id;
@@ -1076,6 +1308,10 @@ static struct dsa_switch_driver bcm_sf2_switch_driver = {
 	.port_join_bridge	= bcm_sf2_sw_br_join,
 	.port_leave_bridge	= bcm_sf2_sw_br_leave,
 	.port_stp_update	= bcm_sf2_sw_br_set_stp_state,
+	.port_fdb_prepare	= bcm_sf2_sw_fdb_prepare,
+	.port_fdb_add		= bcm_sf2_sw_fdb_add,
+	.port_fdb_del		= bcm_sf2_sw_fdb_del,
+	.port_fdb_dump		= bcm_sf2_sw_fdb_dump,
 };
 
 static int __init bcm_sf2_init(void)
diff --git a/drivers/net/dsa/bcm_sf2.h b/drivers/net/dsa/bcm_sf2.h
index 789d7b7737da..cc98abc0aaf3 100644
--- a/drivers/net/dsa/bcm_sf2.h
+++ b/drivers/net/dsa/bcm_sf2.h
@@ -19,6 +19,8 @@
 #include <linux/mutex.h>
 #include <linux/mii.h>
 #include <linux/ethtool.h>
+#include <linux/types.h>
+#include <linux/bitops.h>
 
 #include <net/dsa.h>
 
@@ -50,6 +52,60 @@ struct bcm_sf2_port_status {
 	u32 vlan_ctl_mask;
 };
 
+struct bcm_sf2_arl_entry {
+	u8 port;
+	u8 mac[ETH_ALEN];
+	u16 vid;
+	u8 is_valid:1;
+	u8 is_age:1;
+	u8 is_static:1;
+};
+
+static inline void bcm_sf2_mac_from_u64(u64 src, u8 *dst)
+{
+	unsigned int i;
+
+	for (i = 0; i < ETH_ALEN; i++)
+		dst[ETH_ALEN - 1 - i] = (src >> (8 * i)) & 0xff;
+}
+
+static inline u64 bcm_sf2_mac_to_u64(const u8 *src)
+{
+	unsigned int i;
+	u64 dst = 0;
+
+	for (i = 0; i < ETH_ALEN; i++)
+		dst |= (u64)src[ETH_ALEN - 1 - i] << (8 * i);
+
+	return dst;
+}
+
+static inline void bcm_sf2_arl_to_entry(struct bcm_sf2_arl_entry *ent,
+					u64 mac_vid, u32 fwd_entry)
+{
+	memset(ent, 0, sizeof(*ent));
+	ent->port = fwd_entry & PORTID_MASK;
+	ent->is_valid = !!(fwd_entry & ARL_VALID);
+	ent->is_age = !!(fwd_entry & ARL_AGE);
+	ent->is_static = !!(fwd_entry & ARL_STATIC);
+	bcm_sf2_mac_from_u64(mac_vid, ent->mac);
+	ent->vid = mac_vid >> VID_SHIFT;
+}
+
+static inline void bcm_sf2_arl_from_entry(u64 *mac_vid, u32 *fwd_entry,
+					  const struct bcm_sf2_arl_entry *ent)
+{
+	*mac_vid = bcm_sf2_mac_to_u64(ent->mac);
+	*mac_vid |= (u64)(ent->vid & VID_MASK) << VID_SHIFT;
+	*fwd_entry = ent->port & PORTID_MASK;
+	if (ent->is_valid)
+		*fwd_entry |= ARL_VALID;
+	if (ent->is_static)
+		*fwd_entry |= ARL_STATIC;
+	if (ent->is_age)
+		*fwd_entry |= ARL_AGE;
+}
+
 struct bcm_sf2_priv {
 	/* Base registers, keep those in order with BCM_SF2_REGS_NAME */
 	void __iomem			*core;
diff --git a/drivers/net/dsa/bcm_sf2_regs.h b/drivers/net/dsa/bcm_sf2_regs.h
index fa4e6e78c9ea..97780d43b5c0 100644
--- a/drivers/net/dsa/bcm_sf2_regs.h
+++ b/drivers/net/dsa/bcm_sf2_regs.h
@@ -231,6 +231,49 @@
 #define CORE_BRCM_HDR_RX_DIS		0x0980
 #define CORE_BRCM_HDR_TX_DIS		0x0988
 
+#define CORE_ARLA_NUM_ENTRIES		1024
+
+#define CORE_ARLA_RWCTL			0x1400
+#define  ARL_RW				(1 << 0)
+#define  IVL_SVL_SELECT			(1 << 6)
+#define  ARL_STRTDN			(1 << 7)
+
+#define CORE_ARLA_MAC			0x1408
+#define CORE_ARLA_VID			0x1420
+#define  ARLA_VIDTAB_INDX_MASK		0x1fff
+
+#define CORE_ARLA_MACVID0		0x1440
+#define  MAC_MASK			0xffffffffff
+#define  VID_SHIFT			48
+#define  VID_MASK			0xfff
+
+#define CORE_ARLA_FWD_ENTRY0		0x1460
+#define  PORTID_MASK			0x1ff
+#define  ARL_CON_SHIFT			9
+#define  ARL_CON_MASK			0x3
+#define  ARL_PRI_SHIFT			11
+#define  ARL_PRI_MASK			0x7
+#define  ARL_AGE			(1 << 14)
+#define  ARL_STATIC			(1 << 15)
+#define  ARL_VALID			(1 << 16)
+
+#define CORE_ARLA_MACVID_ENTRY(x)	(CORE_ARLA_MACVID0 + ((x) * 0x40))
+#define CORE_ARLA_FWD_ENTRY(x)		(CORE_ARLA_FWD_ENTRY0 + ((x) * 0x40))
+
+#define CORE_ARLA_SRCH_CTL		0x1540
+#define  ARLA_SRCH_VLID			(1 << 0)
+#define  IVL_SVL_SELECT			(1 << 6)
+#define  ARLA_SRCH_STDN			(1 << 7)
+
+#define CORE_ARLA_SRCH_ADR		0x1544
+#define  ARLA_SRCH_ADR_VALID		(1 << 15)
+
+#define CORE_ARLA_SRCH_RSLT_0_MACVID	0x1580
+#define CORE_ARLA_SRCH_RSLT_0		0x15a0
+
+#define CORE_ARLA_SRCH_RSLT_MACVID(x)	(CORE_ARLA_SRCH_RSLT_0_MACVID + ((x) * 0x40))
+#define CORE_ARLA_SRCH_RSLT(x)		(CORE_ARLA_SRCH_RSLT_0 + ((x) * 0x40))
+
 #define CORE_MEM_PSM_VDD_CTRL		0x2380
 #define  P_TXQ_PSM_VDD_SHIFT		2
 #define  P_TXQ_PSM_VDD_MASK		0x3
-- 
2.1.0

Re: [PATCH net-next] net: dsa: bcm_sf2: Implement FDB operations

[Florian Fainelli]

On 06/03/2016 05:05 PM, Florian Fainelli wrote:
> Add support for the FDB add, delete, and dump operations. The add and
> delete operations are implemented using directed ARL operations using
> the specified MAC address and consist in a read operation, write and
> readback operation.
> 
> The dump operation consists in using the ARL search and software
> filtering entries which are not for the desired port.

And this one too should be discarded.
-- 
Florian

Re: [PATCH net-next] net: dsa: bcm_sf2: Implement FDB operations

[Vivien Didelot]

Florian Fainelli <f.fainelli@gmail.com> writes:

> Add support for the FDB add, delete, and dump operations. The add and
> delete operations are implemented using directed ARL operations using
> the specified MAC address and consist in a read operation, write and
> readback operation.
>
> The dump operation consists in using the ARL search and software
> filtering entries which are not for the desired port.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

[PATCH net-next 2/9] net: dsa: Add support for parsing the old binding

[Florian Fainelli]

Extend dsa2.c to support parsing for the old binding, which mostly means
looking for more or less the same properties in different places.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/dsa2.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 126 insertions(+), 16 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index b5640d8ffbae..23273fd984a8 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -408,21 +408,11 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
 	dst->applied = false;
 }
 
-static int dsa_cpu_parse(struct device_node *port, u32 index,
-			 struct dsa_switch_tree *dst,
-			 struct dsa_switch *ds)
+static int _dsa_cpu_parse(struct dsa_switch_tree *dst,
+			   struct dsa_switch *ds,
+			   struct net_device *ethernet_dev,
+			   u32 index)
 {
-	struct net_device *ethernet_dev;
-	struct device_node *ethernet;
-
-	ethernet = of_parse_phandle(port, "ethernet", 0);
-	if (!ethernet)
-		return -EINVAL;
-
-	ethernet_dev = of_find_net_device_by_node(ethernet);
-	if (!ethernet_dev)
-		return -EPROBE_DEFER;
-
 	if (!ds->master_netdev)
 		ds->master_netdev = ethernet_dev;
 
@@ -445,6 +435,24 @@ static int dsa_cpu_parse(struct device_node *port, u32 index,
 	return 0;
 }
 
+static int dsa_cpu_parse(struct device_node *port, u32 index,
+			 struct dsa_switch_tree *dst,
+			 struct dsa_switch *ds)
+{
+	struct net_device *ethernet_dev;
+	struct device_node *ethernet;
+
+	ethernet = of_parse_phandle(port, "ethernet", 0);
+	if (!ethernet)
+		return -EINVAL;
+
+	ethernet_dev = of_find_net_device_by_node(ethernet);
+	if (!ethernet_dev)
+		return -EPROBE_DEFER;
+
+	return _dsa_cpu_parse(dst, ds, ethernet_dev, index);
+}
+
 static int dsa_ds_parse(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 {
 	struct device_node *port;
@@ -552,6 +560,108 @@ static struct device_node *dsa_get_ports(struct dsa_switch *ds,
 	return ports;
 }
 
+static int _dsa_register_switch_legacy(struct dsa_switch *ds, struct device_node *np)
+{
+	const char *compat = of_get_property(np, "compatible", NULL);
+	struct device_node *dn, *ethernet;
+	struct net_device *ethernet_dev;
+	struct dsa_switch_tree *dst;
+	u32 tree = 0, index;
+	int err;
+
+	/* Tree is implied by how many devices are present in the DT with the
+	 * supported compatible strings from net/dsa/dsa.c
+	 */
+	for_each_compatible_node(dn, NULL, compat) {
+		if (dn != np)
+			tree++;
+	}
+
+	/* index is present in the "reg" property, second cell */
+	err = of_property_read_u32_index(np->child, "reg", 1, &index);
+	if (err)
+		return err;
+
+	if (index >= DSA_MAX_SWITCHES)
+		return -EINVAL;
+
+	err = dsa_parse_ports_dn(np->child, ds);
+	if (err)
+		return err;
+
+	dst = dsa_get_dst(tree);
+	if (!dst) {
+		dst = dsa_add_dst(tree);
+		if (!dst)
+			return -ENOMEM;
+	}
+
+	if (dst->ds[index]) {
+		err = -EBUSY;
+		goto out;
+	}
+
+	ds->dst = dst;
+	ds->index = index;
+	dsa_dst_add_ds(dst, ds, index);
+
+	err = dsa_dst_complete(dst);
+	if (err < 0)
+		goto out;
+
+	if (err == 1) {
+		/* Not all switches registered yet */
+		err = 0;
+		goto out;
+	}
+
+	if (dst->applied) {
+		pr_info("DSA: Disjoint trees?\n");
+		err = -EINVAL;
+		goto out_del_dst;
+	}
+
+	ethernet = of_parse_phandle(np, "dsa,ethernet", 0);
+	if (!ethernet) {
+		err = -EINVAL;
+		goto out_del_dst;
+	}
+
+	ethernet_dev = of_find_net_device_by_node(ethernet);
+	if (!ethernet_dev) {
+		err = -EPROBE_DEFER;
+		goto out_del_dst;
+	}
+
+	err = _dsa_cpu_parse(dst, ds, ethernet_dev, index);
+	if (err)
+		goto out_del_dst;
+
+	if (!dst->master_netdev) {
+		pr_warn("Tree has no master device\n");
+		goto out_del_dst;
+	}
+
+	pr_info("DSA: tree %d parsed\n", dst->tree);
+
+	err = dsa_dst_apply(dst);
+	if (err) {
+		dsa_dst_unapply(dst);
+		goto out_del_dst;
+	}
+
+	dsa_put_dst(dst);
+
+	return 0;
+
+out_del_dst:
+	dsa_dst_del_ds(dst, ds, ds->index);
+out:
+	dsa_put_dst(dst);
+
+	return err;
+}
+
 static int __dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
 {
 	struct device_node *ports = dsa_get_ports(ds, np);
@@ -626,8 +736,8 @@ static int _dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
 {
 	struct device_node *ports = dsa_get_ports(ds, np);
 
-	if (IS_ERR(ports))
-		return PTR_ERR(ports);
+	if (IS_ERR(ports) && PTR_ERR(ports) == -EINVAL)
+		return _dsa_register_switch_legacy(ds, np);
 
 	return __dsa_register_switch(ds, np);
 }
-- 
2.7.4

Re: [PATCH net-next 2/9] net: dsa: Add support for parsing the old binding

[Andrew Lunn]

> @@ -626,8 +736,8 @@ static int _dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
>  {
>  	struct device_node *ports = dsa_get_ports(ds, np);
>  
> -	if (IS_ERR(ports))
> -		return PTR_ERR(ports);
> +	if (IS_ERR(ports) && PTR_ERR(ports) == -EINVAL)
> +		return _dsa_register_switch_legacy(ds, np);
>  
>  	return __dsa_register_switch(ds, np);
>  }

Hi Florian

Could you put this into dsa_register_switch() and so avoid all the __
prefixes.

Thanks
	Andrew

Re: [PATCH net-next 2/9] net: dsa: Add support for parsing the old binding

[Andrew Lunn]

> -static int dsa_cpu_parse(struct device_node *port, u32 index,
> -			 struct dsa_switch_tree *dst,
> -			 struct dsa_switch *ds)
> +static int _dsa_cpu_parse(struct dsa_switch_tree *dst,
> +			   struct dsa_switch *ds,
> +			   struct net_device *ethernet_dev,
> +			   u32 index)
>  {
> -	struct net_device *ethernet_dev;
> -	struct device_node *ethernet;
> -
> -	ethernet = of_parse_phandle(port, "ethernet", 0);
> -	if (!ethernet)
> -		return -EINVAL;
> -
> -	ethernet_dev = of_find_net_device_by_node(ethernet);
> -	if (!ethernet_dev)
> -		return -EPROBE_DEFER;
> -

Hi Florian

You have just removed all the actual DT parsing. So i would give this
a different name, and avoid the _ prefix.

> +static int _dsa_register_switch_legacy(struct dsa_switch *ds, struct device_node *np)
> +{

We might want to call this _dsa_register_switch_legacy_sf2, since the
code only supports what is needed for your rather odd sf2 binding. It
does not appear to work for the generic DSA binding.

>  static int __dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
>  {
>  	struct device_node *ports = dsa_get_ports(ds, np);
> @@ -626,8 +736,8 @@ static int _dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
>  {
>  	struct device_node *ports = dsa_get_ports(ds, np);
>  
> -	if (IS_ERR(ports))
> -		return PTR_ERR(ports);
> +	if (IS_ERR(ports) && PTR_ERR(ports) == -EINVAL)
> +		return _dsa_register_switch_legacy(ds, np);

Maybe put this inside if config_enabled(CONFIG_NET_DSA_BCM_SF2)  {}?

      Andrew

Re: [PATCH net-next 2/9] net: dsa: Add support for parsing the old binding

[Florian Fainelli]

Le 04/06/2016 13:00, Andrew Lunn a écrit :
>> -static int dsa_cpu_parse(struct device_node *port, u32 index,
>> -			 struct dsa_switch_tree *dst,
>> -			 struct dsa_switch *ds)
>> +static int _dsa_cpu_parse(struct dsa_switch_tree *dst,
>> +			   struct dsa_switch *ds,
>> +			   struct net_device *ethernet_dev,
>> +			   u32 index)
>>  {
>> -	struct net_device *ethernet_dev;
>> -	struct device_node *ethernet;
>> -
>> -	ethernet = of_parse_phandle(port, "ethernet", 0);
>> -	if (!ethernet)
>> -		return -EINVAL;
>> -
>> -	ethernet_dev = of_find_net_device_by_node(ethernet);
>> -	if (!ethernet_dev)
>> -		return -EPROBE_DEFER;
>> -
> 
> Hi Florian
> 
> You have just removed all the actual DT parsing. So i would give this
> a different name, and avoid the _ prefix.
> 
>> +static int _dsa_register_switch_legacy(struct dsa_switch *ds, struct device_node *np)
>> +{
> 
> We might want to call this _dsa_register_switch_legacy_sf2, since the
> code only supports what is needed for your rather odd sf2 binding.

The SF2 binding just encapsulates the normal legacy DSA binding in its
simplified, one switch only configuration, that makes things much
easier, but not way off.

> It does not appear to work for the generic DSA binding.

It would if we also added support for parsing and filing in a routing
table, so yes, that is currently missing.

How about offering dsa_of_parse() (or another name) as a helper function
which fills up a dsa_switch / dsa_switch_tree structure to the best it
can, and then let a driver dealing with the old binding call into the
bottom parts of dsa_register_switch()? That could allow us to remove
some of the duplicated code from net/dsa/dsa.c by doing so

The other approach could be to have custom DT parsing in bcm_sf2.c and
also call the bottom parts of dsa_register_switch() once it has properly
initialized ds/dst.

How much support do we want to have for the old binding for in tree
platforms? Is the plan to migrate them all to the new binding?

-- 
Florian

Re: [PATCH net-next 2/9] net: dsa: Add support for parsing the old binding

[Andrew Lunn]

> How much support do we want to have for the old binding for in tree
> platforms? Is the plan to migrate them all to the new binding?

I think there are three cases to consider.

1) There are some old boards using setup.c files which have a platform
   device, platform data, etc. I've never used DSA in this way, and it
   could be all the recent additions have broken this. We might want
   to test this, and if it is in fact broken, and has been for a
   while, it indicates nobody uses those boards any more. We might
   suggest removing them. Even if they do work, i doubt anybody is
   interested in converting them to device tree. So we might have to
   keep the platform data support around.

2) In tree devices using the DT binding. We can update them all to the
   new binding. The kirkwood boards don't have a u-boot which is DT
   capable. Some of the armada boards do have a DT capable uboot, but
   all these boards have been added by the community, so i suspect
   they are not flashed never to be changed again.

3) Out of tree devices using the DT binding. As far as i can see,
   there is no in three board actually using the Broadcom SF2 driver
   and its odd binding. However from talking to you, i know there are
   devices out in the wild using this binding, and their DT blob is
   fixed, never to be changed again.

It actually seems odd to me that we have a nice new binding and an
implement which is reasonably clean, and we want to add code to
support a legacy binding for an out of tree board.

I need to think on this for a while. However, i don't see the old code
and binding going away anytime soon. It will take a few cycles to
determine if the old platform device/platform data still works, and to
remove the old boards if not. We can update the in tree devices to the
new binding, but we should keep the old binding a while to aid the
transition.

I'm tempted to say you should keep using the old code to support your
out of tree devices. You should define a new binding for SF2 which
conforms to the device tree binding document which just got accepted,
and add it to SF 2 alongside the legacy binding. And it would be great
if you could go the last step and actually add a boards device tree
file using it.

I'm hesitant to add legacy binding support for SF2 to the new DSA2
code. We should try to keep it free of cruft, and set a good example
for others to follow when they bring along there new drivers.

	Andrew

Re: [PATCH net-next 2/9] net: dsa: Add support for parsing the old binding

[Florian Fainelli]

On 06/05/2016 08:19 PM, Andrew Lunn wrote:
>> How much support do we want to have for the old binding for in tree
>> platforms? Is the plan to migrate them all to the new binding?
> 
> I think there are three cases to consider.
> 
> 1) There are some old boards using setup.c files which have a platform
>    device, platform data, etc. I've never used DSA in this way, and it
>    could be all the recent additions have broken this. We might want
>    to test this, and if it is in fact broken, and has been for a
>    while, it indicates nobody uses those boards any more. We might
>    suggest removing them. Even if they do work, i doubt anybody is
>    interested in converting them to device tree. So we might have to
>    keep the platform data support around.

We had a report a while ago of breakage, which got addressed and fixed
upstream, so if it breaks again, it will get fixed again.

> 
> 2) In tree devices using the DT binding. We can update them all to the
>    new binding. The kirkwood boards don't have a u-boot which is DT
>    capable. Some of the armada boards do have a DT capable uboot, but
>    all these boards have been added by the community, so i suspect
>    they are not flashed never to be changed again.
> 
> 3) Out of tree devices using the DT binding. As far as i can see,
>    there is no in three board actually using the Broadcom SF2 driver
>    and its odd binding. However from talking to you, i know there are
>    devices out in the wild using this binding, and their DT blob is
>    fixed, never to be changed again.

The concept of an "in-tree" board does not make much sense once the
bootloader provides a blob to the kernel, and synchronizing the Device
Tree sources with what a bootloader provides is just a pain with no
reward as long as the binding remains standard and works.

> 
> It actually seems odd to me that we have a nice new binding and an
> implement which is reasonably clean, and we want to add code to
> support a legacy binding for an out of tree board.
> 
> I need to think on this for a while. However, i don't see the old code
> and binding going away anytime soon. It will take a few cycles to
> determine if the old platform device/platform data still works, and to
> remove the old boards if not. We can update the in tree devices to the
> new binding, but we should keep the old binding a while to aid the
> transition.

I do not see the need for platform data going away actually, there are
tons of devices out there that are not supported using Device Tree, yet
feature Ethernet switches that could easily be supported would we want
to add support for that, and clearly an answer along the lines of let's
add Device Tree support for these platforms is not going to fly.

> 
> I'm tempted to say you should keep using the old code to support your
> out of tree devices. You should define a new binding for SF2 which
> conforms to the device tree binding document which just got accepted,
> and add it to SF 2 alongside the legacy binding. And it would be great
> if you could go the last step and actually add a boards device tree
> file using it.

I suppose I could do that.

> 
> I'm hesitant to add legacy binding support for SF2 to the new DSA2
> code. We should try to keep it free of cruft, and set a good example
> for others to follow when they bring along there new drivers.

What if this code was moved to the bcm_sf2.c where it matters and there
is just the bottom part of dsa_register_switch() exposed instead?
-- 
Florian

[PATCH net-next 3/9] net: dsa: Provide unique DSA slave MII bus names

[Florian Fainelli]

In case we have multiples trees and switches with the same index, we
need to add another discriminating id: the switch tree.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/slave.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 15a492261895..a51dfedf0014 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -49,7 +49,8 @@ void dsa_slave_mii_bus_init(struct dsa_switch *ds)
 	ds->slave_mii_bus->name = "dsa slave smi";
 	ds->slave_mii_bus->read = dsa_slave_phy_read;
 	ds->slave_mii_bus->write = dsa_slave_phy_write;
-	snprintf(ds->slave_mii_bus->id, MII_BUS_ID_SIZE, "dsa-%d", ds->index);
+	snprintf(ds->slave_mii_bus->id, MII_BUS_ID_SIZE, "dsa-%d.%d",
+		 ds->dst->tree, ds->index);
 	ds->slave_mii_bus->parent = ds->dev;
 	ds->slave_mii_bus->phy_mask = ~ds->phys_mii_mask;
 }
-- 
2.7.4

Re: [PATCH net-next 3/9] net: dsa: Provide unique DSA slave MII bus names

[Andrew Lunn]

On Fri, Jun 03, 2016 at 05:05:25PM -0700, Florian Fainelli wrote:
> In case we have multiples trees and switches with the same index, we
> need to add another discriminating id: the switch tree.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

     Andrew

Re: [PATCH net-next 3/9] net: dsa: Provide unique DSA slave MII bus names

[Vivien Didelot]

Florian Fainelli <f.fainelli@gmail.com> writes:

> In case we have multiples trees and switches with the same index, we
> need to add another discriminating id: the switch tree.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

[PATCH net-next 4/9] net: dsa: Initialize ds->enabled_port_mask and ds->phys_mii_mask

[Florian Fainelli]

Some drivers heavily rely on these two bitmasks to contain the correct
values for them to successfully probe and initialize at drv->setup()
time, calculate correct values to put in both masks.

To avoid multiple ports lookup, we also try to set dst->cpu_port during
dsa_parse_ports_dn(), which is mostly useful for the case where we probe
using the legacy binding which has properties/nodes in different places
and does not use dsa_dst_parse().

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/dsa2.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 47 insertions(+), 12 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 23273fd984a8..e8386157de30 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -281,6 +281,7 @@ static void dsa_user_port_unapply(struct device_node *port, u32 index,
 	if (ds->ports[index].netdev) {
 		dsa_slave_destroy(ds->ports[index].netdev);
 		ds->ports[index].netdev = NULL;
+		ds->enabled_port_mask &= ~(1 << index);
 	}
 }
 
@@ -290,6 +291,13 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 	u32 index;
 	int err;
 
+	/* Initialize ds->phys_mii_mask before registering the slave MDIO bus
+	 * driver and before drv->setup() has run, since the switch drivers and
+	 * the slave MDIO bus driver rely on these values for probing PHY
+	 * devices or not
+	 */
+	ds->phys_mii_mask = ds->enabled_port_mask;
+
 	err = ds->drv->setup(ds);
 	if (err < 0)
 		return err;
@@ -302,6 +310,18 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 	if (err < 0)
 		return err;
 
+	if (!ds->slave_mii_bus && ds->drv->phy_read) {
+		ds->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
+		if (!ds->slave_mii_bus)
+			return err;
+
+		dsa_slave_mii_bus_init(ds);
+
+		err = mdiobus_register(ds->slave_mii_bus);
+		if (err < 0)
+			return err;
+	}
+
 	for (index = 0; index < DSA_MAX_PORTS; index++) {
 		port = ds->ports[index].dn;
 		if (!port)
@@ -421,7 +441,11 @@ static int _dsa_cpu_parse(struct dsa_switch_tree *dst,
 
 	if (dst->cpu_switch == -1) {
 		dst->cpu_switch = ds->index;
-		dst->cpu_port = index;
+		/* Only assign dst->cpu_port if not done already by
+		 * dsa_parse_ports_dn
+		 */
+		if (!dst->cpu_port)
+			dst->cpu_port = index;
 	}
 
 	dst->tag_ops = dsa_resolve_tag_protocol(ds->drv->tag_protocol);
@@ -517,6 +541,15 @@ static int dsa_parse_ports_dn(struct device_node *ports, struct dsa_switch *ds)
 			return -EINVAL;
 
 		ds->ports[reg].dn = port;
+
+		if (dsa_port_is_cpu(port))
+			ds->dst->cpu_port = reg;
+		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 |= 1 << reg;
 	}
 
 	return 0;
@@ -585,10 +618,6 @@ static int _dsa_register_switch_legacy(struct dsa_switch *ds, struct device_node
 	if (index >= DSA_MAX_SWITCHES)
 		return -EINVAL;
 
-	err = dsa_parse_ports_dn(np->child, ds);
-	if (err)
-		return err;
-
 	dst = dsa_get_dst(tree);
 	if (!dst) {
 		dst = dsa_add_dst(tree);
@@ -596,12 +625,17 @@ static int _dsa_register_switch_legacy(struct dsa_switch *ds, struct device_node
 			return -ENOMEM;
 	}
 
+	ds->dst = dst;
+
+	err = dsa_parse_ports_dn(np->child, ds);
+	if (err)
+		return err;
+
 	if (dst->ds[index]) {
 		err = -EBUSY;
 		goto out;
 	}
 
-	ds->dst = dst;
 	ds->index = index;
 	dsa_dst_add_ds(dst, ds, index);
 
@@ -633,7 +667,7 @@ static int _dsa_register_switch_legacy(struct dsa_switch *ds, struct device_node
 		goto out_del_dst;
 	}
 
-	err = _dsa_cpu_parse(dst, ds, ethernet_dev, index);
+	err = _dsa_cpu_parse(dst, ds, ethernet_dev, dst->cpu_port);
 	if (err)
 		goto out_del_dst;
 
@@ -676,10 +710,6 @@ static int __dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
 	if (IS_ERR(ports))
 		return PTR_ERR(ports);
 
-	err = dsa_parse_ports_dn(ports, ds);
-	if (err)
-		return err;
-
 	dst = dsa_get_dst(tree);
 	if (!dst) {
 		dst = dsa_add_dst(tree);
@@ -687,12 +717,17 @@ static int __dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
 			return -ENOMEM;
 	}
 
+	ds->dst = dst;
+
+	err = dsa_parse_ports_dn(ports, ds);
+	if (err)
+		return err;
+
 	if (dst->ds[index]) {
 		err = -EBUSY;
 		goto out;
 	}
 
-	ds->dst = dst;
 	ds->index = index;
 	dsa_dst_add_ds(dst, ds, index);
 
-- 
2.7.4

Re: [PATCH net-next 4/9] net: dsa: Initialize ds->enabled_port_mask and ds->phys_mii_mask

[Andrew Lunn]

> @@ -517,6 +541,15 @@ static int dsa_parse_ports_dn(struct device_node *ports, struct dsa_switch *ds)
>  			return -EINVAL;
>  
>  		ds->ports[reg].dn = port;
> +
> +		if (dsa_port_is_cpu(port))
> +			ds->dst->cpu_port = reg;
> +		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 |= 1 << reg;

Hi Florian

You need to be careful here. There can be multiple CPU ports, in
different switches. We want dst->cpu_port to be deterministic,
independent of the order switches are registered. Which is why i set
it as part of dsa_cpu_parse(), which only happens when all the
switches have registered, and we are parsing their device tree nodes
in order. So we guarantee dst->cpu_port is the first CPU node.

You now set dst->cpu_port via dsa_parse_ports_dn(), so it is now non
deterministic, it depends on the probe order of the switches.

In the long run, i want to deprecate and then remove dst->cpu_port,
but i'm not that far yet.

Please rethink this part of the patch, keeping in mind you have
multiple switches, with multiple CPU and DSA ports, all connected in
some crazy fashion.

       Andrew

Re: [PATCH net-next 4/9] net: dsa: Initialize ds->enabled_port_mask and ds->phys_mii_mask

[Florian Fainelli]

Le 04/06/2016 13:29, Andrew Lunn a écrit :
>> @@ -517,6 +541,15 @@ static int dsa_parse_ports_dn(struct device_node *ports, struct dsa_switch *ds)
>>  			return -EINVAL;
>>  
>>  		ds->ports[reg].dn = port;
>> +
>> +		if (dsa_port_is_cpu(port))
>> +			ds->dst->cpu_port = reg;
>> +		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 |= 1 << reg;
> 
> Hi Florian
> 
> You need to be careful here. There can be multiple CPU ports, in
> different switches. We want dst->cpu_port to be deterministic,
> independent of the order switches are registered. Which is why i set
> it as part of dsa_cpu_parse(), which only happens when all the
> switches have registered, and we are parsing their device tree nodes
> in order. So we guarantee dst->cpu_port is the first CPU node.

Ah OK, I completely missed that part and just wanted to avoid walking
the ports children nodes more than twice.

We might be able to get away with just initializing
ds->enabled_port_mask here actually.

> 
> You now set dst->cpu_port via dsa_parse_ports_dn(), so it is now non
> deterministic, it depends on the probe order of the switches.
> 
> In the long run, i want to deprecate and then remove dst->cpu_port,
> but i'm not that far yet.
> 
> Please rethink this part of the patch, keeping in mind you have
> multiple switches, with multiple CPU and DSA ports, all connected in
> some crazy fashion.
-- 
Florian

[PATCH net-next 5/9] net: dsa: Export suspend/resume functions

[Florian Fainelli]

In preparation for allowing switch drivers to implement system-wide
suspend/resume functions, export dsa_switch_suspend and
dsa_switch_resume() such that these are callable from the appropriate
driver specific suspend/resume functions.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 include/net/dsa.h | 14 ++++++++++++++
 net/dsa/dsa.c     |  6 ++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index d6ed5dee73e5..c5abaeda84f3 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -382,4 +382,18 @@ static inline bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
 
 void dsa_unregister_switch(struct dsa_switch *ds);
 int dsa_register_switch(struct dsa_switch *ds, struct device_node *np);
+#ifdef CONFIG_PM_SLEEP
+int dsa_switch_suspend(struct dsa_switch *ds);
+int dsa_switch_resume(struct dsa_switch *ds);
+#else
+static inline int dsa_switch_suspend(struct dsa_switch *ds)
+{
+	return 0;
+}
+static inline int dsa_switch_resume(struct dsa_switch *ds)
+{
+	return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
 #endif
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index d8cb2acd4f0a..ebc29a1bee08 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -500,7 +500,7 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
 }
 
 #ifdef CONFIG_PM_SLEEP
-static int dsa_switch_suspend(struct dsa_switch *ds)
+int dsa_switch_suspend(struct dsa_switch *ds)
 {
 	int i, ret = 0;
 
@@ -519,8 +519,9 @@ static int dsa_switch_suspend(struct dsa_switch *ds)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(dsa_switch_suspend);
 
-static int dsa_switch_resume(struct dsa_switch *ds)
+int dsa_switch_resume(struct dsa_switch *ds)
 {
 	int i, ret = 0;
 
@@ -542,6 +543,7 @@ static int dsa_switch_resume(struct dsa_switch *ds)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(dsa_switch_resume);
 #endif
 
 /* platform driver init and cleanup *****************************************/
-- 
2.7.4

Re: [PATCH net-next 5/9] net: dsa: Export suspend/resume functions

[Andrew Lunn]

On Fri, Jun 03, 2016 at 05:05:27PM -0700, Florian Fainelli wrote:
> In preparation for allowing switch drivers to implement system-wide
> suspend/resume functions, export dsa_switch_suspend and
> dsa_switch_resume() such that these are callable from the appropriate
> driver specific suspend/resume functions.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

     Andrew

> ---
>  include/net/dsa.h | 14 ++++++++++++++
>  net/dsa/dsa.c     |  6 ++++--
>  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index d6ed5dee73e5..c5abaeda84f3 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -382,4 +382,18 @@ static inline bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
>  
>  void dsa_unregister_switch(struct dsa_switch *ds);
>  int dsa_register_switch(struct dsa_switch *ds, struct device_node *np);
> +#ifdef CONFIG_PM_SLEEP
> +int dsa_switch_suspend(struct dsa_switch *ds);
> +int dsa_switch_resume(struct dsa_switch *ds);
> +#else
> +static inline int dsa_switch_suspend(struct dsa_switch *ds)
> +{
> +	return 0;
> +}
> +static inline int dsa_switch_resume(struct dsa_switch *ds)
> +{
> +	return 0;
> +}
> +#endif /* CONFIG_PM_SLEEP */
> +
>  #endif
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index d8cb2acd4f0a..ebc29a1bee08 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -500,7 +500,7 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
>  }
>  
>  #ifdef CONFIG_PM_SLEEP
> -static int dsa_switch_suspend(struct dsa_switch *ds)
> +int dsa_switch_suspend(struct dsa_switch *ds)
>  {
>  	int i, ret = 0;
>  
> @@ -519,8 +519,9 @@ static int dsa_switch_suspend(struct dsa_switch *ds)
>  
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(dsa_switch_suspend);
>  
> -static int dsa_switch_resume(struct dsa_switch *ds)
> +int dsa_switch_resume(struct dsa_switch *ds)
>  {
>  	int i, ret = 0;
>  
> @@ -542,6 +543,7 @@ static int dsa_switch_resume(struct dsa_switch *ds)
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(dsa_switch_resume);
>  #endif
>  
>  /* platform driver init and cleanup *****************************************/
> -- 
> 2.7.4
> 

Re: [PATCH net-next 5/9] net: dsa: Export suspend/resume functions

[Vivien Didelot]

Florian Fainelli <f.fainelli@gmail.com> writes:

> In preparation for allowing switch drivers to implement system-wide
> suspend/resume functions, export dsa_switch_suspend and
> dsa_switch_resume() such that these are callable from the appropriate
> driver specific suspend/resume functions.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Tested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

[PATCH net-next 6/9] net: dsa: Add initialization helper for CPU port ethtool_ops

[Florian Fainelli]

Add a helper function: dsa_cpu_port_ethtool_init() which initializes a
custom ethtool_ops structure with custom DSA ethtool operations for CPU
ports. This is a preliminary change to move the initialization outside
of net/dsa/slave.c.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/dsa_priv.h |  1 +
 net/dsa/slave.c    | 14 ++++++++------
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index b42f1a5f95f3..106a9f067f94 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -58,6 +58,7 @@ const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol);
 /* slave.c */
 extern const struct dsa_device_ops notag_netdev_ops;
 void dsa_slave_mii_bus_init(struct dsa_switch *ds);
+void dsa_cpu_port_ethtool_init(struct ethtool_ops *ops);
 int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
 		     int port, const char *name);
 void dsa_slave_destroy(struct net_device *slave_dev);
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index a51dfedf0014..8d159932e082 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -865,6 +865,13 @@ static void dsa_slave_poll_controller(struct net_device *dev)
 }
 #endif
 
+void dsa_cpu_port_ethtool_init(struct ethtool_ops *ops)
+{
+	ops->get_sset_count = dsa_cpu_port_get_sset_count;
+	ops->get_ethtool_stats = dsa_cpu_port_get_ethtool_stats;
+	ops->get_strings = dsa_cpu_port_get_strings;
+}
+
 static const struct ethtool_ops dsa_slave_ethtool_ops = {
 	.get_settings		= dsa_slave_get_settings,
 	.set_settings		= dsa_slave_set_settings,
@@ -1124,12 +1131,7 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
 		       sizeof(struct ethtool_ops));
 		memcpy(&dsa_cpu_port_ethtool_ops, &dst->master_ethtool_ops,
 		       sizeof(struct ethtool_ops));
-		dsa_cpu_port_ethtool_ops.get_sset_count =
-					dsa_cpu_port_get_sset_count;
-		dsa_cpu_port_ethtool_ops.get_ethtool_stats =
-					dsa_cpu_port_get_ethtool_stats;
-		dsa_cpu_port_ethtool_ops.get_strings =
-					dsa_cpu_port_get_strings;
+		dsa_cpu_port_ethtool_init(&dsa_cpu_port_ethtool_ops);
 		master->ethtool_ops = &dsa_cpu_port_ethtool_ops;
 	}
 	eth_hw_addr_inherit(slave_dev, master);
-- 
2.7.4

[PATCH net-next 6/7] net: dsa: bcm_sf2: Make it a real platform device driver

[Florian Fainelli]

The Broadcom Starfighter 2 switch driver should be a proper platform
driver, now that the DSA code has been updated to allow that, register a
switch device, feed it with the proper configuration data coming from
Device Tree and register our switch device with DSA.

The bulk of the changes consist in moving what bcm_sf2_sw_setup() did
into the platform driver probe function.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/bcm_sf2.c | 251 ++++++++++++++++++++++++++++------------------
 net/dsa/dsa.c             |   1 -
 2 files changed, 151 insertions(+), 101 deletions(-)

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 73df91bb0466..8dac74a6b5df 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -934,77 +934,8 @@ static void bcm_sf2_identify_ports(struct bcm_sf2_priv *priv,
 
 static int bcm_sf2_sw_setup(struct dsa_switch *ds)
 {
-	const char *reg_names[BCM_SF2_REGS_NUM] = BCM_SF2_REGS_NAME;
 	struct bcm_sf2_priv *priv = ds_to_priv(ds);
-	struct device_node *dn;
-	void __iomem **base;
 	unsigned int port;
-	unsigned int i;
-	u32 reg, rev;
-	int ret;
-
-	spin_lock_init(&priv->indir_lock);
-	mutex_init(&priv->stats_mutex);
-
-	/* All the interesting properties are at the parent device_node
-	 * level
-	 */
-	dn = ds->cd->of_node->parent;
-	bcm_sf2_identify_ports(priv, ds->cd->of_node);
-
-	priv->irq0 = irq_of_parse_and_map(dn, 0);
-	priv->irq1 = irq_of_parse_and_map(dn, 1);
-
-	base = &priv->core;
-	for (i = 0; i < BCM_SF2_REGS_NUM; i++) {
-		*base = of_iomap(dn, i);
-		if (*base == NULL) {
-			pr_err("unable to find register: %s\n", reg_names[i]);
-			ret = -ENOMEM;
-			goto out_unmap;
-		}
-		base++;
-	}
-
-	ret = bcm_sf2_sw_rst(priv);
-	if (ret) {
-		pr_err("unable to software reset switch: %d\n", ret);
-		goto out_unmap;
-	}
-
-	/* Disable all interrupts and request them */
-	bcm_sf2_intr_disable(priv);
-
-	ret = request_irq(priv->irq0, bcm_sf2_switch_0_isr, 0,
-			  "switch_0", priv);
-	if (ret < 0) {
-		pr_err("failed to request switch_0 IRQ\n");
-		goto out_unmap;
-	}
-
-	ret = request_irq(priv->irq1, bcm_sf2_switch_1_isr, 0,
-			  "switch_1", priv);
-	if (ret < 0) {
-		pr_err("failed to request switch_1 IRQ\n");
-		goto out_free_irq0;
-	}
-
-	/* Reset the MIB counters */
-	reg = core_readl(priv, CORE_GMNCFGCFG);
-	reg |= RST_MIB_CNT;
-	core_writel(priv, reg, CORE_GMNCFGCFG);
-	reg &= ~RST_MIB_CNT;
-	core_writel(priv, reg, CORE_GMNCFGCFG);
-
-	/* Get the maximum number of ports for this switch */
-	priv->hw_params.num_ports = core_readl(priv, CORE_IMP0_PRT_ID) + 1;
-	if (priv->hw_params.num_ports > DSA_MAX_PORTS)
-		priv->hw_params.num_ports = DSA_MAX_PORTS;
-
-	/* Assume a single GPHY setup if we can't read that property */
-	if (of_property_read_u32(dn, "brcm,num-gphy",
-				 &priv->hw_params.num_gphy))
-		priv->hw_params.num_gphy = 1;
 
 	/* Enable all valid ports and disable those unused */
 	for (port = 0; port < priv->hw_params.num_ports; port++) {
@@ -1034,31 +965,7 @@ static int bcm_sf2_sw_setup(struct dsa_switch *ds)
 	else
 		ds->phys_mii_mask = 0;
 
-	rev = reg_readl(priv, REG_SWITCH_REVISION);
-	priv->hw_params.top_rev = (rev >> SWITCH_TOP_REV_SHIFT) &
-					SWITCH_TOP_REV_MASK;
-	priv->hw_params.core_rev = (rev & SF2_REV_MASK);
-
-	rev = reg_readl(priv, REG_PHY_REVISION);
-	priv->hw_params.gphy_rev = rev & PHY_REVISION_MASK;
-
-	pr_info("Starfighter 2 top: %x.%02x, core: %x.%02x base: 0x%p, IRQs: %d, %d\n",
-		priv->hw_params.top_rev >> 8, priv->hw_params.top_rev & 0xff,
-		priv->hw_params.core_rev >> 8, priv->hw_params.core_rev & 0xff,
-		priv->core, priv->irq0, priv->irq1);
-
 	return 0;
-
-out_free_irq0:
-	free_irq(priv->irq0, priv);
-out_unmap:
-	base = &priv->core;
-	for (i = 0; i < BCM_SF2_REGS_NUM; i++) {
-		if (*base)
-			iounmap(*base);
-		base++;
-	}
-	return ret;
 }
 
 static int bcm_sf2_sw_set_addr(struct dsa_switch *ds, u8 *addr)
@@ -1370,7 +1277,7 @@ static int bcm_sf2_sw_set_wol(struct dsa_switch *ds, int port,
 	return p->ethtool_ops->set_wol(p, wol);
 }
 
-static struct dsa_switch_driver bcm_sf2_switch_driver = {
+static struct dsa_switch_driver bcm_sf2_switch_ops = {
 	.tag_protocol		= DSA_TAG_PROTO_BRCM,
 	.probe			= bcm_sf2_sw_drv_probe,
 	.setup			= bcm_sf2_sw_setup,
@@ -1400,19 +1307,163 @@ static struct dsa_switch_driver bcm_sf2_switch_driver = {
 	.port_fdb_dump		= bcm_sf2_sw_fdb_dump,
 };
 
-static int __init bcm_sf2_init(void)
+static int bcm_sf2_sw_probe(struct platform_device *pdev)
 {
-	register_switch_driver(&bcm_sf2_switch_driver);
+	const char *reg_names[BCM_SF2_REGS_NUM] = BCM_SF2_REGS_NAME;
+	struct resource *res;
+	struct dsa_switch *ds;
+	struct bcm_sf2_priv *priv;
+	struct device_node *dn = pdev->dev.of_node;
+	void __iomem **base;
+	unsigned int i;
+	u32 reg, rev;
+	int ret;
+
+	ds = devm_kzalloc(&pdev->dev, sizeof(*ds) + sizeof(*priv), GFP_KERNEL);
+	if (!ds)
+		return -ENOMEM;
+
+	priv = (struct bcm_sf2_priv *)(ds + 1);
+
+	ds->priv = priv;
+	ds->dev = &pdev->dev;
+	ds->drv = &bcm_sf2_switch_ops;
+
+	spin_lock_init(&priv->indir_lock);
+	mutex_init(&priv->stats_mutex);
+
+	/* The port information of the switch is in the immediate child
+	 * sub-node
+	 */
+	bcm_sf2_identify_ports(priv, dn->child);
+
+	priv->irq0 = platform_get_irq(pdev, 0);
+	priv->irq1 = platform_get_irq(pdev, 1);
+
+	base = &priv->core;
+	for (i = 0; i < BCM_SF2_REGS_NUM; i++) {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+		*base = devm_ioremap_resource(&pdev->dev, res);
+		if (*base == NULL) {
+			dev_err(&pdev->dev,
+				"unable to find register: %s\n", reg_names[i]);
+			return -ENODEV;
+		}
+		base++;
+	}
+
+	ret = bcm_sf2_sw_rst(priv);
+	if (ret) {
+		pr_err("unable to software reset switch: %d\n", ret);
+		return ret;
+	}
+
+	/* Disable all interrupts and request them */
+	bcm_sf2_intr_disable(priv);
+
+	ret = devm_request_irq(&pdev->dev, priv->irq0, bcm_sf2_switch_0_isr, 0,
+			       "switch_0", priv);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to request switch_0 IRQ\n");
+		return ret;
+	}
+
+	ret = devm_request_irq(&pdev->dev, priv->irq1, bcm_sf2_switch_1_isr, 0,
+			       "switch_1", priv);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to request switch_1 IRQ\n");
+		return ret;
+	}
+
+	/* Reset the MIB counters */
+	reg = core_readl(priv, CORE_GMNCFGCFG);
+	reg |= RST_MIB_CNT;
+	core_writel(priv, reg, CORE_GMNCFGCFG);
+	reg &= ~RST_MIB_CNT;
+	core_writel(priv, reg, CORE_GMNCFGCFG);
+
+	/* Get the maximum number of ports for this switch */
+	priv->hw_params.num_ports = core_readl(priv, CORE_IMP0_PRT_ID) + 1;
+	if (priv->hw_params.num_ports > DSA_MAX_PORTS)
+		priv->hw_params.num_ports = DSA_MAX_PORTS;
+
+	/* Assume a single GPHY setup if we can't read that property */
+	if (of_property_read_u32(dn, "brcm,num-gphy",
+				 &priv->hw_params.num_gphy))
+		priv->hw_params.num_gphy = 1;
+
+	rev = reg_readl(priv, REG_SWITCH_REVISION);
+	priv->hw_params.top_rev = (rev >> SWITCH_TOP_REV_SHIFT) &
+					SWITCH_TOP_REV_MASK;
+	priv->hw_params.core_rev = (rev & SF2_REV_MASK);
+
+	rev = reg_readl(priv, REG_PHY_REVISION);
+	priv->hw_params.gphy_rev = rev & PHY_REVISION_MASK;
+
+	dev_set_drvdata(&pdev->dev, ds);
+
+	ret = dsa_register_switch(ds, pdev->dev.of_node);
+	if (ret)
+		return ret;
+
+	dev_info(&pdev->dev,
+		"Starfighter 2 top: %x.%02x, core: %x.%02x base: 0x%p, IRQs: %d, %d\n",
+		priv->hw_params.top_rev >> 8, priv->hw_params.top_rev & 0xff,
+		priv->hw_params.core_rev >> 8, priv->hw_params.core_rev & 0xff,
+		priv->core, priv->irq0, priv->irq1);
+
+	return 0;
+}
+
+static int bcm_sf2_sw_remove(struct platform_device *pdev)
+{
+	struct dsa_switch *ds = platform_get_drvdata(pdev);
+	struct bcm_sf2_priv *priv = ds_to_priv(ds);
+
+	/* Disable all ports and interrupts */
+	priv->wol_ports_mask = 0;
+	bcm_sf2_sw_suspend(ds);
+	dsa_unregister_switch(ds);
 
 	return 0;
 }
-module_init(bcm_sf2_init);
 
-static void __exit bcm_sf2_exit(void)
+#ifdef CONFIG_PM_SLEEP
+static int bcm_sf2_suspend(struct device *dev)
 {
-	unregister_switch_driver(&bcm_sf2_switch_driver);
+	struct platform_device *pdev = to_platform_device(dev);
+	struct dsa_switch *ds = platform_get_drvdata(pdev);
+
+	return dsa_switch_suspend(ds);
 }
-module_exit(bcm_sf2_exit);
+
+static int bcm_sf2_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct dsa_switch *ds = platform_get_drvdata(pdev);
+
+	return dsa_switch_resume(ds);
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static SIMPLE_DEV_PM_OPS(bcm_sf2_pm_ops,
+			 bcm_sf2_suspend, bcm_sf2_resume);
+
+static const struct of_device_id bcm_sf2_of_match[] = {
+	{ .compatible = "brcm,bcm7445-switch-v4.0" },
+	{ /* sentinel */ },
+};
+
+static struct platform_driver bcm_sf2_driver = {
+	.probe	= bcm_sf2_sw_probe,
+	.remove	= bcm_sf2_sw_remove,
+	.driver = {
+		.name = "brcm-sf2",
+		.of_match_table = bcm_sf2_of_match,
+		.pm = &bcm_sf2_pm_ops,
+	},
+};
+module_platform_driver(bcm_sf2_driver);
 
 MODULE_AUTHOR("Broadcom Corporation");
 MODULE_DESCRIPTION("Driver for Broadcom Starfighter 2 ethernet switch chip");
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index ebc29a1bee08..d0f2b9792f47 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -1037,7 +1037,6 @@ static int dsa_resume(struct device *d)
 static SIMPLE_DEV_PM_OPS(dsa_pm_ops, dsa_suspend, dsa_resume);
 
 static const struct of_device_id dsa_of_match_table[] = {
-	{ .compatible = "brcm,bcm7445-switch-v4.0" },
 	{ .compatible = "marvell,dsa", },
 	{}
 };
-- 
2.7.4

[PATCH net-next 7/9] net: dsa: Initialize CPU port ethtool ops per tree

[Florian Fainelli]

Now that we can properly support multiple distinct trees in the system,
using a global variable: dsa_cpu_port_ethtool_ops is getting clobbered
as soon as the second switch tree gets probed, and we don't want that.

We need to move this to be dynamically allocated, and since we can't
really be comparing addresses anymore to determine first time
initialization versus any other times, just move this to dsa.c and
dsa2.c where the remainder of the dst/ds initialization happens.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/dsa.c      | 28 ++++++++++++++++++++++++++++
 net/dsa/dsa2.c     |  2 +-
 net/dsa/dsa_priv.h |  2 ++
 net/dsa/slave.c    | 10 ----------
 4 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index ebc29a1bee08..8fb8de6fb82d 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -266,6 +266,30 @@ const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol)
 	return ops;
 }
 
+int dsa_cpu_port_ethtool_setup(struct dsa_switch_tree *dst,
+			       struct dsa_switch *ds)
+{
+	struct net_device *master;
+	struct ethtool_ops *cpu_ops;
+
+	master = ds->dst->master_netdev;
+	if (ds->master_netdev)
+		master = ds->master_netdev;
+
+	cpu_ops = devm_kzalloc(ds->dev, sizeof(*cpu_ops), GFP_KERNEL);
+	if (!cpu_ops)
+		return -ENOMEM;
+
+	memcpy(&dst->master_ethtool_ops, master->ethtool_ops,
+	       sizeof(struct ethtool_ops));
+	memcpy(cpu_ops, &dst->master_ethtool_ops,
+	       sizeof(struct ethtool_ops));
+	dsa_cpu_port_ethtool_init(cpu_ops);
+	master->ethtool_ops = cpu_ops;
+
+	return 0;
+}
+
 static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
 {
 	struct dsa_switch_driver *drv = ds->drv;
@@ -379,6 +403,10 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
 		ret = 0;
 	}
 
+	ret = dsa_cpu_port_ethtool_setup(dst, ds);
+	if (ret)
+		return ret;
+
 #ifdef CONFIG_NET_DSA_HWMON
 	/* If the switch provides a temperature sensor,
 	 * register with hardware monitoring subsystem.
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index e8386157de30..938262010524 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -346,7 +346,7 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 			continue;
 	}
 
-	return 0;
+	return dsa_cpu_port_ethtool_setup(dst, ds);
 }
 
 static void dsa_ds_unapply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 106a9f067f94..3bb88b2fb580 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -54,6 +54,8 @@ int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
 		      struct device_node *port_dn, int port);
 void dsa_cpu_dsa_destroy(struct device_node *port_dn);
 const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol);
+int dsa_cpu_port_ethtool_setup(struct dsa_switch_tree *dst,
+			       struct dsa_switch *ds);
 
 /* slave.c */
 extern const struct dsa_device_ops notag_netdev_ops;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 8d159932e082..7236eb26dc97 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -892,8 +892,6 @@ static const struct ethtool_ops dsa_slave_ethtool_ops = {
 	.get_eee		= dsa_slave_get_eee,
 };
 
-static struct ethtool_ops dsa_cpu_port_ethtool_ops;
-
 static const struct net_device_ops dsa_slave_netdev_ops = {
 	.ndo_open	 	= dsa_slave_open,
 	.ndo_stop		= dsa_slave_close,
@@ -1126,14 +1124,6 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
 
 	slave_dev->features = master->vlan_features;
 	slave_dev->ethtool_ops = &dsa_slave_ethtool_ops;
-	if (master->ethtool_ops != &dsa_cpu_port_ethtool_ops) {
-		memcpy(&dst->master_ethtool_ops, master->ethtool_ops,
-		       sizeof(struct ethtool_ops));
-		memcpy(&dsa_cpu_port_ethtool_ops, &dst->master_ethtool_ops,
-		       sizeof(struct ethtool_ops));
-		dsa_cpu_port_ethtool_init(&dsa_cpu_port_ethtool_ops);
-		master->ethtool_ops = &dsa_cpu_port_ethtool_ops;
-	}
 	eth_hw_addr_inherit(slave_dev, master);
 	slave_dev->priv_flags |= IFF_NO_QUEUE;
 	slave_dev->netdev_ops = &dsa_slave_netdev_ops;
-- 
2.7.4

Re: [PATCH net-next 7/9] net: dsa: Initialize CPU port ethtool ops per tree

[Andrew Lunn]

> index e8386157de30..938262010524 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -346,7 +346,7 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
>  			continue;
>  	}
>  
> -	return 0;
> +	return dsa_cpu_port_ethtool_setup(dst, ds);

Hi Florian

This is wrong. Remember, multiple CPU ports. You need to apply this to
each CPU port.

     Andrew

Re: [PATCH net-next 7/9] net: dsa: Initialize CPU port ethtool ops per tree

[Florian Fainelli]

Le 04/06/2016 13:38, Andrew Lunn a écrit :
>> index e8386157de30..938262010524 100644
>> --- a/net/dsa/dsa2.c
>> +++ b/net/dsa/dsa2.c
>> @@ -346,7 +346,7 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
>>  			continue;
>>  	}
>>  
>> -	return 0;
>> +	return dsa_cpu_port_ethtool_setup(dst, ds);
> 
> Hi Florian
> 
> This is wrong. Remember, multiple CPU ports. You need to apply this to
> each CPU port.

We do not quite support that properly though, we still do not create a
"cpu" network device, and there is only a single master netdev per
dsa_switch at the moment, making the secondary CPU port interfaces
impossible to overlay their backing "master" netdev with their companion
switch port statistics at the moment.

This fixes a real bug where we end-up clobbering
dsa_cpu_port_ethtool_ops as soon as there are multiple trees being
registered, and this can now happen.
-- 
Florian

Re: [PATCH net-next 7/9] net: dsa: Initialize CPU port ethtool ops per tree

[Andrew Lunn]

On Sun, Jun 05, 2016 at 03:29:01PM -0700, Florian Fainelli wrote:
> Le 04/06/2016 13:38, Andrew Lunn a écrit :
> >> index e8386157de30..938262010524 100644
> >> --- a/net/dsa/dsa2.c
> >> +++ b/net/dsa/dsa2.c
> >> @@ -346,7 +346,7 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
> >>  			continue;
> >>  	}
> >>  
> >> -	return 0;
> >> +	return dsa_cpu_port_ethtool_setup(dst, ds);
> > 
> > Hi Florian
> > 
> > This is wrong. Remember, multiple CPU ports. You need to apply this to
> > each CPU port.
> 
> We do not quite support that properly though, we still do not create a
> "cpu" network device, and there is only a single master netdev per
> dsa_switch at the moment, making the secondary CPU port interfaces
> impossible to overlay their backing "master" netdev with their companion
> switch port statistics at the moment.

Hi Florian.

Using that argument, you should probably call
dsa_cpu_port_ethtool_setup() in dsa_dst_apply, since that is what
manipulates the master interface to make it handle DSA frames.

	    Andrew

[PATCH net-next 7/7] net: dsa: bcm_sf2: Register our slave MDIO bus

[Florian Fainelli]

Register a slave MDIO bus which allows us to divert problematic
read/writes towards conflicting pseudo-PHY address (30). Do no longer
rely on DSA's slave_mii_bus, but instead provide our own implementation
which offers more flexibility as to what to do, and when to register it.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/bcm_sf2.c | 141 ++++++++++++++++++++++++++++++++--------------
 drivers/net/dsa/bcm_sf2.h |   6 ++
 2 files changed, 106 insertions(+), 41 deletions(-)

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 8dac74a6b5df..502ca65435bf 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -22,6 +22,7 @@
 #include <linux/of_irq.h>
 #include <linux/of_address.h>
 #include <linux/of_net.h>
+#include <linux/of_mdio.h>
 #include <net/dsa.h>
 #include <linux/ethtool.h>
 #include <linux/if_bridge.h>
@@ -948,23 +949,6 @@ static int bcm_sf2_sw_setup(struct dsa_switch *ds)
 			bcm_sf2_port_disable(ds, port, NULL);
 	}
 
-	/* Include the pseudo-PHY address and the broadcast PHY address to
-	 * divert reads towards our workaround. This is only required for
-	 * 7445D0, since 7445E0 disconnects the internal switch pseudo-PHY such
-	 * that we can use the regular SWITCH_MDIO master controller instead.
-	 *
-	 * By default, DSA initializes ds->phys_mii_mask to
-	 * ds->enabled_port_mask to have a 1:1 mapping between Port address
-	 * and PHY address in order to utilize the slave_mii_bus instance to
-	 * read from Port PHYs. This is not what we want here, so we
-	 * initialize phys_mii_mask 0 to always utilize the "master" MDIO
-	 * bus backed by the "mdio-unimac" driver.
-	 */
-	if (of_machine_is_compatible("brcm,bcm7445d0"))
-		ds->phys_mii_mask |= ((1 << BRCM_PSEUDO_PHY_ADDR) | (1 << 0));
-	else
-		ds->phys_mii_mask = 0;
-
 	return 0;
 }
 
@@ -985,10 +969,9 @@ static u32 bcm_sf2_sw_get_phy_flags(struct dsa_switch *ds, int port)
 	return priv->hw_params.gphy_rev;
 }
 
-static int bcm_sf2_sw_indir_rw(struct dsa_switch *ds, int op, int addr,
+static int bcm_sf2_sw_indir_rw(struct bcm_sf2_priv *priv, int op, int addr,
 			       int regnum, u16 val)
 {
-	struct bcm_sf2_priv *priv = ds_to_priv(ds);
 	int ret = 0;
 	u32 reg;
 
@@ -1017,32 +1000,31 @@ static int bcm_sf2_sw_indir_rw(struct dsa_switch *ds, int op, int addr,
 	return ret & 0xffff;
 }
 
-static int bcm_sf2_sw_phy_read(struct dsa_switch *ds, int addr, int regnum)
+static int bcm_sf2_sw_phy_read(struct mii_bus *bus, int addr, int regnum)
 {
-	/* Intercept reads from the MDIO broadcast address or Broadcom
-	 * pseudo-PHY address
+	struct bcm_sf2_priv *priv = bus->priv;
+
+	/* Intercept reads from Broadcom pseudo-PHY address, else, send
+	 * them to our master MDIO bus controller
 	 */
-	switch (addr) {
-	case 0:
-	case BRCM_PSEUDO_PHY_ADDR:
-		return bcm_sf2_sw_indir_rw(ds, 1, addr, regnum, 0);
-	default:
-		return 0xffff;
-	}
+	if (addr == BRCM_PSEUDO_PHY_ADDR && priv->indir_phy_mask & BIT(addr))
+		return bcm_sf2_sw_indir_rw(priv, 1, addr, regnum, 0);
+	else
+		return mdiobus_read(priv->master_mii_bus, addr, regnum);
 }
 
-static int bcm_sf2_sw_phy_write(struct dsa_switch *ds, int addr, int regnum,
+static int bcm_sf2_sw_phy_write(struct mii_bus *bus, int addr, int regnum,
 				u16 val)
 {
-	/* Intercept writes to the MDIO broadcast address or Broadcom
-	 * pseudo-PHY address
+	struct bcm_sf2_priv *priv = bus->priv;
+
+	/* Intercept writes to the Broadcom pseudo-PHY address, else,
+	 * send them to our master MDIO bus controller
 	 */
-	switch (addr) {
-	case 0:
-	case BRCM_PSEUDO_PHY_ADDR:
-		bcm_sf2_sw_indir_rw(ds, 0, addr, regnum, val);
-		break;
-	}
+	if (addr == BRCM_PSEUDO_PHY_ADDR && priv->indir_phy_mask & BIT(addr))
+		bcm_sf2_sw_indir_rw(priv, 0, addr, regnum, val);
+	else
+		mdiobus_write(priv->master_mii_bus, addr, regnum, val);
 
 	return 0;
 }
@@ -1283,8 +1265,6 @@ static struct dsa_switch_driver bcm_sf2_switch_ops = {
 	.setup			= bcm_sf2_sw_setup,
 	.set_addr		= bcm_sf2_sw_set_addr,
 	.get_phy_flags		= bcm_sf2_sw_get_phy_flags,
-	.phy_read		= bcm_sf2_sw_phy_read,
-	.phy_write		= bcm_sf2_sw_phy_write,
 	.get_strings		= bcm_sf2_sw_get_strings,
 	.get_ethtool_stats	= bcm_sf2_sw_get_ethtool_stats,
 	.get_sset_count		= bcm_sf2_sw_get_sset_count,
@@ -1307,6 +1287,75 @@ static struct dsa_switch_driver bcm_sf2_switch_ops = {
 	.port_fdb_dump		= bcm_sf2_sw_fdb_dump,
 };
 
+static int bcm_sf2_mdio_register(struct dsa_switch *ds)
+{
+	struct bcm_sf2_priv *priv = ds_to_priv(ds);
+	struct device_node *dn;
+	static int index;
+	int err;
+
+	/* Find our integratd MDIO bus node */
+	dn = of_find_compatible_node(NULL, NULL, "brcm,unimac-mdio");
+	priv->master_mii_bus = of_mdio_find_bus(dn);
+	if (!priv->master_mii_bus)
+		return -EPROBE_DEFER;
+
+	get_device(&priv->master_mii_bus->dev);
+	priv->master_mii_dn = dn;
+
+	priv->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
+	if (!priv->slave_mii_bus)
+		return -ENOMEM;
+
+	priv->slave_mii_bus->priv = priv;
+	priv->slave_mii_bus->name = "sf2 slave mii";
+	priv->slave_mii_bus->read = bcm_sf2_sw_phy_read;
+	priv->slave_mii_bus->write = bcm_sf2_sw_phy_write;
+	if (dn)
+		snprintf(priv->slave_mii_bus->id, MII_BUS_ID_SIZE, "%s",
+			 ds->dev->of_node->full_name);
+	else
+		snprintf(priv->slave_mii_bus->id, MII_BUS_ID_SIZE, "sf2-%d",
+			 index++);
+	priv->slave_mii_bus->dev.of_node = dn;
+
+	/* Include the pseudo-PHY address to divert reads towards our
+	 * workaround. This is only required for 7445D0, since 7445E0
+	 * disconnects the internal switch pseudo-PHY such that we can use the
+	 * regular SWITCH_MDIO master controller instead.
+	 *
+	 * Here we flag the pseudo PHY as needing special treatment and would
+	 * otherwise make all other PHY read/writes go to the master MDIO bus
+	 * controller that comes with this switch backed by the "mdio-unimac"
+	 * driver.
+	 */
+	if (of_machine_is_compatible("brcm,bcm7445d0"))
+		priv->indir_phy_mask |= (1 << BRCM_PSEUDO_PHY_ADDR);
+	else
+		priv->indir_phy_mask = 0;
+	priv->slave_mii_bus->parent = ds->dev;
+	priv->slave_mii_bus->phy_mask = ~priv->indir_phy_mask;
+
+	if (dn)
+		err = of_mdiobus_register(priv->slave_mii_bus, dn);
+	else
+		err = mdiobus_register(priv->slave_mii_bus);
+
+	if (err)
+		of_node_put(dn);
+
+	return err;
+}
+
+static void bcm_sf2_mdio_unregister(struct dsa_switch *ds)
+{
+	struct bcm_sf2_priv *priv = ds_to_priv(ds);
+
+	mdiobus_unregister(priv->slave_mii_bus);
+	if (priv->master_mii_dn)
+		of_node_put(priv->master_mii_dn);
+}
+
 static int bcm_sf2_sw_probe(struct platform_device *pdev)
 {
 	const char *reg_names[BCM_SF2_REGS_NUM] = BCM_SF2_REGS_NAME;
@@ -1402,10 +1451,18 @@ static int bcm_sf2_sw_probe(struct platform_device *pdev)
 
 	dev_set_drvdata(&pdev->dev, ds);
 
-	ret = dsa_register_switch(ds, pdev->dev.of_node);
+	ret = bcm_sf2_mdio_register(ds);
 	if (ret)
 		return ret;
 
+	ds->slave_mii_bus = priv->slave_mii_bus;
+
+	ret = dsa_register_switch(ds, pdev->dev.of_node);
+	if (ret) {
+		bcm_sf2_mdio_unregister(ds);
+		return ret;
+	}
+
 	dev_info(&pdev->dev,
 		"Starfighter 2 top: %x.%02x, core: %x.%02x base: 0x%p, IRQs: %d, %d\n",
 		priv->hw_params.top_rev >> 8, priv->hw_params.top_rev & 0xff,
@@ -1424,6 +1481,8 @@ static int bcm_sf2_sw_remove(struct platform_device *pdev)
 	priv->wol_ports_mask = 0;
 	bcm_sf2_sw_suspend(ds);
 	dsa_unregister_switch(ds);
+	put_device(&priv->master_mii_bus->dev);
+	bcm_sf2_mdio_unregister(ds);
 
 	return 0;
 }
diff --git a/drivers/net/dsa/bcm_sf2.h b/drivers/net/dsa/bcm_sf2.h
index 200b1f5fdb56..bde11ebb2742 100644
--- a/drivers/net/dsa/bcm_sf2.h
+++ b/drivers/net/dsa/bcm_sf2.h
@@ -142,6 +142,12 @@ struct bcm_sf2_priv {
 
 	/* Bitmask of ports having an integrated PHY */
 	unsigned int			int_phy_mask;
+
+	/* Master and slave MDIO bus controller */
+	unsigned int			indir_phy_mask;
+	struct device_node		*master_mii_dn;
+	struct mii_bus			*slave_mii_bus;
+	struct mii_bus			*master_mii_bus;
 };
 
 struct bcm_sf2_hw_stats {
-- 
2.7.4

Re: [PATCH net-next 7/7] net: dsa: bcm_sf2: Register our slave MDIO bus

[Andrew Lunn]

> -static int bcm_sf2_sw_phy_read(struct dsa_switch *ds, int addr, int regnum)
> +static int bcm_sf2_sw_phy_read(struct mii_bus *bus, int addr, int regnum)

Hi Florian

When i did the same for mv88e6xxx, i renamed _phy_ to _mdio_. That
keeps with the general pattern for other mdio drivers, making it clear
there can be something else than a phy on the bus.

> +	priv->slave_mii_bus->priv = priv;
> +	priv->slave_mii_bus->name = "sf2 slave mii";

> +	if (dn)
> +		snprintf(priv->slave_mii_bus->id, MII_BUS_ID_SIZE, "%s",
> +			 ds->dev->of_node->full_name);

I tried lots of different things with mv88e6xxx, including i think
this. The problem is full_name can be quite long, so it gets
truncated. And i often found two different mv88e6xxx ended up with
identical truncated names, since the unique part was getting
discarded.

> +	else
> +		snprintf(priv->slave_mii_bus->id, MII_BUS_ID_SIZE, "sf2-%d",
> +			 index++);

So i ended up with this, which is 100% reliable. 

   Andrew

[PATCH net-next 8/9] net: dsa: bcm_sf2: Make it a real platform device driver

[Florian Fainelli]

The Broadcom Starfighter 2 switch driver should be a proper platform
driver, now that the DSA code has been updated to allow that, register a
switch device, feed it with the proper configuration data coming from
Device Tree and register our switch device with DSA.

The bulk of the changes consist in moving what bcm_sf2_sw_setup() did
into the platform driver probe function.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/bcm_sf2.c | 251 ++++++++++++++++++++++++++++------------------
 net/dsa/dsa.c             |   1 -
 2 files changed, 151 insertions(+), 101 deletions(-)

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 73df91bb0466..8dac74a6b5df 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -934,77 +934,8 @@ static void bcm_sf2_identify_ports(struct bcm_sf2_priv *priv,
 
 static int bcm_sf2_sw_setup(struct dsa_switch *ds)
 {
-	const char *reg_names[BCM_SF2_REGS_NUM] = BCM_SF2_REGS_NAME;
 	struct bcm_sf2_priv *priv = ds_to_priv(ds);
-	struct device_node *dn;
-	void __iomem **base;
 	unsigned int port;
-	unsigned int i;
-	u32 reg, rev;
-	int ret;
-
-	spin_lock_init(&priv->indir_lock);
-	mutex_init(&priv->stats_mutex);
-
-	/* All the interesting properties are at the parent device_node
-	 * level
-	 */
-	dn = ds->cd->of_node->parent;
-	bcm_sf2_identify_ports(priv, ds->cd->of_node);
-
-	priv->irq0 = irq_of_parse_and_map(dn, 0);
-	priv->irq1 = irq_of_parse_and_map(dn, 1);
-
-	base = &priv->core;
-	for (i = 0; i < BCM_SF2_REGS_NUM; i++) {
-		*base = of_iomap(dn, i);
-		if (*base == NULL) {
-			pr_err("unable to find register: %s\n", reg_names[i]);
-			ret = -ENOMEM;
-			goto out_unmap;
-		}
-		base++;
-	}
-
-	ret = bcm_sf2_sw_rst(priv);
-	if (ret) {
-		pr_err("unable to software reset switch: %d\n", ret);
-		goto out_unmap;
-	}
-
-	/* Disable all interrupts and request them */
-	bcm_sf2_intr_disable(priv);
-
-	ret = request_irq(priv->irq0, bcm_sf2_switch_0_isr, 0,
-			  "switch_0", priv);
-	if (ret < 0) {
-		pr_err("failed to request switch_0 IRQ\n");
-		goto out_unmap;
-	}
-
-	ret = request_irq(priv->irq1, bcm_sf2_switch_1_isr, 0,
-			  "switch_1", priv);
-	if (ret < 0) {
-		pr_err("failed to request switch_1 IRQ\n");
-		goto out_free_irq0;
-	}
-
-	/* Reset the MIB counters */
-	reg = core_readl(priv, CORE_GMNCFGCFG);
-	reg |= RST_MIB_CNT;
-	core_writel(priv, reg, CORE_GMNCFGCFG);
-	reg &= ~RST_MIB_CNT;
-	core_writel(priv, reg, CORE_GMNCFGCFG);
-
-	/* Get the maximum number of ports for this switch */
-	priv->hw_params.num_ports = core_readl(priv, CORE_IMP0_PRT_ID) + 1;
-	if (priv->hw_params.num_ports > DSA_MAX_PORTS)
-		priv->hw_params.num_ports = DSA_MAX_PORTS;
-
-	/* Assume a single GPHY setup if we can't read that property */
-	if (of_property_read_u32(dn, "brcm,num-gphy",
-				 &priv->hw_params.num_gphy))
-		priv->hw_params.num_gphy = 1;
 
 	/* Enable all valid ports and disable those unused */
 	for (port = 0; port < priv->hw_params.num_ports; port++) {
@@ -1034,31 +965,7 @@ static int bcm_sf2_sw_setup(struct dsa_switch *ds)
 	else
 		ds->phys_mii_mask = 0;
 
-	rev = reg_readl(priv, REG_SWITCH_REVISION);
-	priv->hw_params.top_rev = (rev >> SWITCH_TOP_REV_SHIFT) &
-					SWITCH_TOP_REV_MASK;
-	priv->hw_params.core_rev = (rev & SF2_REV_MASK);
-
-	rev = reg_readl(priv, REG_PHY_REVISION);
-	priv->hw_params.gphy_rev = rev & PHY_REVISION_MASK;
-
-	pr_info("Starfighter 2 top: %x.%02x, core: %x.%02x base: 0x%p, IRQs: %d, %d\n",
-		priv->hw_params.top_rev >> 8, priv->hw_params.top_rev & 0xff,
-		priv->hw_params.core_rev >> 8, priv->hw_params.core_rev & 0xff,
-		priv->core, priv->irq0, priv->irq1);
-
 	return 0;
-
-out_free_irq0:
-	free_irq(priv->irq0, priv);
-out_unmap:
-	base = &priv->core;
-	for (i = 0; i < BCM_SF2_REGS_NUM; i++) {
-		if (*base)
-			iounmap(*base);
-		base++;
-	}
-	return ret;
 }
 
 static int bcm_sf2_sw_set_addr(struct dsa_switch *ds, u8 *addr)
@@ -1370,7 +1277,7 @@ static int bcm_sf2_sw_set_wol(struct dsa_switch *ds, int port,
 	return p->ethtool_ops->set_wol(p, wol);
 }
 
-static struct dsa_switch_driver bcm_sf2_switch_driver = {
+static struct dsa_switch_driver bcm_sf2_switch_ops = {
 	.tag_protocol		= DSA_TAG_PROTO_BRCM,
 	.probe			= bcm_sf2_sw_drv_probe,
 	.setup			= bcm_sf2_sw_setup,
@@ -1400,19 +1307,163 @@ static struct dsa_switch_driver bcm_sf2_switch_driver = {
 	.port_fdb_dump		= bcm_sf2_sw_fdb_dump,
 };
 
-static int __init bcm_sf2_init(void)
+static int bcm_sf2_sw_probe(struct platform_device *pdev)
 {
-	register_switch_driver(&bcm_sf2_switch_driver);
+	const char *reg_names[BCM_SF2_REGS_NUM] = BCM_SF2_REGS_NAME;
+	struct resource *res;
+	struct dsa_switch *ds;
+	struct bcm_sf2_priv *priv;
+	struct device_node *dn = pdev->dev.of_node;
+	void __iomem **base;
+	unsigned int i;
+	u32 reg, rev;
+	int ret;
+
+	ds = devm_kzalloc(&pdev->dev, sizeof(*ds) + sizeof(*priv), GFP_KERNEL);
+	if (!ds)
+		return -ENOMEM;
+
+	priv = (struct bcm_sf2_priv *)(ds + 1);
+
+	ds->priv = priv;
+	ds->dev = &pdev->dev;
+	ds->drv = &bcm_sf2_switch_ops;
+
+	spin_lock_init(&priv->indir_lock);
+	mutex_init(&priv->stats_mutex);
+
+	/* The port information of the switch is in the immediate child
+	 * sub-node
+	 */
+	bcm_sf2_identify_ports(priv, dn->child);
+
+	priv->irq0 = platform_get_irq(pdev, 0);
+	priv->irq1 = platform_get_irq(pdev, 1);
+
+	base = &priv->core;
+	for (i = 0; i < BCM_SF2_REGS_NUM; i++) {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+		*base = devm_ioremap_resource(&pdev->dev, res);
+		if (*base == NULL) {
+			dev_err(&pdev->dev,
+				"unable to find register: %s\n", reg_names[i]);
+			return -ENODEV;
+		}
+		base++;
+	}
+
+	ret = bcm_sf2_sw_rst(priv);
+	if (ret) {
+		pr_err("unable to software reset switch: %d\n", ret);
+		return ret;
+	}
+
+	/* Disable all interrupts and request them */
+	bcm_sf2_intr_disable(priv);
+
+	ret = devm_request_irq(&pdev->dev, priv->irq0, bcm_sf2_switch_0_isr, 0,
+			       "switch_0", priv);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to request switch_0 IRQ\n");
+		return ret;
+	}
+
+	ret = devm_request_irq(&pdev->dev, priv->irq1, bcm_sf2_switch_1_isr, 0,
+			       "switch_1", priv);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to request switch_1 IRQ\n");
+		return ret;
+	}
+
+	/* Reset the MIB counters */
+	reg = core_readl(priv, CORE_GMNCFGCFG);
+	reg |= RST_MIB_CNT;
+	core_writel(priv, reg, CORE_GMNCFGCFG);
+	reg &= ~RST_MIB_CNT;
+	core_writel(priv, reg, CORE_GMNCFGCFG);
+
+	/* Get the maximum number of ports for this switch */
+	priv->hw_params.num_ports = core_readl(priv, CORE_IMP0_PRT_ID) + 1;
+	if (priv->hw_params.num_ports > DSA_MAX_PORTS)
+		priv->hw_params.num_ports = DSA_MAX_PORTS;
+
+	/* Assume a single GPHY setup if we can't read that property */
+	if (of_property_read_u32(dn, "brcm,num-gphy",
+				 &priv->hw_params.num_gphy))
+		priv->hw_params.num_gphy = 1;
+
+	rev = reg_readl(priv, REG_SWITCH_REVISION);
+	priv->hw_params.top_rev = (rev >> SWITCH_TOP_REV_SHIFT) &
+					SWITCH_TOP_REV_MASK;
+	priv->hw_params.core_rev = (rev & SF2_REV_MASK);
+
+	rev = reg_readl(priv, REG_PHY_REVISION);
+	priv->hw_params.gphy_rev = rev & PHY_REVISION_MASK;
+
+	dev_set_drvdata(&pdev->dev, ds);
+
+	ret = dsa_register_switch(ds, pdev->dev.of_node);
+	if (ret)
+		return ret;
+
+	dev_info(&pdev->dev,
+		"Starfighter 2 top: %x.%02x, core: %x.%02x base: 0x%p, IRQs: %d, %d\n",
+		priv->hw_params.top_rev >> 8, priv->hw_params.top_rev & 0xff,
+		priv->hw_params.core_rev >> 8, priv->hw_params.core_rev & 0xff,
+		priv->core, priv->irq0, priv->irq1);
+
+	return 0;
+}
+
+static int bcm_sf2_sw_remove(struct platform_device *pdev)
+{
+	struct dsa_switch *ds = platform_get_drvdata(pdev);
+	struct bcm_sf2_priv *priv = ds_to_priv(ds);
+
+	/* Disable all ports and interrupts */
+	priv->wol_ports_mask = 0;
+	bcm_sf2_sw_suspend(ds);
+	dsa_unregister_switch(ds);
 
 	return 0;
 }
-module_init(bcm_sf2_init);
 
-static void __exit bcm_sf2_exit(void)
+#ifdef CONFIG_PM_SLEEP
+static int bcm_sf2_suspend(struct device *dev)
 {
-	unregister_switch_driver(&bcm_sf2_switch_driver);
+	struct platform_device *pdev = to_platform_device(dev);
+	struct dsa_switch *ds = platform_get_drvdata(pdev);
+
+	return dsa_switch_suspend(ds);
 }
-module_exit(bcm_sf2_exit);
+
+static int bcm_sf2_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct dsa_switch *ds = platform_get_drvdata(pdev);
+
+	return dsa_switch_resume(ds);
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static SIMPLE_DEV_PM_OPS(bcm_sf2_pm_ops,
+			 bcm_sf2_suspend, bcm_sf2_resume);
+
+static const struct of_device_id bcm_sf2_of_match[] = {
+	{ .compatible = "brcm,bcm7445-switch-v4.0" },
+	{ /* sentinel */ },
+};
+
+static struct platform_driver bcm_sf2_driver = {
+	.probe	= bcm_sf2_sw_probe,
+	.remove	= bcm_sf2_sw_remove,
+	.driver = {
+		.name = "brcm-sf2",
+		.of_match_table = bcm_sf2_of_match,
+		.pm = &bcm_sf2_pm_ops,
+	},
+};
+module_platform_driver(bcm_sf2_driver);
 
 MODULE_AUTHOR("Broadcom Corporation");
 MODULE_DESCRIPTION("Driver for Broadcom Starfighter 2 ethernet switch chip");
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 8fb8de6fb82d..7abe0cd3af9c 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -1065,7 +1065,6 @@ static int dsa_resume(struct device *d)
 static SIMPLE_DEV_PM_OPS(dsa_pm_ops, dsa_suspend, dsa_resume);
 
 static const struct of_device_id dsa_of_match_table[] = {
-	{ .compatible = "brcm,bcm7445-switch-v4.0" },
 	{ .compatible = "marvell,dsa", },
 	{}
 };
-- 
2.7.4

Re: [PATCH net-next 8/9] net: dsa: bcm_sf2: Make it a real platform device driver

[Andrew Lunn]

> -static struct dsa_switch_driver bcm_sf2_switch_driver = {
> +static struct dsa_switch_driver bcm_sf2_switch_ops = {
>  	.tag_protocol		= DSA_TAG_PROTO_BRCM,
>  	.probe			= bcm_sf2_sw_drv_probe,
>  	.setup			= bcm_sf2_sw_setup,

Does this also allow you to kill bcm_sf2_sw_drv_probe?

dsa2 will never call it, and now you have your old binding parsed by
the legacy code in dsa2, you might not need it at all.

     Andrew

Re: [PATCH net-next 8/9] net: dsa: bcm_sf2: Make it a real platform device driver

[Florian Fainelli]

Le 04/06/2016 13:55, Andrew Lunn a écrit :
>> -static struct dsa_switch_driver bcm_sf2_switch_driver = {
>> +static struct dsa_switch_driver bcm_sf2_switch_ops = {
>>  	.tag_protocol		= DSA_TAG_PROTO_BRCM,
>>  	.probe			= bcm_sf2_sw_drv_probe,
>>  	.setup			= bcm_sf2_sw_setup,
> 
> Does this also allow you to kill bcm_sf2_sw_drv_probe?
> 
> dsa2 will never call it, and now you have your old binding parsed by
> the legacy code in dsa2, you might not need it at all.

Good point, yes, that can go away now, thanks!
-- 
Florian

[PATCH net-next 9/9] net: dsa: bcm_sf2: Register our slave MDIO bus

[Florian Fainelli]

Register a slave MDIO bus which allows us to divert problematic
read/writes towards conflicting pseudo-PHY address (30). Do no longer
rely on DSA's slave_mii_bus, but instead provide our own implementation
which offers more flexibility as to what to do, and when to register it.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/bcm_sf2.c | 141 ++++++++++++++++++++++++++++++++--------------
 drivers/net/dsa/bcm_sf2.h |   6 ++
 2 files changed, 106 insertions(+), 41 deletions(-)

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 8dac74a6b5df..502ca65435bf 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -22,6 +22,7 @@
 #include <linux/of_irq.h>
 #include <linux/of_address.h>
 #include <linux/of_net.h>
+#include <linux/of_mdio.h>
 #include <net/dsa.h>
 #include <linux/ethtool.h>
 #include <linux/if_bridge.h>
@@ -948,23 +949,6 @@ static int bcm_sf2_sw_setup(struct dsa_switch *ds)
 			bcm_sf2_port_disable(ds, port, NULL);
 	}
 
-	/* Include the pseudo-PHY address and the broadcast PHY address to
-	 * divert reads towards our workaround. This is only required for
-	 * 7445D0, since 7445E0 disconnects the internal switch pseudo-PHY such
-	 * that we can use the regular SWITCH_MDIO master controller instead.
-	 *
-	 * By default, DSA initializes ds->phys_mii_mask to
-	 * ds->enabled_port_mask to have a 1:1 mapping between Port address
-	 * and PHY address in order to utilize the slave_mii_bus instance to
-	 * read from Port PHYs. This is not what we want here, so we
-	 * initialize phys_mii_mask 0 to always utilize the "master" MDIO
-	 * bus backed by the "mdio-unimac" driver.
-	 */
-	if (of_machine_is_compatible("brcm,bcm7445d0"))
-		ds->phys_mii_mask |= ((1 << BRCM_PSEUDO_PHY_ADDR) | (1 << 0));
-	else
-		ds->phys_mii_mask = 0;
-
 	return 0;
 }
 
@@ -985,10 +969,9 @@ static u32 bcm_sf2_sw_get_phy_flags(struct dsa_switch *ds, int port)
 	return priv->hw_params.gphy_rev;
 }
 
-static int bcm_sf2_sw_indir_rw(struct dsa_switch *ds, int op, int addr,
+static int bcm_sf2_sw_indir_rw(struct bcm_sf2_priv *priv, int op, int addr,
 			       int regnum, u16 val)
 {
-	struct bcm_sf2_priv *priv = ds_to_priv(ds);
 	int ret = 0;
 	u32 reg;
 
@@ -1017,32 +1000,31 @@ static int bcm_sf2_sw_indir_rw(struct dsa_switch *ds, int op, int addr,
 	return ret & 0xffff;
 }
 
-static int bcm_sf2_sw_phy_read(struct dsa_switch *ds, int addr, int regnum)
+static int bcm_sf2_sw_phy_read(struct mii_bus *bus, int addr, int regnum)
 {
-	/* Intercept reads from the MDIO broadcast address or Broadcom
-	 * pseudo-PHY address
+	struct bcm_sf2_priv *priv = bus->priv;
+
+	/* Intercept reads from Broadcom pseudo-PHY address, else, send
+	 * them to our master MDIO bus controller
 	 */
-	switch (addr) {
-	case 0:
-	case BRCM_PSEUDO_PHY_ADDR:
-		return bcm_sf2_sw_indir_rw(ds, 1, addr, regnum, 0);
-	default:
-		return 0xffff;
-	}
+	if (addr == BRCM_PSEUDO_PHY_ADDR && priv->indir_phy_mask & BIT(addr))
+		return bcm_sf2_sw_indir_rw(priv, 1, addr, regnum, 0);
+	else
+		return mdiobus_read(priv->master_mii_bus, addr, regnum);
 }
 
-static int bcm_sf2_sw_phy_write(struct dsa_switch *ds, int addr, int regnum,
+static int bcm_sf2_sw_phy_write(struct mii_bus *bus, int addr, int regnum,
 				u16 val)
 {
-	/* Intercept writes to the MDIO broadcast address or Broadcom
-	 * pseudo-PHY address
+	struct bcm_sf2_priv *priv = bus->priv;
+
+	/* Intercept writes to the Broadcom pseudo-PHY address, else,
+	 * send them to our master MDIO bus controller
 	 */
-	switch (addr) {
-	case 0:
-	case BRCM_PSEUDO_PHY_ADDR:
-		bcm_sf2_sw_indir_rw(ds, 0, addr, regnum, val);
-		break;
-	}
+	if (addr == BRCM_PSEUDO_PHY_ADDR && priv->indir_phy_mask & BIT(addr))
+		bcm_sf2_sw_indir_rw(priv, 0, addr, regnum, val);
+	else
+		mdiobus_write(priv->master_mii_bus, addr, regnum, val);
 
 	return 0;
 }
@@ -1283,8 +1265,6 @@ static struct dsa_switch_driver bcm_sf2_switch_ops = {
 	.setup			= bcm_sf2_sw_setup,
 	.set_addr		= bcm_sf2_sw_set_addr,
 	.get_phy_flags		= bcm_sf2_sw_get_phy_flags,
-	.phy_read		= bcm_sf2_sw_phy_read,
-	.phy_write		= bcm_sf2_sw_phy_write,
 	.get_strings		= bcm_sf2_sw_get_strings,
 	.get_ethtool_stats	= bcm_sf2_sw_get_ethtool_stats,
 	.get_sset_count		= bcm_sf2_sw_get_sset_count,
@@ -1307,6 +1287,75 @@ static struct dsa_switch_driver bcm_sf2_switch_ops = {
 	.port_fdb_dump		= bcm_sf2_sw_fdb_dump,
 };
 
+static int bcm_sf2_mdio_register(struct dsa_switch *ds)
+{
+	struct bcm_sf2_priv *priv = ds_to_priv(ds);
+	struct device_node *dn;
+	static int index;
+	int err;
+
+	/* Find our integratd MDIO bus node */
+	dn = of_find_compatible_node(NULL, NULL, "brcm,unimac-mdio");
+	priv->master_mii_bus = of_mdio_find_bus(dn);
+	if (!priv->master_mii_bus)
+		return -EPROBE_DEFER;
+
+	get_device(&priv->master_mii_bus->dev);
+	priv->master_mii_dn = dn;
+
+	priv->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
+	if (!priv->slave_mii_bus)
+		return -ENOMEM;
+
+	priv->slave_mii_bus->priv = priv;
+	priv->slave_mii_bus->name = "sf2 slave mii";
+	priv->slave_mii_bus->read = bcm_sf2_sw_phy_read;
+	priv->slave_mii_bus->write = bcm_sf2_sw_phy_write;
+	if (dn)
+		snprintf(priv->slave_mii_bus->id, MII_BUS_ID_SIZE, "%s",
+			 ds->dev->of_node->full_name);
+	else
+		snprintf(priv->slave_mii_bus->id, MII_BUS_ID_SIZE, "sf2-%d",
+			 index++);
+	priv->slave_mii_bus->dev.of_node = dn;
+
+	/* Include the pseudo-PHY address to divert reads towards our
+	 * workaround. This is only required for 7445D0, since 7445E0
+	 * disconnects the internal switch pseudo-PHY such that we can use the
+	 * regular SWITCH_MDIO master controller instead.
+	 *
+	 * Here we flag the pseudo PHY as needing special treatment and would
+	 * otherwise make all other PHY read/writes go to the master MDIO bus
+	 * controller that comes with this switch backed by the "mdio-unimac"
+	 * driver.
+	 */
+	if (of_machine_is_compatible("brcm,bcm7445d0"))
+		priv->indir_phy_mask |= (1 << BRCM_PSEUDO_PHY_ADDR);
+	else
+		priv->indir_phy_mask = 0;
+	priv->slave_mii_bus->parent = ds->dev;
+	priv->slave_mii_bus->phy_mask = ~priv->indir_phy_mask;
+
+	if (dn)
+		err = of_mdiobus_register(priv->slave_mii_bus, dn);
+	else
+		err = mdiobus_register(priv->slave_mii_bus);
+
+	if (err)
+		of_node_put(dn);
+
+	return err;
+}
+
+static void bcm_sf2_mdio_unregister(struct dsa_switch *ds)
+{
+	struct bcm_sf2_priv *priv = ds_to_priv(ds);
+
+	mdiobus_unregister(priv->slave_mii_bus);
+	if (priv->master_mii_dn)
+		of_node_put(priv->master_mii_dn);
+}
+
 static int bcm_sf2_sw_probe(struct platform_device *pdev)
 {
 	const char *reg_names[BCM_SF2_REGS_NUM] = BCM_SF2_REGS_NAME;
@@ -1402,10 +1451,18 @@ static int bcm_sf2_sw_probe(struct platform_device *pdev)
 
 	dev_set_drvdata(&pdev->dev, ds);
 
-	ret = dsa_register_switch(ds, pdev->dev.of_node);
+	ret = bcm_sf2_mdio_register(ds);
 	if (ret)
 		return ret;
 
+	ds->slave_mii_bus = priv->slave_mii_bus;
+
+	ret = dsa_register_switch(ds, pdev->dev.of_node);
+	if (ret) {
+		bcm_sf2_mdio_unregister(ds);
+		return ret;
+	}
+
 	dev_info(&pdev->dev,
 		"Starfighter 2 top: %x.%02x, core: %x.%02x base: 0x%p, IRQs: %d, %d\n",
 		priv->hw_params.top_rev >> 8, priv->hw_params.top_rev & 0xff,
@@ -1424,6 +1481,8 @@ static int bcm_sf2_sw_remove(struct platform_device *pdev)
 	priv->wol_ports_mask = 0;
 	bcm_sf2_sw_suspend(ds);
 	dsa_unregister_switch(ds);
+	put_device(&priv->master_mii_bus->dev);
+	bcm_sf2_mdio_unregister(ds);
 
 	return 0;
 }
diff --git a/drivers/net/dsa/bcm_sf2.h b/drivers/net/dsa/bcm_sf2.h
index 200b1f5fdb56..bde11ebb2742 100644
--- a/drivers/net/dsa/bcm_sf2.h
+++ b/drivers/net/dsa/bcm_sf2.h
@@ -142,6 +142,12 @@ struct bcm_sf2_priv {
 
 	/* Bitmask of ports having an integrated PHY */
 	unsigned int			int_phy_mask;
+
+	/* Master and slave MDIO bus controller */
+	unsigned int			indir_phy_mask;
+	struct device_node		*master_mii_dn;
+	struct mii_bus			*slave_mii_bus;
+	struct mii_bus			*master_mii_bus;
 };
 
 struct bcm_sf2_hw_stats {
-- 
2.7.4

Re: [PATCH net-next 0/9] net: dsa: misc improvements

[Florian Fainelli]

On 06/03/2016 05:05 PM, Florian Fainelli wrote:
> Hi all,
> 
> This patch series builds on top of Andrew's "New DSA bind, switches as devices"
> patch set and does the following:
> 
> - add support for the old DSA binding with the new dsa_register_switch() API
>   which is needed by some platforms where the Device Tree is pretty much frozen
> 
> - add a few helper functions/goodies for net/dsa/dsa2.c to be as close as possible
>   from net/dsa/dsa.c in terms of what drivers can expect, in particular the slave
>   MDIO bus and the enabled_port_mask and phy_mii_mask
> 
> - fix the CPU port ethtools ops to work in a multiple tree setup since we can
>   no longer assume a single tree is supported
> 
> - finally conver the bcm_sf2 driver to be a true platform device driver and
>   slightly rework its internal vs. external MDIO bus indirect read/writes to
>   be cleaner

David, I cleaned up my mess in patchwork to leave just the relevant
patches for you to review and apply, sorry about that, it's Friday.
-- 
Florian

[PATCH net-next] net: dsa: Provide CPU port statistics to master netdev

[Florian Fainelli]

This patch overloads the DSA master netdev, aka CPU Ethernet MAC to also
include switch-side statistics, which is useful for debugging purposes,
when the switch is not properly connected to the Ethernet MAC (duplex
mismatch, (RG)MII electrical issues etc.).

We accomplish this by retaining the original copy of the master netdev's
ethtool_ops, and just overload the 3 operations we care about:
get_sset_count, get_strings and get_ethtool_stats so as to intercept
these calls and call into the original master_netdev ethtool_ops, plus
our own.

We take this approach as opposed to providing a set of DSA helper
functions that would retrive the CPU port's statistics, because the
entire purpose of DSA is to allow unmodified Ethernet MAC drivers to be
used as CPU conduit interfaces, therefore, statistics overlay in such
drivers would simply not scale.

The new ethtool -S <iface> output would therefore look like this now:
<iface> statistics
p<2 digits cpu port number>_<switch MIB counter names>

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes from RFC:

- prepend the CPU port as a prefix to make it clear what the stats are
  about, master netdev interface stats are unchanged

 include/net/dsa.h |  5 ++++
 net/dsa/slave.c   | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 2d280aba97e2..8e86af87c84f 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -111,6 +111,11 @@ struct dsa_switch_tree {
 	enum dsa_tag_protocol	tag_protocol;
 
 	/*
+	 * Original copy of the master netdev ethtool_ops
+	 */
+	struct ethtool_ops	master_ethtool_ops;
+
+	/*
 	 * The switch and port to which the CPU is attached.
 	 */
 	s8			cpu_switch;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 3b6750f5e68b..5ea8a40c8d33 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -666,6 +666,78 @@ static void dsa_slave_get_strings(struct net_device *dev,
 	}
 }
 
+static void dsa_cpu_port_get_ethtool_stats(struct net_device *dev,
+					   struct ethtool_stats *stats,
+					   uint64_t *data)
+{
+	struct dsa_switch_tree *dst = dev->dsa_ptr;
+	struct dsa_switch *ds = dst->ds[0];
+	s8 cpu_port = dst->cpu_port;
+	int count = 0;
+
+	if (dst->master_ethtool_ops.get_sset_count) {
+		count = dst->master_ethtool_ops.get_sset_count(dev,
+							       ETH_SS_STATS);
+		dst->master_ethtool_ops.get_ethtool_stats(dev, stats, data);
+	}
+
+	if (ds->drv->get_ethtool_stats)
+		ds->drv->get_ethtool_stats(ds, cpu_port, data + count);
+}
+
+static int dsa_cpu_port_get_sset_count(struct net_device *dev, int sset)
+{
+	struct dsa_switch_tree *dst = dev->dsa_ptr;
+	struct dsa_switch *ds = dst->ds[0];
+	int count = 0;
+
+	if (dst->master_ethtool_ops.get_sset_count)
+		count += dst->master_ethtool_ops.get_sset_count(dev, sset);
+
+	if (sset == ETH_SS_STATS && ds->drv->get_sset_count)
+		count += ds->drv->get_sset_count(ds);
+
+	return count;
+}
+
+static void dsa_cpu_port_get_strings(struct net_device *dev,
+				     uint32_t stringset, uint8_t *data)
+{
+	struct dsa_switch_tree *dst = dev->dsa_ptr;
+	struct dsa_switch *ds = dst->ds[0];
+	s8 cpu_port = dst->cpu_port;
+	int len = ETH_GSTRING_LEN;
+	int mcount = 0, count;
+	unsigned int i;
+	uint8_t pfx[4];
+	uint8_t *ndata;
+
+	snprintf(pfx, sizeof(pfx), "p%.2d", cpu_port);
+	/* We do not want to be NULL-terminated, since this is a prefix */
+	pfx[sizeof(pfx) - 1] = '_';
+
+	if (dst->master_ethtool_ops.get_sset_count) {
+		mcount = dst->master_ethtool_ops.get_sset_count(dev,
+								ETH_SS_STATS);
+		dst->master_ethtool_ops.get_strings(dev, stringset, data);
+	}
+
+	if (stringset == ETH_SS_STATS && ds->drv->get_strings) {
+		ndata = data + mcount * len;
+		/* This function copies ETH_GSTRINGS_LEN bytes, we will mangle
+		 * the output after to prepend our CPU port prefix we
+		 * constructed earlier
+		 */
+		ds->drv->get_strings(ds, cpu_port, ndata);
+		count = ds->drv->get_sset_count(ds);
+		for (i = 0; i < count; i++) {
+			memmove(ndata + (i * len + sizeof(pfx)),
+				ndata + i * len, len - sizeof(pfx));
+			memcpy(ndata + i * len, pfx, sizeof(pfx));
+		}
+	}
+}
+
 static void dsa_slave_get_ethtool_stats(struct net_device *dev,
 					struct ethtool_stats *stats,
 					uint64_t *data)
@@ -821,6 +893,8 @@ static const struct ethtool_ops dsa_slave_ethtool_ops = {
 	.get_eee		= dsa_slave_get_eee,
 };
 
+static struct ethtool_ops dsa_cpu_port_ethtool_ops;
+
 static const struct net_device_ops dsa_slave_netdev_ops = {
 	.ndo_open	 	= dsa_slave_open,
 	.ndo_stop		= dsa_slave_close,
@@ -1038,6 +1112,7 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
 		     int port, char *name)
 {
 	struct net_device *master = ds->dst->master_netdev;
+	struct dsa_switch_tree *dst = ds->dst;
 	struct net_device *slave_dev;
 	struct dsa_slave_priv *p;
 	int ret;
@@ -1049,6 +1124,19 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
 
 	slave_dev->features = master->vlan_features;
 	slave_dev->ethtool_ops = &dsa_slave_ethtool_ops;
+	if (master->ethtool_ops != &dsa_cpu_port_ethtool_ops) {
+		memcpy(&dst->master_ethtool_ops, master->ethtool_ops,
+		       sizeof(struct ethtool_ops));
+		memcpy(&dsa_cpu_port_ethtool_ops, &dst->master_ethtool_ops,
+		       sizeof(struct ethtool_ops));
+		dsa_cpu_port_ethtool_ops.get_sset_count =
+					dsa_cpu_port_get_sset_count;
+		dsa_cpu_port_ethtool_ops.get_ethtool_stats =
+					dsa_cpu_port_get_ethtool_stats;
+		dsa_cpu_port_ethtool_ops.get_strings =
+					dsa_cpu_port_get_strings;
+		master->ethtool_ops = &dsa_cpu_port_ethtool_ops;
+	}
 	eth_hw_addr_inherit(slave_dev, master);
 	slave_dev->priv_flags |= IFF_NO_QUEUE;
 	slave_dev->netdev_ops = &dsa_slave_netdev_ops;
-- 
2.1.0

Re: [PATCH net-next] net: dsa: Provide CPU port statistics to master netdev

[Andrew Lunn]

> +	if (stringset == ETH_SS_STATS && ds->drv->get_strings) {
> +		ndata = data + mcount * len;
> +		/* This function copies ETH_GSTRINGS_LEN bytes, we will mangle
> +		 * the output after to prepend our CPU port prefix we
> +		 * constructed earlier
> +		 */
> +		ds->drv->get_strings(ds, cpu_port, ndata);
> +		count = ds->drv->get_sset_count(ds);
> +		for (i = 0; i < count; i++) {
> +			memmove(ndata + (i * len + sizeof(pfx)),
> +				ndata + i * len, len - sizeof(pfx));
> +			memcpy(ndata + i * len, pfx, sizeof(pfx));

Hi Florian

Did you check what happens if this causes the NULL terminator to be
discarded? Does ethtool handle that? As i said before, it is unclear
if one is required.

	   Andrew

Re: [PATCH net-next] net: dsa: Provide CPU port statistics to master netdev

[Florian Fainelli]

On 27/04/16 12:03, Andrew Lunn wrote:
>> +	if (stringset == ETH_SS_STATS && ds->drv->get_strings) {
>> +		ndata = data + mcount * len;
>> +		/* This function copies ETH_GSTRINGS_LEN bytes, we will mangle
>> +		 * the output after to prepend our CPU port prefix we
>> +		 * constructed earlier
>> +		 */
>> +		ds->drv->get_strings(ds, cpu_port, ndata);
>> +		count = ds->drv->get_sset_count(ds);
>> +		for (i = 0; i < count; i++) {
>> +			memmove(ndata + (i * len + sizeof(pfx)),
>> +				ndata + i * len, len - sizeof(pfx));
>> +			memcpy(ndata + i * len, pfx, sizeof(pfx));
> 
> Hi Florian
> 
> Did you check what happens if this causes the NULL terminator to be
> discarded? Does ethtool handle that? As i said before, it is unclear
> if one is required.

I just did yes. So ethtool has a do_gstringset() function which
NULL-terminates every strings set except the statistics kind
(ETH_SS_STATS or ETH_SS_PHY_STATS) but this is not much of a problem
because it limits the output to ETH_GSTRING_LEN anyway.

After injecting a bit of error in net/dsa/slave.c to have a much bigger
prefix making us push the stats names, the stats are correcty truncated
by ethtool. So we seem to be good to go with the current code in kernel
and user space.
-- 
Florian

Re: [PATCH net-next] net: dsa: Provide CPU port statistics to master netdev

[David Miller]

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Wed, 27 Apr 2016 11:45:14 -0700

> This patch overloads the DSA master netdev, aka CPU Ethernet MAC to also
> include switch-side statistics, which is useful for debugging purposes,
> when the switch is not properly connected to the Ethernet MAC (duplex
> mismatch, (RG)MII electrical issues etc.).
> 
> We accomplish this by retaining the original copy of the master netdev's
> ethtool_ops, and just overload the 3 operations we care about:
> get_sset_count, get_strings and get_ethtool_stats so as to intercept
> these calls and call into the original master_netdev ethtool_ops, plus
> our own.
> 
> We take this approach as opposed to providing a set of DSA helper
> functions that would retrive the CPU port's statistics, because the
> entire purpose of DSA is to allow unmodified Ethernet MAC drivers to be
> used as CPU conduit interfaces, therefore, statistics overlay in such
> drivers would simply not scale.
> 
> The new ethtool -S <iface> output would therefore look like this now:
> <iface> statistics
> p<2 digits cpu port number>_<switch MIB counter names>
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Applied, thanks Florian.