[PATCH] i2c: i801: use i2c_mark_adapter_suspended/resumed

[PATCH] i2c: i801: use i2c_mark_adapter_suspended/resumed

[Heiner Kallweit]

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;
 }
-- 
2.42.0

Re: [PATCH] i2c: i801: use i2c_mark_adapter_suspended/resumed

[Jean Delvare]

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...

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH] i2c: i801: use i2c_mark_adapter_suspended/resumed

[Andi Shyti]

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

Re: [PATCH] i2c: i801: use i2c_mark_adapter_suspended/resumed

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 438 bytes --]

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!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]