* Re: [PATCH net-next v2 1/2] drivers: net: stmmac: handle start time set in the past for flexible PPS
2025-07-29 14:52 ` [PATCH net-next v2 1/2] drivers: net: stmmac: handle start time set in the past for flexible PPS Gatien Chevallier
@ 2025-07-29 16:58 ` Simon Horman
2025-07-30 9:01 ` Gatien CHEVALLIER
2025-07-30 3:36 ` kernel test robot
2025-07-30 21:39 ` kernel test robot
2 siblings, 1 reply; 7+ messages in thread
From: Simon Horman @ 2025-07-29 16:58 UTC (permalink / raw)
To: Gatien Chevallier
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Richard Cochran,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, netdev,
linux-stm32, linux-arm-kernel, linux-kernel, devicetree
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 1/2] drivers: net: stmmac: handle start time set in the past for flexible PPS
2025-07-29 16:58 ` Simon Horman
@ 2025-07-30 9:01 ` Gatien CHEVALLIER
0 siblings, 0 replies; 7+ messages in thread
From: Gatien CHEVALLIER @ 2025-07-30 9:01 UTC (permalink / raw)
To: Simon Horman
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Richard Cochran,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, netdev,
linux-stm32, linux-arm-kernel, linux-kernel, devicetree
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 1/2] drivers: net: stmmac: handle start time set in the past for flexible PPS
2025-07-29 14:52 ` [PATCH net-next v2 1/2] drivers: net: stmmac: handle start time set in the past for flexible PPS Gatien Chevallier
2025-07-29 16:58 ` Simon Horman
@ 2025-07-30 3:36 ` kernel test robot
2025-07-30 21:39 ` kernel test robot
2 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2025-07-30 3:36 UTC (permalink / raw)
To: Gatien Chevallier, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
Richard Cochran, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: llvm, oe-kbuild-all, netdev, linux-stm32, linux-arm-kernel,
linux-kernel, devicetree, Gatien Chevallier
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 1/2] drivers: net: stmmac: handle start time set in the past for flexible PPS
2025-07-29 14:52 ` [PATCH net-next v2 1/2] drivers: net: stmmac: handle start time set in the past for flexible PPS Gatien Chevallier
2025-07-29 16:58 ` Simon Horman
2025-07-30 3:36 ` kernel test robot
@ 2025-07-30 21:39 ` kernel test robot
2 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2025-07-30 21:39 UTC (permalink / raw)
To: Gatien Chevallier, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
Richard Cochran, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: llvm, oe-kbuild-all, netdev, linux-stm32, linux-arm-kernel,
linux-kernel, devicetree, Gatien Chevallier
Hi 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
^ permalink raw reply [flat|nested] 7+ messages in thread