| Message ID | 0d13ed54-af1d-4c21-a90c-ba8c6b03f67e@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: i801: use i2c_mark_adapter_suspended/resumed | expand |
Hi Heiner, Wolrfam, On Wed, 20 Sep 2023 09:29:28 +0200, Heiner Kallweit wrote: > When entering the suspend callback, at first we should ensure that > transfers are finished and I2C core can't start further transfers. > Use i2c_mark_adapter_suspended() for this purpose, and complement it > with a call to i2c_mark_adapter_resumed() in the resume path. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > Rebased version of a previously discussed patch, now w/o touching > the remove and shutdown path. > --- > drivers/i2c/busses/i2c-i801.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c > index 6d02a8b88..26f132277 100644 > --- a/drivers/i2c/busses/i2c-i801.c > +++ b/drivers/i2c/busses/i2c-i801.c > @@ -1818,6 +1818,7 @@ static int i801_suspend(struct device *dev) > { > struct i801_priv *priv = dev_get_drvdata(dev); > > + i2c_mark_adapter_suspended(&priv->adapter); > i801_restore_regs(priv); > > return 0; > @@ -1829,6 +1830,7 @@ static int i801_resume(struct device *dev) > > i801_setup_hstcfg(priv); > i801_enable_host_notify(&priv->adapter); > + i2c_mark_adapter_resumed(&priv->adapter); > > return 0; > } As I said before, I wish this was handled by the driver core instead of device drivers individually, but until that is implemented, I'm fine with this patch, only because other drivers are already doing the same, so it can't be that bad. Reviewed-by: Jean Delvare <jdelvare@suse.de> To be honest, I'm not even sure why this is necessary. I thought that the driver core was already suspending the device tree from the leafs to the root, and resuming from the root to the leafs, so a child would never be active when its parent isn't. So I just can't see how an I2C transfer can be initiated by an I2C device driver (child) to a suspended I2C adapter (parent). But I'm probably missing something in the driver model. Power management has become so complex over the years...
Hi Heiner, On Wed, Sep 20, 2023 at 09:29:28AM +0200, Heiner Kallweit wrote: > When entering the suspend callback, at first we should ensure that > transfers are finished and I2C core can't start further transfers. > Use i2c_mark_adapter_suspended() for this purpose, and complement it > with a call to i2c_mark_adapter_resumed() in the resume path. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > Rebased version of a previously discussed patch, now w/o touching > the remove and shutdown path. > --- > drivers/i2c/busses/i2c-i801.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c > index 6d02a8b88..26f132277 100644 > --- a/drivers/i2c/busses/i2c-i801.c > +++ b/drivers/i2c/busses/i2c-i801.c > @@ -1818,6 +1818,7 @@ static int i801_suspend(struct device *dev) > { > struct i801_priv *priv = dev_get_drvdata(dev); > > + i2c_mark_adapter_suspended(&priv->adapter); > i801_restore_regs(priv); > > return 0; > @@ -1829,6 +1830,7 @@ static int i801_resume(struct device *dev) > > i801_setup_hstcfg(priv); > i801_enable_host_notify(&priv->adapter); > + i2c_mark_adapter_resumed(&priv->adapter); I think I already reviewed this patch previously and I had same concerns as Jean, anyway, Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Andi
On Wed, Sep 20, 2023 at 09:29:28AM +0200, Heiner Kallweit wrote: > When entering the suspend callback, at first we should ensure that > transfers are finished and I2C core can't start further transfers. > Use i2c_mark_adapter_suspended() for this purpose, and complement it > with a call to i2c_mark_adapter_resumed() in the resume path. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Applied to for-next, thanks!
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index 6d02a8b88..26f132277 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -1818,6 +1818,7 @@ static int i801_suspend(struct device *dev) { struct i801_priv *priv = dev_get_drvdata(dev); + i2c_mark_adapter_suspended(&priv->adapter); i801_restore_regs(priv); return 0; @@ -1829,6 +1830,7 @@ static int i801_resume(struct device *dev) i801_setup_hstcfg(priv); i801_enable_host_notify(&priv->adapter); + i2c_mark_adapter_resumed(&priv->adapter); return 0; }
When entering the suspend callback, at first we should ensure that transfers are finished and I2C core can't start further transfers. Use i2c_mark_adapter_suspended() for this purpose, and complement it with a call to i2c_mark_adapter_resumed() in the resume path. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- Rebased version of a previously discussed patch, now w/o touching the remove and shutdown path. --- drivers/i2c/busses/i2c-i801.c | 2 ++ 1 file changed, 2 insertions(+)