LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Wolfram Sang <wsa@the-dreams.de>
To: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Baolin Wang <baolin.wang@linaro.org>,
	Mark Brown <broonie@kernel.org>,
	linux-i2c@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>
Subject: I2C PM overhaul needed? (Re: [PATCH 1/2] i2c: sprd: Prevent i2c accesses after suspend is called)
Date: Fri, 4 May 2018 14:24:47 +0200	[thread overview]
Message-ID: <20180504122447.u3xgrkperxz5dpcz@ninjato> (raw)
In-Reply-To: <3485f73f-e356-6db0-89fc-d51bf8bdab71@ti.com>

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

Hi Grygorii,

thanks for stepping in.  I kept thinking about better I2C core support
for such situations and the more input the better.

> And you have to fix it (touch screen) - not your i2c driver. Otherwise, you can get
> situation when set of I2C transfers (executed from some kthread/work/threaded_irq/..)
> will be just interrupted in the middle - usual behavior after this is (I2C timeout) [and/or
> not-functional I2C client device [and/or I2C bus stuck (worst case)].

This. I also tend to think that most issues need to be fixed in the
client drivers ensuring proper states of client devices when suspending
/ resuming.. I wonder, though, if the core shouldn't assist by
guaranteeing that an on-going transfer has finished before suspending?
Or more technically, wait for a locked adapter to become unlocked again?

I still need to set it up and test, yet seeing that the EEPROM driver
at24.c has no suspend/resume callbacks, I'd assume a big write operation
will only be partially done when interrupted by a suspend.

> In case, somebody is trying to access I2C after .suspend_noirq() stage I2C bus driver 
> should produce big fat warning and, most probably, abort suspend.
> Above, in general, can be part of I2C core functionality.

Also this. However, there might be an exception of devices like PMICs
which may need to be accessed to trigger the final suspend state.

I at least know of some Renesas boards which needed the I2C connected
PMIC to do a system reset (not sure about suspend, need to recheck
that). That still today causes problems because interrupts are disabled
then.

To handle that, I imagined an additional adapter callback like
'master_xfer_irqless' to be used for such special I2C messages. These
kind of special messages could be tagged with a new I2C_M_something
flag.

And maybe this could be used here, too? Introduce this flag for very
late/early messages. If they have it, messages are even sent in
suspend_noirq() phase with the master_xfer_irqless() callback, otherwise
we will have the WARNing printed out.

Thoughts? Any other cases missed so far?

Kind regards,

   Wolfram


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

  reply	other threads:[~2018-05-04 12:24 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-09  6:40 [PATCH 1/2] i2c: sprd: Prevent i2c accesses after suspend is called Baolin Wang
2018-04-09  6:40 ` [PATCH 2/2] i2c: sprd: Fix the i2c count issue Baolin Wang
2018-04-27 12:14   ` Wolfram Sang
2018-04-09 20:56 ` [PATCH 1/2] i2c: sprd: Prevent i2c accesses after suspend is called Grygorii Strashko
2018-04-10  8:08   ` Baolin Wang
2018-04-27 12:14 ` Wolfram Sang
2018-05-02  3:27   ` Baolin Wang
2018-05-02  5:23     ` Wolfram Sang
2018-05-02  5:48       ` Baolin Wang
2018-05-03 16:26         ` Grygorii Strashko
2018-05-04 12:24           ` Wolfram Sang [this message]
2018-05-05  1:54             ` I2C PM overhaul needed? (Re: [PATCH 1/2] i2c: sprd: Prevent i2c accesses after suspend is called) Mark Brown
2018-05-05  8:32               ` Wolfram Sang
2018-05-09  8:18                 ` Mark Brown
2018-05-07 17:48             ` Grygorii Strashko
2018-05-08 16:32               ` Wolfram Sang
2018-05-08 18:31                 ` Peter Rosin
2018-05-11 15:14                 ` Grygorii Strashko

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=20180504122447.u3xgrkperxz5dpcz@ninjato \
    --to=wsa@the-dreams.de \
    --cc=baolin.wang@linaro.org \
    --cc=broonie@kernel.org \
    --cc=grygorii.strashko@ti.com \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --subject='I2C PM overhaul needed? (Re: [PATCH 1/2] i2c: sprd: Prevent i2c accesses after suspend is called)' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).