Andrew Lunn writes:
>> +static void sparx5_attr_stp_state_set(struct sparx5_port *port,
>> + struct switchdev_trans *trans,
>> + u8 state)
>> +{
>> + struct sparx5 *sparx5 = port->sparx5;
>> +
>> + if (!test_bit(port->portno, sparx5->bridge_mask)) {
>> + netdev_err(port->ndev,
>> + "Controlling non-bridged port %d?\n", port->portno);
>> + return;
>> + }
>> +
>> + switch (state) {
>> + case BR_STATE_FORWARDING:
>> + set_bit(port->portno, sparx5->bridge_fwd_mask);
>> + break;
>> + default:
>> + clear_bit(port->portno, sparx5->bridge_fwd_mask);
>> + break;
>> + }
>
> That is pretty odd. What about listening, learning, blocking?
>
This only handles simple forward/block. We'll add the learning state as
well.
>> +static int sparx5_port_bridge_join(struct sparx5_port *port,
>> + struct net_device *bridge)
>> +{
>> + struct sparx5 *sparx5 = port->sparx5;
>> +
>> + if (bitmap_empty(sparx5->bridge_mask, SPX5_PORTS))
>> + /* First bridged port */
>> + sparx5->hw_bridge_dev = bridge;
>> + else
>> + if (sparx5->hw_bridge_dev != bridge)
>> + /* This is adding the port to a second bridge, this is
>> + * unsupported
>> + */
>> + return -ENODEV;
>> +
>> + set_bit(port->portno, sparx5->bridge_mask);
>> +
>> + /* Port enters in bridge mode therefor don't need to copy to CPU
>> + * frames for multicast in case the bridge is not requesting them
>> + */
>> + __dev_mc_unsync(port->ndev, sparx5_mc_unsync);
>> +
>> + return 0;
>> +}
>
> This looks suspiciously empty? Don't you need to tell the hardware
> which ports this port is bridges to? Normally you see some code which
> walks all the ports and finds those in the same bridge, and sets a bit
> which allows these ports to talk to each other. Is that code somewhere
> else?
>
This is applied when the STP state is handled - sparx5_update_fwd().
This is pretty much as in the ocelot driver, which can a somewhat
similar switch - and driver - architecture.
> Andrew
Thank you for your comments,
---Lars
--
Lars Povlsen,
Microchip
