Re: [PATCH v2 1/2] i2c: i801: Fix resume bug

[Wolfram Sang <wsa@kernel.org>]

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

Hi Volker, hi Jean,

On Sun, Sep 06, 2020 at 10:00:50AM +0200, Volker Rümelin wrote:
> Hi Jean,
> 
> with these two patches the code in i2c-i801.c looks really good.
> 
> But there is an issue with the reproducer.

I am not familiar with the HW; do we want these two patches here or does
the issue below need to be solved first? And if we want them, is it
still stable material?

Regards,

   Wolfram

> 
> > I noticed this bug in a QEMU x86_64 q35 VM booted with OVMF. OVMF
> > doesn't inititialize the SMBus master. After 1s of SMBus inactivity
> > autosuspend disables the SMBus master. To reproduce please note QEMU's
> > ICH9 SMBus emulation does not handle interrupts and it's necessary
> > to pass the parameter disable_features=0x10 to the i2c_i801 driver.
> 
> Since commit a9c8088c7988e "i2c: i801: Don't restore config
> registers on runtime PM" the reproducer doesn't work anymore.
> This is because commit a9c8088c7988e works as intended and the
> pm->runtime_* callbacks no longer call i801_suspend() and
> i801_resume().
> 
> But there is more. With the SMBus master in runtime suspended state
> the direct-complete mechanism skips the calls to the pm->suspend
> and pm->resume callbacks on system suspend/resume. I am convinced
> in nearly all cases this disables the fix from commit a5aaea37858fb
> "i2c-i801: Restore the device state before leaving".
> 
> At the moment I see two ways to fix this problem. One way is to
> revert a9c8088c7988e "i2c: i801: Don't restore config registers
> on runtime PM", the other is to set the driver flag
> DPM_FLAG_NO_DIRECT_COMPLETE in i801_probe(). I tested both, but I
> can't decide which way is better.
> 
> With best regards,
> Volker
> 

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