[PATCH 1/3] i2c: mux: pca954x: Track unknown channel state

[Bart Van Assche]

pca954x sets last_chan to 0 (disconnected) if initializing, selecting
or disconnecting the mux fails. A failed write may have reached the
mux, e.g. if only the ACK was lost, so the mux state is unknown after
such a failure. Since idle_state_store() skips the disconnect write if
last_chan is 0 and the new idle state is MUX_IDLE_DISCONNECT, a channel
may remain connected after the idle state has been set to disconnect.

Set last_chan to -1 (unknown) after any failed control register write
so that idle_state_store() no longer mistakes an unknown mux state for
disconnected. Make the last_chan test in idle_state_store() explicit
since last_chan is no longer a plain register value. Make
pca954x_init() use a local variable for the register value so that
last_chan is only updated once the outcome of the register write is
known.

Fixes: f1fb64b04bf4 ("i2c: mux: pca954x: allow management of device idle state via sysfs")
Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 8fca709ed279..e34b4668a1fa 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -107,7 +107,7 @@ struct chip_desc {
 struct pca954x {
 	const struct chip_desc *chip;
 
-	u8 last_chan;		/* last register value */
+	s16 last_chan;		/* last register value, or -1 if unknown */
 	/* MUX_IDLE_AS_IS, MUX_IDLE_DISCONNECT or >= 0 for channel */
 	s32 idle_state;
 
@@ -327,7 +327,7 @@ static int pca954x_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) {
 		ret = pca954x_reg_write(muxc->parent, client, regval);
-		data->last_chan = ret < 0 ? 0 : regval;
+		data->last_chan = ret < 0 ? -1 : regval;
 	}
 
 	return ret;
@@ -338,6 +338,7 @@ static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
 	struct pca954x *data = i2c_mux_priv(muxc);
 	struct i2c_client *client = data->client;
 	s32 idle_state;
+	int ret;
 
 	idle_state = READ_ONCE(data->idle_state);
 	if (idle_state >= 0)
@@ -346,9 +347,9 @@ static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
 
 	if (idle_state == MUX_IDLE_DISCONNECT) {
 		/* Deselect active channel */
-		data->last_chan = 0;
-		return pca954x_reg_write(muxc->parent, client,
-					 data->last_chan);
+		ret = pca954x_reg_write(muxc->parent, client, 0);
+		data->last_chan = ret < 0 ? -1 : 0;
+		return ret;
 	}
 
 	/* otherwise leave as-is */
@@ -392,7 +393,7 @@ static ssize_t idle_state_store(struct device *dev,
 	 * Set the mux into a state consistent with the new
 	 * idle_state.
 	 */
-	if (data->last_chan || val != MUX_IDLE_DISCONNECT)
+	if (data->last_chan != 0 || val != MUX_IDLE_DISCONNECT)
 		ret = pca954x_deselect_mux(muxc, 0);
 
 	i2c_unlock_bus(muxc->parent, I2C_LOCK_SEGMENT);
@@ -480,12 +481,13 @@ static void pca954x_cleanup(struct i2c_mux_core *muxc)
 
 static int pca954x_init(struct i2c_client *client, struct pca954x *data)
 {
+	u8 regval;
 	int ret;
 
 	if (data->idle_state >= 0)
-		data->last_chan = pca954x_regval(data, data->idle_state);
+		regval = pca954x_regval(data, data->idle_state);
 	else
-		data->last_chan = 0; /* Disconnect multiplexer */
+		regval = 0; /* Disconnect multiplexer */
 
 	if (device_is_compatible(&client->dev, "maxim,max7357")) {
 		if (i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WRITE_BYTE_DATA)) {
@@ -505,18 +507,17 @@ static int pca954x_init(struct i2c_client *client, struct pca954x *data)
 						      "maxim,preconnection-wiggle-test-enable"))
 				conf |= MAX7357_CONF_PRECONNECT_TEST;
 
-			ret = i2c_smbus_write_byte_data(client, data->last_chan, conf);
+			ret = i2c_smbus_write_byte_data(client, regval, conf);
 		} else {
 			dev_warn(&client->dev, "Write byte data not supported."
 				 "Cannot enable enhanced mode features\n");
-			ret = i2c_smbus_write_byte(client, data->last_chan);
+			ret = i2c_smbus_write_byte(client, regval);
 		}
 	} else {
-		ret = i2c_smbus_write_byte(client, data->last_chan);
+		ret = i2c_smbus_write_byte(client, regval);
 	}
 
-	if (ret < 0)
-		data->last_chan = 0;
+	data->last_chan = ret < 0 ? -1 : regval;
 
 	return ret;
 }