Subject: Re: [PATCH] cxl: Set up and enable PSL Timebase

From: Michael Neuling <[email-masked]>
Date: Mon, 01 Jun 2015 19:08:08 +1000

On Mon, 2015-06-01 at 09:37 +0200, Philippe Bergheaud wrote:
> Michael Neuling wrote:
> > On Thu, 2015-05-28 at 15:12 +0200, Philippe Bergheaud wrote:
> > 
> >>This patch configures the PSL Timebase function and enables it,
> >>after the CAPP has been initialized by OPAL. Failures are reported
> >>and ignored.
> > 
> > 
> > Needs an Signed-off-by.
> Yes.
> > Comments inline.
> > 
> > 
> >>---
> >> drivers/misc/cxl/cxl.h |    5 +++++
> >> drivers/misc/cxl/pci.c |   35 +++++++++++++++++++++++++++++++++++
> >> 2 files changed, 40 insertions(+), 0 deletions(-)
> >>
> >>diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
> >>index a1cee47..38a7cf9 100644
> >>--- a/drivers/misc/cxl/cxl.h
> >>+++ b/drivers/misc/cxl/cxl.h
> >>@@ -82,8 +82,10 @@ static const cxl_p1_reg_t CXL_PSL_AFUSEL  = {0x00B0};
> >> /* 0x00C0:7EFF Implementation dependent area */
> >> static const cxl_p1_reg_t CXL_PSL_FIR1      = {0x0100};
> >> static const cxl_p1_reg_t CXL_PSL_FIR2      = {0x0108};
> >>+static const cxl_p1_reg_t CXL_PSL_Timebase  = {0x0110};
> >> static const cxl_p1_reg_t CXL_PSL_VERSION   = {0x0118};
> >> static const cxl_p1_reg_t CXL_PSL_RESLCKTO  = {0x0128};
> >>+static const cxl_p1_reg_t CXL_PSL_TB_CTLSTAT = {0x0140};
> >> static const cxl_p1_reg_t CXL_PSL_FIR_CNTL  = {0x0148};
> >> static const cxl_p1_reg_t CXL_PSL_DSNDCTL   = {0x0150};
> >> static const cxl_p1_reg_t CXL_PSL_SNWRALLOC = {0x0158};
> >>@@ -151,6 +153,9 @@ static const cxl_p2n_reg_t CXL_PSL_WED_An     = {0x0A0};
> >> #define CXL_PSL_SPAP_Size_Shift 4
> >> #define CXL_PSL_SPAP_V    0x0000000000000001ULL
> >> 
> >>+/****** CXL_PSL_Control ****************************************************/
> >>+#define CXL_PSL_Control_tb 0x0000000000000001ULL
> >>+
> >> /****** CXL_PSL_DLCNTL *****************************************************/
> >> #define CXL_PSL_DLCNTL_D (0x1ull << (63-28))
> >> #define CXL_PSL_DLCNTL_C (0x1ull << (63-29))
> >>diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> >>index fc938de..afd89cc 100644
> >>--- a/drivers/misc/cxl/pci.c
> >>+++ b/drivers/misc/cxl/pci.c
> >>@@ -360,6 +360,38 @@ static int init_implementation_adapter_regs(struct cxl *adapter, struct pci_dev
> >> 	return 0;
> >> }
> >> 
> >>+#define TBSYNC_CNT(n) (((u64)n & 0x7) << (63-6))
> >>+
> >>+static int cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *dev)
> >>+{
> >>+	u64 psl_tb;
> >>+	int delta;
> >>+	unsigned int retry = 0;
> >>+
> >>+	/*
> >>+	 * Setup PSL Timebase Control and Status register
> >>+	 * with the recommended Timebase Sync Count value
> >>+	 */
> >>+	cxl_p1_write(adapter, CXL_PSL_TB_CTLSTAT, TBSYNC_CNT(2));  
> > 
> > 
> > 2?  
> Quoting the PSL workbook description of the PSL_TB_CTLSTAT register:
> 
> 4:6 tbsync_cnt
> TimebaseSyncCount. Number of 250MHz cycles x 2048 before initiating another Timebase Recalibration sequence.
> Processor chipTimebase facilities receive a tod_sync pulse every 16us or 4000 250 MHz cycles so '010' is the Recommended value.
> 000 = never
> 001 = 2048
> 010 = 4096 (2 * 2048)
> ...
> 111 = 14336 (7 * 2048)
> 
> Will make the TimebaseSyncCount unit explicit. Something like:
> 
> #define _2048_250MHZ_CYCLES 1
> cxl_p1_write(adapter, CXL_PSL_TB_CTLSTAT, TBSYNC_CNT(2 * _2048_250MHZ_CYCLES));

Sounds good!

> > 
> >>+
> >>+	/* Enable PSL Timebase */
> >>+	cxl_p1_write(adapter, CXL_PSL_Control, CXL_PSL_Control_tb);
> >>+	/* Wait until CORE TB and PSL TB difference <= 16usecs */
> > 
> > 
> > How many tries does this normally take?
> Two. The second attempt always succeds.

Ok

> > Should we have a sleep in here to wait for it to sync rather than just
> > coming back around right away?
> Yes, will add msleep(1) at the beginning of the loop (as the first attempt always fails).

Humm, ok.  Is there any documentation to say how long it's suppose to
take?  

Mikey