From 5cc65c01cdc577621d1320f4aa03a1382c5a4a45 Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Tue, 4 Aug 2026 04:10:26 +0100 Subject: [PATCH 1/6] net: pcs: mtk-lynxi: check regmap reads in mtk_pcs_lynxi_get_state() mtk_pcs_lynxi_get_state() ignores regmap_read()'s return value; a failed read leaves bm and adv holding uninitialized stack values which are then decoded into the reported link state. The regmaps backing the MT7531 SGMII PCS instances sit on an MDIO bus where reads can fail. Check both reads and report the link as down on error; phylink presets state->link before the callback, so a bare return would leave a failed read reported as link-up. Signed-off-by: Daniel Golle Reviewed-by: Andrew Lunn Link: https://patch.msgid.link/fce70657fc03bbaf60a04c0fbf2f418531135c4f.1785811140.git.daniel@makrotopia.org Signed-off-by: Jakub Kicinski --- drivers/net/pcs/pcs-mtk-lynxi.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/net/pcs/pcs-mtk-lynxi.c b/drivers/net/pcs/pcs-mtk-lynxi.c index a753bd88cbc2..7290fc3e5d18 100644 --- a/drivers/net/pcs/pcs-mtk-lynxi.c +++ b/drivers/net/pcs/pcs-mtk-lynxi.c @@ -113,8 +113,11 @@ static void mtk_pcs_lynxi_get_state(struct phylink_pcs *pcs, unsigned int bm, adv; /* Read the BMSR and LPA */ - regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &bm); - regmap_read(mpcs->regmap, SGMSYS_PCS_ADVERTISE, &adv); + if (regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &bm) || + regmap_read(mpcs->regmap, SGMSYS_PCS_ADVERTISE, &adv)) { + state->link = false; + return; + } phylink_mii_c22_pcs_decode_state(state, neg_mode, FIELD_GET(SGMII_BMSR, bm), From 573d6e3afe3d01e34dc82c77bf22ca96a74d805e Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Tue, 4 Aug 2026 04:10:33 +0100 Subject: [PATCH 2/6] net: dsa: mt7530: check bus->read() error in core_rmw() core_rmw() accesses the MMD core registers directly rather than through the regmap and has the same unchecked bus->read() as the one just fixed in the MDIO regmap backend: a negative errno is consumed as register data, modified and written back to the switch. Check the read and bail out like the surrounding bus accesses do. Signed-off-by: Daniel Golle Reviewed-by: Andrew Lunn Link: https://patch.msgid.link/48bb9f0b311a9efeda2a6b24a7e05d4792393a3b.1785811140.git.daniel@makrotopia.org Signed-off-by: Jakub Kicinski --- drivers/net/dsa/mt7530.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index 55131bfd11f6..5ff203fba37e 100644 --- a/drivers/net/dsa/mt7530.c +++ b/drivers/net/dsa/mt7530.c @@ -124,8 +124,12 @@ core_rmw(struct mt7530_priv *priv, u32 reg, u32 mask, u32 set) goto err; /* Read the content of the MMD's selected register */ - val = bus->read(bus, MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr), + ret = bus->read(bus, MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr), MII_MMD_DATA); + if (ret < 0) + goto err; + val = ret; + val &= ~mask; val |= set; /* Write the data into MMD's selected register */ From 1c95dcb7e923beebccdc79adc3dc1ec8c52295d0 Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Tue, 4 Aug 2026 04:10:40 +0100 Subject: [PATCH 3/6] net: dsa: mt7530: error out on failed PHY_IAC command writes MT7531_PHY_ACS_ST is only ever set by the command write that precedes each poll in the MT7531 indirect PHY access functions, and that write's return value is discarded. A failed write leaves ACS_ST at 0 from the previous access, so the poll succeeds on its first iteration and the functions return stale IAC contents as if they were fresh PHY data. Check the writes and bail out before polling. Signed-off-by: Daniel Golle Reviewed-by: Andrew Lunn Link: https://patch.msgid.link/c34602e63a20ebbfb97babd145c82832d7a0b523.1785811140.git.daniel@makrotopia.org Signed-off-by: Jakub Kicinski --- drivers/net/dsa/mt7530.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index 5ff203fba37e..f60a5136799b 100644 --- a/drivers/net/dsa/mt7530.c +++ b/drivers/net/dsa/mt7530.c @@ -565,7 +565,9 @@ mt7531_ind_c45_phy_read(struct mt7530_priv *priv, int port, int devad, reg = MT7531_MDIO_CL45_ADDR | MT7531_MDIO_PHY_ADDR(port) | MT7531_MDIO_DEV_ADDR(devad) | regnum; - mt7530_mii_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST); + ret = mt7530_mii_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST); + if (ret < 0) + goto out; ret = regmap_read_poll_timeout(priv->regmap, MT7531_PHY_IAC, val, !(val & MT7531_PHY_ACS_ST), 20, 100000); @@ -576,7 +578,9 @@ mt7531_ind_c45_phy_read(struct mt7530_priv *priv, int port, int devad, reg = MT7531_MDIO_CL45_READ | MT7531_MDIO_PHY_ADDR(port) | MT7531_MDIO_DEV_ADDR(devad); - mt7530_mii_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST); + ret = mt7530_mii_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST); + if (ret < 0) + goto out; ret = regmap_read_poll_timeout(priv->regmap, MT7531_PHY_IAC, val, !(val & MT7531_PHY_ACS_ST), 20, 100000); @@ -610,7 +614,9 @@ mt7531_ind_c45_phy_write(struct mt7530_priv *priv, int port, int devad, reg = MT7531_MDIO_CL45_ADDR | MT7531_MDIO_PHY_ADDR(port) | MT7531_MDIO_DEV_ADDR(devad) | regnum; - mt7530_mii_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST); + ret = mt7530_mii_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST); + if (ret < 0) + goto out; ret = regmap_read_poll_timeout(priv->regmap, MT7531_PHY_IAC, val, !(val & MT7531_PHY_ACS_ST), 20, 100000); @@ -621,7 +627,9 @@ mt7531_ind_c45_phy_write(struct mt7530_priv *priv, int port, int devad, reg = MT7531_MDIO_CL45_WRITE | MT7531_MDIO_PHY_ADDR(port) | MT7531_MDIO_DEV_ADDR(devad) | data; - mt7530_mii_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST); + ret = mt7530_mii_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST); + if (ret < 0) + goto out; ret = regmap_read_poll_timeout(priv->regmap, MT7531_PHY_IAC, val, !(val & MT7531_PHY_ACS_ST), 20, 100000); @@ -654,7 +662,9 @@ mt7531_ind_c22_phy_read(struct mt7530_priv *priv, int port, int regnum) val = MT7531_MDIO_CL22_READ | MT7531_MDIO_PHY_ADDR(port) | MT7531_MDIO_REG_ADDR(regnum); - mt7530_mii_write(priv, MT7531_PHY_IAC, val | MT7531_PHY_ACS_ST); + ret = mt7530_mii_write(priv, MT7531_PHY_IAC, val | MT7531_PHY_ACS_ST); + if (ret < 0) + goto out; ret = regmap_read_poll_timeout(priv->regmap, MT7531_PHY_IAC, val, !(val & MT7531_PHY_ACS_ST), 20, 100000); @@ -689,7 +699,9 @@ mt7531_ind_c22_phy_write(struct mt7530_priv *priv, int port, int regnum, reg = MT7531_MDIO_CL22_WRITE | MT7531_MDIO_PHY_ADDR(port) | MT7531_MDIO_REG_ADDR(regnum) | data; - mt7530_mii_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST); + ret = mt7530_mii_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST); + if (ret < 0) + goto out; ret = regmap_read_poll_timeout(priv->regmap, MT7531_PHY_IAC, reg, !(reg & MT7531_PHY_ACS_ST), 20, 100000); From 1ae63b018d3d70e96d420b59a1334860cf232bc6 Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Tue, 4 Aug 2026 04:10:46 +0100 Subject: [PATCH 4/6] net: dsa: mt7530: check CORE_PLL_GROUP4 access in mt7531_setup() mt7531_setup() reads CORE_PLL_GROUP4 through the MT7531 indirect c45 PHY access, modifies it and writes it back to enable the PHY core PLL, but checks neither the read nor the write. Now that the indirect access functions propagate command-write failures, a failed read returns a negative errno that would be bit-modified and written back into the PLL register, and a failed write-back would go unnoticed. Check both and bail out. The adjacent EEE advertisement writes push a constant value and cannot corrupt state, so they are left as is. Signed-off-by: Daniel Golle Link: https://patch.msgid.link/a7dfe3b66ea6ac1ae7915034de0527060e6ddcd4.1785811140.git.daniel@makrotopia.org Signed-off-by: Jakub Kicinski --- drivers/net/dsa/mt7530.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index f60a5136799b..c60e22ea270e 100644 --- a/drivers/net/dsa/mt7530.c +++ b/drivers/net/dsa/mt7530.c @@ -2781,14 +2781,20 @@ mt7531_setup(struct dsa_switch *ds) * phy_[read,write]_mmd_indirect is called, we provide our own * mt7531_ind_mmd_phy_[read,write] to complete this function. */ - val = mt7531_ind_c45_phy_read(priv, + ret = mt7531_ind_c45_phy_read(priv, MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr), MDIO_MMD_VEND2, CORE_PLL_GROUP4); + if (ret < 0) + return ret; + + val = ret; val |= MT7531_RG_SYSPLL_DMY2 | MT7531_PHY_PLL_BYPASS_MODE; val &= ~MT7531_PHY_PLL_OFF; - mt7531_ind_c45_phy_write(priv, - MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr), - MDIO_MMD_VEND2, CORE_PLL_GROUP4, val); + ret = mt7531_ind_c45_phy_write(priv, + MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr), + MDIO_MMD_VEND2, CORE_PLL_GROUP4, val); + if (ret < 0) + return ret; /* Disable EEE advertisement on the switch PHYs. */ for (i = MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr); From f67b0bae07fe7eddf919ee1f03fc66d351547a83 Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Tue, 4 Aug 2026 04:10:53 +0100 Subject: [PATCH 5/6] net: dsa: mt7530: check command register writes in fdb and vlan cmd mt7530_fdb_cmd() and mt7530_vlan_cmd() start a command by writing the BUSY bit to MT7530_ATC / MT7530_VTCR, then poll for it to clear. mt7530_write() discards the write's return value, so a failed command write leaves BUSY unset and the poll succeeds on its first read, reporting a command that never ran as done -- returning stale FDB data or silently dropping a VLAN table update. Return mt7530_mii_write()'s error from mt7530_write() and check it in both command helpers. Signed-off-by: Daniel Golle Link: https://patch.msgid.link/0e5d65a672313286e5a8ce28a9faba9c8972dbb6.1785811140.git.daniel@makrotopia.org Signed-off-by: Jakub Kicinski --- drivers/net/dsa/mt7530.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index c60e22ea270e..d66d926a71e1 100644 --- a/drivers/net/dsa/mt7530.c +++ b/drivers/net/dsa/mt7530.c @@ -185,14 +185,18 @@ mt7530_mii_read(struct mt7530_priv *priv, u32 reg) return val; } -static void +static int mt7530_write(struct mt7530_priv *priv, u32 reg, u32 val) { + int ret; + mt7530_mutex_lock(priv); - mt7530_mii_write(priv, reg, val); + ret = mt7530_mii_write(priv, reg, val); mt7530_mutex_unlock(priv); + + return ret; } static u32 @@ -249,7 +253,9 @@ mt7530_fdb_cmd(struct mt7530_priv *priv, enum mt7530_fdb_cmd cmd, u32 *rsp) /* Set the command operating upon the MAC address entries */ val = ATC_BUSY | ATC_MAT(0) | cmd; - mt7530_write(priv, MT7530_ATC, val); + ret = mt7530_write(priv, MT7530_ATC, val); + if (ret) + return ret; mt7530_mutex_lock(priv); @@ -1632,7 +1638,9 @@ mt7530_vlan_cmd(struct mt7530_priv *priv, enum mt7530_vlan_cmd cmd, u16 vid) int ret; val = VTCR_BUSY | VTCR_FUNC(cmd) | vid; - mt7530_write(priv, MT7530_VTCR, val); + ret = mt7530_write(priv, MT7530_VTCR, val); + if (ret) + return ret; mt7530_mutex_lock(priv); From dd52b3df25ed25a7a881d45d9fbfaa0ac7dec5d5 Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Tue, 4 Aug 2026 04:11:01 +0100 Subject: [PATCH 6/6] net: dsa: mt7530: serialize the regmap IRQ chip like every other user The switch register regmap is created with .disable_locking = true; every other user in this driver calls mt7530_mutex_lock()/unlock() around it, which takes priv->bus->mdio_lock, since the underlying mt7530_regmap_read()/write() issue raw, unserialized bus->read()/ write() MDIO transactions. mt7530_setup_irq() hands this same unlocked regmap straight to devm_regmap_add_irq_chip_fwnode(), whose threaded IRQ handler then calls regmap_read()/regmap_update_bits() on it without ever calling mt7530_mutex_lock(). An interrupt firing while another thread is mid-transaction on the same regmap (e.g. a paged register access, or an indirect PHY access) can interleave with the IRQ handler's own paged access and corrupt page selection on either side. Use struct regmap_irq_chip's handle_mask_sync hook to call mt7530_mutex_lock()/unlock() around the mask register write regmap-irq issues whenever a consumer of one of the mapped sub-IRQs enables, disables, requests or frees its line. This needs a per-device copy of mt7530_regmap_irq_chip, since devm_regmap_add_irq_chip_fwnode() keeps a pointer to it rather than copying it. handle_pre_irq/handle_post_irq, which would additionally cover the status read and ack write the threaded handler does directly, bracket the whole handler including its handle_nested_irq() calls. Lockdep caught this on hardware: those calls reach phy_interrupt() for the per-port PHY IRQ lines mapped through this chip, which takes phydev->lock, while phy_attach_direct() and this driver's own indirect PHY access already establish the opposite order (phydev->lock, then priv->bus->mdio_lock) elsewhere. Using them here would close that cycle, so they are not used. regmap_irq_sync_unlock() also has its own init_ack_masked path, used by this chip, which unconditionally does its own regmap_write() to ack currently-masked IRQs; that path has no per-driver hook. Together with the threaded handler's own status read and ack write, these stay unprotected -- a narrower, harder-to-hit gap than the recurring mask sync above -- and will be closed once the switch regmap moves to regmap's own locking in the driver-wide register access cleanup. Signed-off-by: Daniel Golle Reviewed-by: Andrew Lunn Link: https://patch.msgid.link/818840879e9cd20f8d568789da29b3474c8f3ab9.1785811140.git.daniel@makrotopia.org Signed-off-by: Jakub Kicinski --- drivers/net/dsa/mt7530.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index d66d926a71e1..2b7be091c056 100644 --- a/drivers/net/dsa/mt7530.c +++ b/drivers/net/dsa/mt7530.c @@ -2319,6 +2319,21 @@ static const struct regmap_irq mt7530_irqs[] = { REGMAP_IRQ_REG_LINE(31, 32), /* ACL */ }; +/* Serialize regmap-irq's mask sync like every other regmap user */ +static int mt7530_irq_mask_sync(int index, unsigned int mask_buf_def, + unsigned int mask_buf, void *irq_drv_data) +{ + struct mt7530_priv *priv = irq_drv_data; + int ret; + + mt7530_mutex_lock(priv); + ret = regmap_update_bits(priv->regmap, MT7530_SYS_INT_EN, + mask_buf_def, ~mask_buf); + mt7530_mutex_unlock(priv); + + return ret; +} + static const struct regmap_irq_chip mt7530_regmap_irq_chip = { .name = KBUILD_MODNAME, .status_base = MT7530_SYS_INT_STS, @@ -2328,12 +2343,14 @@ static const struct regmap_irq_chip mt7530_regmap_irq_chip = { .irqs = mt7530_irqs, .num_irqs = ARRAY_SIZE(mt7530_irqs), .num_regs = 1, + .handle_mask_sync = mt7530_irq_mask_sync, }; static int mt7530_setup_irq(struct mt7530_priv *priv) { struct regmap_irq_chip_data *irq_data; + struct regmap_irq_chip *chip; struct device *dev = priv->dev; struct device_node *np = dev->of_node; int irq, ret; @@ -2353,10 +2370,17 @@ mt7530_setup_irq(struct mt7530_priv *priv) if (priv->id == ID_MT7530 || priv->id == ID_MT7621) mt7530_set(priv, MT7530_TOP_SIG_CTRL, TOP_SIG_CTRL_NORMAL); + chip = devm_kmemdup(dev, &mt7530_regmap_irq_chip, sizeof(*chip), + GFP_KERNEL); + if (!chip) + return -ENOMEM; + + chip->irq_drv_data = priv; + ret = devm_regmap_add_irq_chip_fwnode(dev, dev_fwnode(dev), priv->regmap, irq, IRQF_ONESHOT, - 0, &mt7530_regmap_irq_chip, + 0, chip, &irq_data); if (ret) return ret;