Re: [PATCH net-next] vmxnet3: support higher link speeds from vmxnet3 v9

[Simon Horman <horms@kernel.org>]

On Tue, Sep 17, 2024 at 03:59:46PM -0700, Ronak Doshi wrote:
> Until now, vmxnet3 was default reporting 10Gbps as link speed.
> Vmxnet3 v9 adds support for user to configure higher link speeds.
> User can configure the link speed via VMs advanced parameters options
> in VCenter. This speed is reported in gbps by hypervisor.
> 
> This patch adds support for vmxnet3 to report higher link speeds and
> converts it to mbps as expected by Linux stack.
> 
> Signed-off-by: Ronak Doshi <ronak.doshi@broadcom.com>
> Acked-by: Guolin Yang <guolin.yang@broadcom.com>
> ---
>  drivers/net/vmxnet3/vmxnet3_drv.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
> index b70654c7ad34..bb514b72c8b5 100644
> --- a/drivers/net/vmxnet3/vmxnet3_drv.c
> +++ b/drivers/net/vmxnet3/vmxnet3_drv.c
> @@ -201,6 +201,8 @@ vmxnet3_check_link(struct vmxnet3_adapter *adapter, bool affectTxQueue)
>  
>  	adapter->link_speed = ret >> 16;
>  	if (ret & 1) { /* Link is up. */

Hi Ronak,

I think it would be nice to add a comment regarding the logic added below,
particularly the inequality.  It took me more than one reading to
understand it in the presence of the patch description. I expected may have
remained a mystery without some accompanying text.

> +		if (VMXNET3_VERSION_GE_9(adapter) && adapter->link_speed < 10000)

Please consider limiting Networking code to 80 columns wide where it
can trivially be achieved, as appears to be the case here.

checkpatch can be run with an option to flag this.

> +			adapter->link_speed = adapter->link_speed * 1000;

>  		netdev_info(adapter->netdev, "NIC Link is Up %d Mbps\n",
>  			    adapter->link_speed);
>  		netif_carrier_on(adapter->netdev);

net-next is currently closed for the v6.12 merge window.
Please repost this patch after it reopens, which will be after
v6.12-rc1 is released, most likely a little under two weeks from now.

-- 
pw-bot: changes-requested