Messages in this thread | | | Date | Mon, 15 Aug 2022 11:32:09 -0700 | Subject | Re: [PATCH net-next] net: phy: broadcom: Implement suspend/resume for AC131 and BCM5241 | From | Florian Fainelli <> |
| |
On 8/15/22 11:09, Florian Fainelli wrote: > On 8/15/22 11:00, Russell King (Oracle) wrote: >> On Mon, Aug 15, 2022 at 10:43:56AM -0700, Florian Fainelli wrote: >>> + /* We cannot use a read/modify/write here otherwise the PHY >>> continues >>> + * to drive LEDs which defeats the purpose of low power mode. >>> + */ >> ... >>> + /* Set standby mode */ >>> + reg = phy_read(phydev, MII_BRCM_FET_SHDW_AUXMODE4); >>> + if (reg < 0) { >>> + err = reg; >>> + goto done; >>> + } >>> + >>> + reg |= MII_BRCM_FET_SHDW_AM4_STANDBY; >>> + >>> + err = phy_write(phydev, MII_BRCM_FET_SHDW_AUXMODE4, reg); >> >> Does the read-modify-write problem extend to this register? Why would >> the PHY behave differently whether you used phy_modify() here or not? >> On the mdio bus, it should be exactly the same - the only difference >> is that we're guaranteed to hold the lock over the sequence whereas >> this drops and re-acquires the lock. > > What read-modify-write problem are you referring to, that is, are you > talking about my statement about setting BMCR.PDOWN only or something else?
Sorry, hit send too quickly, I see what problem you are referring to. v2 coming up shortly utilizing phy_modify() and simplifying the return path (no need for done label, etc.)
Thanks -- Florian
| |