Check PMIC error and enable statuses after pmic_enable

Change-Id: I26bc700a3b368c9bd110ee62b1037a6c6d14491e
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/83886
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Hostboot CI <hostboot-ci+hostboot@us.ibm.com>
Reviewed-by: Louis Stermole <stermole@us.ibm.com>
Reviewed-by: STEPHEN GLANCY <sglancy@us.ibm.com>
Reviewed-by: Jennifer A Stofer <stofer@us.ibm.com>
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/83904
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Reviewed-by: Daniel M Crowell <dcrowell@us.ibm.com>

7 files changed, 519 insertions, 53 deletions

diff --git a/src/import/chips/ocmb/common/include/pmic_regs_fld.H b/src/import/chips/ocmb/common/include/pmic_regs_fld.H
index 1301d0e05..16946f491 100644
--- a/src/import/chips/ocmb/common/include/pmic_regs_fld.H
+++ b/src/import/chips/ocmb/common/include/pmic_regs_fld.H
@@ -69,6 +69,31 @@ struct pmicFields<mss::pmic::product::JEDEC_COMPLIANT>
     static constexpr uint8_t SEQUENCE_SWC_ENABLE            = 0x04;
     static constexpr uint8_t SEQUENCE_SWD_ENABLE            = 0x03;
 
+    // R04
+    static constexpr uint8_t R04_GLOBAL_ERROR_COUNT                     = 0x07;
+    static constexpr uint8_t R04_GLOBAL_ERROR_LOG_BUCK_OV_OR_UV         = 0x06;
+    static constexpr uint8_t R04_GLOBAL_ERROR_LOG_VIN_BULK_OVER_VOLTAGE = 0x05;
+    static constexpr uint8_t R04_GLOBAL_ERROR_LOG_CRITICAL_TEMPERATURE  = 0x04;
+
+    // R05
+    static constexpr uint8_t R05_SWA_POWER_GOOD      = 0x06;
+    static constexpr uint8_t R05_SWB_POWER_GOOD      = 0x05;
+    static constexpr uint8_t R05_SWC_POWER_GOOD      = 0x04;
+    static constexpr uint8_t R05_SWD_POWER_GOOD      = 0x03;
+    static constexpr uint8_t R05_PMIC_ERROR_LOG      = 0x02; // TK ARRAY
+
+    // R06
+    static constexpr uint8_t R06_SWA_UNDER_VOLTAGE_LOCKOUT  = 0x07;
+    static constexpr uint8_t R06_SWB_UNDER_VOLTAGE_LOCKOUT  = 0x06;
+    static constexpr uint8_t R06_SWC_UNDER_VOLTAGE_LOCKOUT  = 0x05;
+    static constexpr uint8_t R06_SWD_UNDER_VOLTAGE_LOCKOUT  = 0x04;
+    static constexpr uint8_t R06_SWA_OVER_VOLTAGE           = 0x03;
+    static constexpr uint8_t R06_SWB_OVER_VOLTAGE           = 0x02;
+    static constexpr uint8_t R06_SWC_OVER_VOLTAGE           = 0x01;
+    static constexpr uint8_t R06_SWD_OVER_VOLTAGE           = 0x00;
+
+    // R07 all reserved
+
     // R08
     static constexpr uint8_t R08_VIN_BULK_INPUT_PWR_GOOD_STATUS = 0x07;
     static constexpr uint8_t R08_CRITICAL_TEMP_SHUTDOWN_STATUS  = 0x06;
diff --git a/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_common_utils.C b/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_common_utils.C
index 3906c55ef..06487ffbd 100644
--- a/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_common_utils.C
+++ b/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_common_utils.C
@@ -145,5 +145,202 @@ fapi_try_exit:
     return ((i_rc == fapi2::FAPI2_RC_SUCCESS) ? fapi2::current_err : i_rc);
 }
 
+///
+/// @brief Check if PMIC is IDT vendor
+///
+/// @param[in] i_pmic_target PMIC target
+/// @param[out] o_is_idt true/false
+/// @return fapi2::ReturnCode FAPI2_RC_SUCCESS iff success, else error code
+/// @note Can't unit test this properly as R3D is hardcoded in simics
+///
+fapi2::ReturnCode pmic_is_idt(const fapi2::Target<fapi2::TARGET_TYPE_PMIC>& i_pmic_target, bool& o_is_idt)
+{
+    o_is_idt = false;
+    using REGS = pmicRegs<mss::pmic::product::JEDEC_COMPLIANT>;
+    fapi2::buffer<uint8_t> l_reg_contents;
+
+    FAPI_TRY(mss::pmic::i2c::reg_read(i_pmic_target, REGS::R3D_VENDOR_ID_BYTE_1, l_reg_contents));
+
+    o_is_idt = (l_reg_contents == mss::pmic::vendor::IDT_SHORT);
+
+fapi_try_exit:
+    return fapi2::current_err;
+}
+
+namespace status
+{
+
+///
+/// @brief Checks that the PMIC is enabled via VR Enable bit
+///
+/// @param[in] i_ocmb_target OCMB target
+/// @param[in] i_pmic_target PMIC target
+/// @return fapi2::ReturnCode FAPI2_RC_SUCCESS iff success, else error code
+///
+fapi2::ReturnCode check_for_vr_enable(
+    const fapi2::Target<fapi2::TARGET_TYPE_OCMB_CHIP>& i_ocmb_target,
+    const fapi2::Target<fapi2::TargetType::TARGET_TYPE_PMIC>& i_pmic_target)
+{
+    fapi2::buffer<uint8_t> l_vr_enable_buffer;
+
+    FAPI_TRY(mss::pmic::i2c::reg_read_reverse_buffer(i_pmic_target, REGS::R32, l_vr_enable_buffer),
+             "start_vr_enable: Could not read address 0x%02hhX of %s to check for VR Enable",
+             REGS::R32,
+             mss::c_str(i_pmic_target));
+
+    // Make sure we are enabled
+    FAPI_ASSERT(l_vr_enable_buffer.getBit<FIELDS::R32_VR_ENABLE>(),
+                fapi2::PMIC_NOT_ENABLED()
+                .set_PMIC_TARGET(i_pmic_target)
+                .set_OCMB_TARGET(i_ocmb_target),
+                "PMIC %s was not identified as enabled by checking VR Enable bit",
+                mss::c_str(i_pmic_target));
+
+    return fapi2::FAPI2_RC_SUCCESS;
+
+fapi_try_exit:
+    return fapi2::current_err;
+}
+
+///
+/// @brief Check the statuses of all PMICs present on the given OCMB chip
+///
+/// @param[in] i_ocmb_target OCMB target
+/// @return fapi2::ReturnCode FAPI2_RC_SUCCESS if success, else error code
+/// @note the returned target is only valid if o_errors returns as true. Else, the target is an uninitialized blank target!
+///
+fapi2::ReturnCode check_all_pmics(const fapi2::Target<fapi2::TARGET_TYPE_OCMB_CHIP>& i_ocmb_target)
+{
+    const char* l_ocmb_c_str = mss::c_str(i_ocmb_target);
+
+    // Initialize returnable PMIC with a blank target. This blank target won't be used if there's no error.
+    // If there is an error, this will be overwritten with a valid erroneous PMIC
+    bool l_pmic_error = false;
+
+    fapi2::current_err = fapi2::FAPI2_RC_SUCCESS;
+
+    // Start success so we can't log and return the same error in loop logic
+    fapi2::ReturnCode l_rc = fapi2::FAPI2_RC_SUCCESS;
+
+    // Check that the PMICs are enabled and without errors
+    for (const auto& l_pmic : mss::find_targets<fapi2::TARGET_TYPE_PMIC>(i_ocmb_target))
+    {
+        FAPI_TRY(check_for_vr_enable(i_ocmb_target, l_pmic),
+                 "PMIC %s did not return enabled status", mss::c_str(l_pmic));
+
+        FAPI_TRY(mss::pmic::status::check_pmic(l_pmic, l_pmic_error));
+
+        FAPI_ASSERT_NOEXIT(!l_pmic_error,
+                           fapi2::PMIC_STATUS_ERRORS()
+                           .set_OCMB_TARGET(i_ocmb_target)
+                           .set_PMIC_TARGET(l_pmic),
+                           "PMIC on OCMB %s had one or more status bits set after running pmic_enable(). "
+                           "One of possibly several bad PMICs: %s",
+                           l_ocmb_c_str, mss::c_str(l_pmic));
+
+        if (l_rc != fapi2::FAPI2_RC_SUCCESS)
+        {
+            fapi2::logError(fapi2::current_err, fapi2::FAPI2_ERRL_SEV_UNRECOVERABLE);
+        }
+
+        // Reset for next loop
+        l_rc = fapi2::current_err;
+        l_pmic_error = false;
+        fapi2::current_err = fapi2::FAPI2_RC_SUCCESS;
+    }
+
+    // Else, exit on whatever RC we had
+    FAPI_TRY(l_rc);
+
+fapi_try_exit:
+    return fapi2::current_err;
+}
+
+///
+/// @brief Check the PMIC's status codes and report back if an error occurred
+///
+/// @param[in] i_pmic_target PMIC target
+/// @param[out] o_error true/false
+/// @return fapi2::ReturnCode FAPI2_RC_SUCCESS iff success, else error
+///
+fapi2::ReturnCode check_pmic(
+    const fapi2::Target<fapi2::TARGET_TYPE_PMIC>& i_pmic_target,
+    bool& o_error)
+{
+    o_error = false;
+    bool l_pmic_is_idt = false;
+
+    FAPI_TRY(pmic_is_idt(i_pmic_target, l_pmic_is_idt));
+
+    if (l_pmic_is_idt)
+    {
+        // These registers reflect the previous power down cycle of the PMIC. Therefore, they may
+        // not necessarily cause issues on this life. So, we do not need to worry about keeping track if
+        // these failed with l_status_error, but the check_fields() function will still print them out.
+        bool l_status_error = false;
+
+        // if we exit from this try, there were i2c errors
+        FAPI_TRY(mss::pmic::status::check_fields(i_pmic_target, mss::pmic::status::IDT_SPECIFIC_STATUS_FIELDS, l_status_error));
+    }
+
+    {
+        bool l_status_error = false;
+
+        // if we exit from this try, there were i2c errors
+        FAPI_TRY(mss::pmic::status::check_fields(i_pmic_target, mss::pmic::status::STATUS_FIELDS, l_status_error));
+        o_error = l_status_error;
+    }
+
+fapi_try_exit:
+    return fapi2::current_err;
+}
+
+///
+/// @brief Check an individual set of PMIC status codes
+///
+/// @param[in] i_pmic_target PMIC target
+/// @param[in] i_statuses STATUS object to check
+/// @param[out] o_error At least one error bit was found to be set
+/// @return fapi2::ReturnCode FAPI2_RC_SUCCESS iff success, else error in case of an I2C read error
+///
+fapi2::ReturnCode check_fields(
+    const fapi2::Target<fapi2::TARGET_TYPE_PMIC>& i_pmic_target,
+    const std::vector<std::pair<uint8_t, std::vector<status_field>>>& i_statuses,
+    bool& o_error)
+{
+    o_error = false;
+
+    for (const auto& l_reg_bit_pair : i_statuses)
+    {
+        fapi2::buffer<uint8_t> l_reg_contents;
+        FAPI_TRY(mss::pmic::i2c::reg_read_reverse_buffer(i_pmic_target, l_reg_bit_pair.first, l_reg_contents));
+
+        for (const auto& l_status : l_reg_bit_pair.second)
+        {
+            if (l_reg_contents.getBit(l_status.l_reg_field))
+            {
+                // Print it out
+                FAPI_ERR("%s :: REG 0x%02x bit %u was set on %s",
+                         l_status.l_error_description,
+                         l_reg_bit_pair.first,
+                         l_status.l_reg_field,
+                         mss::c_str(i_pmic_target));
+                // We don't want to exit out here, errors can be independent of each other and we should check them all.
+                // Since there's no easy way to FFDC each individual error, we will report them here and then worry about
+                // return codes in the caller of this function
+                o_error = true;
+            }
+        }
+
+        l_reg_contents.flush<0>();
+    }
+
+    return fapi2::FAPI2_RC_SUCCESS;
+
+fapi_try_exit:
+    return fapi2::current_err;
+}
+
+} // status
 } // pmic
 } // mss
diff --git a/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_common_utils.H b/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_common_utils.H
index 1cc720992..4b73203cd 100644
--- a/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_common_utils.H
+++ b/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_common_utils.H
@@ -159,6 +159,161 @@ fapi2::ReturnCode unlock_vendor_region(const fapi2::Target<fapi2::TargetType::TA
 fapi2::ReturnCode lock_vendor_region(const fapi2::Target<fapi2::TargetType::TARGET_TYPE_PMIC>& i_pmic_target,
                                      const fapi2::ReturnCode i_rc = fapi2::FAPI2_RC_SUCCESS);
 
+///
+/// @brief Check if PMIC is IDT vendor
+///
+/// @param[in] i_pmic_target PMIC target
+/// @param[out] o_is_idt true/false
+/// @return fapi2::ReturnCode FAPI2_RC_SUCCESS iff success, else error code
+///
+fapi2::ReturnCode pmic_is_idt(const fapi2::Target<fapi2::TARGET_TYPE_PMIC>& i_pmic_target, bool& o_is_idt);
+
+namespace status
+{
+
+///
+/// @brief Information for each field that we can iterate through
+///
+struct status_field
+{
+    uint8_t l_reg_field;
+    const char* l_error_description;
+};
+
+static const std::vector<std::pair<uint8_t, std::vector<status_field>>> IDT_SPECIFIC_STATUS_FIELDS =
+{
+    {
+        REGS::R04,
+        {   {FIELDS::R04_GLOBAL_ERROR_COUNT, "GLOBAL_ERROR_COUNT: >1 error count since last erase operation"},
+            {FIELDS::R04_GLOBAL_ERROR_LOG_BUCK_OV_OR_UV, "GLOBAL_ERROR_LOG: BUCK OV/UV: Error occurred"},
+            {FIELDS::R04_GLOBAL_ERROR_LOG_VIN_BULK_OVER_VOLTAGE, "GLOBAL_ERROR_LOG: VIN_BULK_OVER_VOLTAGE"},
+            {FIELDS::R04_GLOBAL_ERROR_LOG_CRITICAL_TEMPERATURE, "GLOBAL_ERROR_LOG: CRITICAL_TEMPERATURE"}
+        }
+    },
+
+    {
+        REGS::R05,
+        {   {FIELDS::R05_SWA_POWER_GOOD, "PMIC POWER ON: SWA_PWR_NOT_GOOD"},
+            {FIELDS::R05_SWB_POWER_GOOD, "PMIC POWER ON: SWB_PWR_NOT_GOOD"},
+            {FIELDS::R05_SWC_POWER_GOOD, "PMIC POWER ON: SWC_PWR_NOT_GOOD"},
+            {FIELDS::R05_SWD_POWER_GOOD, "PMIC POWER ON: SWD_PWR_NOT_GOOD"}
+        }
+    },
+
+    {
+        REGS::R06,
+        {   {FIELDS::R06_SWA_UNDER_VOLTAGE_LOCKOUT, "SWA_UNDER_VOLTAGE_LOCKOUT"},
+            {FIELDS::R06_SWB_UNDER_VOLTAGE_LOCKOUT, "SWB_UNDER_VOLTAGE_LOCKOUT"},
+            {FIELDS::R06_SWC_UNDER_VOLTAGE_LOCKOUT, "SWC_UNDER_VOLTAGE_LOCKOUT"},
+            {FIELDS::R06_SWD_UNDER_VOLTAGE_LOCKOUT, "SWD_UNDER_VOLTAGE_LOCKOUT"},
+            {FIELDS::R06_SWA_OVER_VOLTAGE, "SWA_OVER_VOLTAGE"},
+            {FIELDS::R06_SWB_OVER_VOLTAGE, "SWB_OVER_VOLTAGE"},
+            {FIELDS::R06_SWC_OVER_VOLTAGE, "SWC_OVER_VOLTAGE"},
+            {FIELDS::R06_SWD_OVER_VOLTAGE, "SWD_OVER_VOLTAGE"}
+        }
+    }
+};
+
+///
+/// @brief const vector of statuses to check
+///
+static const std::vector<std::pair<uint8_t, std::vector<status_field>>> STATUS_FIELDS =
+{
+    {
+        REGS::R08,
+        {   {FIELDS::R08_VIN_BULK_INPUT_PWR_GOOD_STATUS, "VIN_BULK_INPUT_PWR_NOT_GOOD"},
+            {FIELDS::R08_CRITICAL_TEMP_SHUTDOWN_STATUS,  "CRITICAL_TEMP_SHUTDOWN"},
+            {FIELDS::R08_SWA_PWR_GOOD_STATUS,            "SWA_PWR_NOT_GOOD"},
+            {FIELDS::R08_SWB_PWR_GOOD_STATUS,            "SWB_PWR_NOT_GOOD"},
+            {FIELDS::R08_SWC_PWR_GOOD_STATUS,            "SWC_PWR_NOT_GOOD"},
+            {FIELDS::R08_SWD_PWR_GOOD_STATUS,            "SWD_PWR_NOT_GOOD"},
+            {FIELDS::R08_VIN_MGMT_INPUT_OVER_VOLTAGE,    "VIN_MGMT_INPUT_OVER_VOLTAGE"},
+            {FIELDS::R08_VIN_BULK_INPUT_OVER_VOLTAGE,    "VIN_BULK_INPUT_OVER_VOLTAGE"}
+        }
+    },
+
+    {
+        REGS::R09,
+        {   {FIELDS::R09_PMIC_HIGH_TEMP_WARNING_STATUS,                      "PMIC Temperature exceeded warning threshold"},
+            {FIELDS::R09_VBIAS_PWR_GOOD_STATUS,                              "VBIAS_PWR_NOT_GOOD"},
+            {FIELDS::R09_VOUT_1_8_V_PWR_GOOD_STATUS,                         "VOUT_1.8V_PWR_NOT_GOOD"},
+            {FIELDS::R09_VIN_MGMT_TO_VIN_BULK_SWITCHOVER_STATUS,             "VIN_MGMT is removed (using VIN_Bulk)"},
+            {FIELDS::R09_SWA_HIGH_OUTPUT_CURRENT_CONSUMPTION_WARNING_STATUS, "SWA_HIGH_OUTPUT_CURRENT_CONSUMPTION_WARNING"},
+            {FIELDS::R09_SWB_HIGH_OUTPUT_CURRENT_CONSUMPTION_WARNING_STATUS, "SWB_HIGH_OUTPUT_CURRENT_CONSUMPTION_WARNING"},
+            {FIELDS::R09_SWC_HIGH_OUTPUT_CURRENT_CONSUMPTION_WARNING_STATUS, "SWC_HIGH_OUTPUT_CURRENT_CONSUMPTION_WARNING"},
+            {FIELDS::R09_SWD_HIGH_OUTPUT_CURRENT_CONSUMPTION_WARNING_STATUS, "SWD_HIGH_OUTPUT_CURRENT_CONSUMPTION_WARNING"}
+        }
+    },
+
+    {
+        REGS::R0A,
+        {   {FIELDS::R0A_SWA_OUTPUT_OVER_VOLTAGE_STATUS, "SWA_OUTPUT_OVER_VOLTAGE"},
+            {FIELDS::R0A_SWB_OUTPUT_OVER_VOLTAGE_STATUS, "SWB_OUTPUT_OVER_VOLTAGE"},
+            {FIELDS::R0A_SWC_OUTPUT_OVER_VOLTAGE_STATUS, "SWC_OUTPUT_OVER_VOLTAGE"},
+            {FIELDS::R0A_SWD_OUTPUT_OVER_VOLTAGE_STATUS, "SWD_OUTPUT_OVER_VOLTAGE"},
+            {FIELDS::R0A_PEC_ERROR_STATUS,               "PEC_ERROR"},
+            {FIELDS::R0A_PARITY_ERROR_STATUS,            "PARITY_ERROR"},
+            {FIELDS::R0A_IBI_STATUS,                     "PENDING_IBI"}
+        }
+    },
+
+    {
+        REGS::R0B,
+        {   {FIELDS::R0B_SWA_OUTPUT_CURRENT_LIMITER_WARNING_STATUS, "SWA_OUTPUT_CURRENT_LIMITER_EVENT"},
+            {FIELDS::R0B_SWB_OUTPUT_CURRENT_LIMITER_WARNING_STATUS, "SWB_OUTPUT_CURRENT_LIMITER_EVENT"},
+            {FIELDS::R0B_SWC_OUTPUT_CURRENT_LIMITER_WARNING_STATUS, "SWC_OUTPUT_CURRENT_LIMITER_EVENT"},
+            {FIELDS::R0B_SWD_OUTPUT_CURRENT_LIMITER_WARNING_STATUS, "SWD_OUTPUT_CURRENT_LIMITER_EVENT"},
+            {FIELDS::R0B_SWA_OUTPUT_UNDER_VOLTAGE_LOCKOUT_STATUS  , "SWA_OUTPUT_UNDER_VOLTAGE_LOCKOUT"},
+            {FIELDS::R0B_SWB_OUTPUT_UNDER_VOLTAGE_LOCKOUT_STATUS  , "SWB_OUTPUT_UNDER_VOLTAGE_LOCKOUT"},
+            {FIELDS::R0B_SWC_OUTPUT_UNDER_VOLTAGE_LOCKOUT_STATUS  , "SWC_OUTPUT_UNDER_VOLTAGE_LOCKOUT"},
+            {FIELDS::R0B_SWD_OUTPUT_UNDER_VOLTAGE_LOCKOUT_STATUS  , "SWD_OUTPUT_UNDER_VOLTAGE_LOCKOUT"}
+        }
+    }
+};
+
+///
+/// @brief Checks that the PMIC is enabled via VR Enable bit
+///
+/// @param[in] i_ocmb_target OCMB target
+/// @param[in] i_pmic_target PMIC target
+/// @return fapi2::ReturnCode FAPI2_RC_SUCCESS iff success, else error code
+///
+fapi2::ReturnCode check_for_vr_enable(
+    const fapi2::Target<fapi2::TARGET_TYPE_OCMB_CHIP>& i_ocmb_target,
+    const fapi2::Target<fapi2::TargetType::TARGET_TYPE_PMIC>& i_pmic_target);
+
+///
+/// @brief Check the statuses of all PMICs present on the given OCMB chip
+///
+/// @param[in] i_ocmb_target OCMB target
+/// @return fapi2::ReturnCode FAPI2_RC_SUCCESS if success, else error code
+/// @note the returned target is only valid if o_errors returns as true. Else, the target is an uninitialized blank target!
+///
+fapi2::ReturnCode check_all_pmics(const fapi2::Target<fapi2::TARGET_TYPE_OCMB_CHIP>& i_ocmb_target);
+
+///
+/// @brief Check the PMIC's status codes and report back if an error occurred
+///
+/// @param[in] i_pmic_target PMIC target
+/// @param[out] o_error true/false
+/// @return fapi2::ReturnCode FAPI2_RC_SUCCESS iff success, else error
+///
+fapi2::ReturnCode check_pmic(const fapi2::Target<fapi2::TARGET_TYPE_PMIC>& i_pmic_target, bool& o_error);
+
+///
+/// @brief Check the IDT specific status codes
+///
+/// @param[in] i_pmic_target PMIC target
+/// @param[in] i_statuses STATUS object to check
+/// @param[out] o_error At least one error bit was found to be set
+/// @return fapi2::ReturnCode FAPI2_RC_SUCCESS iff success, else error in case of an I2C read error
+///
+fapi2::ReturnCode check_fields(
+    const fapi2::Target<fapi2::TARGET_TYPE_PMIC>& i_pmic_target,
+    const std::vector<std::pair<uint8_t, std::vector<status_field>>>& i_statuses,
+    bool& o_error);
+
+} // status
 } // pmic
 } // mss
 
diff --git a/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_enable_utils.C b/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_enable_utils.C
index ec85b7da4..d77805122 100644
--- a/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_enable_utils.C
+++ b/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_enable_utils.C
@@ -44,6 +44,7 @@
 #include <generic/memory/lib/utils/index.H>
 #include <generic/memory/lib/utils/find.H>
 #include <mss_pmic_attribute_getters.H>
+#include <mss_generic_attribute_getters.H>
 
 namespace mss
 {
@@ -441,8 +442,9 @@ fapi_try_exit:
 ///
 /// @brief Enable pmics using manual mode (direct VR enable, no SPD fields)
 /// @param[in] i_pmics vector of PMICs to enable
+/// @return FAPI2_RC_SUCCESS iff success, else error
 ///
-fapi2::ReturnCode enable_manual(const std::vector<fapi2::Target<fapi2::TARGET_TYPE_PMIC>> i_pmics)
+fapi2::ReturnCode enable_manual(const std::vector<fapi2::Target<fapi2::TARGET_TYPE_PMIC>>& i_pmics)
 {
     using CONSTS = mss::pmic::consts<mss::pmic::product::JEDEC_COMPLIANT>;
     using REGS = pmicRegs<mss::pmic::product::JEDEC_COMPLIANT>;
@@ -479,8 +481,8 @@ fapi_try_exit:
 /// @param[in] i_vendor_id - the vendor ID of the PMIC to bias
 /// @return fapi2::ReturnCode - FAPI2_RC_SUCCESS if successful
 ///
-fapi2::ReturnCode enable_chip_1U_2U(const fapi2::Target<fapi2::TargetType::TARGET_TYPE_PMIC>& i_pmic_target,
-                                    const fapi2::Target<fapi2::TargetType::TARGET_TYPE_DIMM>& i_dimm_target,
+fapi2::ReturnCode enable_chip_1U_2U(const fapi2::Target<fapi2::TARGET_TYPE_PMIC>& i_pmic_target,
+                                    const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_dimm_target,
                                     const uint16_t i_vendor_id)
 {
     FAPI_INF("Setting PMIC %s settings from SPD", mss::c_str(i_pmic_target));
@@ -518,5 +520,64 @@ fapi_try_exit:
     return fapi2::current_err;
 }
 
+///
+/// @brief Enable PMIC for SPD mode
+///
+/// @param[in] i_pmics vector of PMICs sorted by mss::index
+/// @param[in] i_dimms const vector of DIMMs sorted by mss::index
+/// @return fapi2::ReturnCode FAPI2_RC_SUCCESS iff success, else error code
+///
+fapi2::ReturnCode pmic_enable_SPD(
+    std::vector<fapi2::Target<fapi2::TARGET_TYPE_PMIC>>& i_pmics,
+    const std::vector<fapi2::Target<fapi2::TARGET_TYPE_DIMM>>& i_dimms)
+{
+    uint8_t l_module_height = 0;
+    FAPI_TRY(mss::attr::get_dram_module_height(i_dimms[0], l_module_height));
+
+    FAPI_ASSERT(l_module_height == fapi2::ENUM_ATTR_MEM_EFF_DRAM_MODULE_HEIGHT_1U ||
+                l_module_height == fapi2::ENUM_ATTR_MEM_EFF_DRAM_MODULE_HEIGHT_2U,
+                fapi2::PMIC_DIMM_SPD_UNSUPPORTED_MODULE_HEIGHT()
+                .set_TARGET(i_dimms[0])
+                .set_VALUE(l_module_height),
+                "DIMM %s module height attribute not identified as 1U or 2U. "
+                "ENUM_ATTR_MEM_EFF_DRAM_MODULE_HEIGHT of %u . Not supported yet.",
+                mss::c_str(i_dimms[0]), l_module_height);
+
+    // Now we know there are 2 pmics per dimm (for DDIMM only... TK will change with future refactor)
+    static constexpr uint8_t PMICS_PER_DIMM = 2;
+
+    for (uint8_t l_dimm_index = 0; l_dimm_index < i_dimms.size(); ++l_dimm_index)
+    {
+        // The PMICs are in sorted order
+        const auto& l_dimm = i_dimms[l_dimm_index];
+        FAPI_TRY(mss::pmic::order_pmics_by_sequence(l_dimm, l_dimm_index, PMICS_PER_DIMM, i_pmics));
+
+        // Now the PMICs are in the right order of DIMM and the right order by their defined SPD sequence within each dimm
+        // Let's kick off the enables
+        for (const auto& l_pmic : i_pmics)
+        {
+            // Get the corresponding DIMM target to feed to the helpers
+            const auto& l_dimm = i_dimms[mss::index(l_pmic) / PMICS_PER_DIMM];
+            uint16_t l_vendor_id = 0;
+
+            // Get vendor ID
+            FAPI_TRY(mss::pmic::get_mfg_id[mss::index(l_pmic)](l_dimm, l_vendor_id));
+
+            // Poll to make sure PBULK reports good, then we can enable the chip and write/read registers
+            FAPI_TRY(mss::pmic::poll_for_pbulk_good(l_pmic),
+                     "pmic_enable: poll for pbulk good either failed, or returned not good status on PMIC %s",
+                     mss::c_str(l_pmic));
+
+            // Call the enable procedure
+            FAPI_TRY((mss::pmic::enable_chip_1U_2U(l_pmic, l_dimm, l_vendor_id)),
+                     "pmic_enable: Error enabling PMIC %s", mss::c_str(l_pmic));
+        }
+    }
+
+    return fapi2::FAPI2_RC_SUCCESS;
+
+fapi_try_exit:
+    return fapi2::current_err;
+}
 } // pmic
 } // mss
diff --git a/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_enable_utils.H b/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_enable_utils.H
index a393209a6..680bb8f9b 100644
--- a/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_enable_utils.H
+++ b/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_enable_utils.H
@@ -750,7 +750,7 @@ fapi_try_exit:
 /// @brief Enable pmics using manual mode (direct VR enable, no SPD fields)
 /// @param[in] i_pmics vector of PMICs to enable
 ///
-fapi2::ReturnCode enable_manual(const std::vector<fapi2::Target<fapi2::TARGET_TYPE_PMIC>> i_pmics);
+fapi2::ReturnCode enable_manual(const std::vector<fapi2::Target<fapi2::TARGET_TYPE_PMIC>>& i_pmics);
 
 ///
 /// @brief Function to enable 1U and 2U pmics
@@ -760,10 +760,20 @@ fapi2::ReturnCode enable_manual(const std::vector<fapi2::Target<fapi2::TARGET_TY
 /// @param[in] i_vendor_id - the vendor ID of the PMIC to bias
 /// @return fapi2::ReturnCode - FAPI2_RC_SUCCESS if successful
 ///
-fapi2::ReturnCode enable_chip_1U_2U(const fapi2::Target<fapi2::TargetType::TARGET_TYPE_PMIC>& i_pmic_target,
-                                    const fapi2::Target<fapi2::TargetType::TARGET_TYPE_DIMM>& i_dimm_target,
+fapi2::ReturnCode enable_chip_1U_2U(const fapi2::Target<fapi2::TARGET_TYPE_PMIC>& i_pmic_target,
+                                    const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_dimm_target,
                                     const uint16_t i_vendor_id);
 
+///
+/// @brief Enable PMIC for SPD mode
+///
+/// @param[in] i_pmics vector of PMICs sorted by mss::index
+/// @param[in] i_dimms const vector of DIMMs sorted by mss::index
+/// @return fapi2::ReturnCode FAPI2_RC_SUCCESS iff success, else error code
+///
+fapi2::ReturnCode pmic_enable_SPD(
+    std::vector<fapi2::Target<fapi2::TARGET_TYPE_PMIC>>& i_pmics,
+    const std::vector<fapi2::Target<fapi2::TARGET_TYPE_DIMM>>& i_dimms);
 }
 } // mss
 
diff --git a/src/import/chips/ocmb/common/procedures/hwp/pmic/pmic_enable.C b/src/import/chips/ocmb/common/procedures/hwp/pmic/pmic_enable.C
index 578ddeac3..bec758f65 100644
--- a/src/import/chips/ocmb/common/procedures/hwp/pmic/pmic_enable.C
+++ b/src/import/chips/ocmb/common/procedures/hwp/pmic/pmic_enable.C
@@ -41,7 +41,6 @@
 #include <lib/utils/pmic_consts.H>
 #include <generic/memory/lib/utils/shared/mss_generic_consts.H>
 #include <generic/memory/lib/utils/c_str.H>
-#include <mss_generic_attribute_getters.H>
 
 extern "C"
 {
@@ -68,58 +67,22 @@ extern "C"
         if (i_mode == mss::pmic::enable_mode::MANUAL)
         {
             FAPI_TRY(mss::pmic::enable_manual(l_pmics));
-            return fapi2::FAPI2_RC_SUCCESS;
         }
-
-        if (!l_dimms.empty())
+        else
         {
-            uint8_t l_module_height = 0;
-            FAPI_TRY(mss::attr::get_dram_module_height(l_dimms[0], l_module_height));
-
-            FAPI_ASSERT(l_module_height == fapi2::ENUM_ATTR_MEM_EFF_DRAM_MODULE_HEIGHT_1U ||
-                        l_module_height == fapi2::ENUM_ATTR_MEM_EFF_DRAM_MODULE_HEIGHT_2U,
-                        fapi2::PMIC_DIMM_SPD_UNSUPPORTED_MODULE_HEIGHT()
-                        .set_TARGET(l_dimms[0])
-                        .set_VALUE(l_module_height),
-                        "DIMM %s module height attribute not identified as 1U or 2U. "
-                        "ENUM_ATTR_MEM_EFF_DRAM_MODULE_HEIGHT of %u . Not supported yet.",
-                        mss::c_str(l_dimms[0]), l_module_height);
-
-            // Else, 1 or 2
+            if (!l_dimms.empty())
             {
-                static constexpr uint8_t PMICS_PER_DIMM = 2;
-
-                for (uint8_t l_dimm_index = 0; l_dimm_index < l_dimms.size(); ++l_dimm_index)
-                {
-                    // The PMICs are in sorted order
-                    const auto& l_dimm = l_dimms[l_dimm_index];
-                    FAPI_TRY(mss::pmic::order_pmics_by_sequence(l_dimm, l_dimm_index, PMICS_PER_DIMM, l_pmics));
-
-                    // Now the PMICs are in the right order of DIMM and the right order by their defined SPD sequence within each dimm
-                    // Let's kick off the enables
-                    for (const auto& l_pmic : l_pmics)
-                    {
-                        // Get the corresponding DIMM target to feed to the helpers
-                        const auto& l_dimm = l_dimms[mss::index(l_pmic) / PMICS_PER_DIMM];
-                        uint16_t l_vendor_id = 0;
-
-                        // Get vendor ID
-                        FAPI_TRY(mss::pmic::get_mfg_id[mss::index(l_pmic)](l_dimm, l_vendor_id));
-
-                        // Poll to make sure PBULK reports good, then we can enable the chip and write/read registers
-                        FAPI_TRY(mss::pmic::poll_for_pbulk_good(l_pmic),
-                                 "pmic_enable: poll for pbulk good either failed, or returned not good status on PMIC %s",
-                                 mss::c_str(l_pmic));
-
-                        // Call the enable procedure
-                        FAPI_TRY((mss::pmic::enable_chip_1U_2U
-                                  (l_pmic, l_dimm, l_vendor_id)),
-                                 "pmic_enable: Error enabling PMIC %s", mss::c_str(l_pmic));
-                    }
-                }
+                FAPI_TRY(mss::pmic::pmic_enable_SPD(l_pmics, l_dimms));
             }
         }
 
+        // Check that all the PMIC statuses are good post-enable
+        FAPI_TRY(mss::pmic::status::check_all_pmics(i_ocmb_target),
+                 "Bad statuses returned, or error checking statuses of PMICs on %s", mss::c_str(i_ocmb_target));
+
+        // If we get here, statuses are good
+        FAPI_INF("All status codes were OK for PMICs on %s", mss::c_str(i_ocmb_target));
+
         return fapi2::FAPI2_RC_SUCCESS;
 
     fapi_try_exit:
diff --git a/src/import/chips/ocmb/common/procedures/xml/error_info/pmic_errors.xml b/src/import/chips/ocmb/common/procedures/xml/error_info/pmic_errors.xml
index 8b83a48fa..fdc355620 100644
--- a/src/import/chips/ocmb/common/procedures/xml/error_info/pmic_errors.xml
+++ b/src/import/chips/ocmb/common/procedures/xml/error_info/pmic_errors.xml
@@ -151,4 +151,59 @@
     </callout>
   </hwpError>
 
+  <hwpError>
+    <rc>RC_PMIC_NOT_ENABLED</rc>
+    <description>
+       After running pmic_enable, the PMIC VR Enable bit did not remain set.
+       Therefore, the PMIC did not enable successfully.
+    </description>
+    <ffdc>PMIC_TARGET</ffdc>
+    <ffdc>OCMB_TARGET</ffdc>
+    <callout>
+      <procedure>CODE</procedure>
+      <priority>MEDIUM</priority>
+    </callout>
+    <callout>
+      <target>PMIC_TARGET</target>
+      <priority>HIGH</priority>
+    </callout>
+    <callout>
+      <target>OCMB_TARGET</target>
+      <priority>HIGH</priority>
+    </callout>
+    <deconfigure>
+      <target>OCMB_TARGET</target>
+    </deconfigure>
+    <gard>
+      <target>OCMB_TARGET</target>
+    </gard>
+  </hwpError>
+
+  <hwpError>
+    <rc>RC_PMIC_STATUS_ERRORS</rc>
+    <description>
+       After running pmic_enable, one or more error status bits were set on the PMICs of this OCMB.
+    </description>
+    <ffdc>OCMB_TARGET</ffdc>
+    <ffdc>PMIC_TARGET</ffdc>
+    <callout>
+      <procedure>CODE</procedure>
+      <priority>MEDIUM</priority>
+    </callout>
+    <callout>
+      <target>PMIC_TARGET</target>
+      <priority>HIGH</priority>
+    </callout>
+    <callout>
+      <target>OCMB_TARGET</target>
+      <priority>HIGH</priority>
+    </callout>
+    <deconfigure>
+      <target>OCMB_TARGET</target>
+    </deconfigure>
+    <gard>
+      <target>OCMB_TARGET</target>
+    </gard>
+  </hwpError>
+
 </hwpErrors>