> -----Original Message-----
> From: Nguyen, Anthony L <[email protected]>
> Sent: Wednesday, May 27, 2026 11:22 PM
> To: Korba, Przemyslaw <[email protected]>;
> [email protected]
> Cc: [email protected]; Kitszel, Przemyslaw
> <[email protected]>; Loktionov, Aleksandr
> <[email protected]>;
> Kubalewski, Arkadiusz <[email protected]>
> Subject: Re: [PATCH iwl-net] ice: fall back to SBQ when LL PHY timer
> interface times out
>
>
>
> On 5/11/2026 2:58 AM, Przemyslaw Korba wrote:
> > The low-latency (LL) PHY timer interface relies on a tight, atomic poll
> > of the PF_SB_ATQBAL register with a 2ms timeout. After an NVM update /
> > EMPR, FW may need significantly longer than 2ms to start responding to
> > ATQBAL commands. The first PHY adjust or incval write issued by
> > ice_ptp_rebuild_owner() fails with -ETIMEDOUT.
> >
> > Fix this by falling back to the existing SBQ-based PHY register write
> > path when LL times out. This makes sure PTP is initialized when FW takes
> > longer than expected to come back online.
> >
> > Steps to reproduce:
> > ./nvmupdate64e -if devlink -f
> > Update E810 card with nvmupdate64e, and observe dmesg errors:
> > Failed to write PHC increment value, status -110
> > PTP reset failed, error: -110 (-ETIMEDOUT)
> >
> > Fixes: ef9a64c07294 ("ice: implement low latency PHY timer updates")
> > Signed-off-by: Przemyslaw Korba <[email protected]>
> > ---
> > drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 40 +++++++++++----------
> > 1 file changed, 22 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> > b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> > index 2c18e16fe053..9cd323bd9739 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> > @@ -4771,15 +4771,12 @@ static int ice_ptp_prep_phy_adj_ll_e810(struct
> > ice_hw *hw, s32 adj)
> > !FIELD_GET(REG_LL_PROXY_H_EXEC, val),
> > 10, REG_LL_PROXY_H_TIMEOUT_US, false, hw,
> > REG_LL_PROXY_H);
> > - if (err) {
> > - ice_debug(hw, ICE_DBG_PTP, "Failed to prepare PHY timer
> > adjustment using low latency interface\n");
> > - spin_unlock_irq(¶ms->atqbal_wq.lock);
> > - return err;
> > - }
> > -
> > spin_unlock_irq(¶ms->atqbal_wq.lock);
> >
> > - return 0;
> > + if (err)
> > + ice_debug(hw, ICE_DBG_PTP, "Failed to prepare PHY timer
> > adjustment using low latency interface\n");
> > +
> > + return err;
>
> From Sashiko:
>
> If read_poll_timeout_atomic() times out, the REG_LL_PROXY_H_EXEC bit
> remains set, meaning the command is still pending in the firmware queue.
>
> Since the commit message notes the firmware may just be delayed rather
> than failed, will the firmware eventually process this pending command
> and overwrite the shadow registers?
>
> If the delayed firmware execution completes after the SBQ fallback has
> already applied the update, could this regression cause silent PTP clock
> corruption on a subsequent synchronization?
>
Hi Tony, thanks for review!
Both LL and SBQ paths write the same incval/adj value to the PHY shadow
registers.
How I see it is that even if this case happens , and FW eventually processes
the timed-out LL command,
it writes the same value that SBQ already wrote so no corruption there.
> > @@ -4890,8 +4889,13 @@ static int ice_ptp_prep_phy_incval_e810(struct
> > ice_hw *hw, u64 incval)
> > u8 tmr_idx;
> > int err;
> >
> > - if (hw->dev_caps.ts_dev_info.ll_phy_tmr_update)
> > - return ice_ptp_prep_phy_incval_ll_e810(hw, incval);
> > + if (hw->dev_caps.ts_dev_info.ll_phy_tmr_update) {
> > + err = ice_ptp_prep_phy_incval_ll_e810(hw, incval);
> > + if (!err)
> > + return 0;
>
> Looking at ice_ptp_prep_phy_adj_ll_e810(), it can return -ERESTARTSYS if
> the wait_event_interruptible_locked_irq() is interrupted by a signal.
>
> Does this unconditional fallback catch the -ERESTARTSYS return value and
> inadvertently continue with the blocking SBQ path, rather than
> propagating the signal interruption to userspace?
>
> Should this fallback only be triggered when err is -ETIMEDOUT?
>
I agree, let me send v2
Again, thanks for review 😊
> > + ice_debug(hw, ICE_DBG_PTP, "LL incval failed (%d), falling back
> > to SBQ\n",
> > + err);
> > + }
> >
> > tmr_idx = hw->func_caps.ts_func_info.tmr_index_owned;
> > low = lower_32_bits(incval);
> >
> > base-commit: 80b47e88f7ead00b0795e9f2833f1d0cafe11d90
