Add support for new pmic sequencing SPD fields

Change-Id: I8847090585161375fbb2c0ef853cffed80a67cc3
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/82961
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Reviewed-by: Louis Stermole <stermole@us.ibm.com>
Reviewed-by: STEPHEN GLANCY <sglancy@us.ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Hostboot CI <hostboot-ci+hostboot@us.ibm.com>
Tested-by: HWSV CI <hwsv-ci+hostboot@us.ibm.com>
Reviewed-by: Jennifer A Stofer <stofer@us.ibm.com>
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/83375
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Reviewed-by: Christian R Geddes <crgeddes@us.ibm.com>

14 files changed, 480 insertions, 240 deletions

diff --git a/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/eff_config/pmic_attr_engine_traits.H b/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/eff_config/pmic_attr_engine_traits.H
index 45ac80abd..ebee62c53 100644
--- a/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/eff_config/pmic_attr_engine_traits.H
+++ b/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/eff_config/pmic_attr_engine_traits.H
@@ -2379,6 +2379,108 @@ struct attrEngineTraits<P, pmic::attr_eff_engine_fields, pmic::attr_eff_engine_f
     }
 };
 
+//
+/// @brief Traits for attr_engine
+/// @class attrEngineTraits
+/// @tparam P processor type
+/// @note P, pmic::attr_eff_engine_fields, PMIC0_SEQUENCE partial specialization
+///
+template< proc_type P>
+struct attrEngineTraits<P, pmic::attr_eff_engine_fields, pmic::attr_eff_engine_fields::PMIC0_SEQUENCE>
+{
+    using attr_type = fapi2::ATTR_MEM_EFF_PMIC0_SEQUENCE_Type;
+    using attr_integral_type = std::remove_all_extents<attr_type>::type;
+    static constexpr fapi2::TargetType TARGET_TYPE = fapi2::ATTR_MEM_EFF_PMIC0_SEQUENCE_TargetType;
+    static constexpr pmic::ffdc_codes FFDC_CODE = pmic::SET_PMIC0_SEQUENCE;
+
+    ///
+    /// @brief attribute getter
+    /// @param[in] i_target the fapi2 target
+    /// @param[out] o_setting array to populate
+    /// @return FAPI2_RC_SUCCESS iff okay
+    ///
+    static fapi2::ReturnCode get_attr(const fapi2::Target<TARGET_TYPE>& i_target,
+                                      attr_type& o_setting)
+    {
+        return attr::get_pmic0_sequence(i_target, o_setting);
+    }
+
+    ///
+    /// @brief attribute setter
+    /// @param[in] i_target the fapi2 target
+    /// @param[in] i_setting array to set
+    /// @return FAPI2_RC_SUCCESS iff okay
+    ///
+    static fapi2::ReturnCode set_attr(const fapi2::Target<TARGET_TYPE>& i_target,
+                                      attr_type& i_setting)
+    {
+        return attr::set_pmic0_sequence(i_target, i_setting);
+    }
+
+    ///
+    /// @brief Computes setting for attribute
+    /// @param[in] i_spd_data EFD data
+    /// @param[out] o_setting value we want to set attr with
+    /// @return FAPI2_RC_SUCCESS iff okay
+    ///
+    static fapi2::ReturnCode get_value_to_set(const spd::facade& i_spd_data,
+            attr_integral_type& o_setting)
+    {
+        return i_spd_data.sequence_pmic0(o_setting);
+    }
+};
+
+//
+/// @brief Traits for attr_engine
+/// @class attrEngineTraits
+/// @tparam P processor type
+/// @note P, pmic::attr_eff_engine_fields, PMIC1_SEQUENCE partial specialization
+///
+template< proc_type P>
+struct attrEngineTraits<P, pmic::attr_eff_engine_fields, pmic::attr_eff_engine_fields::PMIC1_SEQUENCE>
+{
+    using attr_type = fapi2::ATTR_MEM_EFF_PMIC1_SEQUENCE_Type;
+    using attr_integral_type = std::remove_all_extents<attr_type>::type;
+    static constexpr fapi2::TargetType TARGET_TYPE = fapi2::ATTR_MEM_EFF_PMIC1_SEQUENCE_TargetType;
+    static constexpr pmic::ffdc_codes FFDC_CODE = pmic::SET_PMIC1_SEQUENCE;
+
+    ///
+    /// @brief attribute getter
+    /// @param[in] i_target the fapi2 target
+    /// @param[out] o_setting array to populate
+    /// @return FAPI2_RC_SUCCESS iff okay
+    ///
+    static fapi2::ReturnCode get_attr(const fapi2::Target<TARGET_TYPE>& i_target,
+                                      attr_type& o_setting)
+    {
+        return attr::get_pmic1_sequence(i_target, o_setting);
+    }
+
+    ///
+    /// @brief attribute setter
+    /// @param[in] i_target the fapi2 target
+    /// @param[in] i_setting array to set
+    /// @return FAPI2_RC_SUCCESS iff okay
+    ///
+    static fapi2::ReturnCode set_attr(const fapi2::Target<TARGET_TYPE>& i_target,
+                                      attr_type& i_setting)
+    {
+        return attr::set_pmic1_sequence(i_target, i_setting);
+    }
+
+    ///
+    /// @brief Computes setting for attribute
+    /// @param[in] i_spd_data EFD data
+    /// @param[out] o_setting value we want to set attr with
+    /// @return FAPI2_RC_SUCCESS iff okay
+    ///
+    static fapi2::ReturnCode get_value_to_set(const spd::facade& i_spd_data,
+            attr_integral_type& o_setting)
+    {
+        return i_spd_data.sequence_pmic1(o_setting);
+    }
+};
+
 }//mss
 
 #endif
diff --git a/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_consts.H b/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_consts.H
index 1d1b4feaf..4cb511dae 100644
--- a/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_consts.H
+++ b/src/import/chips/ocmb/common/procedures/hwp/pmic/lib/utils/pmic_consts.H
@@ -156,9 +156,11 @@ enum class attr_eff_engine_fields
 
     PMIC0_MFG_ID                        = 43,
     PMIC1_MFG_ID                        = 44,
+    PMIC0_SEQUENCE                      = 45,
+    PMIC1_SEQUENCE                      = 46,
 
     // Dispatcher set to last enum value
-    DISPATCHER = PMIC1_MFG_ID,
+    DISPATCHER = PMIC1_SEQUENCE,
 };
 
 ///
@@ -220,7 +222,8 @@ enum ffdc_codes
     SET_PMIC0_MFG_ID                        = 0x107C,
     SET_PMIC1_MFG_ID                        = 0x107D,
 
-    SET_DRAM_MODULE_HEIGHT                  = 0x107E,
+    SET_PMIC0_SEQUENCE                      = 0x107E,
+    SET_PMIC1_SEQUENCE                      = 0x107F,
 };
 
 ///
@@ -292,41 +295,6 @@ struct consts<mss::pmic::product::JEDEC_COMPLIANT>
     static constexpr uint32_t VOLT_STEP                    = 5;
 };
 
-///
-/// @brief PMIC traits that change depending on DIMM module height
-///
-/// @tparam H module_height enum
-///
-template<mss::pmic::module_height H>
-struct pmic_traits;
-
-///
-/// @brief pmic traits for 1U dimms
-///
-template <>
-struct pmic_traits<mss::pmic::module_height::HEIGHT_1U>
-{
-    static constexpr uint8_t PMICS_PER_DIMM = 2;
-};
-
-///
-/// @brief pmic traits for 2U dimms
-///
-template <>
-struct pmic_traits<mss::pmic::module_height::HEIGHT_2U>
-{
-    static constexpr uint8_t PMICS_PER_DIMM = 2;
-};
-
-///
-/// @brief pmic traits for 4U dimms
-///
-template <>
-struct pmic_traits<mss::pmic::module_height::HEIGHT_4U>
-{
-    static constexpr uint8_t PMICS_PER_DIMM = 4;
-};
-
 namespace i2c
 {
 
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 65bf83b5a..ec85b7da4 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
@@ -380,5 +380,143 @@ fapi_try_exit:
     return fapi2::current_err;
 }
 
+///
+/// @brief Order PMICs by sequence defined in the SPD
+///
+/// @param[in] i_dimm DIMM target to pull SPD fields from
+/// @param[in] i_dimm_index index of the DIMM target
+/// @param[in] i_pmics_per_dimm number of PMICs per dimm
+/// @param[in,out] io_pmics vector of PMICs that will be re-ordered in place
+/// @return fapi2::ReturnCode
+///
+fapi2::ReturnCode order_pmics_by_sequence(
+    const fapi2::Target<fapi2::TARGET_TYPE_DIMM> i_dimm,
+    const uint8_t i_dimm_index,
+    const uint8_t i_pmics_per_dimm,
+    std::vector<fapi2::Target<fapi2::TARGET_TYPE_PMIC>>& io_pmics)
+{
+    const auto l_begin_offset = i_dimm_index * i_pmics_per_dimm;
+    const auto l_iterator_begin = io_pmics.begin() + l_begin_offset;
+    const auto l_iterator_end = l_iterator_begin + i_pmics_per_dimm;
+
+    fapi2::ReturnCode l_rc_0_out = fapi2::FAPI2_RC_SUCCESS;
+    fapi2::ReturnCode l_rc_1_out = fapi2::FAPI2_RC_SUCCESS;
+
+    std::sort(l_iterator_begin, l_iterator_end, [&l_rc_0_out, &l_rc_1_out, i_dimm] (
+                  const fapi2::Target<fapi2::TARGET_TYPE_PMIC>& l_first_pmic,
+                  const fapi2::Target<fapi2::TARGET_TYPE_PMIC>& l_second_pmic) -> bool
+    {
+        // Here we should only be dealing with PMICs of the same DIMM. So we can just check the first one which dimm we're on
+        uint8_t l_sequence_pmic_0 = 0;
+        uint8_t l_sequence_pmic_1 = 0;
+
+        // Need to pull out the RC's manually. Goto's in lambdas apparently don't play nicely
+        fapi2::ReturnCode l_rc_0 = mss::pmic::get_sequence[mss::index(l_first_pmic)](i_dimm, l_sequence_pmic_0);
+        fapi2::ReturnCode l_rc_1 = mss::pmic::get_sequence[mss::index(l_second_pmic)](i_dimm, l_sequence_pmic_1);
+
+        // Hold on to an error if we see one
+        if (l_rc_0 != fapi2::FAPI2_RC_SUCCESS)
+        {
+            l_rc_0_out = l_rc_0;
+        }
+        if (l_rc_1 != fapi2::FAPI2_RC_SUCCESS)
+        {
+            l_rc_1_out = l_rc_1;
+        }
+
+        return l_sequence_pmic_0 < l_sequence_pmic_1;
+    });
+
+    FAPI_TRY(l_rc_0_out, "Error getting sequencing attributes for PMICs associated with DIMM 0 target %s",
+             mss::c_str(i_dimm));
+    FAPI_TRY(l_rc_1_out, "Error getting sequencing attributes for PMICs associated with DIMM 1 target %s",
+             mss::c_str(i_dimm));
+
+    return fapi2::FAPI2_RC_SUCCESS;
+
+fapi_try_exit:
+    return fapi2::current_err;
+}
+
+///
+/// @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)
+{
+    using CONSTS = mss::pmic::consts<mss::pmic::product::JEDEC_COMPLIANT>;
+    using REGS = pmicRegs<mss::pmic::product::JEDEC_COMPLIANT>;
+    using FIELDS = pmicFields<mss::pmic::product::JEDEC_COMPLIANT>;
+
+    for (const auto& l_pmic : i_pmics)
+    {
+        fapi2::buffer<uint8_t> l_programmable_mode;
+        l_programmable_mode.writeBit<FIELDS::R2F_SECURE_MODE>(CONSTS::PROGRAMMABLE_MODE);
+
+        FAPI_INF("Enabling PMIC %s with default settings", mss::c_str(l_pmic));
+
+        // Make sure power is applied and we can read the PMIC
+        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));
+
+        // Enable programmable mode
+        FAPI_TRY(mss::pmic::i2c::reg_write_reverse_buffer(l_pmic, REGS::R2F, l_programmable_mode));
+
+        // Start VR Enable
+        FAPI_TRY(mss::pmic::start_vr_enable(l_pmic),
+                 "Error starting VR_ENABLE on PMIC %s", mss::c_str(l_pmic));
+    }
+
+fapi_try_exit:
+    return fapi2::current_err;
+}
+///
+/// @brief Function to enable 1U and 2U pmics
+///
+/// @param[in] i_pmic_target - the pmic target
+/// @param[in] i_dimm_target - the dimm target that the PMIC resides on
+/// @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,
+                                    const uint16_t i_vendor_id)
+{
+    FAPI_INF("Setting PMIC %s settings from SPD", mss::c_str(i_pmic_target));
+
+    // Make sure it is TI or IDT
+    FAPI_ASSERT((i_vendor_id == mss::pmic::vendor::IDT ||
+                 i_vendor_id == mss::pmic::vendor::TI),
+                fapi2::PMIC_CHIP_NOT_RECOGNIZED()
+                .set_TARGET(i_pmic_target)
+                .set_VENDOR_ID(i_vendor_id),
+                "Unknown PMIC: %s with vendor ID 0x%04hhX",
+                mss::c_str(i_pmic_target),
+                uint8_t(i_vendor_id) );
+
+    if (i_vendor_id == mss::pmic::vendor::IDT)
+    {
+        FAPI_TRY(mss::pmic::bias_with_spd_settings<mss::pmic::vendor::IDT>(i_pmic_target, i_dimm_target),
+                 "enable_chip<IDT, 1U/2U>: Error biasing PMIC %s with SPD settings",
+                 mss::c_str(i_pmic_target));
+    }
+    else // assert done in pmic_enable.C that vendor is IDT or TI
+    {
+        FAPI_TRY(mss::pmic::bias_with_spd_settings<mss::pmic::vendor::TI>(i_pmic_target, i_dimm_target),
+                 "enable_chip<TI, 1U/2U>: Error biasing PMIC %s with SPD settings",
+                 mss::c_str(i_pmic_target));
+    }
+
+    // Start VR Enable
+    FAPI_TRY(mss::pmic::start_vr_enable(i_pmic_target),
+             "Error starting VR_ENABLE on PMIC %s", mss::c_str(i_pmic_target));
+
+    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 5de810f55..a393209a6 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
@@ -57,6 +57,13 @@ typedef fapi2::ReturnCode (*pmic_attr_ptr_signed)(const fapi2::Target<fapi2::TAR
         int8_t& o_value);
 // Pointers below allow for run-time attribute getter selection by PMIC ID (0,1)
 
+// PMIC0/1 sequence order
+static constexpr pmic_attr_ptr get_sequence[] =
+{
+    mss::attr::get_pmic0_sequence,
+    mss::attr::get_pmic1_sequence
+};
+
 // Voltage Setting
 static constexpr pmic_attr_ptr get_swa_voltage_setting[] =
 {
@@ -504,6 +511,21 @@ fapi_try_exit:
 }
 
 ///
+/// @brief Order PMICs by sequence defined in the SPD
+///
+/// @param[in] i_dimm DIMM target to pull SPD fields from
+/// @param[in] i_dimm_index index of the DIMM target
+/// @param[in] i_pmics_per_dimm number of PMICs per dimm
+/// @param[in,out] io_pmics vector of PMICs that will be re-ordered in place
+/// @return fapi2::ReturnCode
+///
+fapi2::ReturnCode order_pmics_by_sequence(
+    const fapi2::Target<fapi2::TARGET_TYPE_DIMM> i_dimm,
+    const uint8_t i_dimm_index,
+    const uint8_t i_pmics_per_dimm,
+    std::vector<fapi2::Target<fapi2::TARGET_TYPE_PMIC>>& io_pmics);
+
+///
 /// @brief Bias with spd voltages for IDT pmic
 ///
 /// @param[in] i_pmic_target PMIC target
@@ -725,69 +747,22 @@ fapi_try_exit:
 //------------------- ENABLE FUNCTIONS-----------------//
 
 ///
-/// @brief template function for the chip-specific enable functions
-///
-/// @tparam H module_height
-/// @param[in] i_pmic_target - the pmic target
-/// @param[in] i_dimm_target - the dimm target that the PMIC resides on
-/// @param[in] i_vendor_id - the vendor ID of the PMIC to bias
-/// @param[in] i_mode enable mode operation
-/// @return fapi2::ReturnCode - FAPI2_RC_SUCCESS if successful
+/// @brief Enable pmics using manual mode (direct VR enable, no SPD fields)
+/// @param[in] i_pmics vector of PMICs to enable
 ///
-template <mss::pmic::module_height H>
-fapi2::ReturnCode enable_chip(const fapi2::Target<fapi2::TargetType::TARGET_TYPE_PMIC>& i_pmic_target,
-                              const fapi2::Target<fapi2::TargetType::TARGET_TYPE_DIMM>& i_dimm_target,
-                              const uint16_t i_vendor_id);
+fapi2::ReturnCode enable_manual(const std::vector<fapi2::Target<fapi2::TARGET_TYPE_PMIC>> i_pmics);
 
 ///
-/// @brief enable procedure for IDT PMIC and 1U or 2U DIMM
+/// @brief Function to enable 1U and 2U pmics
 ///
-/// @param[in] i_pmic_target - the pmic_target
+/// @param[in] i_pmic_target - the pmic target
 /// @param[in] i_dimm_target - the dimm target that the PMIC resides on
 /// @param[in] i_vendor_id - the vendor ID of the PMIC to bias
-/// @param[in] i_mode enable mode operation
 /// @return fapi2::ReturnCode - FAPI2_RC_SUCCESS if successful
 ///
-template <>
-inline fapi2::ReturnCode enable_chip<mss::pmic::module_height::HEIGHT_1U>(
-    const fapi2::Target<fapi2::TARGET_TYPE_PMIC>& i_pmic_target,
-    const fapi2::Target<fapi2::TargetType::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));
-
-    // Make sure it is TI or IDT
-    FAPI_ASSERT((i_vendor_id == mss::pmic::vendor::IDT ||
-                 i_vendor_id == mss::pmic::vendor::TI),
-                fapi2::PMIC_CHIP_NOT_RECOGNIZED()
-                .set_TARGET(i_pmic_target)
-                .set_VENDOR_ID(i_vendor_id),
-                "Unknown PMIC: %s with vendor ID 0x%04hhX",
-                mss::c_str(i_pmic_target),
-                uint8_t(i_vendor_id) );
-
-    if (i_vendor_id == mss::pmic::vendor::IDT)
-    {
-        FAPI_TRY(mss::pmic::bias_with_spd_settings<mss::pmic::vendor::IDT>(i_pmic_target, i_dimm_target),
-                 "enable_chip<IDT, 1U/2U>: Error biasing PMIC %s with SPD settings",
-                 mss::c_str(i_pmic_target));
-    }
-    else // assert done in pmic_enable.C that vendor is IDT or TI
-    {
-        FAPI_TRY(mss::pmic::bias_with_spd_settings<mss::pmic::vendor::TI>(i_pmic_target, i_dimm_target),
-                 "enable_chip<TI, 1U/2U>: Error biasing PMIC %s with SPD settings",
-                 mss::c_str(i_pmic_target));
-    }
-
-    // Start VR Enable
-    FAPI_TRY(mss::pmic::start_vr_enable(i_pmic_target),
-             "Error starting VR_ENABLE on PMIC %s", mss::c_str(i_pmic_target));
-
-    return fapi2::FAPI2_RC_SUCCESS;
-
-fapi_try_exit:
-    return fapi2::current_err;
-}
+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,
+                                    const uint16_t i_vendor_id);
 
 }
 } // 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 d13574ac8..578ddeac3 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
@@ -54,8 +54,8 @@ extern "C"
     fapi2::ReturnCode pmic_enable(const fapi2::Target<fapi2::TARGET_TYPE_OCMB_CHIP>& i_ocmb_target,
                                   const mss::pmic::enable_mode i_mode)
     {
-        auto l_dimms = mss::find_targets<fapi2::TARGET_TYPE_DIMM>(i_ocmb_target);
-        auto l_pmics = mss::find_targets<fapi2::TARGET_TYPE_PMIC>(i_ocmb_target);
+        auto l_dimms = mss::find_targets_sorted_by_index<fapi2::TARGET_TYPE_DIMM>(i_ocmb_target);
+        auto l_pmics = mss::find_targets_sorted_by_index<fapi2::TARGET_TYPE_PMIC>(i_ocmb_target);
 
         // Check that we have PMICs (we wouldn't on gemini, for example)
         if (l_pmics.empty())
@@ -64,114 +64,59 @@ extern "C"
             return fapi2::FAPI2_RC_SUCCESS;
         }
 
-        // Sort by index (low to high) since find_targets may not return the correct order
-        std::sort(l_dimms.begin(), l_dimms.end(),
-                  [] (const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& l_first_dimm,
-                      const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& l_second_dimm) -> bool
-        {
-            return mss::index(l_first_dimm) < mss::index(l_second_dimm);
-        });
-
-        std::sort(l_pmics.begin(), l_pmics.end(),
-                  [] (const fapi2::Target<fapi2::TARGET_TYPE_PMIC>& l_first_pmic,
-                      const fapi2::Target<fapi2::TARGET_TYPE_PMIC>& l_second_pmic) -> bool
-        {
-            return mss::index(l_first_pmic) < mss::index(l_second_pmic);
-        });
-
-        uint8_t l_pmic_index = 0;
-
-        // If we're enabling via internal settings, we can just run VR ENABLE down the line
+        // // If we're enabling via internal settings, we can just run VR ENABLE down the line
         if (i_mode == mss::pmic::enable_mode::MANUAL)
         {
-            using CONSTS = mss::pmic::consts<mss::pmic::product::JEDEC_COMPLIANT>;
-            using REGS = pmicRegs<mss::pmic::product::JEDEC_COMPLIANT>;
-            using FIELDS = pmicFields<mss::pmic::product::JEDEC_COMPLIANT>;
-
-            for (const auto& l_pmic : l_pmics)
-            {
-                fapi2::buffer<uint8_t> l_programmable_mode;
-                l_programmable_mode.writeBit<FIELDS::R2F_SECURE_MODE>(CONSTS::PROGRAMMABLE_MODE);
-
-                FAPI_INF("Enabling PMIC %s with default settings", mss::c_str(l_pmic));
-
-                // Make sure power is applied and we can read the PMIC
-                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));
-
-                // Enable programmable mode
-                FAPI_TRY(mss::pmic::i2c::reg_write_reverse_buffer(l_pmic, REGS::R2F, l_programmable_mode));
-
-                // Start VR Enable
-                FAPI_TRY(mss::pmic::start_vr_enable(l_pmic),
-                         "Error starting VR_ENABLE on PMIC %s", mss::c_str(l_pmic));
-            }
-
+            FAPI_TRY(mss::pmic::enable_manual(l_pmics));
             return fapi2::FAPI2_RC_SUCCESS;
         }
 
-        // Start at PMIC0. If there was ever a weird case where there is a 4U dimm
-        // on the same OCMB as a 2U dimm (is this possible?),
-        // we would have 6 total PMICs. So, we need to keep
-        // track of where we left off for the last pmic we enabled
-
-        // Not asserting vectors non-empty because there could be OCMBs without DIMMs on them
-        for (const auto& l_dimm : l_dimms)
+        if (!l_dimms.empty())
         {
-            // Get module height for DIMM to determine the number of PMICs we should be using
             uint8_t l_module_height = 0;
-            FAPI_TRY(mss::attr::get_dram_module_height(l_dimm, l_module_height));
-
-            if (l_module_height == mss::pmic::module_height::HEIGHT_1U ||
-                l_module_height == mss::pmic::module_height::HEIGHT_2U)
+            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
             {
-                // 1U and 2U are the same sequence, use 1U traits
-                using PMIC_TRAITS = mss::pmic::pmic_traits<mss::pmic::module_height::HEIGHT_1U>;
+                static constexpr uint8_t PMICS_PER_DIMM = 2;
 
-                uint16_t l_vendor_id = 0;
-
-                // PMIC0 and PMIC1 of each DIMM
-                for (uint8_t l_current_pmic = 0; l_current_pmic < PMIC_TRAITS::PMICS_PER_DIMM; ++l_current_pmic)
+                for (uint8_t l_dimm_index = 0; l_dimm_index < l_dimms.size(); ++l_dimm_index)
                 {
-                    const auto l_current_pmic_target = l_pmics[l_pmic_index + l_current_pmic];
-                    // Get vendor ID
-                    FAPI_TRY(mss::pmic::get_mfg_id[l_current_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_current_pmic_target),
-                             "pmic_enable: poll for pbulk good either failed, or returned not good status on PMIC %s",
-                             mss::c_str(l_current_pmic_target));
-
-                    // Call the enable procedure
-                    FAPI_TRY((mss::pmic::enable_chip
-                              <mss::pmic::module_height::HEIGHT_1U>
-                              (l_current_pmic_target, l_dimm, l_vendor_id)),
-                             "pmic_enable: Error enabling PMIC %s", mss::c_str(l_current_pmic_target));
+                    // 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));
+                    }
                 }
-
-                // Increment by the number of PMICs that were enabled and move on to the next dimm
-                l_pmic_index += PMIC_TRAITS::PMICS_PER_DIMM;
-            }
-            else // 4U DIMM:
-            {
-                // Asserting out here as if we see a 4U at this point we shouldn't be able to proceed
-                // Ugly assert false, but we need the above else later so we will use this for now
-                FAPI_ASSERT(false,
-                            fapi2::PMIC_DIMM_SPD_4U()
-                            .set_TARGET(l_dimm),
-                            "DIMM %s module height attribute identified as 4U. Not supported yet.",
-                            mss::c_str(l_dimm));
-
-                // The enable algorithm will be:
-                // Load SPD for PMIC0 and PMIC1
-                // Broadcast enable both together
-
-                // Load SPD for PMIC2 and PMIC3 (which should be the same data as for PMIC0 and PMIC1)
-                // Broadcast and enable both together
-
-                // using PMIC_TRAITS = mss::pmic::pmic_traits<mss::pmic::module_height::HEIGHT_4U>;
-                // l_pmic_index += PMIC_TRAITS::PMICS_PER_DIMM;
             }
         }
 
diff --git a/src/import/chips/ocmb/common/procedures/xml/attribute_info/pmic_eff_attributes.xml b/src/import/chips/ocmb/common/procedures/xml/attribute_info/pmic_eff_attributes.xml
index c671d8eab..a2488a6df 100644
--- a/src/import/chips/ocmb/common/procedures/xml/attribute_info/pmic_eff_attributes.xml
+++ b/src/import/chips/ocmb/common/procedures/xml/attribute_info/pmic_eff_attributes.xml
@@ -597,6 +597,32 @@
     </attribute>
 
     <attribute>
+        <id>ATTR_MEM_EFF_PMIC0_SEQUENCE</id>
+        <targetType>TARGET_TYPE_MEM_PORT</targetType>
+        <description>
+          Sequence order to enable PMIC0
+        </description>
+        <initToZero></initToZero>
+        <valueType>uint8</valueType>
+        <writeable/>
+        <array>2</array>
+        <mssAccessorName>pmic0_sequence</mssAccessorName>
+    </attribute>
+
+    <attribute>
+        <id>ATTR_MEM_EFF_PMIC1_SEQUENCE</id>
+        <targetType>TARGET_TYPE_MEM_PORT</targetType>
+        <description>
+          Sequence order to enable PMIC1
+        </description>
+        <initToZero></initToZero>
+        <valueType>uint8</valueType>
+        <writeable/>
+        <array>2</array>
+        <mssAccessorName>pmic1_sequence</mssAccessorName>
+    </attribute>
+
+    <attribute>
         <id>ATTR_MEM_EFF_EFD_PMIC0_SWA_VOLTAGE_OFFSET</id>
         <targetType>TARGET_TYPE_MEM_PORT</targetType>
         <description>
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 fd80afa98..8b83a48fa 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
@@ -134,12 +134,13 @@
   </hwpError>
 
   <hwpError>
-    <rc>RC_PMIC_DIMM_SPD_4U</rc>
+    <rc>RC_PMIC_DIMM_SPD_UNSUPPORTED_MODULE_HEIGHT</rc>
     <description>
-       The module_height attribute SPD of this DIMM was read as 4U.
-       4U is not supported yet for pmic_enable().
+       The module_height attribute SPD of this DIMM was not identified as 1U or 2U.
+       Other heights (4U) are not supported at this time.
     </description>
     <ffdc>TARGET</ffdc>
+    <ffdc>VALUE</ffdc>
     <callout>
       <procedure>CODE</procedure>
       <priority>MEDIUM</priority>
diff --git a/src/import/generic/memory/lib/spd/common/dimm_module_decoder.H b/src/import/generic/memory/lib/spd/common/dimm_module_decoder.H
index c8ab503ec..b1c47aa3f 100644
--- a/src/import/generic/memory/lib/spd/common/dimm_module_decoder.H
+++ b/src/import/generic/memory/lib/spd/common/dimm_module_decoder.H
@@ -1492,6 +1492,28 @@ class dimm_module_decoder
         }
 
         ///
+        /// @brief Decodes PMIC0 Sequence Order
+        /// @param[out] o_output encoding from SPD
+        /// @return FAPI2_RC_SUCCESS if okay
+        ///
+        virtual fapi2::ReturnCode sequence_pmic0(uint8_t& o_output) const
+        {
+            o_output = 0;
+            return fapi2::FAPI2_RC_SUCCESS;
+        }
+
+        ///
+        /// @brief Decodes PMIC1 Sequence Order
+        /// @param[out] o_output encoding from SPD
+        /// @return FAPI2_RC_SUCCESS if okay
+        ///
+        virtual fapi2::ReturnCode sequence_pmic1(uint8_t& o_output) const
+        {
+            o_output = 0;
+            return fapi2::FAPI2_RC_SUCCESS;
+        }
+
+        ///
         /// @brief Decodes DRAM Manufacturer ID code
         /// @param[out] o_value dram manufacturing id code
         /// @return FAPI2_RC_SUCCESS iff okay
diff --git a/src/import/generic/memory/lib/spd/ddimm/ddr4/ddimm_decoder_ddr4.H b/src/import/generic/memory/lib/spd/ddimm/ddr4/ddimm_decoder_ddr4.H
index 7c1b4f5c9..b72fde057 100644
--- a/src/import/generic/memory/lib/spd/ddimm/ddr4/ddimm_decoder_ddr4.H
+++ b/src/import/generic/memory/lib/spd/ddimm/ddr4/ddimm_decoder_ddr4.H
@@ -642,19 +642,6 @@ class decoder< DDR4, DDIMM_MODULE, R > : public dimm_module_decoder
         }
 
         ///
-        /// @brief Decodes VDD_Core PMIC0 -> VDD_CORE_PMIC0
-        /// @param[out] o_output encoding from SPD
-        /// @return FAPI2_RC_SUCCESS if okay
-        ///
-        virtual fapi2::ReturnCode vdd_core_pmic0(uint8_t& o_output) const override
-        {
-            FAPI_TRY( (mss::spd::reader<fields_t::VDD_CORE_PMIC0, R>(iv_target, iv_data, o_output)) );
-
-        fapi_try_exit:
-            return fapi2::current_err;
-        }
-
-        ///
         /// @brief Decodes PMIC0 Manfacture ID code 2nd byte
         /// @param[out] o_output encoding from SPD - multiple fields used
         /// @return FAPI2_RC_SUCCESS if okay
@@ -696,19 +683,6 @@ class decoder< DDR4, DDIMM_MODULE, R > : public dimm_module_decoder
         }
 
         ///
-        /// @brief Decodes VDD_Core PMIC1 -> VDD_CORE_PMIC1
-        /// @param[out] o_output encoding from SPD
-        /// @return FAPI2_RC_SUCCESS if okay
-        ///
-        virtual fapi2::ReturnCode vdd_core_pmic1(uint8_t& o_output) const override
-        {
-            FAPI_TRY( (mss::spd::reader<fields_t::VDD_CORE_PMIC1, R>(iv_target, iv_data, o_output)) );
-
-        fapi_try_exit:
-            return fapi2::current_err;
-        }
-
-        ///
         /// @brief Decodes PMIC1 Manfacture ID code 2nd byte
         /// @param[out] o_output encoding from SPD - multiple fields used
         /// @return FAPI2_RC_SUCCESS if okay
@@ -1400,6 +1374,32 @@ class decoder< DDR4, DDIMM_MODULE, R > : public dimm_module_decoder
         }
 
         ///
+        /// @brief Decodes PMIC1 Sequence -> PMIC0_SEQUENCE
+        /// @param[out] o_output encoding from SPD
+        /// @return FAPI2_RC_SUCCESS if okay
+        ///
+        virtual fapi2::ReturnCode sequence_pmic0(uint8_t& o_output) const override
+        {
+            FAPI_TRY((mss::spd::reader<fields_t::PMIC0_SEQUENCE, R>(iv_target, iv_data, o_output)));
+
+        fapi_try_exit:
+            return fapi2::current_err;
+        }
+
+        ///
+        /// @brief Decodes PMIC1 Sequence -> PMIC1_SEQUENCE
+        /// @param[out] o_output encoding from SPD
+        /// @return FAPI2_RC_SUCCESS if okay
+        ///
+        virtual fapi2::ReturnCode sequence_pmic1(uint8_t& o_output) const override
+        {
+            FAPI_TRY((mss::spd::reader<fields_t::PMIC1_SEQUENCE, R>(iv_target, iv_data, o_output)));
+
+        fapi_try_exit:
+            return fapi2::current_err;
+        }
+
+        ///
         /// @brief Decodes DRAM manufacturing ID Code
         /// @param[out] o_output encoding from SPD
         /// @return FAPI2_RC_SUCCESS if okay
diff --git a/src/import/generic/memory/lib/spd/spd_facade.H b/src/import/generic/memory/lib/spd/spd_facade.H
index 009059c09..d28138dba 100644
--- a/src/import/generic/memory/lib/spd/spd_facade.H
+++ b/src/import/generic/memory/lib/spd/spd_facade.H
@@ -2418,6 +2418,19 @@ class facade final
         }
 
         ///
+        /// @brief Decodes PMIC0 Sequence -> PMIC0_SEQUENCE
+        /// @param[out] o_output encoding from SPD
+        /// @return FAPI2_RC_SUCCESS if okay
+        ///
+        fapi2::ReturnCode sequence_pmic0(uint8_t& o_output) const
+        {
+            FAPI_TRY( iv_dimm_module_decoder->sequence_pmic0(o_output) );
+
+        fapi_try_exit:
+            return fapi2::current_err;
+        }
+
+        ///
         /// @brief Decodes PMIC1 SWA Voltage Setting -> PMIC1_SWA_VOLT_SET
         /// @param[out] o_output encoding from SPD
         /// @return FAPI2_RC_SUCCESS if okay
@@ -2741,6 +2754,19 @@ class facade final
         fapi_try_exit:
             return fapi2::current_err;
         }
+
+        ///
+        /// @brief Decodes PMIC0 Sequence -> PMIC0_SEQUENCE
+        /// @param[out] o_output encoding from SPD
+        /// @return FAPI2_RC_SUCCESS if okay
+        ///
+        fapi2::ReturnCode sequence_pmic1(uint8_t& o_output) const
+        {
+            FAPI_TRY( iv_dimm_module_decoder->sequence_pmic1(o_output) );
+
+        fapi_try_exit:
+            return fapi2::current_err;
+        }
 };
 
 ///
diff --git a/src/import/generic/memory/lib/spd/spd_fields_ddr4.H b/src/import/generic/memory/lib/spd/spd_fields_ddr4.H
index 194eba510..b60e69222 100644
--- a/src/import/generic/memory/lib/spd/spd_fields_ddr4.H
+++ b/src/import/generic/memory/lib/spd/spd_fields_ddr4.H
@@ -959,10 +959,10 @@ class fields<DDR4, DDIMM_MODULE>
             VIN_BULK_ENDURANT_START = 6,
             VIN_BULK_ENDURANT_LEN = 2,
 
-            // Byte 226: VDD_Core PMIC0
-            VDD_CORE_PMIC0_BYTE = 226,
-            VDD_CORE_PMIC0_START = 0,
-            VDD_CORE_PMIC0_LEN = 8,
+            // Byte 226: PMIC0 Sequence
+            PMIC0_SEQUENCE_BYTE = 226,
+            PMIC0_SEQUENCE_START = 0,
+            PMIC0_SEQUENCE_LEN = 8,
 
             // Byte 227: PMIC0 Manfacture ID code 1st byte
             PMIC0_MFG_CODE1_BYTE = 227,
@@ -979,10 +979,10 @@ class fields<DDR4, DDIMM_MODULE>
             PMIC0_REV_START = 0,
             PMIC0_REV_LEN = 8,
 
-            // Byte 230: VDD_Core PMIC1
-            VDD_CORE_PMIC1_BYTE = 230,
-            VDD_CORE_PMIC1_START = 0,
-            VDD_CORE_PMIC1_LEN = 8,
+            // Byte 230: PMIC1 Sequence
+            PMIC1_SEQUENCE_BYTE = 230,
+            PMIC1_SEQUENCE_START = 0,
+            PMIC1_SEQUENCE_LEN = 8,
 
             // Byte 231: PMIC1 Manfacture ID code 1st byte
             PMIC1_MFG_CODE1_BYTE = 231,
@@ -1288,8 +1288,8 @@ class fields<DDR4, DDIMM_MODULE>
         static constexpr field_t VIN_BULK_OPERABLE{VIN_BULK_BYTE, VIN_BULK_OPERABLE_START, VIN_BULK_OPERABLE_LEN};
         static constexpr field_t VIN_BULK_ENDURANT{VIN_BULK_BYTE, VIN_BULK_ENDURANT_START, VIN_BULK_ENDURANT_LEN};
 
-        // Byte 226: VDD_Core PMIC0
-        static constexpr field_t VDD_CORE_PMIC0{VDD_CORE_PMIC0_BYTE, VDD_CORE_PMIC0_START, VDD_CORE_PMIC0_LEN};
+        // Byte 226: PMIC0 Sequence
+        static constexpr field_t PMIC0_SEQUENCE{PMIC0_SEQUENCE_BYTE, PMIC0_SEQUENCE_START, PMIC0_SEQUENCE_LEN};
 
         // Byte 227: PMIC0 Manfacture ID code 1st byte
         static constexpr field_t PMIC0_CONT_CODE{PMIC0_MFG_CODE1_BYTE, PMIC0_CONT_CODE_START, PMIC0_CONT_CODE_LEN};
@@ -1300,8 +1300,8 @@ class fields<DDR4, DDIMM_MODULE>
         // Byte 229: PMIC0 Revision Number
         static constexpr field_t PMIC0_REV{PMIC0_REV_BYTE, PMIC0_REV_START, PMIC0_REV_LEN};
 
-        // Byte 230: VDD_Core PMIC1
-        static constexpr field_t VDD_CORE_PMIC1{VDD_CORE_PMIC1_BYTE, VDD_CORE_PMIC1_START, VDD_CORE_PMIC1_LEN};
+        // Byte 230: PMIC1 Sequence
+        static constexpr field_t PMIC1_SEQUENCE{PMIC1_SEQUENCE_BYTE, PMIC1_SEQUENCE_START, PMIC1_SEQUENCE_LEN};
 
         // Byte 231: PMIC1 Manfacture ID code 1st byte
         static constexpr field_t PMIC1_CONT_CODE{PMIC1_MFG_CODE1_BYTE, PMIC1_CONT_CODE_START, PMIC1_CONT_CODE_LEN};
diff --git a/src/import/generic/memory/lib/spd/spd_traits_ddr4.H b/src/import/generic/memory/lib/spd/spd_traits_ddr4.H
index eb0fde595..a26ab64e3 100644
--- a/src/import/generic/memory/lib/spd/spd_traits_ddr4.H
+++ b/src/import/generic/memory/lib/spd/spd_traits_ddr4.H
@@ -4450,16 +4450,16 @@ class readerTraits < fields< DDR4, DDIMM_MODULE>::VIN_BULK_ENDURANT, R >
 /// @class readerTraits
 /// @brief trait structure to hold static SPD information
 /// @tparam R the revision of the SPD field
-/// @note VDD_CORE_PMIC0 field specialization
+/// @note PMIC0_SEQUENCE field specialization
 /// @note valid for all revs
 ///
 template< rev R >
-class readerTraits < fields< DDR4, DDIMM_MODULE>::VDD_CORE_PMIC0, R >
+class readerTraits < fields< DDR4, DDIMM_MODULE>::PMIC0_SEQUENCE, R >
 {
     public:
 
-        static constexpr size_t COMPARISON_VAL = 0xff;
-        static constexpr const char* FIELD_STR = "VDD_Core PMIC0";
+        static constexpr size_t COMPARISON_VAL = 0x4;
+        static constexpr const char* FIELD_STR = "PMIC0 Sequence";
 
         template <typename T>
         using COMPARISON_OP = std::less_equal<T>;
@@ -4526,16 +4526,16 @@ class readerTraits < fields< DDR4, DDIMM_MODULE>::PMIC0_REV, R >
 /// @class readerTraits
 /// @brief trait structure to hold static SPD information
 /// @tparam R the revision of the SPD field
-/// @note VDD_CORE_PMIC1 field specialization
+/// @note PMIC1_SEQUENCE field specialization
 /// @note valid for all revs
 ///
 template< rev R >
-class readerTraits < fields< DDR4, DDIMM_MODULE>::VDD_CORE_PMIC1, R >
+class readerTraits < fields< DDR4, DDIMM_MODULE>::PMIC1_SEQUENCE, R >
 {
     public:
 
-        static constexpr size_t COMPARISON_VAL = 0xff;
-        static constexpr const char* FIELD_STR = "VDD_Core PMIC1";
+        static constexpr size_t COMPARISON_VAL = 0x04;
+        static constexpr const char* FIELD_STR = "PMIC1 Sequence";
 
         template <typename T>
         using COMPARISON_OP = std::less_equal<T>;
diff --git a/src/import/generic/memory/lib/utils/find.H b/src/import/generic/memory/lib/utils/find.H
index f96252913..dd5a6dbfd 100644
--- a/src/import/generic/memory/lib/utils/find.H
+++ b/src/import/generic/memory/lib/utils/find.H
@@ -39,10 +39,12 @@
 #include <fapi2.H>
 #include <vector>
 #include <generic/memory/lib/utils/c_str.H>
+#include <generic/memory/lib/utils/index.H>
 #include <generic/memory/lib/utils/shared/mss_generic_consts.H>
 
 namespace mss
 {
+
 ///
 /// @brief Helper to find a set of elements based on a fapi2 target
 /// @tparam M the target type to be returned
@@ -258,6 +260,24 @@ find_targets( const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
 }
 
 ///
+/// @brief Helper to find a set of elements based on a fapi2 target, then sort them
+/// @tparam M the target type to be returned
+/// @tparam T the fapi2 target type of the argument
+/// @param[in] i_target the fapi2 target T
+/// @param[in] i_state [optional] fapi2 target state (defaults to TARGET_STATE_FUNCTIONAL)
+/// @return a vector of M targets sorted by mss::index.
+/// @note this uses mss::index so the targets will be sorted via ATTR_REL_POS of their immediate parent
+///
+template< fapi2::TargetType M, fapi2::TargetType T >
+static inline std::vector< fapi2::Target<M> > find_targets_sorted_by_index( const fapi2::Target<T>& i_target,
+        fapi2::TargetState i_state = fapi2::TARGET_STATE_FUNCTIONAL )
+{
+    std::vector<fapi2::Target<M>> l_targets = find_targets<M, T>(i_target, i_state);
+    sort_targets_by_index(l_targets);
+    return l_targets;
+}
+
+///
 /// @brief find a key value from a vector of STL pairs
 /// @tparam T  input type
 /// @tparam OT the output type to be returned
diff --git a/src/import/generic/memory/lib/utils/index.H b/src/import/generic/memory/lib/utils/index.H
index fde43f711..1657a9e18 100644
--- a/src/import/generic/memory/lib/utils/index.H
+++ b/src/import/generic/memory/lib/utils/index.H
@@ -74,6 +74,23 @@ fapi_try_exit:
 }
 
 ///
+/// @brief Sort the provided target vector in order of index (low to high)
+///
+/// @tparam T TargetType
+/// @param[in,out] io_targets vector of targets to sort
+///
+template <fapi2::TargetType T>
+inline void sort_targets_by_index(std::vector<fapi2::Target<T>>& io_targets)
+{
+    std::sort(io_targets.begin(), io_targets.end(), [] (
+                  const fapi2::Target<T>& l_first_target,
+                  const fapi2::Target<T>& l_second_target) -> bool
+    {
+        return mss::index(l_first_target) < mss::index(l_first_target);
+    });
+}
+
+///
 /// @brief Return an attribute array index from a rank number
 /// @param[in] i_rank uint64_t a rank number DIMM0 {0, 1, 2, 3} DIMM1 {0, 1, 2, 3}
 /// @return size_t the attribute array index.