NACK: [SRU][F:linux-bluefield][PATCH v1 1/1] UBUNTU: SAUCE: mlxbf_gige: clear valid_polarity upon open

Tim Gardner tim.gardner at canonical.com
Wed Sep 8 11:59:26 UTC 2021



On 9/7/21 3:41 PM, David Thompson wrote:
> BugLink: https://bugs.launchpad.net/bugs/1942932
> 
> This patch ensures that the driver's valid_polarity
> is cleared during the open() method so that it always
> matches the receive polarity used by hardware.
> 
> Reviewed-by: Asmaa Mnebhi <asmaa at nvidia.com>
> Signed-off-by: David Thompson <davthompson at nvidia.com>
> ---
>   .../ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c    | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
> index 7caa1ca4461f..79420577f11b 100644
> --- a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
> +++ b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
> @@ -20,7 +20,7 @@
>   #include "mlxbf_gige_regs.h"
>   
>   #define DRV_NAME    "mlxbf_gige"
> -#define DRV_VERSION 1.24
> +#define DRV_VERSION 1.25
>   

Updating the version number appears to have nothing to do with the 
intent of the patch. This should be its own commit with a subject making 
it clear that its a version marker.

>   /* This setting defines the version of the ACPI table
>    * content that is compatible with this driver version.
> @@ -148,6 +148,13 @@ static int mlxbf_gige_open(struct net_device *netdev)
>   	err = mlxbf_gige_clean_port(priv);
>   	if (err)
>   		goto free_irqs;
> +
> +	/* Clear driver's valid_polarity to match hardware,
> +	 * since the above call to clean_port() resets the
> +	 * receive polarity used by hardware.
> +	 */
> +	priv->valid_polarity = 0;
> +
>   	err = mlxbf_gige_rx_init(priv);
>   	if (err)
>   		goto free_irqs;
> @@ -231,8 +238,8 @@ static void mlxbf_gige_set_rx_mode(struct net_device *netdev)
>   			mlxbf_gige_enable_promisc(priv);
>   		else
>   			mlxbf_gige_disable_promisc(priv);

As Kleber pointed out, whats up with this ? Seems like a random 
unintended change.

> +		}
>   	}
> -}
>   
>   static void mlxbf_gige_get_stats64(struct net_device *netdev,
>   				   struct rtnl_link_stats64 *stats)
> 

-- 
-----------
Tim Gardner
Canonical, Inc



More information about the kernel-team mailing list