Hi!
2023-08-31 at 20:17, Marek Vasut wrote:
> The current implementation of pca954x_init() rewrites content of data->last_chan
> which is then populated into the mux select register. Skip this part, so that the
> mux is populated with content of data->last_chan as it was set before suspend.
> This way, the mux state is retained across suspend/resume cycle.
I fail to see in what situation this change makes a significant
difference? For me, it's a nice conservative thing to initialize
to the default state after something comparatively heavy such as
a suspend/resume cycle. If there is a significant difference,
then maybe it's not the usual access patterns after resume since
there are probably other chips initializing as well, in which
case this change might make things worse depending on what
devices you do have and what idle-state you have configured.
> Fixes: e65e228eb096 ("i2c: mux: pca954x: support property idle-state")
> Signed-off-by: Marek Vasut <marex@xxxxxxx>
> ---
> Cc: Peter Rosin <peda@xxxxxxxxxx>
> Cc: Wolfram Sang <wsa@xxxxxxxxxx>
> Cc: linux-i2c@xxxxxxxxxxxxxxx
> ---
> drivers/i2c/muxes/i2c-mux-pca954x.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index 2219062104fbc..97cf475dde0f4 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -620,9 +620,9 @@ static int pca954x_resume(struct device *dev)
> struct pca954x *data = i2c_mux_priv(muxc);
> int ret;
>
> - ret = pca954x_init(client, data);
> + ret = i2c_smbus_write_byte(client, data->last_chan);
> if (ret < 0)
> - dev_err(&client->dev, "failed to verify mux presence\n");
> + dev_err(&client->dev, "failed to restore mux state\n");
data->last_chan is no longer cleared in case the write fails. Is that a
problem?
Cheers,
Peter
>
> return ret;
> }
