public inbox for linux-i2c@vger.kernel.org
 help / color / mirror / Atom feed
From: Jean Delvare <jdelvare@suse.de>
To: Heiner Kallweit <hkallweit1@gmail.com>
Cc: Wolfram Sang <wsa@kernel.org>, Andi Shyti <andi.shyti@kernel.org>,
	linux-i2c@vger.kernel.org
Subject: Re: [PATCH] i2c: i801: use i2c_mark_adapter_suspended/resumed
Date: Thu, 21 Sep 2023 15:01:26 +0200	[thread overview]
Message-ID: <20230921150126.32925f39@endymion.delvare> (raw)
In-Reply-To: <0d13ed54-af1d-4c21-a90c-ba8c6b03f67e@gmail.com>

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

  reply	other threads:[~2023-09-21 17:50 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-20  7:29 [PATCH] i2c: i801: use i2c_mark_adapter_suspended/resumed Heiner Kallweit
2023-09-21 13:01 ` Jean Delvare [this message]
2023-09-21 21:14 ` Andi Shyti
2023-09-22  9:43 ` Wolfram Sang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230921150126.32925f39@endymion.delvare \
    --to=jdelvare@suse.de \
    --cc=andi.shyti@kernel.org \
    --cc=hkallweit1@gmail.com \
    --cc=linux-i2c@vger.kernel.org \
    --cc=wsa@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox