diff mbox series

[net] net: lan966x: Fix configuration of the PCS

Message ID 20221221093315.939133-1-horatiu.vultur@microchip.com (mailing list archive)
State Accepted
Commit d717f9474e3fb7e6bd3e43ca16e131f04320ed6f
Delegated to: Netdev Maintainers
Headers show
Series [net] net: lan966x: Fix configuration of the PCS | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 26 this patch: 26
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 5 this patch: 5
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 24 this patch: 24
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 8 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Horatiu Vultur Dec. 21, 2022, 9:33 a.m. UTC
When the PCS was taken out of reset, we were changing by mistake also
the speed to 100 Mbit. But in case the link was going down, the link
up routine was setting correctly the link speed. If the link was not
getting down then the speed was forced to run at 100 even if the
speed was something else.
On lan966x, to set the speed link to 1G or 2.5G a value of 1 needs to be
written in DEV_CLOCK_CFG_LINK_SPEED. This is similar to the procedure in
lan966x_port_init.

The issue was reproduced using 1000base-x sfp module using the commands:
ip link set dev eth2 up
ip link addr add 10.97.10.2/24 dev eth2
ethtool -s eth2 speed 1000 autoneg off

Fixes: d28d6d2e37d1 ("net: lan966x: add port module support")
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 drivers/net/ethernet/microchip/lan966x/lan966x_port.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Piotr Raczynski Dec. 22, 2022, 8:59 a.m. UTC | #1
On Wed, Dec 21, 2022 at 10:33:15AM +0100, Horatiu Vultur wrote:
> When the PCS was taken out of reset, we were changing by mistake also
> the speed to 100 Mbit. But in case the link was going down, the link
> up routine was setting correctly the link speed. If the link was not
> getting down then the speed was forced to run at 100 even if the
> speed was something else.
> On lan966x, to set the speed link to 1G or 2.5G a value of 1 needs to be
> written in DEV_CLOCK_CFG_LINK_SPEED. This is similar to the procedure in
> lan966x_port_init.
> 
> The issue was reproduced using 1000base-x sfp module using the commands:
> ip link set dev eth2 up
> ip link addr add 10.97.10.2/24 dev eth2
> ethtool -s eth2 speed 1000 autoneg off
> 
> Fixes: d28d6d2e37d1 ("net: lan966x: add port module support")
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  drivers/net/ethernet/microchip/lan966x/lan966x_port.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_port.c b/drivers/net/ethernet/microchip/lan966x/lan966x_port.c
> index 1a61c6cdb0779..0050fcb988b75 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_port.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_port.c
> @@ -381,7 +381,7 @@ int lan966x_port_pcs_set(struct lan966x_port *port,
>  	}
>  
>  	/* Take PCS out of reset */
> -	lan_rmw(DEV_CLOCK_CFG_LINK_SPEED_SET(2) |
> +	lan_rmw(DEV_CLOCK_CFG_LINK_SPEED_SET(LAN966X_SPEED_1000) |
>  		DEV_CLOCK_CFG_PCS_RX_RST_SET(0) |
>  		DEV_CLOCK_CFG_PCS_TX_RST_SET(0),
>  		DEV_CLOCK_CFG_LINK_SPEED |
> -- 
> 2.38.0
> 
LGTM, thanks for getting rid of a magic number here.

Reviewed-by: Piotr Raczynski <piotr.raczynski@intel.com>
patchwork-bot+netdevbpf@kernel.org Dec. 22, 2022, 11:30 a.m. UTC | #2
Hello:

This patch was applied to netdev/net.git (master)
by Paolo Abeni <pabeni@redhat.com>:

On Wed, 21 Dec 2022 10:33:15 +0100 you wrote:
> When the PCS was taken out of reset, we were changing by mistake also
> the speed to 100 Mbit. But in case the link was going down, the link
> up routine was setting correctly the link speed. If the link was not
> getting down then the speed was forced to run at 100 even if the
> speed was something else.
> On lan966x, to set the speed link to 1G or 2.5G a value of 1 needs to be
> written in DEV_CLOCK_CFG_LINK_SPEED. This is similar to the procedure in
> lan966x_port_init.
> 
> [...]

Here is the summary with links:
  - [net] net: lan966x: Fix configuration of the PCS
    https://git.kernel.org/netdev/net/c/d717f9474e3f

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_port.c b/drivers/net/ethernet/microchip/lan966x/lan966x_port.c
index 1a61c6cdb0779..0050fcb988b75 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_port.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_port.c
@@ -381,7 +381,7 @@  int lan966x_port_pcs_set(struct lan966x_port *port,
 	}
 
 	/* Take PCS out of reset */
-	lan_rmw(DEV_CLOCK_CFG_LINK_SPEED_SET(2) |
+	lan_rmw(DEV_CLOCK_CFG_LINK_SPEED_SET(LAN966X_SPEED_1000) |
 		DEV_CLOCK_CFG_PCS_RX_RST_SET(0) |
 		DEV_CLOCK_CFG_PCS_TX_RST_SET(0),
 		DEV_CLOCK_CFG_LINK_SPEED |