From: Hamish Martin hamish.martin@alliedtelesis.co.nz
[ Upstream commit 02a46753601a24e1673d9c28173121055e8e6cc9 ]
Since the initial commit of this driver the I2C bus speed has been reconfigured for every single transfer. This is despite the fact that we never change the speed and it is never "lost" by the chip. Upon investigation we find that what was really happening was that the setting of the bus speed had the side effect of cancelling a previous failed command if there was one, thereby freeing the bus. This is the part that was actually required to keep the bus operational in the face of failed commands.
Instead of always setting the speed, we now correctly cancel any failed commands as they are detected. This means we can just set the bus speed at probe time and remove the previous speed sets on each transfer. This has the effect of improving performance and reducing the number of commands required to complete transfers.
Fixes: 67a95c21463d ("HID: mcp2221: add usb to i2c-smbus host bridge") Signed-off-by: Hamish Martin hamish.martin@alliedtelesis.co.nz Signed-off-by: Jiri Kosina jkosina@suse.cz [romain.sioen@microchip.com: backport to stable, up to 6.8. Add "Fixes" tag] Signed-off-by: Romain Sioen romain.sioen@microchip.com --- drivers/hid/hid-mcp2221.c | 41 ++++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 14 deletions(-)
diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c index c5bfca8ac5e6..ce4e8e6b3d86 100644 --- a/drivers/hid/hid-mcp2221.c +++ b/drivers/hid/hid-mcp2221.c @@ -187,6 +187,25 @@ static int mcp_cancel_last_cmd(struct mcp2221 *mcp) return mcp_send_data_req_status(mcp, mcp->txbuf, 8); }
+/* Check if the last command succeeded or failed and return the result. + * If the command did fail, cancel that command which will free the i2c bus. + */ +static int mcp_chk_last_cmd_status_free_bus(struct mcp2221 *mcp) +{ + int ret; + + ret = mcp_chk_last_cmd_status(mcp); + if (ret) { + /* The last command was a failure. + * Send a cancel which will also free the bus. + */ + usleep_range(980, 1000); + mcp_cancel_last_cmd(mcp); + } + + return ret; +} + static int mcp_set_i2c_speed(struct mcp2221 *mcp) { int ret; @@ -241,7 +260,7 @@ static int mcp_i2c_write(struct mcp2221 *mcp, usleep_range(980, 1000);
if (last_status) { - ret = mcp_chk_last_cmd_status(mcp); + ret = mcp_chk_last_cmd_status_free_bus(mcp); if (ret) return ret; } @@ -308,7 +327,7 @@ static int mcp_i2c_smbus_read(struct mcp2221 *mcp, if (ret) return ret;
- ret = mcp_chk_last_cmd_status(mcp); + ret = mcp_chk_last_cmd_status_free_bus(mcp); if (ret) return ret;
@@ -328,11 +347,6 @@ static int mcp_i2c_xfer(struct i2c_adapter *adapter,
mutex_lock(&mcp->lock);
- /* Setting speed before every transaction is required for mcp2221 */ - ret = mcp_set_i2c_speed(mcp); - if (ret) - goto exit; - if (num == 1) { if (msgs->flags & I2C_M_RD) { ret = mcp_i2c_smbus_read(mcp, msgs, MCP2221_I2C_RD_DATA, @@ -417,9 +431,7 @@ static int mcp_smbus_write(struct mcp2221 *mcp, u16 addr, if (last_status) { usleep_range(980, 1000);
- ret = mcp_chk_last_cmd_status(mcp); - if (ret) - return ret; + ret = mcp_chk_last_cmd_status_free_bus(mcp); }
return ret; @@ -437,10 +449,6 @@ static int mcp_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
mutex_lock(&mcp->lock);
- ret = mcp_set_i2c_speed(mcp); - if (ret) - goto exit; - switch (size) {
case I2C_SMBUS_QUICK: @@ -1152,6 +1160,11 @@ static int mcp2221_probe(struct hid_device *hdev, if (i2c_clk_freq < 50) i2c_clk_freq = 50; mcp->cur_i2c_clk_div = (12000000 / (i2c_clk_freq * 1000)) - 3; + ret = mcp_set_i2c_speed(mcp); + if (ret) { + hid_err(hdev, "can't set i2c speed: %d\n", ret); + return ret; + }
mcp->adapter.owner = THIS_MODULE; mcp->adapter.class = I2C_CLASS_HWMON;