Message ID | 20180801081742.12590-3-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 > > Commit a09c59130688 (ACPI / LPSS: Avoid PM quirks on suspend and > resume from S3) modified the ACPI driver for Intel SoCs (LPSS) to > avoid applying PM quirks on suspend and resume from S3 to address > system-wide suspend and resume problems on some systems, but it is > reported that the same issue also affects hibernation, so extend > the approach used by that commit to cover hibernation as well. > > Fixes: a09c59130688 (ACPI / LPSS: Avoid PM quirks on suspend and resume from S3) > Link: https://bugs.launchpad.net/bugs/1774950 > Reported-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > Cc: 4.15+ <stable@vger.kernel.org> # 4.15+ > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> > (cherry picked from commit 12864ff8545f6b8144fdf1bb89b5663357f29ec4) > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > drivers/acpi/acpi_lpss.c | 26 +++++++++++++++++--------- > 1 file changed, 17 insertions(+), 9 deletions(-) > > diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c > index 35dd15fce2fa..8e37c2317e27 100644 > --- a/drivers/acpi/acpi_lpss.c > +++ b/drivers/acpi/acpi_lpss.c > @@ -736,6 +736,7 @@ static void acpi_lpss_dismiss(struct device *dev) > #define LPSS_GPIODEF0_DMA_LLP BIT(13) > > static DEFINE_MUTEX(lpss_iosf_mutex); > +static bool lpss_iosf_d3_entered; > > static void lpss_iosf_enter_d3_state(void) > { > @@ -778,6 +779,9 @@ static void lpss_iosf_enter_d3_state(void) > > iosf_mbi_modify(LPSS_IOSF_UNIT_LPIOEP, MBI_CR_WRITE, > LPSS_IOSF_GPIODEF0, value1, mask1); > + > + lpss_iosf_d3_entered = true; > + > exit: > mutex_unlock(&lpss_iosf_mutex); > } > @@ -792,6 +796,11 @@ static void lpss_iosf_exit_d3_state(void) > > mutex_lock(&lpss_iosf_mutex); > > + if (!lpss_iosf_d3_entered) > + goto exit; > + > + lpss_iosf_d3_entered = false; > + > iosf_mbi_modify(LPSS_IOSF_UNIT_LPIOEP, MBI_CR_WRITE, > LPSS_IOSF_GPIODEF0, value1, mask1); > > @@ -801,13 +810,13 @@ static void lpss_iosf_exit_d3_state(void) > iosf_mbi_modify(LPSS_IOSF_UNIT_LPIO1, MBI_CFG_WRITE, > LPSS_IOSF_PMCSR, value2, mask2); > > +exit: > mutex_unlock(&lpss_iosf_mutex); > } > > -static int acpi_lpss_suspend(struct device *dev, bool runtime) > +static int acpi_lpss_suspend(struct device *dev, bool wakeup) > { > 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) > @@ -820,14 +829,14 @@ static int acpi_lpss_suspend(struct device *dev, bool runtime) > * wrong status for devices being about to be powered off. See > * lpss_iosf_enter_d3_state() for further information. > */ > - if ((runtime || !pm_suspend_via_firmware()) && > + if (acpi_target_system_state() == ACPI_STATE_S0 && > 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, bool runtime) > +static int acpi_lpss_resume(struct device *dev) > { > struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev)); > int ret; > @@ -836,8 +845,7 @@ static int acpi_lpss_resume(struct device *dev, bool runtime) > * This call is kept first to be in symmetry with > * acpi_lpss_runtime_suspend() one. > */ > - if ((runtime || !pm_resume_via_firmware()) && > - lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available()) > + if (lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available()) > lpss_iosf_exit_d3_state(); > > ret = acpi_dev_resume(dev); > @@ -861,12 +869,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, false); > + return ret ? ret : acpi_lpss_suspend(dev, device_may_wakeup(dev)); > } > > static int acpi_lpss_resume_early(struct device *dev) > { > - int ret = acpi_lpss_resume(dev, false); > + int ret = acpi_lpss_resume(dev); > > return ret ? ret : pm_generic_resume_early(dev); > } > @@ -881,7 +889,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, true); > + int ret = acpi_lpss_resume(dev); > > 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 35dd15fce2fa..8e37c2317e27 100644 --- a/drivers/acpi/acpi_lpss.c +++ b/drivers/acpi/acpi_lpss.c @@ -736,6 +736,7 @@ static void acpi_lpss_dismiss(struct device *dev) #define LPSS_GPIODEF0_DMA_LLP BIT(13) static DEFINE_MUTEX(lpss_iosf_mutex); +static bool lpss_iosf_d3_entered; static void lpss_iosf_enter_d3_state(void) { @@ -778,6 +779,9 @@ static void lpss_iosf_enter_d3_state(void) iosf_mbi_modify(LPSS_IOSF_UNIT_LPIOEP, MBI_CR_WRITE, LPSS_IOSF_GPIODEF0, value1, mask1); + + lpss_iosf_d3_entered = true; + exit: mutex_unlock(&lpss_iosf_mutex); } @@ -792,6 +796,11 @@ static void lpss_iosf_exit_d3_state(void) mutex_lock(&lpss_iosf_mutex); + if (!lpss_iosf_d3_entered) + goto exit; + + lpss_iosf_d3_entered = false; + iosf_mbi_modify(LPSS_IOSF_UNIT_LPIOEP, MBI_CR_WRITE, LPSS_IOSF_GPIODEF0, value1, mask1); @@ -801,13 +810,13 @@ static void lpss_iosf_exit_d3_state(void) iosf_mbi_modify(LPSS_IOSF_UNIT_LPIO1, MBI_CFG_WRITE, LPSS_IOSF_PMCSR, value2, mask2); +exit: mutex_unlock(&lpss_iosf_mutex); } -static int acpi_lpss_suspend(struct device *dev, bool runtime) +static int acpi_lpss_suspend(struct device *dev, bool wakeup) { 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) @@ -820,14 +829,14 @@ static int acpi_lpss_suspend(struct device *dev, bool runtime) * wrong status for devices being about to be powered off. See * lpss_iosf_enter_d3_state() for further information. */ - if ((runtime || !pm_suspend_via_firmware()) && + if (acpi_target_system_state() == ACPI_STATE_S0 && 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, bool runtime) +static int acpi_lpss_resume(struct device *dev) { struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev)); int ret; @@ -836,8 +845,7 @@ static int acpi_lpss_resume(struct device *dev, bool runtime) * This call is kept first to be in symmetry with * acpi_lpss_runtime_suspend() one. */ - if ((runtime || !pm_resume_via_firmware()) && - lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available()) + if (lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available()) lpss_iosf_exit_d3_state(); ret = acpi_dev_resume(dev); @@ -861,12 +869,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, false); + return ret ? ret : acpi_lpss_suspend(dev, device_may_wakeup(dev)); } static int acpi_lpss_resume_early(struct device *dev) { - int ret = acpi_lpss_resume(dev, false); + int ret = acpi_lpss_resume(dev); return ret ? ret : pm_generic_resume_early(dev); } @@ -881,7 +889,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, true); + int ret = acpi_lpss_resume(dev); return ret ? ret : pm_generic_runtime_resume(dev); }