[PATCH] HID: mcp2221: free I2C bus after failed transactions

["Sven Zühlsdorf" <sven.zuehlsdorf@vigem.de>]

[-- Attachment #1: Type: text/plain, Size: 2360 bytes --]

The conversion of unnecessary speed reconfigurations to actual error cleanup
has missed the timeout case in mcp_i2c_smbus_read. While we give up on
waiting for the requested data, the chip is still executing the transaction.
Attempts to initiate further transactions will be rejected with
"I2C engine busy" (translated to -EAGAIN by the driver). This can be
reproduced by attempting a read from a nonexistent device, as e.g. i2cdetect
does by default for the 0x30-0x3f and 0x50-0x5f address ranges.
Explicitly canceling the current transaction fixes this.

In addition, when initiating an I2C transaction and the chip accepts the
command, i.e. doesn't return "I2C engine busy"/-EAGAIN, we still need to
explicitly free the bus, if we detect an error condition in the HID
report's I2C engine state.

Fixes: 02a46753601a ("HID: mcp2221: Don't set bus speed on every transfer")
Signed-off-by: Sven Zühlsdorf <sven.zuehlsdorf@vigem.de>
---
  drivers/hid/hid-mcp2221.c | 19 ++++++++++++++++---
  1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
index 33603b019f975..50237b31c781c 100644
--- a/drivers/hid/hid-mcp2221.c
+++ b/drivers/hid/hid-mcp2221.c
@@ -274,8 +274,11 @@ static int mcp_i2c_write(struct mcp2221 *mcp,
  		memcpy(&mcp->txbuf[4], &msg->buf[idx], len);

  		ret = mcp_send_data_req_status(mcp, mcp->txbuf, len + 4);
-		if (ret)
+		if (ret) {
+			if (ret != -EAGAIN)
+				mcp_cancel_last_cmd(mcp);
  			return ret;
+		}

  		usleep_range(980, 1000);

@@ -332,8 +335,11 @@ static int mcp_i2c_smbus_read(struct mcp2221 *mcp,
  	}

  	ret = mcp_send_data_req_status(mcp, mcp->txbuf, 4);
-	if (ret)
+	if (ret) {
+		if (ret != -EAGAIN)
+			mcp_cancel_last_cmd(mcp);
  		return ret;
+	}

  	mcp->rxbuf_idx = 0;

@@ -353,6 +359,10 @@ static int mcp_i2c_smbus_read(struct mcp2221 *mcp,
  				usleep_range(90, 100);
  				retries++;
  			} else {
+				/* Too many retries.
+				 * Give up and free the bus.
+				 */
+				mcp_cancel_last_cmd(mcp);
  				return ret;
  			}
  		} else {
@@ -454,8 +464,11 @@ static int mcp_smbus_write(struct mcp2221 *mcp, u16 addr,
  	}

  	ret = mcp_send_data_req_status(mcp, mcp->txbuf, data_len);
-	if (ret)
+	if (ret) {
+		if (ret != -EAGAIN)
+			mcp_cancel_last_cmd(mcp);
  		return ret;
+	}

  	if (last_status) {
  		usleep_range(980, 1000);
-- 
2.52.0