[Skiboot] [PATCH] chiptod: Keep boot timestamps contiguous

[Vaidyanathan Srinivasan]

* Michael Neuling <mikey at neuling.org> [2017-12-05 14:23:13]:

> On Tue, 2017-12-05 at 14:19 +1100, Oliver O'Halloran wrote:
> > Currently we reset the timebase value to (almost) zero when
> > synchronising the timebase of each chip to the Chip TOD network which
> > results in this:
> > 
> > [   42.374813167,5] CPU: All 80 processors called in...
> > [    2.222791151,5] FLASH: Found system flash: Macronix MXxxL51235F id:0
> > [    2.222977933,5] BT: Interface initialized, IO 0x00e4
> > 
> > This patch modifies the chiptod_init() process to use the current
> > timebase value rather than resetting it to zero. This results in the
> > timestamps remaining contigious from the start of hostboot until
> > the petikernel starts. e.g.
> > 
> > [   70.188811484,5] CPU: All 144 processors called in...
> > [   72.458004252,5] FLASH: Found system flash:  id:0
> > [   72.458147358,5] BT: Interface initialized, IO 0x00e4
> > 
> > Signed-off-by: Oliver O'Halloran <oohall at gmail.com>
> 
> Hell-Yeahed-by: Michael Neuling <mikey at neuling.org>

Thanks for the patch.  Good usability improvement for people looking
at the console.

Reviewed-by: Vaidyanathan Srinivasan <svaidy at linux.vnet.ibm.com>

> 
> > ---
> > Tested on a Witherspoon and a P8 FSP box. Seems to work ok in both
> > cases.
> > ---
> >  core/init.c  |  8 +++++---
> >  hw/chiptod.c | 15 +++++++++++----
> >  2 files changed, 16 insertions(+), 7 deletions(-)
> > 
> > diff --git a/core/init.c b/core/init.c
> > index 89a2758128e2..d008e2dc40f2 100644
> > --- a/core/init.c
> > +++ b/core/init.c
> > @@ -966,9 +966,11 @@ void __noreturn __nomcount main_cpu_entry(const void
> > *fdt)
> >  	cpu_set_sreset_enable(true);
> >  
> >  	/*
> > -	 * Synchronize time bases. Thi resets all the TB values to a small
> > -	 * value (so they appear to go backward at this point), and
> > synchronize
> > -	 * all core timebases to the global ChipTOD network
> > +	 * Synchronize time bases. Prior to chiptod_init() the timebase
> > +	 * is free-running at a frequency based on the core clock rather
> > +	 * than being synchronised to the ChipTOD network. This means
> > +	 * that the timestamps in early boot might be a little off compared
> > +	 * to wall clock time.
> >  	 */
> >  	chiptod_init();
> >  
> > diff --git a/hw/chiptod.c b/hw/chiptod.c
> > index b9e47741bf21..3a0940d2b8bc 100644
> > --- a/hw/chiptod.c
> > +++ b/hw/chiptod.c
> > @@ -120,9 +120,6 @@
> >  #define LOCAL_CORE_FIR		0x0104000C
> >  #define LFIR_SWITCH_COMPLETE	PPC_BIT(18)
> >  
> > -/* Magic TB value. One step cycle ahead of sync */
> > -#define INIT_TB	0x000000000001ff0
> > -
> >  /* Number of iterations for the various timeouts */
> >  #define TIMEOUT_LOOPS		20000000
> >  
> > @@ -775,6 +772,7 @@ static void chiptod_reset_tod_errors(void)
> >  
> >  static void chiptod_sync_master(void *data)
> >  {
> > +	uint64_t initial_tb_value;
> >  	bool *result = data;
> >  
> >  	prlog(PR_DEBUG, "Master sync on CPU PIR 0x%04x...\n",
> > @@ -824,8 +822,17 @@ static void chiptod_sync_master(void *data)
> >  		goto error;
> >  	}
> >  
> > +	/*
> > +	 * Load the master's current timebase value into the Chip TOD
> > +	 * network. This is so we have sane timestamps across the whole
> > +	 * IPL process. The Chip TOD documentation says that the loaded
> > +	 * value needs to be one STEP before a SYNC. In other words,
> > +	 * set the low bits to 0x1ff0.
> > +	 */
> > +	initial_tb_value = (mftb() & ~0x1fff) | 0x1ff0;

Good idea to sync all TB based on current value of one of them rather
than starting from zero.

> > +
> >  	/* Chip TOD load initial value */
> > -	if (xscom_writeme(TOD_CHIPTOD_LOAD_TB, INIT_TB)) {
> > +	if (xscom_writeme(TOD_CHIPTOD_LOAD_TB, initial_tb_value)) {
> >  		prerror("XSCOM error setting init TB\n");
> >  		goto error;
> >  	}

--Vaidy