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

Leon Romanovsky
2020-02-24T16:26:59Z

On Mon, Feb 24, 2020 at 03:40:27PM +0000, sameehj@amazon.com wrote:
> From: Arthur Kiyanovski <akiyano@amazon.com>
>
> 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 <leonro@mellanox.com>
> Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
> ---
> 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
>