The latest MC firmware version added a new command to retrieve all DPMAC
counters in a single firmware call. Use this new command, when possible,
in dpaa2-mac as well.
In order to use the dpmac_get_statistics() API, two DMA memory areas are
used: one to transmit what counters the driver is requesting and one to
receive the values of those counters. These memory areas are allocated
and DMA mapped at probe time so that we don't waste time at runtime.
And since we are planning to add rmon, eth-ctrl and other standard
statistics using the same infrastructure, make the setup and cleanup
processes as generic as possibile through the dpaa2_mac_setup_stats()
and dpaa2_mac_clear_stats() functions.
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
.../net/ethernet/freescale/dpaa2/dpaa2-mac.c | 195 ++++++++++++++----
.../net/ethernet/freescale/dpaa2/dpaa2-mac.h | 10 +-
2 files changed, 166 insertions(+), 39 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
index 422ce13a7c94..63dc597dbd7c 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
-/* Copyright 2019 NXP */
+/* Copyright 2019, 2024-2026 NXP */
#include <linux/acpi.h>
#include <linux/pcs-lynx.h>
@@ -15,7 +15,121 @@
#define DPMAC_PROTOCOL_CHANGE_VER_MAJOR 4
#define DPMAC_PROTOCOL_CHANGE_VER_MINOR 8
+#define DPMAC_STATS_BUNDLE_VER_MAJOR 4
+#define DPMAC_STATS_BUNDLE_VER_MINOR 10
+
#define DPAA2_MAC_FEATURE_PROTOCOL_CHANGE BIT(0)
+#define DPAA2_MAC_FEATURE_STATS_BUNDLE BIT(1)
+
+struct dpmac_counter {
+ enum dpmac_counter_id id;
+ const char *name;
+};
+
+#define DPMAC_UNSTRUCTURED_COUNTER(counter_id, counter_name) \
+ { \
+ .id = counter_id, \
+ .name = counter_name, \
+ }
+
+static const struct dpmac_counter dpaa2_mac_ethtool_stats[] = {
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_ALL_FRAME, "[mac] rx all frames"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_GOOD_FRAME, "[mac] rx frames ok"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_ERR_FRAME, "[mac] rx frame errors"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_FRAME_DISCARD, "[mac] rx frame discards"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_UCAST_FRAME, "[mac] rx u-cast"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_BCAST_FRAME, "[mac] rx b-cast"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_MCAST_FRAME, "[mac] rx m-cast"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_FRAME_64, "[mac] rx 64 bytes"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_FRAME_127, "[mac] rx 65-127 bytes"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_FRAME_255, "[mac] rx 128-255 bytes"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_FRAME_511, "[mac] rx 256-511 bytes"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_FRAME_1023, "[mac] rx 512-1023 bytes"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_FRAME_1518, "[mac] rx 1024-1518 bytes"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_FRAME_1519_MAX, "[mac] rx 1519-max bytes"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_FRAG, "[mac] rx frags"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_JABBER, "[mac] rx jabber"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_ALIGN_ERR, "[mac] rx align errors"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_OVERSIZED, "[mac] rx oversized"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_VALID_PAUSE_FRAME, "[mac] rx pause"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_BYTE, "[mac] rx bytes"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_EGR_GOOD_FRAME, "[mac] tx frames ok"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_EGR_UCAST_FRAME, "[mac] tx u-cast"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_EGR_MCAST_FRAME, "[mac] tx m-cast"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_EGR_BCAST_FRAME, "[mac] tx b-cast"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_EGR_ERR_FRAME, "[mac] tx frame errors"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_EGR_UNDERSIZED, "[mac] tx undersized"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_EGR_VALID_PAUSE_FRAME, "[mac] tx b-pause"),
+ DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_EGR_BYTE, "[mac] tx bytes"),
+};
+
+#define DPAA2_MAC_NUM_ETHTOOL_STATS ARRAY_SIZE(dpaa2_mac_ethtool_stats)
+
+static void dpaa2_mac_setup_stats(struct dpaa2_mac *mac, struct dpaa2_mac_stats *stats,
+ size_t num_stats, const struct dpmac_counter *counters)
+{
+ struct device *dev = mac->net_dev->dev.parent;
+ u32 *cnt_idx;
+
+ stats->idx_dma_mem = kcalloc(num_stats, sizeof(u32), GFP_KERNEL);
+ if (!stats->idx_dma_mem)
+ goto out;
+
+ stats->values_dma_mem = kcalloc(num_stats, sizeof(u64), GFP_KERNEL);
+ if (!stats->values_dma_mem)
+ goto err_alloc_values;
+
+ cnt_idx = stats->idx_dma_mem;
+ for (size_t i = 0; i < num_stats; i++)
+ *cnt_idx++ = cpu_to_le32((u32)(counters[i].id));
+
+ stats->idx_iova = dma_map_single(dev, stats->idx_dma_mem,
+ num_stats * sizeof(u32),
+ DMA_TO_DEVICE);
+ if (dma_mapping_error(dev, stats->idx_iova))
+ goto err_dma_map_idx;
+
+ stats->values_iova = dma_map_single(dev, stats->values_dma_mem,
+ num_stats * sizeof(u64),
+ DMA_FROM_DEVICE);
+ if (dma_mapping_error(dev, stats->values_iova))
+ goto err_dma_map_values;
+
+ return;
+
+err_dma_map_values:
+ dma_unmap_single(dev, stats->idx_iova, num_stats * sizeof(u32),
+ DMA_TO_DEVICE);
+err_dma_map_idx:
+ kfree(stats->values_dma_mem);
+err_alloc_values:
+ kfree(stats->idx_dma_mem);
+out:
+ stats->idx_dma_mem = NULL;
+ stats->values_dma_mem = NULL;
+}
+
+static void dpaa2_mac_clear_stats(struct dpaa2_mac *mac, struct dpaa2_mac_stats *stats,
+ size_t num_stats)
+{
+ struct device *dev = mac->net_dev->dev.parent;
+
+ if (stats->idx_dma_mem) {
+ dma_unmap_single(dev, stats->idx_iova,
+ num_stats * sizeof(u32),
+ DMA_TO_DEVICE);
+ kfree(stats->idx_dma_mem);
+ stats->idx_dma_mem = NULL;
+ }
+
+ if (stats->values_dma_mem) {
+ dma_unmap_single(dev, stats->values_iova,
+ num_stats * sizeof(u64),
+ DMA_FROM_DEVICE);
+ kfree(stats->values_dma_mem);
+ stats->values_dma_mem = NULL;
+ }
+}
static int dpaa2_mac_cmp_ver(struct dpaa2_mac *mac,
u16 ver_major, u16 ver_minor)
@@ -32,6 +146,10 @@ static void dpaa2_mac_detect_features(struct dpaa2_mac *mac)
if (dpaa2_mac_cmp_ver(mac, DPMAC_PROTOCOL_CHANGE_VER_MAJOR,
DPMAC_PROTOCOL_CHANGE_VER_MINOR) >= 0)
mac->features |= DPAA2_MAC_FEATURE_PROTOCOL_CHANGE;
+
+ if (dpaa2_mac_cmp_ver(mac, DPMAC_STATS_BUNDLE_VER_MAJOR,
+ DPMAC_STATS_BUNDLE_VER_MINOR) >= 0)
+ mac->features |= DPAA2_MAC_FEATURE_STATS_BUNDLE;
}
static int phy_mode(enum dpmac_eth_if eth_if, phy_interface_t *if_mode)
@@ -504,6 +622,10 @@ int dpaa2_mac_open(struct dpaa2_mac *mac)
mac->fw_node = fw_node;
net_dev->dev.of_node = to_of_node(mac->fw_node);
+ if (mac->features & DPAA2_MAC_FEATURE_STATS_BUNDLE)
+ dpaa2_mac_setup_stats(mac, &mac->ethtool_stats,
+ DPAA2_MAC_NUM_ETHTOOL_STATS, dpaa2_mac_ethtool_stats);
+
return 0;
err_close_dpmac:
@@ -515,64 +637,61 @@ void dpaa2_mac_close(struct dpaa2_mac *mac)
{
struct fsl_mc_device *dpmac_dev = mac->mc_dev;
+ if (mac->features & DPAA2_MAC_FEATURE_STATS_BUNDLE)
+ dpaa2_mac_clear_stats(mac, &mac->ethtool_stats, DPAA2_MAC_NUM_ETHTOOL_STATS);
+
dpmac_close(mac->mc_io, 0, dpmac_dev->mc_handle);
if (mac->fw_node)
fwnode_handle_put(mac->fw_node);
}
-static char dpaa2_mac_ethtool_stats[][ETH_GSTRING_LEN] = {
- [DPMAC_CNT_ING_ALL_FRAME] = "[mac] rx all frames",
- [DPMAC_CNT_ING_GOOD_FRAME] = "[mac] rx frames ok",
- [DPMAC_CNT_ING_ERR_FRAME] = "[mac] rx frame errors",
- [DPMAC_CNT_ING_FRAME_DISCARD] = "[mac] rx frame discards",
- [DPMAC_CNT_ING_UCAST_FRAME] = "[mac] rx u-cast",
- [DPMAC_CNT_ING_BCAST_FRAME] = "[mac] rx b-cast",
- [DPMAC_CNT_ING_MCAST_FRAME] = "[mac] rx m-cast",
- [DPMAC_CNT_ING_FRAME_64] = "[mac] rx 64 bytes",
- [DPMAC_CNT_ING_FRAME_127] = "[mac] rx 65-127 bytes",
- [DPMAC_CNT_ING_FRAME_255] = "[mac] rx 128-255 bytes",
- [DPMAC_CNT_ING_FRAME_511] = "[mac] rx 256-511 bytes",
- [DPMAC_CNT_ING_FRAME_1023] = "[mac] rx 512-1023 bytes",
- [DPMAC_CNT_ING_FRAME_1518] = "[mac] rx 1024-1518 bytes",
- [DPMAC_CNT_ING_FRAME_1519_MAX] = "[mac] rx 1519-max bytes",
- [DPMAC_CNT_ING_FRAG] = "[mac] rx frags",
- [DPMAC_CNT_ING_JABBER] = "[mac] rx jabber",
- [DPMAC_CNT_ING_ALIGN_ERR] = "[mac] rx align errors",
- [DPMAC_CNT_ING_OVERSIZED] = "[mac] rx oversized",
- [DPMAC_CNT_ING_VALID_PAUSE_FRAME] = "[mac] rx pause",
- [DPMAC_CNT_ING_BYTE] = "[mac] rx bytes",
- [DPMAC_CNT_EGR_GOOD_FRAME] = "[mac] tx frames ok",
- [DPMAC_CNT_EGR_UCAST_FRAME] = "[mac] tx u-cast",
- [DPMAC_CNT_EGR_MCAST_FRAME] = "[mac] tx m-cast",
- [DPMAC_CNT_EGR_BCAST_FRAME] = "[mac] tx b-cast",
- [DPMAC_CNT_EGR_ERR_FRAME] = "[mac] tx frame errors",
- [DPMAC_CNT_EGR_UNDERSIZED] = "[mac] tx undersized",
- [DPMAC_CNT_EGR_VALID_PAUSE_FRAME] = "[mac] tx b-pause",
- [DPMAC_CNT_EGR_BYTE] = "[mac] tx bytes",
-};
-
-#define DPAA2_MAC_NUM_STATS ARRAY_SIZE(dpaa2_mac_ethtool_stats)
-
int dpaa2_mac_get_sset_count(void)
{
- return DPAA2_MAC_NUM_STATS;
+ return DPAA2_MAC_NUM_ETHTOOL_STATS;
}
void dpaa2_mac_get_strings(u8 **data)
{
int i;
- for (i = 0; i < DPAA2_MAC_NUM_STATS; i++)
- ethtool_puts(data, dpaa2_mac_ethtool_stats[i]);
+ for (i = 0; i < DPAA2_MAC_NUM_ETHTOOL_STATS; i++)
+ ethtool_puts(data, dpaa2_mac_ethtool_stats[i].name);
}
void dpaa2_mac_get_ethtool_stats(struct dpaa2_mac *mac, u64 *data)
{
+ struct device *dev = mac->net_dev->dev.parent;
struct fsl_mc_device *dpmac_dev = mac->mc_dev;
+ u64 *cnt_values;
int i, err;
u64 value;
- for (i = 0; i < DPAA2_MAC_NUM_STATS; i++) {
+ if (!(mac->features & DPAA2_MAC_FEATURE_STATS_BUNDLE))
+ goto fallback;
+
+ if (!mac->ethtool_stats.idx_dma_mem || !mac->ethtool_stats.values_dma_mem)
+ goto fallback;
+
+ err = dpmac_get_statistics(mac->mc_io, 0, dpmac_dev->mc_handle,
+ mac->ethtool_stats.idx_iova, mac->ethtool_stats.values_iova,
+ DPAA2_MAC_NUM_ETHTOOL_STATS);
+ if (err)
+ goto fallback;
+
+ dma_sync_single_for_cpu(dev, mac->ethtool_stats.values_iova,
+ DPAA2_MAC_NUM_ETHTOOL_STATS * sizeof(u64),
+ DMA_FROM_DEVICE);
+
+ cnt_values = mac->ethtool_stats.values_dma_mem;
+ for (i = 0; i < DPAA2_MAC_NUM_ETHTOOL_STATS; i++)
+ *(data + i) = le64_to_cpu(*cnt_values++);
+
+ return;
+
+fallback:
+
+ /* Fallback and retrieve each counter one by one */
+ for (i = 0; i < DPAA2_MAC_NUM_ETHTOOL_STATS; i++) {
err = dpmac_get_counter(mac->mc_io, 0, dpmac_dev->mc_handle,
i, &value);
if (err) {
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
index 53f8d106d11e..386286209606 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
@@ -1,5 +1,5 @@
/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
-/* Copyright 2019 NXP */
+/* Copyright 2019, 2024-2026 NXP */
#ifndef DPAA2_MAC_H
#define DPAA2_MAC_H
@@ -11,6 +11,12 @@
#include "dpmac.h"
#include "dpmac-cmd.h"
+struct dpaa2_mac_stats {
+ u32 *idx_dma_mem;
+ u64 *values_dma_mem;
+ dma_addr_t idx_iova, values_iova;
+};
+
struct dpaa2_mac {
struct fsl_mc_device *mc_dev;
struct dpmac_link_state state;
@@ -28,6 +34,8 @@ struct dpaa2_mac {
struct fwnode_handle *fw_node;
struct phy *serdes_phy;
+
+ struct dpaa2_mac_stats ethtool_stats;
};
static inline bool dpaa2_mac_is_type_phy(struct dpaa2_mac *mac)
--
2.25.1On Wed, Feb 25, 2026 at 05:06:45PM +0200, Ioana Ciornei wrote:
...
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
...
> +static void dpaa2_mac_setup_stats(struct dpaa2_mac *mac, struct dpaa2_mac_stats *stats,
> + size_t num_stats, const struct dpmac_counter *counters)
> +{
> + struct device *dev = mac->net_dev->dev.parent;
> + u32 *cnt_idx;
Hi Ioana,
The type of cnt_idx is u32.
> +
> + stats->idx_dma_mem = kcalloc(num_stats, sizeof(u32), GFP_KERNEL);
> + if (!stats->idx_dma_mem)
> + goto out;
> +
> + stats->values_dma_mem = kcalloc(num_stats, sizeof(u64), GFP_KERNEL);
> + if (!stats->values_dma_mem)
> + goto err_alloc_values;
> +
> + cnt_idx = stats->idx_dma_mem;
As is that of idx_dma_mem. So the types match here.
> + for (size_t i = 0; i < num_stats; i++)
> + *cnt_idx++ = cpu_to_le32((u32)(counters[i].id));
But here __le32 values are assigned to elements of cnt_idx.
I think that the type of both cnt_idx and stats->idx_dma_mem
should probably be __le32 * rather than u32 *.
Flagged by Sparse v0.6.5-rc1.
...
> void dpaa2_mac_get_ethtool_stats(struct dpaa2_mac *mac, u64 *data)
> {
> + struct device *dev = mac->net_dev->dev.parent;
> struct fsl_mc_device *dpmac_dev = mac->mc_dev;
> + u64 *cnt_values;
> int i, err;
> u64 value;
...
> + cnt_values = mac->ethtool_stats.values_dma_mem;
> + for (i = 0; i < DPAA2_MAC_NUM_ETHTOOL_STATS; i++)
> + *(data + i) = le64_to_cpu(*cnt_values++);
Likewise, I think the type of both cnt_values and
mac->ethtool_stats.values_dma_mem should be __le64 8 rather than u64 *.
And there is a similar problem in patch 3/5 centering on the use of
le64_to_cpu() in dpaa2_mac_transfer_stats().
Also flagged by Sparse.
...
On Sun, Mar 01, 2026 at 04:09:25PM +0000, Simon Horman wrote:
> On Wed, Feb 25, 2026 at 05:06:45PM +0200, Ioana Ciornei wrote:
>
> ...
>
> > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
>
> ...
>
> > +static void dpaa2_mac_setup_stats(struct dpaa2_mac *mac, struct dpaa2_mac_stats *stats,
> > + size_t num_stats, const struct dpmac_counter *counters)
> > +{
> > + struct device *dev = mac->net_dev->dev.parent;
> > + u32 *cnt_idx;
>
> Hi Ioana,
>
> The type of cnt_idx is u32.
>
> > +
> > + stats->idx_dma_mem = kcalloc(num_stats, sizeof(u32), GFP_KERNEL);
> > + if (!stats->idx_dma_mem)
> > + goto out;
> > +
> > + stats->values_dma_mem = kcalloc(num_stats, sizeof(u64), GFP_KERNEL);
> > + if (!stats->values_dma_mem)
> > + goto err_alloc_values;
> > +
> > + cnt_idx = stats->idx_dma_mem;
>
> As is that of idx_dma_mem. So the types match here.
>
> > + for (size_t i = 0; i < num_stats; i++)
> > + *cnt_idx++ = cpu_to_le32((u32)(counters[i].id));
>
> But here __le32 values are assigned to elements of cnt_idx.
>
> I think that the type of both cnt_idx and stats->idx_dma_mem
> should probably be __le32 * rather than u32 *.
>
> Flagged by Sparse v0.6.5-rc1.
Yes, I missed this. Thanks for catching it!
>
>
> ...
>
> > void dpaa2_mac_get_ethtool_stats(struct dpaa2_mac *mac, u64 *data)
> > {
> > + struct device *dev = mac->net_dev->dev.parent;
> > struct fsl_mc_device *dpmac_dev = mac->mc_dev;
> > + u64 *cnt_values;
> > int i, err;
> > u64 value;
>
> ...
>
> > + cnt_values = mac->ethtool_stats.values_dma_mem;
> > + for (i = 0; i < DPAA2_MAC_NUM_ETHTOOL_STATS; i++)
> > + *(data + i) = le64_to_cpu(*cnt_values++);
>
> Likewise, I think the type of both cnt_values and
> mac->ethtool_stats.values_dma_mem should be __le64 8 rather than u64 *.
>
> And there is a similar problem in patch 3/5 centering on the use of
> le64_to_cpu() in dpaa2_mac_transfer_stats().
>
> Also flagged by Sparse.
>
Yes, this is valid too. Will fix.
Ioana
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: dpaa2-mac: retrieve MAC statistics in one firmware command
The latest MC firmware version added a new command to retrieve all DPMAC
counters in a single firmware call. Use this new command, when possible,
in dpaa2-mac as well. In order to use the dpmac_get_statistics() API,
two DMA memory areas are used: one to transmit what counters the driver
is requesting and one to receive the values of those counters. These
memory areas are allocated and DMA mapped at probe time so that we don't
waste time at runtime.
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> index 422ce13a7c94..63dc597dbd7c 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
[ ... ]
> @@ -15,7 +15,121 @@
> #define DPMAC_PROTOCOL_CHANGE_VER_MAJOR 4
> #define DPMAC_PROTOCOL_CHANGE_VER_MINOR 8
>
> +#define DPMAC_STATS_BUNDLE_VER_MAJOR 4
> +#define DPMAC_STATS_BUNDLE_VER_MINOR 10
> +
> #define DPAA2_MAC_FEATURE_PROTOCOL_CHANGE BIT(0)
> +#define DPAA2_MAC_FEATURE_STATS_BUNDLE BIT(1)
> +
> +struct dpmac_counter {
> + enum dpmac_counter_id id;
> + const char *name;
> +};
> +
> +#define DPMAC_UNSTRUCTURED_COUNTER(counter_id, counter_name) \
> + { \
> + .id = counter_id, \
> + .name = counter_name, \
> + }
> +
> +static const struct dpmac_counter dpaa2_mac_ethtool_stats[] = {
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_ALL_FRAME, "[mac] rx all frames"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_GOOD_FRAME, "[mac] rx frames ok"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_ERR_FRAME, "[mac] rx frame errors"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_FRAME_DISCARD, "[mac] rx frame discards"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_UCAST_FRAME, "[mac] rx u-cast"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_BCAST_FRAME, "[mac] rx b-cast"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_MCAST_FRAME, "[mac] rx m-cast"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_FRAME_64, "[mac] rx 64 bytes"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_FRAME_127, "[mac] rx 65-127 bytes"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_FRAME_255, "[mac] rx 128-255 bytes"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_FRAME_511, "[mac] rx 256-511 bytes"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_FRAME_1023, "[mac] rx 512-1023 bytes"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_FRAME_1518, "[mac] rx 1024-1518 bytes"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_FRAME_1519_MAX, "[mac] rx 1519-max bytes"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_FRAG, "[mac] rx frags"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_JABBER, "[mac] rx jabber"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_ALIGN_ERR, "[mac] rx align errors"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_OVERSIZED, "[mac] rx oversized"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_VALID_PAUSE_FRAME, "[mac] rx pause"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_ING_BYTE, "[mac] rx bytes"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_EGR_GOOD_FRAME, "[mac] tx frames ok"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_EGR_UCAST_FRAME, "[mac] tx u-cast"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_EGR_MCAST_FRAME, "[mac] tx m-cast"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_EGR_BCAST_FRAME, "[mac] tx b-cast"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_EGR_ERR_FRAME, "[mac] tx frame errors"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_EGR_UNDERSIZED, "[mac] tx undersized"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_EGR_VALID_PAUSE_FRAME, "[mac] tx b-pause"),
> + DPMAC_UNSTRUCTURED_COUNTER(DPMAC_CNT_EGR_BYTE, "[mac] tx bytes"),
> +};
> +
> +#define DPAA2_MAC_NUM_ETHTOOL_STATS ARRAY_SIZE(dpaa2_mac_ethtool_stats)
[ ... ]
> void dpaa2_mac_close(struct dpaa2_mac *mac)
> {
> struct fsl_mc_device *dpmac_dev = mac->mc_dev;
>
> + if (mac->features & DPAA2_MAC_FEATURE_STATS_BUNDLE)
> + dpaa2_mac_clear_stats(mac, &mac->ethtool_stats, DPAA2_MAC_NUM_ETHTOOL_STATS);
> +
> dpmac_close(mac->mc_io, 0, dpmac_dev->mc_handle);
> if (mac->fw_node)
> fwnode_handle_put(mac->fw_node);
> }
>
> -static char dpaa2_mac_ethtool_stats[][ETH_GSTRING_LEN] = {
> - [DPMAC_CNT_ING_ALL_FRAME] = "[mac] rx all frames",
> - [DPMAC_CNT_ING_GOOD_FRAME] = "[mac] rx frames ok",
> - [DPMAC_CNT_ING_ERR_FRAME] = "[mac] rx frame errors",
> - [DPMAC_CNT_ING_FRAME_DISCARD] = "[mac] rx frame discards",
> - [DPMAC_CNT_ING_UCAST_FRAME] = "[mac] rx u-cast",
> - [DPMAC_CNT_ING_BCAST_FRAME] = "[mac] rx b-cast",
> - [DPMAC_CNT_ING_MCAST_FRAME] = "[mac] rx m-cast",
> - [DPMAC_CNT_ING_FRAME_64] = "[mac] rx 64 bytes",
> - [DPMAC_CNT_ING_FRAME_127] = "[mac] rx 65-127 bytes",
> - [DPMAC_CNT_ING_FRAME_255] = "[mac] rx 128-255 bytes",
> - [DPMAC_CNT_ING_FRAME_511] = "[mac] rx 256-511 bytes",
> - [DPMAC_CNT_ING_FRAME_1023] = "[mac] rx 512-1023 bytes",
> - [DPMAC_CNT_ING_FRAME_1518] = "[mac] rx 1024-1518 bytes",
> - [DPMAC_CNT_ING_FRAME_1519_MAX] = "[mac] rx 1519-max bytes",
> - [DPMAC_CNT_ING_FRAG] = "[mac] rx frags",
> - [DPMAC_CNT_ING_JABBER] = "[mac] rx jabber",
> - [DPMAC_CNT_ING_ALIGN_ERR] = "[mac] rx align errors",
> - [DPMAC_CNT_ING_OVERSIZED] = "[mac] rx oversized",
> - [DPMAC_CNT_ING_VALID_PAUSE_FRAME] = "[mac] rx pause",
> - [DPMAC_CNT_ING_BYTE] = "[mac] rx bytes",
> - [DPMAC_CNT_EGR_GOOD_FRAME] = "[mac] tx frames ok",
> - [DPMAC_CNT_EGR_UCAST_FRAME] = "[mac] tx u-cast",
> - [DPMAC_CNT_EGR_MCAST_FRAME] = "[mac] tx m-cast",
> - [DPMAC_CNT_EGR_BCAST_FRAME] = "[mac] tx b-cast",
> - [DPMAC_CNT_EGR_ERR_FRAME] = "[mac] tx frame errors",
> - [DPMAC_CNT_EGR_UNDERSIZED] = "[mac] tx undersized",
> - [DPMAC_CNT_EGR_VALID_PAUSE_FRAME] = "[mac] tx b-pause",
> - [DPMAC_CNT_EGR_BYTE] = "[mac] tx bytes",
> -};
> -
> -#define DPAA2_MAC_NUM_STATS ARRAY_SIZE(dpaa2_mac_ethtool_stats)
> -
> int dpaa2_mac_get_sset_count(void)
> {
> - return DPAA2_MAC_NUM_STATS;
> + return DPAA2_MAC_NUM_ETHTOOL_STATS;
> }
>
> void dpaa2_mac_get_strings(u8 **data)
> {
> int i;
>
> - for (i = 0; i < DPAA2_MAC_NUM_STATS; i++)
> - ethtool_puts(data, dpaa2_mac_ethtool_stats[i]);
> + for (i = 0; i < DPAA2_MAC_NUM_ETHTOOL_STATS; i++)
> + ethtool_puts(data, dpaa2_mac_ethtool_stats[i].name);
> }
>
> void dpaa2_mac_get_ethtool_stats(struct dpaa2_mac *mac, u64 *data)
> {
> + struct device *dev = mac->net_dev->dev.parent;
> struct fsl_mc_device *dpmac_dev = mac->mc_dev;
> + u64 *cnt_values;
> int i, err;
> u64 value;
>
> - for (i = 0; i < DPAA2_MAC_NUM_STATS; i++) {
> + if (!(mac->features & DPAA2_MAC_FEATURE_STATS_BUNDLE))
> + goto fallback;
> +
> + if (!mac->ethtool_stats.idx_dma_mem || !mac->ethtool_stats.values_dma_mem)
> + goto fallback;
> +
> + err = dpmac_get_statistics(mac->mc_io, 0, dpmac_dev->mc_handle,
> + mac->ethtool_stats.idx_iova, mac->ethtool_stats.values_iova,
> + DPAA2_MAC_NUM_ETHTOOL_STATS);
> + if (err)
> + goto fallback;
> +
> + dma_sync_single_for_cpu(dev, mac->ethtool_stats.values_iova,
> + DPAA2_MAC_NUM_ETHTOOL_STATS * sizeof(u64),
> + DMA_FROM_DEVICE);
> +
> + cnt_values = mac->ethtool_stats.values_dma_mem;
> + for (i = 0; i < DPAA2_MAC_NUM_ETHTOOL_STATS; i++)
> + *(data + i) = le64_to_cpu(*cnt_values++);
> +
> + return;
> +
> +fallback:
> +
> + /* Fallback and retrieve each counter one by one */
> + for (i = 0; i < DPAA2_MAC_NUM_ETHTOOL_STATS; i++) {
> err = dpmac_get_counter(mac->mc_io, 0, dpmac_dev->mc_handle,
> i, &value);
^
Does the fallback path need to pass dpaa2_mac_ethtool_stats[i].id instead
of i here?
The old code used a sparse array indexed by enum dpmac_counter_id values:
static char dpaa2_mac_ethtool_stats[][ETH_GSTRING_LEN] = {
[DPMAC_CNT_ING_ALL_FRAME] = "[mac] rx all frames",
...
};
where the loop index i directly corresponded to the enum value passed to
dpmac_get_counter(). The new code uses a compact array where array index 0
contains DPMAC_CNT_ING_ALL_FRAME (enum value 18), but the fallback code
still passes i (sequential 0, 1, 2...) instead of the stored enum value.
When i=0, the code requests counter 0 (DPMAC_CNT_ING_FRAME_64) but the
name is "[mac] rx all frames" (DPMAC_CNT_ING_ALL_FRAME). This would cause
all statistics to report wrong values for their labels.
> if (err) {
> netdev_err(mac->net_dev,
> "dpmac_get_counter(%d) failed\n", i);
[ ... ]
--
pw-bot: cr
On Thu, Feb 26, 2026 at 06:26:13PM -0800, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
>
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> net: dpaa2-mac: retrieve MAC statistics in one firmware command
>
> The latest MC firmware version added a new command to retrieve all DPMAC
> counters in a single firmware call. Use this new command, when possible,
> in dpaa2-mac as well. In order to use the dpmac_get_statistics() API,
> two DMA memory areas are used: one to transmit what counters the driver
> is requesting and one to receive the values of those counters. These
> memory areas are allocated and DMA mapped at probe time so that we don't
> waste time at runtime.
>
> > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> > index 422ce13a7c94..63dc597dbd7c 100644
> > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
>
(...)
> > +fallback:
> > +
> > + /* Fallback and retrieve each counter one by one */
> > + for (i = 0; i < DPAA2_MAC_NUM_ETHTOOL_STATS; i++) {
> > err = dpmac_get_counter(mac->mc_io, 0, dpmac_dev->mc_handle,
> > i, &value);
> ^
>
> Does the fallback path need to pass dpaa2_mac_ethtool_stats[i].id instead
> of i here?
>
> The old code used a sparse array indexed by enum dpmac_counter_id values:
>
> static char dpaa2_mac_ethtool_stats[][ETH_GSTRING_LEN] = {
> [DPMAC_CNT_ING_ALL_FRAME] = "[mac] rx all frames",
> ...
> };
>
> where the loop index i directly corresponded to the enum value passed to
> dpmac_get_counter(). The new code uses a compact array where array index 0
> contains DPMAC_CNT_ING_ALL_FRAME (enum value 18), but the fallback code
> still passes i (sequential 0, 1, 2...) instead of the stored enum value.
>
> When i=0, the code requests counter 0 (DPMAC_CNT_ING_FRAME_64) but the
> name is "[mac] rx all frames" (DPMAC_CNT_ING_ALL_FRAME). This would cause
> all statistics to report wrong values for their labels.
Yes, that is indeed true. Thanks for catching it!
Ioana
© 2016 - 2026 Red Hat, Inc.