[PATCH] cxl: Set up and enable PSL Timebase

[PATCH] cxl: Set up and enable PSL Timebase

[Philippe Bergheaud]

This patch configures the PSL Timebase function and enables it,
after the CAPP has been initialized by OPAL. Failures are reported
and ignored.
---
 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));
+
+	/* Enable PSL Timebase */
+	cxl_p1_write(adapter, CXL_PSL_Control, CXL_PSL_Control_tb);
+	/* Wait until CORE TB and PSL TB difference <= 16usecs */
+	do {
+		if (retry++ > 5) {
+			pr_err("PSL: Timebase sync: giving up!\n");
+			return 1;
+		}
+		psl_tb = cxl_p1_read(adapter, CXL_PSL_Timebase);
+		delta = mftb() - psl_tb;
+		if (delta < 0)
+			delta = -delta;
+	} while (cputime_to_usecs(delta) > 16);
+
+	dev_info(&dev->dev, "PSL: Timebase synced\n");
+	return 0;
+}
+
 static int init_implementation_afu_regs(struct cxl_afu *afu)
 {
 	/* read/write masks for this slice */
@@ -995,6 +1027,9 @@ static struct cxl *cxl_init_adapter(struct pci_dev *dev)
 	if ((rc = pnv_phb_to_cxl(dev, OPAL_PHB_CAPI_MODE_CAPI)))
 		goto err3;
 
+	/* Don't care if this one fails: */
+	cxl_setup_psl_timebase(adapter, dev);
+
 	if ((rc = cxl_register_psl_err_irq(adapter)))
 		goto err3;
 
-- 
1.7.2.5

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

[Michael Neuling]

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. =20

Comments inline.

> ---
>  drivers/misc/cxl/cxl.h |    5 +++++
>  drivers/misc/cxl/pci.c |   35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+), 0 deletions(-)
>=20
> 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  =3D {0x00B0}=
;
>  /* 0x00C0:7EFF Implementation dependent area */
>  static const cxl_p1_reg_t CXL_PSL_FIR1      =3D {0x0100};
>  static const cxl_p1_reg_t CXL_PSL_FIR2      =3D {0x0108};
> +static const cxl_p1_reg_t CXL_PSL_Timebase  =3D {0x0110};
>  static const cxl_p1_reg_t CXL_PSL_VERSION   =3D {0x0118};
>  static const cxl_p1_reg_t CXL_PSL_RESLCKTO  =3D {0x0128};
> +static const cxl_p1_reg_t CXL_PSL_TB_CTLSTAT =3D {0x0140};
>  static const cxl_p1_reg_t CXL_PSL_FIR_CNTL  =3D {0x0148};
>  static const cxl_p1_reg_t CXL_PSL_DSNDCTL   =3D {0x0150};
>  static const cxl_p1_reg_t CXL_PSL_SNWRALLOC =3D {0x0158};
> @@ -151,6 +153,9 @@ static const cxl_p2n_reg_t CXL_PSL_WED_An     =3D {0x=
0A0};
>  #define CXL_PSL_SPAP_Size_Shift 4
>  #define CXL_PSL_SPAP_V    0x0000000000000001ULL
> =20
> +/****** 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 c=
xl *adapter, struct pci_dev
>  	return 0;
>  }
> =20
> +#define TBSYNC_CNT(n) (((u64)n & 0x7) << (63-6))
> +
> +static int cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *d=
ev)
> +{
> +	u64 psl_tb;
> +	int delta;
> +	unsigned int retry =3D 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)); =20

2? =20

> +
> +	/* Enable PSL Timebase */
> +	cxl_p1_write(adapter, CXL_PSL_Control, CXL_PSL_Control_tb);
> +	/* Wait until CORE TB and PSL TB difference <=3D 16usecs */

How many tries does this normally take? =20

Should we have a sleep in here to wait for it to sync rather than just
coming back around right away?

> +	do {
> +		if (retry++ > 5) {
> +			pr_err("PSL: Timebase sync: giving up!\n");
> +			return 1;

Please use negative error codes here.  -EIO?

> +		}
> +		psl_tb =3D cxl_p1_read(adapter, CXL_PSL_Timebase);
> +		delta =3D mftb() - psl_tb;
> +		if (delta < 0)
> +			delta =3D -delta;
> +	} while (cputime_to_usecs(delta) > 16);
> +
> +	dev_info(&dev->dev, "PSL: Timebase synced\n");
> +	return 0;
> +}
> +
>  static int init_implementation_afu_regs(struct cxl_afu *afu)
>  {
>  	/* read/write masks for this slice */
> @@ -995,6 +1027,9 @@ static struct cxl *cxl_init_adapter(struct pci_dev *=
dev)
>  	if ((rc =3D pnv_phb_to_cxl(dev, OPAL_PHB_CAPI_MODE_CAPI)))
>  		goto err3;
> =20
> +	/* Don't care if this one fails: */
> +	cxl_setup_psl_timebase(adapter, dev);

And check it here.

Thanks,
Mikey

> +
>  	if ((rc =3D cxl_register_psl_err_irq(adapter)))
>  		goto err3;
> =20

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

[Philippe Bergheaud]

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));
> 
>>+
>>+	/* 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.
> 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).
> 
>>+	do {
>>+		if (retry++ > 5) {
>>+			pr_err("PSL: Timebase sync: giving up!\n");
>>+			return 1;
> 
> 
> Please use negative error codes here.  -EIO?
OK.
> 
>>+		}
>>+		psl_tb = cxl_p1_read(adapter, CXL_PSL_Timebase);
>>+		delta = mftb() - psl_tb;
>>+		if (delta < 0)
>>+			delta = -delta;
>>+	} while (cputime_to_usecs(delta) > 16);
>>+
>>+	dev_info(&dev->dev, "PSL: Timebase synced\n");
>>+	return 0;
>>+}
>>+
>> static int init_implementation_afu_regs(struct cxl_afu *afu)
>> {
>> 	/* read/write masks for this slice */
>>@@ -995,6 +1027,9 @@ static struct cxl *cxl_init_adapter(struct pci_dev *dev)
>> 	if ((rc = pnv_phb_to_cxl(dev, OPAL_PHB_CAPI_MODE_CAPI)))
>> 		goto err3;
>> 
>>+	/* Don't care if this one fails: */
>>+	cxl_setup_psl_timebase(adapter, dev);
> 
> 
> And check it here.
OK.

Thank you,
Philippe

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

[Michael Neuling]

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:
> >=20
> >>This patch configures the PSL Timebase function and enables it,
> >>after the CAPP has been initialized by OPAL. Failures are reported
> >>and ignored.
> >=20
> >=20
> > Needs an Signed-off-by.
> Yes.
> > Comments inline.
> >=20
> >=20
> >>---
> >> 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  =3D {0x00B=
0};
> >> /* 0x00C0:7EFF Implementation dependent area */
> >> static const cxl_p1_reg_t CXL_PSL_FIR1      =3D {0x0100};
> >> static const cxl_p1_reg_t CXL_PSL_FIR2      =3D {0x0108};
> >>+static const cxl_p1_reg_t CXL_PSL_Timebase  =3D {0x0110};
> >> static const cxl_p1_reg_t CXL_PSL_VERSION   =3D {0x0118};
> >> static const cxl_p1_reg_t CXL_PSL_RESLCKTO  =3D {0x0128};
> >>+static const cxl_p1_reg_t CXL_PSL_TB_CTLSTAT =3D {0x0140};
> >> static const cxl_p1_reg_t CXL_PSL_FIR_CNTL  =3D {0x0148};
> >> static const cxl_p1_reg_t CXL_PSL_DSNDCTL   =3D {0x0150};
> >> static const cxl_p1_reg_t CXL_PSL_SNWRALLOC =3D {0x0158};
> >>@@ -151,6 +153,9 @@ static const cxl_p2n_reg_t CXL_PSL_WED_An     =3D {=
0x0A0};
> >> #define CXL_PSL_SPAP_Size_Shift 4
> >> #define CXL_PSL_SPAP_V    0x0000000000000001ULL
> >>=20
> >>+/****** 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;
> >> }
> >>=20
> >>+#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 =3D 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)); =20
> >=20
> >=20
> > 2? =20
> Quoting the PSL workbook description of the PSL_TB_CTLSTAT register:
>=20
> 4:6 tbsync_cnt
> TimebaseSyncCount. Number of 250MHz cycles x 2048 before initiating anoth=
er 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 =3D never
> 001 =3D 2048
> 010 =3D 4096 (2 * 2048)
> ...
> 111 =3D 14336 (7 * 2048)
>=20
> Will make the TimebaseSyncCount unit explicit. Something like:
>=20
> #define _2048_250MHZ_CYCLES 1
> cxl_p1_write(adapter, CXL_PSL_TB_CTLSTAT, TBSYNC_CNT(2 * _2048_250MHZ_CYC=
LES));

Sounds good!

> >=20
> >>+
> >>+	/* Enable PSL Timebase */
> >>+	cxl_p1_write(adapter, CXL_PSL_Control, CXL_PSL_Control_tb);
> >>+	/* Wait until CORE TB and PSL TB difference <=3D 16usecs */
> >=20
> >=20
> > 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 attemp=
t always fails).

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

Mikey

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

[Philippe Bergheaud]

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

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

[Philippe Bergheaud]

Michael Neuling wrote:
 > Please use negative error codes here.  -EIO?
 > And check it here.

Mikey,

I am reluctant to fail the entire CAPI init after a PSL timebase sync failure.
If we ignore the error, the CAPI device stays available (without timebase sync).
If we honour the error, the CAPI device fails entirely.

I know three reasons why PSL timebase sync can fail:
1. h/w failure
2. OPAL did not initialize the CAPP timebase (wrong OPAL version)
3. the PCIe bus was not powered off/on between shutdown and reboot

I think that it is premature to choose to fail the entire CAPI init in all cases.
In particular, point 3. introduces a regression, as PCIe off/on was never a requirement for booting CAPI on P8.

I have tried one workaround do far: forcing the 0 to 1 transition of the tb bit of the PSL register TB_CTLSTAT.
In vain.

What do you think?

Philippe

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

[Michael Neuling]

On Mon, 2015-06-01 at 15:56 +0200, Philippe Bergheaud wrote:
> Michael Neuling wrote:
>  > Please use negative error codes here.  -EIO?
>  > And check it here.
>=20
> Mikey,
>=20
> I am reluctant to fail the entire CAPI init after a PSL timebase sync fai=
lure.
> If we ignore the error, the CAPI device stays available (without timebase=
 sync).
> If we honour the error, the CAPI device fails entirely.
>=20
> I know three reasons why PSL timebase sync can fail:
> 1. h/w failure

This would be a good reason to fail it.  Bad hardware, we should fail.

> 2. OPAL did not initialize the CAPP timebase (wrong OPAL version)

This would not as we are going to have to deal with older opal for a
while.

Is there a way for us to tell if OPAL has this capability?  I guess we
could add something to the device tree of the PHB node if we know the
timebase has been synced.

> 3. the PCIe bus was not powered off/on between shutdown and reboot

We should fix that.  What's the problem?

> I think that it is premature to choose to fail the entire CAPI init in al=
l cases.
> In particular, point 3. introduces a regression, as PCIe off/on was never=
 a requirement for booting CAPI on P8.

We should fix it.  Is a PERST enough?

> I have tried one workaround do far: forcing the 0 to 1 transition of the =
tb bit of the PSL register TB_CTLSTAT.
> In vain.
>=20
> What do you think?

The OPAL one (2) is the most concerning but let's work out if we can
determine if the syncing has happened in the CAPP unit or not.  If we
know it's synced but it fails your test, then I think we should fail the
whole probe. =20

I the other reasons (1 and 3) shouldn't be ignored. =20

Mikey