Subject: Re: [PATCH] net/amazon: Ensure that driver version is aligned to the linux kernel

From: Leon Romanovsky <[email-masked]>
Date: Mon, 24 Feb 2020 18:26:49 +0200

On Mon, Feb 24, 2020 at 03:40:27PM +0000, [email-masked] wrote:
> From: Arthur Kiyanovski <[email-masked]>
>
> Upstream drivers are managed inside global repository and released all
> together, this ensure that driver version is the same as linux kernel,
> so update amazon drivers to properly reflect it.
>
> Also rename current driver version constants used in interface with
> ENA device FW for code clarity.
>
> Signed-off-by: Leon Romanovsky <[email-masked]>
> Signed-off-by: Arthur Kiyanovski <[email-masked]>
> ---
>  drivers/net/ethernet/amazon/ena/ena_ethtool.c |  1 -
>  drivers/net/ethernet/amazon/ena/ena_netdev.c  | 17 ++++-------------
>  drivers/net/ethernet/amazon/ena/ena_netdev.h  | 16 ++++++++--------
>  3 files changed, 12 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/ethernet/amazon/ena/ena_ethtool.c b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> index 971f02ea5..ad2148b86 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> @@ -404,7 +404,6 @@ static void ena_get_drvinfo(struct net_device *dev,
>  	struct ena_adapter *adapter = netdev_priv(dev);
>
>  	strlcpy(info->driver, DRV_MODULE_NAME, sizeof(info->driver));
> -	strlcpy(info->version, DRV_MODULE_VERSION, sizeof(info->version));
>  	strlcpy(info->bus_info, pci_name(adapter->pdev),
>  		sizeof(info->bus_info));
>  }
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index 1c1a41bd1..b14772812 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -49,12 +49,9 @@
>  #include "ena_netdev.h"
>  #include "ena_pci_id_tbl.h"
>
> -static char version[] = DEVICE_NAME " v" DRV_MODULE_VERSION "\n";
> -
>  MODULE_AUTHOR("Amazon.com, Inc. or its affiliates");
>  MODULE_DESCRIPTION(DEVICE_NAME);
>  MODULE_LICENSE("GPL");
> -MODULE_VERSION(DRV_MODULE_VERSION);
>
>  /* Time in jiffies before concluding the transmitter is hung. */
>  #define TX_TIMEOUT  (5 * HZ)
> @@ -2441,9 +2438,9 @@ static void ena_config_host_info(struct ena_com_dev *ena_dev,
>  	strncpy(host_info->os_dist_str, utsname()->release,
>  		sizeof(host_info->os_dist_str) - 1);
>  	host_info->driver_version =
> -		(DRV_MODULE_VER_MAJOR) |
> -		(DRV_MODULE_VER_MINOR << ENA_ADMIN_HOST_INFO_MINOR_SHIFT) |
> -		(DRV_MODULE_VER_SUBMINOR << ENA_ADMIN_HOST_INFO_SUB_MINOR_SHIFT) |
> +		(DRV_MODULE_GEN_MAJOR) |
> +		(DRV_MODULE_GEN_MINOR << ENA_ADMIN_HOST_INFO_MINOR_SHIFT) |
> +		(DRV_MODULE_GEN_SUBMINOR << ENA_ADMIN_HOST_INFO_SUB_MINOR_SHIFT) |
>  		("K"[0] << ENA_ADMIN_HOST_INFO_MODULE_TYPE_SHIFT);
>  	host_info->num_cpus = num_online_cpus();
>
> @@ -2822,9 +2819,7 @@ static int ena_restore_device(struct ena_adapter *adapter)
>  		netif_carrier_on(adapter->netdev);
>
>  	mod_timer(&adapter->timer_service, round_jiffies(jiffies + HZ));
> -	dev_err(&pdev->dev,
> -		"Device reset completed successfully, Driver info: %s\n",
> -		version);
> +	dev_err(&pdev->dev, "Device reset completed successfully\n");
>
>  	return rc;
>  err_disable_msix:
> @@ -3459,8 +3454,6 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>
>  	dev_dbg(&pdev->dev, "%s\n", __func__);
>
> -	dev_info_once(&pdev->dev, "%s", version);
> -
>  	rc = pci_enable_device_mem(pdev);
>  	if (rc) {
>  		dev_err(&pdev->dev, "pci_enable_device_mem() failed!\n");
> @@ -3766,8 +3759,6 @@ static struct pci_driver ena_pci_driver = {
>
>  static int __init ena_init(void)
>  {
> -	pr_info("%s", version);
> -
>  	ena_wq = create_singlethread_workqueue(DRV_MODULE_NAME);
>  	if (!ena_wq) {
>  		pr_err("Failed to create workqueue\n");
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.h b/drivers/net/ethernet/amazon/ena/ena_netdev.h
> index 2fe5eeea6..ac3f481b7 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.h
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.h
> @@ -44,16 +44,16 @@
>  #include "ena_com.h"
>  #include "ena_eth_com.h"
>
> -#define DRV_MODULE_VER_MAJOR	2
> -#define DRV_MODULE_VER_MINOR	1
> -#define DRV_MODULE_VER_SUBMINOR 0
> +#define DRV_MODULE_GEN_MAJOR	2
> +#define DRV_MODULE_GEN_MINOR	1
> +#define DRV_MODULE_GEN_SUBMINOR 0

I'm completely fine with this variant and did very similar thing for
broadcom driver where authors forwarded similar data to FW, but please
add the following line above those defines:
/* DO NOT CHANGE DRV_MODULE_GEN_* values in in-tree code */

Thanks

>
>  #define DRV_MODULE_NAME		"ena"
> -#ifndef DRV_MODULE_VERSION
> -#define DRV_MODULE_VERSION \
> -	__stringify(DRV_MODULE_VER_MAJOR) "."	\
> -	__stringify(DRV_MODULE_VER_MINOR) "."	\
> -	__stringify(DRV_MODULE_VER_SUBMINOR) "K"
> +#ifndef DRV_MODULE_GENERATION
> +#define DRV_MODULE_GENERATION \
> +	__stringify(DRV_MODULE_GEN_MAJOR) "."	\
> +	__stringify(DRV_MODULE_GEN_MINOR) "."	\
> +	__stringify(DRV_MODULE_GEN_SUBMINOR) "K"
>  #endif
>
>  #define DEVICE_NAME	"Elastic Network Adapter (ENA)"
> --
> 2.24.1.AMZN
>