Re: [PATCH net-next 1/2] net: dsa: Add ability to elect CPU port

[Andrew Lunn <andrew@lunn.ch>]

On Mon, Oct 28, 2019 at 03:32:35PM -0700, Florian Fainelli wrote:
> In a configuration where multiple CPU ports are declared within the
> platform configuration, it may be desirable to make sure that a
> particular CPU port gets used. This is particularly true for Broadcom
> switch that are fairly flexible to some extent in which port can be the
> CPU port, yet will be more featureful if port 8 is elected.

> -static struct dsa_port *dsa_tree_find_first_cpu(struct dsa_switch_tree *dst)
> +static struct dsa_port *dsa_tree_find_cpu(struct dsa_switch_tree *dst)
>  {
> +	struct dsa_switch *ds;
>  	struct dsa_port *dp;
> +	int err;
>  
> -	list_for_each_entry(dp, &dst->ports, list)
> -		if (dsa_port_is_cpu(dp))
> +	list_for_each_entry(dp, &dst->ports, list) {
> +		ds = dp->ds;
> +		if (!dsa_port_is_cpu(dp))
> +			continue;
> +
> +		if (!ds->ops->elect_cpu_port)
>  			return dp;
>  
> +		err = ds->ops->elect_cpu_port(ds, dp->index);
> +		if (err == 0)
> +			return dp;
> +	}
> +
>  	return NULL;
>  }

Hi Florian

I think is_preferred_cpu_port() would be a better name, and maybe a
bool?

Also, i don't think we should be returning NULL at the end like
this. If the device tree does not have the preferred port as CPU port,
we should use dsa_tree_find_cpu() to pick one of the actually offered
CPU ports in DT? It sounds like your hardware will still work if any
port is used as the CPU port.

And maybe we need a is_valid_cpu_port()? Some of the chipsets only
have a subset which can be CPU ports, the hardware is not as flexible.
The core can then validate the CPU port really is valid, rather than
the driver, e.g. qca8k, validating the CPU port in setup() and
returning an error.

    Andrew