diff mbox series

[net-next,4/5] i40e: Refactor and rename i40e_read_pba_string()

Message ID 20231013170755.2367410-5-ivecera@redhat.com (mailing list archive)
State Accepted
Commit df19ea696644bea14dc2b804066a6b91aa4aaf43
Delegated to: Netdev Maintainers
Headers show
Series i40e: Add basic devlink support | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1360 this patch: 1360
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 1385 this patch: 1385
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1385 this patch: 1385
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 121 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Ivan Vecera Oct. 13, 2023, 5:07 p.m. UTC 
Function i40e_read_pba_string() is currently unused but will be used
by subsequent patch to provide board ID via devlink device info.

The function reads PBA block from NVM so it cannot be called during
adapter reset and as we would like to provide PBA ID via devlink
info it is better to read the PBA ID during i40e_probe() and cache
it in i40e_hw structure to avoid a waiting for potential adapter
reset in devlink info callback.

So...
- Remove pba_num and pba_num_size arguments from the function,
  allocate resource managed buffer to store PBA ID string and
  save resulting pointer to i40e_hw->pba_id field
- Make the function void as the PBA ID can be missing and in this
  case (or in case of NVM reading failure) the i40e_hw->pba_id
  will be NULL
- Rename the function to i40e_get_pba_string() to align with other
  functions like i40e_get_oem_version() i40e_get_port_mac_addr()...
- Call this function on init during i40e_probe()

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/net/ethernet/intel/i40e/i40e_common.c | 58 +++++++++++--------
 drivers/net/ethernet/intel/i40e/i40e_main.c   |  1 +
 .../net/ethernet/intel/i40e/i40e_prototype.h  |  3 +-
 drivers/net/ethernet/intel/i40e/i40e_type.h   |  3 +
 4 files changed, 39 insertions(+), 26 deletions(-)

Comments

Jacob Keller Oct. 17, 2023, 5:21 p.m. UTC | #1 
On 10/13/2023 10:07 AM, Ivan Vecera wrote:
> Function i40e_read_pba_string() is currently unused but will be used
> by subsequent patch to provide board ID via devlink device info.
> 
> The function reads PBA block from NVM so it cannot be called during
> adapter reset and as we would like to provide PBA ID via devlink
> info it is better to read the PBA ID during i40e_probe() and cache
> it in i40e_hw structure to avoid a waiting for potential adapter
> reset in devlink info callback.
> 
> So...
> - Remove pba_num and pba_num_size arguments from the function,
>   allocate resource managed buffer to store PBA ID string and
>   save resulting pointer to i40e_hw->pba_id field
> - Make the function void as the PBA ID can be missing and in this
>   case (or in case of NVM reading failure) the i40e_hw->pba_id
>   will be NULL
> - Rename the function to i40e_get_pba_string() to align with other
>   functions like i40e_get_oem_version() i40e_get_port_mac_addr()...
> - Call this function on init during i40e_probe()
> 

And the PBA value shouldn't be changing even with a new NVM image
flashed to the device, so its not something that is likely to have
updated at run time, thus saving during probe is sufficient.

Makes sense.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_common.c | 58 +++++++++++--------
>  drivers/net/ethernet/intel/i40e/i40e_main.c   |  1 +
>  .../net/ethernet/intel/i40e/i40e_prototype.h  |  3 +-
>  drivers/net/ethernet/intel/i40e/i40e_type.h   |  3 +
>  4 files changed, 39 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
> index 6d1042ca0317..04db9cdc7d94 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_common.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
> @@ -821,62 +821,72 @@ void i40e_pre_tx_queue_cfg(struct i40e_hw *hw, u32 queue, bool enable)
>  }
>  
>  /**
> - *  i40e_read_pba_string - Reads part number string from EEPROM
> + *  i40e_get_pba_string - Reads part number string from EEPROM
>   *  @hw: pointer to hardware structure
> - *  @pba_num: stores the part number string from the EEPROM
> - *  @pba_num_size: part number string buffer length
>   *
> - *  Reads the part number string from the EEPROM.
> + *  Reads the part number string from the EEPROM and stores it
> + *  into newly allocated buffer and saves resulting pointer
> + *  to i40e_hw->pba_id field.
>   **/
> -int i40e_read_pba_string(struct i40e_hw *hw, u8 *pba_num,
> -			 u32 pba_num_size)
> +void i40e_get_pba_string(struct i40e_hw *hw)
>  {
> +#define I40E_NVM_PBA_FLAGS_BLK_PRESENT	0xFAFA
>  	u16 pba_word = 0;
>  	u16 pba_size = 0;
>  	u16 pba_ptr = 0;
> -	int status = 0;
> -	u16 i = 0;
> +	int status;
> +	char *ptr;
> +	u16 i;
>  
>  	status = i40e_read_nvm_word(hw, I40E_SR_PBA_FLAGS, &pba_word);
> -	if (status || (pba_word != 0xFAFA)) {
> -		hw_dbg(hw, "Failed to read PBA flags or flag is invalid.\n");
> -		return status;
> +	if (status) {
> +		hw_dbg(hw, "Failed to read PBA flags.\n");
> +		return;
> +	}
> +	if (pba_word != I40E_NVM_PBA_FLAGS_BLK_PRESENT) {
> +		hw_dbg(hw, "PBA block is not present.\n");
> +		return;
>  	}
>  
>  	status = i40e_read_nvm_word(hw, I40E_SR_PBA_BLOCK_PTR, &pba_ptr);
>  	if (status) {
>  		hw_dbg(hw, "Failed to read PBA Block pointer.\n");
> -		return status;
> +		return;
>  	}
>  
>  	status = i40e_read_nvm_word(hw, pba_ptr, &pba_size);
>  	if (status) {
>  		hw_dbg(hw, "Failed to read PBA Block size.\n");
> -		return status;
> +		return;
>  	}
>  
>  	/* Subtract one to get PBA word count (PBA Size word is included in
> -	 * total size)
> +	 * total size) and advance pointer to first PBA word.
>  	 */
>  	pba_size--;
> -	if (pba_num_size < (((u32)pba_size * 2) + 1)) {
> -		hw_dbg(hw, "Buffer too small for PBA data.\n");
> -		return -EINVAL;
> +	pba_ptr++;
> +	if (!pba_size) {
> +		hw_dbg(hw, "PBA ID is empty.\n");
> +		return;
>  	}
>  
> +	ptr = devm_kzalloc(i40e_hw_to_dev(hw), pba_size * 2 + 1, GFP_KERNEL);
> +	if (!ptr)
> +		return;
> +	hw->pba_id = ptr;
> +
>  	for (i = 0; i < pba_size; i++) {
> -		status = i40e_read_nvm_word(hw, (pba_ptr + 1) + i, &pba_word);
> +		status = i40e_read_nvm_word(hw, pba_ptr + i, &pba_word);
>  		if (status) {
>  			hw_dbg(hw, "Failed to read PBA Block word %d.\n", i);
> -			return status;
> +			devm_kfree(i40e_hw_to_dev(hw), hw->pba_id);
> +			hw->pba_id = NULL;
> +			return;
>  		}
>  
> -		pba_num[(i * 2)] = (pba_word >> 8) & 0xFF;
> -		pba_num[(i * 2) + 1] = pba_word & 0xFF;
> +		*ptr++ = (pba_word >> 8) & 0xFF;
> +		*ptr++ = pba_word & 0xFF;
>  	}
> -	pba_num[(pba_size * 2)] = '\0';
> -
> -	return status;
>  }
>  
>  /**
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index ba8fb0556216..3157d14d9b12 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -15846,6 +15846,7 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  		goto err_pf_reset;
>  	}
>  	i40e_get_oem_version(hw);
> +	i40e_get_pba_string(hw);
>  
>  	/* provide nvm, fw, api versions, vendor:device id, subsys vendor:device id */
>  	i40e_nvm_version_str(hw, nvm_ver, sizeof(nvm_ver));
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_prototype.h b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
> index 46b9a05ceb91..001162042050 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_prototype.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
> @@ -341,8 +341,7 @@ i40e_aq_configure_partition_bw(struct i40e_hw *hw,
>  			       struct i40e_aqc_configure_partition_bw_data *bw_data,
>  			       struct i40e_asq_cmd_details *cmd_details);
>  int i40e_get_port_mac_addr(struct i40e_hw *hw, u8 *mac_addr);
> -int i40e_read_pba_string(struct i40e_hw *hw, u8 *pba_num,
> -			 u32 pba_num_size);
> +void i40e_get_pba_string(struct i40e_hw *hw);
>  void i40e_pre_tx_queue_cfg(struct i40e_hw *hw, u32 queue, bool enable);
>  /* prototype for functions used for NVM access */
>  int i40e_init_nvm(struct i40e_hw *hw);
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h
> index dc7cd16ad8fb..aff6dc6afbe2 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_type.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_type.h
> @@ -493,6 +493,9 @@ struct i40e_hw {
>  	struct i40e_nvm_info nvm;
>  	struct i40e_fc_info fc;
>  
> +	/* PBA ID */
> +	const char *pba_id;
> +
>  	/* pci info */
>  	u16 device_id;
>  	u16 vendor_id;
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
index 6d1042ca0317..04db9cdc7d94 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -821,62 +821,72 @@  void i40e_pre_tx_queue_cfg(struct i40e_hw *hw, u32 queue, bool enable)
 }
 
 /**
- *  i40e_read_pba_string - Reads part number string from EEPROM
+ *  i40e_get_pba_string - Reads part number string from EEPROM
  *  @hw: pointer to hardware structure
- *  @pba_num: stores the part number string from the EEPROM
- *  @pba_num_size: part number string buffer length
  *
- *  Reads the part number string from the EEPROM.
+ *  Reads the part number string from the EEPROM and stores it
+ *  into newly allocated buffer and saves resulting pointer
+ *  to i40e_hw->pba_id field.
  **/
-int i40e_read_pba_string(struct i40e_hw *hw, u8 *pba_num,
-			 u32 pba_num_size)
+void i40e_get_pba_string(struct i40e_hw *hw)
 {
+#define I40E_NVM_PBA_FLAGS_BLK_PRESENT	0xFAFA
 	u16 pba_word = 0;
 	u16 pba_size = 0;
 	u16 pba_ptr = 0;
-	int status = 0;
-	u16 i = 0;
+	int status;
+	char *ptr;
+	u16 i;
 
 	status = i40e_read_nvm_word(hw, I40E_SR_PBA_FLAGS, &pba_word);
-	if (status || (pba_word != 0xFAFA)) {
-		hw_dbg(hw, "Failed to read PBA flags or flag is invalid.\n");
-		return status;
+	if (status) {
+		hw_dbg(hw, "Failed to read PBA flags.\n");
+		return;
+	}
+	if (pba_word != I40E_NVM_PBA_FLAGS_BLK_PRESENT) {
+		hw_dbg(hw, "PBA block is not present.\n");
+		return;
 	}
 
 	status = i40e_read_nvm_word(hw, I40E_SR_PBA_BLOCK_PTR, &pba_ptr);
 	if (status) {
 		hw_dbg(hw, "Failed to read PBA Block pointer.\n");
-		return status;
+		return;
 	}
 
 	status = i40e_read_nvm_word(hw, pba_ptr, &pba_size);
 	if (status) {
 		hw_dbg(hw, "Failed to read PBA Block size.\n");
-		return status;
+		return;
 	}
 
 	/* Subtract one to get PBA word count (PBA Size word is included in
-	 * total size)
+	 * total size) and advance pointer to first PBA word.
 	 */
 	pba_size--;
-	if (pba_num_size < (((u32)pba_size * 2) + 1)) {
-		hw_dbg(hw, "Buffer too small for PBA data.\n");
-		return -EINVAL;
+	pba_ptr++;
+	if (!pba_size) {
+		hw_dbg(hw, "PBA ID is empty.\n");
+		return;
 	}
 
+	ptr = devm_kzalloc(i40e_hw_to_dev(hw), pba_size * 2 + 1, GFP_KERNEL);
+	if (!ptr)
+		return;
+	hw->pba_id = ptr;
+
 	for (i = 0; i < pba_size; i++) {
-		status = i40e_read_nvm_word(hw, (pba_ptr + 1) + i, &pba_word);
+		status = i40e_read_nvm_word(hw, pba_ptr + i, &pba_word);
 		if (status) {
 			hw_dbg(hw, "Failed to read PBA Block word %d.\n", i);
-			return status;
+			devm_kfree(i40e_hw_to_dev(hw), hw->pba_id);
+			hw->pba_id = NULL;
+			return;
 		}
 
-		pba_num[(i * 2)] = (pba_word >> 8) & 0xFF;
-		pba_num[(i * 2) + 1] = pba_word & 0xFF;
+		*ptr++ = (pba_word >> 8) & 0xFF;
+		*ptr++ = pba_word & 0xFF;
 	}
-	pba_num[(pba_size * 2)] = '\0';
-
-	return status;
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index ba8fb0556216..3157d14d9b12 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -15846,6 +15846,7 @@  static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto err_pf_reset;
 	}
 	i40e_get_oem_version(hw);
+	i40e_get_pba_string(hw);
 
 	/* provide nvm, fw, api versions, vendor:device id, subsys vendor:device id */
 	i40e_nvm_version_str(hw, nvm_ver, sizeof(nvm_ver));
diff --git a/drivers/net/ethernet/intel/i40e/i40e_prototype.h b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
index 46b9a05ceb91..001162042050 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_prototype.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
@@ -341,8 +341,7 @@  i40e_aq_configure_partition_bw(struct i40e_hw *hw,
 			       struct i40e_aqc_configure_partition_bw_data *bw_data,
 			       struct i40e_asq_cmd_details *cmd_details);
 int i40e_get_port_mac_addr(struct i40e_hw *hw, u8 *mac_addr);
-int i40e_read_pba_string(struct i40e_hw *hw, u8 *pba_num,
-			 u32 pba_num_size);
+void i40e_get_pba_string(struct i40e_hw *hw);
 void i40e_pre_tx_queue_cfg(struct i40e_hw *hw, u32 queue, bool enable);
 /* prototype for functions used for NVM access */
 int i40e_init_nvm(struct i40e_hw *hw);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h
index dc7cd16ad8fb..aff6dc6afbe2 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_type.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_type.h
@@ -493,6 +493,9 @@  struct i40e_hw {
 	struct i40e_nvm_info nvm;
 	struct i40e_fc_info fc;
 
+	/* PBA ID */
+	const char *pba_id;
+
 	/* pci info */
 	u16 device_id;
 	u16 vendor_id;