diff mbox

i2c: mux: mlxcpld: fix i2c mux selection caching

Message ID 1482006551-23108-1-git-send-email-peda@axentia.se
State Accepted
Headers show

Commit Message

Peter Rosin Dec. 17, 2016, 8:29 p.m. UTC 
smbus functions return -ve on error, 0 on success.  However,
__i2c_transfer() have a different return signature - -ve on error, or
number of buffers transferred (which may be zero or greater.)

The upshot of this is that the sense of the text is reversed when using
the mux on a bus supporting the master_xfer method: we cache the value
and never retry if we fail to transfer any buffers, but if we succeed,
we clear the cached value

Fix this by making mlxcpld_mux_reg_write() return a -ve error code for
all failure cases, just as was done in commit 7f638c1cb0a1 ("i2c: mux:
pca954x: fix i2c mux selection caching")

This also aligns the implementations of these two muxes in this area.

Signed-off-by: Peter Rosin <peda@axentia.se>
---

Hi!

I audited the other i2c muxes for similar problems after the recent regression
report for the pca954x driver [1], and found only this.

I obviously don't have the hardware to test this patch.

Anyway, this driver is probably lower profile than the pca954x, but this
still needs fixing in my opinion...

Cheers,
peda

[1] http://www.spinics.net/lists/linux-i2c/msg27586.html

 drivers/i2c/muxes/i2c-mux-mlxcpld.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

Comments

Wolfram Sang Dec. 18, 2016, 7:56 a.m. UTC | #1 
On Sat, Dec 17, 2016 at 09:29:11PM +0100, Peter Rosin wrote:
> smbus functions return -ve on error, 0 on success.  However,
> __i2c_transfer() have a different return signature - -ve on error, or
> number of buffers transferred (which may be zero or greater.)
> 
> The upshot of this is that the sense of the text is reversed when using
> the mux on a bus supporting the master_xfer method: we cache the value
> and never retry if we fail to transfer any buffers, but if we succeed,
> we clear the cached value
> 
> Fix this by making mlxcpld_mux_reg_write() return a -ve error code for
> all failure cases, just as was done in commit 7f638c1cb0a1 ("i2c: mux:
> pca954x: fix i2c mux selection caching")
> 
> This also aligns the implementations of these two muxes in this area.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>

Fixed the commit message and applied to for-current, thanks!
diff mbox

Patch

diff --git a/drivers/i2c/muxes/i2c-mux-mlxcpld.c b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
index 3ab654bbfab5..b7ca249ec9c3 100644
--- a/drivers/i2c/muxes/i2c-mux-mlxcpld.c
+++ b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
@@ -95,6 +95,7 @@  static int mlxcpld_mux_reg_write(struct i2c_adapter *adap,
 				 struct i2c_client *client, u8 val)
 {
 	struct mlxcpld_mux_plat_data *pdata = dev_get_platdata(&client->dev);
+	int ret = -ENODEV;
 
 	if (adap->algo->master_xfer) {
 		struct i2c_msg msg;
@@ -104,17 +105,21 @@  static int mlxcpld_mux_reg_write(struct i2c_adapter *adap,
 		msg.flags = 0;
 		msg.len = 2;
 		msg.buf = msgbuf;
-		return __i2c_transfer(adap, &msg, 1);
+		ret = __i2c_transfer(adap, &msg, 1);
+
+		if (ret >= 0 && ret != 1)
+			ret = -EREMOTEIO;
 	} else if (adap->algo->smbus_xfer) {
 		union i2c_smbus_data data;
 
 		data.byte = val;
-		return adap->algo->smbus_xfer(adap, client->addr,
-					      client->flags, I2C_SMBUS_WRITE,
-					      pdata->sel_reg_addr,
-					      I2C_SMBUS_BYTE_DATA, &data);
-	} else
-		return -ENODEV;
+		ret = adap->algo->smbus_xfer(adap, client->addr,
+					     client->flags, I2C_SMBUS_WRITE,
+					     pdata->sel_reg_addr,
+					     I2C_SMBUS_BYTE_DATA, &data);
+	}
+
+	return ret;
 }
 
 static int mlxcpld_mux_select_chan(struct i2c_mux_core *muxc, u32 chan)
@@ -127,10 +132,7 @@  static int mlxcpld_mux_select_chan(struct i2c_mux_core *muxc, u32 chan)
 	/* Only select the channel if its different from the last channel */
 	if (data->last_chan != regval) {
 		err = mlxcpld_mux_reg_write(muxc->parent, client, regval);
-		if (err)
-			data->last_chan = 0;
-		else
-			data->last_chan = regval;
+		data->last_chan = err < 0 ? 0 : regval;
 	}
 
 	return err;