[PATCH net-next 00/11] net: dsa: parsing stage

[PATCH net-next 00/11] net: dsa: parsing stage

[Vivien Didelot]

When registering a DSA switch, there is basically two stages.

The first stage is the parsing of the switch device, from either device
tree or platform data. It fetches the DSA tree to which it belongs, and
validates its ports. The switch device is then added to the tree, and
the second stage is called if this was the last switch of the tree.

The second stage is the setup of the tree, which validates that the tree
is complete, sets up the routing tables, the default CPU port for user
ports, sets up the switch drivers and finally the master interfaces,
which makes the whole switch fabric functional.

This patch series covers the first parsing stage. It fixes the type of
the switch and tree indexes to unsigned int, simplifies the tree
reference counting and the switch and CPU ports parsing.

Vivien Didelot (11):
  net: dsa: make switch index unsigned
  net: dsa: make tree index unsigned
  net: dsa: simplify tree reference counting
  net: dsa: get and put tree reference counting
  net: dsa: provide a find or new tree helper
  net: dsa: rework switch addition and removal
  net: dsa: get tree before parsing ports
  net: dsa: rework switch parsing
  net: dsa: only check presence of link property
  net: dsa: add one port parsing function per type
  net: dsa: resolve tagging protocol at parse time

 include/net/dsa.h |   4 +-
 net/dsa/dsa2.c    | 323 ++++++++++++++++++++++++++++++------------------------
 net/dsa/slave.c   |   2 +-
 3 files changed, 184 insertions(+), 145 deletions(-)

-- 
2.14.3

[PATCH net-next 01/11] net: dsa: make switch index unsigned

[Vivien Didelot]

Define the DSA switch index as an unsigned int, because it will never be
less than 0.

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

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 50e276dc4c01..fa1c21ab8092 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -209,7 +209,7 @@ struct dsa_switch {
 	 * Parent switch tree, and switch index.
 	 */
 	struct dsa_switch_tree	*dst;
-	int			index;
+	unsigned int		index;
 
 	/* Listener for switch fabric events */
 	struct notifier_block	nb;
-- 
2.14.3

Re: [PATCH net-next 01/11] net: dsa: make switch index unsigned

[Florian Fainelli]

On 11/03/2017 04:05 PM, Vivien Didelot wrote:
> Define the DSA switch index as an unsigned int, because it will never be
> less than 0.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

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

[PATCH net-next 02/11] net: dsa: make tree index unsigned

[Vivien Didelot]

Similarly to a DSA switch and port, rename the tree index from "tree" to
"index" and make it an unsigned int because it isn't supposed to be less
than 0.

u32 is an OF specific data used to retrieve the value and has no need to
be propagated up to the tree index.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 include/net/dsa.h |  2 +-
 net/dsa/dsa2.c    | 14 +++++++-------
 net/dsa/slave.c   |  2 +-
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index fa1c21ab8092..e54332968417 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -116,7 +116,7 @@ struct dsa_switch_tree {
 	struct raw_notifier_head	nh;
 
 	/* Tree identifier */
-	u32 tree;
+	unsigned int index;
 
 	/* Number of switches attached to this tree */
 	struct kref refcount;
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 797d1156b4e6..8b68dc2f5707 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -27,12 +27,12 @@ static DEFINE_MUTEX(dsa2_mutex);
 static const struct devlink_ops dsa_devlink_ops = {
 };
 
-static struct dsa_switch_tree *dsa_get_dst(u32 tree)
+static struct dsa_switch_tree *dsa_get_dst(unsigned int index)
 {
 	struct dsa_switch_tree *dst;
 
 	list_for_each_entry(dst, &dsa_switch_trees, list)
-		if (dst->tree == tree) {
+		if (dst->index == index) {
 			kref_get(&dst->refcount);
 			return dst;
 		}
@@ -53,14 +53,14 @@ static void dsa_put_dst(struct dsa_switch_tree *dst)
 	kref_put(&dst->refcount, dsa_free_dst);
 }
 
-static struct dsa_switch_tree *dsa_add_dst(u32 tree)
+static struct dsa_switch_tree *dsa_add_dst(unsigned int index)
 {
 	struct dsa_switch_tree *dst;
 
 	dst = kzalloc(sizeof(*dst), GFP_KERNEL);
 	if (!dst)
 		return NULL;
-	dst->tree = tree;
+	dst->index = index;
 	INIT_LIST_HEAD(&dst->list);
 	list_add_tail(&dsa_switch_trees, &dst->list);
 	kref_init(&dst->refcount);
@@ -454,7 +454,7 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
 
 	dst->cpu_dp = NULL;
 
-	pr_info("DSA: tree %d unapplied\n", dst->tree);
+	pr_info("DSA: tree %d unapplied\n", dst->index);
 	dst->applied = false;
 }
 
@@ -504,7 +504,7 @@ static int dsa_ds_parse(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 
 	}
 
-	pr_info("DSA: switch %d %d parsed\n", dst->tree, ds->index);
+	pr_info("DSA: switch %d %d parsed\n", dst->index, ds->index);
 
 	return 0;
 }
@@ -549,7 +549,7 @@ static int dsa_dst_parse(struct dsa_switch_tree *dst)
 		}
 	}
 
-	pr_info("DSA: tree %d parsed\n", dst->tree);
+	pr_info("DSA: tree %d parsed\n", dst->index);
 
 	return 0;
 }
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 9b75d0ac4092..814ced75a0cc 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -55,7 +55,7 @@ void dsa_slave_mii_bus_init(struct dsa_switch *ds)
 	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.%d",
-		 ds->dst->tree, ds->index);
+		 ds->dst->index, ds->index);
 	ds->slave_mii_bus->parent = ds->dev;
 	ds->slave_mii_bus->phy_mask = ~ds->phys_mii_mask;
 }
-- 
2.14.3

Re: [PATCH net-next 02/11] net: dsa: make tree index unsigned

[Florian Fainelli]

On 11/03/2017 04:05 PM, Vivien Didelot wrote:
> Similarly to a DSA switch and port, rename the tree index from "tree" to
> "index" and make it an unsigned int because it isn't supposed to be less
> than 0.
> 
> u32 is an OF specific data used to retrieve the value and has no need to
> be propagated up to the tree index.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

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

[PATCH net-next 03/11] net: dsa: simplify tree reference counting

[Vivien Didelot]

DSA trees have a refcount used to automatically free the dsa_switch_tree
structure once there is no switch devices inside of it.

The refcount is incremented when a switch is added to the tree, and
decremented when it is removed from it.

But because of kref_init, the refcount is also incremented at
initialization, and when looking up the tree from the list for symmetry.

Thus the current code stores the number of switches plus one, and makes
the switch registration more complex.

To simplify the switch registration function, we reset the refcount to
zero after initialization and don't increment it when looking up a tree.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/dsa2.c | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 8b68dc2f5707..d3f1a7607463 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -32,10 +32,9 @@ static struct dsa_switch_tree *dsa_get_dst(unsigned int index)
 	struct dsa_switch_tree *dst;
 
 	list_for_each_entry(dst, &dsa_switch_trees, list)
-		if (dst->index == index) {
-			kref_get(&dst->refcount);
+		if (dst->index == index)
 			return dst;
-		}
+
 	return NULL;
 }
 
@@ -48,11 +47,6 @@ static void dsa_free_dst(struct kref *ref)
 	kfree(dst);
 }
 
-static void dsa_put_dst(struct dsa_switch_tree *dst)
-{
-	kref_put(&dst->refcount, dsa_free_dst);
-}
-
 static struct dsa_switch_tree *dsa_add_dst(unsigned int index)
 {
 	struct dsa_switch_tree *dst;
@@ -63,7 +57,10 @@ static struct dsa_switch_tree *dsa_add_dst(unsigned int index)
 	dst->index = index;
 	INIT_LIST_HEAD(&dst->list);
 	list_add_tail(&dsa_switch_trees, &dst->list);
+
+	/* Initialize the reference counter to the number of switches, not 1 */
 	kref_init(&dst->refcount);
+	refcount_set(&dst->refcount.refcount, 0);
 
 	return dst;
 }
@@ -739,10 +736,8 @@ static int _dsa_register_switch(struct dsa_switch *ds)
 			return -ENOMEM;
 	}
 
-	if (dst->ds[index]) {
-		err = -EBUSY;
-		goto out;
-	}
+	if (dst->ds[index])
+		return -EBUSY;
 
 	ds->dst = dst;
 	ds->index = index;
@@ -758,11 +753,9 @@ static int _dsa_register_switch(struct dsa_switch *ds)
 	if (err < 0)
 		goto out_del_dst;
 
-	if (err == 1) {
-		/* Not all switches registered yet */
-		err = 0;
-		goto out;
-	}
+	/* Not all switches registered yet */
+	if (err == 1)
+		return 0;
 
 	if (dst->applied) {
 		pr_info("DSA: Disjoint trees?\n");
@@ -779,13 +772,10 @@ static int _dsa_register_switch(struct dsa_switch *ds)
 		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;
 }
-- 
2.14.3

Re: [PATCH net-next 03/11] net: dsa: simplify tree reference counting

[Florian Fainelli]

On 11/03/2017 04:05 PM, Vivien Didelot wrote:
> DSA trees have a refcount used to automatically free the dsa_switch_tree
> structure once there is no switch devices inside of it.
> 
> The refcount is incremented when a switch is added to the tree, and
> decremented when it is removed from it.
> 
> But because of kref_init, the refcount is also incremented at
> initialization, and when looking up the tree from the list for symmetry.
> 
> Thus the current code stores the number of switches plus one, and makes
> the switch registration more complex.
> 
> To simplify the switch registration function, we reset the refcount to
> zero after initialization and don't increment it when looking up a tree.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

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

[PATCH net-next 04/11] net: dsa: get and put tree reference counting

[Vivien Didelot]

Provide convenient dsa_tree_get and dsa_tree_put functions scoping a DSA
tree used to increment and decrement its reference counter, instead of
poking directly its kref structure.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/dsa2.c | 40 ++++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index d3f1a7607463..609d92684505 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -38,15 +38,6 @@ static struct dsa_switch_tree *dsa_get_dst(unsigned int index)
 	return NULL;
 }
 
-static void dsa_free_dst(struct kref *ref)
-{
-	struct dsa_switch_tree *dst = container_of(ref, struct dsa_switch_tree,
-						   refcount);
-
-	list_del(&dst->list);
-	kfree(dst);
-}
-
 static struct dsa_switch_tree *dsa_add_dst(unsigned int index)
 {
 	struct dsa_switch_tree *dst;
@@ -65,10 +56,35 @@ static struct dsa_switch_tree *dsa_add_dst(unsigned int index)
 	return dst;
 }
 
-static void dsa_dst_add_ds(struct dsa_switch_tree *dst,
-			   struct dsa_switch *ds, u32 index)
+static void dsa_tree_free(struct dsa_switch_tree *dst)
+{
+	list_del(&dst->list);
+	kfree(dst);
+}
+
+static void dsa_tree_get(struct dsa_switch_tree *dst)
 {
 	kref_get(&dst->refcount);
+}
+
+static void dsa_tree_release(struct kref *ref)
+{
+	struct dsa_switch_tree *dst;
+
+	dst = container_of(ref, struct dsa_switch_tree, refcount);
+
+	dsa_tree_free(dst);
+}
+
+static void dsa_tree_put(struct dsa_switch_tree *dst)
+{
+	kref_put(&dst->refcount, dsa_tree_release);
+}
+
+static void dsa_dst_add_ds(struct dsa_switch_tree *dst,
+			   struct dsa_switch *ds, u32 index)
+{
+	dsa_tree_get(dst);
 	dst->ds[index] = ds;
 }
 
@@ -76,7 +92,7 @@ static void dsa_dst_del_ds(struct dsa_switch_tree *dst,
 			   struct dsa_switch *ds, u32 index)
 {
 	dst->ds[index] = NULL;
-	kref_put(&dst->refcount, dsa_free_dst);
+	dsa_tree_put(dst);
 }
 
 /* For platform data configurations, we need to have a valid name argument to
-- 
2.14.3

Re: [PATCH net-next 04/11] net: dsa: get and put tree reference counting

[Florian Fainelli]

On 11/03/2017 04:05 PM, Vivien Didelot wrote:
> Provide convenient dsa_tree_get and dsa_tree_put functions scoping a DSA
> tree used to increment and decrement its reference counter, instead of
> poking directly its kref structure.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

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

[PATCH net-next 05/11] net: dsa: provide a find or new tree helper

[Vivien Didelot]

Rename dsa_get_dst to dsa_tree_find since it doesn't increment the
reference counter, rename dsa_add_dst to dsa_tree_alloc for symmetry
with dsa_tree_free, and provide a convenient dsa_tree_touch function to
find or allocate a new tree.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/dsa2.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 609d92684505..bda222cfc02c 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -21,33 +21,35 @@
 
 #include "dsa_priv.h"
 
-static LIST_HEAD(dsa_switch_trees);
+static LIST_HEAD(dsa_tree_list);
 static DEFINE_MUTEX(dsa2_mutex);
 
 static const struct devlink_ops dsa_devlink_ops = {
 };
 
-static struct dsa_switch_tree *dsa_get_dst(unsigned int index)
+static struct dsa_switch_tree *dsa_tree_find(int index)
 {
 	struct dsa_switch_tree *dst;
 
-	list_for_each_entry(dst, &dsa_switch_trees, list)
+	list_for_each_entry(dst, &dsa_tree_list, list)
 		if (dst->index == index)
 			return dst;
 
 	return NULL;
 }
 
-static struct dsa_switch_tree *dsa_add_dst(unsigned int index)
+static struct dsa_switch_tree *dsa_tree_alloc(int index)
 {
 	struct dsa_switch_tree *dst;
 
 	dst = kzalloc(sizeof(*dst), GFP_KERNEL);
 	if (!dst)
 		return NULL;
+
 	dst->index = index;
+
 	INIT_LIST_HEAD(&dst->list);
-	list_add_tail(&dsa_switch_trees, &dst->list);
+	list_add_tail(&dsa_tree_list, &dst->list);
 
 	/* Initialize the reference counter to the number of switches, not 1 */
 	kref_init(&dst->refcount);
@@ -62,6 +64,17 @@ static void dsa_tree_free(struct dsa_switch_tree *dst)
 	kfree(dst);
 }
 
+static struct dsa_switch_tree *dsa_tree_touch(int index)
+{
+	struct dsa_switch_tree *dst;
+
+	dst = dsa_tree_find(index);
+	if (!dst)
+		dst = dsa_tree_alloc(index);
+
+	return dst;
+}
+
 static void dsa_tree_get(struct dsa_switch_tree *dst)
 {
 	kref_get(&dst->refcount);
@@ -745,12 +758,9 @@ static int _dsa_register_switch(struct dsa_switch *ds)
 			return err;
 	}
 
-	dst = dsa_get_dst(tree);
-	if (!dst) {
-		dst = dsa_add_dst(tree);
-		if (!dst)
-			return -ENOMEM;
-	}
+	dst = dsa_tree_touch(tree);
+	if (!dst)
+		return -ENOMEM;
 
 	if (dst->ds[index])
 		return -EBUSY;
-- 
2.14.3

Re: [PATCH net-next 05/11] net: dsa: provide a find or new tree helper

[Florian Fainelli]

On 11/03/2017 04:05 PM, Vivien Didelot wrote:
> Rename dsa_get_dst to dsa_tree_find since it doesn't increment the
> reference counter, rename dsa_add_dst to dsa_tree_alloc for symmetry
> with dsa_tree_free, and provide a convenient dsa_tree_touch function to
> find or allocate a new tree.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

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

[PATCH net-next 06/11] net: dsa: rework switch addition and removal

[Vivien Didelot]

This patch removes the unnecessary index argument from the
dsa_dst_add_ds and dsa_dst_del_ds functions and renames them to
dsa_tree_add_switch and dsa_tree_remove_switch respectively.

In addition to a more explicit scope, we now check the presence of an
existing switch with the same index directly within dsa_tree_add_switch.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/dsa2.c | 47 +++++++++++++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index bda222cfc02c..5b6a3dad8015 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -94,20 +94,6 @@ static void dsa_tree_put(struct dsa_switch_tree *dst)
 	kref_put(&dst->refcount, dsa_tree_release);
 }
 
-static void dsa_dst_add_ds(struct dsa_switch_tree *dst,
-			   struct dsa_switch *ds, u32 index)
-{
-	dsa_tree_get(dst);
-	dst->ds[index] = ds;
-}
-
-static void dsa_dst_del_ds(struct dsa_switch_tree *dst,
-			   struct dsa_switch *ds, u32 index)
-{
-	dst->ds[index] = NULL;
-	dsa_tree_put(dst);
-}
-
 /* For platform data configurations, we need to have a valid name argument to
  * differentiate a disabled port from an enabled one
  */
@@ -484,6 +470,27 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
 	dst->applied = false;
 }
 
+static void dsa_tree_remove_switch(struct dsa_switch_tree *dst,
+				   unsigned int index)
+{
+	dst->ds[index] = NULL;
+	dsa_tree_put(dst);
+}
+
+static int dsa_tree_add_switch(struct dsa_switch_tree *dst,
+			       struct dsa_switch *ds)
+{
+	unsigned int index = ds->index;
+
+	if (dst->ds[index])
+		return -EBUSY;
+
+	dsa_tree_get(dst);
+	dst->ds[index] = ds;
+
+	return 0;
+}
+
 static int dsa_cpu_parse(struct dsa_port *port, u32 index,
 			 struct dsa_switch_tree *dst,
 			 struct dsa_switch *ds)
@@ -762,9 +769,6 @@ static int _dsa_register_switch(struct dsa_switch *ds)
 	if (!dst)
 		return -ENOMEM;
 
-	if (dst->ds[index])
-		return -EBUSY;
-
 	ds->dst = dst;
 	ds->index = index;
 	ds->cd = pdata;
@@ -773,7 +777,9 @@ static int _dsa_register_switch(struct dsa_switch *ds)
 	for (i = 0; i < DSA_MAX_SWITCHES; ++i)
 		ds->rtable[i] = DSA_RTABLE_NONE;
 
-	dsa_dst_add_ds(dst, ds, index);
+	err = dsa_tree_add_switch(dst, ds);
+	if (err)
+		return err;
 
 	err = dsa_dst_complete(dst);
 	if (err < 0)
@@ -801,7 +807,7 @@ static int _dsa_register_switch(struct dsa_switch *ds)
 	return 0;
 
 out_del_dst:
-	dsa_dst_del_ds(dst, ds, ds->index);
+	dsa_tree_remove_switch(dst, index);
 
 	return err;
 }
@@ -843,10 +849,11 @@ EXPORT_SYMBOL_GPL(dsa_register_switch);
 static void _dsa_unregister_switch(struct dsa_switch *ds)
 {
 	struct dsa_switch_tree *dst = ds->dst;
+	unsigned int index = ds->index;
 
 	dsa_dst_unapply(dst);
 
-	dsa_dst_del_ds(dst, ds, ds->index);
+	dsa_tree_remove_switch(dst, index);
 }
 
 void dsa_unregister_switch(struct dsa_switch *ds)
-- 
2.14.3

[PATCH net-next 07/11] net: dsa: get tree before parsing ports

[Vivien Didelot]

We will need a reference to the dsa_switch_tree when parsing a CPU port,
so fetch it right after parsing the member and before parsing ports.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/dsa2.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 5b6a3dad8015..5918fbddb0ab 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -751,18 +751,10 @@ static int _dsa_register_switch(struct dsa_switch *ds)
 		err = dsa_parse_member_dn(np, &tree, &index);
 		if (err)
 			return err;
-
-		err = dsa_parse_ports_of(np, ds);
-		if (err)
-			return err;
 	} else {
 		err = dsa_parse_member(pdata, &tree, &index);
 		if (err)
 			return err;
-
-		err = dsa_parse_ports(pdata, ds);
-		if (err)
-			return err;
 	}
 
 	dst = dsa_tree_touch(tree);
@@ -773,6 +765,16 @@ static int _dsa_register_switch(struct dsa_switch *ds)
 	ds->index = index;
 	ds->cd = pdata;
 
+	if (np) {
+		err = dsa_parse_ports_of(np, ds);
+		if (err)
+			return err;
+	} else {
+		err = dsa_parse_ports(pdata, ds);
+		if (err)
+			return err;
+	}
+
 	/* Initialize the routing table */
 	for (i = 0; i < DSA_MAX_SWITCHES; ++i)
 		ds->rtable[i] = DSA_RTABLE_NONE;
-- 
2.14.3

[PATCH net-next 08/11] net: dsa: rework switch parsing

[Vivien Didelot]

When parsing a switch, we have to identify to which tree it belongs and
parse its ports. Provide two functions to separate the OF and platform
data specific paths.

Also use the of_property_read_variable_u32_array function to parse the
OF member array instead of calling of_property_read_u32_index twice.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/dsa2.c | 117 ++++++++++++++++++++++++++++-----------------------------
 1 file changed, 58 insertions(+), 59 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 5918fbddb0ab..dfcb6247f2f2 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -617,7 +617,8 @@ static int dsa_port_parse_of(struct dsa_port *dp, struct device_node *dn)
 	return 0;
 }
 
-static int dsa_parse_ports_of(struct device_node *dn, struct dsa_switch *ds)
+static int dsa_switch_parse_ports_of(struct dsa_switch *ds,
+				     struct device_node *dn)
 {
 	struct device_node *ports, *port;
 	struct dsa_port *dp;
@@ -648,6 +649,39 @@ static int dsa_parse_ports_of(struct device_node *dn, struct dsa_switch *ds)
 	return 0;
 }
 
+static int dsa_switch_parse_member_of(struct dsa_switch *ds,
+				      struct device_node *dn)
+{
+	u32 m[2] = { 0, 0 };
+	int sz;
+
+	/* Don't error out if this optional property isn't found */
+	sz = of_property_read_variable_u32_array(dn, "dsa,member", m, 2, 2);
+	if (sz < 0 && sz != -EINVAL)
+		return sz;
+
+	ds->index = m[1];
+	if (ds->index >= DSA_MAX_SWITCHES)
+		return -EINVAL;
+
+	ds->dst = dsa_tree_touch(m[0]);
+	if (!ds->dst)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static int dsa_switch_parse_of(struct dsa_switch *ds, struct device_node *dn)
+{
+	int err;
+
+	err = dsa_switch_parse_member_of(ds, dn);
+	if (err)
+		return err;
+
+	return dsa_switch_parse_ports_of(ds, dn);
+}
+
 static int dsa_port_parse(struct dsa_port *dp, const char *name,
 			  struct device *dev)
 {
@@ -673,7 +707,8 @@ static int dsa_port_parse(struct dsa_port *dp, const char *name,
 	return 0;
 }
 
-static int dsa_parse_ports(struct dsa_chip_data *cd, struct dsa_switch *ds)
+static int dsa_switch_parse_ports(struct dsa_switch *ds,
+				  struct dsa_chip_data *cd)
 {
 	bool valid_name_found = false;
 	struct dsa_port *dp;
@@ -703,40 +738,19 @@ static int dsa_parse_ports(struct dsa_chip_data *cd, struct dsa_switch *ds)
 	return 0;
 }
 
-static int dsa_parse_member_dn(struct device_node *np, u32 *tree, u32 *index)
+static int dsa_switch_parse(struct dsa_switch *ds, struct dsa_chip_data *cd)
 {
-	int err;
+	ds->cd = cd;
 
-	*tree = *index = 0;
+	/* We don't support interconnected switches nor multiple trees via
+	 * platform data, so this is the unique switch of the tree.
+	 */
+	ds->index = 0;
+	ds->dst = dsa_tree_touch(0);
+	if (!ds->dst)
+		return -ENOMEM;
 
-	err = of_property_read_u32_index(np, "dsa,member", 0, tree);
-	if (err) {
-		/* Does not exist, but it is optional */
-		if (err == -EINVAL)
-			return 0;
-		return err;
-	}
-
-	err = of_property_read_u32_index(np, "dsa,member", 1, index);
-	if (err)
-		return err;
-
-	if (*index >= DSA_MAX_SWITCHES)
-		return -EINVAL;
-
-	return 0;
-}
-
-static int dsa_parse_member(struct dsa_chip_data *pd, u32 *tree, u32 *index)
-{
-	if (!pd)
-		return -ENODEV;
-
-	/* We do not support complex trees with dsa_chip_data */
-	*tree = 0;
-	*index = 0;
-
-	return 0;
+	return dsa_switch_parse_ports(ds, cd);
 }
 
 static int _dsa_register_switch(struct dsa_switch *ds)
@@ -744,36 +758,21 @@ static int _dsa_register_switch(struct dsa_switch *ds)
 	struct dsa_chip_data *pdata = ds->dev->platform_data;
 	struct device_node *np = ds->dev->of_node;
 	struct dsa_switch_tree *dst;
-	u32 tree, index;
+	unsigned int index;
 	int i, err;
 
-	if (np) {
-		err = dsa_parse_member_dn(np, &tree, &index);
-		if (err)
-			return err;
-	} else {
-		err = dsa_parse_member(pdata, &tree, &index);
-		if (err)
-			return err;
-	}
+	if (np)
+		err = dsa_switch_parse_of(ds, np);
+	else if (pdata)
+		err = dsa_switch_parse(ds, pdata);
+	else
+		err = -ENODEV;
 
-	dst = dsa_tree_touch(tree);
-	if (!dst)
-		return -ENOMEM;
+	if (err)
+		return err;
 
-	ds->dst = dst;
-	ds->index = index;
-	ds->cd = pdata;
-
-	if (np) {
-		err = dsa_parse_ports_of(np, ds);
-		if (err)
-			return err;
-	} else {
-		err = dsa_parse_ports(pdata, ds);
-		if (err)
-			return err;
-	}
+	index = ds->index;
+	dst = ds->dst;
 
 	/* Initialize the routing table */
 	for (i = 0; i < DSA_MAX_SWITCHES; ++i)
-- 
2.14.3

[PATCH net-next 09/11] net: dsa: only check presence of link property

[Vivien Didelot]

When parsing a port, simply use of_property_read_bool which checks the
presence of a given property, instead of parsing the link phandle.

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

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index dfcb6247f2f2..06bcdb6bc796 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -590,8 +590,8 @@ static int dsa_dst_parse(struct dsa_switch_tree *dst)
 static int dsa_port_parse_of(struct dsa_port *dp, struct device_node *dn)
 {
 	struct device_node *ethernet = of_parse_phandle(dn, "ethernet", 0);
-	struct device_node *link = of_parse_phandle(dn, "link", 0);
 	const char *name = of_get_property(dn, "label", NULL);
+	bool link = of_property_read_bool(dn, "link");
 
 	if (ethernet) {
 		struct net_device *master;
-- 
2.14.3

[PATCH net-next 10/11] net: dsa: add one port parsing function per type

[Vivien Didelot]

Add dsa_port_parse_user, dsa_port_parse_dsa and dsa_port_parse_cpu
functions to factorize the code shared by both OF and pdata parsing.

They don't do much for the moment but will be extended later to support
tagging protocol resolution for example.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/dsa2.c | 56 ++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 20 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 06bcdb6bc796..271a97ef5bf6 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -491,6 +491,32 @@ static int dsa_tree_add_switch(struct dsa_switch_tree *dst,
 	return 0;
 }
 
+static int dsa_port_parse_user(struct dsa_port *dp, const char *name)
+{
+	if (!name)
+		name = "eth%d";
+
+	dp->type = DSA_PORT_TYPE_USER;
+	dp->name = name;
+
+	return 0;
+}
+
+static int dsa_port_parse_dsa(struct dsa_port *dp)
+{
+	dp->type = DSA_PORT_TYPE_DSA;
+
+	return 0;
+}
+
+static int dsa_port_parse_cpu(struct dsa_port *dp, struct net_device *master)
+{
+	dp->type = DSA_PORT_TYPE_CPU;
+	dp->master = master;
+
+	return 0;
+}
+
 static int dsa_cpu_parse(struct dsa_port *port, u32 index,
 			 struct dsa_switch_tree *dst,
 			 struct dsa_switch *ds)
@@ -593,6 +619,8 @@ static int dsa_port_parse_of(struct dsa_port *dp, struct device_node *dn)
 	const char *name = of_get_property(dn, "label", NULL);
 	bool link = of_property_read_bool(dn, "link");
 
+	dp->dn = dn;
+
 	if (ethernet) {
 		struct net_device *master;
 
@@ -600,21 +628,13 @@ static int dsa_port_parse_of(struct dsa_port *dp, struct device_node *dn)
 		if (!master)
 			return -EPROBE_DEFER;
 
-		dp->type = DSA_PORT_TYPE_CPU;
-		dp->master = master;
-	} else if (link) {
-		dp->type = DSA_PORT_TYPE_DSA;
-	} else {
-		if (!name)
-			name = "eth%d";
-
-		dp->type = DSA_PORT_TYPE_USER;
-		dp->name = name;
+		return dsa_port_parse_cpu(dp, master);
 	}
 
-	dp->dn = dn;
+	if (link)
+		return dsa_port_parse_dsa(dp);
 
-	return 0;
+	return dsa_port_parse_user(dp, name);
 }
 
 static int dsa_switch_parse_ports_of(struct dsa_switch *ds,
@@ -694,17 +714,13 @@ static int dsa_port_parse(struct dsa_port *dp, const char *name,
 
 		dev_put(master);
 
-		dp->type = DSA_PORT_TYPE_CPU;
-		dp->master = master;
-	} else if (!strcmp(name, "dsa")) {
-		dp->type = DSA_PORT_TYPE_DSA;
-	} else {
-		dp->type = DSA_PORT_TYPE_USER;
+		return dsa_port_parse_cpu(dp, master);
 	}
 
-	dp->name = name;
+	if (!strcmp(name, "dsa"))
+		return dsa_port_parse_dsa(dp);
 
-	return 0;
+	return dsa_port_parse_user(dp, name);
 }
 
 static int dsa_switch_parse_ports(struct dsa_switch *ds,
-- 
2.14.3

[PATCH net-next 11/11] net: dsa: resolve tagging protocol at parse time

[Vivien Didelot]

Extend the dsa_port_parse_cpu() function to resolve the tagging protocol
at port parsing time, instead of waiting for the whole tree to be
complete.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/dsa2.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 271a97ef5bf6..283104e5ca6a 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -511,22 +511,11 @@ static int dsa_port_parse_dsa(struct dsa_port *dp)
 
 static int dsa_port_parse_cpu(struct dsa_port *dp, struct net_device *master)
 {
-	dp->type = DSA_PORT_TYPE_CPU;
-	dp->master = master;
-
-	return 0;
-}
-
-static int dsa_cpu_parse(struct dsa_port *port, u32 index,
-			 struct dsa_switch_tree *dst,
-			 struct dsa_switch *ds)
-{
+	struct dsa_switch *ds = dp->ds;
+	struct dsa_switch_tree *dst = ds->dst;
 	const struct dsa_device_ops *tag_ops;
 	enum dsa_tag_protocol tag_protocol;
 
-	if (!dst->cpu_dp)
-		dst->cpu_dp = port;
-
 	tag_protocol = ds->ops->get_tag_protocol(ds);
 	tag_ops = dsa_resolve_tag_protocol(tag_protocol);
 	if (IS_ERR(tag_ops)) {
@@ -534,11 +523,21 @@ static int dsa_cpu_parse(struct dsa_port *port, u32 index,
 		return PTR_ERR(tag_ops);
 	}
 
-	dst->cpu_dp->tag_ops = tag_ops;
+	dp->type = DSA_PORT_TYPE_CPU;
+	dp->rcv = tag_ops->rcv;
+	dp->tag_ops = tag_ops;
+	dp->master = master;
+	dp->dst = dst;
 
-	/* Make a few copies for faster access in master receive hot path */
-	dst->cpu_dp->rcv = dst->cpu_dp->tag_ops->rcv;
-	dst->cpu_dp->dst = dst;
+	return 0;
+}
+
+static int dsa_cpu_parse(struct dsa_port *port, u32 index,
+			 struct dsa_switch_tree *dst,
+			 struct dsa_switch *ds)
+{
+	if (!dst->cpu_dp)
+		dst->cpu_dp = port;
 
 	return 0;
 }
-- 
2.14.3

Re: [PATCH net-next 00/11] net: dsa: parsing stage

[David Miller]

From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Fri,  3 Nov 2017 19:05:19 -0400

> When registering a DSA switch, there is basically two stages.
> 
> The first stage is the parsing of the switch device, from either device
> tree or platform data. It fetches the DSA tree to which it belongs, and
> validates its ports. The switch device is then added to the tree, and
> the second stage is called if this was the last switch of the tree.
> 
> The second stage is the setup of the tree, which validates that the tree
> is complete, sets up the routing tables, the default CPU port for user
> ports, sets up the switch drivers and finally the master interfaces,
> which makes the whole switch fabric functional.
> 
> This patch series covers the first parsing stage. It fixes the type of
> the switch and tree indexes to unsigned int, simplifies the tree
> reference counting and the switch and CPU ports parsing.

Series applied, thanks Vivien.