Re: Possible regression caused by commit a192aa923b66a

2018-06-13 Thread Rafael J. Wysocki
On Wed, Jun 13, 2018 at 4:45 AM, Kai Heng Feng
 wrote:
> Hi Rafael,
>
>> On Jun 12, 2018, at 5:17 PM, Rafael J. Wysocki  wrote:
>>
>> On Monday, June 11, 2018 11:52:34 PM CEST Rafael J. Wysocki wrote:
>>>
>>> --703623056e64c488
>>> Content-Type: text/plain; charset="UTF-8"
>>>
>>> On Mon, Jun 11, 2018 at 10:09 AM, Rafael J. Wysocki 
>>> wrote:

 On Mon, Jun 11, 2018 at 8:26 AM, Kai-Heng Feng
  wrote:
>
> Hi Rafael,
>
> There's a regression report [1] that says commit a192aa923b66a ("ACPI /
> LPSS: Consolidate runtime PM and system sleep handling") is the first
> bad
> commit.
>
> From the looks of it, it didn't introduce any behavioral change. So
> your
> help is appreciated.
>
> [1] https://bugs.launchpad.net/bugs/1774950


 Well, the only difference is the iosf quirk AFAICS, but that should be
 easy enough to check.  I'll try to cut a patch for that later today.
>>>
>>>
>>> If the iosf quirk is the source of the problem, the attached patch should
>>> help.
>>
>>
>> The one below should be slightly better, please test this one.
>
>
> Affected users reported that this patch solves the issue for them.

OK, thanks for verifying.  Let me add a changelog to it and resend it, then.

Thanks,
Rafael


Re: Possible regression caused by commit a192aa923b66a

2018-06-13 Thread Rafael J. Wysocki
On Wed, Jun 13, 2018 at 4:45 AM, Kai Heng Feng
 wrote:
> Hi Rafael,
>
>> On Jun 12, 2018, at 5:17 PM, Rafael J. Wysocki  wrote:
>>
>> On Monday, June 11, 2018 11:52:34 PM CEST Rafael J. Wysocki wrote:
>>>
>>> --703623056e64c488
>>> Content-Type: text/plain; charset="UTF-8"
>>>
>>> On Mon, Jun 11, 2018 at 10:09 AM, Rafael J. Wysocki 
>>> wrote:

 On Mon, Jun 11, 2018 at 8:26 AM, Kai-Heng Feng
  wrote:
>
> Hi Rafael,
>
> There's a regression report [1] that says commit a192aa923b66a ("ACPI /
> LPSS: Consolidate runtime PM and system sleep handling") is the first
> bad
> commit.
>
> From the looks of it, it didn't introduce any behavioral change. So
> your
> help is appreciated.
>
> [1] https://bugs.launchpad.net/bugs/1774950


 Well, the only difference is the iosf quirk AFAICS, but that should be
 easy enough to check.  I'll try to cut a patch for that later today.
>>>
>>>
>>> If the iosf quirk is the source of the problem, the attached patch should
>>> help.
>>
>>
>> The one below should be slightly better, please test this one.
>
>
> Affected users reported that this patch solves the issue for them.

OK, thanks for verifying.  Let me add a changelog to it and resend it, then.

Thanks,
Rafael


Re: Possible regression caused by commit a192aa923b66a

2018-06-12 Thread Kai Heng Feng

Hi Rafael,


On Jun 12, 2018, at 5:17 PM, Rafael J. Wysocki  wrote:

On Monday, June 11, 2018 11:52:34 PM CEST Rafael J. Wysocki wrote:

--703623056e64c488
Content-Type: text/plain; charset="UTF-8"

On Mon, Jun 11, 2018 at 10:09 AM, Rafael J. Wysocki   
wrote:

On Mon, Jun 11, 2018 at 8:26 AM, Kai-Heng Feng
 wrote:

Hi Rafael,

There's a regression report [1] that says commit a192aa923b66a ("ACPI /
LPSS: Consolidate runtime PM and system sleep handling") is the first  
bad

commit.

From the looks of it, it didn't introduce any behavioral change. So your
help is appreciated.

[1] https://bugs.launchpad.net/bugs/1774950


Well, the only difference is the iosf quirk AFAICS, but that should be
easy enough to check.  I'll try to cut a patch for that later today.


If the iosf quirk is the source of the problem, the attached patch  
should help.


The one below should be slightly better, please test this one.


Affected users reported that this patch solves the issue for them.

Kai-Heng



---
 drivers/acpi/acpi_lpss.c |   18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

Index: linux-pm/drivers/acpi/acpi_lpss.c
===
--- linux-pm.orig/drivers/acpi/acpi_lpss.c
+++ linux-pm/drivers/acpi/acpi_lpss.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 

 #include "internal.h"
@@ -940,9 +941,10 @@ static void lpss_iosf_exit_d3_state(void
mutex_unlock(_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)
@@ -955,13 +957,14 @@ static int acpi_lpss_suspend(struct devi
 * 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;
@@ -970,7 +973,8 @@ static int acpi_lpss_resume(struct devic
 * 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);
@@ -994,12 +998,12 @@ static int acpi_lpss_suspend_late(struct
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);
 }
@@ -1014,7 +1018,7 @@ static int acpi_lpss_runtime_suspend(str

 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);
 }


Re: Possible regression caused by commit a192aa923b66a

2018-06-12 Thread Kai Heng Feng

Hi Rafael,


On Jun 12, 2018, at 5:17 PM, Rafael J. Wysocki  wrote:

On Monday, June 11, 2018 11:52:34 PM CEST Rafael J. Wysocki wrote:

--703623056e64c488
Content-Type: text/plain; charset="UTF-8"

On Mon, Jun 11, 2018 at 10:09 AM, Rafael J. Wysocki   
wrote:

On Mon, Jun 11, 2018 at 8:26 AM, Kai-Heng Feng
 wrote:

Hi Rafael,

There's a regression report [1] that says commit a192aa923b66a ("ACPI /
LPSS: Consolidate runtime PM and system sleep handling") is the first  
bad

commit.

From the looks of it, it didn't introduce any behavioral change. So your
help is appreciated.

[1] https://bugs.launchpad.net/bugs/1774950


Well, the only difference is the iosf quirk AFAICS, but that should be
easy enough to check.  I'll try to cut a patch for that later today.


If the iosf quirk is the source of the problem, the attached patch  
should help.


The one below should be slightly better, please test this one.


Affected users reported that this patch solves the issue for them.

Kai-Heng



---
 drivers/acpi/acpi_lpss.c |   18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

Index: linux-pm/drivers/acpi/acpi_lpss.c
===
--- linux-pm.orig/drivers/acpi/acpi_lpss.c
+++ linux-pm/drivers/acpi/acpi_lpss.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 

 #include "internal.h"
@@ -940,9 +941,10 @@ static void lpss_iosf_exit_d3_state(void
mutex_unlock(_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)
@@ -955,13 +957,14 @@ static int acpi_lpss_suspend(struct devi
 * 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;
@@ -970,7 +973,8 @@ static int acpi_lpss_resume(struct devic
 * 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);
@@ -994,12 +998,12 @@ static int acpi_lpss_suspend_late(struct
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);
 }
@@ -1014,7 +1018,7 @@ static int acpi_lpss_runtime_suspend(str

 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);
 }


Re: Possible regression caused by commit a192aa923b66a

2018-06-12 Thread Rafael J. Wysocki
On Monday, June 11, 2018 11:52:34 PM CEST Rafael J. Wysocki wrote:
> 
> --703623056e64c488
> Content-Type: text/plain; charset="UTF-8"
> 
> On Mon, Jun 11, 2018 at 10:09 AM, Rafael J. Wysocki  wrote:
> > On Mon, Jun 11, 2018 at 8:26 AM, Kai-Heng Feng
> >  wrote:
> >> Hi Rafael,
> >>
> >> There's a regression report [1] that says commit a192aa923b66a ("ACPI /
> >> LPSS: Consolidate runtime PM and system sleep handling") is the first bad
> >> commit.
> >>
> >> From the looks of it, it didn't introduce any behavioral change. So your
> >> help is appreciated.
> >>
> >> [1] https://bugs.launchpad.net/bugs/1774950
> >
> > Well, the only difference is the iosf quirk AFAICS, but that should be
> > easy enough to check.  I'll try to cut a patch for that later today.
> 
> If the iosf quirk is the source of the problem, the attached patch should 
> help.

The one below should be slightly better, please test this one.

---
 drivers/acpi/acpi_lpss.c |   18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

Index: linux-pm/drivers/acpi/acpi_lpss.c
===
--- linux-pm.orig/drivers/acpi/acpi_lpss.c
+++ linux-pm/drivers/acpi/acpi_lpss.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "internal.h"
@@ -940,9 +941,10 @@ static void lpss_iosf_exit_d3_state(void
mutex_unlock(_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)
@@ -955,13 +957,14 @@ static int acpi_lpss_suspend(struct devi
 * 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;
@@ -970,7 +973,8 @@ static int acpi_lpss_resume(struct devic
 * 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);
@@ -994,12 +998,12 @@ static int acpi_lpss_suspend_late(struct
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);
 }
@@ -1014,7 +1018,7 @@ static int acpi_lpss_runtime_suspend(str
 
 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);
 }



Re: Possible regression caused by commit a192aa923b66a

2018-06-12 Thread Rafael J. Wysocki
On Monday, June 11, 2018 11:52:34 PM CEST Rafael J. Wysocki wrote:
> 
> --703623056e64c488
> Content-Type: text/plain; charset="UTF-8"
> 
> On Mon, Jun 11, 2018 at 10:09 AM, Rafael J. Wysocki  wrote:
> > On Mon, Jun 11, 2018 at 8:26 AM, Kai-Heng Feng
> >  wrote:
> >> Hi Rafael,
> >>
> >> There's a regression report [1] that says commit a192aa923b66a ("ACPI /
> >> LPSS: Consolidate runtime PM and system sleep handling") is the first bad
> >> commit.
> >>
> >> From the looks of it, it didn't introduce any behavioral change. So your
> >> help is appreciated.
> >>
> >> [1] https://bugs.launchpad.net/bugs/1774950
> >
> > Well, the only difference is the iosf quirk AFAICS, but that should be
> > easy enough to check.  I'll try to cut a patch for that later today.
> 
> If the iosf quirk is the source of the problem, the attached patch should 
> help.

The one below should be slightly better, please test this one.

---
 drivers/acpi/acpi_lpss.c |   18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

Index: linux-pm/drivers/acpi/acpi_lpss.c
===
--- linux-pm.orig/drivers/acpi/acpi_lpss.c
+++ linux-pm/drivers/acpi/acpi_lpss.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "internal.h"
@@ -940,9 +941,10 @@ static void lpss_iosf_exit_d3_state(void
mutex_unlock(_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)
@@ -955,13 +957,14 @@ static int acpi_lpss_suspend(struct devi
 * 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;
@@ -970,7 +973,8 @@ static int acpi_lpss_resume(struct devic
 * 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);
@@ -994,12 +998,12 @@ static int acpi_lpss_suspend_late(struct
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);
 }
@@ -1014,7 +1018,7 @@ static int acpi_lpss_runtime_suspend(str
 
 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);
 }



Re: Possible regression caused by commit a192aa923b66a

2018-06-11 Thread Rafael J. Wysocki
On Mon, Jun 11, 2018 at 10:09 AM, Rafael J. Wysocki  wrote:
> On Mon, Jun 11, 2018 at 8:26 AM, Kai-Heng Feng
>  wrote:
>> Hi Rafael,
>>
>> There's a regression report [1] that says commit a192aa923b66a ("ACPI /
>> LPSS: Consolidate runtime PM and system sleep handling") is the first bad
>> commit.
>>
>> From the looks of it, it didn't introduce any behavioral change. So your
>> help is appreciated.
>>
>> [1] https://bugs.launchpad.net/bugs/1774950
>
> Well, the only difference is the iosf quirk AFAICS, but that should be
> easy enough to check.  I'll try to cut a patch for that later today.

If the iosf quirk is the source of the problem, the attached patch should help.
---
 drivers/acpi/acpi_lpss.c |   17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

Index: linux-pm/drivers/acpi/acpi_lpss.c
===
--- linux-pm.orig/drivers/acpi/acpi_lpss.c
+++ linux-pm/drivers/acpi/acpi_lpss.c
@@ -940,9 +940,10 @@ static void lpss_iosf_exit_d3_state(void
 	mutex_unlock(_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)
@@ -955,13 +956,14 @@ static int acpi_lpss_suspend(struct devi
 	 * 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 && 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;
@@ -970,7 +972,8 @@ static int acpi_lpss_resume(struct devic
 	 * 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 && lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON &&
+	iosf_mbi_available())
 		lpss_iosf_exit_d3_state();
 
 	ret = acpi_dev_resume(dev);
@@ -994,12 +997,12 @@ static int acpi_lpss_suspend_late(struct
 		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);
 }
@@ -1014,7 +1017,7 @@ static int acpi_lpss_runtime_suspend(str
 
 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);
 }


Re: Possible regression caused by commit a192aa923b66a

2018-06-11 Thread Rafael J. Wysocki
On Mon, Jun 11, 2018 at 10:09 AM, Rafael J. Wysocki  wrote:
> On Mon, Jun 11, 2018 at 8:26 AM, Kai-Heng Feng
>  wrote:
>> Hi Rafael,
>>
>> There's a regression report [1] that says commit a192aa923b66a ("ACPI /
>> LPSS: Consolidate runtime PM and system sleep handling") is the first bad
>> commit.
>>
>> From the looks of it, it didn't introduce any behavioral change. So your
>> help is appreciated.
>>
>> [1] https://bugs.launchpad.net/bugs/1774950
>
> Well, the only difference is the iosf quirk AFAICS, but that should be
> easy enough to check.  I'll try to cut a patch for that later today.

If the iosf quirk is the source of the problem, the attached patch should help.
---
 drivers/acpi/acpi_lpss.c |   17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

Index: linux-pm/drivers/acpi/acpi_lpss.c
===
--- linux-pm.orig/drivers/acpi/acpi_lpss.c
+++ linux-pm/drivers/acpi/acpi_lpss.c
@@ -940,9 +940,10 @@ static void lpss_iosf_exit_d3_state(void
 	mutex_unlock(_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)
@@ -955,13 +956,14 @@ static int acpi_lpss_suspend(struct devi
 	 * 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 && 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;
@@ -970,7 +972,8 @@ static int acpi_lpss_resume(struct devic
 	 * 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 && lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON &&
+	iosf_mbi_available())
 		lpss_iosf_exit_d3_state();
 
 	ret = acpi_dev_resume(dev);
@@ -994,12 +997,12 @@ static int acpi_lpss_suspend_late(struct
 		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);
 }
@@ -1014,7 +1017,7 @@ static int acpi_lpss_runtime_suspend(str
 
 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);
 }


Re: Possible regression caused by commit a192aa923b66a

2018-06-11 Thread Rafael J. Wysocki
On Mon, Jun 11, 2018 at 8:26 AM, Kai-Heng Feng
 wrote:
> Hi Rafael,
>
> There's a regression report [1] that says commit a192aa923b66a ("ACPI /
> LPSS: Consolidate runtime PM and system sleep handling") is the first bad
> commit.
>
> From the looks of it, it didn't introduce any behavioral change. So your
> help is appreciated.
>
> [1] https://bugs.launchpad.net/bugs/1774950

Well, the only difference is the iosf quirk AFAICS, but that should be
easy enough to check.  I'll try to cut a patch for that later today.


Re: Possible regression caused by commit a192aa923b66a

2018-06-11 Thread Rafael J. Wysocki
On Mon, Jun 11, 2018 at 8:26 AM, Kai-Heng Feng
 wrote:
> Hi Rafael,
>
> There's a regression report [1] that says commit a192aa923b66a ("ACPI /
> LPSS: Consolidate runtime PM and system sleep handling") is the first bad
> commit.
>
> From the looks of it, it didn't introduce any behavioral change. So your
> help is appreciated.
>
> [1] https://bugs.launchpad.net/bugs/1774950

Well, the only difference is the iosf quirk AFAICS, but that should be
easy enough to check.  I'll try to cut a patch for that later today.