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 <and...@lunn.ch>
> Signed-off-by: Florian Fainelli <f.faine...@gmail.com>
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
