[PATCH net v2 1/2] sfc: ptp: serialize host and MC synchronization transactions
flat view
DORMANTno replies
REVIEWED: 3 (1M)
From: Alex Austin <hidden>
Date: 2026-10-15 11:41:44
Also in:
lkml
Subsystem:
networking drivers, sfc network driver, the rest · Maintainers:
Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Edward Cree, Linus Torvalds
Revision v2 of 2 in this series; 3 review trailers (1 from subsystem maintainers).
Revisions (2)
- v1 [diff vs current]
- v2 current
efx_ptp_synchronize() clears the MC-ready DMA flag before
efx_mcdi_rpc_start() acquires the MCDI interface, and processes results
into ptp->timeset[] after efx_mcdi_rpc_finish() has released it. The PPS
worker and the hwtstamp enable path can therefore clobber each other's
handshake flag and timesets.
Hold a PHC mutex across the whole transaction and over the clock
operations, so efx_phc_settime() reads and adjusts atomically and
current_adjfreq cannot be torn by the reset in efx_ptp_start(). Also
bail out when efx_mcdi_rpc_start() fails rather than finishing an RPC
that was never acquired.
Fixes: 7c236c43b838 ("sfc: Add support for IEEE-1588 PTP")
Signed-off-by: Alex Austin <redacted>
Reviewed-by: Alejandro Lucero <redacted>
Reviewed-by: Pieter Jansen van Vuuren <redacted>
Acked-by: Edward Cree <ecree.xilinx@gmail.com>
---
drivers/net/ethernet/sfc/ptp.c | 83 +++++++++++++++++++++++-----------
1 file changed, 57 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 6ca9a75..c2f8b94 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c@@ -36,6 +36,7 @@ #include <linux/errno.h> #include <linux/ktime.h> #include <linux/module.h> +#include <linux/mutex.h> #include <linux/pps_kernel.h> #include <linux/ptp_clock_kernel.h> #include "net_driver.h"
@@ -272,6 +273,8 @@ struct efx_ptp_rxfilter { * frequency adjustment into a fixed point fractional nanosecond format. * @current_adjfreq: Current ppb adjustment. * @phc_clock: Pointer to registered phc device (if primary function) + * @phc_lock: Serializes PHC commands and synchronization, and protects + * @current_adjfreq * @phc_clock_info: Registration structure for phc device * @pps_work: pps work task for handling pps events * @pps_workwq: pps work queue
@@ -331,6 +334,7 @@ struct efx_ptp_data { unsigned int adjfreq_ppb_shift; s64 current_adjfreq; struct ptp_clock *phc_clock; + struct mutex phc_lock; struct ptp_clock_info phc_clock_info; struct work_struct pps_work; struct workqueue_struct *pps_workwq;
@@ -1008,7 +1012,14 @@ efx_ptp_process_times(struct efx_nic *efx, MCDI_DECLARE_STRUCT_PTR(synch_buf), return 0; } -/* Synchronize times between the host and the MC */ +/* Synchronize times between the host and the MC. + * + * The MC serializes PTP requests, so MC_CMD_PTP_OP_ADJUST cannot be applied + * while MC_CMD_PTP_OP_SYNCHRONIZE is collecting timesets. This holds across + * functions of a controller, which share the clock but adjust it only through + * the function that registered it, so a secondary function can capture here + * without serializing against the primary. + */ static int efx_ptp_synchronize(struct efx_nic *efx, unsigned int num_readings) { struct efx_ptp_data *ptp = efx->ptp_data;
@@ -1027,11 +1038,14 @@ static int efx_ptp_synchronize(struct efx_nic *efx, unsigned int num_readings) MCDI_SET_QWORD(synch_buf, PTP_IN_SYNCHRONIZE_START_ADDR, ptp->start.dma_addr); + mutex_lock(&ptp->phc_lock); + /* Clear flag that signals MC ready */ WRITE_ONCE(*start, 0); rc = efx_mcdi_rpc_start(efx, MC_CMD_PTP, synch_buf, MC_CMD_PTP_IN_SYNCHRONIZE_LEN); - EFX_WARN_ON_ONCE_PARANOID(rc); + if (rc) + goto out; /* Wait for start from MCDI (or timeout) */ timeout = jiffies + msecs_to_jiffies(MAX_SYNCHRONISE_WAIT_MS);
@@ -1062,11 +1076,13 @@ static int efx_ptp_synchronize(struct efx_nic *efx, unsigned int num_readings) ++ptp->no_time_syncs; } +out: /* Increment the bad syncs counter if the synchronize fails, whatever * the reason. */ if (rc != 0) ++ptp->bad_syncs; + mutex_unlock(&ptp->phc_lock); return rc; }
@@ -1446,7 +1462,9 @@ static int efx_ptp_start(struct efx_nic *efx) goto fail; ptp->evt_frag_idx = 0; + mutex_lock(&ptp->phc_lock); ptp->current_adjfreq = 0; + mutex_unlock(&ptp->phc_lock); return 0;
@@ -1570,6 +1588,7 @@ int efx_ptp_probe(struct efx_nic *efx, struct efx_channel *channel) if (!efx->ptp_data) return -ENOMEM; + mutex_init(&ptp->phc_lock); ptp->efx = efx; ptp->channel = channel;
@@ -1641,6 +1660,7 @@ fail2: efx_nic_free_buffer(efx, &ptp->start); fail1: + mutex_destroy(&ptp->phc_lock); kfree(efx->ptp_data); efx->ptp_data = NULL;
@@ -1695,6 +1715,7 @@ void efx_ptp_remove(struct efx_nic *efx) destroy_workqueue(efx->ptp_data->workwq); efx_nic_free_buffer(efx, &efx->ptp_data->start); + mutex_destroy(&efx->ptp_data->phc_lock); kfree(efx->ptp_data); efx->ptp_data = NULL; }
@@ -2106,23 +2127,23 @@ static int efx_phc_adjfine(struct ptp_clock_info *ptp, long scaled_ppm) MCDI_SET_QWORD(inadj, PTP_IN_ADJUST_FREQ, adjustment_ns); MCDI_SET_DWORD(inadj, PTP_IN_ADJUST_SECONDS, 0); MCDI_SET_DWORD(inadj, PTP_IN_ADJUST_NANOSECONDS, 0); + mutex_lock(&ptp_data->phc_lock); rc = efx_mcdi_rpc(efx, MC_CMD_PTP, inadj, sizeof(inadj), NULL, 0, NULL); - if (rc != 0) - return rc; - - ptp_data->current_adjfreq = adjustment_ns; - return 0; + if (!rc) + ptp_data->current_adjfreq = adjustment_ns; + mutex_unlock(&ptp_data->phc_lock); + return rc; } -static int efx_phc_adjtime(struct ptp_clock_info *ptp, s64 delta) +static int _efx_phc_adjtime(struct efx_ptp_data *ptp_data, s64 delta) + __must_hold(&ptp_data->phc_lock) { - u32 nic_major, nic_minor; - struct efx_ptp_data *ptp_data = container_of(ptp, - struct efx_ptp_data, - phc_clock_info); - struct efx_nic *efx = ptp_data->efx; MCDI_DECLARE_BUF(inbuf, MC_CMD_PTP_IN_ADJUST_LEN); + struct efx_nic *efx = ptp_data->efx; + u32 nic_major, nic_minor; + + lockdep_assert_held(&ptp_data->phc_lock); efx->ptp_data->ns_to_nic_time(delta, &nic_major, &nic_minor);
@@ -2135,6 +2156,18 @@ static int efx_phc_adjtime(struct ptp_clock_info *ptp, s64 delta) NULL, 0, NULL); } +static int efx_phc_adjtime(struct ptp_clock_info *ptp, s64 delta) +{ + struct efx_ptp_data *ptp_data = container_of(ptp, struct efx_ptp_data, + phc_clock_info); + int rc; + + mutex_lock(&ptp_data->phc_lock); + rc = _efx_phc_adjtime(ptp_data, delta); + mutex_unlock(&ptp_data->phc_lock); + return rc; +} + static int efx_phc_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts) { struct efx_ptp_data *ptp_data = container_of(ptp,
@@ -2166,23 +2199,21 @@ static int efx_phc_settime(struct ptp_clock_info *ptp, { /* Get the current NIC time, efx_phc_gettime. * Subtract from the desired time to get the offset - * call efx_phc_adjtime with the offset + * call _efx_phc_adjtime with the offset */ + struct efx_ptp_data *ptp_data = container_of(ptp, struct efx_ptp_data, + phc_clock_info); + struct timespec64 time_now, delta; int rc; - struct timespec64 time_now; - struct timespec64 delta; + mutex_lock(&ptp_data->phc_lock); rc = efx_phc_gettime(ptp, &time_now); - if (rc != 0) - return rc; - - delta = timespec64_sub(*e_ts, time_now); - - rc = efx_phc_adjtime(ptp, timespec64_to_ns(&delta)); - if (rc != 0) - return rc; - - return 0; + if (!rc) { + delta = timespec64_sub(*e_ts, time_now); + rc = _efx_phc_adjtime(ptp_data, timespec64_to_ns(&delta)); + } + mutex_unlock(&ptp_data->phc_lock); + return rc; } static int efx_phc_enable(struct ptp_clock_info *ptp,
--
2.34.1