OCC Centaur: Check for channel checkstop

Change-Id: I2df9675d655b0391b249e49f7fc036788268e36c
RTC: 191164
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/57280
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Reviewed-by: William A. Bryan <wilbryan@us.ibm.com>
Reviewed-by: Andres A. Lugo-Reyes <aalugore@us.ibm.com>
Reviewed-by: Martha Broyles <mbroyles@us.ibm.com>

8 files changed, 350 insertions, 138 deletions

diff --git a/src/include/registers/mcs_firmware_registers.h b/src/include/registers/mcs_firmware_registers.h
index 6071755..d8357a6 100644
--- a/src/include/registers/mcs_firmware_registers.h
+++ b/src/include/registers/mcs_firmware_registers.h
@@ -51,7 +51,7 @@ typedef union mcfgpr
 } mcfgpr_t;
 
 
-typedef union mcsmode0
+typedef union mcmcicfg
 {
     uint64_t value;
     struct
@@ -67,16 +67,20 @@ typedef union mcsmode0
     struct
     {
 #ifdef _BIG_ENDIAN
-        uint64_t reserved0 : 64;
+        uint64_t dontCare0 : 47;
+        uint64_t disable_channel_fail : 1;
+        uint64_t dontCare1 : 16;
 #else
-        uint64_t reserved0 : 64;
+        uint64_t dontcare1 : 16;
+        uint64_t disable_channel_fail ; 1;
+        uint64_t dontCare0 : 47;
 #endif
     } fields;
-} mcsmode0_t;
+} mcmcicfg_t;
 
 
 
-typedef union mcifir
+typedef union mcchifir
 {
     uint64_t value;
     struct
@@ -92,11 +96,31 @@ typedef union mcifir
     struct
     {
 #ifdef _BIG_ENDIAN
-        uint64_t reserved0 : 64;
+        uint64_t fir_scom_wr_perr : 1;
+        uint64_t fir_scom_cfg_perr : 1;
+        uint64_t fir_dsrc_no_forward_progress : 1;
+        uint64_t fir_dsrc_perf_degrad : 1;
+        uint64_t fir_dmi_channel_fail : 1;
+        uint64_t fir_channel_init_timeout : 1;
+        uint64_t fir_channel_interlock_err : 1;
+        uint64_t dontCare0 : 5;
+        uint64_t fir_replay_buffer_ue : 1;
+        uint64_t dontCare1 : 1;
+        uint64_t fir_replay_buffer_overrun : 1;
+        uint64_t fir_df_sm_perr : 1;
+        uint64_t fir_cen_checkstop : 1;
+        uint64_t dontCare2 : 15;
+        uint64_t fir_dsff_tag_overrun : 1;
+        uint64_t dontCare3 : 7;
+        uint64_t fir_dsff_mca_async_cmd_error : 2;
+        uint64_t fir_dsff_seq_error : 1;
+        uint64_t dontCare4 : 18;
+        uint64_t fir_dsff_timeout : 1;
+        uint64_t dontCare5 : 2;
 #else
-        uint64_t reserved0 : 64;
+        uint64_t dontCare  : 64;
 #endif // _BIG_ENDIAN
     } fields;
-} mcifir_t;
+} mcchifir_t;
 
 #endif
diff --git a/src/include/registers/p9_misc_scom_addresses.h b/src/include/registers/p9_misc_scom_addresses.h
index 1f2dd81..4103d3b 100644
--- a/src/include/registers/p9_misc_scom_addresses.h
+++ b/src/include/registers/p9_misc_scom_addresses.h
@@ -30,9 +30,24 @@
 #define    MCS_3_MCRSVDE    0x0301088E
 #define    MCS_3_MCRSVDF    0x0301088F
 
-// MC Primary Memory Configuration Register (MCFGP)
-#define    MCS_0_MCFGP      0x0501080A
-#define    MCS_1_MCFGP      0x0501088A
-#define    MCS_2_MCFGP      0x0301080A
-#define    MCS_3_MCFGP      0x0301088A
+// MC Memory Configuration Register FIR/CFG
+#define    MCP_CHAN0_CHI_FIR        0x07010900
+#define    MCP_CHAN1_CHI_FIR        0x07010940
+#define    MCP_CHAN2_CHI_FIR        0x07010980
+#define    MCP_CHAN3_CHI_FIR        0x070109C0
+#define    MCP_CHAN4_CHI_FIR        0x08010900
+#define    MCP_CHAN5_CHI_FIR        0x08010940
+#define    MCP_CHAN6_CHI_FIR        0x08010980
+#define    MCP_CHAN7_CHI_FIR        0x080109C0
+
+#define    MCP_CHAN0_CHI_MCICFG1Q   0x0701090E
+#define    MCP_CHAN1_CHI_MCICFG1Q   0x0701094E
+#define    MCP_CHAN2_CHI_MCICFG1Q   0x0701098E
+#define    MCP_CHAN3_CHI_MCICFG1Q   0x070109CE
+#define    MCP_CHAN4_CHI_MCICFG1Q   0x0801090E
+#define    MCP_CHAN5_CHI_MCICFG1Q   0x0801094E
+#define    MCP_CHAN6_CHI_MCICFG1Q   0x0801098E
+#define    MCP_CHAN7_CHI_MCICFG1Q   0x080109CE
+
+
 #endif
diff --git a/src/occ_405/cent/centaur_control.c b/src/occ_405/cent/centaur_control.c
index e5f37ef..283a22b 100755
--- a/src/occ_405/cent/centaur_control.c
+++ b/src/occ_405/cent/centaur_control.c
@@ -491,9 +491,14 @@ bool check_centaur_checkstop(memory_control_task_t * i_memControlTask )
 
         commitErrl(&l_err);
 
-        return TRUE; // a centaur channel checkstop error occured
+        return FALSE; // error was not a channel checkstop
     }
-    return FALSE;    // No centaur channel checkstop errors
+    else
+    {
+        // Remove the centaur sensor and all dimm sensors behind it.
+        cent_chan_checkstop(cent);
+    }
+    return TRUE;    // Centaur channel checkstop
 
 }
 
diff --git a/src/occ_405/cent/centaur_data.c b/src/occ_405/cent/centaur_data.c
index 902f2c7..158937f 100755
--- a/src/occ_405/cent/centaur_data.c
+++ b/src/occ_405/cent/centaur_data.c
@@ -192,9 +192,26 @@ uint8_t      G_centaur_nest_lfir6 = 0;
 //number of SC polls to wait between i2c recovery attempts
 #define CENT_SC_MAX_INTERVAL 256
 
-//determine scom address of MCIFIR register for given Centaur n
-#define MCS0_MCIFIR_N(n) \
-        ( (n<4)? (MCS0_MCIFIR + ((MCS1_MCIFIR - MCS0_MCIFIR) * (n))) : (MCS4_MCIFIR + ((MCS5_MCIFIR - MCS4_MCIFIR) * (n-4))) )
+// There was a centaur channel checkstop, remove the centaur from the enabled bitmask.
+void cent_chan_checkstop(uint32_t i_cent)
+{
+    if(CENTAUR_PRESENT(i_cent))
+    {
+        //remove checkstopped centaur from presence bitmap
+        G_present_centaurs &= ~(CENTAUR_BY_MASK(i_cent));
+
+        // remove the dimm temperature sensors behind this centaur
+        G_dimm_enabled_sensors.bytes[i_cent] = 0;
+
+        TRAC_IMP("Channel checkstop detected on Centaur[%d] G_present_centaurs[0x%08X]",
+                 i_cent,
+                 G_present_centaurs);
+
+        TRAC_IMP("Updated bitmap of enabled dimm temperature sensors: 0x%08X %08X",
+                 G_dimm_enabled_sensors.words[0],
+                 G_dimm_enabled_sensors.words[1]);
+    }
+}
 
 void cent_recovery(uint32_t i_cent)
 {
@@ -255,9 +272,13 @@ void cent_recovery(uint32_t i_cent)
             G_cent_scom_gpe_parms.error.rc) &&
            (!(L_cent_callouts & l_cent_mask)))
         {
-            // Check if the centaur has a channel checkstop. If it does, then do not
-            // log any errors
-            if(G_cent_scom_gpe_parms.error.rc != CENTAUR_CHANNEL_CHECKSTOP)
+            // Check if the centaur has a channel checkstop. If it does then remove the centaur
+            // from the enabled sensor bit map and do not log any errors
+            if(G_cent_scom_gpe_parms.error.rc == CENTAUR_CHANNEL_CHECKSTOP)
+            {
+                cent_chan_checkstop(l_prev_cent);
+            }
+            else // Make error log for inband scom errors
             {
                 //Mark the centaur as being called out
                 L_cent_callouts |= l_cent_mask;
@@ -641,9 +662,13 @@ void centaur_data( void )
             //(as long as the request was scheduled).
             if(!async_request_completed(&l_centaur_data_ptr->gpe_req.request) || l_parms->error.rc )
             {
-                // Check if the centaur has a channel checkstop. If it does, then do not
-                // log any errors
-                if(G_cent_scom_gpe_parms.error.rc != CENTAUR_CHANNEL_CHECKSTOP)
+                // Check if the centaur has a channel checkstop. If it does then do not
+                // log any errors, but remove the centaur from the config
+                if(l_parms->error.rc == CENTAUR_CHANNEL_CHECKSTOP)
+                {
+                    cent_chan_checkstop(l_centaur_data_ptr->prev_centaur);
+                }
+                else  // log the error if it was not a CENTAUR_CHANNEL_CHECKSTOP
                 {
                     //log an error the first time this happens but keep on running.
                     //eventually, we will timeout on the dimm & centaur temps not being updated
@@ -868,7 +893,7 @@ void centaur_data( void )
     }
     while(0);
 
-    //handle centaur i2c recovery requests and centaur workaround - Needed for P9??
+    //handle centaur i2c recovery requests and centaur workaround.
     if(CENTAUR_PRESENT(l_centaur_data_ptr->current_centaur))
     {
         cent_recovery(l_centaur_data_ptr->current_centaur);
diff --git a/src/occ_405/cent/centaur_data.h b/src/occ_405/cent/centaur_data.h
index 0208265..30c0e0d 100755
--- a/src/occ_405/cent/centaur_data.h
+++ b/src/occ_405/cent/centaur_data.h
@@ -176,7 +176,11 @@ void cent_recovery(uint32_t i_cent);
 //associated with the specified OCC centaur id.
 CentaurMemData * cent_get_centaur_data_ptr( const uint8_t i_centaur_id );
 
-
+// Create the centaur configuration object
 uint32_t centaur_configuration_create( CentaurConfiguration_t * i_centaurConfiguration );
+
+// Remove centaur from enabled sensor list due to channel checkstop
+void cent_chan_checkstop(uint32_t i_cent);
+
 #endif //_CENTAUR_DATA_H
 
diff --git a/src/occ_gpe1/gpe_centaur.h b/src/occ_gpe1/gpe_centaur.h
index ad8e2a7..e30a724 100644
--- a/src/occ_gpe1/gpe_centaur.h
+++ b/src/occ_gpe1/gpe_centaur.h
@@ -50,4 +50,11 @@ void gpe_scom_centaur(CentaurConfiguration_t* i_config,
 int centaur_get_mem_data(CentaurConfiguration_t* i_config,
                          CentaurGetMemDataParms_t* i_parms);
 
+/**
+ * Check for channel checkstop
+ * @param[in] The ordinal centaur number
+ * @return [0 | return code]
+ */
+int check_channel_chkstp(unsigned int i_centaur);
+
 #endif
diff --git a/src/occ_gpe1/gpe_centaur_configuration.c b/src/occ_gpe1/gpe_centaur_configuration.c
index cfd4a76..fdf9927 100644
--- a/src/occ_gpe1/gpe_centaur_configuration.c
+++ b/src/occ_gpe1/gpe_centaur_configuration.c
@@ -50,28 +50,28 @@ const uint32_t MCFGPR[OCCHW_NCENTAUR] =
     MCS_3_MCRSVDF
 };
 
-//const uint32_t MCSMODE0[OCCHW_NCENTAUR / 2] __attribute__((section(".sdata2"))) =
-//{
-//    MCS_0_MCMODE0,
-//    MCS_1_MCMODE0,
-//    MCS_2_MCMODE0,
-//    MCS_3_MCMODE0
-//};
-
-//const uint32_t MCFIR[OCCHW_NCENTAUR / 2] __attribute((section(".sdata2"))) =
-//{
-//    MCS_0_MCFIR,
-//    MCS_1_MCFIR,
-//    MCS_2_MCFIR,
-//    MCS_3_MCFIR
-//};
-
-const uint32_t MCFGP[OCCHW_NCENTAUR/2] =
+const uint32_t MCCHIFIR[OCCHW_NCENTAUR] =
 {
-    MCS_0_MCFGP,
-    MCS_1_MCFGP,
-    MCS_2_MCFGP,
-    MCS_3_MCFGP
+    MCP_CHAN0_CHI_FIR,
+    MCP_CHAN1_CHI_FIR,
+    MCP_CHAN2_CHI_FIR,
+    MCP_CHAN3_CHI_FIR,
+    MCP_CHAN4_CHI_FIR,
+    MCP_CHAN5_CHI_FIR,
+    MCP_CHAN6_CHI_FIR,
+    MCP_CHAN7_CHI_FIR
+};
+
+const uint32_t MCMCICFG1Q[OCCHW_NCENTAUR] =
+{
+    MCP_CHAN0_CHI_MCICFG1Q,
+    MCP_CHAN1_CHI_MCICFG1Q,
+    MCP_CHAN2_CHI_MCICFG1Q,
+    MCP_CHAN3_CHI_MCICFG1Q,
+    MCP_CHAN4_CHI_MCICFG1Q,
+    MCP_CHAN5_CHI_MCICFG1Q,
+    MCP_CHAN6_CHI_MCICFG1Q,
+    MCP_CHAN7_CHI_MCICFG1Q
 };
 
 ///////////////////////////////////////////////////////////////
@@ -222,8 +222,15 @@ int gpe_centaur_configuration_create(CentaurConfiguration_t* o_config)
         {
             uint64_t val64;
 
-            //FIR bits have changed from p8 TODO Marc Golub to provide what
-            //firchecks need to be done for P9 
+            // check for channel checkstop
+            rc = check_channel_chkstp(i);
+            if (rc)
+            {
+                // If scom failed OR there is a channel checkstop then
+                // Centaur is not usable.
+                rc = 0;
+                continue;
+            }
 
             // Verify that inband scom has been setup. If not then
             // assume the centaur is either non-existant or not configured.
@@ -420,3 +427,53 @@ int gpe_centaur_configuration_create(CentaurConfiguration_t* o_config)
     return rc;
 }
 
+int check_channel_chkstp(unsigned int i_centaur)
+{
+    int rc = 0;
+    mcchifir_t chifir;
+    mcmcicfg_t chicfg;
+
+    do
+    {
+        rc = getscom_abs(MCCHIFIR[i_centaur], &(chifir.value));
+        if (rc)
+        {
+            PK_TRACE("MCCHIFIR scom failed. rc = %d",rc);
+            break;
+        }
+
+        if(chifir.fields.fir_dsrc_no_forward_progress ||
+           chifir.fields.fir_dmi_channel_fail  ||
+           chifir.fields.fir_channel_init_timeout ||
+           chifir.fields.fir_channel_interlock_err ||
+           chifir.fields.fir_replay_buffer_ue ||
+           chifir.fields.fir_replay_buffer_overrun ||
+           chifir.fields.fir_df_sm_perr ||
+           chifir.fields.fir_cen_checkstop ||
+           chifir.fields.fir_dsff_tag_overrun ||
+           chifir.fields.fir_dsff_mca_async_cmd_error ||
+           chifir.fields.fir_dsff_seq_error ||
+           chifir.fields.fir_dsff_timeout)
+        {
+            PK_TRACE("MCCHIFIR: %08x%08x for channel %d",
+                     chifir.words.high_order,
+                     chifir.words.low_order,
+                     i_centaur);
+
+            rc = getscom_abs(MCMCICFG1Q[i_centaur], &(chicfg.value));
+            if (rc)
+            {
+                PK_TRACE("MCMCICFG scom failed. rc = %d",rc);
+                break;
+            }
+
+            PK_TRACE("MCMCICFG1Q %08x%08x",
+                chicfg.words.high_order,
+                chicfg.words.low_order);
+
+            rc = CENTAUR_CHANNEL_CHECKSTOP;
+        }
+    } while(0);
+
+    return rc;
+}
diff --git a/src/occ_gpe1/gpe_centaur_scom.c b/src/occ_gpe1/gpe_centaur_scom.c
index 1817a19..afe1d3b 100644
--- a/src/occ_gpe1/gpe_centaur_scom.c
+++ b/src/occ_gpe1/gpe_centaur_scom.c
@@ -36,12 +36,13 @@
  * @returns the OCI address to scom the centaur
  * @Post The extended address field in the PBASLVCNT is set
  */
-uint32_t centaur_scom_setup(CentaurConfiguration_t* i_config,
+int centaur_scom_setup(CentaurConfiguration_t* i_config,
                             uint32_t i_centaur_instance,
-                            uint32_t i_scom_address)
+                            uint32_t i_scom_address,
+                            uint32_t *o_oci_addr)
 {
-    uint32_t oci_addr = 0;
- #if defined(__PBASLV__)
+    int rc = 0;
+#if defined(__USE_PBASLV__)
    pba_slvctln_t slvctln;
 #endif
     uint64_t pb_addr = i_config->baseAddress[i_centaur_instance];
@@ -63,34 +64,49 @@ uint32_t centaur_scom_setup(CentaurConfiguration_t* i_config,
     pb_addr  |= 0x0000000002000000ull;
     pb_addr  |= ((uint64_t)scom_address << 3);
 
- #if defined(__PBASLV__)
+#if defined(__USE_PBASLV__)
     // put bits 23:36 of address into slvctln extended addr
     PPE_LVD((i_config->scomParms).slvctl_address, slvctln.value);
     slvctln.fields.extaddr = pb_addr >> 27;
     PPE_STVD((i_config->scomParms).slvctl_address, slvctln.value);
-#endif
+#else
     // HW bug work-around
     {
-        // workaround - don't use extr addr - use pbabar.
+        // workaround - don't use extraddr - use pbabar.
         uint64_t barMsk = 0;
         // put the PBA in the BAR
-        putscom_abs(PBA_BARN(PBA_BAR_CENTAUR), pb_addr);
-        putscom_abs(PBA_BARMSKN(PBA_BAR_CENTAUR), barMsk);
+        rc = putscom_abs(PBA_BARN(PBA_BAR_CENTAUR), pb_addr);
+        if(rc)
+        {
+            PK_TRACE("centaur_scom_setup. putscom fail on PBABAR."
+                     " rc = %d",rc);
+        }
+        else
+        {
+            rc = putscom_abs(PBA_BARMSKN(PBA_BAR_CENTAUR), barMsk);
+            if(rc)
+            {
+                PK_TRACE("centaur_scom_setup. putscom fail on PBABARMSK"
+                         " rc = %d",rc);
+            }
+        }
     }
+#endif
     // make oci address
-    oci_addr = (uint32_t)(pb_addr & 0x07ffffffull);
+    *o_oci_addr = (uint32_t)(pb_addr & 0x07ffffffull);
 
     // upper nibble is PBA region and BAR_SELECT
-    oci_addr  |= ((PBA_BAR_CENTAUR | 0x8) << 28);
-    PK_TRACE_DBG("Centaur OCI scom addr: %08x",oci_addr);
-    return oci_addr;
+    *o_oci_addr  |= ((PBA_BAR_CENTAUR | 0x8) << 28);
+    PK_TRACE_DBG("Centaur OCI scom addr: %08x",*o_oci_addr);
+    return rc;
 }
 
-uint32_t centaur_sensorcache_setup(CentaurConfiguration_t* i_config,
-                                   uint32_t i_centaur_instance)
+int centaur_sensorcache_setup(CentaurConfiguration_t* i_config,
+                                   uint32_t i_centaur_instance,
+                                   uint32_t * o_oci_addr)
 {
-    uint32_t oci_addr = 0;
-#if defined(__PBASLV__)
+    int rc = 0;
+#if defined(__USE_PBASLV__)
     pba_slvctln_t slvctln;
 #endif
     uint64_t pb_addr = i_config->baseAddress[i_centaur_instance];
@@ -98,25 +114,39 @@ uint32_t centaur_sensorcache_setup(CentaurConfiguration_t* i_config,
     // bit 38 set OCI master, bits 39,40 Centaur thermal sensors '10'b
     pb_addr |= 0x0000000003000000ull;
 
-#if defined(__PBASLV__)
+#if defined(__USE_PBASLV__)
     PPE_LVD((i_config->dataParms).slvctl_address, slvctln.value);
     slvctln.fields.extaddr = pb_addr >> 27;
     PPE_STVD((i_config->dataParms).slvctl_address, slvctln.value);
-#endif
+#else
     {
-        // HW bug workaround - don't use extr addr - use pbabar.
+        // HW bug workaround - don't use extaddr - use pbabar.
         uint64_t barMsk = 0;
         // put the PBA in the BAR
-        putscom_abs(PBA_BARN(PBA_BAR_CENTAUR), pb_addr);
-        putscom_abs(PBA_BARMSKN(PBA_BAR_CENTAUR), barMsk);
+        rc = putscom_abs(PBA_BARN(PBA_BAR_CENTAUR), pb_addr);
+        if (rc)
+        {
+            PK_TRACE("centaur_sensorcache_setup: putscom fail on PBABAR,"
+                     " rc = %d",rc);
+        }
+        else
+        {
+            rc = putscom_abs(PBA_BARMSKN(PBA_BAR_CENTAUR), barMsk);
+            if (rc)
+            {
+                PK_TRACE("centaur_sensrocache_setup: putscom fail on"
+                         " PBABARMSK, rc = %d",rc);
+            }
+        }
     }
+#endif
     // make oci address
-    oci_addr = (uint32_t)(pb_addr & 0x07ffffffull);
+    *o_oci_addr = (uint32_t)(pb_addr & 0x07ffffffull);
 
     // PBA space bits[0:1] = '10'  bar select bits[3:4]
-    oci_addr |= ((PBA_BAR_CENTAUR | 0x8) << 28);
+    *o_oci_addr |= ((PBA_BAR_CENTAUR | 0x8) << 28);
 
-     return oci_addr;
+    return rc;
 }
 
 void pbaslvctl_reset(GpePbaParms* i_pba_parms)
@@ -170,6 +200,7 @@ int centaur_get_scom_vector(CentaurConfiguration_t* i_config,
                             uint32_t i_scom_address,
                             uint64_t* o_data)
 {
+    int rc = 0;
     int instance = 0;
     uint64_t pba_slvctln_save;
 
@@ -183,9 +214,17 @@ int centaur_get_scom_vector(CentaurConfiguration_t* i_config,
     {
         if( CHIP_CONFIG_CENTAUR(instance) & (i_config->config))
         {
-            uint32_t oci_addr =
-                centaur_scom_setup(i_config, instance, i_scom_address);
+            uint32_t oci_addr;
+            rc = centaur_scom_setup(i_config,
+                                    instance,
+                                    i_scom_address,
+                                    &oci_addr);
 
+            if(rc)
+            {
+                // Already traced.
+                break;
+            }
             // read centaur scom
             PPE_LVD(oci_addr, *o_data);
         }
@@ -201,7 +240,11 @@ int centaur_get_scom_vector(CentaurConfiguration_t* i_config,
     pbaslvctl_reset(&(i_config->scomParms));
     PPE_STVD((i_config->scomParms).slvctl_address, pba_slvctln_save);
 
-    return rc_from_sibrc();
+    if(!rc)
+    {
+        rc = rc_from_sibrc();
+    }
+    return rc;
 }
 
 int centaur_get_scom(CentaurConfiguration_t* i_config,
@@ -216,10 +259,12 @@ int centaur_get_scom(CentaurConfiguration_t* i_config,
     pbaslvctl_reset(&(i_config->scomParms));
     pba_slvctln_save = pbaslvctl_setup(&(i_config->scomParms));
 
-    oci_addr =
-        centaur_scom_setup(i_config, i_centaur_instance, i_scom_address);
+    rc = centaur_scom_setup(i_config,
+                            i_centaur_instance,
+                            i_scom_address,
+                            &oci_addr);
 
-    if( CHIP_CONFIG_CENTAUR(i_centaur_instance) & (i_config->config))
+    if( !rc && (CHIP_CONFIG_CENTAUR(i_centaur_instance) & (i_config->config)))
     {
         // read centaur scom
         rc = getscom_abs(oci_addr, o_data);
@@ -242,6 +287,7 @@ int centaur_put_scom_all(CentaurConfiguration_t* i_config,
                          uint32_t i_scom_address,
                          uint64_t i_data)
 {
+    int rc = 0;
     int instance = 0;
     uint64_t pba_slvctln_save;
 
@@ -255,8 +301,17 @@ int centaur_put_scom_all(CentaurConfiguration_t* i_config,
     {
         if( CHIP_CONFIG_CENTAUR(instance) & (i_config->config))
         {
-            uint32_t oci_addr =
-                centaur_scom_setup(i_config, instance, i_scom_address);
+            uint32_t oci_addr;
+            rc = centaur_scom_setup(i_config,
+                                    instance,
+                                    i_scom_address,
+                                    &oci_addr);
+
+            if(rc)
+            {
+                // Already traced in centaur_scom_setup
+                break;
+            }
 
             // centaur scom
             PPE_STVD(oci_addr, i_data);
@@ -267,7 +322,11 @@ int centaur_put_scom_all(CentaurConfiguration_t* i_config,
     pbaslvctl_reset(&(i_config->scomParms));
     PPE_STVD((i_config->scomParms).slvctl_address, pba_slvctln_save);
 
-    return rc_from_sibrc();
+    if(!rc)
+    {
+        rc = rc_from_sibrc();
+    }
+    return rc;
 }
 
 int centaur_put_scom(CentaurConfiguration_t* i_config,
@@ -282,17 +341,22 @@ int centaur_put_scom(CentaurConfiguration_t* i_config,
     pbaslvctl_reset(&(i_config->scomParms));
     pba_slvctln_save = pbaslvctl_setup(&(i_config->scomParms));
 
-    oci_addr =
-        centaur_scom_setup(i_config, i_centaur_instance, i_scom_address);
+    rc = centaur_scom_setup(i_config,
+                            i_centaur_instance,
+                            i_scom_address,
+                            &oci_addr);
 
-    if( CHIP_CONFIG_CENTAUR(i_centaur_instance) & (i_config->config))
-    {
-        // write centaur scom
-        rc = putscom_abs(oci_addr, i_data);
-    }
-    else
+    if(!rc)
     {
-        rc = CENTAUR_INVALID_SCOM;
+        if(CHIP_CONFIG_CENTAUR(i_centaur_instance) & (i_config->config))
+        {
+            // write centaur scom
+            rc = putscom_abs(oci_addr, i_data);
+        }
+        else
+        {
+            rc = CENTAUR_INVALID_SCOM;
+        }
     }
 
     // gpe_pba_cntl function?
@@ -317,16 +381,21 @@ int centaur_scom_rmw(CentaurConfiguration_t* i_config,
     pbaslvctl_reset(&(i_config->scomParms));
     pba_slvctln_save = pbaslvctl_setup(&(i_config->scomParms));
 
-    oci_addr =
-        centaur_scom_setup(i_config, i_centaur_instance, i_scom_address);
-
-    rc = getscom_abs(oci_addr, &data64);
+    rc = centaur_scom_setup(i_config,
+                            i_centaur_instance,
+                            i_scom_address,
+                            &oci_addr);
     if(!rc)
     {
-        data64 &= (i_mask ^ 0xffffffffffffffffull);
-        data64 |= *i_data;
 
-        rc = putscom_abs(oci_addr, data64);
+        rc = getscom_abs(oci_addr, &data64);
+        if(!rc)
+        {
+            data64 &= (i_mask ^ 0xffffffffffffffffull);
+            data64 |= *i_data;
+
+            rc = putscom_abs(oci_addr, data64);
+        }
     }
 
     pbaslvctl_reset(&(i_config->scomParms));
@@ -341,6 +410,7 @@ int centaur_scom_rmw_all(CentaurConfiguration_t* i_config,
                          uint64_t i_mask,
                          uint64_t i_data)
 {
+    int rc = 0;
     int instance = 0;
     uint64_t pba_slvctln_save;
 
@@ -355,8 +425,16 @@ int centaur_scom_rmw_all(CentaurConfiguration_t* i_config,
         if( CHIP_CONFIG_CENTAUR(instance) & (i_config->config))
         {
             uint64_t data64;
-            uint32_t oci_addr =
-                centaur_scom_setup(i_config, instance, i_scom_address);
+            uint32_t oci_addr;
+            rc = centaur_scom_setup(i_config,
+                                    instance,
+                                    i_scom_address,
+                                    &oci_addr);
+            if(rc)
+            {
+                // Already traced in centaur_scom_setup
+                break;
+            }
 
             PPE_LVD(oci_addr, data64);
             data64 &= (i_mask ^ 0xffffffffffffffffull);
@@ -369,7 +447,11 @@ int centaur_scom_rmw_all(CentaurConfiguration_t* i_config,
 
     PPE_STVD((i_config->scomParms).slvctl_address, pba_slvctln_save);
 
-    return rc_from_sibrc();
+    if(!rc)
+    {
+        rc = rc_from_sibrc();
+    }
+    return rc;
 }
 
 
@@ -381,17 +463,9 @@ int centaur_get_mem_data(CentaurConfiguration_t* i_config,
     uint32_t oci_addr = 0;
     uint64_t pba_slvctln_save;
     uint64_t data64 = 0;
-    uint64_t barMskOrg = 0;
 
     i_parms->error.rc = CENTAUR_GET_MEM_DATA_DIED;
 
-    // HW bug work-around
-    rc = getscom_abs(PBA_BARMSKN(PBA_BAR_CENTAUR), &barMskOrg);
-    if(rc)
-    {
-        PK_TRACE("Workaround failed to read bar mask. rc = %x",rc);
-    }
-
     pbaslvctl_reset(&(i_config->dataParms));
     pba_slvctln_save = pbaslvctl_setup(&(i_config->dataParms));
 
@@ -404,23 +478,21 @@ int centaur_get_mem_data(CentaurConfiguration_t* i_config,
         }
         else
         {
-            oci_addr = centaur_sensorcache_setup(i_config, i_parms->collect);
+            rc = centaur_sensorcache_setup(i_config, i_parms->collect,&oci_addr);
 
-            // Poke the Centaur sensor cache by writing to base address.
-            data64 = 0;
-            PPE_STVD(oci_addr, data64);
-
-            // Read 128 bytes from centaur cache
-            int i;
-            for(i = 0; i < 128; i += 8)
+            if(!rc)
             {
-                PPE_LVDX(oci_addr, i, data64);
-                PPE_STVDX((i_parms->data), i, data64);
+                // Read 128 bytes from centaur cache
+                int i;
+                for(i = 0; i < 128; i += 8)
+                {
+                    PPE_LVDX(oci_addr, i, data64);
+                    PPE_STVDX((i_parms->data), i, data64);
+                }
             }
         }
     }
-    // TODO Decide to keep this or not.
-#if defined(__JUNK__)
+
     if(!rc && i_parms->update != -1)
     {
         if((i_parms->update >= OCCHW_NCENTAUR) ||
@@ -430,32 +502,35 @@ int centaur_get_mem_data(CentaurConfiguration_t* i_config,
         }
         else
         {
-            oci_addr = centaur_sensorcache_setup(i_config, i_parms->update);
+            rc = centaur_sensorcache_setup(i_config, i_parms->update,&oci_addr);
 
-            //PK_TRACE("CACHE POKE: %08x",oci_addr);
-            // Writing a zero to this address "pokes" the centaur.
-            data64 = 0;
-            PPE_STVD(oci_addr, data64);
+            if(!rc)
+            {
+                // Writing a zero to this address tells the centaur to update
+                // the sensor cache.
+                data64 = 0;
+                PPE_STVD(oci_addr, data64);
+            }
         }
     }
-#endif
 
     pbaslvctl_reset(&(i_config->dataParms));
     PPE_STVD((i_config->dataParms).slvctl_address, pba_slvctln_save);
 
-    // TODO if RC then check for centaur channel checkstop
-    // The MCFIR reg no longer contains a bit for CHANNEL_FAIL_SIGNAL_ACTIVE.
-    // No equivalent has been identified yet for P9. Marc Gollub will provide
-    // if needed.
-    // Return rc = CENTAUR_CHANNEL_CHECKSTOP
-
-    // HW bug work-around
-    rc = putscom_abs(PBA_BARMSKN(PBA_BAR_CENTAUR), barMskOrg);
-    if(rc)
+    if(!rc)
     {
-        PK_TRACE("Work around Failed to set bar mask. rc = %x",rc);
+        int instance =  i_parms->collect;
+        if(instance == -1)
+        {
+            instance = i_parms->update;
+        }
+        if (instance != -1)
+        {
+            rc = check_channel_chkstp(instance);
+        }
     }
 
+
     i_parms->error.rc = rc;
     return rc;
 }