commit ee55117464c86f0fe4483cd86f82da0501e896e1
author Kevin Tung <kevin.tung@quantatw.com> Fri Aug 22 16:49:40 2025 +0800
committer Kevin Tung <kevin.tung.openbmc@gmail.com> Thu Sep 18 01:56:56 2025 +0000
tree b08b454f9d95e2bb984b54172b502c127f088ac5
parent 5f1346aff2a3f7bf2cec0cea7e24b9f9acb3a8f2

i2cvr: restrict apply time to OnReset

Applying VR firmware updates with Immediate ApplyTime may trigger
power fluctuations in a running system and lead to unstable or
abnormal behavior. To ensure safe updates, the ApplyTime for VR
devices is restricted to OnReset. This guarantees updates occur
only during a system reset, when power rails are stable.

Change-Id: If7e4bc5d8fb5bb187b3baacd9e6f782f41738bcd
Signed-off-by: Kevin Tung <kevin.tung.openbmc@gmail.com>

i2c-vr/isl69269/isl69269.cpp

diff --git a/i2c-vr/isl69269/isl69269.cpp b/i2c-vr/isl69269/isl69269.cpp
index dc9cd9e..42fb925 100644
--- a/i2c-vr/isl69269/isl69269.cpp
+++ b/i2c-vr/isl69269/isl69269.cpp
@@ -98,6 +98,12 @@
 
 sdbusplus::async::task<bool> ISL69269::dmaReadWrite(uint8_t* reg, uint8_t* resp)
 {
+    if (reg == nullptr || resp == nullptr)
+    {
+        error("dmaReadWrite invalid input");
+        co_return false;
+    }
+
     uint8_t tbuf[defaultBufferSize] = {0};
     uint8_t tlen = threeByteLen;
     uint8_t rbuf[defaultBufferSize] = {0};
@@ -166,6 +172,12 @@
 
 sdbusplus::async::task<bool> ISL69269::getDeviceId(uint32_t* deviceId)
 {
+    if (deviceId == nullptr)
+    {
+        error("getDeviceId invalid input");
+        co_return false;
+    }
+
     uint8_t tbuf[defaultBufferSize] = {0};
     uint8_t tLen = oneByteLen;
     uint8_t rbuf[defaultBufferSize] = {0};
@@ -186,6 +198,12 @@
 
 sdbusplus::async::task<bool> ISL69269::getDeviceRevision(uint32_t* revision)
 {
+    if (revision == nullptr)
+    {
+        error("getDeviceRevision invalid input");
+        co_return false;
+    }
+
     uint8_t tbuf[defaultBufferSize] = {0};
     uint8_t tlen = oneByteLen;
     uint8_t rbuf[defaultBufferSize] = {0};
@@ -383,6 +401,12 @@
         }
     }
 
+    if (!(co_await getProgStatus()))
+    {
+        error("program failed at getProgStatus");
+        co_return false;
+    }
+
     co_return true;
 }
 
@@ -576,46 +600,6 @@
     co_return true;
 }
 
-sdbusplus::async::task<bool> ISL69269::reset()
-{
-    bool ret = true;
-    // NOLINTBEGIN(clang-analyzer-core.uninitialized.Branch)
-    ret = co_await getProgStatus();
-    // NOLINTEND(clang-analyzer-core.uninitialized.Branch)
-    if (!ret)
-    {
-        error("reset failed at getProgStatus");
-    }
-
-    ret = co_await restoreCfg();
-    if (!ret)
-    {
-        error("reset failed at restoreCfg");
-    }
-
-    // Reset configuration for next update.
-    configuration.addr = 0;
-    configuration.mode = 0;
-    configuration.cfgId = 0;
-    configuration.devIdExp = 0;
-    configuration.devRevExp = 0;
-    configuration.crcExp = 0x00;
-    for (int i = 0; i < configuration.wrCnt; i++)
-    {
-        configuration.pData[i].pec = 0x00;
-        configuration.pData[i].addr = 0x00;
-        configuration.pData[i].cmd = 0x00;
-        for (int j = 0; j < configuration.pData[i].len; j++)
-        {
-            configuration.pData[i].data[j] = 0x00;
-        }
-        configuration.pData[i].len = 0x00;
-    }
-    configuration.wrCnt = 0;
-
-    co_return ret;
-}
-
 bool ISL69269::forcedUpdateAllowed()
 {
     return true;
@@ -624,7 +608,9 @@
 sdbusplus::async::task<bool> ISL69269::updateFirmware(bool force)
 {
     (void)force;
+    // NOLINTBEGIN(clang-analyzer-core.uninitialized.Branch)
     if (!(co_await program()))
+    // NOLINTEND(clang-analyzer-core.uninitialized.Branch)
     {
         error("programing ISL69269 failed");
         co_return false;

i2c-vr/isl69269/isl69269.hpp

diff --git a/i2c-vr/isl69269/isl69269.hpp b/i2c-vr/isl69269/isl69269.hpp
index 986c65d..789bc34 100644
--- a/i2c-vr/isl69269/isl69269.hpp
+++ b/i2c-vr/isl69269/isl69269.hpp
@@ -20,7 +20,6 @@
 
     sdbusplus::async::task<bool> updateFirmware(bool force) final;
     sdbusplus::async::task<bool> getCRC(uint32_t* checksum) final;
-    sdbusplus::async::task<bool> reset() final;
 
     bool forcedUpdateAllowed() final;