| Message ID | 20241009-devel-anna-maria-b4-timers-ptp-timekeeping-v2-14-554456a44a15@linutronix.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | timekeeping: Rework to prepare support of indenpendent PTP clocks | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Wed, Oct 9, 2024 at 1:29 AM Anna-Maria Behnsen <anna-maria@linutronix.de> wrote: > > Instead of explicitly listing all the separate timekeeping actions flags, > introduce a new one which covers all actions except TK_MIRROR action. > > No functional change. > > Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de> > --- > kernel/time/timekeeping.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c > index fcb2b8b232d2..5a747afe64b4 100644 > --- a/kernel/time/timekeeping.c > +++ b/kernel/time/timekeeping.c > @@ -33,6 +33,8 @@ > #define TK_MIRROR (1 << 1) > #define TK_CLOCK_WAS_SET (1 << 2) > > +#define TK_UPDATE_ALL (TK_CLEAR_NTP | TK_CLOCK_WAS_SET) > + Hrm. I feel a little wary around having a flag mask called _ALL when it doesn't actually include all the other flags. I also recognize the "TK_CLEAR_NTP | TK_CLOCK_WAS_SET" arguments can feel repetitive, but I find having them explicitly listed makes the code more readable to me. Combining these common ones together just means there is a 4th option one has to keep in their head to translate. Further, as I look through the logic TK_MIRROR could probably be improved by adding a direction (it's easy to mix up what is being mirrored to what). Maybe TK_MIRROR_TO_SHADOW? But these are mostly just strategies to help my scatterbrained state, so this isn't a hard objection if you disagree. thanks -john
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index fcb2b8b232d2..5a747afe64b4 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -33,6 +33,8 @@ #define TK_MIRROR (1 << 1) #define TK_CLOCK_WAS_SET (1 << 2) +#define TK_UPDATE_ALL (TK_CLEAR_NTP | TK_CLOCK_WAS_SET) + enum timekeeping_adv_mode { /* Update timekeeper when a tick has passed */ TK_ADV_TICK, @@ -1493,7 +1495,7 @@ int do_settimeofday64(const struct timespec64 *ts) tk_set_xtime(tk, ts); out: - timekeeping_update(&tk_core, tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET); + timekeeping_update(&tk_core, tk, TK_UPDATE_ALL | TK_MIRROR); write_seqcount_end(&tk_core.seq); raw_spin_unlock_irqrestore(&tk_core.lock, flags); @@ -1543,7 +1545,7 @@ static int timekeeping_inject_offset(const struct timespec64 *ts) tk_set_wall_to_mono(tk, timespec64_sub(tk->wall_to_monotonic, *ts)); error: /* even if we error out, we forwarded the time, so call update */ - timekeeping_update(&tk_core, tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET); + timekeeping_update(&tk_core, tk, TK_UPDATE_ALL | TK_MIRROR); write_seqcount_end(&tk_core.seq); raw_spin_unlock_irqrestore(&tk_core.lock, flags); @@ -1628,7 +1630,7 @@ static int change_clocksource(void *data) timekeeping_forward_now(tk); old = tk->tkr_mono.clock; tk_setup_internals(tk, new); - timekeeping_update(&tk_core, tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET); + timekeeping_update(&tk_core, tk, TK_UPDATE_ALL | TK_MIRROR); write_seqcount_end(&tk_core.seq); raw_spin_unlock_irqrestore(&tk_core.lock, flags); @@ -1919,7 +1921,7 @@ void timekeeping_inject_sleeptime64(const struct timespec64 *delta) __timekeeping_inject_sleeptime(tk, delta); - timekeeping_update(&tk_core, tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET); + timekeeping_update(&tk_core, tk, TK_UPDATE_ALL | TK_MIRROR); write_seqcount_end(&tk_core.seq); raw_spin_unlock_irqrestore(&tk_core.lock, flags);
Instead of explicitly listing all the separate timekeeping actions flags, introduce a new one which covers all actions except TK_MIRROR action. No functional change. Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de> --- kernel/time/timekeeping.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)