linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] HP EliteBook 855 G7 WWAN modem power resource quirk
@ 2025-08-03 19:18 Maciej S. Szmigiero
  2025-08-03 19:18 ` [PATCH v3 1/2] ACPI: PM: Add power resource init function Maciej S. Szmigiero
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Maciej S. Szmigiero @ 2025-08-03 19:18 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown; +Cc: linux-acpi, linux-kernel

This laptop (and possibly similar models too) has power resource called
"GP12.PXP_" for its Intel XMM7360 WWAN modem.

For this power resource to turn ON power for the modem it needs certain
internal flag called "ONEN" to be set:
Method (_ON, 0, NotSerialized) // _ON_: Power On
{
	If (^^^LPCB.EC0.ECRG)
	{
		If ((ONEN == Zero))
		{
                        Return (Zero)
		}
(..)
	}
}

This flag only gets set from this power resource "_OFF" method, while the
actual modem power gets turned off during suspend by "GP12.PTS" method
called from the global "_PTS" (Prepare To Sleep) method.

In fact, this power resource "_OFF" method implementation just sets the
aforementioned flag:
Method (_OFF, 0, NotSerialized) // _OFF: Power Off
{
	OFEN = Zero
	ONEN = One
}

Upon hibernation finish we try to set this power resource back ON since its
"_STA" method returns 0 and the resource is still considered in use as it
is declared as required for D0 for both the modem ACPI device (GP12.PWAN)
and its parent PCIe port ACPI device (GP12).
But the "_ON" method won't do anything since that "ONEN" flag is not set.

Overall, this means the modem is dead after hibernation finish until the
laptop is rebooted since the modem power has been cut by "_PTS" and its PCI
configuration was lost and not able to be restored.

The easiest way to workaround this issue is to call this power resource
"_OFF" method before calling the "_ON" method to make sure the "ONEN"
flag gets properly set.

This makes the modem alive once again after hibernation finish - with
properly restored PCI configuration space.

Since this platform does *not* support S3 the fact that
acpi_resume_power_resources() is also called during resume from S3 is
not a problem there.

Do the DMI based quirk matching and quirk flag initialization just
once - in acpi_power_resources_init() function similar to existing
acpi_*_init() functions.

This way the whole resume path overhead of this change on other systems
amounts to simple hp_eb_gp12pxp_quirk flag comparison.

Opportunistically convert the single already existing DMI match-based
quirk in this ACPI power resource handler ("leave unused power
resources on" quirk) to the same one-time initialization in
acpi_power_resources_init() function instead of re-running that DMI
match each time acpi_turn_off_unused_power_resources() gets called.


This Intel WWAN modem in general has *a lot* of issues with
suspend/resume on various laptop platforms (not only HP).

More patches are needed for these, hopefully they can be mainlined
too so suspend/resume work out of the box for users (that's
important functionality on a laptop).

See the following ModemManager issue containing patches also for
Thinkpad T14 G1 and Dell Precision 3561:
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/issues/992


Changes from v2:
* Split out the change adding power resource init function and converting
  existing DMI-based quirk into a separate preparatory patch.

* Create a replacement __acpi_power_on() method for the affected power
  resource (including the power OFF and delay part) and call it from
  acpi_resume_power_resources() instead of ordinary __acpi_power_on()
  on the affected platform.

* Rename leave_unused_power_resources_on_quirk into suggested shorter
  unused_power_resources_quirk.


Maciej S. Szmigiero (2):
  ACPI: PM: Add power resource init function
  ACPI: PM: Add HP EliteBook 855 G7 WWAN modem power resource quirk

 drivers/acpi/internal.h |  1 +
 drivers/acpi/power.c    | 90 +++++++++++++++++++++++++++++++++++++++--
 drivers/acpi/scan.c     |  1 +
 3 files changed, 89 insertions(+), 3 deletions(-)


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3 1/2] ACPI: PM: Add power resource init function
  2025-08-03 19:18 [PATCH v3 0/2] HP EliteBook 855 G7 WWAN modem power resource quirk Maciej S. Szmigiero
@ 2025-08-03 19:18 ` Maciej S. Szmigiero
  2025-08-03 19:18 ` [PATCH v3 2/2] ACPI: PM: Add HP EliteBook 855 G7 WWAN modem power resource quirk Maciej S. Szmigiero
  2025-08-25 14:27 ` [PATCH v3 0/2] " Rafael J. Wysocki
  2 siblings, 0 replies; 4+ messages in thread
From: Maciej S. Szmigiero @ 2025-08-03 19:18 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown; +Cc: linux-acpi, linux-kernel

This way DMI based quirk matching and quirk flag initialization can be done
just once - in the newly introduced acpi_power_resources_init() function,
which is similar to existing acpi_*_init() functions.

Convert the single already existing DMI match-based quirk in this ACPI
power resource handler ("leave unused power resources on" quirk) to such
one-time initialization in acpi_power_resources_init() function instead of
re-running that DMI match each time acpi_turn_off_unused_power_resources()
gets called.

Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 drivers/acpi/internal.h |  1 +
 drivers/acpi/power.c    | 10 +++++++++-
 drivers/acpi/scan.c     |  1 +
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index e2781864fdce..63354972ab0b 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -140,6 +140,7 @@ int __acpi_device_uevent_modalias(const struct acpi_device *adev,
 /* --------------------------------------------------------------------------
                                   Power Resource
    -------------------------------------------------------------------------- */
+void acpi_power_resources_init(void);
 void acpi_power_resources_list_free(struct list_head *list);
 int acpi_extract_power_resources(union acpi_object *package, unsigned int start,
 				 struct list_head *list);
diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
index b7243d7563b1..cd9380b1f951 100644
--- a/drivers/acpi/power.c
+++ b/drivers/acpi/power.c
@@ -63,6 +63,8 @@ struct acpi_power_resource_entry {
 	struct acpi_power_resource *resource;
 };
 
+static bool unused_power_resources_quirk;
+
 static LIST_HEAD(acpi_power_resource_list);
 static DEFINE_MUTEX(power_resource_list_lock);
 
@@ -1046,7 +1048,7 @@ void acpi_turn_off_unused_power_resources(void)
 {
 	struct acpi_power_resource *resource;
 
-	if (dmi_check_system(dmi_leave_unused_power_resources_on))
+	if (unused_power_resources_quirk)
 		return;
 
 	mutex_lock(&power_resource_list_lock);
@@ -1065,3 +1067,9 @@ void acpi_turn_off_unused_power_resources(void)
 
 	mutex_unlock(&power_resource_list_lock);
 }
+
+void __init acpi_power_resources_init(void)
+{
+	unused_power_resources_quirk =
+		dmi_check_system(dmi_leave_unused_power_resources_on);
+}
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index fb1fe9f3b1a3..bb74e7834435 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2702,6 +2702,7 @@ void __init acpi_scan_init(void)
 	acpi_memory_hotplug_init();
 	acpi_watchdog_init();
 	acpi_pnp_init();
+	acpi_power_resources_init();
 	acpi_int340x_thermal_init();
 	acpi_init_lpit();
 

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v3 2/2] ACPI: PM: Add HP EliteBook 855 G7 WWAN modem power resource quirk
  2025-08-03 19:18 [PATCH v3 0/2] HP EliteBook 855 G7 WWAN modem power resource quirk Maciej S. Szmigiero
  2025-08-03 19:18 ` [PATCH v3 1/2] ACPI: PM: Add power resource init function Maciej S. Szmigiero
@ 2025-08-03 19:18 ` Maciej S. Szmigiero
  2025-08-25 14:27 ` [PATCH v3 0/2] " Rafael J. Wysocki
  2 siblings, 0 replies; 4+ messages in thread
From: Maciej S. Szmigiero @ 2025-08-03 19:18 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown; +Cc: linux-acpi, linux-kernel

This laptop (and possibly similar models too) has power resource called
"GP12.PXP_" for its Intel XMM7360 WWAN modem.

For this power resource to turn ON power for the modem it needs certain
internal flag called "ONEN" to be set:
Method (_ON, 0, NotSerialized) // _ON_: Power On
{
	If (^^^LPCB.EC0.ECRG)
	{
		If ((ONEN == Zero))
		{
                        Return (Zero)
		}
(..)
	}
}

This flag only gets set from this power resource "_OFF" method, while the
actual modem power gets turned off during suspend by "GP12.PTS" method
called from the global "_PTS" (Prepare To Sleep) method.

In fact, this power resource "_OFF" method implementation just sets the
aforementioned flag:
Method (_OFF, 0, NotSerialized) // _OFF: Power Off
{
	OFEN = Zero
	ONEN = One
}

Upon hibernation finish we try to set this power resource back ON since its
"_STA" method returns 0 and the resource is still considered in use as it
is declared as required for D0 for both the modem ACPI device (GP12.PWAN)
and its parent PCIe port ACPI device (GP12).
But the "_ON" method won't do anything since that "ONEN" flag is not set.

Overall, this means the modem is dead after hibernation finish until the
laptop is rebooted since the modem power has been cut by "_PTS" and its PCI
configuration was lost and not able to be restored.

The easiest way to workaround this issue is to call this power resource
"_OFF" method before calling the "_ON" method to make sure the "ONEN"
flag gets properly set.

This makes the modem alive once again after hibernation finish - with
properly restored PCI configuration space.

Since this platform does *not* support S3 the fact that
acpi_resume_power_resources() is also called during resume from S3 is
not a problem there.

Do the DMI based quirk matching and quirk flag initialization just
once - in acpi_power_resources_init() function.

This way the whole resume path overhead of this change on other systems
amounts to simple hp_eb_gp12pxp_quirk flag comparison.

Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 drivers/acpi/power.c | 80 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 78 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
index cd9380b1f951..361a7721a6a8 100644
--- a/drivers/acpi/power.c
+++ b/drivers/acpi/power.c
@@ -23,6 +23,7 @@
 
 #define pr_fmt(fmt) "ACPI: PM: " fmt
 
+#include <linux/delay.h>
 #include <linux/dmi.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -63,6 +64,7 @@ struct acpi_power_resource_entry {
 	struct acpi_power_resource *resource;
 };
 
+static bool hp_eb_gp12pxp_quirk;
 static bool unused_power_resources_quirk;
 
 static LIST_HEAD(acpi_power_resource_list);
@@ -994,6 +996,38 @@ struct acpi_device *acpi_add_power_resource(acpi_handle handle)
 }
 
 #ifdef CONFIG_ACPI_SLEEP
+static bool resource_is_gp12pxp(acpi_handle handle)
+{
+	const char *path;
+	bool ret;
+
+	path = acpi_handle_path(handle);
+	ret = path && strcmp(path, "\\_SB_.PCI0.GP12.PXP_") == 0;
+	kfree(path);
+
+	return ret;
+}
+
+static void acpi_resume_on_eb_gp12pxp(struct acpi_power_resource *resource)
+{
+	acpi_handle_notice(resource->device.handle,
+			   "HP EB quirk - turning OFF then ON\n");
+
+	__acpi_power_off(resource);
+	__acpi_power_on(resource);
+
+	/*
+	 * Use the same delay as DSDT uses in modem _RST method.
+	 *
+	 * Otherwise we get "Unable to change power state from unknown to D0,
+	 * device inaccessible" error for the modem PCI device after thaw.
+	 *
+	 * This power resource is normally being enabled only during thaw (once)
+	 * so this wait is not a performance issue.
+	 */
+	msleep(200);
+}
+
 void acpi_resume_power_resources(void)
 {
 	struct acpi_power_resource *resource;
@@ -1015,8 +1049,14 @@ void acpi_resume_power_resources(void)
 
 		if (state == ACPI_POWER_RESOURCE_STATE_OFF
 		    && resource->ref_count) {
-			acpi_handle_debug(resource->device.handle, "Turning ON\n");
-			__acpi_power_on(resource);
+			if (hp_eb_gp12pxp_quirk &&
+			    resource_is_gp12pxp(resource->device.handle)) {
+				acpi_resume_on_eb_gp12pxp(resource);
+			} else {
+				acpi_handle_debug(resource->device.handle,
+						  "Turning ON\n");
+				__acpi_power_on(resource);
+			}
 		}
 
 		mutex_unlock(&resource->resource_lock);
@@ -1026,6 +1066,41 @@ void acpi_resume_power_resources(void)
 }
 #endif
 
+static const struct dmi_system_id dmi_hp_elitebook_gp12pxp_quirk[] = {
+/*
+ * This laptop (and possibly similar models too) has power resource called
+ * "GP12.PXP_" for its WWAN modem.
+ *
+ * For this power resource to turn ON power for the modem it needs certain
+ * internal flag called "ONEN" to be set.
+ * This flag only gets set from this power resource "_OFF" method, while the
+ * actual modem power gets turned off during suspend by "GP12.PTS" method
+ * called from the global "_PTS" (Prepare To Sleep) method.
+ * On the other hand, this power resource "_OFF" method implementation just
+ * sets the aforementioned flag without actually doing anything else (it
+ * doesn't contain any code to actually turn off power).
+ *
+ * The above means that when upon hibernation finish we try to set this
+ * power resource back ON since its "_STA" method returns 0 (while the resource
+ * is still considered in use) its "_ON" method won't do anything since
+ * that "ONEN" flag is not set.
+ * Overall, this means the modem is dead until laptop is rebooted since its
+ * power has been cut by "_PTS" and its PCI configuration was lost and not able
+ * to be restored.
+ *
+ * The easiest way to workaround the issue is to call this power resource
+ * "_OFF" method before calling the "_ON" method to make sure the "ONEN"
+ * flag gets properly set.
+ */
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "HP"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook 855 G7 Notebook PC"),
+		},
+	},
+	{}
+};
+
 static const struct dmi_system_id dmi_leave_unused_power_resources_on[] = {
 	{
 		/*
@@ -1070,6 +1145,7 @@ void acpi_turn_off_unused_power_resources(void)
 
 void __init acpi_power_resources_init(void)
 {
+	hp_eb_gp12pxp_quirk = dmi_check_system(dmi_hp_elitebook_gp12pxp_quirk);
 	unused_power_resources_quirk =
 		dmi_check_system(dmi_leave_unused_power_resources_on);
 }

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 0/2] HP EliteBook 855 G7 WWAN modem power resource quirk
  2025-08-03 19:18 [PATCH v3 0/2] HP EliteBook 855 G7 WWAN modem power resource quirk Maciej S. Szmigiero
  2025-08-03 19:18 ` [PATCH v3 1/2] ACPI: PM: Add power resource init function Maciej S. Szmigiero
  2025-08-03 19:18 ` [PATCH v3 2/2] ACPI: PM: Add HP EliteBook 855 G7 WWAN modem power resource quirk Maciej S. Szmigiero
@ 2025-08-25 14:27 ` Rafael J. Wysocki
  2 siblings, 0 replies; 4+ messages in thread
From: Rafael J. Wysocki @ 2025-08-25 14:27 UTC (permalink / raw)
  To: Maciej S. Szmigiero
  Cc: Rafael J. Wysocki, Len Brown, linux-acpi, linux-kernel

On Sun, Aug 3, 2025 at 9:18 PM Maciej S. Szmigiero
<mail@maciej.szmigiero.name> wrote:
>
> This laptop (and possibly similar models too) has power resource called
> "GP12.PXP_" for its Intel XMM7360 WWAN modem.
>
> For this power resource to turn ON power for the modem it needs certain
> internal flag called "ONEN" to be set:
> Method (_ON, 0, NotSerialized) // _ON_: Power On
> {
>         If (^^^LPCB.EC0.ECRG)
>         {
>                 If ((ONEN == Zero))
>                 {
>                         Return (Zero)
>                 }
> (..)
>         }
> }
>
> This flag only gets set from this power resource "_OFF" method, while the
> actual modem power gets turned off during suspend by "GP12.PTS" method
> called from the global "_PTS" (Prepare To Sleep) method.
>
> In fact, this power resource "_OFF" method implementation just sets the
> aforementioned flag:
> Method (_OFF, 0, NotSerialized) // _OFF: Power Off
> {
>         OFEN = Zero
>         ONEN = One
> }
>
> Upon hibernation finish we try to set this power resource back ON since its
> "_STA" method returns 0 and the resource is still considered in use as it
> is declared as required for D0 for both the modem ACPI device (GP12.PWAN)
> and its parent PCIe port ACPI device (GP12).
> But the "_ON" method won't do anything since that "ONEN" flag is not set.
>
> Overall, this means the modem is dead after hibernation finish until the
> laptop is rebooted since the modem power has been cut by "_PTS" and its PCI
> configuration was lost and not able to be restored.
>
> The easiest way to workaround this issue is to call this power resource
> "_OFF" method before calling the "_ON" method to make sure the "ONEN"
> flag gets properly set.
>
> This makes the modem alive once again after hibernation finish - with
> properly restored PCI configuration space.
>
> Since this platform does *not* support S3 the fact that
> acpi_resume_power_resources() is also called during resume from S3 is
> not a problem there.
>
> Do the DMI based quirk matching and quirk flag initialization just
> once - in acpi_power_resources_init() function similar to existing
> acpi_*_init() functions.
>
> This way the whole resume path overhead of this change on other systems
> amounts to simple hp_eb_gp12pxp_quirk flag comparison.
>
> Opportunistically convert the single already existing DMI match-based
> quirk in this ACPI power resource handler ("leave unused power
> resources on" quirk) to the same one-time initialization in
> acpi_power_resources_init() function instead of re-running that DMI
> match each time acpi_turn_off_unused_power_resources() gets called.
>
>
> This Intel WWAN modem in general has *a lot* of issues with
> suspend/resume on various laptop platforms (not only HP).
>
> More patches are needed for these, hopefully they can be mainlined
> too so suspend/resume work out of the box for users (that's
> important functionality on a laptop).
>
> See the following ModemManager issue containing patches also for
> Thinkpad T14 G1 and Dell Precision 3561:
> https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/issues/992
>
>
> Changes from v2:
> * Split out the change adding power resource init function and converting
>   existing DMI-based quirk into a separate preparatory patch.
>
> * Create a replacement __acpi_power_on() method for the affected power
>   resource (including the power OFF and delay part) and call it from
>   acpi_resume_power_resources() instead of ordinary __acpi_power_on()
>   on the affected platform.
>
> * Rename leave_unused_power_resources_on_quirk into suggested shorter
>   unused_power_resources_quirk.
>
>
> Maciej S. Szmigiero (2):
>   ACPI: PM: Add power resource init function
>   ACPI: PM: Add HP EliteBook 855 G7 WWAN modem power resource quirk
>
>  drivers/acpi/internal.h |  1 +
>  drivers/acpi/power.c    | 90 +++++++++++++++++++++++++++++++++++++++--
>  drivers/acpi/scan.c     |  1 +
>  3 files changed, 89 insertions(+), 3 deletions(-)

Both patches applied as 6.18 material with some minor edits in the
changelog of the second patch.

Thanks!

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-08-25 14:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-03 19:18 [PATCH v3 0/2] HP EliteBook 855 G7 WWAN modem power resource quirk Maciej S. Szmigiero
2025-08-03 19:18 ` [PATCH v3 1/2] ACPI: PM: Add power resource init function Maciej S. Szmigiero
2025-08-03 19:18 ` [PATCH v3 2/2] ACPI: PM: Add HP EliteBook 855 G7 WWAN modem power resource quirk Maciej S. Szmigiero
2025-08-25 14:27 ` [PATCH v3 0/2] " Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).