> -----Original Message-----
> From: Nitka, Grzegorz <[email protected]>
> Sent: Tuesday, July 28, 2026 11:13 AM
> To: [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; Kitszel, Przemyslaw
> <[email protected]>; Nguyen, Anthony L
> <[email protected]>; Kubalewski, Arkadiusz
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; Nitka, Grzegorz
> <[email protected]>; Korba, Przemyslaw
> <[email protected]>
> Subject: [PATCH iwl-next 2/3] ice: add TSPLL DPLL device and TIME_REF pin
> for E825
>
> This extends the E825 advanced sync-timing support introduced by the
> tx-clk series, which added the TXC DPLL device for TX reference clock
> control. The TSPLL, the source timer PLL, is now also exposed through
> the dpll subsystem so that its lock status and clock source selection
> are visible and controllable from userspace.
>
> On E825 devices the TSPLL is the source timer PLL, distinct from the
> EEC and PPS DPLLs used on E810. Register it as a DPLL_TYPE_GENERIC
> device for owner PFs.
>
> Add struct ice_dplls::tspll_in, a fwnode-backed input pin named
> "time_ref". The state_on_dpll_get callback queries ICE_CGU_R23 via
> ice_tspll_get_clk_src() and returns CONNECTED when TIME_REF is
> selected as clock source, DISCONNECTED otherwise. The state_on_dpll_set
> callback switches the source between TIME_REF and TCXO via the new
> ice_tspll_set_cfg() helper. Registration is deferred via the dpll
> notifier path if the pin is not yet visible in the subsystem at probe
> time.
>
> Initialize TSPLL DPLL state from direct clock-source/lock reads so the
> first published state reflects hardware and prev_dpll_state matches.
> During periodic polling, the DPLL worker consumes
> READ_ONCE(pf->ptp.tspll_locked), maintained and recovered by the PTP
> periodic worker. When the TSPLL clock source is TCXO (TIME_REF pin not
> selected), UNLOCKED is reported unconditionally to reflect the
> free-running state of the oscillator regardless of the raw lock bit.
> To avoid stale lock-status reads after synchronous source changes, the
> set callback now refreshes tspll.dpll_state immediately and emits a DPLL
> change notification when the cached state changed.
>
> If a TSPLL reconfiguration is applied but the PLL has not yet
> re-acquired lock, treat the internal -EAGAIN result as success so the
> PTP periodic worker can complete recovery, while real -EBUSY failures
> from reset/SBQ paths still propagate to userspace.
>
> Extend ice_dpll_deinit_txclk_pins() with a "flush" parameter so the
> E825 init error path for the TSPLL fwnode pin can tear down TXCLK
> pins without flushing pf->dplls.wq. If the flush ran here, notifier
> work items queued during earlier init steps would be blocked on
> pf->dplls.dpll_init, which is only completed at the unregister_pins
> label reached after this teardown. destroy_workqueue() at that label
> drains the queued items safely. Existing full-teardown callers pass
> flush=true and keep current behavior.
>
> Reviewed-by: Przemyslaw Korba <[email protected]>
> Signed-off-by: Grzegorz Nitka <[email protected]>
> ---
> drivers/net/ethernet/intel/ice/ice_dpll.c | 398 +++++++++++++++++++--
> drivers/net/ethernet/intel/ice/ice_dpll.h | 4 +
> drivers/net/ethernet/intel/ice/ice_ptp.c | 10 +
> drivers/net/ethernet/intel/ice/ice_tspll.c | 34 +-
> drivers/net/ethernet/intel/ice/ice_tspll.h | 2 +
> 5 files changed, 409 insertions(+), 39 deletions(-)
>
[...]
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c
> b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index a997be5f7d8f..cbd9f6455c05 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -2898,6 +2898,7 @@ static void
> ice_ptp_maybe_trigger_tx_interrupt(struct ice_pf *pf)
> static void ice_ptp_tspll_monitor(struct ice_pf *pf)
> {
> bool lock_lost;
> + bool dpll_ready;
> int err;
>
> if (pf->hw.mac_type != ICE_MAC_GENERIC_3K_E825 ||
> @@ -2917,7 +2918,16 @@ static void ice_ptp_tspll_monitor(struct ice_pf
> *pf)
> if (!(pf->ptp.tspll_lock_retries % ICE_TSPLL_LOG_INTERVAL))
> dev_warn(ice_pf_to_dev(pf),
> "TimeSync PLL lock lost. Retrying to acquire
> lock.\n");
> + /* Serialize R23 restart against TSPLL userspace reconfig.
> + * Guard with ICE_FLAG_DPLL because init failure may
> already
> + * have destroyed pf->dplls.lock on the error path.
> + */
> + dpll_ready = test_bit(ICE_FLAG_DPLL, pf->flags);
> + if (dpll_ready)
> + mutex_lock(&pf->dplls.lock);
> err = ice_tspll_restart_e825c(&pf->hw);
> + if (dpll_ready)
> + mutex_unlock(&pf->dplls.lock);
> if (err)
> dev_err_ratelimited(ice_pf_to_dev(pf),
> "Failed to restart TimeSync PLL
> (err:
> %d).\n",
About Sashiko comment from:
https://sashiko.dev/#/patchset/20260728091314.1420656-1-grzegorz.nitka%40intel.com
"Does this code introduce a TOCTOU race leading to hardware state corruption?
During initialization in ice_dpll_init_e825(), there is a window where the
TSPLL DPLL device and pins are fully registered and exposed to userspace
before ICE_FLAG_DPLL is set.
If userspace reconfigures the TSPLL during this window,
ice_dpll_tspll_state_on_dpll_set() (in
drivers/net/ethernet/intel/ice/ice_dpll.c)
will acquire pf->dplls.lock and modify the hardware."
In general, this concern is valid however limited only to very narrow window at
the driver initialization. I think the best way to fix it is to move dpll.lock
init/destroy outside of dpll_init/deinit procedures and handle it in the driver
probe/remove routines. This way, the flag guard is eliminated, so the monitor
code becomes straightforward: "take the mutex, restart the PLL, drop the mutex."
[...]
> --
> 2.39.3
