[LEDE-DEV] [RFC] AR8327 driver: implementing get_port_stats function

[Felix Fietkau]

On 2017-02-25 22:51, Daniel Gonzalez Cabanelas wrote:
> Currently the switch LED trigger only shows link status, but not traffic activity by blinking
> the LEDs on the AR8316/AR8327 switch. It turns out that the get_port_stats is missing.
> 
> I've made this patch, which works ok. But I'm not sure if there is any reason to not implement this 
> function. And probably the function ar8xxx_sw_get_port_stats can be improved.
> 
> There are a few boards with an  AR8316/AR8327 switch that need this trigger, usually on the WAN port.
> As said without get_port_stats the light stays always fixed, like if there wasn't traffic activity. We can use 
> the netdev trigger but then the light is always on even when the cable is unplugged, and there isn't the
> possibility of using a second LED trigger on the same port for indicating a reduced link speed.
> 
> diff --git a/target/linux/generic/files/drivers/net/phy/ar8216.c b/target/linux/generic/files/drivers/net/phy/ar8216.c
> index 37877d5..ffaa77e 100644
> --- a/target/linux/generic/files/drivers/net/phy/ar8216.c
> +++ b/target/linux/generic/files/drivers/net/phy/ar8216.c
> @@ -1001,6 +1001,41 @@ ar8xxx_sw_get_port_link(struct switch_dev *dev, int port,
>  	return 0;
>  }
>  
> +int
> +ar8xxx_sw_get_port_stats(struct switch_dev *dev, int port,
> +		       struct switch_port_stats *stats)
> +{
> +	struct ar8xxx_priv *priv = swdev_to_ar8xxx(dev);
> +	const struct ar8xxx_chip *chip = priv->chip;
> +	u64 *mib_stats, mib_data;
> +	const char *mib_name;
> +	int i;
> +
> +	if (!ar8xxx_has_mib_counters(priv))
> +		return -EOPNOTSUPP;
> +
> +	if (port >= dev->ports)
> +		return -EINVAL;
> +
> +	mutex_lock(&priv->mib_lock);
> +
> +	ar8xxx_mib_fetch_port_stat(priv, port, false);
> +
> +	mib_stats = &priv->mib_stats[port * chip->num_mibs];
> +	for (i = 0; i < chip->num_mibs; i++) {
> +		mib_name = chip->mib_decs[i].name;
> +		mib_data = mib_stats[i];
> +		if (!strcmp(mib_name, "TxByte"))
> +			stats->tx_bytes = mib_data;
> +		if (!strcmp(mib_name, "RxGoodByte"))
> +			stats->rx_bytes = mib_data;
> +	}

Fetching the entire port stats only to look up two fields seems rather
excessive. Please make a function instead that will look up the register
number and fetch only the relevant registers.
The lookup can be further simplified by adding an enum for the mib_stats
array index.

- Felix