[PATCH] i2c: muxes: pca954x: Fix broken reset-gpio usage

[PATCH] i2c: muxes: pca954x: Fix broken reset-gpio usage

[Krzysztof Kozlowski]

Revert commit 690de2902dca ("i2c: muxes: pca954x: Use reset controller
only") and its dependent commit 94c296776403 ("i2c: muxes: pca954x:
Reset if (de)select fails") because the first breaks all users of the
driver, by requiring a completely optional reset-gpio driver.  These
commits cause that mux driver simply stops working when optional
reset-gpio is not included, but that reset-gpio is not pulled anyhow.

Driver cannot remove legacy reset-gpios handling.

Fixes: 690de2902dca ("i2c: muxes: pca954x: Use reset controller only")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 50 +++++++++++++----------------
 1 file changed, 23 insertions(+), 27 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 75c8d08fa24e..b9f370c9f018 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -118,6 +118,7 @@ struct pca954x {
 	raw_spinlock_t lock;
 	struct regulator *supply;
 
+	struct gpio_desc *reset_gpio;
 	struct reset_control *reset_cont;
 };
 
@@ -315,25 +316,6 @@ static u8 pca954x_regval(struct pca954x *data, u8 chan)
 		return 1 << chan;
 }
 
-static void pca954x_reset_assert(struct pca954x *data)
-{
-	if (data->reset_cont)
-		reset_control_assert(data->reset_cont);
-}
-
-static void pca954x_reset_deassert(struct pca954x *data)
-{
-	if (data->reset_cont)
-		reset_control_deassert(data->reset_cont);
-}
-
-static void pca954x_reset_mux(struct pca954x *data)
-{
-	pca954x_reset_assert(data);
-	udelay(1);
-	pca954x_reset_deassert(data);
-}
-
 static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
 {
 	struct pca954x *data = i2c_mux_priv(muxc);
@@ -347,8 +329,6 @@ static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
 		ret = pca954x_reg_write(muxc->parent, client, regval);
 		data->last_chan = ret < 0 ? 0 : regval;
 	}
-	if (ret == -ETIMEDOUT && data->reset_cont)
-		pca954x_reset_mux(data);
 
 	return ret;
 }
@@ -358,7 +338,6 @@ 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 = 0;
 
 	idle_state = READ_ONCE(data->idle_state);
 	if (idle_state >= 0)
@@ -368,10 +347,8 @@ 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;
-		ret = pca954x_reg_write(muxc->parent, client,
-					data->last_chan);
-		if (ret == -ETIMEDOUT && data->reset_cont)
-			pca954x_reset_mux(data);
+		return pca954x_reg_write(muxc->parent, client,
+					 data->last_chan);
 	}
 
 	/* otherwise leave as-is */
@@ -550,10 +527,29 @@ static int pca954x_get_reset(struct device *dev, struct pca954x *data)
 	if (IS_ERR(data->reset_cont))
 		return dev_err_probe(dev, PTR_ERR(data->reset_cont),
 				     "Failed to get reset\n");
+	else if (data->reset_cont)
+		return 0;
+
+	/*
+	 * fallback to legacy reset-gpios
+	 */
+	data->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(data->reset_gpio)) {
+		return dev_err_probe(dev, PTR_ERR(data->reset_gpio),
+				     "Failed to get reset gpio");
+	}
 
 	return 0;
 }
 
+static void pca954x_reset_deassert(struct pca954x *data)
+{
+	if (data->reset_cont)
+		reset_control_deassert(data->reset_cont);
+	else
+		gpiod_set_value_cansleep(data->reset_gpio, 0);
+}
+
 /*
  * I2C init/probing/exit functions
  */
@@ -593,7 +589,7 @@ static int pca954x_probe(struct i2c_client *client)
 	if (ret)
 		goto fail_cleanup;
 
-	if (data->reset_cont) {
+	if (data->reset_cont || data->reset_gpio) {
 		udelay(1);
 		pca954x_reset_deassert(data);
 		/* Give the chip some time to recover. */
-- 
2.48.1

Re: [PATCH] i2c: muxes: pca954x: Fix broken reset-gpio usage

[Krzysztof Kozlowski]

On 03/11/2025 16:19, Krzysztof Kozlowski wrote:
> Revert commit 690de2902dca ("i2c: muxes: pca954x: Use reset controller
> only") and its dependent commit 94c296776403 ("i2c: muxes: pca954x:
> Reset if (de)select fails") because the first breaks all users of the
> driver, by requiring a completely optional reset-gpio driver.  These
> commits cause that mux driver simply stops working when optional
> reset-gpio is not included, but that reset-gpio is not pulled anyhow.
> 
> Driver cannot remove legacy reset-gpios handling.
> 
> Fixes: 690de2902dca ("i2c: muxes: pca954x: Use reset controller only")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---


Maybe I should also add to emphasize:

That offending commit 690de2902dca breaks the ABI which was:
1. documented at
Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml (line 57)
2. implemented in Linux kernel around v3.14.

This ABI breakage was not justified at all, not explained not even
mentioned in the commit msg, which is a requirement (see writing
bindings document).

Best regards,
Krzysztof

Re: [PATCH] i2c: muxes: pca954x: Fix broken reset-gpio usage

[Wolfram Sang]

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


> Maybe I should also add to emphasize:
> 
> That offending commit 690de2902dca breaks the ABI which was:
> 1. documented at
> Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml (line 57)
> 2. implemented in Linux kernel around v3.14.
> 
> This ABI breakage was not justified at all, not explained not even
> mentioned in the commit msg, which is a requirement (see writing
> bindings document).

We were under the assumption that the reset-core will provide exactly
the same functionality at a centralized place. It made sense to me, too,
to avoid all the open-coded reset-gpio handlings in various drivers. I
only learnt from Morimoto-san's regression report that things were not
as I thought.

So, I sent out an RFC to discuss how to handle this issue. And if even
Philipp as the maintainer did not point me directly to revert the
offending i2c-mux commit, despite it is "obviously" sooo wrong, then
probably that optional fallback part in the reset-core needs way more
explanations and documentation.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] i2c: muxes: pca954x: Fix broken reset-gpio usage

[Wolfram Sang]

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

On Mon, Nov 03, 2025 at 04:19:06PM +0100, Krzysztof Kozlowski wrote:
> Revert commit 690de2902dca ("i2c: muxes: pca954x: Use reset controller
> only") and its dependent commit 94c296776403 ("i2c: muxes: pca954x:
> Reset if (de)select fails") because the first breaks all users of the
> driver, by requiring a completely optional reset-gpio driver.  These
> commits cause that mux driver simply stops working when optional
> reset-gpio is not included, but that reset-gpio is not pulled anyhow.
> 
> Driver cannot remove legacy reset-gpios handling.
> 
> Fixes: 690de2902dca ("i2c: muxes: pca954x: Use reset controller only")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Applied to for-current, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]