diff mbox

[RFC,3/4] net-next: dsa: Add support for multiple cpu ports.

Message ID 1483515484-21793-4-git-send-email-john@phrozen.org
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

John Crispin Jan. 4, 2017, 7:38 a.m. UTC 
From: Andrew Lunn <andrew@lunn.ch>

Some boards have two CPU interfaces connected to the switch, e.g. WiFi
access points, with 1 port labeled WAN, 4 ports labeled lan1-lan4, and
two port connected to the SoC.

This patch extends DSA to allows both CPU ports to be used. The "cpu"
node in the DSA tree can now have a phandle to the host interface it
connects to. Each user port can have a phandle to a cpu port which
should be used for traffic between the port and the CPU. Thus simple
load sharing over the two CPU ports can be achieved.

Signed-off-by: John Crispin <john@phrozen.org>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 include/net/dsa.h  |   21 ++++++++++++++++++++-
 net/dsa/dsa2.c     |   36 ++++++++++++++++++++++++++++++------
 net/dsa/dsa_priv.h |    5 +++++
 net/dsa/slave.c    |   27 ++++++++++++++++-----------
 4 files changed, 71 insertions(+), 18 deletions(-)

Comments

Andrew Lunn Jan. 4, 2017, 1:22 p.m. UTC | #1 
On Wed, Jan 04, 2017 at 08:38:03AM +0100, John Crispin wrote:
>  static int dsa_user_port_apply(struct device_node *port, u32 index,
> @@ -475,6 +475,28 @@ static int dsa_cpu_parse(struct device_node *port, u32 index,
>  
>  	dst->rcv = dst->tag_ops->rcv;
>  
> +	dev_hold(ethernet_dev);
> +	ds->cd->port_ethernet[index] = ethernet_dev;
> +
> +	return 0;
> +}
> +
> +static int dsa_user_parse(struct device_node *port, u32 index,
> +			  struct dsa_switch *ds)
> +{

Please put this function next to dsa_cpu_parse(). All the
apply/unapply functions are together, and all the _parse functions
should be together.

> +	struct device_node *cpu_port;
> +	const unsigned int *cpu_port_reg;
> +	int cpu_port_index;
> +
> +	cpu_port = of_parse_phandle(port, "cpu", 0);
> +	if (cpu_port) {
> +		cpu_port_reg = of_get_property(cpu_port, "reg", NULL);
> +		if (!cpu_port_reg)
> +			return -EINVAL;
> +		cpu_port_index = be32_to_cpup(cpu_port_reg);
> +		ds->cd->port_cpu[index] = cpu_port_index;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -482,18 +504,20 @@ static int dsa_ds_parse(struct dsa_switch_tree *dst, struct dsa_switch *ds)
>  {
>  	struct device_node *port;
>  	u32 index;
> -	int err;
> +	int err = 0;
>  
>  	for (index = 0; index < DSA_MAX_PORTS; index++) {
>  		port = ds->ports[index].dn;
>  		if (!port)
>  			continue;
>  
> -		if (dsa_port_is_cpu(port)) {
> +		if (dsa_port_is_cpu(port))
>  			err = dsa_cpu_parse(port, index, dst, ds);
> -			if (err)
> -				return err;
> -		}
> +		else if (!dsa_port_is_dsa(port))
> +			err = dsa_user_parse(port, index,  ds);
> +
> +		if (err)
> +			return err;

Having this if (err) here is correct, but it goes against the general
pattern we have in the code. Please indent it so it is under the
err =, and remove the initialisation of err.

Also, if one branch of an if/else has {}, the coding style says the
other branch should also use {}.

Just to make this code look nicer, i would be tempted to add a helper,
dsa_port_is_user().

>  	}
>  
>  	pr_info("DSA: switch %d %d parsed\n", dst->tree, ds->index);

Thanks
	Andrew
Andrew Lunn Jan. 4, 2017, 2:01 p.m. UTC | #2 
On Wed, Jan 04, 2017 at 08:38:03AM +0100, John Crispin wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> 
> Some boards have two CPU interfaces connected to the switch, e.g. WiFi
> access points, with 1 port labeled WAN, 4 ports labeled lan1-lan4, and
> two port connected to the SoC.
> 
> This patch extends DSA to allows both CPU ports to be used. The "cpu"
> node in the DSA tree can now have a phandle to the host interface it
> connects to. Each user port can have a phandle to a cpu port which
> should be used for traffic between the port and the CPU. Thus simple
> load sharing over the two CPU ports can be achieved.
> 
> Signed-off-by: John Crispin <john@phrozen.org>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  include/net/dsa.h  |   21 ++++++++++++++++++++-
>  net/dsa/dsa2.c     |   36 ++++++++++++++++++++++++++++++------
>  net/dsa/dsa_priv.h |    5 +++++
>  net/dsa/slave.c    |   27 ++++++++++++++++-----------
>  4 files changed, 71 insertions(+), 18 deletions(-)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index b122196..f68180b 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -60,6 +60,8 @@ struct dsa_chip_data {
>  	 */
>  	char		*port_names[DSA_MAX_PORTS];
>  	struct device_node *port_dn[DSA_MAX_PORTS];
> +	struct net_device *port_ethernet[DSA_MAX_PORTS];
> +	int		port_cpu[DSA_MAX_PORTS];

Hi John

My proof of concept patches have "bit rotted" a bit. When implementing
dsa2, i removed the use of dsa_chip_data, aka cd, from all the drivers
and the new binding does not use it at all. I don't want to add it
back again. When Florain removes the old binding in 6 months time, i
expect dsa_chip_data and dsa_platform_data will be removed.

I would be tempted to put these two members into the dsa_port
structure.

	Andrew
Florian Fainelli Jan. 4, 2017, 2:40 p.m. UTC | #3 
On 01/04/2017 06:01 AM, Andrew Lunn wrote:
> On Wed, Jan 04, 2017 at 08:38:03AM +0100, John Crispin wrote:
>> From: Andrew Lunn <andrew@lunn.ch>
>>
>> Some boards have two CPU interfaces connected to the switch, e.g. WiFi
>> access points, with 1 port labeled WAN, 4 ports labeled lan1-lan4, and
>> two port connected to the SoC.
>>
>> This patch extends DSA to allows both CPU ports to be used. The "cpu"
>> node in the DSA tree can now have a phandle to the host interface it
>> connects to. Each user port can have a phandle to a cpu port which
>> should be used for traffic between the port and the CPU. Thus simple
>> load sharing over the two CPU ports can be achieved.
>>
>> Signed-off-by: John Crispin <john@phrozen.org>
>> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
>> ---
>>  include/net/dsa.h  |   21 ++++++++++++++++++++-
>>  net/dsa/dsa2.c     |   36 ++++++++++++++++++++++++++++++------
>>  net/dsa/dsa_priv.h |    5 +++++
>>  net/dsa/slave.c    |   27 ++++++++++++++++-----------
>>  4 files changed, 71 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/net/dsa.h b/include/net/dsa.h
>> index b122196..f68180b 100644
>> --- a/include/net/dsa.h
>> +++ b/include/net/dsa.h
>> @@ -60,6 +60,8 @@ struct dsa_chip_data {
>>  	 */
>>  	char		*port_names[DSA_MAX_PORTS];
>>  	struct device_node *port_dn[DSA_MAX_PORTS];
>> +	struct net_device *port_ethernet[DSA_MAX_PORTS];
>> +	int		port_cpu[DSA_MAX_PORTS];
> 
> Hi John
> 
> My proof of concept patches have "bit rotted" a bit. When implementing
> dsa2, i removed the use of dsa_chip_data, aka cd, from all the drivers
> and the new binding does not use it at all. I don't want to add it
> back again. When Florain removes the old binding in 6 months time, i
> expect dsa_chip_data and dsa_platform_data will be removed.

I am actually in the process of cleaning up my patches that add
platform_data support to net/dsa/dsa2.c this time with an user, and
hopefully a couple more after that, but that maintains the existing
struct dsa_chip_data as-is, no new additions are required, and this
would be purely for non-DT enabled platforms anyway.
Andrew Lunn Jan. 4, 2017, 3:22 p.m. UTC | #4 
> I am actually in the process of cleaning up my patches that add
> platform_data support to net/dsa/dsa2.c this time with an user, and
> hopefully a couple more after that, but that maintains the existing
> struct dsa_chip_data as-is, no new additions are required, and this
> would be purely for non-DT enabled platforms anyway.

Hi Florian

I was hoping we could avoid this complexity. But if there is a real
need, the platform cannot be converted to device tree, we can add it.

	Andrew
diff mbox

Patch

diff --git a/include/net/dsa.h b/include/net/dsa.h
index b122196..f68180b 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -60,6 +60,8 @@  struct dsa_chip_data {
 	 */
 	char		*port_names[DSA_MAX_PORTS];
 	struct device_node *port_dn[DSA_MAX_PORTS];
+	struct net_device *port_ethernet[DSA_MAX_PORTS];
+	int		port_cpu[DSA_MAX_PORTS];
 
 	/*
 	 * An array of which element [a] indicates which port on this
@@ -204,7 +206,7 @@  struct dsa_switch {
 
 static inline bool dsa_is_cpu_port(struct dsa_switch *ds, int p)
 {
-	return !!(ds->index == ds->dst->cpu_switch && p == ds->dst->cpu_port);
+	return !!(ds->cpu_port_mask & (1 << p));
 }
 
 static inline bool dsa_is_dsa_port(struct dsa_switch *ds, int p)
@@ -217,6 +219,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 bool dsa_is_upstream_port(struct dsa_switch *ds, int p)
+{
+	return dsa_is_cpu_port(ds, p) || dsa_is_dsa_port(ds, p);
+}
+
 static inline u8 dsa_upstream_port(struct dsa_switch *ds)
 {
 	struct dsa_switch_tree *dst = ds->dst;
@@ -233,6 +240,18 @@  static inline u8 dsa_upstream_port(struct dsa_switch *ds)
 		return ds->rtable[dst->cpu_switch];
 }
 
+static inline u8 dsa_port_upstream_port(struct dsa_switch *ds, int port)
+{
+	/*
+	 * If this port has a specific upstream cpu port, use it,
+	 * otherwise use the switch default.
+	 */
+	if (ds->cd->port_cpu[port])
+		return ds->cd->port_cpu[port];
+	else
+		return dsa_upstream_port(ds);
+}
+
 struct switchdev_trans;
 struct switchdev_obj;
 struct switchdev_obj_port_fdb;
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 5fff951..1763cd4 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -258,7 +258,7 @@  static void dsa_cpu_port_unapply(struct device_node *port, u32 index,
 {
 	dsa_cpu_dsa_destroy(port);
 	ds->cpu_port_mask &= ~BIT(index);
-
+	dev_put(ds->cd->port_ethernet[index]);
 }
 
 static int dsa_user_port_apply(struct device_node *port, u32 index,
@@ -475,6 +475,28 @@  static int dsa_cpu_parse(struct device_node *port, u32 index,
 
 	dst->rcv = dst->tag_ops->rcv;
 
+	dev_hold(ethernet_dev);
+	ds->cd->port_ethernet[index] = ethernet_dev;
+
+	return 0;
+}
+
+static int dsa_user_parse(struct device_node *port, u32 index,
+			  struct dsa_switch *ds)
+{
+	struct device_node *cpu_port;
+	const unsigned int *cpu_port_reg;
+	int cpu_port_index;
+
+	cpu_port = of_parse_phandle(port, "cpu", 0);
+	if (cpu_port) {
+		cpu_port_reg = of_get_property(cpu_port, "reg", NULL);
+		if (!cpu_port_reg)
+			return -EINVAL;
+		cpu_port_index = be32_to_cpup(cpu_port_reg);
+		ds->cd->port_cpu[index] = cpu_port_index;
+	}
+
 	return 0;
 }
 
@@ -482,18 +504,20 @@  static int dsa_ds_parse(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 {
 	struct device_node *port;
 	u32 index;
-	int err;
+	int err = 0;
 
 	for (index = 0; index < DSA_MAX_PORTS; index++) {
 		port = ds->ports[index].dn;
 		if (!port)
 			continue;
 
-		if (dsa_port_is_cpu(port)) {
+		if (dsa_port_is_cpu(port))
 			err = dsa_cpu_parse(port, index, dst, ds);
-			if (err)
-				return err;
-		}
+		else if (!dsa_port_is_dsa(port))
+			err = dsa_user_parse(port, index,  ds);
+
+		if (err)
+			return err;
 	}
 
 	pr_info("DSA: switch %d %d parsed\n", dst->tree, ds->index);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 6cfd738..7e1e62c 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -24,6 +24,11 @@  struct dsa_device_ops {
 struct dsa_slave_priv {
 	struct sk_buff *	(*xmit)(struct sk_buff *skb,
 					struct net_device *dev);
+	/*
+	 * Which host device do we used to send packets to the switch
+	 * for this port.
+	 */
+	struct net_device	*master;
 
 	/*
 	 * Which switch this port is a part of, and the port index
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index ffd91969..260d4a9 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -61,7 +61,7 @@  static int dsa_slave_get_iflink(const struct net_device *dev)
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
 
-	return p->parent->dst->master_netdev->ifindex;
+	return p->master->ifindex;
 }
 
 static inline bool dsa_port_is_bridged(struct dsa_slave_priv *p)
@@ -96,7 +96,7 @@  static void dsa_port_set_stp_state(struct dsa_switch *ds, int port, u8 state)
 static int dsa_slave_open(struct net_device *dev)
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct net_device *master = p->parent->dst->master_netdev;
+	struct net_device *master = p->master;
 	struct dsa_switch *ds = p->parent;
 	u8 stp_state = dsa_port_is_bridged(p) ?
 			BR_STATE_BLOCKING : BR_STATE_FORWARDING;
@@ -151,7 +151,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->parent->dst->master_netdev;
+	struct net_device *master = p->master;
 	struct dsa_switch *ds = p->parent;
 
 	if (p->phy)
@@ -178,7 +178,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->parent->dst->master_netdev;
+	struct net_device *master = p->master;
 
 	if (change & IFF_ALLMULTI)
 		dev_set_allmulti(master, dev->flags & IFF_ALLMULTI ? 1 : -1);
@@ -189,7 +189,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->parent->dst->master_netdev;
+	struct net_device *master = p->master;
 
 	dev_mc_sync(master, dev);
 	dev_uc_sync(master, dev);
@@ -198,7 +198,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->parent->dst->master_netdev;
+	struct net_device *master = p->master;
 	struct sockaddr *addr = a;
 	int err;
 
@@ -633,7 +633,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->parent->dst->master_netdev;
+	nskb->dev = p->master;
 	dev_queue_xmit(nskb);
 
 	return NETDEV_TX_OK;
@@ -947,7 +947,7 @@  static int dsa_slave_netpoll_setup(struct net_device *dev,
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
 	struct dsa_switch *ds = p->parent;
-	struct net_device *master = ds->dst->master_netdev;
+	struct net_device *master = p->master;
 	struct netpoll *netpoll;
 	int err = 0;
 
@@ -1247,12 +1247,16 @@  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;
+	int port_cpu = ds->cd->port_cpu[port];
 	int ret;
 
-	master = ds->dst->master_netdev;
-	if (ds->master_netdev)
+	if (port_cpu && ds->cd->port_ethernet[port_cpu])
+		master = ds->cd->port_ethernet[port_cpu];
+	else if (ds->master_netdev)
 		master = ds->master_netdev;
-
+	else
+		master = ds->dst->master_netdev;
+	master->dsa_ptr = (void *)ds->dst;
 	slave_dev = alloc_netdev(sizeof(struct dsa_slave_priv), name,
 				 NET_NAME_UNKNOWN, ether_setup);
 	if (slave_dev == NULL)
@@ -1279,6 +1283,7 @@  int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
 	p->parent = ds;
 	p->port = port;
 	p->xmit = dst->tag_ops->xmit;
+	p->master = master;
 
 	p->old_pause = -1;
 	p->old_link = -1;