Re: [PATCH net-next 15/17] net: dsa: Add new binding implementation
From: Florian Fainelli <f.fainelli@gmail.com>
Date: 2016-06-03 21:19:52
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 <andrew@lunn.ch> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Just a few nits that I had not seen before...
quoted hunk ↗ jump to hunk
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