Re: [PATCH v1 rdma-next] i40iw: Report correct firmware version

[Jason Gunthorpe <jgg@ziepe.ca>]

On Fri, Mar 13, 2020 at 04:44:06PM -0500, Shiraz Saleem wrote:
> From: "Sindhu, Devale" <sindhu.devale@intel.com>
> 
> The driver uses a hard-coded value for FW version and
> reports an inconsistent FW version between ibv_devinfo
> and /sys/class/infiniband/i40iw/fw_ver.
> Retrieve the FW version via a Control QP (CQP) operation
> and report it consistently across sysfs and query device.
> 
> Fixes: d37498417947 ("i40iw: add files for iwarp interface")
> Reported-by: Jarod Wilson <jarod@redhat.com>
> Signed-off-by: Sindhu, Devale <sindhu.devale@intel.com>
> Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> ---
> v0-->v1:
> -Remove implicit casts
> -Use static inline function for computing FW version instead of macros
> ---
>  drivers/infiniband/hw/i40iw/i40iw.h        | 22 ++++++-
>  drivers/infiniband/hw/i40iw/i40iw_ctrl.c   | 99 ++++++++++++++++++++++++++++++
>  drivers/infiniband/hw/i40iw/i40iw_d.h      | 26 +++++++-
>  drivers/infiniband/hw/i40iw/i40iw_main.c   |  6 ++
>  drivers/infiniband/hw/i40iw/i40iw_p.h      |  1 +
>  drivers/infiniband/hw/i40iw/i40iw_status.h |  3 +-
>  drivers/infiniband/hw/i40iw/i40iw_type.h   | 12 ++++
>  drivers/infiniband/hw/i40iw/i40iw_verbs.c  | 10 +--
>  8 files changed, 170 insertions(+), 9 deletions(-)

Applied to for-next

> diff --git a/drivers/infiniband/hw/i40iw/i40iw_d.h b/drivers/infiniband/hw/i40iw/i40iw_d.h
> index 6ddaeec..e8367d6 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_d.h
> +++ b/drivers/infiniband/hw/i40iw/i40iw_d.h
> @@ -403,7 +403,7 @@
>  #define I40IW_CQP_OP_MANAGE_ARP                 0x0f
>  #define I40IW_CQP_OP_MANAGE_VF_PBLE_BP          0x10
>  #define I40IW_CQP_OP_MANAGE_PUSH_PAGES          0x11
> -#define I40IW_CQP_OP_MANAGE_PE_TEAM             0x12
> +#define I40IW_CQP_OP_QUERY_RDMA_FEATURES	0x12
>  #define I40IW_CQP_OP_UPLOAD_CONTEXT             0x13
>  #define I40IW_CQP_OP_ALLOCATE_LOC_MAC_IP_TABLE_ENTRY 0x14
>  #define I40IW_CQP_OP_MANAGE_HMC_PM_FUNC_TABLE   0x15
> @@ -431,6 +431,24 @@
>  #define I40IW_CQP_OP_SHMC_PAGES_ALLOCATED       0x2b
>  #define I40IW_CQP_OP_SET_HMC_RESOURCE_PROFILE   0x2d
>  
> +#define I40IW_FEATURE_BUF_SIZE                  (8 * I40IW_MAX_FEATURES)
> +
> +#define I40IW_FW_VER_MINOR_SHIFT        0
> +#define I40IW_FW_VER_MINOR_MASK         \
> +	(0xffffULL << I40IW_FW_VER_MINOR_SHIFT)
> +
> +#define I40IW_FW_VER_MAJOR_SHIFT        16
> +#define I40IW_FW_VER_MAJOR_MASK	        \
> +	(0xffffULL << I40IW_FW_VER_MAJOR_SHIFT)
> +
> +#define I40IW_FEATURE_INFO_SHIFT        0
> +#define I40IW_FEATURE_INFO_MASK         \
> +	(0xffffULL << I40IW_FEATURE_INFO_SHIFT)
> +
> +#define I40IW_FEATURE_CNT_SHIFT         32
> +#define I40IW_FEATURE_CNT_MASK          \
> +	(0xffffULL << I40IW_FEATURE_CNT_SHIFT)

Please see the discussion about these kinds of macros on the EFA
thread - please don't use this scheme in the new
driver. The standard GENMASK/FIELD_PREP/etc should be used

Thanks,
Jason