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

[David Thompson]

> -----Original Message-----
> From: Tim Gardner <tim.gardner at canonical.com>
> Sent: Wednesday, September 8, 2021 7:59 AM
> To: David Thompson <davthompson at nvidia.com>; kernel-
> team at lists.ubuntu.com
> Cc: Meriton Tuli <meriton at nvidia.com>; Khoa Vo <khoav at nvidia.com>; Asmaa
> Mnebhi <asmaa at nvidia.com>
> Subject: NACK: [SRU][F:linux-bluefield][PATCH v1 1/1] UBUNTU: SAUCE:
> mlxbf_gige: clear valid_polarity upon open
> 
> 
> 
> 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.
> 

Understood, I will create a separate commit for bumping up the version.

> >   /* 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.
> 

Yes, this is a mistake and I'll clean it up in next patch version.

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