Message ID | 20180801081742.12590-2-kai.heng.feng@canonical.com |
---|---|
State | New |
Headers | show |
Series | Fix regression on acpi-lpss | expand |
On 01/08/18 09:17, Kai-Heng Feng wrote: > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com> > > BugLink: https://bugs.launchpad.net/bugs/1774950 > > It is reported that commit a192aa923b66a (ACPI / LPSS: Consolidate > runtime PM and system sleep handling) introduced a system suspend > regression on some machines, but the only functional change made by > it was to cause the PM quirks in the LPSS to also be used during > system suspend and resume. While that should always work for > suspend-to-idle, it turns out to be problematic for S3 > (suspend-to-RAM). > > To address that issue restore the previous S3 suspend and resume > behavior of the LPSS to avoid applying PM quirks then. > > Fixes: a192aa923b66a (ACPI / LPSS: Consolidate runtime PM and system sleep handling) > Link: https://bugs.launchpad.net/bugs/1774950 > Reported-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com> > Cc: 4.15+ <stable@vger.kernel.org> # 4.15+ > (cherry picked from commit a09c591306881dfb04387c6ee7b7e2e4683fa531) > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > drivers/acpi/acpi_lpss.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c > index c71f5a2a592e..35dd15fce2fa 100644 > --- a/drivers/acpi/acpi_lpss.c > +++ b/drivers/acpi/acpi_lpss.c > @@ -22,6 +22,7 @@ > #include <linux/pm_domain.h> > #include <linux/pm_runtime.h> > #include <linux/pwm.h> > +#include <linux/suspend.h> > #include <linux/delay.h> > > #include "internal.h" > @@ -803,9 +804,10 @@ static void lpss_iosf_exit_d3_state(void) > mutex_unlock(&lpss_iosf_mutex); > } > > -static int acpi_lpss_suspend(struct device *dev, bool wakeup) > +static int acpi_lpss_suspend(struct device *dev, bool runtime) > { > struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev)); > + bool wakeup = runtime || device_may_wakeup(dev); > int ret; > > if (pdata->dev_desc->flags & LPSS_SAVE_CTX) > @@ -818,13 +820,14 @@ static int acpi_lpss_suspend(struct device *dev, bool wakeup) > * wrong status for devices being about to be powered off. See > * lpss_iosf_enter_d3_state() for further information. > */ > - if (lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available()) > + if ((runtime || !pm_suspend_via_firmware()) && > + lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available()) > lpss_iosf_enter_d3_state(); > > return ret; > } > > -static int acpi_lpss_resume(struct device *dev) > +static int acpi_lpss_resume(struct device *dev, bool runtime) > { > struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev)); > int ret; > @@ -833,7 +836,8 @@ static int acpi_lpss_resume(struct device *dev) > * This call is kept first to be in symmetry with > * acpi_lpss_runtime_suspend() one. > */ > - if (lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available()) > + if ((runtime || !pm_resume_via_firmware()) && > + lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available()) > lpss_iosf_exit_d3_state(); > > ret = acpi_dev_resume(dev); > @@ -857,12 +861,12 @@ static int acpi_lpss_suspend_late(struct device *dev) > return 0; > > ret = pm_generic_suspend_late(dev); > - return ret ? ret : acpi_lpss_suspend(dev, device_may_wakeup(dev)); > + return ret ? ret : acpi_lpss_suspend(dev, false); > } > > static int acpi_lpss_resume_early(struct device *dev) > { > - int ret = acpi_lpss_resume(dev); > + int ret = acpi_lpss_resume(dev, false); > > return ret ? ret : pm_generic_resume_early(dev); > } > @@ -877,7 +881,7 @@ static int acpi_lpss_runtime_suspend(struct device *dev) > > static int acpi_lpss_runtime_resume(struct device *dev) > { > - int ret = acpi_lpss_resume(dev); > + int ret = acpi_lpss_resume(dev, true); > > return ret ? ret : pm_generic_runtime_resume(dev); > } > Positive test results, and clean upstream cherry pick, Acked-by: Colin Ian King <colin.king@canonical.com>
diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c index c71f5a2a592e..35dd15fce2fa 100644 --- a/drivers/acpi/acpi_lpss.c +++ b/drivers/acpi/acpi_lpss.c @@ -22,6 +22,7 @@ #include <linux/pm_domain.h> #include <linux/pm_runtime.h> #include <linux/pwm.h> +#include <linux/suspend.h> #include <linux/delay.h> #include "internal.h" @@ -803,9 +804,10 @@ static void lpss_iosf_exit_d3_state(void) mutex_unlock(&lpss_iosf_mutex); } -static int acpi_lpss_suspend(struct device *dev, bool wakeup) +static int acpi_lpss_suspend(struct device *dev, bool runtime) { struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev)); + bool wakeup = runtime || device_may_wakeup(dev); int ret; if (pdata->dev_desc->flags & LPSS_SAVE_CTX) @@ -818,13 +820,14 @@ static int acpi_lpss_suspend(struct device *dev, bool wakeup) * wrong status for devices being about to be powered off. See * lpss_iosf_enter_d3_state() for further information. */ - if (lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available()) + if ((runtime || !pm_suspend_via_firmware()) && + lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available()) lpss_iosf_enter_d3_state(); return ret; } -static int acpi_lpss_resume(struct device *dev) +static int acpi_lpss_resume(struct device *dev, bool runtime) { struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev)); int ret; @@ -833,7 +836,8 @@ static int acpi_lpss_resume(struct device *dev) * This call is kept first to be in symmetry with * acpi_lpss_runtime_suspend() one. */ - if (lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available()) + if ((runtime || !pm_resume_via_firmware()) && + lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available()) lpss_iosf_exit_d3_state(); ret = acpi_dev_resume(dev); @@ -857,12 +861,12 @@ static int acpi_lpss_suspend_late(struct device *dev) return 0; ret = pm_generic_suspend_late(dev); - return ret ? ret : acpi_lpss_suspend(dev, device_may_wakeup(dev)); + return ret ? ret : acpi_lpss_suspend(dev, false); } static int acpi_lpss_resume_early(struct device *dev) { - int ret = acpi_lpss_resume(dev); + int ret = acpi_lpss_resume(dev, false); return ret ? ret : pm_generic_resume_early(dev); } @@ -877,7 +881,7 @@ static int acpi_lpss_runtime_suspend(struct device *dev) static int acpi_lpss_runtime_resume(struct device *dev) { - int ret = acpi_lpss_resume(dev); + int ret = acpi_lpss_resume(dev, true); return ret ? ret : pm_generic_runtime_resume(dev); }