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
_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
