Re: [PATCH] net: phy: broadcom: add support for BCM5221 phy
flat view
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
Date: 2023-08-11 22:23:16
Also in:
lkml
On Fri, Aug 11, 2023 at 11:53:22PM +0200, Giulio Benetti wrote:
quoted hunk ↗ jump to hunk
+ reg = phy_read(phydev, MII_BRCM_FET_INTREG); + if (reg < 0) + return reg; + + /* Unmask events we are interested in and mask interrupts globally. */ + reg = MII_BRCM_FET_IR_ENABLE | + MII_BRCM_FET_IR_MASK; + + err = phy_write(phydev, MII_BRCM_FET_INTREG, reg); + if (err < 0) + return err;
Please explain why you read MII_BRCM_FET_INTREG, then discard its value and write a replacement value.
quoted hunk ↗ jump to hunk
+ + /* Enable auto MDIX */ + err = phy_clear_bits(phydev, BCM5221_AEGSR, BCM5221_AEGSR_MDIX_DIS); + if (err < 0) + return err; + + /* Enable shadow register access */ + brcmtest = phy_read(phydev, MII_BRCM_FET_BRCMTEST); + if (brcmtest < 0) + return brcmtest; + + reg = brcmtest | MII_BRCM_FET_BT_SRE; + + err = phy_write(phydev, MII_BRCM_FET_BRCMTEST, reg); + if (err < 0) + return err;
I think you should consider locking the MDIO bus while the device is switched to the shadow register set, so that other accesses don't happen that may interfere with this.
quoted hunk ↗ jump to hunk
+static int bcm5221_suspend(struct phy_device *phydev) +{ + int reg, err, err2, brcmtest; + + /* Enable shadow register access */ + brcmtest = phy_read(phydev, MII_BRCM_FET_BRCMTEST); + if (brcmtest < 0) + return brcmtest; + + reg = brcmtest | MII_BRCM_FET_BT_SRE; + + err = phy_write(phydev, MII_BRCM_FET_BRCMTEST, reg); + if (err < 0) + return err; + + /* Force Low Power Mode with clock enabled */ + err = phy_set_bits(phydev, MII_BRCM_FET_SHDW_AUXMODE4, + BCM5221_SHDW_AM4_EN_CLK_LPM | + BCM5221_SHDW_AM4_FORCE_LPM); + + /* Disable shadow register access */ + err2 = phy_write(phydev, MII_BRCM_FET_BRCMTEST, brcmtest); + if (!err) + err = err2;
Same here.
quoted hunk ↗ jump to hunk
+ + return err; +} + +static int bcm5221_resume(struct phy_device *phydev) +{ + int reg, err, err2, brcmtest; + + /* Enable shadow register access */ + brcmtest = phy_read(phydev, MII_BRCM_FET_BRCMTEST); + if (brcmtest < 0) + return brcmtest; + + reg = brcmtest | MII_BRCM_FET_BT_SRE; + + err = phy_write(phydev, MII_BRCM_FET_BRCMTEST, reg); + if (err < 0) + return err; + + /* Exit Low Power Mode with clock enabled */ + err = phy_clear_bits(phydev, MII_BRCM_FET_SHDW_AUXMODE4, + BCM5221_SHDW_AM4_FORCE_LPM); + + /* Disable shadow register access */ + err2 = phy_write(phydev, MII_BRCM_FET_BRCMTEST, brcmtest); + if (!err) + err = err2;
And, of course, same here. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!