Modify freq & dep. files. Add cas latency & unit tests

Change-Id: I205bf48e54fb3c8f19f973f58f8ec1d4c7345a23
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/22632
Tested-by: PPE CI
Tested-by: Jenkins Server
Reviewed-by: Brian R. Silver <bsilver@us.ibm.com>
Tested-by: Hostboot CI
Reviewed-by: JACOB L. HARVEY <jlharvey@us.ibm.com>
Reviewed-by: STEPHEN GLANCY <sglancy@us.ibm.com>
Reviewed-by: Louis Stermole <stermole@us.ibm.com>
Reviewed-by: Jennifer A. Stofer <stofer@us.ibm.com>
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/22634
Tested-by: FSP CI Jenkins
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>

7 files changed, 398 insertions, 446 deletions

diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/eff_config/timing.H b/src/import/chips/p9/procedures/hwp/memory/lib/eff_config/timing.H
index 5ec266ddd..be4c00838 100644
--- a/src/import/chips/p9/procedures/hwp/memory/lib/eff_config/timing.H
+++ b/src/import/chips/p9/procedures/hwp/memory/lib/eff_config/timing.H
@@ -33,52 +33,11 @@
 
 #include <fapi2.H>
 
-#include <mss.H>
-#include <lib/utils/find.H>
 
-using fapi2::TARGET_TYPE_DIMM;
 
 namespace mss
 {
 
-// { density in GBs, tREFI(base) in picoseconds }
-static const std::vector<std::pair<uint8_t, uint64_t> > TREFI_BASE =
-{
-    {2, 7800},
-    {4, 7800},
-    {8, 7800},
-    // 16Gb - TBD
-};
-
-// { density in GBs, tRFC1(min) in picoseconds }
-static const std::vector<std::pair<uint8_t, uint64_t> > TRFC1_MIN =
-{
-    {2, 160},
-    {4, 260},
-    {8, 350},
-    // 16Gb - TBD
-};
-
-
-// { density in GBs, tRFC2(min) in picoseconds }
-static const std::vector<std::pair<uint8_t, uint64_t> > TRFC2_MIN =
-{
-    {2, 110},
-    {4, 160},
-    {8, 260},
-    // 16Gb - TBD
-};
-
-
-// { density in GBs, tRFC4(min) in picoseconds }
-static const std::vector<std::pair<uint8_t, uint64_t> > TRFC4_MIN =
-{
-    {2, 90},
-    {4, 110},
-    {8, 160},
-    // 16Gb - TBD
-};
-
 enum GUARD_BAND
 {
     // Used for caclulating spd timing values - from JEDEC rounding algorithm
@@ -90,7 +49,6 @@ enum GUARD_BAND
     INVERSE_DDR4_CORRECTION_FACTOR = 974,
 };
 
-
 ///
 /// @brief      Calculates timing value
 /// @tparam     T input and output type
@@ -110,7 +68,6 @@ inline T calc_timing_from_timebase(const T i_timing_mtb,
     T l_timing_val = i_timing_mtb * i_mtb_multiplier;
     T l_fine_offset = i_timing_ftb * i_ftb_multiplier;
 
-    // Needed to do integer math but attribute is uint64
     return l_timing_val + l_fine_offset;
 }
 
@@ -137,158 +94,30 @@ inline T calc_nck(T timing_in_ps, T tck_in_ps, T inverse_corr_factor)
     return temp_nck / 1000;
 }
 
-
-//
-// TK - AAM
-// These are only used in eff_config, make them non-member non-friend functions in eff_config.C?
-//
-
 /// @brief Calculates refresh interval time 1 (tREFI 1)
 /// @param[in] i_target FAPI2 target
 /// @param[out] o_value timing val in ps
 /// @return fapi2::ReturnCode
 ///
-inline fapi2::ReturnCode calc_trefi1(const fapi2::Target<TARGET_TYPE_DIMM>& i_target,  uint64_t& o_value)
-{
-    uint8_t  l_quotient = 0;
-    uint8_t l_remainder = 0;
-    uint64_t l_output = 0;
-    uint8_t l_temp_ref_range = 0;
-    uint8_t l_dram_density = 0;
-
-
-    FAPI_TRY(mss::eff_dram_density(i_target, l_dram_density));
-
-    switch(l_temp_ref_range)
-    {
-        case fapi2::ENUM_ATTR_EFF_TEMP_REF_RANGE_NORMAL:
-            mss::find_value_from_key(TREFI_BASE, l_dram_density, o_value);
-            break;
-
-        case fapi2::ENUM_ATTR_EFF_TEMP_REF_RANGE_EXTEND:
-            mss::find_value_from_key(TREFI_BASE, l_dram_density, l_output);
-            l_quotient = l_output / 2;
-            l_remainder = l_output % 2;
-            o_value = l_quotient + (l_remainder == 0 ? 0 : 1);
-            break;
-
-        default:
-            // l_temp_ref_range will be a platform attribute set by the MRW,
-            // which they "shouldn't" mess up as long as use "attribute" enums.
-            // if openpower messes this up we can at least catch it
-            FAPI_ASSERT(false,
-                        fapi2::MSS_INVALID_TEMP_REF_RANGE().
-                        set_TEMP_REF_RANGE(l_temp_ref_range),
-                        "%s Incorrect Temperature Ref. Range received: %d ",
-                        mss::c_str(i_target),
-                        l_temp_ref_range);
-
-            break;
-    }
-
-fapi_try_exit:
-    return fapi2::current_err;
-}
+fapi2::ReturnCode calc_trefi1(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
+                              uint64_t& o_value);
 
 /// @brief Calculates refresh interval time 2 (tREFI 2)
 /// @param[in] i_target FAPI2 target
 /// @param[out] o_value timing val in ps
 /// @return fapi2::ReturnCode
 ///
-inline fapi2::ReturnCode calc_trefi2(const fapi2::Target<TARGET_TYPE_DIMM>& i_target, uint64_t& o_value)
-{
-    uint8_t l_quotient = 0;
-    uint8_t l_remainder = 0;
-    uint64_t l_output = 0;
-    uint8_t l_temp_ref_range = 0;
-    uint8_t l_dram_density = 0;
-
-    FAPI_TRY(mss::eff_temp_ref_range(i_target, l_temp_ref_range));
-    FAPI_TRY(mss::eff_dram_density(i_target, l_dram_density));
-
-    switch(l_temp_ref_range)
-    {
-        case fapi2::ENUM_ATTR_EFF_TEMP_REF_RANGE_NORMAL:
-            mss::find_value_from_key(TREFI_BASE, l_dram_density, l_output);
-            l_quotient = l_output / 2;
-            l_remainder = l_output % 2;
-            o_value = l_quotient + (l_remainder == 0 ? 0 : 1);
-            break;
-
-        case fapi2::ENUM_ATTR_EFF_TEMP_REF_RANGE_EXTEND:
-            mss::find_value_from_key(TREFI_BASE, l_dram_density, l_output);
-            l_quotient = l_output / 4;
-            l_remainder = l_output % 4;
-            o_value = l_quotient + (l_remainder == 0 ? 0 : 1);
-            break;
-
-        default:
-            // l_temp_ref_range will be a platform attribute set by the MRW,
-            // which they "shouldn't" mess up as long as use "attribute" enums.
-            // if openpower messes this up we can at least catch it
-            FAPI_ASSERT(false,
-                        fapi2::MSS_INVALID_TEMP_REF_RANGE().
-                        set_TEMP_REF_RANGE(l_temp_ref_range),
-                        "%s Incorrect Temperature Ref. Range received: %d ",
-                        mss::c_str(i_target),
-                        l_temp_ref_range);
-
-            break;
-    }
-
-fapi_try_exit:
-    return fapi2::current_err;
-}
+fapi2::ReturnCode calc_trefi2(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
+                              uint64_t& o_value);
 
 /// @brief Calculates refresh interval time 4 (tREFI 4)
 /// @param[in] i_target FAPI2 target
 /// @param[out] o_value timing val in ps
 /// @return fapi2::ReturnCode
 ///
-inline fapi2::ReturnCode calc_trefi4( const fapi2::Target<TARGET_TYPE_DIMM>& i_target, uint64_t& o_value)
-{
-    uint8_t l_quotient = 0;
-    uint8_t l_remainder = 0;
-    uint64_t l_output = 0;
-    uint8_t l_temp_ref_range = 0;
-    uint8_t l_dram_density = 0;
-
-    FAPI_TRY(mss::eff_temp_ref_range(i_target, l_temp_ref_range));
-    FAPI_TRY(mss::eff_dram_density(i_target, l_dram_density));
+fapi2::ReturnCode calc_trefi4( const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
+                               uint64_t& o_value);
 
-    switch(l_temp_ref_range)
-    {
-        case fapi2::ENUM_ATTR_EFF_TEMP_REF_RANGE_NORMAL:
-            mss::find_value_from_key(TREFI_BASE, l_dram_density, l_output);
-            l_quotient = l_output / 4;
-            l_remainder = l_output % 4;
-            o_value = l_quotient + (l_remainder == 0 ? 0 : 1);
-            break;
-
-        case fapi2::ENUM_ATTR_EFF_TEMP_REF_RANGE_EXTEND:
-            mss::find_value_from_key(TREFI_BASE, l_dram_density, l_output);
-            l_quotient = l_output / 8;
-            l_remainder = l_output % 8;
-            o_value = l_quotient + (l_remainder == 0 ? 0 : 1);
-            break;
-
-        default:
-            // l_temp_ref_range will be a platform attribute set by the MRW,
-            // which they "shouldn't" mess up as long as use "attribute" enums.
-            // if openpower messes this up we can at least catch it
-            FAPI_ASSERT(false,
-                        fapi2::MSS_INVALID_TEMP_REF_RANGE().
-                        set_TEMP_REF_RANGE(l_temp_ref_range),
-                        "%s Incorrect Temperature Ref. Range received: %d ",
-                        mss::c_str(i_target),
-                        l_temp_ref_range);
-
-            break;
-    }
-
-fapi_try_exit:
-    return fapi2::current_err;
-}
 
 } // mss
 
diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/spd/spd_decoder.C b/src/import/chips/p9/procedures/hwp/memory/lib/spd/spd_decoder.C
index a7eef6466..666b4c84c 100644
--- a/src/import/chips/p9/procedures/hwp/memory/lib/spd/spd_decoder.C
+++ b/src/import/chips/p9/procedures/hwp/memory/lib/spd/spd_decoder.C
@@ -544,14 +544,15 @@ fapi2::ReturnCode base_module_type(const fapi2::Target<TARGET_TYPE_DIMM>& i_targ
     // Extracting desired bits
     l_spd_buffer.extractToRight<BASE_MODULE_START, BASE_MODULE_LEN>(l_field_bits);
 
-    FAPI_DBG("Field_Bits value: %d", l_field_bits);
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
 
-    FAPI_TRY( mss::check::spd::
-              fail_for_invalid_value(i_target,
-                                     mss::find_value_from_key(BASE_MODULE_TYPE_MAP, l_field_bits, o_value),
-                                     BYTE_INDEX,
-                                     l_field_bits,
-                                     "Failed check on Base Module Type") );
+    bool found_key = find_value_from_key(BASE_MODULE_TYPE_MAP, l_field_bits, o_value);
+
+    FAPI_TRY( check::spd::fail_for_invalid_value(i_target,
+              found_key,
+              BYTE_INDEX,
+              l_field_bits,
+              "Failed check on Base Module Type") );
 
     FAPI_DBG("%s. Base Module Type: %d",
              mss::c_str(i_target),
@@ -577,9 +578,12 @@ fapi2::ReturnCode factory(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target
     uint8_t l_additions_rev = 0;
 
     // Get dimm type & revision levels
-    FAPI_TRY(base_module_type(i_target, i_spd_data, l_dimm_type));
-    FAPI_TRY(rev_encoding_level(i_target, i_spd_data, l_encoding_rev));
-    FAPI_TRY(rev_additions_level(i_target, i_spd_data, l_additions_rev));
+    FAPI_TRY( base_module_type(i_target, i_spd_data, l_dimm_type),
+              "Failed to find base module type" );
+    FAPI_TRY( rev_encoding_level(i_target, i_spd_data, l_encoding_rev),
+              "Failed to find encoding level" );
+    FAPI_TRY( rev_additions_level(i_target, i_spd_data, l_additions_rev),
+              "Failed to find additons level" );
 
     // Get decoder object needed for current dimm type and spd rev
     switch(l_dimm_type)
@@ -597,40 +601,38 @@ fapi2::ReturnCode factory(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target
                     {
                         case 0:
                         case 1:
+                            // Rev 1.0 or Rev 1.1
                             o_fact_obj = std::make_shared<decoder>();
                             break;
 
                         default:
-                            FAPI_TRY( mss::check::spd::
-                                      invalid_factory_sel(i_target,
-                                                          l_dimm_type,
-                                                          l_encoding_rev,
-                                                          l_additions_rev,
-                                                          "Additions Level Unsupported!") );
+                            FAPI_TRY( check::spd::invalid_factory_sel(i_target,
+                                      l_dimm_type,
+                                      l_encoding_rev,
+                                      l_additions_rev,
+                                      "Additions Level Unsupported!") );
                             break;
                     }//end additions
 
                     break;
 
                 default:
-                    FAPI_TRY( mss::check::spd::
-                              invalid_factory_sel(i_target,
-                                                  l_dimm_type,
-                                                  l_encoding_rev,
-                                                  l_additions_rev,
-                                                  "Encoding Level Unsupported!") );
+                    FAPI_TRY( check::spd::invalid_factory_sel(i_target,
+                              l_dimm_type,
+                              l_encoding_rev,
+                              l_additions_rev,
+                              "Encoding Level Unsupported!") );
                     break;
             }// end encodings
 
             break;
 
         default:
-            FAPI_TRY( mss::check::spd::
-                      invalid_factory_sel(i_target,
-                                          l_dimm_type,
-                                          l_encoding_rev,
-                                          l_additions_rev,
-                                          "DIMM Type Unsupported!") );
+            FAPI_TRY( check::spd::invalid_factory_sel(i_target,
+                      l_dimm_type,
+                      l_encoding_rev,
+                      l_additions_rev,
+                      "DIMM Type Unsupported!") );
             break;
 
     } // end dimm type
@@ -645,21 +647,25 @@ fapi_try_exit:
 /// @brief       Creates factory object & SPD data caches
 /// @param[in]   i_target controller target
 /// @param[out]  o_factory_caches map of factory objects with a dimm pos. key
-/// @return      fapi2::ReturnCode
+/// @return      FAPI2_RC_SUCCESS if okay
+/// @note        This specialization is suited for creating a cache with platform
+///              SPD data.
 ///
+template<>
 fapi2::ReturnCode populate_decoder_caches( const fapi2::Target<TARGET_TYPE_MCS>& i_target,
-        std::map<uint32_t, std::shared_ptr<decoder> >& o_factory_caches)
+        std::map<uint32_t, std::shared_ptr<decoder> >& o_factory_caches,
+        const std::shared_ptr<decoder>& i_pDecoder)
 {
-    size_t l_spd_size = 0;
-    std::shared_ptr<decoder> l_pDecoder;
+    std::shared_ptr<decoder> l_pDecoder(i_pDecoder);
 
     for( const auto& l_mca : i_target.getChildren<TARGET_TYPE_MCA>() )
     {
         for( const auto& l_dimm : l_mca.getChildren<TARGET_TYPE_DIMM>() )
         {
+            size_t l_spd_size = 0;
+
             // Retrieve SPD size
             FAPI_TRY( getSPD(l_dimm, nullptr, l_spd_size) );
-
             {
                 // Retrieve SPD data
                 uint8_t* l_spd_data = new uint8_t[l_spd_size];
@@ -671,18 +677,43 @@ fapi2::ReturnCode populate_decoder_caches( const fapi2::Target<TARGET_TYPE_MCS>&
                 // Destructor for shared_ptr calls delete, has undefined behavior
                 // So we use a default destruction policy for array types that uses delete[]
                 // If F/W doesn't support this we can include a custom delete in lieu of default_delete
-                l_pDecoder->iv_spd_data = std::shared_ptr<uint8_t>(l_spd_data, std::default_delete<uint8_t[]>());
-
+                l_pDecoder->iv_spd_data = std::shared_ptr<uint8_t>( l_spd_data,
+                                          std::default_delete<uint8_t[]>() );
                 // Populate spd caches maps based on dimm pos
                 o_factory_caches.emplace(std::make_pair(mss::pos(l_dimm), l_pDecoder));
             }
-        }
-    }
+        }// end dimm
+    }// end mca
+
 
 fapi_try_exit:
     return fapi2::current_err;
 }
 
+
+///
+/// @brief       Creates factory object & SPD data caches
+/// @param[in]   i_target the dimm target
+/// @param[out]  o_factory_caches map of factory objects with a dimm pos. key
+/// @return      FAPI2_RC_SUCCESS if okay
+/// @note        This specialization is suited for creating a cache with custom
+///              SPD data.
+///
+template<>
+fapi2::ReturnCode populate_decoder_caches( const fapi2::Target<TARGET_TYPE_DIMM>& i_target,
+        std::map<uint32_t, std::shared_ptr<decoder> >& o_factory_caches,
+        const std::shared_ptr<decoder>& i_pDecoder)
+{
+    std::shared_ptr<decoder> l_pDecoder(i_pDecoder);
+
+    // Custom decoder provided (usually done for testing)
+    // Populate custom spd caches maps based internal dimm pos
+    o_factory_caches.emplace(std::make_pair( mss::pos(i_target), l_pDecoder) );
+
+    // TK - else what do we want here
+    return fapi2::FAPI2_RC_SUCCESS;
+}
+
 /////////////////////////
 // Member Method implementation
 /////////////////////////
@@ -957,6 +988,8 @@ fapi2::ReturnCode decoder::sdram_density(const fapi2::Target<TARGET_TYPE_DIMM>&
     // Extracting desired bits
     l_spd_buffer.extractToRight<SDRAM_CAPACITY_START, SDRAM_CAPACITY_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     // Check to assure SPD DRAM capacity (map) wont be at invalid values
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
@@ -1006,6 +1039,8 @@ fapi2::ReturnCode decoder::banks(const fapi2::Target<TARGET_TYPE_DIMM>& i_target
     // Extracting desired bits
     l_spd_buffer.extractToRight<SDRAM_BANKS_START, SDRAM_BANKS_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     // Check to assure SPD DRAM capacity (map) wont be at invalid values
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
@@ -1054,6 +1089,8 @@ fapi2::ReturnCode decoder::bank_groups(const fapi2::Target<TARGET_TYPE_DIMM>& i_
     // Extracting desired bits
     l_spd_buffer.extractToRight<BANK_GROUP_START, BANK_GROUP_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     // Check to assure SPD DRAM capacity (map) wont be at invalid values
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
@@ -1102,6 +1139,8 @@ fapi2::ReturnCode decoder::column_address_bits(const fapi2::Target<TARGET_TYPE_D
     // Extracting desired bits
     l_spd_buffer.extractToRight<COL_ADDRESS_START, COL_ADDRESS_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     // Check to assure SPD DRAM capacity (map) wont be at invalid values
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
@@ -1150,6 +1189,8 @@ fapi2::ReturnCode decoder::row_address_bits(const fapi2::Target<TARGET_TYPE_DIMM
     // Extracting desired bits
     l_spd_buffer.extractToRight<ROW_ADDRESS_START, ROW_ADDRESS_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     // Check to assure SPD DRAM capacity (map) wont be at invalid values
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
@@ -1183,7 +1224,7 @@ fapi2::ReturnCode decoder::prim_sdram_signal_loading(const fapi2::Target<TARGET_
         uint8_t& o_value)
 {
 
-    constexpr size_t BYTE_INDEX = 5;
+    constexpr size_t BYTE_INDEX = 6;
     uint8_t l_raw_byte  = i_spd_data[BYTE_INDEX];
     uint8_t l_field_bits = 0;
 
@@ -1199,6 +1240,8 @@ fapi2::ReturnCode decoder::prim_sdram_signal_loading(const fapi2::Target<TARGET_
     // Extracting desired bits
     l_spd_buffer.extractToRight<PRIM_SIGNAL_LOAD_START, PRIM_SIGNAL_LOAD_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      mss::find_value_from_key(PRIM_SIGNAL_LOADING_MAP, l_field_bits, o_value),
@@ -1230,7 +1273,7 @@ fapi2::ReturnCode decoder::prim_sdram_die_count(const fapi2::Target<TARGET_TYPE_
         uint8_t& o_value)
 {
 
-    constexpr size_t BYTE_INDEX = 5;
+    constexpr size_t BYTE_INDEX = 6;
     uint8_t l_raw_byte  = i_spd_data[BYTE_INDEX];
     uint8_t l_field_bits = 0;
 
@@ -1246,6 +1289,8 @@ fapi2::ReturnCode decoder::prim_sdram_die_count(const fapi2::Target<TARGET_TYPE_
     // Extracting desired bits
     l_spd_buffer.extractToRight<PRIM_DIE_COUNT_START, PRIM_DIE_COUNT_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      mss::find_value_from_key(PRIM_DIE_COUNT_MAP, l_field_bits, o_value),
@@ -1277,7 +1322,7 @@ fapi2::ReturnCode decoder::prim_sdram_package_type(const fapi2::Target<TARGET_TY
         uint8_t& o_value)
 {
 
-    constexpr size_t BYTE_INDEX = 5;
+    constexpr size_t BYTE_INDEX = 6;
     uint8_t l_raw_byte  = i_spd_data[BYTE_INDEX];
     uint8_t l_field_bits = 0;
 
@@ -1293,6 +1338,8 @@ fapi2::ReturnCode decoder::prim_sdram_package_type(const fapi2::Target<TARGET_TY
     // Extracting desired bits
     l_spd_buffer.extractToRight<PRIM_PACKAGE_TYPE_START, PRIM_PACKAGE_TYPE_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      mss::find_value_from_key(PRIM_PACKAGE_TYPE_MAP, l_field_bits, o_value),
@@ -1341,6 +1388,8 @@ fapi2::ReturnCode decoder::maximum_activate_count(const fapi2::Target<TARGET_TYP
     // Extracting desired bits
     l_spd_buffer.extractToRight<MAC_START, MAC_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      mss::find_value_from_key(MAC_MAP, l_field_bits, o_value),
@@ -1388,6 +1437,8 @@ fapi2::ReturnCode decoder::maximum_activate_window_multiplier(const fapi2::Targe
     // Extracting desired bits
     l_spd_buffer.extractToRight<TMAW_START, TMAW_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      mss::find_value_from_key(TMAW_MAP, l_field_bits, o_value),
@@ -1435,6 +1486,8 @@ fapi2::ReturnCode decoder::soft_post_package_repair(const fapi2::Target<TARGET_T
     // Extracting desired bits
     l_spd_buffer.extractToRight<SOFT_PPR_START, SOFT_PPR_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      mss::find_value_from_key(SOFT_PPR_MAP, l_field_bits, o_value),
@@ -1482,6 +1535,8 @@ fapi2::ReturnCode decoder::post_package_repair(const fapi2::Target<TARGET_TYPE_D
     // Extracting desired bits
     l_spd_buffer.extractToRight<PPR_START, PPR_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      mss::find_value_from_key(PPR_MAP, l_field_bits, o_value),
@@ -1530,6 +1585,8 @@ fapi2::ReturnCode decoder::sec_sdram_signal_loading(const fapi2::Target<TARGET_T
     // Extracting desired bits
     l_spd_buffer.extractToRight<SEC_SIGNAL_LOAD_START, SEC_SIGNAL_LOAD_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      mss::find_value_from_key(SEC_SIGNAL_LOADING_MAP, l_field_bits, o_value),
@@ -1579,6 +1636,8 @@ fapi2::ReturnCode decoder::sec_dram_density_ratio(const fapi2::Target<TARGET_TYP
     // Extracting desired bits
     l_spd_buffer.extractToRight<DENSITY_RATIO_START, DENSITY_RATIO_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      l_field_bits != UNDEFINED,
@@ -1627,6 +1686,8 @@ fapi2::ReturnCode decoder::sec_sdram_die_count(const fapi2::Target<TARGET_TYPE_D
     // Extracting desired bits
     l_spd_buffer.extractToRight<SEC_DIE_COUNT_START, SEC_DIE_COUNT_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      mss::find_value_from_key(SEC_DIE_COUNT_MAP, l_field_bits, o_value),
@@ -1674,6 +1735,8 @@ fapi2::ReturnCode decoder::sec_sdram_package_type(const fapi2::Target<TARGET_TYP
     // Extracting desired bits
     l_spd_buffer.extractToRight<SEC_PACKAGE_TYPE_START, SEC_PACKAGE_TYPE_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      mss::find_value_from_key(SEC_PACKAGE_TYPE_MAP, l_field_bits, o_value),
@@ -1722,6 +1785,8 @@ fapi2::ReturnCode decoder::operable_nominal_voltage(const fapi2::Target<TARGET_T
     // Extracting desired bits
     l_spd_buffer.extractToRight<OPERABLE_START, OPERABLE_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      mss::find_value_from_key(OPERABLE_MAP, l_field_bits, o_value),
@@ -1769,6 +1834,8 @@ fapi2::ReturnCode decoder::endurant_nominal_voltage(const fapi2::Target<TARGET_T
     // Extracting desired bits
     l_spd_buffer.extractToRight<ENDURANT_START, ENDURANT_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      mss::find_value_from_key(ENDURANT_MAP, l_field_bits, o_value),
@@ -1816,6 +1883,8 @@ fapi2::ReturnCode decoder::device_width(const fapi2::Target<TARGET_TYPE_DIMM>& i
     // Extracting desired bits
     l_spd_buffer.extractToRight<SDRAM_WIDTH_START, SDRAM_WIDTH_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      mss::find_value_from_key(DEVICE_WIDTH_MAP, l_field_bits, o_value),
@@ -1864,6 +1933,8 @@ fapi2::ReturnCode decoder::num_package_ranks_per_dimm(const fapi2::Target<TARGET
     // Extracting desired bits
     l_spd_buffer.extractToRight<PACKAGE_RANKS_START, PACKAGE_RANKS_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      mss::find_value_from_key(NUM_PACKAGE_RANKS_MAP, l_field_bits, o_value),
@@ -1912,6 +1983,8 @@ fapi2::ReturnCode decoder::rank_mix(const fapi2::Target<TARGET_TYPE_DIMM>& i_tar
     // Extracting desired bits
     l_spd_buffer.extractToRight<RANK_MIX_START, RANK_MIX_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      (l_field_bits < INVALID_VALUE),
@@ -1961,6 +2034,8 @@ fapi2::ReturnCode decoder::prim_bus_width(const fapi2::Target<TARGET_TYPE_DIMM>&
     // Extracting desired bits
     l_spd_buffer.extractToRight<BUS_WIDTH_START, BUS_WIDTH_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      mss::find_value_from_key(BUS_WIDTH_MAP, l_field_bits, o_value),
@@ -2007,6 +2082,8 @@ fapi2::ReturnCode decoder::bus_width_extension(const fapi2::Target<TARGET_TYPE_D
     // Extracting desired bits
     l_spd_buffer.extractToRight<BUS_EXT_WIDTH_START, BUS_EXT_WIDTH_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      mss::find_value_from_key(BUS_WIDTH_EXT_MAP, l_field_bits, o_value),
@@ -2056,6 +2133,8 @@ fapi2::ReturnCode decoder::thermal_sensor(const fapi2::Target<TARGET_TYPE_DIMM>&
     // Extracting desired bits
     l_spd_buffer.extractToRight<THERM_SENSOR_START, THERM_SENSOR_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      l_field_bits < INVALID_VALUE,
@@ -2105,6 +2184,8 @@ fapi2::ReturnCode decoder::extended_base_module_type(const fapi2::Target<TARGET_
     // Extracting desired bits
     l_spd_buffer.extractToRight<EXT_MOD_TYPE_START, EXT_MOD_TYPE_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     // Currently reserved to 0b000
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
@@ -2155,6 +2236,8 @@ fapi2::ReturnCode decoder::fine_timebase(const fapi2::Target<TARGET_TYPE_DIMM>&
     // Extracting desired bits
     l_spd_buffer.extractToRight<FINE_TIMEBASE_START, FINE_TIMEBASE_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      mss::find_value_from_key(FINE_TIMEBASE_MAP, l_field_bits, o_value),
@@ -2202,6 +2285,8 @@ fapi2::ReturnCode decoder::medium_timebase(const fapi2::Target<TARGET_TYPE_DIMM>
     // Extracting desired bits
     l_spd_buffer.extractToRight<MED_TIMEBASE_START, MED_TIMEBASE_LEN>(l_field_bits);
 
+    FAPI_DBG("Field Bits value: %d", l_field_bits);
+
     FAPI_TRY( mss::check::spd::
               fail_for_invalid_value(i_target,
                                      mss::find_value_from_key(MEDIUM_TIMEBASE_MAP, l_field_bits, o_value),
@@ -2259,6 +2344,9 @@ fapi2::ReturnCode decoder::min_cycle_time(const fapi2::Target<fapi2::TARGET_TYPE
                                     l_timing_val,
                                     "Failed check on the min cycle time (tckmin) in MTB") );
 
+    // Update output after check passes
+    o_value = l_timing_val;
+
     FAPI_DBG("%s. Minimum Cycle Time (tCKmin) in MTB units: %d",
              mss::c_str(i_target),
              o_value);
@@ -2308,6 +2396,9 @@ fapi2::ReturnCode decoder::max_cycle_time(const fapi2::Target<fapi2::TARGET_TYPE
                                     l_timing_val,
                                     "Failed check on the max cycle time (tckmax) in MTB") );
 
+    // Update output after check passes
+    o_value = l_timing_val;
+
     FAPI_DBG("%s. Maximum Cycle Time (tCKmax) in MTB units: %d",
              mss::c_str(i_target),
              o_value);
diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/spd/spd_decoder.H b/src/import/chips/p9/procedures/hwp/memory/lib/spd/spd_decoder.H
index d03de319e..bbc47c784 100644
--- a/src/import/chips/p9/procedures/hwp/memory/lib/spd/spd_decoder.H
+++ b/src/import/chips/p9/procedures/hwp/memory/lib/spd/spd_decoder.H
@@ -94,7 +94,7 @@ enum BYTE_EXTRACT : uint64_t
     ADDRESS_RESERVED_LEN = 2,
 
     // Byte 6
-    PRIM_SIGNAL_LOAD_START = 5,
+    PRIM_SIGNAL_LOAD_START = 6,
     PRIM_SIGNAL_LOAD_LEN = 2,
 
     PACKAGE_RESERVE_START = 4,
@@ -1292,12 +1292,14 @@ fapi2::ReturnCode factory(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target
 
 ///
 /// @brief       Creates factory object & SPD data caches
-/// @param[in]   i_target controller target
+/// @param[in]   i_target the fapi2 target
 /// @param[out]  o_factory_caches map of factory objects with a dimm pos key
-/// @return      fapi2::ReturnCode
+/// @return      FAPI2_RC_SUCCESS if okay
 ///
-fapi2::ReturnCode populate_decoder_caches(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
-        std::map<uint32_t, std::shared_ptr<decoder> >& o_factory_caches);
+template<fapi2::TargetType T>
+fapi2::ReturnCode populate_decoder_caches(const fapi2::Target<T>& i_target,
+        std::map<uint32_t, std::shared_ptr<decoder> >& o_factory_caches,
+        const std::shared_ptr<decoder>& i_pDecoder = nullptr);
 
 
 }// spd
diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/utils/checker.H b/src/import/chips/p9/procedures/hwp/memory/lib/utils/checker.H
index 6656ef6f1..028c272d3 100644
--- a/src/import/chips/p9/procedures/hwp/memory/lib/utils/checker.H
+++ b/src/import/chips/p9/procedures/hwp/memory/lib/utils/checker.H
@@ -31,7 +31,6 @@
 #define _CHECKER_H_
 
 #include <fapi2.H>
-
 #include <mss_attribute_accessors.H>
 #include <lib/shared/mss_const.H>
 
@@ -39,195 +38,121 @@ namespace mss
 {
 namespace check
 {
+
 ///
 /// @brief      Checks homogenous DDR4 dimm configuration (e.g. DDR4)
-/// @param[in]  i_target_mcs  const fapi2::Target<fapi2::TARGET_TYPE_MCS>&
+/// @param[in]  i_target the controller target
 /// @return     FAPI2_RC_SUCCESS iff ok
 ///
-inline fapi2::ReturnCode dram_type(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target_mcs)
+inline fapi2::ReturnCode dram_type(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target)
 {
     uint8_t l_dram_gen[PORTS_PER_MCS][MAX_DIMM_PER_PORT] = {0};
 
     // Retrieve DDR4 dimm attributes
-    FAPI_TRY(eff_dram_gen(i_target_mcs, &(l_dram_gen[0][0])));
+    FAPI_TRY(eff_dram_gen(i_target, &(l_dram_gen[0][0])));
 
     // Make sure all DRAMs are DDR4 or throw an error
-    // Iterate through MCA's/ports
-    for (auto p : i_target_mcs.getChildren<fapi2::TARGET_TYPE_MCA>())
+    for (const auto& l_mca : i_target.getChildren<fapi2::TARGET_TYPE_MCA>())
     {
-        auto port_num = mss::index(p);
+        const auto l_port_num = mss::index(l_mca);
 
-        // Iterate through DIMM targets
-        for (auto d : p.getChildren<fapi2::TARGET_TYPE_DIMM>())
+        for (const auto& l_dimm : l_mca.getChildren<fapi2::TARGET_TYPE_DIMM>())
         {
-            auto dimm_num = mss::index(d);
+            const auto l_dimm_num = mss::index(l_dimm);
 
             FAPI_INF("%s DRAM device type is %llX",
-                     mss::c_str(d),
-                     l_dram_gen[port_num][dimm_num]);
+                     mss::c_str(l_dimm),
+                     l_dram_gen[l_port_num][l_dimm_num]);
 
             // Nimbus supports only DDR4
-            FAPI_ASSERT(l_dram_gen[port_num][dimm_num] == fapi2::ENUM_ATTR_EFF_DRAM_GEN_DDR4,
+            FAPI_ASSERT(l_dram_gen[l_port_num][l_dimm_num] == fapi2::ENUM_ATTR_EFF_DRAM_GEN_DDR4,
                         fapi2::MSS_UNSUPPORTED_DEV_TYPE().
-                        set_DEV_TYPE(l_dram_gen[port_num][dimm_num]),
+                        set_DEV_TYPE(l_dram_gen[l_port_num][l_dimm_num]),
                         "%s Incorrect DRAM device generation, DRAM generation is %llx",
-                        mss::c_str(d),
-                        l_dram_gen[port_num][dimm_num]);
-        }
-
-    }
+                        mss::c_str(l_dimm),
+                        l_dram_gen[l_port_num][l_dimm_num]);
+        }// dimm
+    }// mca
 
 fapi_try_exit:
     return fapi2::current_err;
-}// dram_type
-
-
-///
-/// @brief Check if there is DIMM mixing (and deconfigures unsupported mixing ?? - AAM)
-/// @return FAPI2_RC_SUCCESS iff ok
-/// @note Functionality currently unknown
-///
-inline fapi2::ReturnCode dimm_mixing(void)
-{
-    //FIX p8 ported  code
-    //Need to complete and figure out how to implement for p9 - AAM
-
-    // Anuwat pushed Warren to say IBM systems will not support DIMM mixing
-    // Now DIMM deconfig rules? - AAM
-#if 0
-
-    std::vector<uint8_t> l_module_type_vector;
-    uint8_t l_module_type[mss::PORTS_PER_MCS][mss::MAX_DIMM_PER_PORT] = {0};
-    uint8_t lr_dimm_count;
-    uint8_t r_dimm_count;
-    uint8_t u_dimm_count;
-
-    FAPI_TRY(mss::spd_module_type(i_target_mcs, &l_module_type[0][0]),
-             "Unable to read the SPD module type.");
-
-    for(size_t l_port = 0; l_port < mss::PORTS_PER_MCS; l_port++)
-    {
-        for (size_t l_dimm = 0; l_dimm < mss::MAX_DIMM_PER_PORT; l_dimm++)
-        {
-            l_module_type_vector.push_back(l_module_type[l_port][l_dimm]);
-        }
-    }
-
-    lr_dimm_count = std::count(l_module_type_vector.begin(),
-                               l_module_type_vector.end(),
-                               fapi2::ENUM_ATTR_SPD_MODULE_TYPE_LRDIMM);
-    r_dimm_count = std::count(l_module_type_vector.begin(),
-                              l_module_type_vector.end(),
-                              fapi2::ENUM_ATTR_SPD_MODULE_TYPE_RDIMM);
-    u_dimm_count = std::count(l_module_type_vector.begin(),
-                              l_module_type_vector.end(),
-                              fapi2::ENUM_ATTR_SPD_MODULE_TYPE_UDIMM); // Not supported? - AAM
-
-    count(l_module_type_vector.begin(), l_module_type_vector.end(), fapi2::SO_DIMM); // Not supported? - AAM
 
-    uint8_t temp = std::max({lr_dimm_count, r_dimm_count, u_dimm_count});
-
-    return (temp == r_dimm_count) ? fapi2::RDIMM : fapi2::LRDIMM
-
-#endif
-
-           return fapi2::FAPI2_RC_SUCCESS;
-}// dimm_mixing
-
-///
-/// @brief      Checks what type of system is configured (i.e., single drop, dual drop)
-/// @return     FAPI2_RC_SUCCESS iff ok
-/// @warning    Functionality currently unknown. Used in mss_freq
-///
-inline fapi2::ReturnCode system_drop_type(void)
-{
-
-    //Update for P9, how will we check if a system is single/dual drop?? - AAM
-
-#if 0
-    //Determining the cnfg for imposing any cnfg speed limitations
-    if ( (cur_dimm_spd_valid_u8array[0][0][0] == MSS_FREQ_VALID)
-         && (cur_dimm_spd_valid_u8array[0][0][1] == MSS_FREQ_EMPTY) )
-    {
-        plug_config = MSS_FREQ_SINGLE_DROP;
-        num_ranks_total = num_ranks[0][0][0] + 1;
-    }
-    else if (((cur_dimm_spd_valid_u8array[1][0][0] == MSS_FREQ_VALID)
-              && (cur_dimm_spd_valid_u8array[1][0][1] == MSS_FREQ_EMPTY)) ||
-             ((cur_dimm_spd_valid_u8array[1][0][1] == MSS_FREQ_VALID) && (cur_dimm_spd_valid_u8array[1][0][0] == MSS_FREQ_EMPTY)))
-    {
-        plug_config = MSS_FREQ_SINGLE_DROP;
-        num_ranks_total = num_ranks[1][0][0] + 1;
-    }
-    else if ((cur_dimm_spd_valid_u8array[0][0][0] == MSS_FREQ_VALID)
-             && (cur_dimm_spd_valid_u8array[0][0][1] == MSS_FREQ_VALID))
-    {
-        plug_config = MSS_FREQ_DUAL_DROP;
-        num_ranks_total = (num_ranks[0][0][0] + 1) + (num_ranks[0][0][1] + 1);
-    }
-    else if ((cur_dimm_spd_valid_u8array[1][0][0] == MSS_FREQ_VALID)
-             && (cur_dimm_spd_valid_u8array[1][0][1] == MSS_FREQ_VALID))
-    {
-        plug_config = MSS_FREQ_DUAL_DROP;
-        num_ranks_total = (num_ranks[1][0][0] + 1) + (num_ranks[1][0][1] + 1);
-    }
-    else
-    {
-        plug_config = MSS_FREQ_EMPTY;
-    }
-
-
-    FAPI_INF( "PLUG CONFIG(from SPD): %d, Type of Dimm(from SPD): 0x%02X, Num Ranks(from SPD): %d",
-              plug_config, module_type, num_ranks_total);
-fapi_try_exit:
-    return fapi2::current_err;
-
-#endif
+}// dram_type
 
-    return fapi2::FAPI2_RC_SUCCESS;
-}// system_drop_type
 
 ///
 /// @brief      Checks nominal voltage is correct for all DIMMs
-/// @param[in]  i_target_mcs the fapi2 target
+/// @param[in]  i_target the fapi2 target
 /// @param[in]  i_dimm_state (i.e. functional or non-functional)
 /// @return     ReturnCode
 ///
-inline fapi2::ReturnCode module_nominal_voltage(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target_mcs,
+inline fapi2::ReturnCode module_nominal_voltage(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
         fapi2::TargetState i_dimm_state)
 {
 
     uint64_t l_module_nom_voltage[mss::PORTS_PER_MCS][mss::MAX_DIMM_PER_PORT] = {0};
 
-    FAPI_TRY(mss::spd_module_nominal_voltage(i_target_mcs, &l_module_nom_voltage[0][0]));
+    FAPI_TRY(mss::spd_module_nominal_voltage(i_target, &l_module_nom_voltage[0][0]));
 
-    for (auto p : i_target_mcs.getChildren<fapi2::TARGET_TYPE_MCA>())
+    for (const auto& l_mca : i_target.getChildren<fapi2::TARGET_TYPE_MCA>())
     {
-        auto port_num = mss::index(p);
+        const auto l_port_num = mss::index(l_mca);
 
-        // Iterate through DIMM targets
-        for (auto d : p.getChildren<fapi2::TARGET_TYPE_DIMM>(i_dimm_state))
+        for (const auto& l_dimm : l_mca.getChildren<fapi2::TARGET_TYPE_DIMM>(i_dimm_state))
         {
-            auto dimm_num = mss::index(d);
+            const auto l_dimm_num = mss::index(l_dimm);
 
             FAPI_INF("%s DRAM nominal voltage (in milliseconds) is %d",
-                     mss::c_str(d),
-                     l_module_nom_voltage[port_num][dimm_num]);
+                     mss::c_str(l_dimm),
+                     l_module_nom_voltage[l_port_num][l_dimm_num]);
 
             // All DIMMs have to be operable at 1.2 V, else don't IPL (according to Warren)
-            FAPI_ASSERT( l_module_nom_voltage[port_num][dimm_num] == fapi2::ENUM_ATTR_SPD_MODULE_NOMINAL_VOLTAGE_OPERABLE,
+            FAPI_ASSERT( l_module_nom_voltage[l_port_num][l_dimm_num] == fapi2::ENUM_ATTR_SPD_MODULE_NOMINAL_VOLTAGE_OPERABLE,
                          fapi2::MSS_VOLT_DDR_TYPE_REQUIRED_VOLTAGE().
-                         set_DIMM_VOLTAGE(l_module_nom_voltage[port_num][dimm_num]),
+                         set_DIMM_VOLTAGE(l_module_nom_voltage[l_port_num][l_dimm_num]),
                          "%s.Failed conditional, data returned:o %d.",
-                         mss::c_str(d),
-                         l_module_nom_voltage[port_num][dimm_num] );
+                         mss::c_str(l_dimm),
+                         l_module_nom_voltage[l_port_num][l_dimm_num] );
         }
     }
 
 fapi_try_exit:
     return fapi2::current_err;
+
 }// module_nominal_voltage
 
+
+///
+/// @brief      Checks conditional and implements traces & exits if it fails
+/// @param[in]  i_target the dimm target
+/// @param[in]  i_conditional conditional that we are testing against
+/// @param[in]  i_key map key
+/// @param[in]  i_data additional returned data
+/// @param[in]  i_err_str error string to print out when conditional fails
+/// @return     ReturnCode
+///
+inline fapi2::ReturnCode fail_for_invalid_map(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
+        const bool i_conditional,
+        const uint64_t i_key,
+        const uint64_t i_data = 0,
+        const char* i_err_str = "")
+{
+    // Invalid keys won't return useful or valid data so we allow optional data
+    FAPI_ASSERT(i_conditional,
+                fapi2::MSS_INVALID_KEY().
+                set_KEY(i_key).
+                set_DATA(i_data).
+                set_DIMM_TARGET(i_target),
+                "%s. %s Could not find value from key %d. Returned data: %d.",
+                c_str(i_target),
+                i_err_str,
+                i_key,
+                i_data);
+
+fapi_try_exit:
+    return fapi2::current_err;
+}
+
 namespace spd
 {
 
@@ -243,9 +168,9 @@ namespace spd
 ///
 template< typename T >
 inline fapi2::ReturnCode fail_for_invalid_value(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
-        bool i_conditional,
-        size_t i_spd_byte_index,
-        T i_spd_data,
+        const bool i_conditional,
+        const size_t i_spd_byte_index,
+        const T i_spd_data,
         const char* i_err_str)
 {
     FAPI_ASSERT(i_conditional,
@@ -276,9 +201,9 @@ fapi_try_exit:
 ///
 template< typename T >
 inline void warn_for_invalid_value(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
-                                   bool i_conditional,
-                                   size_t i_spd_byte_index,
-                                   T i_spd_data,
+                                   const bool i_conditional,
+                                   const size_t i_spd_byte_index,
+                                   const T i_spd_data,
                                    const char* i_err_str)
 {
     // Don't print warning conditional is true
@@ -302,9 +227,9 @@ inline void warn_for_invalid_value(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>&
 /// @return     fapi2::ReturnCode
 ///
 inline fapi2::ReturnCode invalid_factory_sel(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
-        uint8_t i_dimm_type,
-        uint8_t i_encoding_rev,
-        uint8_t i_additions_rev,
+        const uint8_t i_dimm_type,
+        const uint8_t i_encoding_rev,
+        const uint8_t i_additions_rev,
         const char* i_err_str)
 {
     FAPI_ASSERT(false,
@@ -323,6 +248,31 @@ fapi_try_exit:
     return fapi2::current_err;
 }// invalid_factory_sel
 
+///
+/// @brief      Checks if valid factory parameters are given
+/// @param[in]  i_target fapi2 dimm target
+/// @param[in]  i_dimm_type DIMM type enumeration
+/// @param[in]  i_encoding_rev SPD encoding level rev number
+/// @param[in]  i_additions_rev SPD additions level rev number
+/// @param[in]  i_err_str string to print out when conditional fails
+/// @return     fapi2::ReturnCode
+///
+inline fapi2::ReturnCode invalid_cache(const fapi2::Target<fapi2::TARGET_TYPE_DIMM>& i_target,
+                                       const bool i_conditional,
+                                       const uint8_t i_dimm_pos)
+{
+    FAPI_ASSERT(i_conditional,
+                fapi2::MSS_INVALID_CACHE().
+                set_DIMM_POS(i_dimm_pos).
+                set_DIMM_TARGET(i_target),
+                "%s. Could not find SPD decoder cache for dimm pos: %d",
+                c_str(i_target),
+                i_dimm_pos);
+
+fapi_try_exit:
+    return fapi2::current_err;
+}// invalid_factory_sel
+
 }// spd
 }// check
 }// mss
diff --git a/src/import/chips/p9/procedures/hwp/memory/lib/utils/find.H b/src/import/chips/p9/procedures/hwp/memory/lib/utils/find.H
index e15563970..1a81db863 100644
--- a/src/import/chips/p9/procedures/hwp/memory/lib/utils/find.H
+++ b/src/import/chips/p9/procedures/hwp/memory/lib/utils/find.H
@@ -31,6 +31,8 @@
 #define _MSS_FIND_H
 
 #include <fapi2.H>
+#include <map>
+#include <vector>
 
 namespace mss
 {
@@ -192,7 +194,9 @@ inline fapi2::Target<fapi2::TARGET_TYPE_MCA> find_target( const fapi2::Target<fa
 /// @return the value corresponding to the key
 ///
 template<typename T, typename OT>
-bool find_value_from_key(const std::vector<std::pair<T, OT> >& i_vector_of_pairs, const T& i_key, OT& o_value)
+bool find_value_from_key(const std::vector<std::pair<T, OT> >& i_vector_of_pairs,
+                         const T& i_key,
+                         OT& o_value)
 {
     // Comparator lambda expression
     auto compare = [](const std::pair<T, OT>& i_lhs, const T & i_value)
@@ -201,7 +205,10 @@ bool find_value_from_key(const std::vector<std::pair<T, OT> >& i_vector_of_pairs
     };
 
     // Find iterator to matching key (if it exists)
-    auto l_value_iterator  =  std::lower_bound(i_vector_of_pairs.begin(), i_vector_of_pairs.end(), i_key, compare);
+    auto l_value_iterator  =  std::lower_bound(i_vector_of_pairs.begin(),
+                              i_vector_of_pairs.end(),
+                              i_key,
+                              compare);
 
     // Did you find it? Let me know.
     if( (l_value_iterator == i_vector_of_pairs.end()) || (i_key != l_value_iterator->first) )
@@ -212,8 +219,7 @@ bool find_value_from_key(const std::vector<std::pair<T, OT> >& i_vector_of_pairs
     o_value = l_value_iterator->second;
     return true;
 
-}
-
+}// find_value_from_key
 
 }// mss
 
diff --git a/src/import/chips/p9/procedures/hwp/memory/p9_mss_freq.C b/src/import/chips/p9/procedures/hwp/memory/p9_mss_freq.C
index 5d27fcd97..46096c172 100644
--- a/src/import/chips/p9/procedures/hwp/memory/p9_mss_freq.C
+++ b/src/import/chips/p9/procedures/hwp/memory/p9_mss_freq.C
@@ -22,26 +22,93 @@
 /// @brief Calculate and save off DIMM frequencies
 ///
 // *HWP HWP Owner: Andre Marin <aamarin@us.ibm.com>
-// *HWP HWP Backup: Luke Mulkey <lwmulkey@us.ibm.com>
 // *HWP FW Owner: Brian Silver <bsilver@us.ibm.com>
 // *HWP Team: Memory
 // *HWP Level: 1
 // *HWP Consumed by: FSP:HB
 
+//----------------------------------------------------------------------
+//  Includes
+//----------------------------------------------------------------------
+#include <cstring>
+#include <cstdint>
+
 #include <fapi2.H>
+#include <mss.H>
 #include <p9_mss_freq.H>
+#include <lib/spd/spd_decoder.H>
+#include <lib/freq/cas_latency.H>
+#include <lib/utils/c_str.H>
+#include <lib/freq/cycle_time.H>
 
 using fapi2::TARGET_TYPE_MCS;
+using fapi2::TARGET_TYPE_MCA;
+using fapi2::TARGET_TYPE_DIMM;
 using fapi2::FAPI2_RC_SUCCESS;
 
-///
-/// @brief Calculate and save off DIMM frequencies
-/// @param[in] i_target the controller (e.g., MCS)
-/// @return FAPI2_RC_SUCCESS iff ok
-///
-fapi2::ReturnCode p9_mss_freq( const fapi2::Target<TARGET_TYPE_MCS>& i_target )
+extern "C"
 {
-    FAPI_INF("Start mss freq");
-    FAPI_INF("End mss freq");
-    return FAPI2_RC_SUCCESS;
-}
+
+    ///
+    /// @brief Calculate and save off DIMM frequencies
+    /// @param[in] i_target, the controller (e.g., MCS)
+    /// @return FAPI2_RC_SUCCESS iff ok
+    ///
+    fapi2::ReturnCode p9_mss_freq( const fapi2::Target<TARGET_TYPE_MCS>& i_target )
+    {
+        uint64_t l_tCKmin = 0;
+        uint64_t l_min_dimm_freq = 0;
+        uint64_t l_desired_cas_latency = 0;
+
+        // Get cached decoder
+        std::map<uint32_t, std::shared_ptr<mss::spd::decoder> > l_factory_caches;
+        FAPI_TRY( mss::spd::populate_decoder_caches(i_target, l_factory_caches),
+                  "Failed to populate decoder cache");
+
+        {
+            // instantiation of class that calculates CL algorithm
+            mss::cas_latency l_cas_latency(i_target, l_factory_caches);
+
+#if 0   // TK - encapsulated functionality left over from p8, how do we tackle this for p9?? -AAM
+            // left for reference
+
+            // TK - Need to add functionality for determining system setting based
+            // on system drop (e.g. single & dual drop) and configuration.
+            // How will we determine a system is single or dual drop?
+            // What will we do if there is dimm mixing?
+            // How does this affect tck timing ? - AAM
+
+            // TK - Need to incorporate code path for overrides
+            FAPI_TRY(mss::check_for_freq_override(l_target_mcbist,
+                                                  l_tCKmin),
+                     "Failed check for freq_override()");
+
+#endif
+
+            // Find CAS latency using JEDEC algorithm
+            l_cas_latency.find_CL(i_target,
+                                  l_desired_cas_latency,
+                                  l_tCKmin);
+
+            // Find dimm transfer speed from selected tCK
+            l_min_dimm_freq = mss::ps_to_freq(l_tCKmin);
+            FAPI_TRY(mss::select_supported_freq(l_min_dimm_freq),
+                     "Failed select_supported_freq()");
+
+            // Set attributes
+            FAPI_TRY(mss::set_freq_attrs(i_target, l_min_dimm_freq),
+                     "Failed set_freq_attrs()");
+
+            FAPI_TRY(mss::set_CL_attr(i_target, l_desired_cas_latency),
+                     "Failed set_CL_attr()");
+
+            FAPI_DBG( "Final Chosen Frequency: %d",  l_min_dimm_freq);
+            FAPI_DBG( "Final Chosen CL: %d",  l_desired_cas_latency);
+        }
+
+    fapi_try_exit:
+        return fapi2::current_err;
+
+    }// p9_mss_freq
+
+}// extern C
diff --git a/src/import/chips/p9/procedures/hwp/memory/p9_mss_freq.H b/src/import/chips/p9/procedures/hwp/memory/p9_mss_freq.H
index 4f00b7c0f..3be3259bd 100644
--- a/src/import/chips/p9/procedures/hwp/memory/p9_mss_freq.H
+++ b/src/import/chips/p9/procedures/hwp/memory/p9_mss_freq.H
@@ -24,91 +24,100 @@
 // *HWP HWP Owner: Andre Marin <aamarin@us.ibm.com>
 // *HWP HWP Backup: Brian Silver <bsilver@us.ibm.com>
 // *HWP Team: Memory
-// *HWP Level: 2
+// *HWP Level: 1
 // *HWP Consumed by: FSP:HB
 
-#ifndef __P9_MSS_FREQ__
-#define __P9_MSS_FREQ__
-
 #include <fapi2.H>
+#include <lib/freq/cas_latency.H>
+#include <lib/freq/cycle_time.H>
+#include <lib/shared/mss_const.H>
+#include <lib/utils/conversions.H>
 
 namespace mss
 {
 
-enum constants : std::uint64_t
-{
-    TAA_MAX_DDR4 = 18000,
-    TWO_MHZ = 2000000,
-    DMI_9_6GB = 0x01,
-    DMI_16_0GB = 0x02,
-    LR_MIN_DDR4_CL = 7,
-    LR_MAX_DDR4_CL = 36,
-    HR_MIN_DDR4_CL = 23,
-    HR_MAX_DDR4_CL = 52,
-};
-
-enum frequencies : std::uint64_t
-{
-    FREQ_2667 = 2667,
-    FREQ_2400 = 2400,
-    FREQ_2133 = 2133,
-    FREQ_1867 = 1867,
-    FREQ_1600 = 1600,
-};
-
-
 ///
-/// @brief      Calculate frequency
-/// @tparam     T input
-/// @param[in]  i_clock T
-/// @return     T (frequency)
+/// @brief      Checks for frequency override and sets dimm frequency and timing values
+/// @param[in]  i_target mcbist fapi2 target
+/// @param[out] o_tCK new cycle time if there is a freq override
+/// @return     FAPI2_RC_SUCCESS iff ok
 ///
-template<typename T>
-inline T calc_freq(T i_clock)
+inline fapi2::ReturnCode check_for_freq_override(const fapi2::Target<fapi2::TARGET_TYPE_MCBIST>& i_target,
+        uint64_t& o_tCK)
 {
-    // Casted so compiler performs operations on equivalent data types
-    T frequency = TWO_MHZ / i_clock;
-    // Round-up if division leaves remainder
-    frequency += ( (TWO_MHZ % i_clock) == 0 ? 0 : 1 );
+    uint64_t l_freq_override = 0;
+
+    FAPI_TRY(freq_override(i_target, l_freq_override),
+             "Failed to override frequency!");
+
+    // If there is no override, don't change anything
+    if ( l_freq_override != fapi2::ENUM_ATTR_MSS_FREQ_OVERRIDE_AUTO)
+    {
+        o_tCK = mss::freq_to_ps(l_freq_override);
+        FAPI_DBG( "Override Frequency Detected: %d", l_freq_override);
+    }
 
-    return frequency;
+fapi_try_exit:
+    return fapi2::current_err;
 }
 
+
 ///
-/// @brief  Calculate timing value (e.g.tCK, tAA, etc.)
-/// @tparam     T input
-/// @param[in]  i_freq T
-/// @return     T (timing)
+/// @brief      Sets DRAM CAS latency attributes
+/// @param[in]  i_target the controller target
+/// @param[in]  i_cas_latency final selected CAS ltency
+/// @return     FAPI2_RC_SUCCESS iff ok
 ///
-template<typename T>
-inline T calc_clock (T i_freq)
+inline fapi2::ReturnCode set_CL_attr(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
+                                     uint64_t i_cas_latency)
 {
-    T timing = TWO_MHZ / i_freq;
-    // Round-up if division leaves remainder
-    timing += ( (TWO_MHZ % i_freq) == 0 ? 0 : 1 );
-
-    return timing;
+    // TK - Not sure if we still want this set at a port level - AAM
+
+    // Declaration of the vector correctly initializes it to the right size
+    // in the case the enum for PORTS_PER_MCS changes
+    std::vector<uint64_t> l_cls_vect(mss::PORTS_PER_MCS, i_cas_latency);
+
+    // set CAS latency attribute
+    // Dereferences pointer of the vector's underlying data
+    // and casts it to uint64_t[2] that FAPI_ATTR_SET is expecting by deduction
+    FAPI_TRY( FAPI_ATTR_SET(fapi2::ATTR_EFF_DRAM_CL,
+                            i_target,
+                            reinterpret_cast<uint64_t(&)[2]>(*l_cls_vect.data()) ) ,
+              "Failed to set CAS latency attribute");
+
+fapi_try_exit:
+    return fapi2::current_err;
 }
 
 ///
-/// @brief  Calculate CAS latency
-/// @tparam[in]  T
-/// @param[in]   tAA_max T,tCKmax T
-/// @return      T cas_latency
+/// @brief      Sets frequency attributes
+/// @param[in]  i_target the controller target
+/// @param[in]  i_dimm_freq final selected dimm freq in MT/s
+/// @return     FAPI2_RC_SUCCESS iff ok
+/// @note       P9 Nimbus will support DMI speeds 16GB and 9.6GB
 ///
-template<typename T>
-inline T calc_cas_latency(T tAA_max, T tCKmax)
+inline fapi2::ReturnCode set_freq_attrs(const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target,
+                                        uint64_t i_dimm_freq)
 {
-    T cas_latency = tAA_max / tCKmax;
-    // Increment if division leaves remainder
-    cas_latency += ( (tAA_max % tCKmax) == 0 ? 0 : 1 );
+    uint64_t l_nest_freq = 1;
+    const auto l_mcbist = i_target.getParent<fapi2::TARGET_TYPE_MCBIST>();
 
-    return cas_latency;
-}
+    // TK
+    //Update for P9, what do we do w/setting nest freq? - AAM
+    // how do we select nest freq if we even have to??
+    FAPI_TRY( FAPI_ATTR_SET(fapi2::ATTR_MSS_NEST_CAPABLE_FREQUENCIES, l_mcbist, l_nest_freq),
+              "Failed to set nest capable frequencies attribute" );
 
+    FAPI_TRY(FAPI_ATTR_SET(fapi2::ATTR_MSS_FREQ, l_mcbist, i_dimm_freq),
+             "Failed to set mss freq attribute");
+
+fapi_try_exit:
+    return fapi2::current_err;
+}
 
 }// mss namespace
 
+
 typedef fapi2::ReturnCode (*p9_mss_freq_FP_t) (const fapi2::Target<fapi2::TARGET_TYPE_MCS>&);
 
 extern "C"
@@ -119,8 +128,6 @@ extern "C"
     /// @param[in] i_target the controller (e.g., MCS)
     /// @return FAPI2_RC_SUCCESS iff ok
     ///
-    fapi2::ReturnCode p9_mss_freq( const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target );
+    fapi2::ReturnCode p9_mss_freq( const fapi2::Target<fapi2::TARGET_TYPE_MCS>& i_target);
 
 }
-
-#endif