[PATCH net-next 0/5] net: dsa: Multi-CPU ground work

[PATCH net-next 0/5] net: dsa: Multi-CPU ground work

[Florian Fainelli]

Hi all,

This patch series prepares the ground for adding mutliple CPU port support to
DSA, and starts by removing redundant pieces of information such as
master_netdev which is cpu_dp->ethernet. Finally drivers are moved away from
directly accessing ds->dst->cpu_dp and use appropriate helper functions.

Note that if you have Device Tree blobs/platform configurations that are
currently listing multiple CPU ports, the proposed behavior in
dsa_ds_get_cpu_dp() will be to return the last bit set in ds->cpu_port_mask.

Future plans include:
- making dst->cpu_dp a flexible data structure (array, list, you name it)
- having the ability for drivers to return a default/preferred CPU port (if
  necessary)

Florian Fainelli (5):
  net: dsa: Remove master_netdev and use dst->cpu_dp->netdev
  net: dsa: Relocate master ethtool operations
  net: dsa: Associate slave network device with CPU port
  net: dsa: Introduce dsa_dst_get_cpu_dp()
  net: dsa: Stop accessing ds->dst->cpu_dp in drivers

 drivers/net/dsa/b53/b53_common.c |  4 +--
 drivers/net/dsa/bcm_sf2.c        | 10 +++++---
 drivers/net/dsa/mt7530.c         |  6 +++--
 drivers/net/dsa/mv88e6060.c      |  3 ++-
 drivers/net/dsa/qca8k.c          |  2 +-
 include/net/dsa.h                | 29 +++++++++------------
 net/dsa/dsa.c                    | 19 ++++----------
 net/dsa/dsa2.c                   | 18 ++++++--------
 net/dsa/dsa_priv.h               | 10 ++++++++
 net/dsa/legacy.c                 | 22 +++++++++-------
 net/dsa/slave.c                  | 54 +++++++++++++++++++++-------------------
 net/dsa/tag_brcm.c               |  5 ++--
 net/dsa/tag_ksz.c                |  5 ++--
 net/dsa/tag_qca.c                |  3 ++-
 net/dsa/tag_trailer.c            |  5 ++--
 15 files changed, 98 insertions(+), 97 deletions(-)

-- 
2.9.3

[PATCH net-next 1/5] net: dsa: Remove master_netdev and use dst->cpu_dp->netdev

[Florian Fainelli]

In preparation for supporting multiple CPU ports, remove
dst->master_netdev and ds->master_netdev and replace them with only one
instance of the common object we have for a port: struct
dsa_port::netdev. ds->master_netdev is currently write only and would be
helpful in the case where we have two switches, both with CPU ports, and
also connected within each other, which the multi-CPU port patch series
would address.

While at it, introduce a helper function used in net/dsa/slave.c to
immediately get a reference on the master network device called
dsa_master_netdev().

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/bcm_sf2.c |  4 ++--
 drivers/net/dsa/mt7530.c  |  4 ++--
 include/net/dsa.h         |  5 -----
 net/dsa/dsa.c             |  9 ++-------
 net/dsa/dsa2.c            | 18 +++++++-----------
 net/dsa/dsa_priv.h        |  5 +++++
 net/dsa/legacy.c          | 22 +++++++++++++---------
 net/dsa/slave.c           | 20 +++++++++-----------
 8 files changed, 40 insertions(+), 47 deletions(-)

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 687a8bae5d73..76e98e8ed315 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -806,7 +806,7 @@ static int bcm_sf2_sw_resume(struct dsa_switch *ds)
 static void bcm_sf2_sw_get_wol(struct dsa_switch *ds, int port,
 			       struct ethtool_wolinfo *wol)
 {
-	struct net_device *p = ds->dst[ds->index].master_netdev;
+	struct net_device *p = ds->dst[ds->index].cpu_dp->netdev;
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
 	struct ethtool_wolinfo pwol;
 
@@ -829,7 +829,7 @@ static void bcm_sf2_sw_get_wol(struct dsa_switch *ds, int port,
 static int bcm_sf2_sw_set_wol(struct dsa_switch *ds, int port,
 			      struct ethtool_wolinfo *wol)
 {
-	struct net_device *p = ds->dst[ds->index].master_netdev;
+	struct net_device *p = ds->dst[ds->index].cpu_dp->netdev;
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
 	s8 cpu_port = ds->dst->cpu_dp->index;
 	struct ethtool_wolinfo pwol;
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 25e00d5e0eec..1e46418a3b74 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -912,11 +912,11 @@ mt7530_setup(struct dsa_switch *ds)
 	struct device_node *dn;
 	struct mt7530_dummy_poll p;
 
-	/* The parent node of master_netdev which holds the common system
+	/* The parent node of cpu_dp->netdev which holds the common system
 	 * controller also is the container for two GMACs nodes representing
 	 * as two netdev instances.
 	 */
-	dn = ds->master_netdev->dev.of_node->parent;
+	dn = ds->dst->cpu_dp->netdev->dev.of_node->parent;
 	priv->ethernet = syscon_node_to_regmap(dn);
 	if (IS_ERR(priv->ethernet))
 		return PTR_ERR(priv->ethernet);
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 2effb0af9d7c..b2fb53f5e28e 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -227,11 +227,6 @@ struct dsa_switch {
 	s8		rtable[DSA_MAX_SWITCHES];
 
 	/*
-	 * The lower device this switch uses to talk to the host
-	 */
-	struct net_device *master_netdev;
-
-	/*
 	 * Slave mii_bus and devices for the individual ports.
 	 */
 	u32			dsa_port_mask;
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index fdc448b30e56..eaab1affeeeb 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -118,10 +118,7 @@ int dsa_cpu_port_ethtool_setup(struct dsa_port *cpu_dp)
 	struct net_device *master;
 	struct ethtool_ops *cpu_ops;
 
-	master = ds->dst->master_netdev;
-	if (ds->master_netdev)
-		master = ds->master_netdev;
-
+	master = ds->dst->cpu_dp->netdev;
 	cpu_ops = devm_kzalloc(ds->dev, sizeof(*cpu_ops), GFP_KERNEL);
 	if (!cpu_ops)
 		return -ENOMEM;
@@ -142,9 +139,7 @@ void dsa_cpu_port_ethtool_restore(struct dsa_port *cpu_dp)
 	struct dsa_switch *ds = cpu_dp->ds;
 	struct net_device *master;
 
-	master = ds->dst->master_netdev;
-	if (ds->master_netdev)
-		master = ds->master_netdev;
+	master = ds->dst->cpu_dp->netdev;
 
 	master->ethtool_ops = ds->dst->master_orig_ethtool_ops;
 }
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index cd13bb54a30c..2674bdf03fef 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -337,7 +337,7 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 		return err;
 
 	if (ds->ops->set_addr) {
-		err = ds->ops->set_addr(ds, dst->master_netdev->dev_addr);
+		err = ds->ops->set_addr(ds, dst->cpu_dp->netdev->dev_addr);
 		if (err < 0)
 			return err;
 	}
@@ -444,7 +444,7 @@ static int dsa_dst_apply(struct dsa_switch_tree *dst)
 	 * sent to the tag format's receive function.
 	 */
 	wmb();
-	dst->master_netdev->dsa_ptr = dst;
+	dst->cpu_dp->netdev->dsa_ptr = dst;
 	dst->applied = true;
 
 	return 0;
@@ -458,7 +458,7 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
 	if (!dst->applied)
 		return;
 
-	dst->master_netdev->dsa_ptr = NULL;
+	dst->cpu_dp->netdev->dsa_ptr = NULL;
 
 	/* If we used a tagging format that doesn't have an ethertype
 	 * field, make sure that all packets from this point get sent
@@ -502,14 +502,10 @@ static int dsa_cpu_parse(struct dsa_port *port, u32 index,
 	if (!ethernet_dev)
 		return -EPROBE_DEFER;
 
-	if (!ds->master_netdev)
-		ds->master_netdev = ethernet_dev;
-
-	if (!dst->master_netdev)
-		dst->master_netdev = ethernet_dev;
-
-	if (!dst->cpu_dp)
+	if (!dst->cpu_dp) {
 		dst->cpu_dp = port;
+		dst->cpu_dp->netdev = ethernet_dev;
+	}
 
 	tag_protocol = ds->ops->get_tag_protocol(ds);
 	dst->tag_ops = dsa_resolve_tag_protocol(tag_protocol);
@@ -576,7 +572,7 @@ static int dsa_dst_parse(struct dsa_switch_tree *dst)
 			return err;
 	}
 
-	if (!dst->master_netdev) {
+	if (!dst->cpu_dp->netdev) {
 		pr_warn("Tree has no master device\n");
 		return -EINVAL;
 	}
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 66ee248796c8..5c510f4ba0ce 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -183,4 +183,9 @@ extern const struct dsa_device_ops qca_netdev_ops;
 /* tag_trailer.c */
 extern const struct dsa_device_ops trailer_netdev_ops;
 
+static inline struct net_device *dsa_master_netdev(struct dsa_slave_priv *p)
+{
+	return p->dp->ds->dst->cpu_dp->netdev;
+}
+
 #endif
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index d534d8f4b9cf..443ebf3b5e5b 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -101,9 +101,12 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
 	struct dsa_switch_tree *dst = ds->dst;
 	struct dsa_chip_data *cd = ds->cd;
 	bool valid_name_found = false;
+	struct net_device *master;
 	int index = ds->index;
 	int i, ret;
 
+	master = dst->cpu_dp->netdev;
+
 	/*
 	 * Validate supplied switch configuration.
 	 */
@@ -116,7 +119,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
 
 		if (!strcmp(name, "cpu")) {
 			if (dst->cpu_dp) {
-				netdev_err(dst->master_netdev,
+				netdev_err(master,
 					   "multiple cpu ports?!\n");
 				return -EINVAL;
 			}
@@ -168,7 +171,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
 		return ret;
 
 	if (ops->set_addr) {
-		ret = ops->set_addr(ds, dst->master_netdev->dev_addr);
+		ret = ops->set_addr(ds, master->dev_addr);
 		if (ret < 0)
 			return ret;
 	}
@@ -195,14 +198,14 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
 
 		ret = dsa_slave_create(ds, parent, i, cd->port_names[i]);
 		if (ret < 0)
-			netdev_err(dst->master_netdev, "[%d]: can't create dsa slave device for port %d(%s): %d\n",
+			netdev_err(master, "[%d]: can't create dsa slave device for port %d(%s): %d\n",
 				   index, i, cd->port_names[i], ret);
 	}
 
 	/* Perform configuration of the CPU and DSA ports */
 	ret = dsa_cpu_dsa_setups(ds, parent);
 	if (ret < 0)
-		netdev_err(dst->master_netdev, "[%d] : can't configure CPU and DSA ports\n",
+		netdev_err(master, "[%d] : can't configure CPU and DSA ports\n",
 			   index);
 
 	ret = dsa_cpu_port_ethtool_setup(ds->dst->cpu_dp);
@@ -217,6 +220,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
 		 struct device *parent, struct device *host_dev)
 {
 	struct dsa_chip_data *cd = dst->pd->chip + index;
+	struct net_device *master = dst->cpu_dp->netdev;
 	const struct dsa_switch_ops *ops;
 	struct dsa_switch *ds;
 	int ret;
@@ -228,11 +232,11 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
 	 */
 	ops = dsa_switch_probe(parent, host_dev, cd->sw_addr, &name, &priv);
 	if (!ops) {
-		netdev_err(dst->master_netdev, "[%d]: could not detect attached switch\n",
+		netdev_err(master, "[%d]: could not detect attached switch\n",
 			   index);
 		return ERR_PTR(-EINVAL);
 	}
-	netdev_info(dst->master_netdev, "[%d]: detected a %s switch\n",
+	netdev_info(master, "[%d]: detected a %s switch\n",
 		    index, name);
 
 
@@ -622,7 +626,7 @@ static int dsa_setup_dst(struct dsa_switch_tree *dst, struct net_device *dev,
 	unsigned configured = 0;
 
 	dst->pd = pd;
-	dst->master_netdev = dev;
+	dst->cpu_dp->netdev = dev;
 
 	for (i = 0; i < pd->nr_chips; i++) {
 		struct dsa_switch *ds;
@@ -718,7 +722,7 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
 {
 	int i;
 
-	dst->master_netdev->dsa_ptr = NULL;
+	dst->cpu_dp->netdev->dsa_ptr = NULL;
 
 	/* If we used a tagging format that doesn't have an ethertype
 	 * field, make sure that all packets from this point get sent
@@ -735,7 +739,7 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
 
 	dsa_cpu_port_ethtool_restore(dst->cpu_dp);
 
-	dev_put(dst->master_netdev);
+	dev_put(dst->cpu_dp->netdev);
 }
 
 static int dsa_remove(struct platform_device *pdev)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 1cfdb31a2f44..d52c9ceb0566 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -66,7 +66,7 @@ static int dsa_slave_get_iflink(const struct net_device *dev)
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
 
-	return p->dp->ds->dst->master_netdev->ifindex;
+	return dsa_master_netdev(p)->ifindex;
 }
 
 static int dsa_slave_open(struct net_device *dev)
@@ -74,7 +74,7 @@ static int dsa_slave_open(struct net_device *dev)
 	struct dsa_slave_priv *p = netdev_priv(dev);
 	struct dsa_port *dp = p->dp;
 	struct dsa_switch *ds = dp->ds;
-	struct net_device *master = ds->dst->master_netdev;
+	struct net_device *master = dsa_master_netdev(p);
 	u8 stp_state = dp->bridge_dev ? BR_STATE_BLOCKING : BR_STATE_FORWARDING;
 	int err;
 
@@ -127,7 +127,7 @@ static int dsa_slave_open(struct net_device *dev)
 static int dsa_slave_close(struct net_device *dev)
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct net_device *master = p->dp->ds->dst->master_netdev;
+	struct net_device *master = dsa_master_netdev(p);
 	struct dsa_switch *ds = p->dp->ds;
 
 	if (p->phy)
@@ -154,7 +154,7 @@ static int dsa_slave_close(struct net_device *dev)
 static void dsa_slave_change_rx_flags(struct net_device *dev, int change)
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct net_device *master = p->dp->ds->dst->master_netdev;
+	struct net_device *master = dsa_master_netdev(p);
 
 	if (change & IFF_ALLMULTI)
 		dev_set_allmulti(master, dev->flags & IFF_ALLMULTI ? 1 : -1);
@@ -165,7 +165,7 @@ static void dsa_slave_change_rx_flags(struct net_device *dev, int change)
 static void dsa_slave_set_rx_mode(struct net_device *dev)
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct net_device *master = p->dp->ds->dst->master_netdev;
+	struct net_device *master = dsa_master_netdev(p);
 
 	dev_mc_sync(master, dev);
 	dev_uc_sync(master, dev);
@@ -174,7 +174,7 @@ static void dsa_slave_set_rx_mode(struct net_device *dev)
 static int dsa_slave_set_mac_address(struct net_device *dev, void *a)
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct net_device *master = p->dp->ds->dst->master_netdev;
+	struct net_device *master = dsa_master_netdev(p);
 	struct sockaddr *addr = a;
 	int err;
 
@@ -375,7 +375,7 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev)
 	/* Queue the SKB for transmission on the parent interface, but
 	 * do not modify its EtherType
 	 */
-	nskb->dev = p->dp->ds->dst->master_netdev;
+	nskb->dev = dsa_master_netdev(p);
 	dev_queue_xmit(nskb);
 
 	return NETDEV_TX_OK;
@@ -684,7 +684,7 @@ static int dsa_slave_netpoll_setup(struct net_device *dev,
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
 	struct dsa_switch *ds = p->dp->ds;
-	struct net_device *master = ds->dst->master_netdev;
+	struct net_device *master = dsa_master_netdev(p);
 	struct netpoll *netpoll;
 	int err = 0;
 
@@ -1141,9 +1141,7 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
 	struct dsa_slave_priv *p;
 	int ret;
 
-	master = ds->dst->master_netdev;
-	if (ds->master_netdev)
-		master = ds->master_netdev;
+	master = ds->dst->cpu_dp->netdev;
 
 	slave_dev = alloc_netdev(sizeof(struct dsa_slave_priv), name,
 				 NET_NAME_UNKNOWN, ether_setup);
-- 
2.9.3

Re: [PATCH net-next 1/5] net: dsa: Remove master_netdev and use dst->cpu_dp->netdev

[Vivien Didelot]

Hi Florian,

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

> -	struct net_device *p = ds->dst[ds->index].master_netdev;
> +	struct net_device *p = ds->dst[ds->index].cpu_dp->netdev;

                               ds->dst->cpu_dp->netdev

ds->dst is not an array anymore, lucky sf2 switch index is always 0 ;-)

>  	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
>  	struct ethtool_wolinfo pwol;
>  
> @@ -829,7 +829,7 @@ static void bcm_sf2_sw_get_wol(struct dsa_switch *ds, int port,
>  static int bcm_sf2_sw_set_wol(struct dsa_switch *ds, int port,
>  			      struct ethtool_wolinfo *wol)
>  {
> -	struct net_device *p = ds->dst[ds->index].master_netdev;
> +	struct net_device *p = ds->dst[ds->index].cpu_dp->netdev;

same here.

Thanks,

        Vivien

Re: [PATCH net-next 1/5] net: dsa: Remove master_netdev and use dst->cpu_dp->netdev

[Florian Fainelli]

On 06/06/2017 10:24 AM, Vivien Didelot wrote:
> Hi Florian,
> 
> Florian Fainelli <f.fainelli@gmail.com> writes:
> 
>> -	struct net_device *p = ds->dst[ds->index].master_netdev;
>> +	struct net_device *p = ds->dst[ds->index].cpu_dp->netdev;
> 
>                                ds->dst->cpu_dp->netdev
> 
> ds->dst is not an array anymore, lucky sf2 switch index is always 0 ;-)
> 
>>  	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
>>  	struct ethtool_wolinfo pwol;
>>  
>> @@ -829,7 +829,7 @@ static void bcm_sf2_sw_get_wol(struct dsa_switch *ds, int port,
>>  static int bcm_sf2_sw_set_wol(struct dsa_switch *ds, int port,
>>  			      struct ethtool_wolinfo *wol)
>>  {
>> -	struct net_device *p = ds->dst[ds->index].master_netdev;
>> +	struct net_device *p = ds->dst[ds->index].cpu_dp->netdev;
> 
> same here.

It's changed in patch 5, so I did not bother doing an intermediate
change considering we ditch this eventually.

Thanks!

> 
> Thanks,
> 
>         Vivien
> 


-- 
Florian

Re: [PATCH net-next 1/5] net: dsa: Remove master_netdev and use dst->cpu_dp->netdev

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2585 bytes --]

Hi Florian,

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Florian-Fainelli/net-dsa-Multi-CPU-ground-work/20170607-164203
config: x86_64-randconfig-s0-06090601 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   net//dsa/slave.c: In function 'dsa_slave_netpoll_setup':
>> net//dsa/slave.c:686: warning: unused variable 'ds'

vim +/ds +686 net//dsa/slave.c

7905288f Florian Fainelli 2014-09-24  670  
afdcf151 Vivien Didelot   2017-01-27  671  	ret = ds->ops->get_eee(ds, p->dp->index, e);
7905288f Florian Fainelli 2014-09-24  672  	if (ret)
7905288f Florian Fainelli 2014-09-24  673  		return ret;
7905288f Florian Fainelli 2014-09-24  674  
7905288f Florian Fainelli 2014-09-24  675  	if (p->phy)
7905288f Florian Fainelli 2014-09-24  676  		ret = phy_ethtool_get_eee(p->phy, e);
7905288f Florian Fainelli 2014-09-24  677  
7905288f Florian Fainelli 2014-09-24  678  	return ret;
7905288f Florian Fainelli 2014-09-24  679  }
7905288f Florian Fainelli 2014-09-24  680  
04ff53f9 Florian Fainelli 2015-07-31  681  #ifdef CONFIG_NET_POLL_CONTROLLER
04ff53f9 Florian Fainelli 2015-07-31  682  static int dsa_slave_netpoll_setup(struct net_device *dev,
04ff53f9 Florian Fainelli 2015-07-31  683  				   struct netpoll_info *ni)
04ff53f9 Florian Fainelli 2015-07-31  684  {
04ff53f9 Florian Fainelli 2015-07-31  685  	struct dsa_slave_priv *p = netdev_priv(dev);
afdcf151 Vivien Didelot   2017-01-27 @686  	struct dsa_switch *ds = p->dp->ds;
6ab303ef Florian Fainelli 2017-06-06  687  	struct net_device *master = dsa_master_netdev(p);
04ff53f9 Florian Fainelli 2015-07-31  688  	struct netpoll *netpoll;
04ff53f9 Florian Fainelli 2015-07-31  689  	int err = 0;
04ff53f9 Florian Fainelli 2015-07-31  690  
04ff53f9 Florian Fainelli 2015-07-31  691  	netpoll = kzalloc(sizeof(*netpoll), GFP_KERNEL);
04ff53f9 Florian Fainelli 2015-07-31  692  	if (!netpoll)
04ff53f9 Florian Fainelli 2015-07-31  693  		return -ENOMEM;
04ff53f9 Florian Fainelli 2015-07-31  694  

:::::: The code at line 686 was first introduced by commit
:::::: afdcf151c1f7346207dcee3f8d6d82991dbbb7e5 net: dsa: store a dsa_port in dsa_slave_priv

:::::: TO: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30296 bytes --]

[PATCH net-next 2/5] net: dsa: Relocate master ethtool operations

[Florian Fainelli]

Relocate master_ethtool_ops and master_orig_ethtool_ops into struct
dsa_port in order to be both consistent, and make things self contained
within the dsa_port structure.

This is a preliminary change to supporting multiple CPU port interfaces.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 include/net/dsa.h | 17 +++++------------
 net/dsa/dsa.c     | 16 ++++++----------
 net/dsa/slave.c   | 16 ++++++++--------
 3 files changed, 19 insertions(+), 30 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index b2fb53f5e28e..7e93869819f9 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -122,12 +122,6 @@ struct dsa_switch_tree {
 	 */
 	struct dsa_platform_data	*pd;
 
-	/*
-	 * Reference to network device to use, and which tagging
-	 * protocol to use.
-	 */
-	struct net_device	*master_netdev;
-
 	/* Copy of tag_ops->rcv for faster access in hot path */
 	struct sk_buff *	(*rcv)(struct sk_buff *skb,
 				       struct net_device *dev,
@@ -135,12 +129,6 @@ struct dsa_switch_tree {
 				       struct net_device *orig_dev);
 
 	/*
-	 * Original copy of the master netdev ethtool_ops
-	 */
-	struct ethtool_ops	master_ethtool_ops;
-	const struct ethtool_ops *master_orig_ethtool_ops;
-
-	/*
 	 * The switch port to which the CPU is attached.
 	 */
 	struct dsa_port		*cpu_dp;
@@ -189,6 +177,11 @@ struct dsa_port {
 	u8			stp_state;
 	struct net_device	*bridge_dev;
 	struct devlink_port	devlink_port;
+	/*
+	 * Original copy of the master netdev ethtool_ops
+	 */
+	struct ethtool_ops	ethtool_ops;
+	const struct ethtool_ops *orig_ethtool_ops;
 };
 
 struct dsa_switch {
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index eaab1affeeeb..2665a66e833d 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -118,15 +118,16 @@ int dsa_cpu_port_ethtool_setup(struct dsa_port *cpu_dp)
 	struct net_device *master;
 	struct ethtool_ops *cpu_ops;
 
-	master = ds->dst->cpu_dp->netdev;
+	master = cpu_dp->netdev;
+
 	cpu_ops = devm_kzalloc(ds->dev, sizeof(*cpu_ops), GFP_KERNEL);
 	if (!cpu_ops)
 		return -ENOMEM;
 
-	memcpy(&ds->dst->master_ethtool_ops, master->ethtool_ops,
+	memcpy(&cpu_dp->ethtool_ops, master->ethtool_ops,
 	       sizeof(struct ethtool_ops));
-	ds->dst->master_orig_ethtool_ops = master->ethtool_ops;
-	memcpy(cpu_ops, &ds->dst->master_ethtool_ops,
+	cpu_dp->orig_ethtool_ops = master->ethtool_ops;
+	memcpy(cpu_ops, &cpu_dp->ethtool_ops,
 	       sizeof(struct ethtool_ops));
 	dsa_cpu_port_ethtool_init(cpu_ops);
 	master->ethtool_ops = cpu_ops;
@@ -136,12 +137,7 @@ int dsa_cpu_port_ethtool_setup(struct dsa_port *cpu_dp)
 
 void dsa_cpu_port_ethtool_restore(struct dsa_port *cpu_dp)
 {
-	struct dsa_switch *ds = cpu_dp->ds;
-	struct net_device *master;
-
-	master = ds->dst->cpu_dp->netdev;
-
-	master->ethtool_ops = ds->dst->master_orig_ethtool_ops;
+	cpu_dp->netdev->ethtool_ops = cpu_dp->orig_ethtool_ops;
 }
 
 void dsa_cpu_dsa_destroy(struct dsa_port *port)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index d52c9ceb0566..ea4ed0285922 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -523,10 +523,10 @@ static void dsa_cpu_port_get_ethtool_stats(struct net_device *dev,
 	s8 cpu_port = dst->cpu_dp->index;
 	int count = 0;
 
-	if (dst->master_ethtool_ops.get_sset_count) {
-		count = dst->master_ethtool_ops.get_sset_count(dev,
+	if (dst->cpu_dp->ethtool_ops.get_sset_count) {
+		count = dst->cpu_dp->ethtool_ops.get_sset_count(dev,
 							       ETH_SS_STATS);
-		dst->master_ethtool_ops.get_ethtool_stats(dev, stats, data);
+		dst->cpu_dp->ethtool_ops.get_ethtool_stats(dev, stats, data);
 	}
 
 	if (ds->ops->get_ethtool_stats)
@@ -539,8 +539,8 @@ static int dsa_cpu_port_get_sset_count(struct net_device *dev, int sset)
 	struct dsa_switch *ds = dst->cpu_dp->ds;
 	int count = 0;
 
-	if (dst->master_ethtool_ops.get_sset_count)
-		count += dst->master_ethtool_ops.get_sset_count(dev, sset);
+	if (dst->cpu_dp->ethtool_ops.get_sset_count)
+		count += dst->cpu_dp->ethtool_ops.get_sset_count(dev, sset);
 
 	if (sset == ETH_SS_STATS && ds->ops->get_sset_count)
 		count += ds->ops->get_sset_count(ds);
@@ -564,10 +564,10 @@ static void dsa_cpu_port_get_strings(struct net_device *dev,
 	/* 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,
+	if (dst->cpu_dp->ethtool_ops.get_sset_count) {
+		mcount = dst->cpu_dp->ethtool_ops.get_sset_count(dev,
 								ETH_SS_STATS);
-		dst->master_ethtool_ops.get_strings(dev, stringset, data);
+		dst->cpu_dp->ethtool_ops.get_strings(dev, stringset, data);
 	}
 
 	if (stringset == ETH_SS_STATS && ds->ops->get_strings) {
-- 
2.9.3

Re: [PATCH net-next 2/5] net: dsa: Relocate master ethtool operations

[Vivien Didelot]

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

> Relocate master_ethtool_ops and master_orig_ethtool_ops into struct
> dsa_port in order to be both consistent, and make things self contained
> within the dsa_port structure.
>
> This is a preliminary change to supporting multiple CPU port interfaces.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

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

[PATCH net-next 3/5] net: dsa: Associate slave network device with CPU port

[Florian Fainelli]

In preparation for supporting multiple CPU ports with DSA, have the
dsa_slave_priv structure know which CPU it is associated with. This will
be important in order to make sure the correct CPU is used for
transmission of the frames. If not for functional reasons, for
performance (e.g: load balancing) and forwarding decisions.

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

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 7e93869819f9..58969b9a090c 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -171,6 +171,7 @@ struct dsa_port {
 	struct dsa_switch	*ds;
 	unsigned int		index;
 	const char		*name;
+	struct dsa_port		*cpu_dp;
 	struct net_device	*netdev;
 	struct device_node	*dn;
 	unsigned int		ageing_time;
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 5c510f4ba0ce..7c2326f3b538 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -185,7 +185,7 @@ extern const struct dsa_device_ops trailer_netdev_ops;
 
 static inline struct net_device *dsa_master_netdev(struct dsa_slave_priv *p)
 {
-	return p->dp->ds->dst->cpu_dp->netdev;
+	return p->dp->cpu_dp->netdev;
 }
 
 #endif
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index ea4ed0285922..de1ab41cfd38 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1139,9 +1139,11 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
 	struct net_device *master;
 	struct net_device *slave_dev;
 	struct dsa_slave_priv *p;
+	struct dsa_port *cpu_dp;
 	int ret;
 
-	master = ds->dst->cpu_dp->netdev;
+	cpu_dp = ds->dst->cpu_dp;
+	master = cpu_dp->netdev;
 
 	slave_dev = alloc_netdev(sizeof(struct dsa_slave_priv), name,
 				 NET_NAME_UNKNOWN, ether_setup);
@@ -1176,6 +1178,7 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
 	p->old_duplex = -1;
 
 	ds->ports[port].netdev = slave_dev;
+	p->dp->cpu_dp = cpu_dp;
 	ret = register_netdev(slave_dev);
 	if (ret) {
 		netdev_err(master, "error %d registering interface %s\n",
-- 
2.9.3

Re: [PATCH net-next 3/5] net: dsa: Associate slave network device with CPU port

[Vivien Didelot]

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

> In preparation for supporting multiple CPU ports with DSA, have the
> dsa_slave_priv structure know which CPU it is associated with. This will

  dsa_port
  
> be important in order to make sure the correct CPU is used for
> transmission of the frames. If not for functional reasons, for
> performance (e.g: load balancing) and forwarding decisions.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  include/net/dsa.h  | 1 +
>  net/dsa/dsa_priv.h | 2 +-
>  net/dsa/slave.c    | 5 ++++-
>  3 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 7e93869819f9..58969b9a090c 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -171,6 +171,7 @@ struct dsa_port {
>  	struct dsa_switch	*ds;
>  	unsigned int		index;
>  	const char		*name;
> +	struct dsa_port		*cpu_dp;
>  	struct net_device	*netdev;
>  	struct device_node	*dn;
>  	unsigned int		ageing_time;
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index 5c510f4ba0ce..7c2326f3b538 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -185,7 +185,7 @@ extern const struct dsa_device_ops trailer_netdev_ops;
>  
>  static inline struct net_device *dsa_master_netdev(struct dsa_slave_priv *p)
>  {
> -	return p->dp->ds->dst->cpu_dp->netdev;
> +	return p->dp->cpu_dp->netdev;
>  }
>  
>  #endif
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index ea4ed0285922..de1ab41cfd38 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -1139,9 +1139,11 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,


>  	struct net_device *master;
>  	struct net_device *slave_dev;
>  	struct dsa_slave_priv *p;
> +	struct dsa_port *cpu_dp;
>  	int ret;
>  
> -	master = ds->dst->cpu_dp->netdev;
> +	cpu_dp = ds->dst->cpu_dp;
> +	master = cpu_dp->netdev;

You may assign them when declaring them, but no big deal.

>  
>  	slave_dev = alloc_netdev(sizeof(struct dsa_slave_priv), name,
>  				 NET_NAME_UNKNOWN, ether_setup);
> @@ -1176,6 +1178,7 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
>  	p->old_duplex = -1;
>  
>  	ds->ports[port].netdev = slave_dev;
> +	p->dp->cpu_dp = cpu_dp;
>  	ret = register_netdev(slave_dev);
>  	if (ret) {
>  		netdev_err(master, "error %d registering interface %s\n",

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

[PATCH net-next 4/5] net: dsa: Introduce dsa_dst_get_cpu_dp()

[Florian Fainelli]

Introduce a helper function which will return a reference to the CPU
port used in a dsa_switch_tree. Right now this is a singleton, but this
will change once we introduce multi-CPU port support, so ease the
transition by converting the affected code paths.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/dsa_priv.h    |  5 +++++
 net/dsa/slave.c       | 31 ++++++++++++++++---------------
 net/dsa/tag_brcm.c    |  5 ++---
 net/dsa/tag_ksz.c     |  5 ++---
 net/dsa/tag_qca.c     |  3 ++-
 net/dsa/tag_trailer.c |  5 ++---
 6 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 7c2326f3b538..49b4b047aed0 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -188,4 +188,9 @@ static inline struct net_device *dsa_master_netdev(struct dsa_slave_priv *p)
 	return p->dp->cpu_dp->netdev;
 }
 
+static inline struct dsa_port *dsa_dst_get_cpu_dp(struct dsa_switch_tree *dst)
+{
+	return dst->cpu_dp;
+}
+
 #endif
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index de1ab41cfd38..a73c1de398b5 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -519,14 +519,14 @@ static void dsa_cpu_port_get_ethtool_stats(struct net_device *dev,
 					   uint64_t *data)
 {
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
-	struct dsa_switch *ds = dst->cpu_dp->ds;
-	s8 cpu_port = dst->cpu_dp->index;
+	struct dsa_port *cpu_dp = dsa_dst_get_cpu_dp(dst);
+	struct dsa_switch *ds = cpu_dp->ds;
+	s8 cpu_port = cpu_dp->index;
 	int count = 0;
 
-	if (dst->cpu_dp->ethtool_ops.get_sset_count) {
-		count = dst->cpu_dp->ethtool_ops.get_sset_count(dev,
-							       ETH_SS_STATS);
-		dst->cpu_dp->ethtool_ops.get_ethtool_stats(dev, stats, data);
+	if (cpu_dp->ethtool_ops.get_sset_count) {
+		count = cpu_dp->ethtool_ops.get_sset_count(dev, ETH_SS_STATS);
+		cpu_dp->ethtool_ops.get_ethtool_stats(dev, stats, data);
 	}
 
 	if (ds->ops->get_ethtool_stats)
@@ -536,11 +536,12 @@ static void dsa_cpu_port_get_ethtool_stats(struct net_device *dev,
 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->cpu_dp->ds;
+	struct dsa_port *cpu_dp = dsa_dst_get_cpu_dp(dst);
+	struct dsa_switch *ds = cpu_dp->ds;
 	int count = 0;
 
-	if (dst->cpu_dp->ethtool_ops.get_sset_count)
-		count += dst->cpu_dp->ethtool_ops.get_sset_count(dev, sset);
+	if (cpu_dp->ethtool_ops.get_sset_count)
+		count += cpu_dp->ethtool_ops.get_sset_count(dev, sset);
 
 	if (sset == ETH_SS_STATS && ds->ops->get_sset_count)
 		count += ds->ops->get_sset_count(ds);
@@ -552,8 +553,9 @@ 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->cpu_dp->ds;
-	s8 cpu_port = dst->cpu_dp->index;
+	struct dsa_port *cpu_dp = dsa_dst_get_cpu_dp(dst);
+	struct dsa_switch *ds = cpu_dp->ds;
+	s8 cpu_port = cpu_dp->index;
 	int len = ETH_GSTRING_LEN;
 	int mcount = 0, count;
 	unsigned int i;
@@ -564,10 +566,9 @@ static void dsa_cpu_port_get_strings(struct net_device *dev,
 	/* We do not want to be NULL-terminated, since this is a prefix */
 	pfx[sizeof(pfx) - 1] = '_';
 
-	if (dst->cpu_dp->ethtool_ops.get_sset_count) {
-		mcount = dst->cpu_dp->ethtool_ops.get_sset_count(dev,
-								ETH_SS_STATS);
-		dst->cpu_dp->ethtool_ops.get_strings(dev, stringset, data);
+	if (cpu_dp->ethtool_ops.get_sset_count) {
+		mcount = cpu_dp->ethtool_ops.get_sset_count(dev, ETH_SS_STATS);
+		cpu_dp->ethtool_ops.get_strings(dev, stringset, data);
 	}
 
 	if (stringset == ETH_SS_STATS && ds->ops->get_strings) {
diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
index c03860907f28..d7ef2b35e61e 100644
--- a/net/dsa/tag_brcm.c
+++ b/net/dsa/tag_brcm.c
@@ -93,12 +93,11 @@ static struct sk_buff *brcm_tag_rcv(struct sk_buff *skb, struct net_device *dev,
 				    struct net_device *orig_dev)
 {
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
-	struct dsa_switch *ds;
+	struct dsa_port *cpu_dp = dsa_dst_get_cpu_dp(dst);
+	struct dsa_switch *ds = cpu_dp->ds;
 	int source_port;
 	u8 *brcm_tag;
 
-	ds = dst->cpu_dp->ds;
-
 	if (unlikely(!pskb_may_pull(skb, BRCM_TAG_LEN)))
 		return NULL;
 
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index b94a334a1d02..c41a24e83e83 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -75,12 +75,11 @@ static struct sk_buff *ksz_rcv(struct sk_buff *skb, struct net_device *dev,
 			       struct net_device *orig_dev)
 {
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
-	struct dsa_switch *ds;
+	struct dsa_port *cpu_dp = dsa_dst_get_cpu_dp(dst);
+	struct dsa_switch *ds = cpu_dp->ds;
 	u8 *tag;
 	int source_port;
 
-	ds = dst->cpu_dp->ds;
-
 	tag = skb_tail_pointer(skb) - KSZ_EGRESS_TAG_LEN;
 
 	source_port = tag[0] & 7;
diff --git a/net/dsa/tag_qca.c b/net/dsa/tag_qca.c
index 4f43cf0b4eff..73244ae388df 100644
--- a/net/dsa/tag_qca.c
+++ b/net/dsa/tag_qca.c
@@ -67,6 +67,7 @@ static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev,
 				   struct net_device *orig_dev)
 {
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
+	struct dsa_port *cpu_dp = dsa_dst_get_cpu_dp(dst);
 	struct dsa_switch *ds;
 	u8 ver;
 	int port;
@@ -95,7 +96,7 @@ static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev,
 	/* This protocol doesn't support cascading multiple switches so it's
 	 * safe to assume the switch is first in the tree
 	 */
-	ds = dst->cpu_dp->ds;
+	ds = cpu_dp->ds;
 	if (!ds)
 		return NULL;
 
diff --git a/net/dsa/tag_trailer.c b/net/dsa/tag_trailer.c
index b4f6db094409..68f664cb7517 100644
--- a/net/dsa/tag_trailer.c
+++ b/net/dsa/tag_trailer.c
@@ -61,12 +61,11 @@ static struct sk_buff *trailer_rcv(struct sk_buff *skb, struct net_device *dev,
 				   struct net_device *orig_dev)
 {
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
-	struct dsa_switch *ds;
+	struct dsa_port *cpu_dp = dsa_dst_get_cpu_dp(dst);
+	struct dsa_switch *ds = cpu_dp->ds;
 	u8 *trailer;
 	int source_port;
 
-	ds = dst->cpu_dp->ds;
-
 	if (skb_linearize(skb))
 		return NULL;
 
-- 
2.9.3

Re: [PATCH net-next 4/5] net: dsa: Introduce dsa_dst_get_cpu_dp()

[Vivien Didelot]

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

> Introduce a helper function which will return a reference to the CPU
> port used in a dsa_switch_tree. Right now this is a singleton, but this
> will change once we introduce multi-CPU port support, so ease the
> transition by converting the affected code paths.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

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

[PATCH net-next 5/5] net: dsa: Stop accessing ds->dst->cpu_dp in drivers

[Florian Fainelli]

Out of the few drivers that do access ds->dst->cpu_dp, there is only a
handful for which we cannot substitute that for either an existing and
equivalent piece of information (b53, bcm_sf2, qca8k), and there is only
one for which we need to introduce a helper: mt7530. We do introduce
dsa_ds_get_cpu_dp() which reads the CPU port from ds->cpu_port_mask.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c |  4 ++--
 drivers/net/dsa/bcm_sf2.c        | 10 ++++++----
 drivers/net/dsa/mt7530.c         |  4 +++-
 drivers/net/dsa/mv88e6060.c      |  3 ++-
 drivers/net/dsa/qca8k.c          |  2 +-
 include/net/dsa.h                |  6 ++++++
 6 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index e68d368e20ac..faec6fcacd31 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1341,7 +1341,7 @@ EXPORT_SYMBOL(b53_fdb_dump);
 int b53_br_join(struct dsa_switch *ds, int port, struct net_device *br)
 {
 	struct b53_device *dev = ds->priv;
-	s8 cpu_port = ds->dst->cpu_dp->index;
+	s8 cpu_port = dev->cpu_port;
 	u16 pvlan, reg;
 	unsigned int i;
 
@@ -1387,7 +1387,7 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *br)
 {
 	struct b53_device *dev = ds->priv;
 	struct b53_vlan *vl = &dev->vlans[0];
-	s8 cpu_port = ds->dst->cpu_dp->index;
+	s8 cpu_port = dev->cpu_port;
 	unsigned int i;
 	u16 pvlan, reg, pvid;
 
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 76e98e8ed315..9744100d0276 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -227,7 +227,7 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
 			      struct phy_device *phy)
 {
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
-	s8 cpu_port = ds->dst->cpu_dp->index;
+	s8 cpu_port = priv->dev->cpu_port;
 	unsigned int i;
 	u32 reg;
 
@@ -806,8 +806,9 @@ static int bcm_sf2_sw_resume(struct dsa_switch *ds)
 static void bcm_sf2_sw_get_wol(struct dsa_switch *ds, int port,
 			       struct ethtool_wolinfo *wol)
 {
-	struct net_device *p = ds->dst[ds->index].cpu_dp->netdev;
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
+	struct dsa_port *cpu_dp = ds->ports[port].cpu_dp;
+	struct net_device *p = cpu_dp->netdev;
 	struct ethtool_wolinfo pwol;
 
 	/* Get the parent device WoL settings */
@@ -829,9 +830,10 @@ static void bcm_sf2_sw_get_wol(struct dsa_switch *ds, int port,
 static int bcm_sf2_sw_set_wol(struct dsa_switch *ds, int port,
 			      struct ethtool_wolinfo *wol)
 {
-	struct net_device *p = ds->dst[ds->index].cpu_dp->netdev;
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
-	s8 cpu_port = ds->dst->cpu_dp->index;
+	struct dsa_port *cpu_dp = ds->ports[port].cpu_dp;
+	struct net_device *p = cpu_dp->netdev;
+	s8 cpu_port = cpu_dp->index;
 	struct ethtool_wolinfo pwol;
 
 	p->ethtool_ops->get_wol(p, &pwol);
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 1e46418a3b74..9b1b76c7b927 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -907,6 +907,7 @@ static int
 mt7530_setup(struct dsa_switch *ds)
 {
 	struct mt7530_priv *priv = ds->priv;
+	struct dsa_port *cpu_dp;
 	int ret, i;
 	u32 id, val;
 	struct device_node *dn;
@@ -916,7 +917,8 @@ mt7530_setup(struct dsa_switch *ds)
 	 * controller also is the container for two GMACs nodes representing
 	 * as two netdev instances.
 	 */
-	dn = ds->dst->cpu_dp->netdev->dev.of_node->parent;
+	cpu_dp = dsa_ds_get_cpu_dp(ds);
+	dn = cpu_dp->netdev->dev.of_node->parent;
 	priv->ethernet = syscon_node_to_regmap(dn);
 	if (IS_ERR(priv->ethernet))
 		return PTR_ERR(priv->ethernet);
diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
index dce7fa57eb55..76fcd4350679 100644
--- a/drivers/net/dsa/mv88e6060.c
+++ b/drivers/net/dsa/mv88e6060.c
@@ -153,6 +153,7 @@ static int mv88e6060_setup_global(struct dsa_switch *ds)
 
 static int mv88e6060_setup_port(struct dsa_switch *ds, int p)
 {
+	struct dsa_port *cpu_dp = dsa_ds_get_cpu_dp(ds);
 	int addr = REG_PORT(p);
 
 	/* Do not force flow control, disable Ingress and Egress
@@ -176,7 +177,7 @@ static int mv88e6060_setup_port(struct dsa_switch *ds, int p)
 		  ((p & 0xf) << PORT_VLAN_MAP_DBNUM_SHIFT) |
 		   (dsa_is_cpu_port(ds, p) ?
 			ds->enabled_port_mask :
-			BIT(ds->dst->cpu_dp->index)));
+			BIT(cpu_dp->index)));
 
 	/* Port Association Vector: when learning source addresses
 	 * of packets, add the address to the address database using
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index b3bee7eab45f..68b45298f6d1 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -506,7 +506,7 @@ qca8k_setup(struct dsa_switch *ds)
 		pr_warn("regmap initialization failed");
 
 	/* Initialize CPU port pad mode (xMII type, delays...) */
-	phy_mode = of_get_phy_mode(ds->dst->cpu_dp->dn);
+	phy_mode = of_get_phy_mode(ds->ports[QCA8K_CPU_PORT].dn);
 	if (phy_mode < 0) {
 		pr_err("Can't find phy-mode for master device\n");
 		return phy_mode;
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 58969b9a090c..264cb3f93764 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -11,6 +11,7 @@
 #ifndef __LINUX_NET_DSA_H
 #define __LINUX_NET_DSA_H
 
+#include <linux/bitops.h>
 #include <linux/if.h>
 #include <linux/if_ether.h>
 #include <linux/list.h>
@@ -261,6 +262,11 @@ static inline bool dsa_is_port_initialized(struct dsa_switch *ds, int p)
 	return ds->enabled_port_mask & (1 << p) && ds->ports[p].netdev;
 }
 
+static inline struct dsa_port *dsa_ds_get_cpu_dp(struct dsa_switch *ds)
+{
+	return &ds->ports[fls(ds->cpu_port_mask) - 1];
+}
+
 static inline u8 dsa_upstream_port(struct dsa_switch *ds)
 {
 	struct dsa_switch_tree *dst = ds->dst;
-- 
2.9.3

Re: [PATCH net-next 5/5] net: dsa: Stop accessing ds->dst->cpu_dp in drivers

[Vivien Didelot]

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

> -	phy_mode = of_get_phy_mode(ds->dst->cpu_dp->dn);
> +	phy_mode = of_get_phy_mode(ds->ports[QCA8K_CPU_PORT].dn);

Is it necessary to use QCA8K_CPU_PORT?

> +static inline struct dsa_port *dsa_ds_get_cpu_dp(struct dsa_switch *ds)
> +{
> +	return &ds->ports[fls(ds->cpu_port_mask) - 1];
> +}

Wouldn't it be better to return the CPU port for a given port?
Something like return ds->ports[port].cpu_dp, so that we ease the
introduction of multiple CPU port a bit more?

Thanks,

        Vivien

Re: [PATCH net-next 5/5] net: dsa: Stop accessing ds->dst->cpu_dp in drivers

[Florian Fainelli]

On 06/06/2017 11:09 AM, Vivien Didelot wrote:
> Florian Fainelli <f.fainelli@gmail.com> writes:
> 
>> -	phy_mode = of_get_phy_mode(ds->dst->cpu_dp->dn);
>> +	phy_mode = of_get_phy_mode(ds->ports[QCA8K_CPU_PORT].dn);
> 
> Is it necessary to use QCA8K_CPU_PORT?
> 
>> +static inline struct dsa_port *dsa_ds_get_cpu_dp(struct dsa_switch *ds)
>> +{
>> +	return &ds->ports[fls(ds->cpu_port_mask) - 1];
>> +}
> 
> Wouldn't it be better to return the CPU port for a given port?
> Something like return ds->ports[port].cpu_dp, so that we ease the
> introduction of multiple CPU port a bit more?

ds->ports[port].cpu_dp only gets assigned at dsa_slave_create() time,
which is after ops->setup() has been called, hence this helper function
in case you need it earlier (e.g: like mv88e6060).
-- 
Florian

Re: [PATCH net-next 5/5] net: dsa: Stop accessing ds->dst->cpu_dp in drivers

[Vivien Didelot]

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

> On 06/06/2017 11:09 AM, Vivien Didelot wrote:
>> Florian Fainelli <f.fainelli@gmail.com> writes:
>> 
>>> -	phy_mode = of_get_phy_mode(ds->dst->cpu_dp->dn);
>>> +	phy_mode = of_get_phy_mode(ds->ports[QCA8K_CPU_PORT].dn);
>> 
>> Is it necessary to use QCA8K_CPU_PORT?
>> 
>>> +static inline struct dsa_port *dsa_ds_get_cpu_dp(struct dsa_switch *ds)
>>> +{
>>> +	return &ds->ports[fls(ds->cpu_port_mask) - 1];
>>> +}
>> 
>> Wouldn't it be better to return the CPU port for a given port?
>> Something like return ds->ports[port].cpu_dp, so that we ease the
>> introduction of multiple CPU port a bit more?
>
> ds->ports[port].cpu_dp only gets assigned at dsa_slave_create() time,
> which is after ops->setup() has been called, hence this helper function
> in case you need it earlier (e.g: like mv88e6060).

I see no reason why we cannot assign ds->ports[x].cpu_dp before calling
ops->setup().

Even though it can be changed later, the DSA core can assign a dedicated
CPU port to each ports and only after that, call into drivers ops.

To me the DSA topology (dsa_ports, CPU, etc.) should be determined
before calling into the hardware and exposing slave interfaces to the
user. Otherwise it is confusing. Am I wrong?

Thanks,

        Vivien

Re: [PATCH net-next 0/5] net: dsa: Multi-CPU ground work

[Vivien Didelot]

Hi Florian,

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

> This patch series prepares the ground for adding mutliple CPU port support to

                                                   multiple
                                                   
> DSA, and starts by removing redundant pieces of information such as
> master_netdev which is cpu_dp->ethernet. Finally drivers are moved away from

                         cpu_dp->netdev
                         
> directly accessing ds->dst->cpu_dp and use appropriate helper functions.
>
> Note that if you have Device Tree blobs/platform configurations that are
> currently listing multiple CPU ports, the proposed behavior in
> dsa_ds_get_cpu_dp() will be to return the last bit set in ds->cpu_port_mask.
>
> Future plans include:
> - making dst->cpu_dp a flexible data structure (array, list, you name it)
> - having the ability for drivers to return a default/preferred CPU port (if
>   necessary)

The overall patchset looks good. I have questions for future work
though.

I am still not sure that we need CPU port references in
dsa_switch_tree. When device tree or pdata is parsed, we have allocated
dsa_switch and dsa_port structures. We should be able validate and
assign all ds->ports[x].cpu_dp, before setting up the switches and
creating the slave devices. What do you think?

Also I see dsa_ptr becoming a pointer to the assosicated dsa_port, and
dsa_port should contain the tagging ops for quick access. That is more
rigourous with the physical representation and much easier for
transparent multi-CPU port support.

Thanks,

        Vivien

Re: [PATCH net-next 0/5] net: dsa: Multi-CPU ground work

[Florian Fainelli]

On 06/06/2017 11:25 AM, Vivien Didelot wrote:
> Hi Florian,
> 
> Florian Fainelli <f.fainelli@gmail.com> writes:
> 
>> This patch series prepares the ground for adding mutliple CPU port support to
> 
>                                                    multiple
>                                                    
>> DSA, and starts by removing redundant pieces of information such as
>> master_netdev which is cpu_dp->ethernet. Finally drivers are moved away from
> 
>                          cpu_dp->netdev
>                          
>> directly accessing ds->dst->cpu_dp and use appropriate helper functions.
>>
>> Note that if you have Device Tree blobs/platform configurations that are
>> currently listing multiple CPU ports, the proposed behavior in
>> dsa_ds_get_cpu_dp() will be to return the last bit set in ds->cpu_port_mask.
>>
>> Future plans include:
>> - making dst->cpu_dp a flexible data structure (array, list, you name it)
>> - having the ability for drivers to return a default/preferred CPU port (if
>>   necessary)
> 
> The overall patchset looks good. I have questions for future work
> though.
> 
> I am still not sure that we need CPU port references in
> dsa_switch_tree. When device tree or pdata is parsed, we have allocated
> dsa_switch and dsa_port structures. We should be able validate and
> assign all ds->ports[x].cpu_dp, before setting up the switches and
> creating the slave devices. What do you think?

True, we should be able to do that, thanks for the suggestion.

> 
> Also I see dsa_ptr becoming a pointer to the assosicated dsa_port, and
> dsa_port should contain the tagging ops for quick access. That is more
> rigourous with the physical representation and much easier for
> transparent multi-CPU port support.

Ultimately, I agree we should probably have dev->dsa_ptr be the actual
CPU port within the switch, and from the switch be able to go to the
collection of switches (dst). This should indeed be a bit more optimized
as there should be less traversal of structures in such a case.

Thanks!
-- 
Florian