Re: [PATCH net-next v3 2/3] net: dsa: mv88e6xxx: add support for bridge flags

[Vivien Didelot <vivien.didelot@gmail.com>]

Hi Russell,

On Wed, 20 Feb 2019 12:36:54 +0000, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> Add support for the bridge flags to Marvell 88e6xxx bridges, allowing
> the multicast and unicast flood properties to be controlled.  These
> can be controlled on a per-port basis via commands such as:
> 
> 	bridge link set dev lan1 flood on|off
> 	bridge link set dev lan1 mcast_flood on|off
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 32e7af5caa69..937370639fe4 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -4692,6 +4692,24 @@ static int mv88e6xxx_port_mdb_del(struct dsa_switch *ds, int port,
>  	return err;
>  }
>  
> +static int mv88e6xxx_port_egress_floods(struct dsa_switch *ds, int port,
> +					 bool unicast, bool multicast)
> +{
> +	struct mv88e6xxx_chip *chip = ds->priv;
> +	int ret = -EOPNOTSUPP;

I'd prefer "err" like in the rest of the driver.

> +
> +	WARN_ON_ONCE(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port));

No need to warn, the driver does not need to care about which port is targeted
by a call.

> +
> +	mutex_lock(&chip->reg_lock);
> +	if (chip->info->ops->port_set_egress_floods)
> +		ret = chip->info->ops->port_set_egress_floods(chip, port,
> +							      unicast,
> +							      multicast);
> +	mutex_unlock(&chip->reg_lock);
> +
> +	return ret;
> +}
> +
>  static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
>  #if IS_ENABLED(CONFIG_NET_DSA_LEGACY)
>  	.probe			= mv88e6xxx_drv_probe,
> @@ -4719,6 +4737,7 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
>  	.set_ageing_time	= mv88e6xxx_set_ageing_time,
>  	.port_bridge_join	= mv88e6xxx_port_bridge_join,
>  	.port_bridge_leave	= mv88e6xxx_port_bridge_leave,
> +	.port_egress_floods	= mv88e6xxx_port_egress_floods,
>  	.port_stp_state_set	= mv88e6xxx_port_stp_state_set,
>  	.port_fast_age		= mv88e6xxx_port_fast_age,
>  	.port_vlan_filtering	= mv88e6xxx_port_vlan_filtering,

Otherwise it looks good to me.