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:
signature.asc
Description: PGP signature
