Re: [dpdk-dev] [PATCH] net/ice: fix incorrect EEPROM data

["Yang, Qiming" <qiming.yang@intel.com>]

Hi, Shougang
We can't change share code except formal share code update.
I'll Nack it.

Qiming

> -----Original Message-----
> From: Wang, ShougangX <shougangx.wang@intel.com>
> Sent: Friday, May 8, 2020 15:14
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; Wang, ShougangX
> <shougangx.wang@intel.com>; stable@dpdk.org
> Subject: [PATCH] net/ice: fix incorrect EEPROM data
> 
> Kernel driver reads EEPROM data from flash but DPDK reads from shadow
> ram. This patch fixes the issue by changing method to get EEPROM data from
> flash.
> 
> Fixes: 68a1ab82ad74 ("net/ice: speed up to retrieve EEPROM")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shougang Wang <shougangx.wang@intel.com>
> ---
>  drivers/net/ice/base/ice_nvm.c |  4 ++--  drivers/net/ice/base/ice_nvm.h |
> 3 +++
>  drivers/net/ice/ice_ethdev.c   | 26 +++++++++++---------------
>  3 files changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/ice/base/ice_nvm.c
> b/drivers/net/ice/base/ice_nvm.c index 1bbd6e209..a5f9f8fe6 100644
> --- a/drivers/net/ice/base/ice_nvm.c
> +++ b/drivers/net/ice/base/ice_nvm.c
> @@ -186,7 +186,7 @@ ice_read_sr_buf_aq(struct ice_hw *hw, u16 offset,
> u16 *words, u16 *data)
>   *
>   * This function will request NVM ownership.
>   */
> -static enum ice_status
> +enum ice_status
>  ice_acquire_nvm(struct ice_hw *hw, enum ice_aq_res_access_type access)
> {
>  	ice_debug(hw, ICE_DBG_TRACE, "%s\n", __func__); @@ -203,7
> +203,7 @@ ice_acquire_nvm(struct ice_hw *hw, enum
> ice_aq_res_access_type access)
>   *
>   * This function will release NVM ownership.
>   */
> -static void ice_release_nvm(struct ice_hw *hw)
> +void ice_release_nvm(struct ice_hw *hw)
>  {
>  	ice_debug(hw, ICE_DBG_TRACE, "%s\n", __func__);
> 
> diff --git a/drivers/net/ice/base/ice_nvm.h
> b/drivers/net/ice/base/ice_nvm.h index e5f8888e3..075e7a59f 100644
> --- a/drivers/net/ice/base/ice_nvm.h
> +++ b/drivers/net/ice/base/ice_nvm.h
> @@ -85,6 +85,9 @@ enum ice_status
>  ice_handle_nvm_access(struct ice_hw *hw, struct ice_nvm_access_cmd
> *cmd,
>  		      union ice_nvm_access_data *data);  enum ice_status
> +ice_acquire_nvm(struct ice_hw *hw, enum ice_aq_res_access_type
> access);
> +void ice_release_nvm(struct ice_hw *hw); enum ice_status
>  ice_read_flat_nvm(struct ice_hw *hw, u32 offset, u32 *length, u8 *data,
>  		  bool read_shadow_ram);
>  enum ice_status
> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
> index d5110c439..3d8118e37 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -3951,8 +3951,7 @@ ice_get_eeprom_length(struct rte_eth_dev *dev)
> {
>  	struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> 
> -	/* Convert word count to byte count */
> -	return hw->nvm.sr_words << 1;
> +	return hw->nvm.flash_size;
>  }
> 
>  static int
> @@ -3960,26 +3959,23 @@ ice_get_eeprom(struct rte_eth_dev *dev,
>  	       struct rte_dev_eeprom_info *eeprom)  {
>  	struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> -	uint16_t *data = eeprom->data;
> -	uint16_t first_word, last_word, nwords;
> +	uint8_t *data = eeprom->data;
>  	enum ice_status status = ICE_SUCCESS;
> +	eeprom->magic = hw->vendor_id | (hw->device_id << 16);
> 
> -	first_word = eeprom->offset >> 1;
> -	last_word = (eeprom->offset + eeprom->length - 1) >> 1;
> -	nwords = last_word - first_word + 1;
> -
> -	if (first_word >= hw->nvm.sr_words ||
> -	    last_word >= hw->nvm.sr_words) {
> -		PMD_DRV_LOG(ERR, "Requested EEPROM bytes out of
> range.");
> -		return -EINVAL;
> +	status = ice_acquire_nvm(hw, ICE_RES_READ);
> +	if (status) {
> +		PMD_DRV_LOG(ERR, "acquire nvm failed.");
> +		return -EIO;
>  	}
> 
> -	eeprom->magic = hw->vendor_id | (hw->device_id << 16);
> +	status = ice_read_flat_nvm(hw, eeprom->offset, &eeprom->length,
> +			data, false);
> +
> +	ice_release_nvm(hw);
> 
> -	status = ice_read_sr_buf(hw, first_word, &nwords, data);
>  	if (status) {
>  		PMD_DRV_LOG(ERR, "EEPROM read failed.");
> -		eeprom->length = sizeof(uint16_t) * nwords;
>  		return -EIO;
>  	}
> 
> --
> 2.17.1