From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Joris Vaisvila <joey@tinyisr.com>,
Daniel Golle <daniel@makrotpia.org>,
Stefan Roese <stefan.roese@mailbox.org>,
Jakub Kicinski <kuba@kernel.org>, Sasha Levin <sashal@kernel.org>,
nbd@nbd.name, lorenzo@kernel.org, andrew+netdev@lunn.ch,
davem@davemloft.net, edumazet@google.com, pabeni@redhat.com,
matthias.bgg@gmail.com, angelogioacchino.delregno@collabora.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org
Subject: [PATCH AUTOSEL 7.0-6.12] net: ethernet: mtk_eth_soc: avoid writing to ESW registers on MT7628
Date: Mon, 20 Apr 2026 09:20:17 -0400 [thread overview]
Message-ID: <20260420132314.1023554-223-sashal@kernel.org> (raw)
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: Joris Vaisvila <joey@tinyisr.com>
[ Upstream commit 9a04d3b2f0708a9e5e1f731bafb69b040bb934a0 ]
The MT7628 has a fixed-link PHY and does not expose MAC control
registers. Writes to these registers only corrupt the ESW VLAN
configuration.
This patch explicitly registers no-op phylink_mac_ops for MT7628, as
after removing the invalid register accesses, the existing
phylink_mac_ops effectively become no-ops.
This code was introduced by commit 296c9120752b
("net: ethernet: mediatek: Add MT7628/88 SoC support")
Signed-off-by: Joris Vaisvila <joey@tinyisr.com>
Reviewed-by: Daniel Golle <daniel@makrotpia.org>
Reviewed-by: Stefan Roese <stefan.roese@mailbox.org>
Link: https://patch.msgid.link/20260226154547.68553-1-joey@tinyisr.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have all the information needed for a thorough analysis. Let me
compile my findings.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: Subject Line**
- Subsystem: `net: ethernet: mtk_eth_soc`
- Action verb: "avoid" — strongly implies a bug fix (preventing harmful
behavior)
- Summary: Avoid writing to ESW (Embedded Switch) registers on MT7628 to
prevent corruption
**Step 1.2: Tags**
- No `Fixes:` tag, but commit body references `296c9120752b` ("net:
ethernet: mediatek: Add MT7628/88 SoC support") from August 2019 as
the introducing commit
- `Signed-off-by: Joris Vaisvila <joey@tinyisr.com>` — author
- `Reviewed-by: Daniel Golle <daniel@makrotpia.org>` — MediaTek ethernet
maintainer/expert
- `Reviewed-by: Stefan Roese <stefan.roese@mailbox.org>` — original
author of the MT7628 support commit
- `Link:` to patch.msgid.link (standard netdev submission)
- `Signed-off-by: Jakub Kicinski <kuba@kernel.org>` — netdev maintainer
applied it
Record: Two reviewer tags from highly relevant people (original MT7628
author + subsystem expert). No syzbot. No explicit Cc: stable.
**Step 1.3: Commit Body**
- Bug: MT7628 has a fixed-link PHY and does not expose MAC control
registers. Writes to `MTK_MAC_MCR(x)` (offset 0x10100) on MT7628 hit
the ESW VLAN configuration instead of non-existent MAC control
registers.
- Symptom: VLAN configuration corruption on MT7628
- Root cause: The phylink_mac_ops callbacks (`link_down`, `link_up`,
`mac_finish`) write to `MTK_MAC_MCR` registers without checking for
MT7628
**Step 1.4: Hidden Bug Fix Detection**
This is clearly a data corruption fix. The word "avoid" means preventing
invalid register writes that corrupt VLAN config.
## PHASE 2: DIFF ANALYSIS
**Step 2.1: Inventory**
- Single file: `drivers/net/ethernet/mediatek/mtk_eth_soc.c`
- Approximate: +27 lines added, -5 lines removed
- Functions modified: `mtk_mac_config` (guard removed), `mtk_add_mac`
(ops selection added)
- Functions added: `rt5350_mac_config`, `rt5350_mac_link_down`,
`rt5350_mac_link_up` (all no-ops), `rt5350_phylink_ops` (new ops
struct)
**Step 2.2: Code Flow Change**
1. In `mtk_mac_config`: The `!MTK_HAS_CAPS(eth->soc->caps,
MTK_SOC_MT7628)` guard was removed. Safe because MT7628 now uses
entirely different (no-op) ops, so this function is never called for
MT7628.
2. In `mtk_add_mac`: Added conditional to select `rt5350_phylink_ops`
for MT7628 instead of `mtk_phylink_ops`.
3. New no-op functions: `rt5350_mac_config`, `rt5350_mac_link_down`,
`rt5350_mac_link_up` — all empty.
**Step 2.3: Bug Mechanism**
Category: **Hardware workaround / data corruption fix**
The bug: On MT7628, register offset 0x10100 is part of the ESW VLAN
configuration, not a MAC control register. The existing
`mtk_mac_link_down()`, `mtk_mac_link_up()`, and `mtk_mac_finish()` all
write to `MTK_MAC_MCR(mac->id)` (= 0x10100) without MT7628 checks. Only
`mtk_mac_config()` had a guard. Every link state change event corrupts
the VLAN configuration.
**Step 2.4: Fix Quality**
- Obviously correct: The fix prevents ALL register writes by
substituting no-op callbacks
- Minimal regression risk: Empty callbacks for a fixed-link PHY that
never needed MAC configuration
- Self-contained in one file
- Reviewed by the original MT7628 author (Stefan Roese) and MediaTek
network expert (Daniel Golle)
## PHASE 3: GIT HISTORY
**Step 3.1: Blame**
- The buggy code in `mtk_mac_link_down`/`mtk_mac_link_up` was introduced
by `b8fc9f30821ec0` (René van Dorst, 2019-08-25) during the phylink
conversion
- The `mtk_mac_config` guard was already in `b8fc9f30821ec0` but was
never added to `link_down`/`link_up`/`finish`
**Step 3.2: Original commit**
- `296c9120752b` ("Add MT7628/88 SoC support") was merged in v5.3-rc6
(August 2019)
- This commit is present in all stable trees from v5.3 onwards
(confirmed in p-5.10, p-5.15 tags)
**Step 3.3/3.4: Author & File History**
- Joris Vaisvila is not a frequent kernel contributor (only 1-2 commits
found)
- However, both reviewers are well-known in this subsystem
- File has 231 commits since 296c9120752b; 32 since v6.12
**Step 3.5: Dependencies**
- The patch is self-contained. The no-op ops pattern doesn't depend on
any other patches.
- In v6.6, the `mtk_mac_finish` function also writes to `MTK_MAC_MCR`
without MT7628 guard — same bug. The no-op ops approach fixes all
callbacks at once.
## PHASE 4: MAILING LIST
Lore/b4 dig returned results but couldn't access full discussions due to
Anubis protection. The patch was submitted as
`20260226154547.68553-1-joey@tinyisr.com` and accepted by Jakub Kicinski
(netdev maintainer).
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1-5.4: Impact Surface**
- `mtk_mac_link_down` is called by phylink whenever the link goes down —
every cable disconnect, PHY negotiation change
- `mtk_mac_link_up` is called on every link up event
- `mtk_mac_finish` is called during PHY configuration
- On MT7628, these are called regularly during normal operation
- `mtk_set_mcr_max_rx` at line 3886 already has its own `MTK_SOC_MT7628`
guard, confirming the developers know these registers don't exist on
MT7628
## PHASE 6: STABLE TREE ANALYSIS
**Step 6.1:** The buggy code exists in ALL stable trees from v5.3+,
including v5.15, v6.1, v6.6, and 6.12.
- In v6.6: `mtk_mac_link_down` at line 689 unconditionally writes to
`MTK_MAC_MCR` — confirmed the same bug
- In v6.6: `mtk_mac_link_up` at line 769 also unconditionally writes to
`MTK_MAC_MCR` — confirmed
- In v6.6: `mtk_mac_finish` at line 660 also writes to `MTK_MAC_MCR` —
confirmed
**Step 6.2: Backport Difficulty**
For v7.0: Should apply cleanly or with minor fuzz.
For v6.6 and older: Will need rework. The `mtk_mac_link_down`/`link_up`
implementations differ significantly (v7.0 has xgmii handling added by
`51cf06ddafc91e`). However, the *concept* of the fix (separate no-op
ops) is portable.
## PHASE 7: SUBSYSTEM CONTEXT
- Subsystem: Network driver (embedded Ethernet), IMPORTANT criticality
for MT7628 users
- MT7628/MT7688 is a widely-used MIPS SoC found in popular embedded
platforms (Omega2, VoCore2, many OpenWrt routers)
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: Affected Users**
- All MT7628/MT7688 users (embedded routers running Linux with VLANs)
**Step 8.2: Trigger Conditions**
- Triggered on every link state change (boot, cable plug/unplug, PHY
state change)
- Extremely common — happens during normal boot
**Step 8.3: Failure Mode**
- **ESW VLAN configuration corruption** — MEDIUM-HIGH severity
- VLAN configuration is silently corrupted, leading to incorrect network
behavior
- Not a crash but a data corruption issue affecting network
configuration
**Step 8.4: Risk-Benefit**
- Benefit: HIGH — prevents VLAN corruption on every MT7628 system
- Risk: LOW — the fix adds empty callback functions and selects them
conditionally; the no-op approach is obviously correct for a fixed-
link PHY with no MAC control registers
## PHASE 9: FINAL SYNTHESIS
**Evidence FOR backporting:**
1. Fixes real data corruption (VLAN config) on real hardware
(MT7628/MT7688)
2. Bug present since v5.3 (2019) — affects all stable trees
3. Reviewed by original MT7628 author and subsystem expert
4. Accepted by netdev maintainer (Jakub Kicinski)
5. Fix is obviously correct (no-op callbacks for hardware without MAC
registers)
6. Single file change, well-contained
7. Other code in the same file already has MT7628 guards for the same
registers (`mtk_set_mcr_max_rx` at line 3886)
**Evidence AGAINST backporting:**
1. ~30 lines of new code (not trivially small, but straightforward)
2. May need rework for older stable trees (6.6, 6.1, 5.15) due to
function refactoring
3. No explicit Cc: stable (expected for commits under review)
**Stable Rules Checklist:**
1. Obviously correct and tested? YES — reviewed by 2 experts, one the
original author
2. Fixes a real bug? YES — VLAN config corruption on MT7628
3. Important issue? YES — data corruption, affects all MT7628 users
4. Small and contained? YES — single file, ~30 lines
5. No new features? CORRECT — only prevents invalid register writes
6. Can apply to stable? YES for 7.0; needs rework for older trees
**Verification:**
- [Phase 1] Parsed tags: Reviewed-by from Daniel Golle and Stefan Roese,
Link to netdev submission
- [Phase 2] Diff analysis: Adds no-op phylink_mac_ops for MT7628,
selects them in `mtk_add_mac()`, removes now-unreachable guard in
`mtk_mac_config()`
- [Phase 3] git blame: buggy code from `b8fc9f30821ec0` (2019), phylink
conversion missing MT7628 guards in link_down/link_up
- [Phase 3] git show 296c9120752b: confirmed original MT7628 support
commit from v5.3 era (2019-08-16)
- [Phase 3] git tag --contains: original commit present in p-5.10,
p-5.15 tags (all active stable trees)
- [Phase 5] Verified `mtk_mac_link_down` writes to
`MTK_MAC_MCR(mac->id)` without MT7628 check (line 731 in current code)
- [Phase 5] Verified `mtk_mac_link_up`→`mtk_gdm_mac_link_up` writes to
`MTK_MAC_MCR(mac->id)` (line 846) without MT7628 check
- [Phase 5] Verified `mtk_mac_finish` writes to `MTK_MAC_MCR(mac->id)`
(line 709/716) without MT7628 check
- [Phase 5] Confirmed `mtk_set_mcr_max_rx` (line 3886) already guards
against MT7628, proving developers know these registers don't exist on
MT7628
- [Phase 6] Verified v6.6 stable has the same bug: `mtk_mac_link_down`
(line 689) and `mtk_mac_link_up` (line 769) unconditionally write to
`MTK_MAC_MCR`
- [Phase 6] `MTK_MAC_MCR(x)` = 0x10100 + x*0x100, confirmed in header
file (line 453)
- [Phase 8] VLAN corruption confirmed by commit message: "Writes to
these registers only corrupt the ESW VLAN configuration"
- UNVERIFIED: Could not access full lore.kernel.org discussion due to
Anubis protection; relied on tags in the commit message
**YES**
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 34 ++++++++++++++++++---
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index ddc321a02fdae..bb8ced22ca3be 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -562,9 +562,7 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode,
int val, ge_mode, err = 0;
u32 i;
- /* MT76x8 has no hardware settings between for the MAC */
- if (!MTK_HAS_CAPS(eth->soc->caps, MTK_SOC_MT7628) &&
- mac->interface != state->interface) {
+ if (mac->interface != state->interface) {
/* Setup soc pin functions */
switch (state->interface) {
case PHY_INTERFACE_MODE_TRGMII:
@@ -956,6 +954,30 @@ static const struct phylink_mac_ops mtk_phylink_ops = {
.mac_enable_tx_lpi = mtk_mac_enable_tx_lpi,
};
+static void rt5350_mac_config(struct phylink_config *config, unsigned int mode,
+ const struct phylink_link_state *state)
+{
+}
+
+static void rt5350_mac_link_down(struct phylink_config *config, unsigned int mode,
+ phy_interface_t interface)
+{
+}
+
+static void rt5350_mac_link_up(struct phylink_config *config,
+ struct phy_device *phy,
+ unsigned int mode, phy_interface_t interface,
+ int speed, int duplex, bool tx_pause, bool rx_pause)
+{
+}
+
+/* MT76x8 (rt5350-eth) does not expose any MAC control registers */
+static const struct phylink_mac_ops rt5350_phylink_ops = {
+ .mac_config = rt5350_mac_config,
+ .mac_link_down = rt5350_mac_link_down,
+ .mac_link_up = rt5350_mac_link_up,
+};
+
static void mtk_mdio_config(struct mtk_eth *eth)
{
u32 val;
@@ -4780,6 +4802,7 @@ static const struct net_device_ops mtk_netdev_ops = {
static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
{
+ const struct phylink_mac_ops *mac_ops = &mtk_phylink_ops;
const __be32 *_id = of_get_property(np, "reg", NULL);
phy_interface_t phy_mode;
struct phylink *phylink;
@@ -4914,9 +4937,12 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
mac->phylink_config.supported_interfaces);
}
+ if (MTK_HAS_CAPS(eth->soc->caps, MTK_SOC_MT7628))
+ mac_ops = &rt5350_phylink_ops;
+
phylink = phylink_create(&mac->phylink_config,
of_fwnode_handle(mac->of_node),
- phy_mode, &mtk_phylink_ops);
+ phy_mode, mac_ops);
if (IS_ERR(phylink)) {
err = PTR_ERR(phylink);
goto free_netdev;
--
2.53.0
next prev parent reply other threads:[~2026-04-20 13:30 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20260420132314.1023554-1-sashal@kernel.org>
2026-04-20 13:17 ` [PATCH AUTOSEL 7.0-6.19] wifi: mt76: avoid to set ACK for MCU command if wait_resp is not set Sasha Levin
2026-04-20 13:17 ` [PATCH AUTOSEL 7.0-6.18] phy: phy-mtk-tphy: Update names and format of kernel-doc comments Sasha Levin
2026-04-20 13:18 ` [PATCH AUTOSEL 7.0-6.12] Bluetooth: btmtk: add MT7902 MCU support Sasha Levin
2026-04-20 13:18 ` [PATCH AUTOSEL 7.0-6.18] wifi: mt76: flush pending TX before channel switch Sasha Levin
2026-04-20 13:18 ` [PATCH AUTOSEL 7.0-6.6] wifi: mt76: fix list corruption in mt76_wcid_cleanup Sasha Levin
2026-04-20 13:18 ` [PATCH AUTOSEL 7.0-6.12] wifi: mt76: add missing lock protection in mt76_sta_state for sta_event callback Sasha Levin
2026-04-20 13:18 ` [PATCH AUTOSEL 7.0-6.1] Bluetooth: btmtk: improve mt79xx firmware setup retry flow Sasha Levin
2026-04-20 13:19 ` [PATCH AUTOSEL 7.0-6.18] wifi: mt76: mt7996: Disable Rx hdr_trans in monitor mode Sasha Levin
2026-04-20 13:19 ` [PATCH AUTOSEL 7.0-6.12] wifi: mt76: mt7925: Skip scan process during suspend Sasha Levin
2026-04-20 13:19 ` [PATCH AUTOSEL 7.0-5.10] wifi: mt76: mt76x02: wake queues after reconfig Sasha Levin
2026-04-20 13:19 ` [PATCH AUTOSEL 7.0-6.12] wifi: mt76: mt7925: resolve link after acquiring mt76 mutex Sasha Levin
2026-04-20 13:19 ` [PATCH AUTOSEL 7.0-6.18] wifi: mt76: mt7996: fix queue pause after scan due to wrong channel switch reason Sasha Levin
2026-04-20 13:20 ` Sasha Levin [this message]
2026-04-20 13:20 ` [PATCH AUTOSEL 7.0-6.6] wifi: mt76: mt7996: reset device after MCU message timeout Sasha Levin
2026-04-20 13:21 ` [PATCH AUTOSEL 7.0-5.10] drm/mediatek: mtk_dsi: enable hs clock during pre-enable Sasha Levin
2026-04-20 13:21 ` [PATCH AUTOSEL 7.0-6.18] wifi: mt76: mt7996: fix frequency separation for station STR mode Sasha Levin
2026-04-20 13:22 ` [PATCH AUTOSEL 6.18] net: airoha: Fix memory leak in airoha_qdma_rx_process() Sasha Levin
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=20260420132314.1023554-223-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=andrew+netdev@lunn.ch \
--cc=angelogioacchino.delregno@collabora.com \
--cc=daniel@makrotpia.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=joey@tinyisr.com \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=lorenzo@kernel.org \
--cc=matthias.bgg@gmail.com \
--cc=nbd@nbd.name \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=patches@lists.linux.dev \
--cc=stable@vger.kernel.org \
--cc=stefan.roese@mailbox.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