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

[Philippe Bergheaud <felix@linux.vnet.ibm.com>]

Michael Neuling wrote:
> 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?  
Could not find any.
With msleep(1) at the beginning if the loop, the first attempt always succeeds, as far as I can see.

Philippe