[PATCH net-next 0/2] net: stmmac: allow generation of flexible PPS relative to MAC time

[PATCH net-next 0/2] net: stmmac: allow generation of flexible PPS relative to MAC time

[Gatien Chevallier]

When doing some testing on stm32mp2x platforms(MACv5), I noticed that
the command previously used with a MACv4 for genering a PPS signal:
echo "0 0 0 1 1" > /sys/class/ptp/ptp0/period
did not work.

This is because the arguments passed through this command must contain
the start time at which the PPS should be generated, relative to the
MAC system time. For some reason, a time set in the past seems to work
with a MACv4.

Because passing such an argument is tedious, introduce
STMMAC_RELATIVE_FLEX_PPS config switch so that the MAC system time
is added to the args to the stmmac_ptp driver.

Example to generate a flexible PPS signal that has a 1s period 3s
relative to when the command was entered before and after setting
STMMAC_RELATIVE_FLEX_PPS:

Before: echo "0 175xxxxxxx 0 1 1" > /sys/class/ptp/ptp0/period

After: echo "0 3 0 1 1" > /sys/class/ptp/ptp0/period

Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
---
Gatien Chevallier (2):
      drivers: net: stmmac: add STMMAC_RELATIVE_FLEX_PPS
      net: stmmac: select STMMAC_RELATIVE_FLEX_PPS for stm32 SoCs

 drivers/net/ethernet/stmicro/stmmac/Kconfig      | 10 ++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c | 13 +++++++++++++
 2 files changed, 23 insertions(+)
---
base-commit: 8aad37d16cffb6c0940d9b213456a2733a786f57
change-id: 20250723-relative_flex_pps-faa2fbcaf835

Best regards,
-- 
Gatien Chevallier <gatien.chevallier@foss.st.com>

[PATCH net-next 1/2] drivers: net: stmmac: add STMMAC_RELATIVE_FLEX_PPS

[Gatien Chevallier]

Add the STMMAC_RELATIVE_FLEX_PPS config switch so that the MAC system
time is added to the requested start time when configuring the flexible
PPS signal start time.

This eases the configuration of the generation of flexible PPS signals.

Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig      |  9 +++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c | 13 +++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 67fa879b1e521ef6d7bd2a8db30d812301eb4cf7..949c744d30f19f5ff480dca4811e678d2b93c450 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -357,3 +357,12 @@ config STMMAC_PCI
 
 	  If unsure, say N.
 endif
+
+config STMMAC_RELATIVE_FLEX_PPS
+	bool "Support for STMMAC system time relative flexible PPS generation"
+	default n
+	help
+	  Say Y to add the MAC system time to the arguments passed to the
+	  PTP driver when requesting a flexible PPS generation. This avoids
+	  the tedious task of passing an absolute time value when using sysfs
+	  entry.
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
index 3767ba495e78d210b0529ee1754e5331f2dd0a47..cc898eab4df6a3e2530a8b03dfdaa9016d1bb0b9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
@@ -180,6 +180,19 @@ 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;
+		if (IS_ENABLED(CONFIG_STMMAC_RELATIVE_FLEX_PPS)) {
+			struct timespec64 curr_time;
+			u64 ns = 0;
+
+			/* Be aware that an offset too low may not trigger flexible PPS generation
+			 * if time spent in this configuration makes this relative time already
+			 * outdated.
+			 */
+			stmmac_get_systime(priv, priv->ptpaddr, &ns);
+			curr_time = ns_to_timespec64(ns);
+			cfg->start.tv_sec += curr_time.tv_sec;
+			cfg->start.tv_nsec += curr_time.tv_nsec;
+		}
 		cfg->period.tv_sec = rq->perout.period.sec;
 		cfg->period.tv_nsec = rq->perout.period.nsec;
 

-- 
2.25.1

Re: [PATCH net-next 1/2] drivers: net: stmmac: add STMMAC_RELATIVE_FLEX_PPS

[Russell King (Oracle)]

On Thu, Jul 24, 2025 at 02:31:18PM +0200, Gatien Chevallier wrote:
> +config STMMAC_RELATIVE_FLEX_PPS
> +	bool "Support for STMMAC system time relative flexible PPS generation"
> +	default n

There is no need for "default n" because the default default is n.

> +	help
> +	  Say Y to add the MAC system time to the arguments passed to the
> +	  PTP driver when requesting a flexible PPS generation. This avoids
> +	  the tedious task of passing an absolute time value when using sysfs
> +	  entry.

How does a distro decide whether to enable or disable this option? What
does it depend on?

If it's only for some platforms and not others (due to causing
regressions) then what is a distro supposed to do with their kernels
that support multiple platforms?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next 1/2] drivers: net: stmmac: add STMMAC_RELATIVE_FLEX_PPS

[Gatien CHEVALLIER]

On 8/5/25 12:30, Russell King (Oracle) wrote:
> On Thu, Jul 24, 2025 at 02:31:18PM +0200, Gatien Chevallier wrote:
>> +config STMMAC_RELATIVE_FLEX_PPS
>> +	bool "Support for STMMAC system time relative flexible PPS generation"
>> +	default n
> 
> There is no need for "default n" because the default default is n.
>

>> +	help
>> +	  Say Y to add the MAC system time to the arguments passed to the
>> +	  PTP driver when requesting a flexible PPS generation. This avoids
>> +	  the tedious task of passing an absolute time value when using sysfs
>> +	  entry.
> 
> How does a distro decide whether to enable or disable this option? What
> does it depend on?
> 
> If it's only for some platforms and not others (due to causing
> regressions) then what is a distro supposed to do with their kernels
> that support multiple platforms?
> 

Hello Russell,

Following Jakub's feedback, I sent a v2 [1] with this config removed.
The idea now is to compare the time value given to the stmmac_ptp driver
and compare it to the MAC system time. If the time value is is the past,
relative to the MAC system time, then it is considered as an offset
relative the the MAC system time.

It's not ideal as it would have probably been better to always consider
an offset but I don't know how popular and used this feature currently
is. Therefore, I did not want to break any existing script by changing
the current behavior.

I'd be very interested to have your take on this as well.

 From what I observed, it's linked to the MAC version. So enabling this
config would have meant knowing what MAC version is embedded. Supporting
multiple platforms then becomes tricky.

[1]: 
https://lore.kernel.org/all/20250729-relative_flex_pps-v2-0-3e5f03525c45@foss.st.com/

[PATCH net-next 2/2] net: stmmac: select STMMAC_RELATIVE_FLEX_PPS for stm32 SoCs

[Gatien Chevallier]

In order to simplify the generation of flexible PPS signals by passing
an time offset relative to the MAC system time, select
STMMAC_RELATIVE_FLEX_PPS for DWMAC_STM32 config.

E.g: "echo "0 3 0 1 1" > /sys/class/ptp/ptp0/period" generates a PPS
that has a 1s period, 3 seconds after entering the command for ptp0 clock,
given that the MAC system time stays stable.

Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 949c744d30f19f5ff480dca4811e678d2b93c450..98fae30ad5a2760ec6b9cebf3dcaf8e4d7d19b77 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -233,6 +233,7 @@ config DWMAC_STM32
 	default ARCH_STM32
 	depends on OF && HAS_IOMEM && (ARCH_STM32 || COMPILE_TEST)
 	select MFD_SYSCON
+	select STMMAC_RELATIVE_FLEX_PPS
 	help
 	  Support for ethernet controller on STM32 SOCs.
 

-- 
2.25.1

Re: [PATCH net-next 0/2] net: stmmac: allow generation of flexible PPS relative to MAC time

[Jakub Kicinski]

On Thu, 24 Jul 2025 14:31:17 +0200 Gatien Chevallier wrote:
> When doing some testing on stm32mp2x platforms(MACv5), I noticed that
> the command previously used with a MACv4 for genering a PPS signal:
> echo "0 0 0 1 1" > /sys/class/ptp/ptp0/period
> did not work.
> 
> This is because the arguments passed through this command must contain
> the start time at which the PPS should be generated, relative to the
> MAC system time. For some reason, a time set in the past seems to work
> with a MACv4.
> 
> Because passing such an argument is tedious, introduce
> STMMAC_RELATIVE_FLEX_PPS config switch so that the MAC system time
> is added to the args to the stmmac_ptp driver.
> 
> Example to generate a flexible PPS signal that has a 1s period 3s
> relative to when the command was entered before and after setting
> STMMAC_RELATIVE_FLEX_PPS:
> 
> Before: echo "0 175xxxxxxx 0 1 1" > /sys/class/ptp/ptp0/period
> 
> After: echo "0 3 0 1 1" > /sys/class/ptp/ptp0/period

Kconfig doesn't seem like a great way of achieving the outcome.
Some per-platform knob would be better.
But ideally we wouldn't do either. Could we possibly guess which
format user has chosen based on the values, at runtime?

Re: [PATCH net-next 0/2] net: stmmac: allow generation of flexible PPS relative to MAC time

[Gatien CHEVALLIER]

On 7/26/25 02:25, Jakub Kicinski wrote:
> On Thu, 24 Jul 2025 14:31:17 +0200 Gatien Chevallier wrote:
>> When doing some testing on stm32mp2x platforms(MACv5), I noticed that
>> the command previously used with a MACv4 for genering a PPS signal:
>> echo "0 0 0 1 1" > /sys/class/ptp/ptp0/period
>> did not work.
>>
>> This is because the arguments passed through this command must contain
>> the start time at which the PPS should be generated, relative to the
>> MAC system time. For some reason, a time set in the past seems to work
>> with a MACv4.
>>
>> Because passing such an argument is tedious, introduce
>> STMMAC_RELATIVE_FLEX_PPS config switch so that the MAC system time
>> is added to the args to the stmmac_ptp driver.
>>
>> Example to generate a flexible PPS signal that has a 1s period 3s
>> relative to when the command was entered before and after setting
>> STMMAC_RELATIVE_FLEX_PPS:
>>
>> Before: echo "0 175xxxxxxx 0 1 1" > /sys/class/ptp/ptp0/period
>>
>> After: echo "0 3 0 1 1" > /sys/class/ptp/ptp0/period
> 
> Kconfig doesn't seem like a great way of achieving the outcome.
> Some per-platform knob would be better.
> But ideally we wouldn't do either. Could we possibly guess which
> format user has chosen based on the values, at runtime?

Hello Jakub,

There are two reasons for which I chose this approach:
1) I did not want to affect other platforms and possibly
break scripts that work with the current behavior. Is it
acceptable to do otherwise? If so, maybe there's no need
for a config switch or a per-platform implementation.
2) SoCs may implement more than one MAC and the system
time for these MACs may or may not be synced + the system
time maintained by a MAC may not be a value that represents
a date.

For these reasons, I'm not sure we can rely on the values
that were given to stmmac_enable() to deduce what behavior
we choose. The ptp_clock_request() structure does not hold
loads of information as well.

Maybe we could compare the time to the current MAC system
time and, if the start time is in the past, consider the
value to be an offset. Therefore, any value set in the past
would be considered as an offset. I see some implementations
doing either that or replacing any value set in the past to
a safe start + a fixed offset.

Best regards,
Gatien

Re: [PATCH net-next 0/2] net: stmmac: allow generation of flexible PPS relative to MAC time

[Jakub Kicinski]

On Mon, 28 Jul 2025 10:15:07 +0200 Gatien CHEVALLIER wrote:
> Maybe we could compare the time to the current MAC system
> time and, if the start time is in the past, consider the
> value to be an offset. Therefore, any value set in the past
> would be considered as an offset. I see some implementations
> doing either that or replacing any value set in the past to
> a safe start + a fixed offset.

Let's try this.

Re: [PATCH net-next 0/2] net: stmmac: allow generation of flexible PPS relative to MAC time

[Gatien CHEVALLIER]

On 7/28/25 17:58, Jakub Kicinski wrote:
> On Mon, 28 Jul 2025 10:15:07 +0200 Gatien CHEVALLIER wrote:
>> Maybe we could compare the time to the current MAC system
>> time and, if the start time is in the past, consider the
>> value to be an offset. Therefore, any value set in the past
>> would be considered as an offset. I see some implementations
>> doing either that or replacing any value set in the past to
>> a safe start + a fixed offset.
> 
> Let's try this.

Ok, I sent a V2 with a proposal implementing this behavior.
Thank you.