Message ID | 8021994.eIdPYGzJH3@tool |
---|---|
State | Superseded |
Delegated to: | Mathias Kresin |
Headers | show |
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
> 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 Hi Felix. Could this function be valid?. I'm afraid I cannot make anything more sophisticated ar8xxx_sw_get_port_stats(struct switch_dev *dev, int port, struct switch_port_stats *stats) { struct ar8xxx_priv *priv = swdev_to_ar8xxx(dev); unsigned int base; unsigned int rx_offset, tx_offset; if (port >= dev->ports) return -EINVAL; base = priv->chip->reg_port_stats_start + priv->chip->reg_port_stats_length * port; if (priv->chip->num_mibs == ARRAY_SIZE(ar8236_mibs)){ rx_offset = AR8236_STATS_RXGOODBYTE; tx_offset = AR8236_STATS_TXBYTE; } else if (priv->chip->num_mibs == ARRAY_SIZE(ar8216_mibs)){ rx_offset = AR8216_STATS_RXGOODBYTE; tx_offset = AR8216_STATS_TXBYTE; } stats->rx_bytes = ar8xxx_read(priv, base + rx_offset); stats->tx_bytes = ar8xxx_read(priv, base + tx_offset); return 0; } Also tested and working ok. Regards Daniel
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; + } + + mutex_unlock(&priv->mib_lock); + + return 0; +} + static int ar8xxx_sw_get_ports(struct switch_dev *dev, struct switch_val *val) { @@ -1696,6 +1731,7 @@ static const struct switch_dev_ops ar8xxx_sw_ops = { .apply_config = ar8xxx_sw_hw_apply, .reset_switch = ar8xxx_sw_reset_switch, .get_port_link = ar8xxx_sw_get_port_link, + .get_port_stats = ar8xxx_sw_get_port_stats, }; static const struct ar8xxx_chip ar8216_chip = { diff --git a/target/linux/generic/files/drivers/net/phy/ar8216.h b/target/linux/generic/files/drivers/net/phy/ar8216.h index d9508b9..8b05cf3 100644 --- a/target/linux/generic/files/drivers/net/phy/ar8216.h +++ b/target/linux/generic/files/drivers/net/phy/ar8216.h @@ -538,6 +538,9 @@ int ar8xxx_sw_get_port_link(struct switch_dev *dev, int port, struct switch_port_link *link); int +ar8xxx_sw_get_port_stats(struct switch_dev *dev, + int port, struct switch_port_stats *stats); +int ar8xxx_sw_set_port_reset_mib(struct switch_dev *dev, const struct switch_attr *attr, struct switch_val *val); diff --git a/target/linux/generic/files/drivers/net/phy/ar8327.c b/target/linux/generic/files/drivers/net/phy/ar8327.c index 9b40cd7..e82563f 100644 --- a/target/linux/generic/files/drivers/net/phy/ar8327.c +++ b/target/linux/generic/files/drivers/net/phy/ar8327.c @@ -1372,6 +1372,7 @@ static const struct switch_dev_ops ar8327_sw_ops = { .apply_config = ar8327_sw_hw_apply, .reset_switch = ar8xxx_sw_reset_switch, .get_port_link = ar8xxx_sw_get_port_link, + .get_port_stats = ar8xxx_sw_get_port_stats, }; const struct ar8xxx_chip ar8327_chip = {