diff --git a/i2c-vr/xdpe1x2xx/xdpe1x2xx.cpp b/i2c-vr/xdpe1x2xx/xdpe1x2xx.cpp
index 59c0560..25a4f26 100644
--- a/i2c-vr/xdpe1x2xx/xdpe1x2xx.cpp
+++ b/i2c-vr/xdpe1x2xx/xdpe1x2xx.cpp
@@ -15,9 +15,6 @@
 namespace phosphor::software::VR
 {
 
-const uint32_t CRC32Poly = 0xEDB88320;
-const int VRResetDelay = 500000;
-
 enum RevisionCode
 {
     REV_A = 0x00,
@@ -28,30 +25,43 @@
 
 enum ProductID
 {
-    ProductIDXDPE15284 = 0x8A,
-    ProductIDXDPE19283 = 0x95,
+    ProductIDXDPE15254 = 0x90,   // Revision C,D
+    ProductIDXDPE15284 = 0x8A,   // Revision A,B,C,D
+    ProductIDXDPE19283AC = 0x95, // Revision A,B,C
+    ProductIDXDPE19283D = 0xAE,  // Revision D
+    ProductIDXDPE192C3AC = 0x96, // Revision A,B,C
+    ProductIDXDPE192C3D = 0xAF,  // Revision D
 };
 
-const uint32_t PMBusICDeviceID = 0xAD;
-const uint32_t PMBusSTLCml = 0x7E;
-const uint32_t IFXICDeviceIDLen = 2;
-const uint32_t IFXMFRAHBAddr = 0xCE;
-const uint32_t IFXMFRRegWrite = 0xDE;
-const uint32_t IFXMFRFwCmdData = 0xFD;
-const uint32_t IFXMFRFwCmd = 0xFE;
-const uint32_t MFRFwCmdReset = 0x0e;
-const uint32_t MFRFwCmdRmng = 0x10;
-const uint32_t MFRFwCmdOTPConfSTO = 0x11;
-const uint32_t MFRFwCmdOTPFileInvd = 0x12;
-const uint32_t MFRFwCmdGetCRC = 0x2D;
-const int XDPE15284CConfSize = 1344;
-const int XDPE19283BConfSize = 1416;
-const uint32_t VRWarnRemaining = 3;
-const uint32_t SectTrim = 0x02;
+constexpr uint8_t PMBusICDeviceID = 0xAD;
+constexpr uint8_t PMBusSTLCml = 0x7E;
+constexpr uint8_t IFXICDeviceIDLen = 2;
+constexpr uint8_t IFXMFRAHBAddr = 0xCE;
+constexpr uint8_t IFXMFRRegWrite = 0xDE;
+constexpr uint8_t IFXMFRFwCmdData = 0xFD;
+constexpr uint8_t IFXMFRFwCmd = 0xFE;
+constexpr uint8_t MFRFwCmdReset = 0x0e;
+constexpr uint8_t MFRFwCmdRmng = 0x10;
+constexpr uint8_t MFRFwCmdGetHWAddress = 0x2E;
+constexpr uint8_t MFRFwCmdOTPConfSTO = 0x11;
+constexpr uint8_t MFRFwCmdOTPFileInvd = 0x12;
+constexpr uint8_t MFRFwCmdGetCRC = 0x2D;
+constexpr int XDPE15284CConfSize = 1344;
+constexpr int XDPE19283BConfSize = 1416;
+constexpr uint8_t VRWarnRemaining = 3;
+constexpr uint8_t SectTrim = 0x02;
+
+constexpr uint16_t MFRDefaultWaitTime = 20;
+constexpr uint16_t MFRGetHWAddressWaitTime = 5;
+constexpr uint16_t MFROTPFileInvalidationWaitTime = 100;
+constexpr uint16_t MFRSectionInvalidationWaitTime = 4;
+constexpr uint16_t VRResetDelay = 500;
+
+constexpr uint32_t CRC32Poly = 0xEDB88320;
 
 const char* const AddressField = "PMBus Address :";
 const char* const ChecksumField = "Checksum :";
-const char* const DataStartTag = "Configuration Data]";
+const char* const DataStartTag = "[Configuration Data]";
 const char* const DataEndTag = "[End Configuration Data]";
 const char* const DataComment = "//";
 const char* const DataXV = "XV";
@@ -63,7 +73,6 @@
 
 sdbusplus::async::task<bool> XDPE1X2XX::getDeviceId(uint8_t* deviceID)
 {
-    bool ret = false;
     uint8_t tbuf[16] = {0};
     tbuf[0] = PMBusICDeviceID;
     tbuf[1] = 2;
@@ -71,22 +80,27 @@
     uint8_t rbuf[16] = {0};
     uint8_t rSize = IFXICDeviceIDLen + 1;
 
-    ret = co_await this->i2cInterface.sendReceive(tbuf, tSize, rbuf, rSize);
-    if (!ret)
+    if (!(co_await this->i2cInterface.sendReceive(tbuf, tSize, rbuf, rSize)))
     {
         error("Failed to get device ID");
         co_return false;
     }
 
+    // According to datasheet:
+    // rbuf[1]: device revision
+    // rbuf[2]: device id
     std::memcpy(deviceID, &rbuf[1], IFXICDeviceIDLen);
+    info.deviceRev = deviceID[0];
+    info.deviceId = deviceID[1];
+    debug("VR Device ID: {ID}", "ID", lg2::hex, rbuf[2]);
+    debug("VR Device Rev: {REV}", "REV", lg2::hex, rbuf[1]);
 
     co_return true;
 }
 
-sdbusplus::async::task<bool> XDPE1X2XX::mfrFWcmd(uint8_t cmd, uint8_t* data,
-                                                 uint8_t* resp)
+sdbusplus::async::task<bool> XDPE1X2XX::mfrFWcmd(
+    uint8_t cmd, uint16_t processTime, uint8_t* data, uint8_t* resp)
 {
-    bool ret = false;
     uint8_t tBuf[16] = {0};
     uint8_t rBuf[16] = {0};
     uint8_t tSize = 0;
@@ -98,8 +112,8 @@
         tBuf[1] = 4; // Block write 4 bytes
         tSize = 6;
         std::memcpy(&tBuf[2], data, 4);
-        ret = co_await this->i2cInterface.sendReceive(tBuf, tSize, rBuf, rSize);
-        if (!ret)
+        if (!(co_await this->i2cInterface.sendReceive(tBuf, tSize, rBuf,
+                                                      rSize)))
         {
             error("Failed to send MFR command: {CMD}", "CMD",
                   std::string("IFXMFRFwCmdDAta"));
@@ -113,23 +127,23 @@
     tBuf[1] = cmd;
     tSize = 2;
     rSize = 0;
-    ret = co_await this->i2cInterface.sendReceive(tBuf, tSize, rBuf, rSize);
-    if (!ret)
+    if (!(co_await this->i2cInterface.sendReceive(tBuf, tSize, rBuf, rSize)))
     {
         error("Failed to send MFR command: {CMD}", "CMD",
               std::string("IFXMFRFwCmd"));
         co_return false;
     }
 
-    co_await sdbusplus::async::sleep_for(ctx, std::chrono::microseconds(20000));
+    co_await sdbusplus::async::sleep_for(
+        ctx, std::chrono::milliseconds(processTime));
 
     if (resp)
     {
         tBuf[0] = IFXMFRFwCmdData;
         tSize = 1;
         rSize = 6;
-        ret = co_await this->i2cInterface.sendReceive(tBuf, tSize, rBuf, rSize);
-        if (!ret)
+        if (!(co_await this->i2cInterface.sendReceive(tBuf, tSize, rBuf,
+                                                      rSize)))
         {
             error("Failed to send MFR command: {CMD}", "CMD",
                   std::string("IFXMFRFwCmdData"));
@@ -149,20 +163,20 @@
 
 sdbusplus::async::task<bool> XDPE1X2XX::getRemainingWrites(uint8_t* remain)
 {
-    bool ret = false;
+    // According to datasheet:
+    // remaingin OTP size = rBuf[0] + 256 * rBuf[1]
     uint8_t tBuf[16] = {0};
     uint8_t rBuf[16] = {0};
     uint8_t devId[2] = {0};
 
-    ret = co_await this->mfrFWcmd(MFRFwCmdRmng, tBuf, rBuf);
-    if (!ret)
+    if (!(co_await this->mfrFWcmd(MFRFwCmdRmng, MFRDefaultWaitTime, tBuf,
+                                  rBuf)))
     {
         error("Failed to request remaining writes");
         co_return false;
     }
 
-    ret = co_await this->getDeviceId(devId);
-    if (!ret)
+    if (!(co_await this->getDeviceId(devId)))
     {
         error("Failed to request device ID for remaining writes");
         co_return false;
@@ -177,7 +191,7 @@
 
     *remain = REMAINING_TIMES(rBuf, configSize);
 
-    co_return 0;
+    co_return true;
 }
 
 int XDPE1X2XX::getConfigSize(uint8_t deviceId, uint8_t revision)
@@ -186,7 +200,7 @@
 
     switch (deviceId)
     {
-        case ProductIDXDPE19283:
+        case ProductIDXDPE19283AC:
             if (revision == REV_B)
             {
                 size = XDPE19283BConfSize;
@@ -210,8 +224,8 @@
     uint8_t tBuf[16] = {0};
     uint8_t rBuf[16] = {0};
 
-    bool ret = co_await this->mfrFWcmd(MFRFwCmdGetCRC, tBuf, rBuf);
-    if (!ret)
+    if (!(co_await this->mfrFWcmd(MFRFwCmdGetCRC, MFRDefaultWaitTime, tBuf,
+                                  rBuf)))
     {
         error("Failed to get CRC value");
         co_return false;
@@ -226,9 +240,7 @@
 }
 
 sdbusplus::async::task<bool> XDPE1X2XX::program(bool force)
-
 {
-    bool ret = false;
     uint8_t tBuf[16] = {0};
     uint8_t rBuf[16] = {0};
     uint8_t remain = 0;
@@ -236,14 +248,16 @@
     int size = 0;
 
     // NOLINTBEGIN(clang-analyzer-core.uninitialized.Branch)
-    ret = co_await getCRC(&sum);
+    if (!(co_await getCRC(&sum)))
     // NOLINTEND(clang-analyzer-core.uninitialized.Branch)
-    if (!ret)
     {
         error("Failed to program the VR");
         co_return -1;
     }
 
+    debug("CRC before programming: {CRC}", "CRC", lg2::hex, sum);
+    debug("CRC of configuration: {CRC}", "CRC", lg2::hex, configuration.sumExp);
+
     if (!force && (sum == configuration.sumExp))
     {
         error("Failed to program the VR - CRC value are equal with no force");
@@ -251,14 +265,15 @@
     }
 
     // NOLINTBEGIN(clang-analyzer-core.uninitialized.Branch)
-    ret = co_await this->getRemainingWrites(&remain);
+    if (!(co_await this->getRemainingWrites(&remain)))
     // NOLINTEND(clang-analyzer-core.uninitialized.Branch)
-    if (!ret)
     {
         error("Failed to program the VR - unable to obtain remaing writes");
         co_return -1;
     }
 
+    debug("Remaining write cycles of VR: {REMAIN}", "REMAIN", remain);
+
     if (!remain)
     {
         error("Failed to program the VR - no remaining write cycles left");
@@ -278,80 +293,87 @@
     // 0xfe 0xfe 0x00 0x00 instructs the command to reprogram all header codes
     // and XVcode. If the old sections are not invalidated in OTP, they can
     // affect the CRC calculation.
+    debug("Invalidate current Configuration");
 
     tBuf[0] = 0xfe;
     tBuf[1] = 0xfe;
     tBuf[2] = 0x00;
     tBuf[3] = 0x00;
 
-    ret = co_await this->mfrFWcmd(MFRFwCmdOTPFileInvd, tBuf, NULL);
-    if (!ret)
+    if (!(co_await this->mfrFWcmd(MFRFwCmdOTPFileInvd,
+                                  MFROTPFileInvalidationWaitTime, tBuf, NULL)))
     {
-        error("Failed to program the VR - Invalidation of currect FW");
-        co_return ret;
+        error("Failed to program the VR - Invalidation of current FW");
+        co_return false;
     }
 
-    co_await sdbusplus::async::sleep_for(ctx,
-                                         std::chrono::microseconds(500000));
-
     for (int i = 0; i < configuration.sectCnt; i++)
     {
+        debug("Programming section: {SEC}", "SEC", i);
         struct configSect* sect = &configuration.section[i];
         if (sect == NULL)
         {
             error(
                 "Failed to program the VR - unexpected NULL section in config");
-            ret = -1;
-            break;
+            co_return false;
         }
 
         if ((i <= 0) || (sect->type != configuration.section[i - 1].type))
         {
+            debug("Section Type: {TYPE}", "TYPE", lg2::hex,
+                  configuration.section[i].type);
+
+            // clear bit0 of PMBUS_STS_CML
             tBuf[0] = PMBusSTLCml;
             tBuf[1] = 0x1;
             uint8_t tSize = 2;
             uint8_t rSize = 0;
-            ret = co_await this->i2cInterface.sendReceive(tBuf, tSize, rBuf,
-                                                          rSize);
-            if (!ret)
+            if (!(co_await this->i2cInterface.sendReceive(tBuf, tSize, rBuf,
+                                                          rSize)))
             {
                 error("Failed to program the VR on sendReceive {CMD}", "CMD",
                       std::string("PMBusSTLCml"));
-                break;
+                co_return false;
             }
 
+            debug("Invalidating section type: {TYPE}", "TYPE", sect->type);
             tBuf[0] = sect->type;
             tBuf[1] = 0x00;
             tBuf[2] = 0x00;
             tBuf[3] = 0x00;
 
-            ret = co_await this->mfrFWcmd(MFRFwCmdOTPFileInvd, tBuf, NULL);
-            if (!ret)
+            if (!(co_await this->mfrFWcmd(MFRFwCmdOTPFileInvd,
+                                          MFRSectionInvalidationWaitTime, tBuf,
+                                          NULL)))
             {
                 error("Failed to program VR on mfrFWCmd on {CMD}", "CMD",
                       std::string("MFRFwCmdOTPFileInvd"));
-                break;
+                co_return false;
             }
 
-            co_await sdbusplus::async::sleep_for(
-                ctx, std::chrono::microseconds(10000)); // Write delay
+            // set scratchpad addr
+            // XDPE192XX Rev A/B: 0x2005e000
+            //           Rev C  : 0x2005e400
+            //           Rev D  : 0x2005f000
+
+            debug("Setting scratchpad address: {ADDR}", "ADDR", lg2::hex,
+                  info.scratchPadAddress);
 
             tBuf[0] = IFXMFRAHBAddr;
             tBuf[1] = 4;
-            tBuf[2] = 0x00;
-            tBuf[3] = 0xe0;
-            tBuf[4] = 0x05;
-            tBuf[5] = 0x20;
+            tBuf[2] = (info.scratchPadAddress) & 0xFF;
+            tBuf[3] = (info.scratchPadAddress >> 8) & 0xFF;
+            tBuf[4] = (info.scratchPadAddress >> 16) & 0xFF;
+            tBuf[5] = (info.scratchPadAddress >> 24) & 0xFF;
             tSize = 6;
             rSize = 0;
 
-            ret = co_await this->i2cInterface.sendReceive(tBuf, tSize, rBuf,
-                                                          rSize);
-            if (!ret)
+            if (!(co_await this->i2cInterface.sendReceive(tBuf, tSize, rBuf,
+                                                          rSize)))
             {
                 error("Failed to program VR on sendReceive on {CMD}", "CMD",
                       std::string("IFXMFRAHBAddr"));
-                break;
+                co_return false;
             }
 
             co_await sdbusplus::async::sleep_for(
@@ -367,71 +389,58 @@
             uint8_t tSize = 6;
             uint8_t rSize = 0;
             memcpy(&tBuf[2], &sect->data[j], 4);
-            ret = co_await this->i2cInterface.sendReceive(tBuf, tSize, rBuf,
-                                                          rSize);
-            if (!ret)
+            if (!(co_await this->i2cInterface.sendReceive(tBuf, tSize, rBuf,
+                                                          rSize)))
             {
                 error("Failed to program the VR on sendReceive {CMD}", "CMD",
                       std::string("IFXMFRRegWrite"));
-                break;
+                co_return false;
             }
-            co_await sdbusplus::async::sleep_for(
-                ctx, std::chrono::microseconds(10000));
-        }
-        if (ret)
-        {
-            break;
+            co_await sdbusplus::async::sleep_for(ctx,
+                                                 std::chrono::milliseconds(10));
         }
 
         size += sect->dataCnt * 4;
         if ((i + 1 >= configuration.sectCnt) ||
             (sect->type != configuration.section[i + 1].type))
         {
+            // wait for programming soak (2ms/byte, at least 200ms)
+            // ex: Config (604 bytes): (604 / 50) + 2 = 14 (1400 ms)
+            uint16_t soakTime = 100 * ((size / 50) + 2);
+
             // Upload to scratchpad
+            debug("Upload from scratch pad to OTP with soak time: {TIME}ms",
+                  "TIME", soakTime);
             std::memcpy(tBuf, &size, 2);
             tBuf[2] = 0x00;
             tBuf[3] = 0x00;
-            bool ret = co_await this->mfrFWcmd(MFRFwCmdOTPConfSTO, tBuf, NULL);
-            if (ret)
+            if (!(co_await this->mfrFWcmd(MFRFwCmdOTPConfSTO, soakTime, tBuf,
+                                          NULL)))
             {
                 error("Failed to program the VR on mfrFWcmd {CMD}", "CMD",
                       std::string("MFRFwCmdOTPConfSTO"));
-                break;
+                co_return false;
             }
 
-            // wait for programming soak (2ms/byte, at least 200ms)
-            // ex: Config (604 bytes): (604 / 50) + 2 = 14 (1400 ms)
-            size = (size / 50) + 2;
-            for (int j = 0; j < size; j++)
-            {
-                co_await sdbusplus::async::sleep_for(
-                    ctx, std::chrono::microseconds(100000));
-            }
-
+            // Read status faults after programming
             tBuf[0] = PMBusSTLCml;
             uint8_t tSize = 1;
             uint8_t rSize = 1;
-            ret = co_await this->i2cInterface.sendReceive(rBuf, tSize, tBuf,
-                                                          rSize);
-            if (!ret)
+            if (!(co_await this->i2cInterface.sendReceive(rBuf, tSize, tBuf,
+                                                          rSize)))
             {
                 error("Failed to program VR on sendReceive {CMD}", "CMD",
                       std::string("PMBusSTLCml"));
-                break;
+                co_return false;
             }
             if (rBuf[0] & 0x01)
             {
-                error("Failed to program VR - response code invalid");
-                break;
+                error("Failed to program VR - status fault indicated error");
+                co_return false;
             }
         }
     }
 
-    if (!ret)
-    {
-        co_return false;
-    }
-
     co_return true;
 }
 
@@ -454,7 +463,7 @@
     return size;
 }
 
-int XDPE1X2XX::parseImage(const uint8_t* image, size_t image_size)
+bool XDPE1X2XX::parseImage(const uint8_t* image, size_t image_size)
 {
     size_t lenEndTag = strlen(DataEndTag);
     size_t lenStartTag = strlen(DataStartTag);
@@ -522,7 +531,7 @@
                         }
                         if ((++sectIndex) >= MaxSectCnt)
                         {
-                            return -1;
+                            return false;
                         }
 
                         configuration.section[sectIndex].type = sectType;
@@ -532,7 +541,7 @@
 
                     if (dataCnt >= MaxSectDataCnt)
                     {
-                        return -1;
+                        return false;
                     }
 
                     configuration.section[sectIndex].data[dataCnt++] = dWord;
@@ -574,10 +583,10 @@
         }
     }
 
-    return 0;
+    return true;
 }
 
-int XDPE1X2XX::checkImage()
+bool XDPE1X2XX::checkImage()
 {
     uint8_t i;
     uint32_t crc;
@@ -589,14 +598,14 @@
         if (sect == NULL)
         {
             error("Failed to check image - unexpected NULL section");
-            return -1;
+            return false;
         }
 
-        crc = calcCRC32(&sect->data[2], 2);
+        crc = calcCRC32(&sect->data[0], 2);
         if (crc != sect->data[2])
         {
             error("Failed to check image - first CRC value mismatch");
-            return -1;
+            return false;
         }
         sum += crc;
 
@@ -605,30 +614,32 @@
         if (crc != sect->data[sect->dataCnt - 1])
         {
             error("Failed to check image - second CRC value mismatch");
-            return -1;
+            return false;
         }
         sum += crc;
     }
 
     if (sum != configuration.sumExp)
     {
+        debug("Calculated CRC: {CRC}", "CRC", lg2::hex, sum);
+        debug("Config CRC {CRC}", "CRC", lg2::hex, configuration.sumExp);
         error("Failed to check image - third CRC value mismatch");
-        return -1;
+        return false;
     }
 
-    return 0;
+    return true;
 }
 
 sdbusplus::async::task<bool> XDPE1X2XX::verifyImage(const uint8_t* image,
                                                     size_t imageSize)
 {
-    if (parseImage(image, imageSize) < 0)
+    if (!parseImage(image, imageSize))
     {
         error("Failed to update firmware on parsing Image");
         co_return false;
     }
 
-    if (checkImage() < 0)
+    if (!checkImage())
     {
         error("Failed to update firmware on check image");
         co_return false;
@@ -637,17 +648,58 @@
     co_return true;
 }
 
-sdbusplus::async::task<bool> XDPE1X2XX::updateFirmware(bool force)
+sdbusplus::async::task<bool> XDPE1X2XX::getScratchPadAddress()
 {
-    // NOLINTBEGIN(clang-analyzer-core.uninitialized.Branch)
-    bool ret = co_await program(force);
-    // NOLINTEND(clang-analyzer-core.uninitialized.Branch)
-    if (!ret)
+    uint8_t tbuf[16] = {0};
+    uint8_t rbuf[16] = {0};
+
+    tbuf[0] = 0x02;
+    tbuf[1] = 0x00;
+    tbuf[2] = 0x00;
+    tbuf[3] = 0x00;
+
+    if (!(co_await mfrFWcmd(MFRFwCmdGetHWAddress, MFRGetHWAddressWaitTime, tbuf,
+                            rbuf)))
     {
-        error("Failed to update firmware on program");
+        error("mfrFWcmd call failed to retrieve scratchpad address");
         co_return false;
     }
 
+    info.scratchPadAddress = (static_cast<uint32_t>(rbuf[3]) << 24) |
+                             (static_cast<uint32_t>(rbuf[2]) << 16) |
+                             (static_cast<uint32_t>(rbuf[1]) << 8) |
+                             (static_cast<uint32_t>(rbuf[0]));
+
+    debug("Scratchpad Address: {ADDR}", "ADDR", lg2::hex,
+          info.scratchPadAddress);
+
+    co_return true;
+}
+
+sdbusplus::async::task<bool> XDPE1X2XX::updateFirmware(bool force)
+{
+    bool ret = true;
+    if (!(co_await getScratchPadAddress()))
+    {
+        error("Failed to retrieve scratchpad address");
+        co_return false;
+    }
+
+    // NOLINTBEGIN(clang-analyzer-core.uninitialized.Branch)
+    ret = co_await program(force);
+    if (!ret)
+    // NOLINTEND(clang-analyzer-core.uninitialized.Branch)
+    {
+        error("Failed to update firmware on program");
+    }
+
+    info.deviceId = 0;
+    info.deviceRev = 0;
+    info.remainingWrites = 0;
+    info.scratchPadAddress = 0;
+    info.actualCRC = 0;
+    info.configSize = 0;
+
     // Reset the configuration
     configuration.addr = 0;
     configuration.totalCnt = 0;
@@ -663,21 +715,22 @@
         }
     }
 
+    if (!ret)
+    {
+        co_return false;
+    }
+
     co_return true;
 }
 
 sdbusplus::async::task<bool> XDPE1X2XX::reset()
 {
-    bool ret = co_await mfrFWcmd(MFRFwCmdReset, NULL, NULL);
-    if (!ret)
+    if (!(co_await mfrFWcmd(MFRFwCmdReset, VRResetDelay, NULL, NULL)))
     {
         error("Failed to reset the VR");
         co_return false;
     }
 
-    co_await sdbusplus::async::sleep_for(
-        ctx, std::chrono::microseconds(VRResetDelay));
-
     co_return true;
 }