ACK: [PATCH 2/2] ACPI / LPSS: Avoid PM quirks on suspend and resume from hibernation

[Colin Ian King]

On 01/08/18 09:17, Kai-Heng Feng wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki at 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 at canonical.com>
> Cc: 4.15+ <stable at vger.kernel.org> # 4.15+
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki at intel.com>
> Reviewed-by: Mika Westerberg <mika.westerberg at linux.intel.com>
> (cherry picked from commit 12864ff8545f6b8144fdf1bb89b5663357f29ec4)
> Signed-off-by: Kai-Heng Feng <kai.heng.feng at 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 at canonical.com>