From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from pidgin.makrotopia.org (pidgin.makrotopia.org [185.142.180.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DE94224886A; Thu, 30 Jul 2026 00:15:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.142.180.65 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785370510; cv=none; b=Gg9EafrU0iyst/lWN4XJsjgRHN3JHEU+2u0XUKdCdJk/6TczsbVA1yo/iG1Q+jXVPps5h5goIBvOrcm3uEzy5SWSNO0FVUcYht5itdhqKq33B6oRYo771qsMdmrJfo6U31qr3y0YzsHL+CQ7dBIdM979SCLl+LDSNgE0AXmdqIQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785370510; c=relaxed/simple; bh=sFZa7FIXg7FFK5W3HxfxCnGUjpjpw8cMI14WD+ZRbHM=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=QS2qHQzZ/0ANaH1CBq8j3YoawvIuR5EnT5mDtmqrO/ttsitJzCn9ctLYg5Y+D3e3gadqcMQim8BDLNbXZG/AIHq7rYlOYkxsLJ14vAb79xzENxO+XvPOjHcDuizkFmalYo7EoTXtK05cOFg8ti3bNog6pZgb3VDmIDOuBajJlew= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=makrotopia.org; spf=pass smtp.mailfrom=makrotopia.org; arc=none smtp.client-ip=185.142.180.65 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=makrotopia.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=makrotopia.org Received: from local by pidgin.makrotopia.org with esmtpsa (TLS1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.99) (envelope-from ) id 1wpEQQ-000000002gI-1Bf0; Thu, 30 Jul 2026 00:15:02 +0000 Date: Thu, 30 Jul 2026 01:14:59 +0100 From: Daniel Golle To: "Chester A. Unal" , Daniel Golle , Andrew Lunn , Vladimir Oltean , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Matthias Brugger , AngeloGioacchino Del Regno , Alexander Couzens , Heiner Kallweit , Russell King , Russell King , Sean Wang , Florian Fainelli , Landen Chao , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org Subject: [PATCH net v2 4/5] net: dsa: mt7530: error out on failed reads in MT7531 PHY polling Message-ID: References: Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: The MT7531 indirect PHY access functions poll MT7531_PHY_IAC through a helper which returns 0 when the underlying read fails, so a failed bus transaction clears MT7531_PHY_ACS_ST and the access carries on, returning garbage PHY register data to phylib. Poll using regmap_read_poll_timeout(), which stops on read errors and propagates them. These functions hold the MDIO bus lock across the whole sequence, so the unlocked regmap accesses remain correct. Remove the now-unused _mt7530_unlocked_read(). MT7531_PHY_ACS_ST is only ever set by the command write that precedes each poll, and that write's return value was discarded. A failed write leaves ACS_ST at 0 from the previous access, so the hardened poll above succeeds on its first iteration and the functions return stale IAC contents as if they were fresh PHY data, exactly the failure this patch otherwise closes. Check the write too and bail out before polling. Fixes: c288575f7810 ("net: dsa: mt7530: Add the support of MT7531 switch") Signed-off-by: Daniel Golle Reviewed-by: Andrew Lunn --- v2: * also check mt7530_mii_write()'s return for the PHY_IAC command word in all four helpers; an unchecked failed write left ACS_ST clear and made the poll succeed instantly on stale data (found by Sashiko AI review) * print ret in the poll timeout messages instead of a fixed string, since ret can now be a propagated bus error rather than -ETIMEDOUT drivers/net/dsa/mt7530.c | 110 ++++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 60 deletions(-) diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index aab26970f39b..29b8b28e8d2c 100644 --- a/drivers/net/dsa/mt7530.c +++ b/drivers/net/dsa/mt7530.c @@ -195,12 +195,6 @@ mt7530_write(struct mt7530_priv *priv, u32 reg, u32 val) mt7530_mutex_unlock(priv); } -static u32 -_mt7530_unlocked_read(struct mt7530_dummy_poll *p) -{ - return mt7530_mii_read(p->priv, p->reg); -} - static u32 _mt7530_read(struct mt7530_dummy_poll *p) { @@ -555,40 +549,41 @@ static int mt7531_ind_c45_phy_read(struct mt7530_priv *priv, int port, int devad, int regnum) { - struct mt7530_dummy_poll p; u32 reg, val; int ret; - INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC); - mt7530_mutex_lock(priv); - ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val, - !(val & MT7531_PHY_ACS_ST), 20, 100000); + ret = regmap_read_poll_timeout(priv->regmap, MT7531_PHY_IAC, val, + !(val & MT7531_PHY_ACS_ST), 20, 100000); if (ret < 0) { - dev_err(priv->dev, "poll timeout\n"); + dev_err(priv->dev, "PHY_IAC poll failed: %d\n", ret); goto out; } 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 = readx_poll_timeout(_mt7530_unlocked_read, &p, val, - !(val & MT7531_PHY_ACS_ST), 20, 100000); + ret = regmap_read_poll_timeout(priv->regmap, MT7531_PHY_IAC, val, + !(val & MT7531_PHY_ACS_ST), 20, 100000); if (ret < 0) { - dev_err(priv->dev, "poll timeout\n"); + dev_err(priv->dev, "PHY_IAC poll failed: %d\n", ret); goto out; } 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 = readx_poll_timeout(_mt7530_unlocked_read, &p, val, - !(val & MT7531_PHY_ACS_ST), 20, 100000); + ret = regmap_read_poll_timeout(priv->regmap, MT7531_PHY_IAC, val, + !(val & MT7531_PHY_ACS_ST), 20, 100000); if (ret < 0) { - dev_err(priv->dev, "poll timeout\n"); + dev_err(priv->dev, "PHY_IAC poll failed: %d\n", ret); goto out; } @@ -603,42 +598,41 @@ static int mt7531_ind_c45_phy_write(struct mt7530_priv *priv, int port, int devad, int regnum, u16 data) { - struct mt7530_dummy_poll p; u32 val, reg; int ret; - INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC); - mt7530_mutex_lock(priv); - ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val, - !(val & MT7531_PHY_ACS_ST), 20, 100000); + ret = regmap_read_poll_timeout(priv->regmap, MT7531_PHY_IAC, val, + !(val & MT7531_PHY_ACS_ST), 20, 100000); if (ret < 0) { - dev_err(priv->dev, "poll timeout\n"); + dev_err(priv->dev, "PHY_IAC poll failed: %d\n", ret); goto out; } 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 = readx_poll_timeout(_mt7530_unlocked_read, &p, val, - !(val & MT7531_PHY_ACS_ST), 20, 100000); + ret = regmap_read_poll_timeout(priv->regmap, MT7531_PHY_IAC, val, + !(val & MT7531_PHY_ACS_ST), 20, 100000); if (ret < 0) { - dev_err(priv->dev, "poll timeout\n"); + dev_err(priv->dev, "PHY_IAC poll failed: %d\n", ret); goto out; } 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 = readx_poll_timeout(_mt7530_unlocked_read, &p, val, - !(val & MT7531_PHY_ACS_ST), 20, 100000); - if (ret < 0) { - dev_err(priv->dev, "poll timeout\n"); + 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); + if (ret < 0) + dev_err(priv->dev, "PHY_IAC poll failed: %d\n", ret); out: mt7530_mutex_unlock(priv); @@ -649,30 +643,29 @@ mt7531_ind_c45_phy_write(struct mt7530_priv *priv, int port, int devad, static int mt7531_ind_c22_phy_read(struct mt7530_priv *priv, int port, int regnum) { - struct mt7530_dummy_poll p; int ret; u32 val; - INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC); - mt7530_mutex_lock(priv); - ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val, - !(val & MT7531_PHY_ACS_ST), 20, 100000); + ret = regmap_read_poll_timeout(priv->regmap, MT7531_PHY_IAC, val, + !(val & MT7531_PHY_ACS_ST), 20, 100000); if (ret < 0) { - dev_err(priv->dev, "poll timeout\n"); + dev_err(priv->dev, "PHY_IAC poll failed: %d\n", ret); goto out; } 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 = readx_poll_timeout(_mt7530_unlocked_read, &p, val, - !(val & MT7531_PHY_ACS_ST), 20, 100000); + ret = regmap_read_poll_timeout(priv->regmap, MT7531_PHY_IAC, val, + !(val & MT7531_PHY_ACS_ST), 20, 100000); if (ret < 0) { - dev_err(priv->dev, "poll timeout\n"); + dev_err(priv->dev, "PHY_IAC poll failed: %d\n", ret); goto out; } @@ -687,32 +680,29 @@ static int mt7531_ind_c22_phy_write(struct mt7530_priv *priv, int port, int regnum, u16 data) { - struct mt7530_dummy_poll p; int ret; u32 reg; - INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC); - mt7530_mutex_lock(priv); - ret = readx_poll_timeout(_mt7530_unlocked_read, &p, reg, - !(reg & MT7531_PHY_ACS_ST), 20, 100000); + ret = regmap_read_poll_timeout(priv->regmap, MT7531_PHY_IAC, reg, + !(reg & MT7531_PHY_ACS_ST), 20, 100000); if (ret < 0) { - dev_err(priv->dev, "poll timeout\n"); + dev_err(priv->dev, "PHY_IAC poll failed: %d\n", ret); goto out; } 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 = readx_poll_timeout(_mt7530_unlocked_read, &p, reg, - !(reg & MT7531_PHY_ACS_ST), 20, 100000); - if (ret < 0) { - dev_err(priv->dev, "poll timeout\n"); + 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); + if (ret < 0) + dev_err(priv->dev, "PHY_IAC poll failed: %d\n", ret); out: mt7530_mutex_unlock(priv); -- 2.55.0