Re: [RFC net-next PATCH 3/3] mv88e6xxx: rmon: Use RMU to collect rmon data.

[Andrew Lunn <andrew@lunn.ch>]

> @@ -1310,16 +1323,22 @@ static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
>  	struct mv88e6xxx_chip *chip = ds->priv;
>  	int ret;
>  
> -	mv88e6xxx_reg_lock(chip);
> +	if (chip->rmu.ops && chip->rmu.ops->get_rmon) {
> +		ret = chip->rmu.ops->get_rmon(chip, port, data);
> +		if (ret == -ETIMEDOUT)
> +			return;
> +	} else {
>  
> -	ret = mv88e6xxx_stats_snapshot(chip, port);
> -	mv88e6xxx_reg_unlock(chip);
> +		mv88e6xxx_reg_lock(chip);
>  
> -	if (ret < 0)
> -		return;
> +		ret = mv88e6xxx_stats_snapshot(chip, port);
> +		mv88e6xxx_reg_unlock(chip);
>  
> -	mv88e6xxx_get_stats(chip, port, data);
> +		if (ret < 0)
> +			return;
>  
> +		mv88e6xxx_get_stats(chip, port, data);
> +	}
>  }

I don't particularly like the way this is all mixed together.

Could you try to split it, so there is an MDIO set of functions and an
RMU set of functions. Maybe you have some helpers which are used by
both.

I would also suggest you try to think about ATU dump and VTU dump. You
ideally want a code structure which is very similar for all these dump
operations. Take a look at how qca8k-8xxx.c does things.

Is it documented in the datasheet that when RMU is used a snapshot is
not required?

    Andrew