Felipe, On Thu, 15 Aug 2019, Felipe Balbi wrote: > Thomas Gleixner <t...@linutronix.de> writes: > > On Tue, 16 Jul 2019, Felipe Balbi wrote: > > > > So some information what those interfaces are used for and why they are > > needed would be really helpful. > > Okay, I have some more details about this. The TGPIO device itself uses > ART since TSC is not directly available to anything other than the > CPU. The 'problem' here is that reading ART incurs extra latency which > we would like to avoid. Therefore, we use TSC and scale it to > nanoseconds which, would be the same as ART to ns.
Fine. But that's not really correct: TSC = art_to_tsc_offset + ART * scale; > >> +void get_tsc_ns(struct system_counterval_t *tsc_counterval, u64 *tsc_ns) Why is this not returning the result instead of having that pointer indirection? > >> +{ > >> + u64 tmp, res, rem; > >> + u64 cycles; > >> + > >> + tsc_counterval->cycles = clocksource_tsc.read(NULL); > >> + cycles = tsc_counterval->cycles; > >> + tsc_counterval->cs = art_related_clocksource; So this does more than returning the TSC time converted to nanoseconds. The function name should reflect this. Plus both functions want kernel-doc explaining what they do. > >> + rem = do_div(cycles, tsc_khz); > >> + > >> + res = cycles * USEC_PER_SEC; > >> + tmp = rem * USEC_PER_SEC; > >> + > >> + do_div(tmp, tsc_khz); > >> + res += tmp; > >> + > >> + *tsc_ns = res; > >> +} > >> +EXPORT_SYMBOL(get_tsc_ns); > >> + > >> +u64 get_art_ns_now(void) > >> +{ > >> + struct system_counterval_t tsc_cycles; > >> + u64 tsc_ns; > >> + > >> + get_tsc_ns(&tsc_cycles, &tsc_ns); > >> + > >> + return tsc_ns; > >> +} > >> +EXPORT_SYMBOL(get_art_ns_now); > > > > While the changes look innocuous I'm missing the big picture why this needs > > to emulate ART instead of simply using TSC directly. > > i don't think we're emulating ART here (other than the name in the > function). We're just reading TSC and converting to nanoseconds, right? Well, the function name says clearly: get_art_ns_now(). But you are not using ART, you use the TSC and derive the ART value from it (incorrectly). Thanks, tglx