6.1-stable review patch. If anyone has any objections, please let me know.
------------------
From: Hamish Martin hamish.martin@alliedtelesis.co.nz
commit 02a46753601a24e1673d9c28173121055e8e6cc9 upstream.
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.
Signed-off-by: Hamish Martin hamish.martin@alliedtelesis.co.nz Signed-off-by: Jiri Kosina jkosina@suse.cz Fixes: 67a95c21463d ("HID: mcp2221: add usb to i2c-smbus host bridge") [romain.sioen@microchip.com: backport to stable, up to 6.8. Add "Fixes" tag] Signed-off-by: Romain Sioen romain.sioen@microchip.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- drivers/hid/hid-mcp2221.c | 41 +++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-)
--- a/drivers/hid/hid-mcp2221.c +++ b/drivers/hid/hid-mcp2221.c @@ -169,6 +169,25 @@ static int mcp_cancel_last_cmd(struct mc 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; @@ -223,7 +242,7 @@ static int mcp_i2c_write(struct mcp2221 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; } @@ -290,7 +309,7 @@ static int mcp_i2c_smbus_read(struct 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;
@@ -310,11 +329,6 @@ static int mcp_i2c_xfer(struct i2c_adapt
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, @@ -399,9 +413,7 @@ static int mcp_smbus_write(struct mcp222 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; @@ -419,10 +431,6 @@ static int mcp_smbus_xfer(struct i2c_ada
mutex_lock(&mcp->lock);
- ret = mcp_set_i2c_speed(mcp); - if (ret) - goto exit; - switch (size) {
case I2C_SMBUS_QUICK: @@ -870,6 +878,11 @@ static int mcp2221_probe(struct hid_devi 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;