[PATCH net-next 0/3] net: phy: Better support for BCM54810

[PATCH net-next 0/3] net: phy: Better support for BCM54810

[Florian Fainelli]

Hi David, Andrew, Heiner,

This patch series updates the broadcom PHY driver to better support the
BCM54810 and allow it to make use of the exiting
bcm54xx_adjust_rxrefclk() as well as fix suspend/resume for it.

Florian Fainelli (3):
  net: phy: broadcom: Allow BCM54810 to use bcm54xx_adjust_rxrefclk()
  net: phy: broadcom: Have bcm54xx_adjust_rxrefclk() check for flags
  net: phy: broadcom: Wire suspend/resume for BCM54810

 drivers/net/phy/broadcom.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

-- 
2.17.1

[PATCH net-next 1/3] net: phy: broadcom: Allow BCM54810 to use bcm54xx_adjust_rxrefclk()

[Florian Fainelli]

The function bcm54xx_adjust_rxrefclk() works correctly on the BCM54810
PHY, allow this device ID to proceed through.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/broadcom.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 7d68b28bb893..4ad2128cc454 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -194,7 +194,8 @@ static void bcm54xx_adjust_rxrefclk(struct phy_device *phydev)
 	/* Abort if we are using an untested phy. */
 	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_BCM50610M &&
+	    BRCM_PHY_MODEL(phydev) != PHY_ID_BCM54810)
 		return;
 
 	val = bcm_phy_read_shadow(phydev, BCM54XX_SHD_SCR3);
-- 
2.17.1

[PATCH net-next 2/3] net: phy: broadcom: Have bcm54xx_adjust_rxrefclk() check for flags

[Florian Fainelli]

bcm54xx_adjust_rxrefclk() already checks for the flags and will
correctly reacting to the 3 different flags it check, allow it to be
unconditionally called.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/broadcom.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 4ad2128cc454..b4eae84a9195 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -273,10 +273,7 @@ static int bcm54xx_config_init(struct phy_device *phydev)
 	    (phydev->dev_flags & PHY_BRCM_CLEAR_RGMII_MODE))
 		bcm_phy_write_shadow(phydev, BCM54XX_SHD_RGMII_MODE, 0);
 
-	if ((phydev->dev_flags & PHY_BRCM_RX_REFCLK_UNUSED) ||
-	    (phydev->dev_flags & PHY_BRCM_DIS_TXCRXC_NOENRGY) ||
-	    (phydev->dev_flags & PHY_BRCM_AUTO_PWRDWN_ENABLE))
-		bcm54xx_adjust_rxrefclk(phydev);
+	bcm54xx_adjust_rxrefclk(phydev);
 
 	if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54210E) {
 		err = bcm54210e_config_init(phydev);
-- 
2.17.1

[PATCH net-next 3/3] net: phy: broadcom: Wire suspend/resume for BCM54810

[Florian Fainelli]

The BCM54810 PHY can use the standard BMCR Power down suspend, but needs
a custom resume routine which first clear the Power down bit, and then
re-initializes the PHY. While in low-power mode, the PHY only accepts
writes to the BMCR register. The datasheet clearly says it:

Reads or writes to any MII register other than MII Control register
(address 00h) while the device is in the standby power-down mode may
cause unpredictable results.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/broadcom.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index b4eae84a9195..ab24692a92c6 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -313,6 +313,20 @@ static int bcm54xx_config_init(struct phy_device *phydev)
 	return 0;
 }
 
+static int bcm54xx_resume(struct phy_device *phydev)
+{
+	int ret;
+
+	/* Writes to register other than BMCR would be ignored
+	 * unless we clear the PDOWN bit first
+	 */
+	ret = genphy_resume(phydev);
+	if (ret < 0)
+		return ret;
+
+	return bcm54xx_config_init(phydev);
+}
+
 static int bcm5482_config_init(struct phy_device *phydev)
 {
 	int err, reg;
@@ -706,6 +720,8 @@ static struct phy_driver broadcom_drivers[] = {
 	.config_aneg    = bcm5481_config_aneg,
 	.ack_interrupt  = bcm_phy_ack_intr,
 	.config_intr    = bcm_phy_config_intr,
+	.suspend	= genphy_suspend,
+	.resume		= bcm54xx_resume,
 }, {
 	.phy_id		= PHY_ID_BCM5482,
 	.phy_id_mask	= 0xfffffff0,
-- 
2.17.1

Re: [PATCH net-next 2/3] net: phy: broadcom: Have bcm54xx_adjust_rxrefclk() check for flags

[Andrew Lunn]

On Fri, Feb 14, 2020 at 03:38:52PM -0800, Florian Fainelli wrote:
> bcm54xx_adjust_rxrefclk() already checks for the flags and will
> correctly reacting to the 3 different flags it check, allow it to be
> unconditionally called.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/net/phy/broadcom.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
> index 4ad2128cc454..b4eae84a9195 100644
> --- a/drivers/net/phy/broadcom.c
> +++ b/drivers/net/phy/broadcom.c
> @@ -273,10 +273,7 @@ static int bcm54xx_config_init(struct phy_device *phydev)
>  	    (phydev->dev_flags & PHY_BRCM_CLEAR_RGMII_MODE))
>  		bcm_phy_write_shadow(phydev, BCM54XX_SHD_RGMII_MODE, 0);
>  
> -	if ((phydev->dev_flags & PHY_BRCM_RX_REFCLK_UNUSED) ||
> -	    (phydev->dev_flags & PHY_BRCM_DIS_TXCRXC_NOENRGY) ||
> -	    (phydev->dev_flags & PHY_BRCM_AUTO_PWRDWN_ENABLE))
> -		bcm54xx_adjust_rxrefclk(phydev);
> +	bcm54xx_adjust_rxrefclk(phydev);

Hi Florian

PHY_BRCM_RX_REFCLK_UNUSED is not unconditionally checked in
bcm54xx_adjust_rxrefclk(), where as here it is. I assume this is O.K?
The same is tree for PHY_BRCM_AUTO_PWRDWN_ENABLE.  Maybe worth a
comment in the commit message if you need to respin.

      Andrew

Re: [PATCH net-next 3/3] net: phy: broadcom: Wire suspend/resume for BCM54810

[Andrew Lunn]

On Fri, Feb 14, 2020 at 03:38:53PM -0800, Florian Fainelli wrote:
> The BCM54810 PHY can use the standard BMCR Power down suspend, but needs
> a custom resume routine which first clear the Power down bit, and then
> re-initializes the PHY. While in low-power mode, the PHY only accepts
> writes to the BMCR register. The datasheet clearly says it:
> 
> Reads or writes to any MII register other than MII Control register
> (address 00h) while the device is in the standby power-down mode may
> cause unpredictable results.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 1/3] net: phy: broadcom: Allow BCM54810 to use bcm54xx_adjust_rxrefclk()

[Andrew Lunn]

On Fri, Feb 14, 2020 at 03:38:51PM -0800, Florian Fainelli wrote:
> The function bcm54xx_adjust_rxrefclk() works correctly on the BCM54810
> PHY, allow this device ID to proceed through.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 0/3] net: phy: Better support for BCM54810

[David Miller]

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Fri, 14 Feb 2020 15:38:50 -0800

> Hi David, Andrew, Heiner,
> 
> This patch series updates the broadcom PHY driver to better support the
> BCM54810 and allow it to make use of the exiting
> bcm54xx_adjust_rxrefclk() as well as fix suspend/resume for it.

Florian, please respond to Andrew's feedback on patch #2 wrt. the checking
of PHY_BRCM_RX_REFCLK_UNUSED.

Thank you.

Re: [PATCH net-next 2/3] net: phy: broadcom: Have bcm54xx_adjust_rxrefclk() check for flags

[Florian Fainelli]

On 2/15/20 8:08 AM, Andrew Lunn wrote:
> On Fri, Feb 14, 2020 at 03:38:52PM -0800, Florian Fainelli wrote:
>> bcm54xx_adjust_rxrefclk() already checks for the flags and will
>> correctly reacting to the 3 different flags it check, allow it to be
>> unconditionally called.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  drivers/net/phy/broadcom.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
>> index 4ad2128cc454..b4eae84a9195 100644
>> --- a/drivers/net/phy/broadcom.c
>> +++ b/drivers/net/phy/broadcom.c
>> @@ -273,10 +273,7 @@ static int bcm54xx_config_init(struct phy_device *phydev)
>>  	    (phydev->dev_flags & PHY_BRCM_CLEAR_RGMII_MODE))
>>  		bcm_phy_write_shadow(phydev, BCM54XX_SHD_RGMII_MODE, 0);
>>  
>> -	if ((phydev->dev_flags & PHY_BRCM_RX_REFCLK_UNUSED) ||
>> -	    (phydev->dev_flags & PHY_BRCM_DIS_TXCRXC_NOENRGY) ||
>> -	    (phydev->dev_flags & PHY_BRCM_AUTO_PWRDWN_ENABLE))
>> -		bcm54xx_adjust_rxrefclk(phydev);
>> +	bcm54xx_adjust_rxrefclk(phydev);
> 
> Hi Florian
> 
> PHY_BRCM_RX_REFCLK_UNUSED is not unconditionally checked in
> bcm54xx_adjust_rxrefclk(), where as here it is. I assume this is O.K?
> The same is tree for PHY_BRCM_AUTO_PWRDWN_ENABLE.  Maybe worth a
> comment in the commit message if you need to respin.

Indeed, let me correct that in the commit message.
-- 
Florian