RE: [PATCH 3/6] Timer: expose monotonic clock and counter value

[Jianyong Wu (Arm Technology China)]

Hi Marc,

> -----Original Message-----
> From: Marc Zyngier <maz@xxxxxxxxxx>
> Sent: Wednesday, September 18, 2019 1:10 AM
> To: Jianyong Wu (Arm Technology China) <Jianyong.Wu@xxxxxxx>;
> netdev@xxxxxxxxxxxxxxx; pbonzini@xxxxxxxxxx;
> sean.j.christopherson@xxxxxxxxx; richardcochran@xxxxxxxxx; Mark Rutland
> <Mark.Rutland@xxxxxxx>; Will Deacon <Will.Deacon@xxxxxxx>; Suzuki
> Poulose <Suzuki.Poulose@xxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx; Steve Capper <Steve.Capper@xxxxxxx>;
> Kaly Xin (Arm Technology China) <Kaly.Xin@xxxxxxx>; Justin He (Arm
> Technology China) <Justin.He@xxxxxxx>; nd <nd@xxxxxxx>; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH 3/6] Timer: expose monotonic clock and counter value
> 
> On 17/09/2019 12:24, Jianyong Wu wrote:
> > A number of PTP drivers (such as ptp-kvm) are assuming what the
> > current clock source is, which could lead to interesting effects on
> > systems where the clocksource can change depending on external events.
> >
> > For this purpose, add a new API that retrives both the current
> > monotonic clock as well as its counter value.
> >
> > Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> > Signed-off-by: Jianyong Wu <jianyong.wu@xxxxxxx>
> 
> There must be something wrong with the way you've taken this patch in your
> tree. My authorship is gone (not that I deeply care about it, but it is good
> practice to keep attributions), and the subject line has been rewritten.
> 
> I'd appreciate it if you could fix this in a future revision of this series. For
> reference, the original patch is here[1].
> 

Sorry for "steal" your patch, I'm not familiar with it and neglect this important change.
I just copy this patch from your email and add the subject myself.
I will fix all of them later in v3.

> > ---
> >  include/linux/timekeeping.h |  3 +++
> >  kernel/time/timekeeping.c   | 13 +++++++++++++
> >  2 files changed, 16 insertions(+)
> >
> > diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
> > index a8ab0f143ac4..a5389adaa8bc 100644
> > --- a/include/linux/timekeeping.h
> > +++ b/include/linux/timekeeping.h
> > @@ -247,6 +247,9 @@ extern int get_device_system_crosststamp(
> >  			struct system_time_snapshot *history,
> >  			struct system_device_crosststamp *xtstamp);
> >
> > +/* Obtain current monotonic clock and its counter value  */ extern
> > +void get_current_counterval(struct system_counterval_t *sc);
> > +
> >  /*
> >   * Simultaneously snapshot realtime and monotonic raw clocks
> >   */
> > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> > index 44b726bab4bd..07a0969625b1 100644
> > --- a/kernel/time/timekeeping.c
> > +++ b/kernel/time/timekeeping.c
> > @@ -1098,6 +1098,19 @@ static bool cycle_between(u64 before, u64 test,
> u64 after)
> >  	return false;
> >  }
> >
> > +/**
> > + * get_current_counterval - Snapshot the current clocksource and counter
> value
> > + * @sc:	Pointer to a struct containing the current clocksource and its
> value
> > + */
> > +void get_current_counterval(struct system_counterval_t *sc) {
> > +	struct timekeeper *tk = &tk_core.timekeeper;
> > +
> > +	sc->cs = READ_ONCE(tk->tkr_mono.clock);
> > +	sc->cycles = sc->cs->read(sc->cs);
> > +}
> > +EXPORT_SYMBOL_GPL(get_current_counterval);
> 
> This export wasn't in my original patch. I guess you need it because your ptp
> driver builds as a module? It'd be good to mention it in the commit log.
> 
Yeah, ptp_kvm will be a module, so export is necessary. I will mention this change
in commit log.

Thanks
Jianyong Wu

> > +
> >  /**
> >   * get_device_system_crosststamp - Synchronously capture
> system/device timestamp
> >   * @get_time_fn:	Callback to get simultaneous device time and
> >
> 
> Thanks,
> 
> 	M.
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-
> platforms.git/commit/?h=timer/counterval&id=a6e8abce025691b6a55e1c195
> 878d7f76bfeb9d1
> --
> Jazz is not dead, it just smells funny...