Fix UCD invalid command error for 0 byte block read SMBus requests

If a UCD device had a legitimate 0-sized reponse to a block read request,
this set off a bug in the code where the i2c driver would issue a 0 byte
read request without a start, stop, or address bit, which causes the i2c
engine to throw a command invalid error.  This commit fixes the logic so that
in that condition, the driver either skips the offending read (for cases where
a PEC byte follows), or completes the transaction with a stop bit (for
no PEC byte case)

Change-Id: I64011b4e540120c8547fa3ca84a4dd68f45b479a
CQ: SW462736
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/75940
Reviewed-by: Ilya Smirnov <ismirno@us.ibm.com>
Reviewed-by: Matthew Raybuck <matthew.raybuck@ibm.com>
Reviewed-by: Michael Baiocchi <mbaiocch@us.ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>

1 files changed, 30 insertions, 20 deletions

diff --git a/src/usr/i2c/i2c.C b/src/usr/i2c/i2c.C
index 35e5fccf8..ff8629010 100755
--- a/src/usr/i2c/i2c.C
+++ b/src/usr/i2c/i2c.C
@@ -1932,32 +1932,42 @@ errlHndl_t i2cCommonOp( DeviceFW::OperationType i_opType,
                    "Block count read size mismatch; expected %d but got %d",
                    blockCountSizeExp,blockCountSize);
 
+            // From this point onwards, we don't want a start bit or address to
+            // be sent
+            i_args.with_start     = false;
+            i_args.with_address   = false;
+
             // Now read the specified number of data bytes.
             // If there is no PEC byte, complete the transaction with a stop
             // and inform the engine there is no subsequent read.  If the PEC
-            // byte is supported, withhold the stop inform the engine there is
-            // an additional read transaction coming.  Since this is a
+            // byte is supported, withhold the stop to inform the engine there
+            // is an additional read transaction coming.  Since this is a
             // continuation of the read, withhold the start bit and the address.
-            // nor is an address.
-            i_args.with_start     = false;
-            i_args.with_address   = false;
-            i_args.with_stop      = i_args.smbus.usePec ? false : true;
-            i_args.read_continue  = i_args.smbus.usePec ? true : false;
-
-            size_t dataBytesSize = blockRead.blockCount;
-            const auto dataBytesSizeExp = dataBytesSize;
-            err = i2cRead(i_target,
-                          blockRead.dataBytes,
-                          dataBytesSize,
-                          i_args);
-            if(err)
+            // If the data count was zero, we -have- to
+            // skip the read unless there is no PEC byte (and thus a stop),
+            // because a transaction without a stop/start/address of 0 length
+            // will otherwise trigger a command invalid condition from the I2C
+            // engine.
+            if(blockRead.blockCount || !i_args.smbus.usePec)
             {
-                break;
-            }
+                i_args.with_stop      = i_args.smbus.usePec ? false : true;
+                i_args.read_continue  = i_args.smbus.usePec ? true : false;
 
-            assert(dataBytesSize == dataBytesSizeExp,
-                   "Data bytes read size mismatch; expected %d but got %d",
-                   dataBytesSizeExp,dataBytesSize);
+                size_t dataBytesSize = blockRead.blockCount;
+                const auto dataBytesSizeExp = dataBytesSize;
+                err = i2cRead(i_target,
+                              blockRead.dataBytes,
+                              dataBytesSize,
+                              i_args);
+                if(err)
+                {
+                    break;
+                }
+
+                assert(dataBytesSize == dataBytesSizeExp,
+                       "Data bytes read size mismatch; expected %d but got %d",
+                       dataBytesSizeExp,dataBytesSize);
+            }
 
             if(i_args.smbus.usePec)
             {