Le 04/06/2016 13:29, Andrew Lunn a écrit : >> @@ -517,6 +541,15 @@ static int dsa_parse_ports_dn(struct device_node >> *ports, struct dsa_switch *ds) >> return -EINVAL; >> >> ds->ports[reg].dn = port; >> + >> + if (dsa_port_is_cpu(port)) >> + ds->dst->cpu_port = reg; >> + else >> + /* Initialize enabled_port_mask now for drv->setup() >> + * to have access to a correct value, just like what >> + * net/dsa/dsa.c::dsa_switch_setup_one does. >> + */ >> + ds->enabled_port_mask |= 1 << reg; > > Hi Florian > > You need to be careful here. There can be multiple CPU ports, in > different switches. We want dst->cpu_port to be deterministic, > independent of the order switches are registered. Which is why i set > it as part of dsa_cpu_parse(), which only happens when all the > switches have registered, and we are parsing their device tree nodes > in order. So we guarantee dst->cpu_port is the first CPU node.
Ah OK, I completely missed that part and just wanted to avoid walking the ports children nodes more than twice. We might be able to get away with just initializing ds->enabled_port_mask here actually. > > You now set dst->cpu_port via dsa_parse_ports_dn(), so it is now non > deterministic, it depends on the probe order of the switches. > > In the long run, i want to deprecate and then remove dst->cpu_port, > but i'm not that far yet. > > Please rethink this part of the patch, keeping in mind you have > multiple switches, with multiple CPU and DSA ports, all connected in > some crazy fashion. -- Florian
