diff mbox series

[v7,01/12] timekeeping: Add base clock properties in clocksource structure

Message ID 20240430085225.18086-2-lakshmi.sowjanya.d@intel.com (mailing list archive)
State Superseded
Headers show
Series Add support for Intel PPS Generator | expand

Commit Message

D, Lakshmi Sowjanya April 30, 2024, 8:52 a.m. UTC
From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>

Add base clock hardware abstraction in clocksource structure.

Provide generic functionality in convert_base_to_cs() to convert base
clock timestamps to system clocksource without requiring architecture
specific parameters.

This is intended to replace convert_art_to_tsc() and
convert_art_ns_to_tsc() functions which are specific to convert ART
(Always Running Timer) time to the corresponding TSC value.

Add the infrastructure in get_device_system_crosststamp().

Co-developed-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Co-developed-by: Christopher S. Hall <christopher.s.hall@intel.com>
Signed-off-by: Christopher S. Hall <christopher.s.hall@intel.com>
Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
---
 include/linux/clocksource.h | 27 +++++++++++++++++++++++++
 include/linux/timekeeping.h |  2 ++
 kernel/time/timekeeping.c   | 39 +++++++++++++++++++++++++++++++++++--
 3 files changed, 66 insertions(+), 2 deletions(-)

Comments

Peter Hilber April 30, 2024, 2:28 p.m. UTC | #1
On 30.04.24 10:52, lakshmi.sowjanya.d@intel.com wrote:
> From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> 
> Add base clock hardware abstraction in clocksource structure.
> 
> Provide generic functionality in convert_base_to_cs() to convert base
> clock timestamps to system clocksource without requiring architecture
> specific parameters.
> 
> This is intended to replace convert_art_to_tsc() and
> convert_art_ns_to_tsc() functions which are specific to convert ART
> (Always Running Timer) time to the corresponding TSC value.
> 
> Add the infrastructure in get_device_system_crosststamp().
> 
> Co-developed-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Co-developed-by: Christopher S. Hall <christopher.s.hall@intel.com>
> Signed-off-by: Christopher S. Hall <christopher.s.hall@intel.com>
> Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> ---
>  include/linux/clocksource.h | 27 +++++++++++++++++++++++++
>  include/linux/timekeeping.h |  2 ++
>  kernel/time/timekeeping.c   | 39 +++++++++++++++++++++++++++++++++++--
>  3 files changed, 66 insertions(+), 2 deletions(-)
> 
[...]
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index b58dffc58a8f..4e5e931e766a 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -1193,6 +1193,42 @@ static bool timestamp_in_interval(u64 start, u64 end, u64 ts)
>  	return false;
>  }
>  
> +static bool convert_clock(u64 *val, u32 numerator, u32 denominator)
> +{
> +	u64 rem, res;
> +
> +	if (!numerator || !denominator)
> +		return false;
> +
> +	res = div64_u64_rem(*val, denominator, &rem) * numerator;
> +	*val = res + div_u64(rem * numerator, denominator);
> +	return true;
> +}
> +
> +static bool convert_base_to_cs(struct system_counterval_t *scv)
> +{
> +	struct clocksource *cs = tk_core.timekeeper.tkr_mono.clock;
> +	struct clocksource_base *base = cs->base;
> +	u32 num, den;
> +
> +	/* The timestamp was taken from the time keeper clock source */
> +	if (cs->id == scv->cs_id)
> +		return true;
> +
> +	/* Check whether cs_id matches the base clock */
> +	if (!base || base->id != scv->cs_id)
> +		return false;
> +
> +	num = scv->use_nsecs ? cs->freq_khz : base->numerator;
> +	den = scv->use_nsecs ? USEC_PER_SEC : base->denominator;
> +
> +	if (!convert_clock(&scv->cycles, num, den))
> +		return false;
> +
> +	scv->cycles += base->offset;
> +	return true;
> +}
> +
>  /**
>   * get_device_system_crosststamp - Synchronously capture system/device timestamp
>   * @get_time_fn:	Callback to get simultaneous device time and
> @@ -1238,8 +1274,7 @@ int get_device_system_crosststamp(int (*get_time_fn)
>  		 * system counter value is the same as for the currently
>  		 * installed timekeeper clocksource
>  		 */
> -		if (system_counterval.cs_id == CSID_GENERIC ||
> -		    tk->tkr_mono.clock->id != system_counterval.cs_id)
> +		if (!convert_base_to_cs(&system_counterval))
>  			return -ENODEV;

AFAIU the error condition

	system_counterval.cs_id == CSID_GENERIC

is silently dropped by this patch, but shouldn't be. 

get_device_system_crosststamp() can only check the identity of a
clocksource (base) for non-generic ids.

Regards,

Peter
D, Lakshmi Sowjanya May 9, 2024, 4:38 a.m. UTC | #2
> -----Original Message-----
> From: Peter Hilber <peter.hilber@opensynergy.com>
> Sent: Tuesday, April 30, 2024 7:58 PM
> To: D, Lakshmi Sowjanya <lakshmi.sowjanya.d@intel.com>;
> tglx@linutronix.de; jstultz@google.com; giometti@enneenne.com;
> corbet@lwn.net; linux-kernel@vger.kernel.org
> Cc: x86@kernel.org; netdev@vger.kernel.org; linux-doc@vger.kernel.org;
> intel-wired-lan@lists.osuosl.org; andriy.shevchenko@linux.intel.com; Dong,
> Eddie <eddie.dong@intel.com>; Hall, Christopher S
> <christopher.s.hall@intel.com>; Brandeburg, Jesse
> <jesse.brandeburg@intel.com>; davem@davemloft.net;
> alexandre.torgue@foss.st.com; joabreu@synopsys.com;
> mcoquelin.stm32@gmail.com; perex@perex.cz; linux-
> sound@vger.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>;
> N, Pandith <pandith.n@intel.com>; Mohan, Subramanian
> <subramanian.mohan@intel.com>; T R, Thejesh Reddy
> <thejesh.reddy.t.r@intel.com>
> Subject: Re: [PATCH v7 01/12] timekeeping: Add base clock properties in
> clocksource structure
> 
> On 30.04.24 10:52, lakshmi.sowjanya.d@intel.com wrote:
> > From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> >
> > Add base clock hardware abstraction in clocksource structure.
> >
> > Provide generic functionality in convert_base_to_cs() to convert base
> > clock timestamps to system clocksource without requiring architecture
> > specific parameters.
> >
> > This is intended to replace convert_art_to_tsc() and
> > convert_art_ns_to_tsc() functions which are specific to convert ART
> > (Always Running Timer) time to the corresponding TSC value.
> >
> > Add the infrastructure in get_device_system_crosststamp().
> >
> > Co-developed-by: Thomas Gleixner <tglx@linutronix.de>
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > Co-developed-by: Christopher S. Hall <christopher.s.hall@intel.com>
> > Signed-off-by: Christopher S. Hall <christopher.s.hall@intel.com>
> > Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> > ---
> >  include/linux/clocksource.h | 27 +++++++++++++++++++++++++
> > include/linux/timekeeping.h |  2 ++
> >  kernel/time/timekeeping.c   | 39
> +++++++++++++++++++++++++++++++++++--
> >  3 files changed, 66 insertions(+), 2 deletions(-)
> >
> [...]
> > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> > index b58dffc58a8f..4e5e931e766a 100644
> > --- a/kernel/time/timekeeping.c
> > +++ b/kernel/time/timekeeping.c
> > @@ -1193,6 +1193,42 @@ static bool timestamp_in_interval(u64 start, u64
> end, u64 ts)
> >  	return false;
> >  }
> >
> > +static bool convert_clock(u64 *val, u32 numerator, u32 denominator) {
> > +	u64 rem, res;
> > +
> > +	if (!numerator || !denominator)
> > +		return false;
> > +
> > +	res = div64_u64_rem(*val, denominator, &rem) * numerator;
> > +	*val = res + div_u64(rem * numerator, denominator);
> > +	return true;
> > +}
> > +
> > +static bool convert_base_to_cs(struct system_counterval_t *scv) {
> > +	struct clocksource *cs = tk_core.timekeeper.tkr_mono.clock;
> > +	struct clocksource_base *base = cs->base;
> > +	u32 num, den;
> > +
> > +	/* The timestamp was taken from the time keeper clock source */
> > +	if (cs->id == scv->cs_id)
> > +		return true;
> > +
> > +	/* Check whether cs_id matches the base clock */
> > +	if (!base || base->id != scv->cs_id)
> > +		return false;
> > +
> > +	num = scv->use_nsecs ? cs->freq_khz : base->numerator;
> > +	den = scv->use_nsecs ? USEC_PER_SEC : base->denominator;
> > +
> > +	if (!convert_clock(&scv->cycles, num, den))
> > +		return false;
> > +
> > +	scv->cycles += base->offset;
> > +	return true;
> > +}
> > +
> >  /**
> >   * get_device_system_crosststamp - Synchronously capture
> system/device timestamp
> >   * @get_time_fn:	Callback to get simultaneous device time and
> > @@ -1238,8 +1274,7 @@ int get_device_system_crosststamp(int
> (*get_time_fn)
> >  		 * system counter value is the same as for the currently
> >  		 * installed timekeeper clocksource
> >  		 */
> > -		if (system_counterval.cs_id == CSID_GENERIC ||
> > -		    tk->tkr_mono.clock->id != system_counterval.cs_id)
> > +		if (!convert_base_to_cs(&system_counterval))
> >  			return -ENODEV;
> 
> AFAIU the error condition
> 
> 	system_counterval.cs_id == CSID_GENERIC
> 
> is silently dropped by this patch, but shouldn't be.
> 
> get_device_system_crosststamp() can only check the identity of a
> clocksource (base) for non-generic ids.
> 
> Regards,
> 
> Peter

Thanks Peter,
Noted. The check will be added as below:

if (system_counterval.cs_id == CSID_GENERIC ||
                         !convert_base_to_cs(&system_counterval))

Regards,
Sowjanya
diff mbox series

Patch

diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 0ad8b550bb4b..d35b677b08fe 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -21,6 +21,7 @@ 
 #include <asm/div64.h>
 #include <asm/io.h>
 
+struct clocksource_base;
 struct clocksource;
 struct module;
 
@@ -50,6 +51,7 @@  struct module;
  *			multiplication
  * @name:		Pointer to clocksource name
  * @list:		List head for registration (internal)
+ * @freq_khz:		Clocksource frequency in khz.
  * @rating:		Rating value for selection (higher is better)
  *			To avoid rating inflation the following
  *			list should give you a guide as to how
@@ -70,6 +72,8 @@  struct module;
  *			validate the clocksource from which the snapshot was
  *			taken.
  * @flags:		Flags describing special properties
+ * @base:		Hardware abstraction for clock on which a clocksource
+ *			is based
  * @enable:		Optional function to enable the clocksource
  * @disable:		Optional function to disable the clocksource
  * @suspend:		Optional suspend function for the clocksource
@@ -107,10 +111,12 @@  struct clocksource {
 	u64			max_cycles;
 	const char		*name;
 	struct list_head	list;
+	u32			freq_khz;
 	int			rating;
 	enum clocksource_ids	id;
 	enum vdso_clock_mode	vdso_clock_mode;
 	unsigned long		flags;
+	struct clocksource_base *base;
 
 	int			(*enable)(struct clocksource *cs);
 	void			(*disable)(struct clocksource *cs);
@@ -306,4 +312,25 @@  static inline unsigned int clocksource_get_max_watchdog_retry(void)
 
 void clocksource_verify_percpu(struct clocksource *cs);
 
+/**
+ * struct clocksource_base - hardware abstraction for clock on which a clocksource
+ *			is based
+ * @id:			Defaults to CSID_GENERIC. The id value is used for conversion
+ *			functions which require that the current clocksource is based
+ *			on a clocksource_base with a particular ID in certain snapshot
+ *			functions to allow callers to validate the clocksource from
+ *			which the snapshot was taken.
+ * @freq_khz:		Nominal frequency of the base clock in kHz
+ * @offset:		Offset between the base clock and the clocksource
+ * @numerator:		Numerator of the clock ratio between base clock and the clocksource
+ * @denominator:	Denominator of the clock ratio between base clock and the clocksource
+ */
+struct clocksource_base {
+	enum clocksource_ids	id;
+	u32			freq_khz;
+	u64			offset;
+	u32			numerator;
+	u32			denominator;
+};
+
 #endif /* _LINUX_CLOCKSOURCE_H */
diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index 0ea7823b7f31..b2ee182d891e 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -310,10 +310,12 @@  struct system_device_crosststamp {
  *		timekeeping code to verify comparability of two cycle values.
  *		The default ID, CSID_GENERIC, does not identify a specific
  *		clocksource.
+ * @use_nsecs:	@cycles is in nanoseconds.
  */
 struct system_counterval_t {
 	u64			cycles;
 	enum clocksource_ids	cs_id;
+	bool			use_nsecs;
 };
 
 /*
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index b58dffc58a8f..4e5e931e766a 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1193,6 +1193,42 @@  static bool timestamp_in_interval(u64 start, u64 end, u64 ts)
 	return false;
 }
 
+static bool convert_clock(u64 *val, u32 numerator, u32 denominator)
+{
+	u64 rem, res;
+
+	if (!numerator || !denominator)
+		return false;
+
+	res = div64_u64_rem(*val, denominator, &rem) * numerator;
+	*val = res + div_u64(rem * numerator, denominator);
+	return true;
+}
+
+static bool convert_base_to_cs(struct system_counterval_t *scv)
+{
+	struct clocksource *cs = tk_core.timekeeper.tkr_mono.clock;
+	struct clocksource_base *base = cs->base;
+	u32 num, den;
+
+	/* The timestamp was taken from the time keeper clock source */
+	if (cs->id == scv->cs_id)
+		return true;
+
+	/* Check whether cs_id matches the base clock */
+	if (!base || base->id != scv->cs_id)
+		return false;
+
+	num = scv->use_nsecs ? cs->freq_khz : base->numerator;
+	den = scv->use_nsecs ? USEC_PER_SEC : base->denominator;
+
+	if (!convert_clock(&scv->cycles, num, den))
+		return false;
+
+	scv->cycles += base->offset;
+	return true;
+}
+
 /**
  * get_device_system_crosststamp - Synchronously capture system/device timestamp
  * @get_time_fn:	Callback to get simultaneous device time and
@@ -1238,8 +1274,7 @@  int get_device_system_crosststamp(int (*get_time_fn)
 		 * system counter value is the same as for the currently
 		 * installed timekeeper clocksource
 		 */
-		if (system_counterval.cs_id == CSID_GENERIC ||
-		    tk->tkr_mono.clock->id != system_counterval.cs_id)
+		if (!convert_base_to_cs(&system_counterval))
 			return -ENODEV;
 		cycles = system_counterval.cycles;