Re: [PATCH v2 2/2] e1000e: ignore factory-default checksum value on TGP platform

[Vlad URSU <vlad@ursu.me>]

On 23.06.2025 19:18, Jacek Kowalski wrote:
> Vlad,
> 
> could you verify that the following patch works for you?
> 
>> diff --git a/drivers/net/ethernet/intel/e1000e/defines.h b/drivers/net/ethernet/intel/e1000e/defines.h
>> index 8294a7c4f122..01696eb8dace 100644
>> --- a/drivers/net/ethernet/intel/e1000e/defines.h
>> +++ b/drivers/net/ethernet/intel/e1000e/defines.h
>> @@ -637,6 +637,7 @@
>>   
>>   /* For checksumming, the sum of all words in the NVM should equal 0xBABA. */
>>   #define NVM_SUM                    0xBABA
>> +#define NVM_SUM_FACTORY_DEFAULT    0xFFFF
>>   
>>   /* PBA (printed board assembly) number words */
>>   #define NVM_PBA_OFFSET_0           8
>> diff --git a/drivers/net/ethernet/intel/e1000e/nvm.c b/drivers/net/ethernet/intel/e1000e/nvm.c
>> index e609f4df86f4..37cbf9236d84 100644
>> --- a/drivers/net/ethernet/intel/e1000e/nvm.c
>> +++ b/drivers/net/ethernet/intel/e1000e/nvm.c
>> @@ -558,6 +558,11 @@ s32 e1000e_validate_nvm_checksum_generic(struct e1000_hw *hw)
>>   		checksum += nvm_data;
>>   	}
>>   
>> +	if (hw->mac.type == e1000_pch_tgp && checksum == (u16)NVM_SUM_FACTORY_DEFAULT) {
>> +		e_dbg("Factory-default NVM Checksum on TGP platform - ignoring\n");
>> +		return 0;
>> +	}
>> +
>>   	if (checksum != (u16)NVM_SUM) {
>>   		e_dbg("NVM Checksum Invalid\n");
>>   		return -E1000_ERR_NVM;
> 

No, it doesn't.

You are comparing the wrong value with NVM_SUM_FACTORY_DEFAULT. You 
should check it against the checksum word 0x3F (NVM bytes 0x7E and 0x7F) 
which is used to ensure that the base NVM image
is a valid image, and which in my case is left unchanged by Dell in the 
firmware.

I believe the changes should look something like this:

---
  drivers/net/ethernet/intel/e1000e/defines.h |  3 +++
  drivers/net/ethernet/intel/e1000e/nvm.c     | 13 ++++++++++++-
  2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000e/defines.h 
b/drivers/net/ethernet/intel/e1000e/defines.h
index 8294a7c4f122..996a0f4d2b49 100644
--- a/drivers/net/ethernet/intel/e1000e/defines.h
+++ b/drivers/net/ethernet/intel/e1000e/defines.h
@@ -638,6 +638,9 @@
  /* For checksumming, the sum of all words in the NVM should equal 
0xBABA. */
  #define NVM_SUM                    0xBABA

+/* Factory default value for the NVM checksum word */
+#define NVM_CHECKSUM_WORD_FACTORY_DEFAULT       0xFFFF
+
  /* PBA (printed board assembly) number words */
  #define NVM_PBA_OFFSET_0           8
  #define NVM_PBA_OFFSET_1           9
diff --git a/drivers/net/ethernet/intel/e1000e/nvm.c 
b/drivers/net/ethernet/intel/e1000e/nvm.c
index e609f4df86f4..4620efac0208 100644
--- a/drivers/net/ethernet/intel/e1000e/nvm.c
+++ b/drivers/net/ethernet/intel/e1000e/nvm.c
@@ -547,7 +547,18 @@ s32 e1000e_validate_nvm_checksum_generic(struct 
e1000_hw *hw)
  {
  	s32 ret_val;
  	u16 checksum = 0;
-	u16 i, nvm_data;
+	u16 i, nvm_data, checksum_word;
+
+	ret_val = e1000_read_nvm(hw, NVM_CHECKSUM_REG, 1, &checksum_word);
+	if (ret_val) {
+		e_dbg("NVM Read Error\n");
+		return ret_val;
+	}
+
+	if (hw->mac.type == e1000_pch_tgp && checksum_word == 
(u16)NVM_CHECKSUM_WORD_FACTORY_DEFAULT) {
+		e_dbg("Factory-default NVM Checksum word on TGP platform - ignoring\n");
+		return 0;
+	}

  	for (i = 0; i < (NVM_CHECKSUM_REG + 1); i++) {
  		ret_val = e1000_read_nvm(hw, i, 1, &nvm_data);
--