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 | 31 ++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
index 3767ba495e78d210b0529ee1754e5331f2dd0a47..5c712b33851081b5ae1dbf2a0988919ae647a9e2 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
*
@@ -172,6 +174,10 @@ static int stmmac_enable(struct ptp_clock_info *ptp,
switch (rq->type) {
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;
--
2.25.1Hi Gatien,
kernel test robot noticed the following build errors:
[auto build test ERROR on fa582ca7e187a15e772e6a72fe035f649b387a60]
url: https://github.com/intel-lab-lkp/linux/commits/Gatien-Chevallier/drivers-net-stmmac-handle-start-time-set-in-the-past-for-flexible-PPS/20250729-225635
base: fa582ca7e187a15e772e6a72fe035f649b387a60
patch link: https://lore.kernel.org/r/20250729-relative_flex_pps-v2-1-3e5f03525c45%40foss.st.com
patch subject: [PATCH net-next v2 1/2] drivers: net: stmmac: handle start time set in the past for flexible PPS
config: riscv-allyesconfig (https://download.01.org/0day-ci/archive/20250731/202507310541.o0TF0jd1-lkp@intel.com/config)
compiler: clang version 16.0.6 (https://github.com/llvm/llvm-project 7cbf1a2591520c2491aa35339f227775f4d3adf6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250731/202507310541.o0TF0jd1-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/202507310541.o0TF0jd1-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c:177:3: error: expected expression
struct timespec64 curr_time;
^
>> drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c:207:3: error: use of undeclared identifier 'curr_time'
curr_time = ns_to_timespec64(ns);
^
drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c:209:49: error: use of undeclared identifier 'curr_time'
cfg->start = timespec64_add_safe(cfg->start, curr_time);
^
3 errors generated.
vim +177 drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
163
164 static int stmmac_enable(struct ptp_clock_info *ptp,
165 struct ptp_clock_request *rq, int on)
166 {
167 struct stmmac_priv *priv =
168 container_of(ptp, struct stmmac_priv, ptp_clock_ops);
169 void __iomem *ptpaddr = priv->ptpaddr;
170 struct stmmac_pps_cfg *cfg;
171 int ret = -EOPNOTSUPP;
172 unsigned long flags;
173 u32 acr_value;
174
175 switch (rq->type) {
176 case PTP_CLK_REQ_PEROUT:
> 177 struct timespec64 curr_time;
178 u64 target_ns = 0;
179 u64 ns = 0;
180
181 /* Reject requests with unsupported flags */
182 if (rq->perout.flags)
183 return -EOPNOTSUPP;
184
185 cfg = &priv->pps[rq->perout.index];
186
187 cfg->start.tv_sec = rq->perout.start.sec;
188 cfg->start.tv_nsec = rq->perout.start.nsec;
189
190 /* A time set in the past won't trigger the start of the flexible PPS generation for
191 * the GMAC5. For some reason it does for the GMAC4 but setting a time in the past
192 * should be addressed anyway. Therefore, any value set it the past is considered as
193 * an offset compared to the current MAC system time.
194 * Be aware that an offset too low may not trigger flexible PPS generation
195 * if time spent in this configuration makes the targeted time already outdated.
196 * To address this, add a safe time offset.
197 */
198 if (!cfg->start.tv_sec && cfg->start.tv_nsec < PTP_SAFE_TIME_OFFSET_NS)
199 cfg->start.tv_nsec += PTP_SAFE_TIME_OFFSET_NS;
200
201 target_ns = cfg->start.tv_nsec + ((u64)cfg->start.tv_sec * NSEC_PER_SEC);
202
203 stmmac_get_systime(priv, priv->ptpaddr, &ns);
204 if (ns > TIME64_MAX - PTP_SAFE_TIME_OFFSET_NS)
205 return -EINVAL;
206
> 207 curr_time = ns_to_timespec64(ns);
208 if (target_ns < ns + PTP_SAFE_TIME_OFFSET_NS) {
209 cfg->start = timespec64_add_safe(cfg->start, curr_time);
210 if (cfg->start.tv_sec == TIME64_MAX)
211 return -EINVAL;
212 }
213
214 cfg->period.tv_sec = rq->perout.period.sec;
215 cfg->period.tv_nsec = rq->perout.period.nsec;
216
217 write_lock_irqsave(&priv->ptp_lock, flags);
218 ret = stmmac_flex_pps_config(priv, priv->ioaddr,
219 rq->perout.index, cfg, on,
220 priv->sub_second_inc,
221 priv->systime_flags);
222 write_unlock_irqrestore(&priv->ptp_lock, flags);
223 break;
224 case PTP_CLK_REQ_EXTTS: {
225 u8 channel;
226
227 mutex_lock(&priv->aux_ts_lock);
228 acr_value = readl(ptpaddr + PTP_ACR);
229 channel = ilog2(FIELD_GET(PTP_ACR_MASK, acr_value));
230 acr_value &= ~PTP_ACR_MASK;
231
232 if (on) {
233 if (FIELD_GET(PTP_ACR_MASK, acr_value)) {
234 netdev_err(priv->dev,
235 "Cannot enable auxiliary snapshot %d as auxiliary snapshot %d is already enabled",
236 rq->extts.index, channel);
237 mutex_unlock(&priv->aux_ts_lock);
238 return -EBUSY;
239 }
240
241 priv->plat->flags |= STMMAC_FLAG_EXT_SNAPSHOT_EN;
242
243 /* Enable External snapshot trigger */
244 acr_value |= PTP_ACR_ATSEN(rq->extts.index);
245 acr_value |= PTP_ACR_ATSFC;
246 } else {
247 priv->plat->flags &= ~STMMAC_FLAG_EXT_SNAPSHOT_EN;
248 }
249 netdev_dbg(priv->dev, "Auxiliary Snapshot %d %s.\n",
250 rq->extts.index, on ? "enabled" : "disabled");
251 writel(acr_value, ptpaddr + PTP_ACR);
252 mutex_unlock(&priv->aux_ts_lock);
253 /* wait for auxts fifo clear to finish */
254 ret = readl_poll_timeout(ptpaddr + PTP_ACR, acr_value,
255 !(acr_value & PTP_ACR_ATSFC),
256 10, 10000);
257 break;
258 }
259
260 default:
261 break;
262 }
263
264 return ret;
265 }
266
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Gatien,
kernel test robot noticed the following build warnings:
[auto build test WARNING on fa582ca7e187a15e772e6a72fe035f649b387a60]
url: https://github.com/intel-lab-lkp/linux/commits/Gatien-Chevallier/drivers-net-stmmac-handle-start-time-set-in-the-past-for-flexible-PPS/20250729-225635
base: fa582ca7e187a15e772e6a72fe035f649b387a60
patch link: https://lore.kernel.org/r/20250729-relative_flex_pps-v2-1-3e5f03525c45%40foss.st.com
patch subject: [PATCH net-next v2 1/2] drivers: net: stmmac: handle start time set in the past for flexible PPS
config: x86_64-buildonly-randconfig-002-20250730 (https://download.01.org/0day-ci/archive/20250730/202507301148.TVzOecMo-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250730/202507301148.TVzOecMo-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/202507301148.TVzOecMo-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c:177:3: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
177 | struct timespec64 curr_time;
| ^
1 warning generated.
vim +177 drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
163
164 static int stmmac_enable(struct ptp_clock_info *ptp,
165 struct ptp_clock_request *rq, int on)
166 {
167 struct stmmac_priv *priv =
168 container_of(ptp, struct stmmac_priv, ptp_clock_ops);
169 void __iomem *ptpaddr = priv->ptpaddr;
170 struct stmmac_pps_cfg *cfg;
171 int ret = -EOPNOTSUPP;
172 unsigned long flags;
173 u32 acr_value;
174
175 switch (rq->type) {
176 case PTP_CLK_REQ_PEROUT:
> 177 struct timespec64 curr_time;
178 u64 target_ns = 0;
179 u64 ns = 0;
180
181 /* Reject requests with unsupported flags */
182 if (rq->perout.flags)
183 return -EOPNOTSUPP;
184
185 cfg = &priv->pps[rq->perout.index];
186
187 cfg->start.tv_sec = rq->perout.start.sec;
188 cfg->start.tv_nsec = rq->perout.start.nsec;
189
190 /* A time set in the past won't trigger the start of the flexible PPS generation for
191 * the GMAC5. For some reason it does for the GMAC4 but setting a time in the past
192 * should be addressed anyway. Therefore, any value set it the past is considered as
193 * an offset compared to the current MAC system time.
194 * Be aware that an offset too low may not trigger flexible PPS generation
195 * if time spent in this configuration makes the targeted time already outdated.
196 * To address this, add a safe time offset.
197 */
198 if (!cfg->start.tv_sec && cfg->start.tv_nsec < PTP_SAFE_TIME_OFFSET_NS)
199 cfg->start.tv_nsec += PTP_SAFE_TIME_OFFSET_NS;
200
201 target_ns = cfg->start.tv_nsec + ((u64)cfg->start.tv_sec * NSEC_PER_SEC);
202
203 stmmac_get_systime(priv, priv->ptpaddr, &ns);
204 if (ns > TIME64_MAX - PTP_SAFE_TIME_OFFSET_NS)
205 return -EINVAL;
206
207 curr_time = ns_to_timespec64(ns);
208 if (target_ns < ns + PTP_SAFE_TIME_OFFSET_NS) {
209 cfg->start = timespec64_add_safe(cfg->start, curr_time);
210 if (cfg->start.tv_sec == TIME64_MAX)
211 return -EINVAL;
212 }
213
214 cfg->period.tv_sec = rq->perout.period.sec;
215 cfg->period.tv_nsec = rq->perout.period.nsec;
216
217 write_lock_irqsave(&priv->ptp_lock, flags);
218 ret = stmmac_flex_pps_config(priv, priv->ioaddr,
219 rq->perout.index, cfg, on,
220 priv->sub_second_inc,
221 priv->systime_flags);
222 write_unlock_irqrestore(&priv->ptp_lock, flags);
223 break;
224 case PTP_CLK_REQ_EXTTS: {
225 u8 channel;
226
227 mutex_lock(&priv->aux_ts_lock);
228 acr_value = readl(ptpaddr + PTP_ACR);
229 channel = ilog2(FIELD_GET(PTP_ACR_MASK, acr_value));
230 acr_value &= ~PTP_ACR_MASK;
231
232 if (on) {
233 if (FIELD_GET(PTP_ACR_MASK, acr_value)) {
234 netdev_err(priv->dev,
235 "Cannot enable auxiliary snapshot %d as auxiliary snapshot %d is already enabled",
236 rq->extts.index, channel);
237 mutex_unlock(&priv->aux_ts_lock);
238 return -EBUSY;
239 }
240
241 priv->plat->flags |= STMMAC_FLAG_EXT_SNAPSHOT_EN;
242
243 /* Enable External snapshot trigger */
244 acr_value |= PTP_ACR_ATSEN(rq->extts.index);
245 acr_value |= PTP_ACR_ATSFC;
246 } else {
247 priv->plat->flags &= ~STMMAC_FLAG_EXT_SNAPSHOT_EN;
248 }
249 netdev_dbg(priv->dev, "Auxiliary Snapshot %d %s.\n",
250 rq->extts.index, on ? "enabled" : "disabled");
251 writel(acr_value, ptpaddr + PTP_ACR);
252 mutex_unlock(&priv->aux_ts_lock);
253 /* wait for auxts fifo clear to finish */
254 ret = readl_poll_timeout(ptpaddr + PTP_ACR, acr_value,
255 !(acr_value & PTP_ACR_ATSFC),
256 10, 10000);
257 break;
258 }
259
260 default:
261 break;
262 }
263
264 return ret;
265 }
266
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Tue, Jul 29, 2025 at 04:52:00PM +0200, Gatien Chevallier wrote:
> 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 | 31 ++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> index 3767ba495e78d210b0529ee1754e5331f2dd0a47..5c712b33851081b5ae1dbf2a0988919ae647a9e2 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
> *
> @@ -172,6 +174,10 @@ static int stmmac_enable(struct ptp_clock_info *ptp,
>
> switch (rq->type) {
> case PTP_CLK_REQ_PEROUT:
> + struct timespec64 curr_time;
> + u64 target_ns = 0;
> + u64 ns = 0;
> +
I think you need to wrap this case in {}, as is already done for the following
case.
Clang 20.1.8 W=1 build warn about the current arrangement as follows.
.../stmmac_ptp.c:177:3: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
177 | struct timespec64 curr_time;
| ^
1 warning generated.
GCC 8.5.0 (but not 15.1.0) also flags this problem.
Also, please note:
## Form letter - net-next-closed
The merge window for v6.17 has begun and therefore net-next is closed
for new drivers, features, code refactoring and optimizations. We are
currently accepting bug fixes only.
Please repost when net-next reopens after 11th August.
RFC patches sent for review only are obviously welcome at any time.
See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
--
pw-bot: defer
On 7/29/25 18:58, Simon Horman wrote:
> On Tue, Jul 29, 2025 at 04:52:00PM +0200, Gatien Chevallier wrote:
>> 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 | 31 ++++++++++++++++++++++++
>> 1 file changed, 31 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
>> index 3767ba495e78d210b0529ee1754e5331f2dd0a47..5c712b33851081b5ae1dbf2a0988919ae647a9e2 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
>> *
>> @@ -172,6 +174,10 @@ static int stmmac_enable(struct ptp_clock_info *ptp,
>>
>> switch (rq->type) {
>> case PTP_CLK_REQ_PEROUT:
>> + struct timespec64 curr_time;
>> + u64 target_ns = 0;
>> + u64 ns = 0;
>> +
>
> I think you need to wrap this case in {}, as is already done for the following
> case.
>
> Clang 20.1.8 W=1 build warn about the current arrangement as follows.
>
> .../stmmac_ptp.c:177:3: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
> 177 | struct timespec64 curr_time;
> | ^
> 1 warning generated.
>
> GCC 8.5.0 (but not 15.1.0) also flags this problem.
>
Hello Simon,
Ok I will comply for V3.
> Also, please note:
>
> ## Form letter - net-next-closed
>
> The merge window for v6.17 has begun and therefore net-next is closed
> for new drivers, features, code refactoring and optimizations. We are
> currently accepting bug fixes only.
>
> Please repost when net-next reopens after 11th August.
>
> RFC patches sent for review only are obviously welcome at any time.
>
> See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
>
Thank you for pointing this out.
Best regards,
Gatien
© 2016 - 2025 Red Hat, Inc.