In case the time arguments used for flexible PPS signal generation are in
the past, consider the arguments to be a time offset relative to the MAC
system time.
This way, past time use case is handled and it avoids the tedious work
of passing an absolute time value for the flexible PPS signal generation
while not breaking existing scripts that may rely on this behavior.
Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c | 35 +++++++++++++++++++++++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
index 3767ba495e78d210b0529ee1754e5331f2dd0a47..0de10a309e1e945fddfcda39a6c388f3dfff7c92 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
@@ -10,6 +10,8 @@
#include "stmmac.h"
#include "stmmac_ptp.h"
+#define PTP_SAFE_TIME_OFFSET_NS 500000
+
/**
* stmmac_adjust_freq
*
@@ -171,7 +173,11 @@ static int stmmac_enable(struct ptp_clock_info *ptp,
u32 acr_value;
switch (rq->type) {
- case PTP_CLK_REQ_PEROUT:
+ case PTP_CLK_REQ_PEROUT: {
+ struct timespec64 curr_time;
+ u64 target_ns = 0;
+ u64 ns = 0;
+
/* Reject requests with unsupported flags */
if (rq->perout.flags)
return -EOPNOTSUPP;
@@ -180,6 +186,31 @@ static int stmmac_enable(struct ptp_clock_info *ptp,
cfg->start.tv_sec = rq->perout.start.sec;
cfg->start.tv_nsec = rq->perout.start.nsec;
+
+ /* A time set in the past won't trigger the start of the flexible PPS generation for
+ * the GMAC5. For some reason it does for the GMAC4 but setting a time in the past
+ * should be addressed anyway. Therefore, any value set it the past is considered as
+ * an offset compared to the current MAC system time.
+ * Be aware that an offset too low may not trigger flexible PPS generation
+ * if time spent in this configuration makes the targeted time already outdated.
+ * To address this, add a safe time offset.
+ */
+ if (!cfg->start.tv_sec && cfg->start.tv_nsec < PTP_SAFE_TIME_OFFSET_NS)
+ cfg->start.tv_nsec += PTP_SAFE_TIME_OFFSET_NS;
+
+ target_ns = cfg->start.tv_nsec + ((u64)cfg->start.tv_sec * NSEC_PER_SEC);
+
+ stmmac_get_systime(priv, priv->ptpaddr, &ns);
+ if (ns > TIME64_MAX - PTP_SAFE_TIME_OFFSET_NS)
+ return -EINVAL;
+
+ curr_time = ns_to_timespec64(ns);
+ if (target_ns < ns + PTP_SAFE_TIME_OFFSET_NS) {
+ cfg->start = timespec64_add_safe(cfg->start, curr_time);
+ if (cfg->start.tv_sec == TIME64_MAX)
+ return -EINVAL;
+ }
+
cfg->period.tv_sec = rq->perout.period.sec;
cfg->period.tv_nsec = rq->perout.period.nsec;
@@ -190,6 +221,8 @@ static int stmmac_enable(struct ptp_clock_info *ptp,
priv->systime_flags);
write_unlock_irqrestore(&priv->ptp_lock, flags);
break;
+ }
+
case PTP_CLK_REQ_EXTTS: {
u8 channel;
--
2.25.1
Hi Gatien, kernel test robot noticed the following build errors: [auto build test ERROR on 242041164339594ca019481d54b4f68a7aaff64e] url: https://github.com/intel-lab-lkp/linux/commits/Gatien-Chevallier/drivers-net-stmmac-handle-start-time-set-in-the-past-for-flexible-PPS/20250827-190905 base: 242041164339594ca019481d54b4f68a7aaff64e patch link: https://lore.kernel.org/r/20250827-relative_flex_pps-v3-1-673e77978ba2%40foss.st.com patch subject: [PATCH net-next v3 1/2] drivers: net: stmmac: handle start time set in the past for flexible PPS config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20250828/202508281615.ExryCwiA-lkp@intel.com/config) compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261) rustc: rustc 1.88.0 (6b00bc388 2025-06-23) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250828/202508281615.ExryCwiA-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202508281615.ExryCwiA-lkp@intel.com/ All errors (new ones prefixed by >>, old ones prefixed by <<): >> ERROR: modpost: "timespec64_add_safe" [drivers/net/ethernet/stmicro/stmmac/stmmac.ko] undefined! -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
On Wed, 27 Aug 2025 13:04:58 +0200 Gatien Chevallier wrote: > + curr_time = ns_to_timespec64(ns); > + if (target_ns < ns + PTP_SAFE_TIME_OFFSET_NS) { > + cfg->start = timespec64_add_safe(cfg->start, curr_time); Is there a strong reason to use timespec64_add_safe()? It's not exported to modules: ERROR: modpost: "timespec64_add_safe" [drivers/net/ethernet/stmicro/stmmac/stmmac.ko] undefined! -- pw-bot: cr
On 8/28/25 04:31, Jakub Kicinski wrote: > On Wed, 27 Aug 2025 13:04:58 +0200 Gatien Chevallier wrote: >> + curr_time = ns_to_timespec64(ns); >> + if (target_ns < ns + PTP_SAFE_TIME_OFFSET_NS) { >> + cfg->start = timespec64_add_safe(cfg->start, curr_time); > > Is there a strong reason to use timespec64_add_safe()? > It's not exported to modules: > ERROR: modpost: "timespec64_add_safe" [drivers/net/ethernet/stmicro/stmmac/stmmac.ko] undefined! Hello Jakub, you're absolutely right. I don't know how I did not encounter the build error while performing some tests, that I'm getting now as well. The handling of overflows is already done in that function. Either I can make a patch to export the symbol or handle the computation in the driver. What do you think is best? Cheers, Gatien
On Fri, 29 Aug 2025 12:51:40 +0200 Gatien CHEVALLIER wrote: > On 8/28/25 04:31, Jakub Kicinski wrote: > > On Wed, 27 Aug 2025 13:04:58 +0200 Gatien Chevallier wrote: > >> + curr_time = ns_to_timespec64(ns); > >> + if (target_ns < ns + PTP_SAFE_TIME_OFFSET_NS) { > >> + cfg->start = timespec64_add_safe(cfg->start, curr_time); > > > > Is there a strong reason to use timespec64_add_safe()? > > It's not exported to modules: > > ERROR: modpost: "timespec64_add_safe" [drivers/net/ethernet/stmicro/stmmac/stmmac.ko] undefined! > > Hello Jakub, > > you're absolutely right. I don't know how I did not encounter the build > error while performing some tests, that I'm getting now as well. > > The handling of overflows is already done in that function. Either > I can make a patch to export the symbol or handle the computation in the > driver. What do you think is best? The odds of me being right about time related code are only slightly better than 50/50, and I don't know what "flexible PPS" is :) But in principle, if the reason you need to check for overflow is valid -- add the export. The time maintainers will tell us if they don't want it.
© 2016 - 2025 Red Hat, Inc.