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

From: Vladimir Oltean <olteanv@gmail.com>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: netdev@vger.kernel.org, Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Michael Chan <mchan@broadcom.com>,
	"open list:BROADCOM ETHERNET PHY DRIVERS" 
	<bcm-kernel-feedback-list@broadcom.com>,
	open list <linux-kernel@vger.kernel.org>,
	michael@walle.cc
Subject: Re: [PATCH net-next v2 3/3] net: phy: broadcom: Allow BCM54210E to configure APD
Date: Sat, 13 Feb 2021 12:42:45 +0200	[thread overview]
Message-ID: <20210213104245.uti4qb2u2r5nblef@skbuf> (raw)
In-Reply-To: <20210213034632.2420998-4-f.fainelli@gmail.com>

On Fri, Feb 12, 2021 at 07:46:32PM -0800, Florian Fainelli wrote:
> 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>

>  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?

>  
>  /* 01010: Auto Power-Down */
>  #define BCM54XX_SHD_APD			0x0a
> @@ -253,7 +254,6 @@
>  #define BCM54810_EXP_BROADREACH_LRE_MISC_CTL_EN	(1 << 0)
>  #define BCM54810_SHD_CLK_CTL			0x3
>  #define BCM54810_SHD_CLK_CTL_GTXCLK_EN		(1 << 9)
> -#define BCM54810_SHD_SCR3_TRDDAPD		0x0100
>  
>  /* BCM54612E Registers */
>  #define BCM54612E_EXP_SPARE0		(MII_BCM54XX_EXP_SEL_ETC + 0x34)
> -- 
> 2.25.1
> 