* [PATCH] i2c: mux: mlxcpld: fix i2c mux selection caching
@ 2016-12-17 20:29 Peter Rosin
2016-12-17 20:34 ` Peter Rosin
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Peter Rosin @ 2016-12-17 20:29 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Vadim Pasternak, Michael Shych, Wolfram Sang, linux-i2c
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(-)
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;
--
2.1.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] i2c: mux: mlxcpld: fix i2c mux selection caching
2016-12-17 20:29 [PATCH] i2c: mux: mlxcpld: fix i2c mux selection caching Peter Rosin
@ 2016-12-17 20:34 ` Peter Rosin
2016-12-17 20:56 ` Vadim Pasternak
2016-12-18 7:56 ` Wolfram Sang
2 siblings, 0 replies; 4+ messages in thread
From: Peter Rosin @ 2016-12-17 20:34 UTC (permalink / raw)
To: linux-kernel; +Cc: Vadim Pasternak, Michael Shych, Wolfram Sang, linux-i2c
On 2016-12-17 21:29, 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
s/text/test/
> 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
missing terminating period
>
> 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>
> ---
Sorry for the poor proof-reading skills.
Cheers,
peda
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] i2c: mux: mlxcpld: fix i2c mux selection caching
2016-12-17 20:29 [PATCH] i2c: mux: mlxcpld: fix i2c mux selection caching Peter Rosin
2016-12-17 20:34 ` Peter Rosin
@ 2016-12-17 20:56 ` Vadim Pasternak
2016-12-18 7:56 ` Wolfram Sang
2 siblings, 0 replies; 4+ messages in thread
From: Vadim Pasternak @ 2016-12-17 20:56 UTC (permalink / raw)
To: Peter Rosin, linux-kernel; +Cc: Michael Shych, Wolfram Sang, linux-i2c
> -----Original Message-----
> From: Peter Rosin [mailto:peda@axentia.se]
> Sent: Saturday, December 17, 2016 10:29 PM
> To: linux-kernel@vger.kernel.org
> Cc: Peter Rosin <peda@axentia.se>; Vadim Pasternak
> <vadimp@mellanox.com>; Michael Shych <michaelsh@mellanox.com>;
> Wolfram Sang <wsa@the-dreams.de>; linux-i2c@vger.kernel.org
> Subject: [PATCH] i2c: mux: mlxcpld: fix i2c mux selection caching
>
> 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>
> ---
Acked-by: Vadim Pasternak <vadimp@mellanox.com>
>
> 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(-)
>
> 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;
> --
> 2.1.4
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] i2c: mux: mlxcpld: fix i2c mux selection caching
2016-12-17 20:29 [PATCH] i2c: mux: mlxcpld: fix i2c mux selection caching Peter Rosin
2016-12-17 20:34 ` Peter Rosin
2016-12-17 20:56 ` Vadim Pasternak
@ 2016-12-18 7:56 ` Wolfram Sang
2 siblings, 0 replies; 4+ messages in thread
From: Wolfram Sang @ 2016-12-18 7:56 UTC (permalink / raw)
To: Peter Rosin; +Cc: linux-kernel, Vadim Pasternak, Michael Shych, linux-i2c
[-- Attachment #1: Type: text/plain, Size: 911 bytes --]
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!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-12-18 7:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-17 20:29 [PATCH] i2c: mux: mlxcpld: fix i2c mux selection caching Peter Rosin
2016-12-17 20:34 ` Peter Rosin
2016-12-17 20:56 ` Vadim Pasternak
2016-12-18 7:56 ` Wolfram Sang
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).