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

2015-08-14 Thread Daniel Axtens 
Hi Philippe,

I was trying to test Timebase Sync today and realised that it hasn't
been applied to mpe's next tree, and that it no longer applies cleanly.

Would you be able to respin it please?

I also have some minor comments inline.

On Wed, 2015-06-10 at 16:01 +0200, Philippe Bergheaud wrote:
 This patch configures the PSL Timebase function and enables it,
 after the CAPP has been initialized by OPAL.
 
This changelog should go after a line like this:
---
 V2:
  - Clear CXL_PSL_ErrIVTE_tberror bit
  - Define the sync count unit
  - Wait 1ms before each test
  - Use negative error code
  - Do not ignore errors
  - Except if timebase is not supported by OPAL
  - Be silent on success
 
 Signed-off-by: Philippe Bergheaud fe...@linux.vnet.ibm.com
 ---
  drivers/misc/cxl/cxl.h |5 
  drivers/misc/cxl/pci.c |   57 
 +++-
  2 files changed, 61 insertions(+), 1 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_V0x0001ULL
  
 +/** CXL_PSL_Control /
 +#define CXL_PSL_Control_tb 0x0001ULL
 +
  /** 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..ea1a79f 100644
 --- a/drivers/misc/cxl/pci.c
 +++ b/drivers/misc/cxl/pci.c
 @@ -360,6 +360,55 @@ static int init_implementation_adapter_regs(struct cxl 
 *adapter, struct pci_dev
   return 0;
  }
  
 +#define TBSYNC_CNT(n) (((u64)n  0x7)  (63-6))
 +#define _2048_250MHZ_CYCLES 1
 +
I'm not quite clear what these macros do or mean, and I'm also a little
bit uncomfortable with them just sitting in the middle of the file.

In particular, what does _2048_250MHZ_CYCLES mean?

 +static int cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *dev)
 +{
 + u64 psl_tb;
 + int delta;
 + unsigned int retry = 0;
 + struct device_node *np;
 +
 + if (!(np = pnv_pci_to_phb_node(dev)))
 + return -ENODEV;
 +
 + /* Do not fail when CAPP timebase sync is not supported by OPAL */
 + of_node_get(np);
 + if (! of_get_property(np, ibm,capp-timebase-sync, NULL)) {
 + of_node_put(np);
 + pr_err(PSL: Timebase sync: OPAL support missing\n);
 + return 0;
 + }
 + of_node_put(np);
Do we need to grab a reference to the node?
 +
 + /*
 +  * 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 * _2048_250MHZ_CYCLES));
 +
 + /* Enable PSL Timebase */
 + cxl_p1_write(adapter, CXL_PSL_Control, 0x);
 + cxl_p1_write(adapter, CXL_PSL_Control, CXL_PSL_Control_tb);
 +
Is there a reason we need to write 0s to PSL control before we write the
timebase control value?

 + /* Wait until CORE TB and PSL TB difference = 16usecs */
 + do {
 + msleep(1);
 + if (retry++  5) {
 + pr_err(PSL: Timebase sync: giving up!\n);
 + return -EIO;
 + }
 + psl_tb = cxl_p1_read(adapter, CXL_PSL_Timebase);
 + delta = mftb() - psl_tb;
 + if (delta  0)
 + delta = -delta;
 + } while (cputime_to_usecs(delta)  16);
 +
 + return 0;
 +}
 +
  static int init_implementation_afu_regs(struct cxl_afu *afu)
  {
   /* read/write masks for this slice */
 @@ -952,9 +1001,12 @@ static struct cxl *cxl_alloc_adapter(struct pci_dev 
 *dev)
   return adapter;
  }
  
 +#define CXL_PSL_ErrIVTE_tberror (0x1ull  (63-31))
 +
Again, could we put this somewhere else, rather than in the middle of
the file?
  static int sanitise_adapter_regs(struct cxl *adapter)
  {
 - cxl_p1_write(adapter, CXL_PSL_ErrIVTE, 0x);
 + /* Clear PSL tberror bit by writing 1 to it */
 + cxl_p1_write(adapter, CXL_PSL_ErrIVTE,

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

2015-07-07 Thread Ian Munsie 
Acked-by: Ian Munsie imun...@au1.ibm.com

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

2015-06-22 Thread Philippe Bergheaud 

Philippe Bergheaud wrote:

This patch configures the PSL Timebase function and enables it,
after the CAPP has been initialized by OPAL.

V2:
 - Clear CXL_PSL_ErrIVTE_tberror bit
 - Define the sync count unit
 - Wait 1ms before each test
 - Use negative error code
 - Do not ignore errors
 - Except if timebase is not supported by OPAL
 - Be silent on success

Signed-off-by: Philippe Bergheaud fe...@linux.vnet.ibm.com
---
 drivers/misc/cxl/cxl.h |5 
 drivers/misc/cxl/pci.c |   57 +++-
 2 files changed, 61 insertions(+), 1 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_V0x0001ULL
 
+/** CXL_PSL_Control /

+#define CXL_PSL_Control_tb 0x0001ULL
+
 /** 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..ea1a79f 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -360,6 +360,55 @@ static int init_implementation_adapter_regs(struct cxl 
*adapter, struct pci_dev
return 0;
 }
 
+#define TBSYNC_CNT(n) (((u64)n  0x7)  (63-6))

+#define _2048_250MHZ_CYCLES 1
+
+static int cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *dev)
+{
+   u64 psl_tb;
+   int delta;
+   unsigned int retry = 0;
+   struct device_node *np;
+
+   if (!(np = pnv_pci_to_phb_node(dev)))
+   return -ENODEV;
+
+   /* Do not fail when CAPP timebase sync is not supported by OPAL */
+   of_node_get(np);
+   if (! of_get_property(np, ibm,capp-timebase-sync, NULL)) {
+   of_node_put(np);
+   pr_err(PSL: Timebase sync: OPAL support missing\n);
+   return 0;
+   }
+   of_node_put(np);
+
+   /*
+* 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 * _2048_250MHZ_CYCLES));
+
+   /* Enable PSL Timebase */
+   cxl_p1_write(adapter, CXL_PSL_Control, 0x);
+   cxl_p1_write(adapter, CXL_PSL_Control, CXL_PSL_Control_tb);
+
+   /* Wait until CORE TB and PSL TB difference = 16usecs */
+   do {
+   msleep(1);
+   if (retry++  5) {
+   pr_err(PSL: Timebase sync: giving up!\n);
+   return -EIO;
+   }
+   psl_tb = cxl_p1_read(adapter, CXL_PSL_Timebase);
+   delta = mftb() - psl_tb;
+   if (delta  0)
+   delta = -delta;
+   } while (cputime_to_usecs(delta)  16);
+
+   return 0;
+}
+
 static int init_implementation_afu_regs(struct cxl_afu *afu)
 {
/* read/write masks for this slice */
@@ -952,9 +1001,12 @@ static struct cxl *cxl_alloc_adapter(struct pci_dev *dev)
return adapter;
 }
 
+#define CXL_PSL_ErrIVTE_tberror (0x1ull  (63-31))

+
 static int sanitise_adapter_regs(struct cxl *adapter)
 {
-   cxl_p1_write(adapter, CXL_PSL_ErrIVTE, 0x);
+   /* Clear PSL tberror bit by writing 1 to it */
+   cxl_p1_write(adapter, CXL_PSL_ErrIVTE, CXL_PSL_ErrIVTE_tberror);
return cxl_tlb_slb_invalidate(adapter);
 }
 
@@ -995,6 +1047,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;
 
+	if ((rc = cxl_setup_psl_timebase(adapter, dev)))

+   goto err3;
+
if ((rc = cxl_register_psl_err_irq(adapter)))
goto err3;
 

Update.

With ipmitool:
PSL timebase sync always succeeds

With bml:
1. PSL timebase sync fails after a clean shutdown / reboot
2. PSL timebase sync succeeds after a checkstop halt / reboot
3. PSL timebase sync succeeds after a pci off / on cycle

It seems that PERST does not reset the PSL correctly, with bml.

Philippe

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

2015-06-10 Thread Philippe Bergheaud 
This patch configures the PSL Timebase function and enables it,
after the CAPP has been initialized by OPAL.

V2:
 - Clear CXL_PSL_ErrIVTE_tberror bit
 - Define the sync count unit
 - Wait 1ms before each test
 - Use negative error code
 - Do not ignore errors
 - Except if timebase is not supported by OPAL
 - Be silent on success

Signed-off-by: Philippe Bergheaud fe...@linux.vnet.ibm.com
---
 drivers/misc/cxl/cxl.h |5 
 drivers/misc/cxl/pci.c |   57 +++-
 2 files changed, 61 insertions(+), 1 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_V0x0001ULL
 
+/** CXL_PSL_Control /
+#define CXL_PSL_Control_tb 0x0001ULL
+
 /** 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..ea1a79f 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -360,6 +360,55 @@ static int init_implementation_adapter_regs(struct cxl 
*adapter, struct pci_dev
return 0;
 }
 
+#define TBSYNC_CNT(n) (((u64)n  0x7)  (63-6))
+#define _2048_250MHZ_CYCLES 1
+
+static int cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *dev)
+{
+   u64 psl_tb;
+   int delta;
+   unsigned int retry = 0;
+   struct device_node *np;
+
+   if (!(np = pnv_pci_to_phb_node(dev)))
+   return -ENODEV;
+
+   /* Do not fail when CAPP timebase sync is not supported by OPAL */
+   of_node_get(np);
+   if (! of_get_property(np, ibm,capp-timebase-sync, NULL)) {
+   of_node_put(np);
+   pr_err(PSL: Timebase sync: OPAL support missing\n);
+   return 0;
+   }
+   of_node_put(np);
+
+   /*
+* 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 * _2048_250MHZ_CYCLES));
+
+   /* Enable PSL Timebase */
+   cxl_p1_write(adapter, CXL_PSL_Control, 0x);
+   cxl_p1_write(adapter, CXL_PSL_Control, CXL_PSL_Control_tb);
+
+   /* Wait until CORE TB and PSL TB difference = 16usecs */
+   do {
+   msleep(1);
+   if (retry++  5) {
+   pr_err(PSL: Timebase sync: giving up!\n);
+   return -EIO;
+   }
+   psl_tb = cxl_p1_read(adapter, CXL_PSL_Timebase);
+   delta = mftb() - psl_tb;
+   if (delta  0)
+   delta = -delta;
+   } while (cputime_to_usecs(delta)  16);
+
+   return 0;
+}
+
 static int init_implementation_afu_regs(struct cxl_afu *afu)
 {
/* read/write masks for this slice */
@@ -952,9 +1001,12 @@ static struct cxl *cxl_alloc_adapter(struct pci_dev *dev)
return adapter;
 }
 
+#define CXL_PSL_ErrIVTE_tberror (0x1ull  (63-31))
+
 static int sanitise_adapter_regs(struct cxl *adapter)
 {
-   cxl_p1_write(adapter, CXL_PSL_ErrIVTE, 0x);
+   /* Clear PSL tberror bit by writing 1 to it */
+   cxl_p1_write(adapter, CXL_PSL_ErrIVTE, CXL_PSL_ErrIVTE_tberror);
return cxl_tlb_slb_invalidate(adapter);
 }
 
@@ -995,6 +1047,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;
 
+   if ((rc = cxl_setup_psl_timebase(adapter, dev)))
+   goto err3;
+
if ((rc = cxl_register_psl_err_irq(adapter)))
goto err3;
 
-- 
1.7.2.5

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev