Thread (34 messages) 34 messages, 6 authors, 2026-03-03

Re: [net-next,2/5] net: dpaa2-mac: retrieve MAC statistics in one firmware command

From: Ioana Ciornei <ioana.ciornei@nxp.com>
Date: 2026-02-27 10:38:03
Also in: linux-kselftest, lkml

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.
quoted
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
(...)
quoted
+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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help