diff mbox series

[RFC,net-next,3/6] ptp: Add free running time support

Message ID 20220306085658.1943-4-gerhard@engleder-embedded.com (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series ptp: Support hardware clocks with additional free running time | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 193 this patch: 193
netdev/cc_maintainers success CCed 3 of 3 maintainers
netdev/build_clang success Errors and warnings before: 32 this patch: 32
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 229 this patch: 229
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 86 lines checked
netdev/kdoc success Errors and warnings before: 5 this patch: 5
netdev/source_inline success Was 0 now: 0

Commit Message

Gerhard Engleder March 6, 2022, 8:56 a.m. UTC
ptp vclocks require a clock with free running time for the timecounter.
Currently only a physical clock forced to free running is supported.
If vclocks are used, then the physical clock cannot be synchronized
anymore. The synchronized time is not available in hardware in this
case. As a result, timed transmission with ETF/TAPRIO hardware support
is not possible anymore.

If hardware would support a free running time additionally to the
physical clock, then the physical clock does not need to be forced to
free running. Thus, the physical clocks can still be synchronized while
vclocks are in use.

The physical clock could be used to synchronize the time domain of the
TSN network and trigger ETF/TAPRIO. In parallel vclocks can be used to
synchronize other time domains.

Allow read and cross time stamp of additional free running time for
physical clocks. Let vclocks use free running time if available.

Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
---
 drivers/ptp/ptp_vclock.c         | 20 +++++++++++++++-----
 include/linux/ptp_clock_kernel.h | 27 +++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 5 deletions(-)

Comments

Richard Cochran March 6, 2022, 4:36 p.m. UTC | #1
On Sun, Mar 06, 2022 at 09:56:55AM +0100, Gerhard Engleder wrote:
> ptp vclocks require a clock with free running time for the timecounter.
> Currently only a physical clock forced to free running is supported.
> If vclocks are used, then the physical clock cannot be synchronized
> anymore. The synchronized time is not available in hardware in this
> case. As a result, timed transmission with ETF/TAPRIO hardware support
> is not possible anymore.
> 
> If hardware would support a free running time additionally to the
> physical clock, then the physical clock does not need to be forced to
> free running. Thus, the physical clocks can still be synchronized while
> vclocks are in use.
> 
> The physical clock could be used to synchronize the time domain of the
> TSN network and trigger ETF/TAPRIO. In parallel vclocks can be used to
> synchronize other time domains.
> 
> Allow read and cross time stamp of additional free running time for
> physical clocks. Let vclocks use free running time if available.
> 
> Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
> ---
>  drivers/ptp/ptp_vclock.c         | 20 +++++++++++++++-----
>  include/linux/ptp_clock_kernel.h | 27 +++++++++++++++++++++++++++
>  2 files changed, 42 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/ptp/ptp_vclock.c b/drivers/ptp/ptp_vclock.c
> index cb179a3ea508..3715d75ee8bd 100644
> --- a/drivers/ptp/ptp_vclock.c
> +++ b/drivers/ptp/ptp_vclock.c
> @@ -68,7 +68,10 @@ static int ptp_vclock_gettimex(struct ptp_clock_info *ptp,
>  	int err;
>  	u64 ns;
>  
> -	err = pptp->info->gettimex64(pptp->info, &pts, sts);
> +	if (pptp->info->getfreeruntimex64)
> +		err = pptp->info->getfreeruntimex64(pptp->info, &pts, sts);
> +	else
> +		err = pptp->info->gettimex64(pptp->info, &pts, sts);

Why all this extra if/then/else here and at registration?
Just provide new ptp_vclock_ helpers and drop the run time conditionals.

>  	if (err)
>  		return err;
>  
> @@ -104,7 +107,10 @@ static int ptp_vclock_getcrosststamp(struct ptp_clock_info *ptp,
>  	int err;
>  	u64 ns;
>  
> -	err = pptp->info->getcrosststamp(pptp->info, xtstamp);
> +	if (pptp->info->getfreeruncrosststamp)
> +		err = pptp->info->getfreeruncrosststamp(pptp->info, xtstamp);
> +	else
> +		err = pptp->info->getcrosststamp(pptp->info, xtstamp);

same here

>  	if (err)
>  		return err;
>  
> @@ -143,7 +149,9 @@ static u64 ptp_vclock_read(const struct cyclecounter *cc)
>  	struct ptp_clock *ptp = vclock->pclock;
>  	struct timespec64 ts = {};
>  
> -	if (ptp->info->gettimex64)
> +	if (ptp->info->getfreeruntimex64)
> +		ptp->info->getfreeruntimex64(ptp->info, &ts, NULL);
> +	else if (ptp->info->gettimex64)
>  		ptp->info->gettimex64(ptp->info, &ts, NULL);
>  	else
>  		ptp->info->gettime64(ptp->info, &ts);
> @@ -168,11 +176,13 @@ struct ptp_vclock *ptp_vclock_register(struct ptp_clock *pclock)
>  
>  	vclock->pclock = pclock;
>  	vclock->info = ptp_vclock_info;
> -	if (pclock->info->gettimex64)
> +	if (pclock->info->getfreeruntimex64 || pclock->info->gettimex64)
>  		vclock->info.gettimex64 = ptp_vclock_gettimex;
>  	else
>  		vclock->info.gettime64 = ptp_vclock_gettime;
> -	if (pclock->info->getcrosststamp)
> +	if ((pclock->info->getfreeruntimex64 &&
> +	     pclock->info->getfreeruncrosststamp) ||
> +	    pclock->info->getcrosststamp)
>  		vclock->info.getcrosststamp = ptp_vclock_getcrosststamp;
>  	vclock->cc = ptp_vclock_cc;
>  
> diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
> index 554454cb8693..b291517fc7c8 100644
> --- a/include/linux/ptp_clock_kernel.h
> +++ b/include/linux/ptp_clock_kernel.h
> @@ -108,6 +108,28 @@ struct ptp_system_timestamp {
>   * @settime64:  Set the current time on the hardware clock.
>   *              parameter ts: Time value to set.
>   *
> + * @getfreeruntimex64:  Reads the current free running time from the hardware
> + *                      clock and optionally also the system clock. This
> + *                      operation requires hardware support for an additional
> + *                      free running time including support for hardware time
> + *                      stamps based on that free running time.
> + *                      The free running time must be completely independet from
> + *                      the actual time of the PTP clock. It must be monotonic
> + *                      and its frequency must be constant.
> + *                      parameter ts: Holds the PHC free running timestamp.
> + *                      parameter sts: If not NULL, it holds a pair of
> + *                      timestamps from the system clock. The first reading is
> + *                      made right before reading the lowest bits of the PHC
> + *                      free running timestamp and the second reading
> + *                      immediately follows that.
> + *
> + * @getfreeruncrosststamp:  Reads the current time from the free running
> + *                          hardware clock and system clock simultaneously.
> + *                          parameter cts: Contains timestamp (device,system)
> + *                          pair, where device time is the free running time
> + *                          also used for @getfreeruntimex64 and system time is
> + *                          realtime and monotonic.
> + *
>   * @enable:   Request driver to enable or disable an ancillary feature.
>   *            parameter request: Desired resource to enable or disable.
>   *            parameter on: Caller passes one to enable or zero to disable.
> @@ -155,6 +177,11 @@ struct ptp_clock_info {
>  	int (*getcrosststamp)(struct ptp_clock_info *ptp,
>  			      struct system_device_crosststamp *cts);
>  	int (*settime64)(struct ptp_clock_info *p, const struct timespec64 *ts);
> +	int (*getfreeruntimex64)(struct ptp_clock_info *ptp,
> +				 struct timespec64 *ts,
> +				 struct ptp_system_timestamp *sts);
> +	int (*getfreeruncrosststamp)(struct ptp_clock_info *ptp,
> +				     struct system_device_crosststamp *cts);

Wow, that is really hard to read.  Suggest freerun_gettimex64 and
freerun_crosststamp instead.

>  	int (*enable)(struct ptp_clock_info *ptp,
>  		      struct ptp_clock_request *request, int on);
>  	int (*verify)(struct ptp_clock_info *ptp, unsigned int pin,
> -- 
> 2.20.1
> 

Thanks,
Richard
diff mbox series

Patch

diff --git a/drivers/ptp/ptp_vclock.c b/drivers/ptp/ptp_vclock.c
index cb179a3ea508..3715d75ee8bd 100644
--- a/drivers/ptp/ptp_vclock.c
+++ b/drivers/ptp/ptp_vclock.c
@@ -68,7 +68,10 @@  static int ptp_vclock_gettimex(struct ptp_clock_info *ptp,
 	int err;
 	u64 ns;
 
-	err = pptp->info->gettimex64(pptp->info, &pts, sts);
+	if (pptp->info->getfreeruntimex64)
+		err = pptp->info->getfreeruntimex64(pptp->info, &pts, sts);
+	else
+		err = pptp->info->gettimex64(pptp->info, &pts, sts);
 	if (err)
 		return err;
 
@@ -104,7 +107,10 @@  static int ptp_vclock_getcrosststamp(struct ptp_clock_info *ptp,
 	int err;
 	u64 ns;
 
-	err = pptp->info->getcrosststamp(pptp->info, xtstamp);
+	if (pptp->info->getfreeruncrosststamp)
+		err = pptp->info->getfreeruncrosststamp(pptp->info, xtstamp);
+	else
+		err = pptp->info->getcrosststamp(pptp->info, xtstamp);
 	if (err)
 		return err;
 
@@ -143,7 +149,9 @@  static u64 ptp_vclock_read(const struct cyclecounter *cc)
 	struct ptp_clock *ptp = vclock->pclock;
 	struct timespec64 ts = {};
 
-	if (ptp->info->gettimex64)
+	if (ptp->info->getfreeruntimex64)
+		ptp->info->getfreeruntimex64(ptp->info, &ts, NULL);
+	else if (ptp->info->gettimex64)
 		ptp->info->gettimex64(ptp->info, &ts, NULL);
 	else
 		ptp->info->gettime64(ptp->info, &ts);
@@ -168,11 +176,13 @@  struct ptp_vclock *ptp_vclock_register(struct ptp_clock *pclock)
 
 	vclock->pclock = pclock;
 	vclock->info = ptp_vclock_info;
-	if (pclock->info->gettimex64)
+	if (pclock->info->getfreeruntimex64 || pclock->info->gettimex64)
 		vclock->info.gettimex64 = ptp_vclock_gettimex;
 	else
 		vclock->info.gettime64 = ptp_vclock_gettime;
-	if (pclock->info->getcrosststamp)
+	if ((pclock->info->getfreeruntimex64 &&
+	     pclock->info->getfreeruncrosststamp) ||
+	    pclock->info->getcrosststamp)
 		vclock->info.getcrosststamp = ptp_vclock_getcrosststamp;
 	vclock->cc = ptp_vclock_cc;
 
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 554454cb8693..b291517fc7c8 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -108,6 +108,28 @@  struct ptp_system_timestamp {
  * @settime64:  Set the current time on the hardware clock.
  *              parameter ts: Time value to set.
  *
+ * @getfreeruntimex64:  Reads the current free running time from the hardware
+ *                      clock and optionally also the system clock. This
+ *                      operation requires hardware support for an additional
+ *                      free running time including support for hardware time
+ *                      stamps based on that free running time.
+ *                      The free running time must be completely independet from
+ *                      the actual time of the PTP clock. It must be monotonic
+ *                      and its frequency must be constant.
+ *                      parameter ts: Holds the PHC free running timestamp.
+ *                      parameter sts: If not NULL, it holds a pair of
+ *                      timestamps from the system clock. The first reading is
+ *                      made right before reading the lowest bits of the PHC
+ *                      free running timestamp and the second reading
+ *                      immediately follows that.
+ *
+ * @getfreeruncrosststamp:  Reads the current time from the free running
+ *                          hardware clock and system clock simultaneously.
+ *                          parameter cts: Contains timestamp (device,system)
+ *                          pair, where device time is the free running time
+ *                          also used for @getfreeruntimex64 and system time is
+ *                          realtime and monotonic.
+ *
  * @enable:   Request driver to enable or disable an ancillary feature.
  *            parameter request: Desired resource to enable or disable.
  *            parameter on: Caller passes one to enable or zero to disable.
@@ -155,6 +177,11 @@  struct ptp_clock_info {
 	int (*getcrosststamp)(struct ptp_clock_info *ptp,
 			      struct system_device_crosststamp *cts);
 	int (*settime64)(struct ptp_clock_info *p, const struct timespec64 *ts);
+	int (*getfreeruntimex64)(struct ptp_clock_info *ptp,
+				 struct timespec64 *ts,
+				 struct ptp_system_timestamp *sts);
+	int (*getfreeruncrosststamp)(struct ptp_clock_info *ptp,
+				     struct system_device_crosststamp *cts);
 	int (*enable)(struct ptp_clock_info *ptp,
 		      struct ptp_clock_request *request, int on);
 	int (*verify)(struct ptp_clock_info *ptp, unsigned int pin,