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

[Piotr Raczynski <piotr.raczynski@intel.com>]

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>