Re: [PATCH net-next 15/17] net: dsa: Add new binding implementation

[Florian Fainelli]

On 06/03/2016 09:44 AM, Andrew Lunn wrote:
> The existing DSA binding has a number of limitations and problems. The
> main problem is that it cannot represent a switch as a linux device,
> hanging off some bus. It is limited to one CPU port. The DSA platform
> device is artificial, and does not really represent hardware.
> 
> Implement a new binding which can be embedded into any type of node on
> a bus to represent one switch device, and its links to other switches.
> 
> Signed-off-by: Andrew Lunn 
> Signed-off-by: Florian Fainelli 

Just a few nits that I had not seen before...


> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 6c314f300424..d8cb2acd4f0a 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -294,6 +294,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, 
> struct device *parent)
>   }
>   dst->cpu_switch = index;
>   dst->cpu_port = i;
> + ds->cpu_port_mask |= 1 << i;
>   } else if (!strcmp(name, "dsa")) {
>   ds->dsa_port_mask |= 1 << i;
>   } else {

We might want to undo setting the cpu_port_mask bit in
dsa_cpu_dsa_destroy()?

[snip]

> +static int dsa_ds_complete(struct dsa_switch_tree *dst, struct dsa_switch 
> *ds)
> +{
> + struct device_node *port;
> + u32 index;
> + int err;
> +
> + for (index = 0; index < DSA_MAX_PORTS; index++) {
> + port = ds->ports[index].dn;
> + if (!port)
> + continue;
> +
> + if (!dsa_port_is_dsa(port))
> + continue;
> +
> + ds->dsa_port_mask |= 1 << index;
> +
> + err = dsa_port_complete(dst, ds, port, index);
> + if (err != 0)

Should we move ds->dsa_port_mask |= 1 << index into dsa_port_complete,
for a) symetry with code undoing this, and b) avoid letting this bit be
set in case dsa_port_complete() returns an error?
-- 
Florian

Re: [PATCH net-next 15/17] net: dsa: Add new binding implementation

[Vivien Didelot]

Andrew Lunn  writes:

> The existing DSA binding has a number of limitations and problems. The
> main problem is that it cannot represent a switch as a linux device,
> hanging off some bus. It is limited to one CPU port. The DSA platform
> device is artificial, and does not really represent hardware.
>
> Implement a new binding which can be embedded into any type of node on
> a bus to represent one switch device, and its links to other switches.
>
> Signed-off-by: Andrew Lunn 
> Signed-off-by: Florian Fainelli 

Reviewed-by: Vivien Didelot 

[PATCH net-next 15/17] net: dsa: Add new binding implementation

[Andrew Lunn]

The existing DSA binding has a number of limitations and problems. The
main problem is that it cannot represent a switch as a linux device,
hanging off some bus. It is limited to one CPU port. The DSA platform
device is artificial, and does not really represent hardware.

Implement a new binding which can be embedded into any type of node on
a bus to represent one switch device, and its links to other switches.

Signed-off-by: Andrew Lunn 
Signed-off-by: Florian Fainelli 
---
 drivers/net/dsa/mv88e6xxx.c |   7 +
 include/net/dsa.h   |  20 ++
 net/dsa/Makefile|   2 +-
 net/dsa/dsa.c   |   1 +
 net/dsa/dsa2.c  | 653 
 net/dsa/dsa_priv.h  |   2 +-
 net/dsa/slave.c |   8 +-
 7 files changed, 689 insertions(+), 4 deletions(-)
 create mode 100644 net/dsa/dsa2.c

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 192b39cbdbcd..ee06055444f9 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3748,6 +3748,12 @@ int mv88e6xxx_probe(struct mdio_device *mdiodev)
 
dev_set_drvdata(dev, ds);
 
+   err = dsa_register_switch(ds, mdiodev->dev.of_node);
+   if (err) {
+   mv88e6xxx_mdio_unregister(ps);
+   return err;
+   }
+
dev_info(dev, "switch 0x%x probed: %s, revision %u\n",
 prod_num, ps->info->name, rev);
 
@@ -3759,6 +3765,7 @@ static void mv88e6xxx_remove(struct mdio_device *mdiodev)
struct dsa_switch *ds = dev_get_drvdata(>dev);
struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
+   dsa_unregister_switch(ds);
put_device(>bus->dev);
 
mv88e6xxx_mdio_unregister(ps);
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 11e8f09d32e3..d6ed5dee73e5 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -85,6 +85,17 @@ struct dsa_platform_data {
 struct packet_type;
 
 struct dsa_switch_tree {
+   struct list_headlist;
+
+   /* Tree identifier */
+   u32 tree;
+
+   /* Number of switches attached to this tree */
+   struct kref refcount;
+
+   /* Has this tree been applied to the hardware? */
+   bool applied;
+
/*
 * Configuration data for the platform device that owns
 * this dsa switch tree instance.
@@ -171,9 +182,15 @@ struct dsa_switch {
 #endif
 
/*
+* 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;
+   u32 cpu_port_mask;
u32 enabled_port_mask;
u32 phys_mii_mask;
struct dsa_port ports[DSA_MAX_PORTS];
@@ -362,4 +379,7 @@ static inline bool dsa_uses_tagged_protocol(struct 
dsa_switch_tree *dst)
 {
return dst->rcv != NULL;
 }
+
+void dsa_unregister_switch(struct dsa_switch *ds);
+int dsa_register_switch(struct dsa_switch *ds, struct device_node *np);
 #endif
diff --git a/net/dsa/Makefile b/net/dsa/Makefile
index da06ed1df620..8af4ded70f1c 100644
--- a/net/dsa/Makefile
+++ b/net/dsa/Makefile
@@ -1,6 +1,6 @@
 # the core
 obj-$(CONFIG_NET_DSA) += dsa_core.o
-dsa_core-y += dsa.o slave.o
+dsa_core-y += dsa.o slave.o dsa2.o
 
 # tagging formats
 dsa_core-$(CONFIG_NET_DSA_TAG_BRCM) += tag_brcm.o
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 6c314f300424..d8cb2acd4f0a 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -294,6 +294,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, 
struct device *parent)
}
dst->cpu_switch = index;
dst->cpu_port = i;
+   ds->cpu_port_mask |= 1 << i;
} else if (!strcmp(name, "dsa")) {
ds->dsa_port_mask |= 1 << i;
} else {
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
new file mode 100644
index ..4e5051bed643
--- /dev/null
+++ b/net/dsa/dsa2.c
@@ -0,0 +1,653 @@
+/*
+ * net/dsa/dsa2.c - Hardware switch handling, binding version 2
+ * Copyright (c) 2008-2009 Marvell Semiconductor
+ * Copyright (c) 2013 Florian Fainelli 
+ * Copyright (c) 2016 Andrew Lunn 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "dsa_priv.h"
+
+static LIST_HEAD(dsa_switch_trees);
+static DEFINE_MUTEX(dsa2_mutex);
+
+static struct dsa_switch_tree *dsa_get_dst(u32 tree)
+{
+   struct dsa_switch_tree *dst;
+
+