Re: [PATCH 3/3] net: phy: dp83867: sync dp83867_phy_rest

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 23.05.24 16:40, Stefan Kerkmann wrote:
> This is a port of the `dp83867_phy_reset` function at the state of linux
> commit a129b41fe0a8b4da828c46b10f5244ca07a3fec3 ("Revert "net: phy:
> dp83867: perform soft reset and retain established link"). Which
> performs a reset before starting the initial configuration.
> 
> It is a refactoring of commit 095cd32961aab64cfe72941ce43d99876852e12b
> ("net: phy: dp83867: reset PHY on probe") which already ported parts of
> the function, but diverted from the upstream implementation.

Even if it doesn't matter for your original issue, the DP83867_DSP_FFE_CFG
configuration you import looks like a good thing to have.

> Signed-off-by: Stefan Kerkmann <s.kerkmann@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  drivers/net/phy/dp83867.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
> index aefc651489..3c9a8e6355 100644
> --- a/drivers/net/phy/dp83867.c
> +++ b/drivers/net/phy/dp83867.c
> @@ -362,8 +362,6 @@ static int dp83867_of_init(struct phy_device *phydev)
>  	return 0;
>  }
>  
> -static int dp83867_phy_reset(struct phy_device *phydev); /* see below */
> -
>  static int dp83867_probe(struct phy_device *phydev)
>  {
>  	struct dp83867_private *dp83867;
> @@ -372,8 +370,6 @@ static int dp83867_probe(struct phy_device *phydev)
>  
>  	phydev->priv = dp83867;
>  
> -	dp83867_phy_reset(phydev);
> -
>  	return dp83867_of_init(phydev);
>  }
>  
> @@ -571,14 +567,33 @@ static int dp83867_phy_reset(struct phy_device *phydev)
>  {
>  	int err;
>  
> +	err = phy_write(phydev, DP83867_CTRL, DP83867_SW_RESET);
> +	if (err < 0)
> +		return err;
> +
> +	udelay(20);
> +
> +	err = phy_modify(phydev, MII_DP83867_PHYCTRL,
> +			 DP83867_PHYCR_FORCE_LINK_GOOD, 0);
> +	if (err < 0)
> +		return err;
> +
> +	/* Configure the DSP Feedforward Equalizer Configuration register to
> +	 * improve short cable (< 1 meter) performance. This will not affect
> +	 * long cable performance.
> +	 */
> +	err = phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_DSP_FFE_CFG,
> +			    0x0e81);
> +	if (err < 0)
> +		return err;
> +
>  	err = phy_write(phydev, DP83867_CTRL, DP83867_SW_RESTART);
>  	if (err < 0)
>  		return err;
>  
>  	udelay(20);
>  
> -	return phy_modify(phydev, MII_DP83867_PHYCTRL,
> -			 DP83867_PHYCR_FORCE_LINK_GOOD, 0);
> +	return 0;
>  }
>  
>  static struct phy_driver dp83867_driver[] = {
> @@ -590,6 +605,7 @@ static struct phy_driver dp83867_driver[] = {
>  
>  		.probe          = dp83867_probe,
>  		.config_init	= dp83867_config_init,
> +		.soft_reset	= dp83867_phy_reset,
>  
>  		.read_status	= dp83867_read_status,
>  	},
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |