Re: [PATCH v2 0/9] i2c: move handling of suspended adapters to the core

From: Hans de Goede
Date: Wed Dec 26 2018 - 06:47:10 EST


On 26-12-18 12:01, Geert Uytterhoeven wrote:
Hi Wolfram,

On Sat, Dec 22, 2018 at 9:26 PM Wolfram Sang
<wsa+renesas@xxxxxxxxxxxxxxxxxxxx> wrote:
Here is the new version without specific I2C helpers but using the
'is_suspended' flag from the PM core. I didn't like messing with the
flag directly, so I did a helper in patch 1. So far, I like the
approach. The diffstat looks nice, and i2c-rcar.c and i2c-sh_mobile.c
rejected rightfully too later transfers without further modifications.
Tested on a Renesas Lager board (R-Car H2).

I dropped a few Tested-by tags because I think this approach is too
different from V1 to keep them. I hope you guys can have a look again.
Thanks for all the testing, so far!

A branch can be found here:

git:// renesas/i2c/reject-when-suspended

If this series is acceptable, I'd suggest to take it via my i2c tree
after rc1. And then I'll provide an immutable branch for the PM tree to
pick. Let me know if this works for you.

This breaks resume of the CS2000 clock driver on Salvator-X(S) (and
presumable ULCB, too):

Accessing already suspended I2C/SMBus adapter
WARNING: CPU: 1 PID: 1186 at drivers/i2c/i2c-core-smbus.c:551
Modules linked in:
CPU: 1 PID: 1186 Comm: s2idle Not tainted
4.20.0-salvator-x-08442-ge5992c41ac706409 #264
Hardware name: Renesas Salvator-X board based on r8a7795 ES1.x (DT)
pstate: 60400005 (nZCv daif +PAN -UAO)
pc : __i2c_smbus_xfer+0x38/0x66c
lr : __i2c_smbus_xfer+0x38/0x66c
sp : ffffff8013413880
x29: ffffff8013413880 x28: ffffffc6f78b4820
x27: 0000000000000010 x26: ffffff8010cf6178
x25: ffffff8013413976 x24: 0000000000000002
x23: 0000000000000016 x22: ffffffc6f7872088
x21: 0000000000000000 x20: 000000000000004f
x19: ffffffc6f7872088 x18: 000000000000000a
x17: 0000000000000000 x16: 0000000000000000
x15: 0000000000029c80 x14: 0720072007200720
x13: 0720072007200720 x12: 0720072007200720
x11: 0720072007200720 x10: 0720072007200720
x9 : ffffff801100e6d0 x8 : 6461207375424d53
x7 : ffffff8011c825c8 x6 : ffffff8011c82000
x5 : 0000000000000000 x4 : ffffff8013414000
x3 : ffffff80134136e0 x2 : 00000046ee875000
x1 : 82c6d7c720d64000 x0 : 0000000000000000
Call trace:

The CS2000 driver uses SET_LATE_SYSTEM_SLEEP_PM_OPS().

Suspend/resume of the BD9571 driver works fine, as that one uses

Ok, so this is the same thing I noticed, the approach using
the pm-core is_suspend flag only works for adapters which
suspend during the regular suspend phase and as such that
approach cannot work everywhere.