Message-ID: <f76ad3e6-fccb-a481-8283-c8ff3100a82b@gmail.com>
Date:   Sun, 28 Nov 2021 15:57:12 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.1
Subject: Re: [PATCH] net/mlx4_en: Update reported link modes for 1/10G
Content-Language: en-US
To:     Erik Ekman <erik@kryo.se>, "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>
Cc:     Michael Stapelberg <michael@stapelberg.ch>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
References: <20211128123712.82096-1-erik@kryo.se>
From:   Tariq Toukan <ttoukan.linux@gmail.com>
In-Reply-To: <20211128123712.82096-1-erik@kryo.se>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 11/28/2021 2:37 PM, Erik Ekman wrote:
> When link modes were initially added in commit 2c762679435dc
> ("net/mlx4_en: Use PTYS register to query ethtool settings") and
> later updated for the new ethtool API in commit 3d8f7cc78d0eb
> ("net: mlx4: use new ETHTOOL_G/SSETTINGS API") the only 1/10G non-baseT
> link modes configured were 1000baseKX, 10000baseKX4 and 10000baseKR.
> It looks like these got picked to represent other modes since nothing
> better was available.
> 
> Switch to using more specific link modes added in commit 5711a98221443
> ("net: ethtool: add support for 1000BaseX and missing 10G link modes").
> 
> Tested with MCX311A-XCAT connected via DAC.
> Before:
> 
> % sudo ethtool enp3s0
> Settings for enp3s0:
> 	Supported ports: [ FIBRE ]
> 	Supported link modes:   1000baseKX/Full
> 	                        10000baseKR/Full
> 	Supported pause frame use: Symmetric Receive-only
> 	Supports auto-negotiation: No
> 	Supported FEC modes: Not reported
> 	Advertised link modes:  1000baseKX/Full
> 	                        10000baseKR/Full
> 	Advertised pause frame use: Symmetric
> 	Advertised auto-negotiation: No
> 	Advertised FEC modes: Not reported
> 	Speed: 10000Mb/s
> 	Duplex: Full
> 	Auto-negotiation: off
> 	Port: Direct Attach Copper
> 	PHYAD: 0
> 	Transceiver: internal
> 	Supports Wake-on: d
> 	Wake-on: d
>          Current message level: 0x00000014 (20)
>                                 link ifdown
> 	Link detected: yes
> 
> With this change:
> 
> % sudo ethtool enp3s0
> 	Settings for enp3s0:
> 	Supported ports: [ FIBRE ]
> 	Supported link modes:   1000baseX/Full
> 	                        10000baseCR/Full
>   	                        10000baseSR/Full
> 	Supported pause frame use: Symmetric Receive-only
> 	Supports auto-negotiation: No
> 	Supported FEC modes: Not reported
> 	Advertised link modes:  1000baseX/Full
>   	                        10000baseCR/Full
>   	                        10000baseSR/Full
> 	Advertised pause frame use: Symmetric
> 	Advertised auto-negotiation: No
> 	Advertised FEC modes: Not reported
> 	Speed: 10000Mb/s
> 	Duplex: Full
> 	Auto-negotiation: off
> 	Port: Direct Attach Copper
> 	PHYAD: 0
> 	Transceiver: internal
> 	Supports Wake-on: d
> 	Wake-on: d
>          Current message level: 0x00000014 (20)
>                                 link ifdown
> 	Link detected: yes
> 
> Tested-by: Michael Stapelberg <michael@stapelberg.ch>
> Signed-off-by: Erik Ekman <erik@kryo.se>
> ---
>   drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> index 066d79e4ecfc..10238bedd694 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> @@ -670,7 +670,7 @@ void __init mlx4_en_init_ptys2ethtool_map(void)
>   	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_1000BASE_T, SPEED_1000,
>   				       ETHTOOL_LINK_MODE_1000baseT_Full_BIT);
>   	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_1000BASE_CX_SGMII, SPEED_1000,
> -				       ETHTOOL_LINK_MODE_1000baseKX_Full_BIT);
> +				       ETHTOOL_LINK_MODE_1000baseX_Full_BIT);
>   	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_1000BASE_KX, SPEED_1000,
>   				       ETHTOOL_LINK_MODE_1000baseKX_Full_BIT);
>   	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_10GBASE_T, SPEED_10000,
> @@ -682,9 +682,9 @@ void __init mlx4_en_init_ptys2ethtool_map(void)
>   	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_10GBASE_KR, SPEED_10000,
>   				       ETHTOOL_LINK_MODE_10000baseKR_Full_BIT);
>   	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_10GBASE_CR, SPEED_10000,
> -				       ETHTOOL_LINK_MODE_10000baseKR_Full_BIT);
> +				       ETHTOOL_LINK_MODE_10000baseCR_Full_BIT);
>   	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_10GBASE_SR, SPEED_10000,
> -				       ETHTOOL_LINK_MODE_10000baseKR_Full_BIT);
> +				       ETHTOOL_LINK_MODE_10000baseSR_Full_BIT);
>   	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_20GBASE_KR2, SPEED_20000,
>   				       ETHTOOL_LINK_MODE_20000baseMLD2_Full_BIT,
>   				       ETHTOOL_LINK_MODE_20000baseKR2_Full_BIT);
> 

LGTM. Thanks for your patch.
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>