Re: [PATCH net 2/5] e1000e: do not cache the hwtstamp config before it is applied

[Linkui Xiao <xiaolinkui@126.com>]

Hi Jake,

Thank you for applying the patch and for the review.

On 2026/11/3 06:15, Jacob Keller wrote:
> On 10/28/2026 2:04 AM, Linkui Xiao wrote:
>> From: Linkui Xiao <xiaolinkui@kylinos.cn>
>>
>> e1000e_config_hwtstamp() stores the requested configuration in
>> adapter->hwtstamp_config right after the rx filter has been
>> translated, but the two register writes that follow are verified by
>> reading the registers back and each of them can fail with -EAGAIN.
>>
>> When one of them does, the driver is left holding a configuration the
>> hardware never accepted:
>>
>>   - e1000e_hwtstamp_get() hands it back to user space, so an
>>     application that reads the configuration back is told that
>>     hardware time stamping is in effect while it is not;
>>
>>   - e1000e_systim_reset() re-applies it after every reset, including
>>     the ones the watchdog triggers, and keeps failing;
>>
>>   - e1000_watchdog() starts polling TSYNCRXCTL_VALID for the cached rx
>>     filter, and e1000_xmit_frame() still parks skbs that ask for a Tx
>>     time stamp because it only looks at FLAG_HAS_HW_TIMESTAMP, so they
>>     are eventually dropped by e1000e_tx_hwtstamp_work() with a
>>     "clearing Tx timestamp hang" warning.
>>
>> Cache the configuration only once both registers have been verified,
>> so that adapter->hwtstamp_config always describes what the hardware
>> is programmed with.
>>
> 
> Any idea why this got marked as patch 2 out of 5? This doesn't appear on
> either netdev or iwl with any other patches.
Regarding your question about the "2/5" marking: it was an oversight on 
my end. I generated this fix from a local branch that contained other 
commits, and `git format-patch` automatically added the series 
numbering. The patch is indeed a standalone fix and was not intended to 
be part of a series posted elsewhere.

I'll make sure to use a proper invocation for single patches going forward.

Thanks again for your time and for taking this fix into the Intel Wired 
LAN tree.

Best regards,
Linkui
> 
> I'm going ahead and applying it to Intel Wired LAN regardless, since the
> fix looks good.
> 
> Thanks,
> Jake
> 
>> Fixes: 62d7e3a2d006 ("e1000e: Validate hwtstamp_config completely before applying it")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
>> ---
>>   drivers/net/ethernet/intel/e1000e/netdev.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
>> index 35f5590228eb..7ae33273734f 100644
>> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
>> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
>> @@ -3715,8 +3715,6 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter,
>>   		return -ERANGE;
>>   	}
>>   
>> -	adapter->hwtstamp_config = *config;
>> -
>>   	/* enable/disable Tx h/w time stamping */
>>   	regval = er32(TSYNCTXCTL);
>>   	regval &= ~E1000_TSYNCTXCTL_ENABLED;
>> @@ -3763,6 +3761,8 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter,
>>   	er32(RXSTMPH);
>>   	er32(TXSTMPH);
>>   
>> +	adapter->hwtstamp_config = *config;
>> +
>>   	return 0;
>>   }
>>