Re: [PATCH 1/2] i2c: mux: pca954x: Make sure the mux remains configured the same as before resume

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/31/23 23:24, Peter Rosin wrote:
Hi!

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.

Isn't it better to keep the hardware in the same state it was before it entered suspend ? For me, that's the behavior I would expect from suspend/resume .

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?

If the write fails here, the hardware is in undefined state anyway .
Either the next attempt to flip the switch would help bring it back, or, the system is in undefined state.



[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux