diff mbox series

[net-next,1/3] net: phy: marvell-88q2xxx: Align soft reset for mv88q2110 and mv88q2220

Message ID 20240906133951.3433788-2-niklas.soderlund+renesas@ragnatech.se (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: phy: marvell-88q2xxx: Enable auto negotiation for mv88q2110 | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 16 this patch: 16
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 16 this patch: 16
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 16 this patch: 16
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 81 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-09-07--06-00 (tests: 722)

Commit Message

Niklas Söderlund Sept. 6, 2024, 1:39 p.m. UTC 
The soft reset implementations for mv88q2110 and mv88q2220 differ as the
later need to consider that auto negation is supported on mv88q2220
devices. In preparation of enabling auto negotiation on mv88q2110 merge
the two rest functions into a device generic one.

The mv88q2220 behavior is kept as is but extended to wait for the reset
bit to be clears before continuing, as was done previously on mv88q2220.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/net/phy/marvell-88q2xxx.c | 60 ++++++++++++++-----------------
 1 file changed, 26 insertions(+), 34 deletions(-)

Comments

Andrew Lunn Sept. 10, 2024, 8:21 p.m. UTC | #1 
On Fri, Sep 06, 2024 at 03:39:49PM +0200, Niklas Söderlund wrote:
> The soft reset implementations for mv88q2110 and mv88q2220 differ as the
> later need to consider that auto negation is supported on mv88q2220
> devices. In preparation of enabling auto negotiation on mv88q2110 merge
> the two rest functions into a device generic one.
> 
> The mv88q2220 behavior is kept as is but extended to wait for the reset
> bit to be clears before continuing, as was done previously on mv88q2220.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

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

    Andrew
Dimitri Fedrau Sept. 12, 2024, 4:51 p.m. UTC | #2 
Am Fri, Sep 06, 2024 at 03:39:49PM +0200 schrieb Niklas Söderlund:
> The soft reset implementations for mv88q2110 and mv88q2220 differ as the
> later need to consider that auto negation is supported on mv88q2220
> devices. In preparation of enabling auto negotiation on mv88q2110 merge
> the two rest functions into a device generic one.
> 
> The mv88q2220 behavior is kept as is but extended to wait for the reset
> bit to be clears before continuing, as was done previously on mv88q2220.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/net/phy/marvell-88q2xxx.c | 60 ++++++++++++++-----------------
>  1 file changed, 26 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
> index c812f16eaa3a..850beb4b1722 100644
> --- a/drivers/net/phy/marvell-88q2xxx.c
> +++ b/drivers/net/phy/marvell-88q2xxx.c
> @@ -179,15 +179,34 @@ static int mv88q2xxx_soft_reset(struct phy_device *phydev)
>  	int ret;
>  	int val;
>  
> -	ret = phy_write_mmd(phydev, MDIO_MMD_PCS,
> -			    MDIO_PCS_1000BT1_CTRL, MDIO_PCS_1000BT1_CTRL_RESET);
> +	/* Enable RESET of DCL */
> +	if (phydev->autoneg == AUTONEG_ENABLE || phydev->speed == SPEED_1000) {
> +		ret = phy_write_mmd(phydev, MDIO_MMD_PCS, 0xfe1b, 0x48);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_1000BT1_CTRL,
> +			    MDIO_PCS_1000BT1_CTRL_RESET);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = phy_read_mmd_poll_timeout(phydev, MDIO_MMD_PCS,
> +					MDIO_PCS_1000BT1_CTRL, val,
> +					!(val & MDIO_PCS_1000BT1_CTRL_RESET),
> +					50000, 600000, true);
>  	if (ret < 0)
>  		return ret;
>  
> -	return phy_read_mmd_poll_timeout(phydev, MDIO_MMD_PCS,
> -					 MDIO_PCS_1000BT1_CTRL, val,
> -					 !(val & MDIO_PCS_1000BT1_CTRL_RESET),
> -					 50000, 600000, true);
> +	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, 0xffe4, 0xc);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Disable RESET of DCL */
> +	if (phydev->autoneg == AUTONEG_ENABLE || phydev->speed == SPEED_1000)
> +		return phy_write_mmd(phydev, MDIO_MMD_PCS, 0xfe1b, 0x58);
> +
> +	return 0;
>  }
>  
>  static int mv88q2xxx_read_link_gbit(struct phy_device *phydev)
> @@ -705,33 +724,6 @@ static int mv88q2xxx_probe(struct phy_device *phydev)
>  	return mv88q2xxx_hwmon_probe(phydev);
>  }
>  
> -static int mv88q222x_soft_reset(struct phy_device *phydev)
> -{
> -	int ret;
> -
> -	/* Enable RESET of DCL */
> -	if (phydev->autoneg == AUTONEG_ENABLE || phydev->speed == SPEED_1000) {
> -		ret = phy_write_mmd(phydev, MDIO_MMD_PCS, 0xfe1b, 0x48);
> -		if (ret < 0)
> -			return ret;
> -	}
> -
> -	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_1000BT1_CTRL,
> -			    MDIO_PCS_1000BT1_CTRL_RESET);
> -	if (ret < 0)
> -		return ret;
> -
> -	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, 0xffe4, 0xc);
> -	if (ret < 0)
> -		return ret;
> -
> -	/* Disable RESET of DCL */
> -	if (phydev->autoneg == AUTONEG_ENABLE || phydev->speed == SPEED_1000)
> -		return phy_write_mmd(phydev, MDIO_MMD_PCS, 0xfe1b, 0x58);
> -
> -	return 0;
> -}
> -
>  static int mv88q222x_write_mmd_vals(struct phy_device *phydev,
>  				    const struct mmd_val *vals, size_t len)
>  {
> @@ -906,7 +898,7 @@ static struct phy_driver mv88q2xxx_driver[] = {
>  		.aneg_done		= genphy_c45_aneg_done,
>  		.config_init		= mv88q222x_config_init,
>  		.read_status		= mv88q2xxx_read_status,
> -		.soft_reset		= mv88q222x_soft_reset,
> +		.soft_reset		= mv88q2xxx_soft_reset,
>  		.config_intr		= mv88q2xxx_config_intr,
>  		.handle_interrupt	= mv88q2xxx_handle_interrupt,
>  		.set_loopback		= genphy_c45_loopback,
> -- 
> 2.46.0
>

Hi Niklas,

tested with a mv88q2220 device, worked as expected.

Tested-by: Dimitri Fedrau <dima.fedrau@gmail.com>

Best regards,
Dimitri Fedrau
Stefan Eichenberger Sept. 25, 2024, 12:22 p.m. UTC | #3 
On Fri, Sep 06, 2024 at 03:39:49PM +0200, Niklas Söderlund wrote:
> The soft reset implementations for mv88q2110 and mv88q2220 differ as the
> later need to consider that auto negation is supported on mv88q2220
> devices. In preparation of enabling auto negotiation on mv88q2110 merge
> the two rest functions into a device generic one.
> 
> The mv88q2220 behavior is kept as is but extended to wait for the reset
> bit to be clears before continuing, as was done previously on mv88q2220.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/net/phy/marvell-88q2xxx.c | 60 ++++++++++++++-----------------
>  1 file changed, 26 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
> index c812f16eaa3a..850beb4b1722 100644
> --- a/drivers/net/phy/marvell-88q2xxx.c
> +++ b/drivers/net/phy/marvell-88q2xxx.c
> @@ -179,15 +179,34 @@ static int mv88q2xxx_soft_reset(struct phy_device *phydev)
>  	int ret;
>  	int val;
>  
> -	ret = phy_write_mmd(phydev, MDIO_MMD_PCS,
> -			    MDIO_PCS_1000BT1_CTRL, MDIO_PCS_1000BT1_CTRL_RESET);
> +	/* Enable RESET of DCL */
> +	if (phydev->autoneg == AUTONEG_ENABLE || phydev->speed == SPEED_1000) {
> +		ret = phy_write_mmd(phydev, MDIO_MMD_PCS, 0xfe1b, 0x48);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_1000BT1_CTRL,
> +			    MDIO_PCS_1000BT1_CTRL_RESET);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = phy_read_mmd_poll_timeout(phydev, MDIO_MMD_PCS,
> +					MDIO_PCS_1000BT1_CTRL, val,
> +					!(val & MDIO_PCS_1000BT1_CTRL_RESET),
> +					50000, 600000, true);
>  	if (ret < 0)
>  		return ret;
>  
> -	return phy_read_mmd_poll_timeout(phydev, MDIO_MMD_PCS,
> -					 MDIO_PCS_1000BT1_CTRL, val,
> -					 !(val & MDIO_PCS_1000BT1_CTRL_RESET),
> -					 50000, 600000, true);
> +	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, 0xffe4, 0xc);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Disable RESET of DCL */
> +	if (phydev->autoneg == AUTONEG_ENABLE || phydev->speed == SPEED_1000)
> +		return phy_write_mmd(phydev, MDIO_MMD_PCS, 0xfe1b, 0x58);
> +
> +	return 0;
>  }
>  
>  static int mv88q2xxx_read_link_gbit(struct phy_device *phydev)
> @@ -705,33 +724,6 @@ static int mv88q2xxx_probe(struct phy_device *phydev)
>  	return mv88q2xxx_hwmon_probe(phydev);
>  }
>  
> -static int mv88q222x_soft_reset(struct phy_device *phydev)
> -{
> -	int ret;
> -
> -	/* Enable RESET of DCL */
> -	if (phydev->autoneg == AUTONEG_ENABLE || phydev->speed == SPEED_1000) {
> -		ret = phy_write_mmd(phydev, MDIO_MMD_PCS, 0xfe1b, 0x48);
> -		if (ret < 0)
> -			return ret;
> -	}
> -
> -	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_1000BT1_CTRL,
> -			    MDIO_PCS_1000BT1_CTRL_RESET);
> -	if (ret < 0)
> -		return ret;
> -
> -	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, 0xffe4, 0xc);
> -	if (ret < 0)
> -		return ret;
> -
> -	/* Disable RESET of DCL */
> -	if (phydev->autoneg == AUTONEG_ENABLE || phydev->speed == SPEED_1000)
> -		return phy_write_mmd(phydev, MDIO_MMD_PCS, 0xfe1b, 0x58);
> -
> -	return 0;
> -}
> -
>  static int mv88q222x_write_mmd_vals(struct phy_device *phydev,
>  				    const struct mmd_val *vals, size_t len)
>  {
> @@ -906,7 +898,7 @@ static struct phy_driver mv88q2xxx_driver[] = {
>  		.aneg_done		= genphy_c45_aneg_done,
>  		.config_init		= mv88q222x_config_init,
>  		.read_status		= mv88q2xxx_read_status,
> -		.soft_reset		= mv88q222x_soft_reset,
> +		.soft_reset		= mv88q2xxx_soft_reset,
>  		.config_intr		= mv88q2xxx_config_intr,
>  		.handle_interrupt	= mv88q2xxx_handle_interrupt,
>  		.set_loopback		= genphy_c45_loopback,
> -- 
> 2.46.0
> 

Tested with a mv88q2110 device.

Tested-by: Stefan Eichenberger <eichest@gmail.com>

Regards,
Stefan
diff mbox series

Patch

diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
index c812f16eaa3a..850beb4b1722 100644
--- a/drivers/net/phy/marvell-88q2xxx.c
+++ b/drivers/net/phy/marvell-88q2xxx.c
@@ -179,15 +179,34 @@  static int mv88q2xxx_soft_reset(struct phy_device *phydev)
 	int ret;
 	int val;
 
-	ret = phy_write_mmd(phydev, MDIO_MMD_PCS,
-			    MDIO_PCS_1000BT1_CTRL, MDIO_PCS_1000BT1_CTRL_RESET);
+	/* Enable RESET of DCL */
+	if (phydev->autoneg == AUTONEG_ENABLE || phydev->speed == SPEED_1000) {
+		ret = phy_write_mmd(phydev, MDIO_MMD_PCS, 0xfe1b, 0x48);
+		if (ret < 0)
+			return ret;
+	}
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_1000BT1_CTRL,
+			    MDIO_PCS_1000BT1_CTRL_RESET);
+	if (ret < 0)
+		return ret;
+
+	ret = phy_read_mmd_poll_timeout(phydev, MDIO_MMD_PCS,
+					MDIO_PCS_1000BT1_CTRL, val,
+					!(val & MDIO_PCS_1000BT1_CTRL_RESET),
+					50000, 600000, true);
 	if (ret < 0)
 		return ret;
 
-	return phy_read_mmd_poll_timeout(phydev, MDIO_MMD_PCS,
-					 MDIO_PCS_1000BT1_CTRL, val,
-					 !(val & MDIO_PCS_1000BT1_CTRL_RESET),
-					 50000, 600000, true);
+	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, 0xffe4, 0xc);
+	if (ret < 0)
+		return ret;
+
+	/* Disable RESET of DCL */
+	if (phydev->autoneg == AUTONEG_ENABLE || phydev->speed == SPEED_1000)
+		return phy_write_mmd(phydev, MDIO_MMD_PCS, 0xfe1b, 0x58);
+
+	return 0;
 }
 
 static int mv88q2xxx_read_link_gbit(struct phy_device *phydev)
@@ -705,33 +724,6 @@  static int mv88q2xxx_probe(struct phy_device *phydev)
 	return mv88q2xxx_hwmon_probe(phydev);
 }
 
-static int mv88q222x_soft_reset(struct phy_device *phydev)
-{
-	int ret;
-
-	/* Enable RESET of DCL */
-	if (phydev->autoneg == AUTONEG_ENABLE || phydev->speed == SPEED_1000) {
-		ret = phy_write_mmd(phydev, MDIO_MMD_PCS, 0xfe1b, 0x48);
-		if (ret < 0)
-			return ret;
-	}
-
-	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_1000BT1_CTRL,
-			    MDIO_PCS_1000BT1_CTRL_RESET);
-	if (ret < 0)
-		return ret;
-
-	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, 0xffe4, 0xc);
-	if (ret < 0)
-		return ret;
-
-	/* Disable RESET of DCL */
-	if (phydev->autoneg == AUTONEG_ENABLE || phydev->speed == SPEED_1000)
-		return phy_write_mmd(phydev, MDIO_MMD_PCS, 0xfe1b, 0x58);
-
-	return 0;
-}
-
 static int mv88q222x_write_mmd_vals(struct phy_device *phydev,
 				    const struct mmd_val *vals, size_t len)
 {
@@ -906,7 +898,7 @@  static struct phy_driver mv88q2xxx_driver[] = {
 		.aneg_done		= genphy_c45_aneg_done,
 		.config_init		= mv88q222x_config_init,
 		.read_status		= mv88q2xxx_read_status,
-		.soft_reset		= mv88q222x_soft_reset,
+		.soft_reset		= mv88q2xxx_soft_reset,
 		.config_intr		= mv88q2xxx_config_intr,
 		.handle_interrupt	= mv88q2xxx_handle_interrupt,
 		.set_loopback		= genphy_c45_loopback,