Re: [dpdk-dev] [PATCH] net/ice: send driver version to firmware

["Zhang, Qi Z" <qi.z.zhang@intel.com>]

> -----Original Message-----
> From: Yang, Qiming
> Sent: Thursday, April 4, 2019 10:28 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org; Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>
> Subject: RE: [PATCH] net/ice: send driver version to firmware
> 
> 
> 
> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Friday, March 29, 2019 9:30 AM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>
> Cc: dev@dpdk.org; Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Subject: [PATCH] net/ice: send driver version to firmware
> 
> The driver must send its version information to the firmware, so the firmware
> knows the driver is up. Otherwise, it will cause unexpected OS package
> downloading when multiple driver instances running on the same device.
> 
> Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> ---
>  drivers/net/ice/base/ice_adminq_cmd.h | 12 ++++++++++++
>  drivers/net/ice/base/ice_common.c     | 36
> +++++++++++++++++++++++++++++++++++
>  drivers/net/ice/base/ice_common.h     |  3 +++
>  drivers/net/ice/base/ice_type.h       |  4 ++++
>  drivers/net/ice/ice_ethdev.c          | 21 ++++++++++++++++++++
>  5 files changed, 76 insertions(+)
> 
> diff --git a/drivers/net/ice/base/ice_adminq_cmd.h
> b/drivers/net/ice/base/ice_adminq_cmd.h
> index d2ab9eeff..bbdca83fc 100644
> --- a/drivers/net/ice/base/ice_adminq_cmd.h
> +++ b/drivers/net/ice/base/ice_adminq_cmd.h
> @@ -38,6 +38,17 @@ struct ice_aqc_get_ver {  };
> 
> 
> +/* Send driver version (indirect 0x0002) */ struct ice_aqc_driver_ver {
> +	u8 major_ver;
> +	u8 minor_ver;
> +	u8 build_ver;
> +	u8 subbuild_ver;
> +	u8 reserved[4];
> +	__le32 addr_high;
> +	__le32 addr_low;
> +};
> +
> 
>  /* Queue Shutdown (direct 0x0003) */
>  struct ice_aqc_q_shutdown {
> @@ -2182,6 +2193,7 @@ struct ice_aq_desc {
>  		u8 raw[16];
>  		struct ice_aqc_generic generic;
>  		struct ice_aqc_get_ver get_ver;
> +		struct ice_aqc_driver_ver driver_ver;
>  		struct ice_aqc_q_shutdown q_shutdown;
>  		struct ice_aqc_req_res res_owner;
>  		struct ice_aqc_manage_mac_read mac_read; diff --git
> a/drivers/net/ice/base/ice_common.c b/drivers/net/ice/base/ice_common.c
> index 3d2e5f347..c74e4e1d4 100644
> --- a/drivers/net/ice/base/ice_common.c
> +++ b/drivers/net/ice/base/ice_common.c
> @@ -1512,6 +1512,42 @@ enum ice_status ice_aq_get_fw_ver(struct ice_hw
> *hw, struct ice_sq_cd *cd)
>  	return status;
>  }
> 
> +/**
> + * ice_aq_send_driver_ver
> + * @hw: pointer to the HW struct
> + * @dv: driver's major, minor version
> + * @cd: pointer to command details structure or NULL
> + *
> + * Send the driver version (0x0002) to the firmware  */ enum ice_status
> +ice_aq_send_driver_ver(struct ice_hw *hw, struct ice_driver_ver *dv,
> +		       struct ice_sq_cd *cd)
> +{
> +	struct ice_aqc_driver_ver *cmd;
> +	struct ice_aq_desc desc;
> +	u16 len;
> +
> +	cmd = &desc.params.driver_ver;
> +
> +	if (!dv)
> +		return ICE_ERR_PARAM;
> +
> +	ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_driver_ver);
> +
> +	desc.flags |= CPU_TO_LE16(ICE_AQ_FLAG_RD);
> +	cmd->major_ver = dv->major_ver;
> +	cmd->minor_ver = dv->minor_ver;
> +	cmd->build_ver = dv->build_ver;
> +	cmd->subbuild_ver = dv->subbuild_ver;
> +
> +	len = 0;
> +	while (len < sizeof(dv->driver_string) &&
> +	       IS_ASCII(dv->driver_string[len]) && dv->driver_string[len])
> +		len++;
> +
> +	return ice_aq_send_cmd(hw, &desc, dv->driver_string, len, cd); }
> 
>  /**
>   * ice_aq_q_shutdown
> diff --git a/drivers/net/ice/base/ice_common.h
> b/drivers/net/ice/base/ice_common.h
> index e8f2ce9d8..58c66fdc0 100644
> --- a/drivers/net/ice/base/ice_common.h
> +++ b/drivers/net/ice/base/ice_common.h
> @@ -119,6 +119,9 @@ ice_aq_send_cmd(struct ice_hw *hw, struct ice_aq_desc
> *desc,  enum ice_status ice_aq_get_fw_ver(struct ice_hw *hw, struct
> ice_sq_cd *cd);
> 
>  enum ice_status
> +ice_aq_send_driver_ver(struct ice_hw *hw, struct ice_driver_ver *dv,
> +		       struct ice_sq_cd *cd);
> +enum ice_status
>  ice_aq_get_phy_caps(struct ice_port_info *pi, bool qual_mods, u8
> report_mode,
>  		    struct ice_aqc_get_phy_caps_data *caps,
>  		    struct ice_sq_cd *cd);
> diff --git a/drivers/net/ice/base/ice_type.h b/drivers/net/ice/base/ice_type.h
> index b0bdec2df..e4979b832 100644
> --- a/drivers/net/ice/base/ice_type.h
> +++ b/drivers/net/ice/base/ice_type.h
> @@ -22,6 +22,10 @@
>  #define MIN_T(_t, _a, _b)	min((_t)(_a), (_t)(_b))
>  #endif
> 
> +#ifndef IS_ASCII
> +#define IS_ASCII(_ch)  ((_ch) < 0x80)
> +#endif
> +
>  #include "ice_status.h"
>  #include "ice_hw_autogen.h"
>  #include "ice_devids.h"
> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c index
> 85311dde0..922a211f1 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -1244,6 +1244,21 @@ ice_setup_vsi(struct ice_pf *pf, enum ice_vsi_type
> type)  }
> 
>  static int
> +ice_send_driver_ver(struct ice_hw *hw)
> +{
> +	struct ice_driver_ver dv;
> +
> +	/* we don't have driver version use 0 for dummy */
> +	dv.major_ver = 0;
> +	dv.minor_ver = 0;
> +	dv.build_ver = 0;
> +	dv.subbuild_ver = 0;
> +	strncpy((char *)dv.driver_string, "dpdk", sizeof(dv.driver_string));
> +
> +	return ice_aq_send_driver_ver(hw, &dv, NULL); }
> +
> +static int
>  ice_pf_setup(struct ice_pf *pf)
>  {
>  	struct ice_vsi *vsi;
> @@ -1401,6 +1416,12 @@ ice_dev_init(struct rte_eth_dev *dev)
>  		goto err_pf_setup;
>  	}
> 
> +	ret = ice_send_driver_ver(hw);
> +	if (ret) {
> +		PMD_INIT_LOG(ERR, "Failed to send driver version");
> +		goto err_pf_setup;
> +	}
> +
>  	vsi = pf->main_vsi;
> 
>  	/* Disable double vlan by default */
> --
> 2.13.6
> 
> 
> Should the share code part and PMD part be separated into two patches?

That might be better, but I assume you are still ok with this :)

> Acked-by: Qiming Yang <qiming.yang@intel.com>

Applied to dpdk-next-net-intel

Thanks
Qi