linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] i2c: i2c-mux-pca954x: only reset last channel on error
@ 2016-12-16 21:08 Vivien Didelot
  2016-12-16 21:30 ` Peter Rosin
  0 siblings, 1 reply; 2+ messages in thread
From: Vivien Didelot @ 2016-12-16 21:08 UTC (permalink / raw)
  To: Peter Rosin; +Cc: Wolfram Sang, linux-i2c, linux-kernel, kernel, Vivien Didelot

The current code is selecting the mux channel before each operation and
does not benefit from the cached value (data->last_chan).

That is because pca954x_select_chan() considers any non-zero values from
pca954x_reg_write() as an error. But this function (via __i2c_transfer)
returns either a negative error code or the positive number of messages
executed.

Only check "ret" against negative values to restore the caching.

Fixes: 463e8f845cbf ("i2c: mux: pca954x: retry updating the mux selection on failure")
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 8bc3d36..0469f1e 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -179,7 +179,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 : regval;
+		data->last_chan = ret < 0 ? 0 : regval;
 	}
 
 	return ret;
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] i2c: i2c-mux-pca954x: only reset last channel on error
  2016-12-16 21:08 [PATCH] i2c: i2c-mux-pca954x: only reset last channel on error Vivien Didelot
@ 2016-12-16 21:30 ` Peter Rosin
  0 siblings, 0 replies; 2+ messages in thread
From: Peter Rosin @ 2016-12-16 21:30 UTC (permalink / raw)
  To: Vivien Didelot
  Cc: Wolfram Sang, linux-i2c, linux-kernel, kernel, Russell King

On 2016-12-16 22:08, Vivien Didelot wrote:
> The current code is selecting the mux channel before each operation and
> does not benefit from the cached value (data->last_chan).
> 
> That is because pca954x_select_chan() considers any non-zero values from
> pca954x_reg_write() as an error. But this function (via __i2c_transfer)
> returns either a negative error code or the positive number of messages
> executed.
> 
> Only check "ret" against negative values to restore the caching.
> 
> Fixes: 463e8f845cbf ("i2c: mux: pca954x: retry updating the mux selection on failure")
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Hi Vivien,

Thanks for the report, and sorry for breaking things. However, your
patch got beaten by an hour by Russel King who sent approximately
the same patch. I had already responded to his message when I saw
yours, so I did not Cc you.

Cheers,
peda

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2016-12-16 21:30 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-16 21:08 [PATCH] i2c: i2c-mux-pca954x: only reset last channel on error Vivien Didelot
2016-12-16 21:30 ` Peter Rosin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).