This patch updates the mlxbf_gige driver to support the
"get_pause_stats()" callback, which enables display of
pause frame counters via "ethtool -I -a oob_net0".
The pause frame counters are only enabled if the "counters_en"
bit is asserted in the LLU general config register. If this bit
is not asserted, reads to these counters return bogus values.
A run-time check is made to return counter values of 0 if
the counters are not actually enabled.
Reviewed-by: Asmaa Mnebhi <[email protected]>
Signed-off-by: David Thompson <[email protected]>
---
.../mellanox/mlxbf_gige/mlxbf_gige_ethtool.c | 39 +++++++++++++++++++
.../mellanox/mlxbf_gige/mlxbf_gige_regs.h | 30 ++++++++++++++
2 files changed, 69 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_ethtool.c b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_ethtool.c
index 253d7ad9b809..107acab06b7e 100644
--- a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_ethtool.c
@@ -124,6 +124,44 @@ static void mlxbf_gige_get_pauseparam(struct net_device *netdev,
pause->tx_pause = 1;
}
+static bool mlxbf_gige_llu_counters_enabled(struct mlxbf_gige *priv)
+{
+ u32 data;
+
+ if (priv->hw_version == MLXBF_GIGE_VERSION_BF2) {
+ data = readl(priv->llu_base + MLXBF_GIGE_BF2_LLU_GENERAL_CONFIG);
+ if (data & MLXBF_GIGE_BF2_LLU_COUNTERS_EN)
+ return true;
+ } else {
+ data = readl(priv->llu_base + MLXBF_GIGE_BF3_LLU_GENERAL_CONFIG);
+ if (data & MLXBF_GIGE_BF3_LLU_COUNTERS_EN)
+ return true;
+ }
+
+ return false;
+}
+
+static void mlxbf_gige_get_pause_stats(struct net_device *netdev,
+ struct ethtool_pause_stats *pause_stats)
+{
+ struct mlxbf_gige *priv = netdev_priv(netdev);
+ u64 data_lo, data_hi;
+
+ /* Read LLU counters only if they are enabled */
+ if (mlxbf_gige_llu_counters_enabled(priv)) {
+ data_lo = readl(priv->llu_base + MLXBF_GIGE_TX_PAUSE_CNT_LO);
+ data_hi = readl(priv->llu_base + MLXBF_GIGE_TX_PAUSE_CNT_HI);
+ pause_stats->tx_pause_frames = (data_hi << 32) | data_lo;
+
+ data_lo = readl(priv->llu_base + MLXBF_GIGE_RX_PAUSE_CNT_LO);
+ data_hi = readl(priv->llu_base + MLXBF_GIGE_RX_PAUSE_CNT_HI);
+ pause_stats->rx_pause_frames = (data_hi << 32) | data_lo;
+ } else {
+ pause_stats->tx_pause_frames = 0;
+ pause_stats->rx_pause_frames = 0;
+ }
+}
+
const struct ethtool_ops mlxbf_gige_ethtool_ops = {
.get_link = ethtool_op_get_link,
.get_ringparam = mlxbf_gige_get_ringparam,
@@ -134,6 +172,7 @@ const struct ethtool_ops mlxbf_gige_ethtool_ops = {
.get_ethtool_stats = mlxbf_gige_get_ethtool_stats,
.nway_reset = phy_ethtool_nway_reset,
.get_pauseparam = mlxbf_gige_get_pauseparam,
+ .get_pause_stats = mlxbf_gige_get_pause_stats,
.get_link_ksettings = phy_ethtool_get_link_ksettings,
.set_link_ksettings = phy_ethtool_set_link_ksettings,
};
diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_regs.h b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_regs.h
index cd0973229c9b..98a8681c21b9 100644
--- a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_regs.h
+++ b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_regs.h
@@ -99,4 +99,34 @@
#define MLXBF_GIGE_100M_IPG_SIZE 119
#define MLXBF_GIGE_10M_IPG_SIZE 1199
+/* Offsets into OOB LLU block for pause frame counters */
+#define MLXBF_GIGE_BF2_TX_PAUSE_CNT_HI 0x33d8
+#define MLXBF_GIGE_BF2_TX_PAUSE_CNT_LO 0x33dc
+#define MLXBF_GIGE_BF2_RX_PAUSE_CNT_HI 0x3210
+#define MLXBF_GIGE_BF2_RX_PAUSE_CNT_LO 0x3214
+
+#define MLXBF_GIGE_BF3_TX_PAUSE_CNT_HI 0x3a88
+#define MLXBF_GIGE_BF3_TX_PAUSE_CNT_LO 0x3a8c
+#define MLXBF_GIGE_BF3_RX_PAUSE_CNT_HI 0x38c0
+#define MLXBF_GIGE_BF3_RX_PAUSE_CNT_LO 0x38c4
+
+#define MLXBF_GIGE_TX_PAUSE_CNT_HI ((priv->hw_version == MLXBF_GIGE_VERSION_BF2) ? \
+ MLXBF_GIGE_BF2_TX_PAUSE_CNT_HI : \
+ MLXBF_GIGE_BF3_TX_PAUSE_CNT_HI)
+#define MLXBF_GIGE_TX_PAUSE_CNT_LO ((priv->hw_version == MLXBF_GIGE_VERSION_BF2) ? \
+ MLXBF_GIGE_BF2_TX_PAUSE_CNT_LO : \
+ MLXBF_GIGE_BF3_TX_PAUSE_CNT_LO)
+#define MLXBF_GIGE_RX_PAUSE_CNT_HI ((priv->hw_version == MLXBF_GIGE_VERSION_BF2) ? \
+ MLXBF_GIGE_BF2_RX_PAUSE_CNT_HI : \
+ MLXBF_GIGE_BF3_RX_PAUSE_CNT_HI)
+#define MLXBF_GIGE_RX_PAUSE_CNT_LO ((priv->hw_version == MLXBF_GIGE_VERSION_BF2) ? \
+ MLXBF_GIGE_BF2_RX_PAUSE_CNT_LO : \
+ MLXBF_GIGE_BF3_RX_PAUSE_CNT_LO)
+
+#define MLXBF_GIGE_BF2_LLU_GENERAL_CONFIG 0x2110
+#define MLXBF_GIGE_BF3_LLU_GENERAL_CONFIG 0x2030
+
+#define MLXBF_GIGE_BF2_LLU_COUNTERS_EN BIT(0)
+#define MLXBF_GIGE_BF3_LLU_COUNTERS_EN BIT(4)
+
#endif /* !defined(__MLXBF_GIGE_REGS_H__) */
--
2.30.1
On Tue, 5 Mar 2024 10:18:51 -0500 David Thompson wrote:
> + /* Read LLU counters only if they are enabled */
> + if (mlxbf_gige_llu_counters_enabled(priv)) {
> + data_lo = readl(priv->llu_base + MLXBF_GIGE_TX_PAUSE_CNT_LO);
> + data_hi = readl(priv->llu_base + MLXBF_GIGE_TX_PAUSE_CNT_HI);
> + pause_stats->tx_pause_frames = (data_hi << 32) | data_lo;
> +
> + data_lo = readl(priv->llu_base + MLXBF_GIGE_RX_PAUSE_CNT_LO);
> + data_hi = readl(priv->llu_base + MLXBF_GIGE_RX_PAUSE_CNT_HI);
> + pause_stats->rx_pause_frames = (data_hi << 32) | data_lo;
> + } else {
> + pause_stats->tx_pause_frames = 0;
> + pause_stats->rx_pause_frames = 0;
Counters are not enabled, meaning we don't know how many frames were
sent? Or pause frames are not enabled, therefore we know it's 0?
If the latter we should add a comment clarifying that, if the former:
* @get_pause_stats: Report pause frame statistics. Drivers must not zero
* statistics which they don't report. The stats structure is initialized
* to ETHTOOL_STAT_NOT_SET indicating driver does not report statistics.
--
pw-bot: cr
> -----Original Message-----
> From: Jakub Kicinski <[email protected]>
> Sent: Tuesday, March 5, 2024 1:35 PM
> To: David Thompson <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Asmaa Mnebhi
> <[email protected]>
> Subject: Re: [PATCH net-next v1] mlxbf_gige: add support to display pause frame
> counters
>
> On Tue, 5 Mar 2024 10:18:51 -0500 David Thompson wrote:
> > + /* Read LLU counters only if they are enabled */
> > + if (mlxbf_gige_llu_counters_enabled(priv)) {
> > + data_lo = readl(priv->llu_base +
> MLXBF_GIGE_TX_PAUSE_CNT_LO);
> > + data_hi = readl(priv->llu_base +
> MLXBF_GIGE_TX_PAUSE_CNT_HI);
> > + pause_stats->tx_pause_frames = (data_hi << 32) | data_lo;
> > +
> > + data_lo = readl(priv->llu_base +
> MLXBF_GIGE_RX_PAUSE_CNT_LO);
> > + data_hi = readl(priv->llu_base +
> MLXBF_GIGE_RX_PAUSE_CNT_HI);
> > + pause_stats->rx_pause_frames = (data_hi << 32) | data_lo;
> > + } else {
> > + pause_stats->tx_pause_frames = 0;
> > + pause_stats->rx_pause_frames = 0;
>
> Counters are not enabled, meaning we don't know how many frames were sent?
> Or pause frames are not enabled, therefore we know it's 0?
>
> If the latter we should add a comment clarifying that, if the former:
>
> * @get_pause_stats: Report pause frame statistics. Drivers must not zero
> * statistics which they don't report. The stats structure is initialized
> * to ETHTOOL_STAT_NOT_SET indicating driver does not report statistics.
> --
> pw-bot: cr
Hi Jakub, thank you for your comments.
In this case it's the former, so pause frames are enabled we just don't know how
many are sent or received. I will update the driver patch accordingly and send v2.
Regards, Dave