Hi Volker, On Sun, 6 Sep 2020 10:00:50 +0200, Volker Rümelin wrote: > with these two patches the code in i2c-i801.c looks really good. Great, thanks for the review. > But there is an issue with the reproducer. > > > 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(). To be honest I was surprised that you were able to reproduce the resume bug with QEMU in the first place. I did not remember about commit a9c8088c7988e. > 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". My understanding of the PM subsystem is fairly limited (it evolves faster than my leaning curve I'm afraid) but you are most certainly right on that. > 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. Reverting a9c8088c7988e would make run-time PM inefficient again, so I am opposed to doing that. Flag DPM_FLAG_NO_DIRECT_COMPLETE seems to have been introduced for exactly this purpose, so I would just use that. To be honest I don't really understand why it is not the default, but again I don't know much about the PM subsystem. -- Jean Delvare SUSE L3 Support
