| Message ID | 20240430085225.18086-2-lakshmi.sowjanya.d@intel.com |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Add support for Intel PPS Generator | expand |
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
> -----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 --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;