L3 update - p9_avsbus_xxx HWPs

- rebased
- Added note to XML to clarify the lack of callouts in deference to the
  PM Reset flow

Change-Id: I5c6a38bd92e256abefedc00ae20d0a5c748fc12c
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/44692
Reviewed-by: RANGANATHPRASAD G. BRAHMASAMUDRA <prasadbgr@in.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Hostboot CI <hostboot-ci+hostboot@us.ibm.com>
Reviewed-by: Jennifer A. Stofer <stofer@us.ibm.com>

2 files changed, 75 insertions, 147 deletions

diff --git a/import/chips/p9/procedures/hwp/lib/p9_avsbus_lib.C b/import/chips/p9/procedures/hwp/lib/p9_avsbus_lib.C
index e537b7c4..81d31ef8 100644
--- a/import/chips/p9/procedures/hwp/lib/p9_avsbus_lib.C
+++ b/import/chips/p9/procedures/hwp/lib/p9_avsbus_lib.C
@@ -22,15 +22,17 @@
 /* permissions and limitations under the License.                         */
 /*                                                                        */
 /* IBM_PROLOG_END_TAG                                                     */
+
 ///
 /// @file  p9_avsbus_lib.C
 /// @brief Library functions for AVSBus
 ///
-// *HW Owner    : Sudheendra K Srivathsa <sudheendraks@in.ibm.com>
-// *FW Owner    : Sangeetha T S <sangeet2@in.ibm.com>
-// *Team        : PM
-// *Consumed by : HB
-// *Level       : 2
+/// *HWP HWP Owner        : Greg Still <stillgs@us.ibm.com>
+/// *HWP HWP Backup Owner : Brian Vanderpool <vanderp@us.ibm.com>
+/// *HWP FW Owner         : Prasad BG Ranganath <prasadbgr@in.ibm.com>
+/// *Team                 : PM
+/// *Consumed by          : SBE:SGPE
+/// *Level                : 3
 ///
 
 #include <p9_avsbus_lib.H>
@@ -40,12 +42,11 @@
 //##############################################################################
 // Function which generates a 3 bit CRC value for 29 bit data
 //##############################################################################
-#define AVS_CRC_MASK      0x00000007
 #define AVS_CRC_DATA_MASK 0xFFFFFFF8
 uint32_t avsCRCcalc(const uint32_t i_avs_cmd)
 {
-    //Polynomial= x^3 + x^1 + x^0 = 1*x^3 + 0*x^2 + 1*x^1 + 1*x^0 = divisor(1011)
-
+    //Polynomial = x^3 + x^1 + x^0 = 1*x^3 + 0*x^2 + 1*x^1 + 1*x^0
+    //           = divisor(1011)
     uint32_t o_crc_value = 0;
     uint32_t l_polynomial = 0xB0000000;
     uint32_t l_msb =        0x80000000;
@@ -56,7 +57,7 @@ uint32_t avsCRCcalc(const uint32_t i_avs_cmd)
     {
         if (o_crc_value & l_msb)
         {
-            //if l_msb is 1'b1, divide by l_polynomial and shift l_polynomial
+            // if l_msb is 1'b1, divide by l_polynomial and shift l_polynomial
             // to the right
             o_crc_value = o_crc_value ^ l_polynomial;
             l_polynomial = l_polynomial >> 1;
@@ -71,21 +72,17 @@ uint32_t avsCRCcalc(const uint32_t i_avs_cmd)
     }
 
     FAPI_INF("The computed CRC Value is %d", o_crc_value)
-
     return o_crc_value;
 }
 
 //##############################################################################
-
-
-//##############################################################################
 // Function which initializes the OCB O2S registers
 //##############################################################################
 fapi2::ReturnCode
-avsInitExtVoltageControl(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>&
-                         i_target,
-                         const uint8_t i_avsBusNum,
-                         const uint8_t i_o2sBridgeNum)
+avsInitExtVoltageControl(
+    const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
+    const uint8_t i_avsBusNum,
+    const uint8_t i_o2sBridgeNum)
 {
 
     fapi2::buffer<uint64_t> l_data64;
@@ -100,7 +97,6 @@ avsInitExtVoltageControl(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>&
 
     //uint32_t O2SCTRLF_value = 0b10000010000011111100000000000000;
     ocb_o2sctrlf0a_t O2SCTRLF_value;
-
     O2SCTRLF_value.fields.o2s_frame_size_an = p9avslib::O2S_FRAME_SIZE;
     O2SCTRLF_value.fields.o2s_out_count1_an = p9avslib::O2S_FRAME_SIZE;
     O2SCTRLF_value.fields.o2s_in_delay1_an = p9avslib::O2S_IN_DELAY1;
@@ -108,10 +104,11 @@ avsInitExtVoltageControl(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>&
     l_data64.insertFromRight<0, 6>(O2SCTRLF_value.fields.o2s_frame_size_an);
     l_data64.insertFromRight<6, 6>(O2SCTRLF_value.fields.o2s_out_count1_an);
     l_data64.insertFromRight<12, 6>(O2SCTRLF_value.fields.o2s_in_delay1_an);
-
     FAPI_TRY(putScom(i_target,
-                     p9avslib::OCB_O2SCTRLF[i_avsBusNum][i_o2sBridgeNum], l_data64));
-    // Note to self:  the buffer is a 32bit buffer.  make sure it is left
+                     p9avslib::OCB_O2SCTRLF[i_avsBusNum][i_o2sBridgeNum],
+                     l_data64));
+
+    // Note:  the buffer is a 32bit buffer.  make sure it is left
     // aligned for the SCOM
 
     // O2SCTRLS
@@ -123,13 +120,11 @@ avsInitExtVoltageControl(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>&
     ocb_o2sctrls0a_t O2SCTRLS_value;
     O2SCTRLS_value.fields.o2s_in_count2_an = p9avslib::O2S_FRAME_SIZE;
 
-    //O2SCTRLS_value.value = 0b00000000000010000000000000000000;
-
     l_data64.flush<0>();
     l_data64.insertFromRight<12, 6>(O2SCTRLS_value.fields.o2s_in_count2_an);
-
     FAPI_TRY(putScom(i_target,
-                     p9avslib::OCB_O2SCTRLS[i_avsBusNum][i_o2sBridgeNum], l_data64));
+                     p9avslib::OCB_O2SCTRLS[i_avsBusNum][i_o2sBridgeNum],
+                     l_data64));
 
     // O2SCTRL2
     // [    0] o2s_bridge_enable
@@ -151,22 +146,17 @@ avsInitExtVoltageControl(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>&
     // 1 / (AVSBus frequency *8) held in an attribute allows a
     // fully l_data64 driven computation without a divide operation.
 
-
-    // uint32_t O2SCTRL1_value = 0b10000000000100000100000000000000;
-
-    // uint32_t nest_freq;
-    // FAPI_ATTR_GET(ATTR_PB_FREQ, &nest_freq);
-
     ocb_o2sctrl10a_t O2SCTRL1_value;
     O2SCTRL1_value.fields.o2s_bridge_enable_an = 1;
-    //O2SCTRL1_value.fields.o2s_clock_divider_an = 0x04;//Attribute supported added below
 
     //Nest frequency attribute in MHz
-    FAPI_TRY(FAPI_ATTR_GET(fapi2::ATTR_FREQ_PB_MHZ, fapi2::Target<fapi2::TARGET_TYPE_SYSTEM>(),
+    FAPI_TRY(FAPI_ATTR_GET(fapi2::ATTR_FREQ_PB_MHZ,
+                           fapi2::Target<fapi2::TARGET_TYPE_SYSTEM>(),
                            l_nest_frequency));
 
     // AVSBus frequency attribute in KHz
-    FAPI_TRY(FAPI_ATTR_GET(fapi2::ATTR_AVSBUS_FREQUENCY, fapi2::Target<fapi2::TARGET_TYPE_SYSTEM>(),
+    FAPI_TRY(FAPI_ATTR_GET(fapi2::ATTR_AVSBUS_FREQUENCY,
+                           fapi2::Target<fapi2::TARGET_TYPE_SYSTEM>(),
                            l_value));
 
     if (l_value == 0)
@@ -190,17 +180,15 @@ avsInitExtVoltageControl(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>&
     O2SCTRL1_value.fields.o2s_cpol_an = 0;
     O2SCTRL1_value.fields.o2s_cpha_an = 1;
 
-//    O2SCTRL2_value.value  = 0b00000000000000000000000000000000;
-
     l_data64.flush<0>();
     l_data64.insertFromRight<0, 1>(O2SCTRL1_value.fields.o2s_bridge_enable_an);
     l_data64.insertFromRight<2, 1>(O2SCTRL1_value.fields.o2s_cpol_an);
     l_data64.insertFromRight<3, 1>(O2SCTRL1_value.fields.o2s_cpha_an);
     l_data64.insertFromRight<4, 10>(O2SCTRL1_value.fields.o2s_clock_divider_an);
     l_data64.insertFromRight<17, 1>(O2SCTRL1_value.fields.o2s_nr_of_frames_an);
-
     FAPI_TRY(putScom(i_target,
-                     p9avslib::OCB_O2SCTRL1[i_avsBusNum][i_o2sBridgeNum], l_data64));
+                     p9avslib::OCB_O2SCTRL1[i_avsBusNum][i_o2sBridgeNum],
+                     l_data64));
 
     // O2SCTRL1
     // OCC O2S Control2
@@ -214,24 +202,22 @@ avsInitExtVoltageControl(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>&
     l_data64.flush<0>();
     l_data64.insertFromRight<0, 17>
     (O2SCTRL2_value.fields.o2s_inter_frame_delay_an);
-
     FAPI_TRY(putScom(i_target,
-                     p9avslib::OCB_O2SCTRL2[i_avsBusNum][i_o2sBridgeNum], l_data64));
+                     p9avslib::OCB_O2SCTRL2[i_avsBusNum][i_o2sBridgeNum],
+                     l_data64));
 
 fapi_try_exit:
     return fapi2::current_err;
 }
-//##############################################################################
-
 
 //##############################################################################
 // Function polls OCB status register O2SST for o2s_ongoing=0
 //##############################################################################
 fapi2::ReturnCode
-avsPollVoltageTransDone(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>&
-                        i_target,
-                        const uint8_t i_avsBusNum,
-                        const uint8_t i_o2sBridgeNum)
+avsPollVoltageTransDone(
+    const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
+    const uint8_t i_avsBusNum,
+    const uint8_t i_o2sBridgeNum)
 {
     fapi2::ReturnCode l_rc = fapi2::FAPI2_RC_SUCCESS;
     fapi2::buffer<uint64_t> l_data64;
@@ -241,17 +227,16 @@ avsPollVoltageTransDone(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>&
     while (l_count < p9avslib::MAX_POLL_COUNT_AVS)
     {
         FAPI_TRY(getScom(i_target, p9avslib::OCB_O2SST[i_avsBusNum]
-                         [i_o2sBridgeNum], l_data64), "Error in polling ongoing bit");
+                         [i_o2sBridgeNum], l_data64),
+                 "Error from getscom 0x%.16llX",
+                 p9avslib::OCB_O2SST[i_avsBusNum]);
 
-        l_data64 &= 0x8000000000000000ull;
-
-        if (l_data64 == 0)
+        if (!l_data64.getBit<0>())
         {
             break;  // Leave the polling loop as "ongoing" has deasserted
         }
 
         l_count++;
-        // May put a delay (or sleep) in here to keep off of the OCI bus
     }
 
     // Check for timeout condition
@@ -272,7 +257,6 @@ avsPollVoltageTransDone(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>&
 fapi_try_exit:
     return fapi2::current_err;
 }
-//##############################################################################
 
 //##############################################################################
 // Function which outputs a downstream command
@@ -291,7 +275,6 @@ avsDriveCommand(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
 
     fapi2::buffer<uint64_t> l_data64;
     fapi2::buffer<uint32_t> l_data64WithoutCRC;
-
     fapi2::ReturnCode l_rc;
 
     uint32_t l_StartCode = 0b01;
@@ -299,11 +282,10 @@ avsDriveCommand(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
     uint32_t l_crc;
 
     // clear sticky bits in o2s_status_reg
-    // @todo check this register in the EPM code
     l_data64.setBit<1, 1>();
     FAPI_TRY(putScom(i_target,
-                     p9avslib::OCB_O2SCMD[i_avsBusNum][i_o2sBridgeNum], l_data64));
-
+                     p9avslib::OCB_O2SCMD[i_avsBusNum][i_o2sBridgeNum],
+                     l_data64));
 
     // MSB sent out first always, which should be start code 0b01
     // compose and send frame
@@ -315,14 +297,6 @@ avsDriveCommand(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
     //      l_CmdType(3:2),
     //      l_StartCode(1:0)
     l_data64.flush<0>();
-
-    //l_data64 = l_data64 |
-    //       (l_StartCode << 60) |
-    //       (i_CmdType << 28) |
-    //       (i_CmdGroup << 27) |
-    //       (i_CmdDataType << 23) |
-    //       (i_RailSelect << 19) |
-    //       (l_Reserved << 3);
     l_data64.insertFromRight<0, 2>(l_StartCode);
     l_data64.insertFromRight<2, 2>(i_CmdType);
     l_data64.insertFromRight<4, 1>(i_CmdGroup);
@@ -331,13 +305,10 @@ avsDriveCommand(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
     l_data64.insertFromRight<13, 16>(i_CmdData);
     l_data64.insertFromRight<29, 3>(l_Reserved);
 
+    // Generate CRC
     l_data64.extract(l_data64WithoutCRC, 0, 32);
-
-    // generate CRC
     l_crc = avsCRCcalc(l_data64WithoutCRC);
-
     l_data64.insertFromRight<29, 3>(l_crc);
-
     FAPI_TRY(putScom(i_target,
                      p9avslib::OCB_O2SWD[i_avsBusNum][i_o2sBridgeNum], l_data64));
 
@@ -381,7 +352,8 @@ avsVoltageRead(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
     fapi2::buffer<uint64_t> l_data64;
 
     // Values as per VRM spec
-    // Cmd 0b11, cmd group 0, cmd data type - 0b0000 for voltage read, outbound data = 0xFFFf
+    // Cmd 0b11, cmd group 0, cmd data type - 0b0000 for voltage read,
+    // outbound data = 0xFFFf
     uint32_t l_CmdType     = 3; // read
     uint32_t l_CmdGroup    = 0;
     uint32_t l_CmdDataType = 0;
@@ -408,7 +380,6 @@ avsVoltageRead(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
 fapi_try_exit:
     return fapi2::current_err;
 }
-//##############################################################################
 
 //##############################################################################
 // Function which writes to OCB registers to initiate a AVS write transaction
@@ -433,13 +404,11 @@ avsVoltageWrite(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
                              l_CmdType,
                              l_CmdGroup,
                              l_CmdDataType,
-                             i_Voltage));   // @todo command data flow
+                             i_Voltage));
 
 fapi_try_exit:
     return fapi2::current_err;
 }
-//##############################################################################
-
 
 //##############################################################################
 // Function which writes to OCB registers to initialize the AVS Slave with an
@@ -460,12 +429,12 @@ avsIdleFrame(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
                      l_scomdata),
              "Error clearing sticky bits in o2s_status_reg");
 
-
     FAPI_INF("Sending idle frame of all 1s");
     // Send the idle frame
     l_scomdata = l_idleframe;
     FAPI_TRY(putScom(i_target,
-                     p9avslib::OCB_O2SWD[i_avsBusNum][i_o2sBridgeNum], l_scomdata));
+                     p9avslib::OCB_O2SWD[i_avsBusNum][i_o2sBridgeNum],
+                     l_scomdata));
 
     // Wait on o2s_ongoing = 0
     FAPI_TRY(avsPollVoltageTransDone(i_target, i_avsBusNum, i_o2sBridgeNum));
@@ -479,13 +448,11 @@ fapi_try_exit:
                     .set_CHIP_TARGET(i_target)
                     .set_AVSBUS_NUM(i_avsBusNum)
                     .set_AVSBUS_BRIDGE_NUM(i_o2sBridgeNum),
-                    "AVS Idle frame funciton fail");
+                    "AVS Idle frame function fails");
     }
 
     return fapi2::current_err;
 }
-//##############################################################################
-
 
 //##############################################################################
 // Function which reads the data response from the AVSBus and validates it.
@@ -503,19 +470,13 @@ avsValidateResponse(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
     fapi2::buffer<uint8_t>  l_data_status_code;
     fapi2::buffer<uint32_t> l_rsp_data;
 
-    uint32_t                l_rsp_computed_crc;
-    uint8_t                 l_attr_is_simulation;
-
+    uint32_t l_rsp_computed_crc;
     o_goodResponse = false;
 
-    // Attribute to skip error checks for simulation
-    FAPI_TRY(FAPI_ATTR_GET(fapi2::ATTR_IS_SIMULATION,
-                           fapi2::Target<fapi2::TARGET_TYPE_SYSTEM>(),
-                           l_attr_is_simulation));
-
     // Read the data response register
     FAPI_DBG("Reading the OS2SRD register to check status");
-    FAPI_TRY(getScom(i_target, p9avslib::OCB_O2SRD[i_avsBusNum][i_o2sBridgeNum], l_data64));
+    FAPI_TRY(getScom(i_target, p9avslib::OCB_O2SRD[i_avsBusNum][i_o2sBridgeNum],
+                     l_data64));
 
     // Status Return Code and Received CRC
     l_data64.extractToRight(l_data_status_code, 0, 2);
@@ -525,7 +486,6 @@ avsValidateResponse(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
     // Compute CRC on Response frame
     l_rsp_computed_crc = avsCRCcalc(l_rsp_data);
 
-
     if ((l_data_status_code == 0) &&                           // no error code
         (l_rsp_rcvd_crc == l_rsp_computed_crc) &&              // good crc
         (l_rsp_data != 0) && (l_rsp_data != 0xFFFFFFFF))       // valid response
@@ -537,55 +497,50 @@ avsValidateResponse(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
         FAPI_INF("Incorrect response received - Computed CRC %X Received %X - Full Response %08X",
                  l_rsp_computed_crc, l_rsp_rcvd_crc, l_rsp_data);
 
-
-
         if(l_rsp_data == 0x00000000)
         {
-            FAPI_DBG("ERROR: AVS command failed failed. All 0 response data received possibly due to AVSBus IO RI/DIs disabled.");
+            FAPI_DBG("ERROR: AVS command failed. All 0 response data received possibly due to AVSBus IO RI/DIs disabled.");
             FAPI_ASSERT((i_throw_assert != true),
                         fapi2::PM_AVSBUS_ZERO_RESP_ERROR().set_TARGET(i_target).set_BUS(i_avsBusNum).set_BRIDGE(i_o2sBridgeNum),
-                        "ERROR: AVS command failed failed. All 0 response data received possibly due to AVSBus IO RI/DIs disabled.");
+                        "ERROR: AVS command failed. All 0 response data received possibly due to AVSBus IO RI/DIs disabled.");
         }
         else if(l_rsp_data == 0xFFFFFFFF)
         {
             FAPI_DBG("ERROR: AVS command failed failed. No response from VRM device, Check AVSBus interface connectivity to VRM in system.");
             FAPI_ASSERT((i_throw_assert != true),
                         fapi2::PM_AVSBUS_NO_RESP_ERROR().set_TARGET(i_target).set_BUS(i_avsBusNum).set_BRIDGE(i_o2sBridgeNum),
-                        "ERROR: AVS command failed failed. No response from VRM device, Check AVSBus interface connectivity to VRM in system.");
+                        "ERROR: AVS command failed. No response from VRM device, Check AVSBus interface connectivity to VRM in system.");
         }
         else if(l_rsp_rcvd_crc != l_rsp_computed_crc)
         {
             FAPI_DBG("ERROR: AVS command failed failed. Bad CRC detected by P9 on AVSBus Slave Segement.");
             FAPI_ASSERT((i_throw_assert != true),
                         fapi2::PM_AVSBUS_MASTER_BAD_CRC_ERROR().set_TARGET(i_target).set_BUS(i_avsBusNum).set_BRIDGE(i_o2sBridgeNum),
-                        "ERROR: AVS command failed failed. Bad CRC detected by P9 on AVSBus Slave Segement.");
+                        "ERROR: AVS command failed. Bad CRC detected by P9 on AVSBus Slave Segement.");
         }
         else if(l_data_status_code == 0x02)
         {
             FAPI_DBG("ERROR: AVS command failed failed. Bad CRC indicated by Slave VRM on AVSBus Master Segement.");
             FAPI_ASSERT((i_throw_assert != true),
                         fapi2::PM_AVSBUS_SLAVE_BAD_CRC_ERROR().set_TARGET(i_target).set_BUS(i_avsBusNum).set_BRIDGE(i_o2sBridgeNum),
-                        "ERROR: AVS command failed failed. Bad CRC indicated by Slave VRM on AVSBus Master Segement.");
+                        "ERROR: AVS command failed. Bad CRC indicated by Slave VRM on AVSBus Master Segement.");
         }
         else if(l_data_status_code == 0x01)
         {
             FAPI_DBG("ERROR: AVS command failed failed. Valid data sent but no action is taken due to unavailable resource.");
             FAPI_ASSERT((i_throw_assert != true),
                         fapi2::PM_AVSBUS_UNAVAILABLE_RESOURCE_ERROR().set_TARGET(i_target).set_BUS(i_avsBusNum).set_BRIDGE(i_o2sBridgeNum),
-                        "ERROR: AVS command failed failed. Valid data sent but no action is taken due to unavailable resource.");
+                        "ERROR: AVS command failed. Valid data sent but no action is taken due to unavailable resource.");
         }
         else if(l_data_status_code == 0x03)
         {
             FAPI_DBG("ERROR: AVS command failed failed. Unknown resource, invalid data, incorrect data or incorrect action.");
             FAPI_ASSERT((i_throw_assert != true), fapi2::PM_AVSBUS_INVALID_DATA_ERROR().set_TARGET(i_target),
-                        "ERROR: AVS command failed failed. Unknown resource, invalid data, incorrect data or incorrect action.");
+                        "ERROR: AVS command failed. Unknown resource, invalid data, incorrect data or incorrect action.");
         }
-
-
     }
 
 fapi_try_exit:
-
     return fapi2::current_err;
 
 }
diff --git a/import/chips/p9/procedures/hwp/lib/p9_avsbus_lib.H b/import/chips/p9/procedures/hwp/lib/p9_avsbus_lib.H
index 442fefe6..3b865459 100644
--- a/import/chips/p9/procedures/hwp/lib/p9_avsbus_lib.H
+++ b/import/chips/p9/procedures/hwp/lib/p9_avsbus_lib.H
@@ -27,24 +27,22 @@
 /// @file  p9_avsbus_lib.H
 /// @brief Library functions for AVSBus
 ///
-/// *HW Owner    : Sudheendra K Srivathsa <sudheendraks@in.ibm.com>
-/// *FW Owner    : Sangeetha T S <sangeet2@in.ibm.com>
-/// *Team        : PM
-/// *Consumed by : HB
-/// *Level       : 2
+/// *HWP HWP Owner        : Greg Still <stillgs@us.ibm.com>
+/// *HWP HWP Backup Owner : Brian Vanderpool <vanderp@us.ibm.com>
+/// *HWP FW Owner         : Prasad BG Ranganath <prasadbgr@in.ibm.com>
+/// *Team                 : PM
+/// *Consumed by          : SBE:SGPE
+/// *Level                : 3
 ///
-/// @todo (to be considered in L2/L3 development) AVSBus timing parameters
-///  as attributes or not.  They were hardcoded in P8.
 
 #ifndef __P9_AVSBUS_LIB_H__
 #define __P9_AVSBUS_LIB_H__
 
 #include <fapi2.H>
 
-
-
 namespace p9avslib
 {
+
 enum avsRails
 {
     VDD,
@@ -135,11 +133,7 @@ union avsStatus
 
 enum avslibconstants
 {
-
-// @todo:  This should be calculated based on time (eg 100ms) and the projected
-// time that a SCOM poll will take.
-// const uint32_t MAX_POLL_COUNT_AVS = 0x1000;
-// AVSBUS_FREQUENCY specified in Khz, Default value 10 MHz
+    // AVSBUS_FREQUENCY specified in Khz, Default value 10 MHz
     MAX_POLL_COUNT_AVS = 0x1000,
     AVS_CRC_DATA_MASK = 0xfffffff8,
     O2S_FRAME_SIZE = 0x20,
@@ -147,9 +141,6 @@ enum avslibconstants
     AVSBUS_FREQUENCY = 0x2710
 };
 
-// Constant definitions
-//const uint64_t O2S_FRAME_SIZE = 0x20;
-
 // OIMR Mask Values
 const uint32_t OCB_OIMR1_MASK_VALUES[2][2] =
 {
@@ -166,45 +157,36 @@ enum avsBusOpType
     VLTG_WTITE_TYPE,
 };
 
-//const uint64_t OCB_O2SST_MASK = 0x8000000000000000;
-
 } //end of p9avslib namespace
+
 using namespace p9avslib;
 
 ///@brief Generates a 3 bit CRC value for 29 bit data
 ///@param[i] i_data
 ///@return   3 bit CRC result (right aligned)
-
 uint32_t avsCRCcalc(uint32_t i_avs_cmd);
 
-
-
 ///@brief Initialize an O2S bridge for AVSBus usage
 ///@param[i] i_target       Chip target
 ///@param[i] i_avsBusNum    AVSBus Number (0 or 1)
 ///@param[i] i_o2sBridgeNum O2S Bridge Number (0 or 1)
 ///@return   FAPI2::ReturnCode defined in XML
-
 fapi2::ReturnCode
 avsInitExtVoltageControl(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
                          const uint8_t i_avsBusNum,
                          const uint8_t i_o2sBridgeNum);
 
-
 ///@brief Polls OCB status register O2SST for o2s_ongoing=0
 ///@param[i] i_target       Chip target
 ///@param[i] i_avsBusNum    AVSBus Number (0 or 1)
 ///@param[i] i_o2sBridgeNum O2S Bridge Number (0 or 1)
 ///@return   FAPI2::SUCCESS
-///@return   FAPI2::RC_PROCPM_AVSBUS_TIMEOUT
-
+///@return FAPI2_RC_SUCCESS if success, else error code.
 fapi2::ReturnCode
 avsPollVoltageTransDone(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
                         const uint8_t i_avsBusNum,
                         const uint8_t i_o2sBridgeNum);
 
-
-
 ///@brief Drives a downstream command to a select bus via a selected bridge
 ///@param[i] i_target       Chip target
 ///@param[i] i_avsBusNum    AVSBus Number (0 or 1)
@@ -212,10 +194,9 @@ avsPollVoltageTransDone(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_tar
 ///@param[i] i_RailSelect   Rail Select (value depends on the system implementation)
 ///@param[i] i_CmdType      Defined by AVSBus spec (4b, right justified)
 ///@param[i] i_CmdGroup     Defined by AVSBus spec (0 = AVSBus defined; 1 = Mfg defined)
+///@param[i] i_CmdDataType  Command data type
 ///@param[i] i_CmdData      Defined by AVSBus spec and command dependent (16b, right justified)
-///@return   FAPI2::SUCCESS
-///@return   FAPI2::RC_PROCPM_AVSBUS_TIMEOUT
-
+///@return FAPI2_RC_SUCCESS if success, else error code.
 fapi2::ReturnCode
 avsDriveCommand(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
                 const uint8_t  i_avsBusNum,
@@ -227,17 +208,13 @@ avsDriveCommand(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
                 const uint32_t i_CmdData,
                 enum avsBusOpType i_opType = avsBusOpType::NO_OP);
 
-
-
 ///@brief Perform an AVS read transaction
 ///@param[i] i_target       Chip target
 ///@param[i] i_avsBusNum    AVSBus Number (0 or 1)
 ///@param[i] i_o2sBridgeNum O2S Bridge Number (0 or 1)
 ///@param[i] i_RailSelect   Rail Select (value depends on the system implementation)
-///@param[o] o_CmdData      Defined by AVSBus spec and command dependent (16b, right justified)
-///@return   FAPI2::SUCCESS
-///@return   FAPI2::RC_PROCPM_AVSBUS_TIMEOUT
-
+///@param[o] o_Voltage      Output voltage value
+///@return FAPI2_RC_SUCCESS if success, else error code.
 fapi2::ReturnCode
 avsVoltageRead(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
                const uint8_t i_avsBusNum,
@@ -245,16 +222,13 @@ avsVoltageRead(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
                const uint32_t i_RailSelect,
                uint32_t& o_Voltage);
 
-
 ///@brief Perform an AVS write transaction
 ///@param[i] i_target       Chip target
 ///@param[i] i_avsBusNum    AVSBus Number (0 or 1)
 ///@param[i] i_o2sBridgeNum O2S Bridge Number (0 or 1)
 ///@param[i] i_RailSelect   Rail Select (value depends on the system implementation)
 ///@param[i] i_CmdData      Defined by AVSBus spec and command dependent (16b, right justified)
-///@return   FAPI2::SUCCESS
-///@return   FAPI2::RC_PROCPM_AVSBUS_TIMEOUT
-
+///@return FAPI2_RC_SUCCESS if success, else error code.
 fapi2::ReturnCode
 avsVoltageWrite(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
                 const uint8_t i_avsBusNum,
@@ -262,13 +236,11 @@ avsVoltageWrite(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
                 const uint32_t i_RailSelect,
                 const uint32_t o_Voltage);
 
-
 ///@brief Drive an Idle Frame on an AVSBus
 ///@param[i] i_target       Chip target
 ///@param[i] i_avsBusNum    AVSBus Number (0 or 1)
 ///@param[i] i_o2sBridgeNum O2S Bridge Number (0 or 1)
-///@return   FAPI2::SUCCESS
-
+///@return FAPI2_RC_SUCCESS if success, else error code.
 fapi2::ReturnCode
 avsIdleFrame(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
              const uint8_t i_avsBusNum,
@@ -276,10 +248,11 @@ avsIdleFrame(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
 
 ///@brief Validate the AVSBUS slave response
 ///@param[i]  i_target       Chip target
-///@param[i] i_avsBusNum    AVSBus Number (0 or 1)
-///@param[i] i_o2sBridgeNum O2S Bridge Number (0 or 1)
+///@param[i] i_avsBusNum     AVSBus Number (0 or 1)
+///@param[i] i_o2sBridgeNum  O2S Bridge Number (0 or 1)
 ///@param[i] i_throwAssert   Should this routine throw an assert
-///@param[o] o_goodResponse  Was the response valid
+///@param[o] o_goodResponse  Non-zero if the response is valid
+///@return FAPI2_RC_SUCCESS if success, else error code.
 fapi2::ReturnCode
 avsValidateResponse(const fapi2::Target<fapi2::TARGET_TYPE_PROC_CHIP>& i_target,
                     const uint8_t i_avsBusNum,