Created attachment 292913 [details] pmgraph output for the slow S3 resume On a HP KBL platform I have, the hp_accel driver takes more than 1.1 second to resume from S3 or freeze state. And most of the time (1 second) is spent in evaluating the _INI method. So I checked the code a little bit, below is the code path lis3lv02d_resume() -> lis3lv02d_poweron() -> lis3->init() -> lis3lv02d_acpi_init() -> _INI According to the ACPI spec, _INI method can only be invoked during initialization time. I'm not sure why we need this in the .resume() callback, but this seems to be an ACPI spec violation to me. Raised the issue here and hopefully we can get this fixed.
Quick self intro: I have take over drivers/platform/x86 maintainership from Andy; and I'm working my way through the bugzilla backlog. I guess that calling _INI on resume could very well be against the ACPI spec, but I believe that more drivers are doing it. Still if skipping the _INI call on resume shaves a second of the resume call then we should probably indeed only do it on power-on (and resume-from-hibernate) I would certainly welcome a patch to do that. p.s. on the subject of the lis3lv02d code have you seen this oops/WARN_ON ? : https://bugzilla.redhat.com/show_bug.cgi?id=1813080 The backtrace is from a resume-from-hibernate, but it happens on boot and on regular resume too (the backtrace's for those are in private bugs). I ve started writing a fix for this: https://github.com/jwrdegoede/linux-sunxi/commit/4627dfbad4d005625071b6e491886b627435144f But I never finished it due to an unresponsive maintainer ... This mostly (but not always) seems to trigger on resume, so it is somewhat related. I guess that the driver may simply be doing too much on resume.
Andy, I'm answering here instead of github, since my git tree does not has stable commit ids it is better to discuss things here: > I'm wondering why we are still using old non-IIO driver? Can we switch to > IIO-based one? Is there an IIO based driver for these devices? AFAIK the drivers/misc driver was originally written for the HP implementation of these sensors (which was mostly for HDD fall protection) and the code ties into the HP ACPI tables, so switching to a new generic driver might not be so easy. There are a lot of HP models which use this driver, so I'm afraid there is a pretty big chance of regressions if we make drastic changes.
(In reply to Hans de Goede from comment #2) > Andy, I'm answering here instead of github, since my git tree does not has > stable commit ids it is better to discuss things here: > > > I'm wondering why we are still using old non-IIO driver? Can we switch to > > IIO-based one? > > Is there an IIO based driver for these devices? Of course I have checked before replying here. https://elixir.bootlin.com/linux/latest/source/drivers/iio/accel/st_accel_i2c.c > AFAIK the drivers/misc > driver was originally written for the HP implementation of these sensors > (which was mostly for HDD fall protection) and the code ties into the HP > ACPI tables, so switching to a new generic driver might not be so easy. > There are a lot of HP models which use this driver, so I'm afraid there is a > pretty big chance of regressions if we make drastic changes. I see, but I think we might consider to go with an IIO driver for new models / laptops.
Using the iio driver on newer models does sound like a good idea (assuming someone can test this for us) but how do we make the new driver bind on newer models and stick with the old one on older models ? So I just checked and it seems the old code binds to something which is described as the ACPI_MDPS_CLASS ? And it also registers a led-class device for a hdd-fall-protection LED ? This is all somewhat messy and without manpower + hw access to clean it up I guess we are just stuck with putting a bandaid on it when necessary. Which is less then ideal. Alternatively I wonder if we should not just disable this driver in a lot of cases, it is already being ignored by userspace for screen rotation since it is not that kind of accelerometer ...
Long time no news, should we close this report? Or what is the latest status?
(In reply to Andy Shevchenko from comment #5) > Long time no news, should we close this report? Or what is the latest status? This has very likely been fixed already by: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=79d341e26ebcdbc622348aaaab6f8f89b6fdb25f So lets close this.