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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 22 Dec 2022 09:59:39 +0100
From:   Piotr Raczynski <piotr.raczynski@...el.com>
To:     Horatiu Vultur <horatiu.vultur@...rochip.com>
CC:     <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
        <pabeni@...hat.com>, <UNGLinuxDriver@...rochip.com>,
        <linux@...linux.org.uk>
Subject: Re: [PATCH net] net: lan966x: Fix configuration of the PCS

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@...rochip.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@...el.com>