diff mbox series

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

Message ID 91eafe4e922dab0930c7218e7baf597e6c6138a4.1631050524.git.davthompson@nvidia.com
State New
Headers show
Series UBUNTU: SAUCE: mlxbf_gige: clear valid_polarity upon open | expand

Commit Message

David Thompson Sept. 7, 2021, 9:41 p.m. UTC 
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@nvidia.com>
Signed-off-by: David Thompson <davthompson@nvidia.com>
---
 .../ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c    | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Kleber Sacilotto de Souza Sept. 8, 2021, 8:37 a.m. UTC | #1 
On 07.09.21 23:41, 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@nvidia.com>
> Signed-off-by: David Thompson <davthompson@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
>   
>   /* 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);
> +		}
>   	}
> -}

Is the above change in the curly brackets expected? This doesn't seem to have
any functional change but it breaks visual indentation.

>   
>   static void mlxbf_gige_get_stats64(struct net_device *netdev,
>   				   struct rtnl_link_stats64 *stats)
>
Tim Gardner Sept. 8, 2021, 11:59 a.m. UTC | #2 
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@nvidia.com>
> Signed-off-by: David Thompson <davthompson@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)
>
David Thompson Sept. 8, 2021, 2:50 p.m. UTC | #3 
> -----Original Message-----
> From: Tim Gardner <tim.gardner@canonical.com>
> Sent: Wednesday, September 8, 2021 7:59 AM
> To: David Thompson <davthompson@nvidia.com>; kernel-
> team@lists.ubuntu.com
> Cc: Meriton Tuli <meriton@nvidia.com>; Khoa Vo <khoav@nvidia.com>; Asmaa
> Mnebhi <asmaa@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@nvidia.com>
> > Signed-off-by: David Thompson <davthompson@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
diff mbox series

Patch

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
 
 /* 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);
+		}
 	}
-}
 
 static void mlxbf_gige_get_stats64(struct net_device *netdev,
 				   struct rtnl_link_stats64 *stats)