Add retry support to I2CInterface
Enhance the I2CInterface class hierarchy to support retrying failed I2C
operations.
Add an optional parameter to i2c::create() to specify the maximum number
of retries to perform for an I2C operation. Default to 0 retries so
that existing code will not be affected.
Testing:
* Tested each modified function
* When I2C operation succeeds
* When I2C operation fails
* When no retries are done
* When retries are done
* When retried operation succeeds
* When retried operation fails
* See complete test plan at
https://gist.github.com/smccarney/8d203a40d9984402ac495dc3d689c12d
Signed-off-by: Shawn McCarney <shawnmm@us.ibm.com>
Change-Id: I2b0e782e5caaafa9908ef5625687377b959b38ff
diff --git a/tools/i2c/i2c.cpp b/tools/i2c/i2c.cpp
index 7a0bbf4..0dbfe9a 100644
--- a/tools/i2c/i2c.cpp
+++ b/tools/i2c/i2c.cpp
@@ -24,7 +24,13 @@
if (cachedFuncs == NO_FUNCS)
{
// Get functionality from adapter
- if (ioctl(fd, I2C_FUNCS, &cachedFuncs) < 0)
+ int ret = 0, retries = 0;
+ do
+ {
+ ret = ioctl(fd, I2C_FUNCS, &cachedFuncs);
+ } while ((ret < 0) && (++retries <= maxRetries));
+
+ if (ret < 0)
{
throw I2CException("Failed to get funcs", busStr, devAddr, errno);
}
@@ -51,7 +57,6 @@
devAddr);
}
break;
-
case I2C_SMBUS_WORD_DATA:
if (!(funcs & I2C_FUNC_SMBUS_READ_WORD_DATA))
{
@@ -98,7 +103,6 @@
devAddr);
}
break;
-
case I2C_SMBUS_WORD_DATA:
if (!(funcs & I2C_FUNC_SMBUS_WRITE_WORD_DATA))
{
@@ -121,7 +125,7 @@
}
break;
default:
- fprintf(stderr, "Unexpected read size type: %d\n", type);
+ fprintf(stderr, "Unexpected write size type: %d\n", type);
assert(false);
}
}
@@ -133,13 +137,25 @@
throw I2CException("Device already open", busStr, devAddr);
}
- fd = ::open(busStr.c_str(), O_RDWR);
+ int retries = 0;
+ do
+ {
+ fd = ::open(busStr.c_str(), O_RDWR);
+ } while ((fd == -1) && (++retries <= maxRetries));
+
if (fd == -1)
{
throw I2CException("Failed to open", busStr, devAddr, errno);
}
- if (ioctl(fd, I2C_SLAVE, devAddr) < 0)
+ retries = 0;
+ int ret = 0;
+ do
+ {
+ ret = ioctl(fd, I2C_SLAVE, devAddr);
+ } while ((ret < 0) && (++retries <= maxRetries));
+
+ if (ret < 0)
{
// Close device since setting slave address failed
closeWithoutException();
@@ -151,10 +167,18 @@
void I2CDevice::close()
{
checkIsOpen();
- if (::close(fd) == -1)
+
+ int ret = 0, retries = 0;
+ do
+ {
+ ret = ::close(fd);
+ } while ((ret == -1) && (++retries <= maxRetries));
+
+ if (ret == -1)
{
throw I2CException("Failed to close", busStr, devAddr, errno);
}
+
fd = INVALID_FD;
cachedFuncs = NO_FUNCS;
}
@@ -164,11 +188,17 @@
checkIsOpen();
checkReadFuncs(I2C_SMBUS_BYTE);
- int ret = i2c_smbus_read_byte(fd);
+ int ret = 0, retries = 0;
+ do
+ {
+ ret = i2c_smbus_read_byte(fd);
+ } while ((ret < 0) && (++retries <= maxRetries));
+
if (ret < 0)
{
throw I2CException("Failed to read byte", busStr, devAddr, errno);
}
+
data = static_cast<uint8_t>(ret);
}
@@ -177,11 +207,17 @@
checkIsOpen();
checkReadFuncs(I2C_SMBUS_BYTE_DATA);
- int ret = i2c_smbus_read_byte_data(fd, addr);
+ int ret = 0, retries = 0;
+ do
+ {
+ ret = i2c_smbus_read_byte_data(fd, addr);
+ } while ((ret < 0) && (++retries <= maxRetries));
+
if (ret < 0)
{
throw I2CException("Failed to read byte data", busStr, devAddr, errno);
}
+
data = static_cast<uint8_t>(ret);
}
@@ -189,27 +225,41 @@
{
checkIsOpen();
checkReadFuncs(I2C_SMBUS_WORD_DATA);
- int ret = i2c_smbus_read_word_data(fd, addr);
+
+ int ret = 0, retries = 0;
+ do
+ {
+ ret = i2c_smbus_read_word_data(fd, addr);
+ } while ((ret < 0) && (++retries <= maxRetries));
+
if (ret < 0)
{
throw I2CException("Failed to read word data", busStr, devAddr, errno);
}
+
data = static_cast<uint16_t>(ret);
}
void I2CDevice::read(uint8_t addr, uint8_t& size, uint8_t* data, Mode mode)
{
checkIsOpen();
- int ret = -1;
+
+ int ret = -1, retries = 0;
switch (mode)
{
case Mode::SMBUS:
checkReadFuncs(I2C_SMBUS_BLOCK_DATA);
- ret = i2c_smbus_read_block_data(fd, addr, data);
+ do
+ {
+ ret = i2c_smbus_read_block_data(fd, addr, data);
+ } while ((ret < 0) && (++retries <= maxRetries));
break;
case Mode::I2C:
checkReadFuncs(I2C_SMBUS_I2C_BLOCK_DATA);
- ret = i2c_smbus_read_i2c_block_data(fd, addr, size, data);
+ do
+ {
+ ret = i2c_smbus_read_i2c_block_data(fd, addr, size, data);
+ } while ((ret < 0) && (++retries <= maxRetries));
if (ret != size)
{
throw I2CException("Failed to read i2c block data", busStr,
@@ -217,10 +267,12 @@
}
break;
}
+
if (ret < 0)
{
throw I2CException("Failed to read block data", busStr, devAddr, errno);
}
+
size = static_cast<uint8_t>(ret);
}
@@ -229,7 +281,13 @@
checkIsOpen();
checkWriteFuncs(I2C_SMBUS_BYTE);
- if (i2c_smbus_write_byte(fd, data) < 0)
+ int ret = 0, retries = 0;
+ do
+ {
+ ret = i2c_smbus_write_byte(fd, data);
+ } while ((ret < 0) && (++retries <= maxRetries));
+
+ if (ret < 0)
{
throw I2CException("Failed to write byte", busStr, devAddr, errno);
}
@@ -240,7 +298,13 @@
checkIsOpen();
checkWriteFuncs(I2C_SMBUS_BYTE_DATA);
- if (i2c_smbus_write_byte_data(fd, addr, data) < 0)
+ int ret = 0, retries = 0;
+ do
+ {
+ ret = i2c_smbus_write_byte_data(fd, addr, data);
+ } while ((ret < 0) && (++retries <= maxRetries));
+
+ if (ret < 0)
{
throw I2CException("Failed to write byte data", busStr, devAddr, errno);
}
@@ -251,7 +315,13 @@
checkIsOpen();
checkWriteFuncs(I2C_SMBUS_WORD_DATA);
- if (i2c_smbus_write_word_data(fd, addr, data) < 0)
+ int ret = 0, retries = 0;
+ do
+ {
+ ret = i2c_smbus_write_word_data(fd, addr, data);
+ } while ((ret < 0) && (++retries <= maxRetries));
+
+ if (ret < 0)
{
throw I2CException("Failed to write word data", busStr, devAddr, errno);
}
@@ -261,18 +331,26 @@
Mode mode)
{
checkIsOpen();
- int ret = -1;
+
+ int ret = -1, retries = 0;
switch (mode)
{
case Mode::SMBUS:
checkWriteFuncs(I2C_SMBUS_BLOCK_DATA);
- ret = i2c_smbus_write_block_data(fd, addr, size, data);
+ do
+ {
+ ret = i2c_smbus_write_block_data(fd, addr, size, data);
+ } while ((ret < 0) && (++retries <= maxRetries));
break;
case Mode::I2C:
checkWriteFuncs(I2C_SMBUS_I2C_BLOCK_DATA);
- ret = i2c_smbus_write_i2c_block_data(fd, addr, size, data);
+ do
+ {
+ ret = i2c_smbus_write_i2c_block_data(fd, addr, size, data);
+ } while ((ret < 0) && (++retries <= maxRetries));
break;
}
+
if (ret < 0)
{
throw I2CException("Failed to write block data", busStr, devAddr,
@@ -281,16 +359,19 @@
}
std::unique_ptr<I2CInterface> I2CDevice::create(uint8_t busId, uint8_t devAddr,
- InitialState initialState)
+ InitialState initialState,
+ int maxRetries)
{
- std::unique_ptr<I2CDevice> dev(new I2CDevice(busId, devAddr, initialState));
+ std::unique_ptr<I2CDevice> dev(
+ new I2CDevice(busId, devAddr, initialState, maxRetries));
return dev;
}
std::unique_ptr<I2CInterface> create(uint8_t busId, uint8_t devAddr,
- I2CInterface::InitialState initialState)
+ I2CInterface::InitialState initialState,
+ int maxRetries)
{
- return I2CDevice::create(busId, devAddr, initialState);
+ return I2CDevice::create(busId, devAddr, initialState, maxRetries);
}
} // namespace i2c
diff --git a/tools/i2c/i2c.hpp b/tools/i2c/i2c.hpp
index 738547c..23acdbc 100644
--- a/tools/i2c/i2c.hpp
+++ b/tools/i2c/i2c.hpp
@@ -19,11 +19,13 @@
* @param[in] busId - The i2c bus ID
* @param[in] devAddr - The device address of the I2C device
* @param[in] initialState - Initial state of the I2CDevice object
+ * @param[in] maxRetries - Maximum number of times to retry an I2C operation
*/
explicit I2CDevice(uint8_t busId, uint8_t devAddr,
- InitialState initialState = InitialState::OPEN) :
+ InitialState initialState = InitialState::OPEN,
+ int maxRetries = 0) :
busId(busId),
- devAddr(devAddr)
+ devAddr(devAddr), maxRetries(maxRetries)
{
busStr = "/dev/i2c-" + std::to_string(busId);
if (initialState == InitialState::OPEN)
@@ -44,6 +46,9 @@
/** @brief The i2c device address in the bus */
uint8_t devAddr;
+ /** @brief Maximum number of times to retry an I2C operation */
+ int maxRetries = 0;
+
/** @brief The file descriptor of the opened i2c device */
int fd = INVALID_FD;
@@ -164,12 +169,14 @@
* @param[in] busId - The i2c bus ID
* @param[in] devAddr - The device address of the i2c
* @param[in] initialState - Initial state of the I2CInterface object
+ * @param[in] maxRetries - Maximum number of times to retry an I2C operation
*
* @return The unique_ptr holding the I2CInterface
*/
static std::unique_ptr<I2CInterface>
create(uint8_t busId, uint8_t devAddr,
- InitialState initialState = InitialState::OPEN);
+ InitialState initialState = InitialState::OPEN,
+ int maxRetries = 0);
};
} // namespace i2c
diff --git a/tools/i2c/i2c_interface.hpp b/tools/i2c/i2c_interface.hpp
index 10ee00a..77b3d7b 100644
--- a/tools/i2c/i2c_interface.hpp
+++ b/tools/i2c/i2c_interface.hpp
@@ -194,11 +194,13 @@
* @param[in] busId - The i2c bus ID
* @param[in] devAddr - The device address of the i2c
* @param[in] initialState - Initial state of the I2CInterface object
+ * @param[in] maxRetries - Maximum number of times to retry an I2C operation
*
* @return The unique_ptr holding the I2CInterface
*/
std::unique_ptr<I2CInterface> create(
uint8_t busId, uint8_t devAddr,
- I2CInterface::InitialState initialState = I2CInterface::InitialState::OPEN);
+ I2CInterface::InitialState initialState = I2CInterface::InitialState::OPEN,
+ int maxRetries = 0);
} // namespace i2c
diff --git a/tools/i2c/test/mocked_i2c_interface.cpp b/tools/i2c/test/mocked_i2c_interface.cpp
index 2fed41e..4747490 100644
--- a/tools/i2c/test/mocked_i2c_interface.cpp
+++ b/tools/i2c/test/mocked_i2c_interface.cpp
@@ -7,7 +7,7 @@
std::unique_ptr<I2CInterface>
create(uint8_t /*busId*/, uint8_t /*devAddr*/,
- I2CInterface::InitialState /*initialState*/)
+ I2CInterface::InitialState /*initialState*/, int /*maxRetries*/)
{
return std::make_unique<MockedI2CInterface>();
}