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

[Vlad URSU]

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);
--