Re: [PATCH net v2 07/15] ice: E822: cancel offset verification work during reset preparation

[Jacob Keller <jacob.e.keller@intel.com>]

On 10/12/2026 4:15 PM, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [Medium] The commit message gives a false reason for leaving PF resets
>   out of the new cancellation.
> 
> --
> 
> ice: E822: cancel offset verification work during reset preparation
> 
> This patch cancels the E822 offset verification work (ov_work) in
> ice_ptp_prepare_for_reset() for CORE, GLOBAL and EMP resets, so the
> work does not keep polling while the PHY is restarted. PF resets
> return early and skip the cancel. The commit message says this is
> because nothing else would restart the task after a PF reset.
> 
>> The larger resets will restart the PHY, canceling any outstanding vernier
>> calibration and forcing a restart. Preemptively cancel the task for non-PF
>> resets to avoid doing unnecessary checks while resetting. The task is not
>> canceled for a PF reset, otherwise nothing would restart the task to finish
>> vernier calibration after the reset.
> 
> [ ... ]
> 
>> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
>> index 7f82439c47ee5..7cc151b01a13e 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
>> @@ -3006,6 +3006,9 @@ void ice_ptp_prepare_for_reset(struct ice_pf *pf, enum ice_reset_req reset_type)
>>  	if (reset_type == ICE_RESET_PFR)
>>  		return;
>>  
>> +	if (hw->mac_type == ICE_MAC_GENERIC)
>> +		kthread_cancel_delayed_work_sync(&ptp->port.ov_work);
>> +
> 
> [Severity: Medium]
> Is the statement "nothing would restart the task to finish vernier
> calibration after the reset" accurate for this tree?
> 
> Near the end of ice_rebuild(), ice_ptp_link_change() is called for
> every reset type, PF reset included:
> 
> drivers/net/ethernet/intel/ice/ice_main.c:ice_rebuild() {
>     ...
> 	if (test_bit(ICE_FLAG_PTP_SUPPORTED, pf->flags) && pf->hw.port_info) {
> 		bool link_up;
> 
> 		link_up = !!(pf->hw.port_info->phy.link_info.link_info &
> 			     ICE_AQ_LINK_UP);
> 		ice_ptp_link_change(pf, link_up);
> 	}
>     ...
> }
> 
> ice_misc_intr() sets hw->reset_ongoing only on the OICR path for
> CORER/GLOBR/EMPR. For a PF reset, ice_ptp_link_change() therefore does
> not skip the hardware writes. On ICE_MAC_GENERIC it reaches
> ice_ptp_port_phy_restart(), which cancels ov_work and queues it again:
> 
> drivers/net/ethernet/intel/ice/ice_ptp.c:ice_ptp_port_phy_restart() {
>     ...
> 	case ICE_MAC_GENERIC:
> 		/* Start the PHY timer in Vernier mode */
> 		kthread_cancel_delayed_work_sync(&ptp_port->ov_work);
> 		...
> 		kthread_queue_delayed_work(pf->ptp.kworker, &ptp_port->ov_work,
> 					   0);
> 		break;
>     ...
> }
> 
> When the link is down, the same function calls ice_ptp_port_phy_stop()
> instead, which also cancels ov_work.
> 
> The call chain for a PF reset looks like this:
> 
> ice_do_reset(PFR)->ice_rebuild()->ice_ptp_link_change()
>     ->ice_ptp_port_phy_restart()
> 
> If that is right, cancelling ov_work for PF resets would also be safe.
> With the early ICE_RESET_PFR return placed before the new cancel, PF
> resets still go through the 100 ms reschedule loop in
> ice_ptp_wait_for_offsets(), because ICE_PFR_REQ makes
> ice_is_reset_in_progress() return true.
> 
> Could the cancel be moved above the PFR early return? If not, could the
> commit message give the actual reason for exempting PF resets?
> 

I can move the cancel and update the commit message.

>>  	if (ice_pf_src_tmr_owned(pf) && hw->mac_type == ICE_MAC_GENERIC_3K_E825)
>>  		ice_ptp_prepare_rebuild_sec(pf, false, reset_type);
>>
>