| Message ID | 43b100c5-9d53-46eb-bee0-940ab948722a@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v2] r8169: use the extended tally counter available from RTL8125 | expand |
On 10.10.2024 12:50, Heiner Kallweit wrote: > The new hw stat fields partially duplicate existing fields, but with a > larger field size now. Use these new fields to reduce the risk of > overflows. In addition add support for relevant new fields which are > available from RTL8125 only. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > v2: > - added code for enabling the extended tally counter > - included relevant new fields > --- > drivers/net/ethernet/realtek/r8169_main.c | 40 ++++++++++++++++++++++- > 1 file changed, 39 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c > index 665105430..71339910b 100644 > --- a/drivers/net/ethernet/realtek/r8169_main.c > +++ b/drivers/net/ethernet/realtek/r8169_main.c > @@ -1777,11 +1777,26 @@ static const char rtl8169_gstrings[][ETH_GSTRING_LEN] = { > "tx_underrun", > }; > > +static const char rtl8125_gstrings[][ETH_GSTRING_LEN] = { > + "tx_bytes", > + "rx_bytes", > + "tx_pause_on", > + "tx_pause_off", > + "rx_pause_on", > + "rx_pause_off", > +}; > + > static int rtl8169_get_sset_count(struct net_device *dev, int sset) > { > switch (sset) { > case ETH_SS_STATS: > - return ARRAY_SIZE(rtl8169_gstrings); > + struct rtl8169_private *tp = netdev_priv(dev); > + > + if (rtl_is_8125(tp)) > + return ARRAY_SIZE(rtl8169_gstrings) + > + ARRAY_SIZE(rtl8125_gstrings); > + else > + return ARRAY_SIZE(rtl8169_gstrings); > default: > return -EOPNOTSUPP; > } > @@ -1873,13 +1888,33 @@ static void rtl8169_get_ethtool_stats(struct net_device *dev, > data[10] = le32_to_cpu(counters->rx_multicast); > data[11] = le16_to_cpu(counters->tx_aborted); > data[12] = le16_to_cpu(counters->tx_underrun); > + > + if (rtl_is_8125(tp)) { > + data[5] = le32_to_cpu(counters->align_errors32); > + data[10] = le64_to_cpu(counters->rx_multicast64); > + data[11] = le32_to_cpu(counters->tx_aborted32); > + data[12] = le32_to_cpu(counters->tx_underrun32); > + > + data[13] = le64_to_cpu(counters->tx_octets); > + data[14] = le64_to_cpu(counters->rx_octets); > + data[15] = le32_to_cpu(counters->tx_pause_on); > + data[16] = le32_to_cpu(counters->tx_pause_off); > + data[17] = le32_to_cpu(counters->rx_pause_on); > + data[18] = le32_to_cpu(counters->rx_pause_off); > + } > } > > static void rtl8169_get_strings(struct net_device *dev, u32 stringset, u8 *data) > { > switch(stringset) { > case ETH_SS_STATS: > + struct rtl8169_private *tp = netdev_priv(dev); patchwork lists the following warning for a clang build: warning: label followed by a declaration is a C23 extension [-Wc23-extensions] gcc 14.2.1 however had no problem with this code, and also checkpatch didn't complain. Is this code acceptable or should it be changed?
On Thu, Oct 10, 2024 at 10:08:02PM +0200, Heiner Kallweit wrote: > On 10.10.2024 12:50, Heiner Kallweit wrote: > > The new hw stat fields partially duplicate existing fields, but with a > > larger field size now. Use these new fields to reduce the risk of > > overflows. In addition add support for relevant new fields which are > > available from RTL8125 only. > > > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> ... > > @@ -1873,13 +1888,33 @@ static void rtl8169_get_ethtool_stats(struct net_device *dev, > > data[10] = le32_to_cpu(counters->rx_multicast); > > data[11] = le16_to_cpu(counters->tx_aborted); > > data[12] = le16_to_cpu(counters->tx_underrun); > > + > > + if (rtl_is_8125(tp)) { > > + data[5] = le32_to_cpu(counters->align_errors32); > > + data[10] = le64_to_cpu(counters->rx_multicast64); > > + data[11] = le32_to_cpu(counters->tx_aborted32); > > + data[12] = le32_to_cpu(counters->tx_underrun32); > > + > > + data[13] = le64_to_cpu(counters->tx_octets); > > + data[14] = le64_to_cpu(counters->rx_octets); > > + data[15] = le32_to_cpu(counters->tx_pause_on); > > + data[16] = le32_to_cpu(counters->tx_pause_off); > > + data[17] = le32_to_cpu(counters->rx_pause_on); > > + data[18] = le32_to_cpu(counters->rx_pause_off); > > + } > > } > > > > static void rtl8169_get_strings(struct net_device *dev, u32 stringset, u8 *data) > > { > > switch(stringset) { > > case ETH_SS_STATS: > > + struct rtl8169_private *tp = netdev_priv(dev); > > patchwork lists the following warning for a clang build: > warning: label followed by a declaration is a C23 extension [-Wc23-extensions] > > gcc 14.2.1 however had no problem with this code, and also checkpatch didn't > complain. Is this code acceptable or should it be changed? Yes, I see that too. My feeling is that it would be best to change it as it seems likely that this will break compilation somewhere.
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c index 665105430..71339910b 100644 --- a/drivers/net/ethernet/realtek/r8169_main.c +++ b/drivers/net/ethernet/realtek/r8169_main.c @@ -1777,11 +1777,26 @@ static const char rtl8169_gstrings[][ETH_GSTRING_LEN] = { "tx_underrun", }; +static const char rtl8125_gstrings[][ETH_GSTRING_LEN] = { + "tx_bytes", + "rx_bytes", + "tx_pause_on", + "tx_pause_off", + "rx_pause_on", + "rx_pause_off", +}; + static int rtl8169_get_sset_count(struct net_device *dev, int sset) { switch (sset) { case ETH_SS_STATS: - return ARRAY_SIZE(rtl8169_gstrings); + struct rtl8169_private *tp = netdev_priv(dev); + + if (rtl_is_8125(tp)) + return ARRAY_SIZE(rtl8169_gstrings) + + ARRAY_SIZE(rtl8125_gstrings); + else + return ARRAY_SIZE(rtl8169_gstrings); default: return -EOPNOTSUPP; } @@ -1873,13 +1888,33 @@ static void rtl8169_get_ethtool_stats(struct net_device *dev, data[10] = le32_to_cpu(counters->rx_multicast); data[11] = le16_to_cpu(counters->tx_aborted); data[12] = le16_to_cpu(counters->tx_underrun); + + if (rtl_is_8125(tp)) { + data[5] = le32_to_cpu(counters->align_errors32); + data[10] = le64_to_cpu(counters->rx_multicast64); + data[11] = le32_to_cpu(counters->tx_aborted32); + data[12] = le32_to_cpu(counters->tx_underrun32); + + data[13] = le64_to_cpu(counters->tx_octets); + data[14] = le64_to_cpu(counters->rx_octets); + data[15] = le32_to_cpu(counters->tx_pause_on); + data[16] = le32_to_cpu(counters->tx_pause_off); + data[17] = le32_to_cpu(counters->rx_pause_on); + data[18] = le32_to_cpu(counters->rx_pause_off); + } } static void rtl8169_get_strings(struct net_device *dev, u32 stringset, u8 *data) { switch(stringset) { case ETH_SS_STATS: + struct rtl8169_private *tp = netdev_priv(dev); + memcpy(data, rtl8169_gstrings, sizeof(rtl8169_gstrings)); + if (rtl_is_8125(tp)) { + data += sizeof(rtl8169_gstrings); + memcpy(data, rtl8125_gstrings, sizeof(rtl8125_gstrings)); + } break; } } @@ -3894,6 +3929,9 @@ static void rtl_hw_start_8125(struct rtl8169_private *tp) break; } + /* enable extended tally counter */ + r8168_mac_ocp_modify(tp, 0xea84, 0, BIT(1) | BIT(0)); + rtl_hw_config(tp); }
The new hw stat fields partially duplicate existing fields, but with a larger field size now. Use these new fields to reduce the risk of overflows. In addition add support for relevant new fields which are available from RTL8125 only. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- v2: - added code for enabling the extended tally counter - included relevant new fields --- drivers/net/ethernet/realtek/r8169_main.c | 40 ++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-)