[PATCH] net: phy: bcm7xxx: preserve __phy_write() error in write_mmd()

[PATCH] net: phy: bcm7xxx: preserve __phy_write() error in write_mmd()

[Haotian Zhang]

bcm7xxx_28nm_ephy_write_mmd() jumps to reset_shadow_mode when the write of
the shadow register address fails and then returns the value of the
__phy_set_clr_bits() cleanup call, overwriting the negative error code from
__phy_write(). Since __phy_set_clr_bits() returns the positive register
value on success, the failure is reported to phy_write_mmd() as a success.
The write of the shadow register value is also unchecked the same way.

Keep the negative error code in ret and return it after the shadow mode
reset, and check the shadow register value write, matching the read_mmd()
counterpart.

Fixes: d88fd1b546ff ("net: phy: bcm7xxx: Fixed indirect MMD operations")
Assisted-by: DeepSeek-V4.1-Flash
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
---
 drivers/net/phy/bcm7xxx.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
index 71a163f62c0e..a0c758e2b214 100644
--- a/drivers/net/phy/bcm7xxx.c
+++ b/drivers/net/phy/bcm7xxx.c
@@ -677,12 +677,13 @@ static int bcm7xxx_28nm_ephy_write_mmd(struct phy_device *phydev,
 		goto reset_shadow_mode;
 
 	/* Write the desired value in the shadow register */
-	__phy_write(phydev, MII_BCM7XXX_SHD_2_CTRL_STAT, val);
+	ret = __phy_write(phydev, MII_BCM7XXX_SHD_2_CTRL_STAT, val);
 
 reset_shadow_mode:
 	/* reset shadow mode 2 */
-	return __phy_set_clr_bits(phydev, MII_BCM7XXX_TEST, 0,
-				  MII_BCM7XXX_SHD_MODE_2);
+	__phy_set_clr_bits(phydev, MII_BCM7XXX_TEST, 0,
+			   MII_BCM7XXX_SHD_MODE_2);
+	return ret;
 }
 
 static int bcm7xxx_28nm_ephy_resume(struct phy_device *phydev)
-- 
2.25.1

Re: [PATCH] net: phy: bcm7xxx: preserve __phy_write() error in write_mmd()

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH] net: phy: bcm7xxx: preserve __phy_write() error in write_mmd()

[Andrew Lunn]

On Fri, Oct 09, 2026 at 12:42:40AM +0800, Haotian Zhang wrote:
> bcm7xxx_28nm_ephy_write_mmd() jumps to reset_shadow_mode when the write of
> the shadow register address fails and then returns the value of the
> __phy_set_clr_bits() cleanup call, overwriting the negative error code from
> __phy_write(). Since __phy_set_clr_bits() returns the positive register
> value on success, the failure is reported to phy_write_mmd() as a success.
> The write of the shadow register value is also unchecked the same way.
> 
> Keep the negative error code in ret and return it after the shadow mode
> reset, and check the shadow register value write, matching the read_mmd()
> counterpart.

Probably not worth the churn. MDIO operation don't fail. It is a big
shift register which just clocks the bits out on the MDIO line. There
is no checksum, there is no checking if the device responded, nothing.

The only time i've seen an MDIO bus fail is because the driver for it
was fatally broken resulting in all operations failing. And we would
not of got this far if the bus driver was FUBAR.

Please work on fixing issues which bother people, not theoretical
issues which never happen.

    Andrew

---
pw-bot: cr