From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>,
Andrew Lunn <andrew+netdev@lunn.ch>,
Chen-Yu Tsai <wens@kernel.org>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>,
linux-arm-kernel@lists.infradead.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-sunxi@lists.linux.dev, netdev@vger.kernel.org,
Paolo Abeni <pabeni@redhat.com>,
Samuel Holland <samuel@sholland.org>,
Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
Subject: [PATCH RFC net-next 00/10] net: stmmac: clean up / fix synopsys IP version checks
Date: Wed, 8 Apr 2026 10:26:20 +0100 [thread overview]
Message-ID: <adYfPBHsXxQUsMyr@shell.armlinux.org.uk> (raw)
Oh lookie, another relatively large stmmac patch series as a result of
reviewing Jitendra Vegiraju's patch series! Yay! :(
This series cleans up and in some cases fixes the Synopsys IP version
checks, which is a per-core numberspace.
For example, for dwmac1000 and dwmac4, we have versions such as v3.4 to
up v5.2, which are represented by two BCD digits. For XGMAC, we have
versions v2.1 and v2.2 represented by 0x21 and 0x22 respectively. For
XLGMAC, we have v2.0, which is 0x20.
Note that hwif.c supports GMAC4 core type with the IP version in the
range 0..0x40, which overlaps with the GMAC core type.
Thus, testing the Synopsys IP version for <= v4.0 will match dwmac1000,
potentially some dwmac4, and all xgmac and xlgmac cores. In some cases,
the latter two are provably incorrect.
The Synopsys IP version is named "synopsys_id" in the driver - as
stated, it is the Synopsys IP version, and the register field is called
snpsver. Use this name throughout the driver to avoid confusion. Also,
dev_id seems to actually be userver.
The first decision making change is for dche, which is only looked at
by the dwmac4 DMA code which requires core_type == DWMAC_CORE_GMAC4.
Thus, it makes no sense to merely look at the IP version. Since setting
this to true makes no sense for non-GMAC4 cores, force it to false for
non-DWMAC_CORE_GMAC4 cores.
The MAC .debug() method is only populated by the dwmac1000 and dwmac4
cores, yet the test checks only for the IP version >= v3.5. While this
works today, it may not work in the future given the different version
numberspaces. Make this conditional on DWMAC_CORE_GMAC or GMAC4 core
types to limit the version check. Ideally, it would be better to
move the version check inside the .dwbug() method implementation, but
that would require the method signature to change.
As getting to that point involved quite a bit of research, add comments
into stmmac_get_ethtool_stats() so others don't have to re-analyse the
code.
Clean up the resulting code in stmmac_get_ethtool_stats() given the
complexity, adding documentation for each test.
Address the rx_coe handling, which checks for IP version >= v4.0 or
the XGMAC core type. However, the members being printed are only
present in the GMAC4 and XGMAC core type in one path, and GMAC in
the other. There is an odd-ball which is sun8i, but we preserve its
current behaviour. Also, given the research, add documentation in
appropriate places. Note that priv->plat->rx_coe (and also
priv->hw->rx_csum) has different value spaces depending on the core
type - which is not nice.
Lastly, make the printing of the COE type, which as identified above,
depend on the GMAC core type.
This leaves four locations unaddressed:
1) PTP filter code:
case HWTSTAMP_FILTER_PTP_V2_EVENT:
/* PTP v2/802.AS1 any layer, any kind of event packet */
config->rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
ptp_v2 = PTP_TCR_TSVER2ENA;
snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
if (priv->snpsver < DWMAC_CORE_4_10)
ts_event_en = PTP_TCR_TSEVNTENA;
I don't yet have an answer for this one, but it does mean that
PTP_TCR_TSEVNTENA will be set for XGMAC cores for
HWTSTAMP_FILTER_PTP_V2_EVENT... if they have PTP support.
2) riwt watchdog:
/* Rx Watchdog is available in the COREs newer than the 3.40.
* In some case, for example on bugged HW this feature
* has to be disable and this can be done by passing the
* riwt_off field from the platform.
*/
if ((priv->snpsver >= DWMAC_CORE_3_50 ||
priv->plat->core_type == DWMAC_CORE_XGMAC) &&
!priv->plat->riwt_off) {
priv->use_riwt = 1;
This to me looks like the version check is only really applicable for
the DWMAC_CORE_GMAC core type. So, I'm thinking that:
if (priv->plat->core_type >= DWMAC_CORE_GMAC4 ||
(priv->plat->core_type == DWMAC_CORE_GMAC &&
priv->snpsver >= DWMAC_CORE_3_50)) {
if (!priv->plat->rwit_off) {
...
would be clearer (note the splitting of the if() conditions which seems
to me would be more readable than trying to munge it all into one
expression.)
3) max_mtu setup:
/* Set the maximum MTU.
* For XGMAC cores, 16KiB.
* For cores using enhanced descriptors or GMAC cores >= v4.00, 9kB.
* For everything else, PAGE_SIZE - NET_SKB_PAD - NET_IP_ALIGN -
* aligned skb_shared_info.
*/
if (priv->plat->core_type == DWMAC_CORE_XGMAC)
ndev->max_mtu = XGMAC_JUMBO_LEN;
else if (priv->plat->enh_desc || priv->snpsver >= DWMAC_CORE_4_00)
ndev->max_mtu = JUMBO_LEN;
else
ndev->max_mtu = SKB_MAX_HEAD(NET_SKB_PAD + NET_IP_ALIGN);
which I've previously identified is very buggy, and I'm not sure what
the real tests here should be. I'm quite certain that the max_mtu
values are basically completely wrong, especially in the last case.
This needs a lot of analysis and comparing with documentation to get
to the bottom of what the tests and resulting max_mtu values should
actually be.
.../net/ethernet/stmicro/stmmac/dwmac-loongson.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/hwif.c | 54 +++++++++++-----------
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 +-
.../net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 51 +++++++++++---------
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 24 ++++++----
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 8 ++--
include/linux/stmmac.h | 7 +++
9 files changed, 85 insertions(+), 67 deletions(-)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
next reply other threads:[~2026-04-08 9:26 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-08 9:26 Russell King (Oracle) [this message]
2026-04-08 9:26 ` [PATCH RFC net-next 01/10] net: stmmac: rename min_id to min_snpsver Russell King (Oracle)
2026-04-08 9:26 ` [PATCH RFC net-next 02/10] net: stmmac: rename dev_id to userver Russell King (Oracle)
2026-04-08 9:26 ` [PATCH RFC net-next 03/10] net: stmmac: always fill in ver->userver Russell King (Oracle)
2026-04-08 9:26 ` [PATCH RFC net-next 04/10] net: stmmac: use ver->userver and ver->snpsver to print version Russell King (Oracle)
2026-04-08 9:27 ` [PATCH RFC net-next 05/10] net: stmmac: rename confusing synopsys_id Russell King (Oracle)
2026-04-08 9:27 ` [PATCH RFC net-next 06/10] net: stmmac: dche is only for GMAC4 cores Russell King (Oracle)
2026-04-08 9:27 ` [PATCH RFC net-next 07/10] net: stmmac: limit MAC .debug() to dwmac1000 and dwmac4 Russell King (Oracle)
2026-04-08 9:27 ` [PATCH RFC net-next 08/10] net: stmmac: simplify stmmac_get_ethtool_stats() Russell King (Oracle)
2026-04-08 9:27 ` [PATCH RFC net-next 09/10] net: stmmac: clean up test for rx_coe debug printing Russell King (Oracle)
2026-04-08 9:27 ` [PATCH RFC net-next 10/10] net: stmmac: only print receive COE type for GMAC cores Russell King (Oracle)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=adYfPBHsXxQUsMyr@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=alexandre.torgue@foss.st.com \
--cc=andrew+netdev@lunn.ch \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=jitendra.vegiraju@broadcom.com \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=linux-sunxi@lists.linux.dev \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=samuel@sholland.org \
--cc=wens@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox