From cf29aa3e9203d94386ddc092c7141ac4660c646f Mon Sep 17 00:00:00 2001 From: Nick Bofferding Date: Fri, 12 Apr 2019 14:45:14 -0500 Subject: 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 Reviewed-by: Matthew Raybuck Reviewed-by: Michael Baiocchi Tested-by: Jenkins Server Tested-by: Jenkins OP Build CI Tested-by: Jenkins OP HW Tested-by: FSP CI Jenkins Reviewed-by: Daniel M. Crowell --- src/usr/i2c/i2c.C | 50 ++++++++++++++++++++++--------------- src/usr/isteps/ucd/updateUcdFlash.C | 2 +- 2 files changed, 31 insertions(+), 21 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) { diff --git a/src/usr/isteps/ucd/updateUcdFlash.C b/src/usr/isteps/ucd/updateUcdFlash.C index 20bb56f46..5a261c57f 100644 --- a/src/usr/isteps/ucd/updateUcdFlash.C +++ b/src/usr/isteps/ucd/updateUcdFlash.C @@ -750,7 +750,7 @@ public: // The MFR Revision represented as ASCII characters excluding // null terminator. uint8_t str[MFR_REVISION_MAX_SIZE]; - } mfrBuf; + } mfrBuf = {0}; size = MFR_REVISION_MAX_SIZE; cmd = MFR_REVISION; -- cgit v1.2.1