Re: [PATCH net-next v2 3/3] net: phy: broadcom: Allow BCM54210E to configure APD

On 2/14/2021 8:29 PM, Florian Fainelli wrote:

On 2/13/2021 2:42 AM, Vladimir Oltean wrote:

quoted

On Fri, Feb 12, 2021 at 07:46:32PM -0800, Florian Fainelli wrote:

quoted

BCM54210E/BCM50212E has been verified to work correctly with the
auto-power down configuration done by bcm54xx_adjust_rxrefclk(), add it
to the list of PHYs working.

While we are at it, provide an appropriate name for the bit we are
changing which disables the RXC and TXC during auto-power down when
there is no energy on the cable.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

quoted

 drivers/net/phy/broadcom.c | 8 +++++---
 include/linux/brcmphy.h    | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 3ce266ab521b..91fbd26c809e 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c

@@ -193,6 +193,7 @@ static void bcm54xx_adjust_rxrefclk(struct phy_device *phydev)
 	if (BRCM_PHY_MODEL(phydev) != PHY_ID_BCM57780 &&
 	    BRCM_PHY_MODEL(phydev) != PHY_ID_BCM50610 &&
 	    BRCM_PHY_MODEL(phydev) != PHY_ID_BCM50610M &&
+	    BRCM_PHY_MODEL(phydev) != PHY_ID_BCM54210E &&
 	    BRCM_PHY_MODEL(phydev) != PHY_ID_BCM54810 &&
 	    BRCM_PHY_MODEL(phydev) != PHY_ID_BCM54811)
 		return;

@@ -227,9 +228,10 @@ static void bcm54xx_adjust_rxrefclk(struct phy_device *phydev)
 		val |= BCM54XX_SHD_SCR3_DLLAPD_DIS;
 
 	if (phydev->dev_flags & PHY_BRCM_DIS_TXCRXC_NOENRGY) {
-		if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810 ||
-		    BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54811)
-			val |= BCM54810_SHD_SCR3_TRDDAPD;
+		if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54210E ||
+		    BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810 ||
+		    BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54210E)
+			val |= BCM54XX_SHD_SCR3_RXCTXC_DIS;
 		else
 			val |= BCM54XX_SHD_SCR3_TRDDAPD;
 	}

diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index 844dcfe789a2..16597d3fa011 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h

@@ -193,6 +193,7 @@
 #define  BCM54XX_SHD_SCR3_DEF_CLK125	0x0001
 #define  BCM54XX_SHD_SCR3_DLLAPD_DIS	0x0002
 #define  BCM54XX_SHD_SCR3_TRDDAPD	0x0004
+#define  BCM54XX_SHD_SCR3_RXCTXC_DIS	0x0100

Curiously enough, my BCM5464R datasheet does say:

The TXC and RXC outputs can be disabled during auto-power down by setting the “1000BASE-T/100BASE-TX/10BASE-T
Spare Control 3 Register (Address 1Ch, Shadow Value 00101),” bit 8 =1.

but when I go to the definition of the register, bit 8 is hidden. Odd.

How can I ensure that the auto power down feature is doing something?

I am trying to confirm what the expected power levels should be from the
54210E product engineer so I can give you an estimate of what you should
see with and without while measure the PHY's regulators.

Took a while but for the 54210E reference board here are the numbers,
your mileage will vary depending on the supplies, regulator efficiency
and PCB design around the PHY obviously:

BMCR.PDOWN:			86.12 mW
auto-power down:		77.84 mW
auto-power-down, DLL disabled:  30.83 mW
IDDQ-low power:			 9.85 mW (requires a RESETn toggle)
IDDQ with soft recovery:	10.75 mW

Interestingly, the 50212E that I am using requires writing the PDOWN bit
and only that bit (not a RMW) in order to get in a correct state, both
LEDs keep flashing when that happens, fixes coming.

When net-next opens back up I will submit patches to support IDDQ with
soft recovery since that is clearly much better than the standard power
down and it does not require a RESETn toggle.
-- 
Florian