Hi Andrew, Florian,
I suggest to use this RFC to agree on a consistent and robust API for
the DSA layer. Some functions have non-intuitive names or signatures.
What about:
<namespace>_<action>[_<object>]
where <namespace> matches the first argument. So instead of
dsa_cpu_dsa_setup, we would have:
dsa_switch_setup_cpu_dsa_port(struct dsa_switch *ds, ...)
[...]
Andrew Lunn <and...@lunn.ch> writes:
> /* basic switch operations
> **************************************************/
> -static int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct net_device
> *master)
> +int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
> + struct device_node *port_dn, int port)
> {
We will need the port number in the struct dsa_port at some point. The
device_node is already a member of this structure.
I suggest to add the port index before this patch and change this
function for a more logical:
dsa_switch_setup_cpu_dsa_port(struct dsa_switch *ds, struct dsa_port *dp)
[...]
> /* Perform configuration of the CPU and DSA ports */
> - ret = dsa_cpu_dsa_setup(ds, dst->master_netdev);
> + ret = dsa_cpu_dsa_setups(ds, parent);
> if (ret < 0) {
> netdev_err(dst->master_netdev, "[%d] : can't configure CPU and
> DSA ports\n",
> index);
dsa_cpu_dsa_setups is not really meaningful and doesn't add much
value. At least I'd call it dsa_switch_setup_cpu_dsa_ports(), or better,
move the loop directly where it is needed so we know which port sucked:
dsa_switch_setup_one:
/* Perform configuration of the CPU and DSA ports */
for (port = 0; port < DSA_MAX_PORTS; port++) {
if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
continue;
ret = dsa_switch_setup_cpu_dsa_port(ds, dev, ds->ports[port]);
if (ret) {
netdev_err(dst->master_netdev,
"[%d] : can't configure port %d\n",
index, ds->ports[port]->index);
break;
}
}
Thanks,
Vivien
