netdev - [PATCH net 2/3] net: phy: broadcom: Only set BMCR.PDOWN to suspend

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20210310204106.2767772-3-f.fainelli@gmail.com>
Date:   Wed, 10 Mar 2021 12:41:05 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     netdev@...r.kernel.org
Cc:     Florian Fainelli <f.fainelli@...il.com>, andrew@...n.ch,
        hkallweit1@...il.com, kuba@...nel.org, davem@...emloft.net
Subject: [PATCH net 2/3] net: phy: broadcom: Only set BMCR.PDOWN to suspend

B50212E PHYs have been observed to get into an incorrect state with the
visible effect of having both activity and link LEDs flashing
alternatively instead of being turned off as intended when
genphy_suspend() was issued. The BCM54810 is a similar design and
equally suffers from that issue.

The datasheet is not particularly clear whether a read/modify/write
sequence is acceptable and only indicates that BMCR.PDOWN=1 should be
utilized to enter the power down mode. When this was done the PHYs were
always measured to have power levels that match the expectations and
LEDs powered off.

Fixes: fe26821fa614 ("net: phy: broadcom: Wire suspend/resume for BCM54810")
Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
---
 drivers/net/phy/broadcom.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index b8eb736fb456..b33ffd44f799 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -388,6 +388,21 @@ static int bcm54xx_config_init(struct phy_device *phydev)
 	return 0;
 }
 
+static int bcm54xx_suspend(struct phy_device *phydev)
+{
+	/* We cannot perform a read/modify/write like what genphy_suspend()
+	 * does because depending on the time we can observe the PHY having
+	 * both of its LEDs flashing indicating that it is in an incorrect
+	 * state and not powered down as expected.
+	 *
+	 * There is not a clear indication in the datasheet whether a
+	 * read/modify/write would be acceptable, but a blind write to the
+	 * register has been proven to be functional unlike the
+	 * Read/Modify/Write.
+	 */
+	return phy_write(phydev, MII_BMCR, BMCR_PDOWN);
+}
+
 static int bcm54xx_resume(struct phy_device *phydev)
 {
 	int ret;
@@ -778,7 +793,7 @@ static struct phy_driver broadcom_drivers[] = {
 	.config_aneg    = bcm5481_config_aneg,
 	.config_intr    = bcm_phy_config_intr,
 	.handle_interrupt = bcm_phy_handle_interrupt,
-	.suspend	= genphy_suspend,
+	.suspend	= bcm54xx_suspend,
 	.resume		= bcm54xx_resume,
 }, {
 	.phy_id         = PHY_ID_BCM54811,
-- 
2.25.1