Skip to contentSkip to conversation

platform/x86: bitland-mifs-wmi: Don't abort suspend when platform profile access fails

PATCH In review Chris Taraszka 2 participants 4 messages Sep 11 – Sep 13

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 platform/x86: bitland-mifs-wmi: Don't abort suspend when platform profile access fails

On a Xiaomi Book Pro 14 (BIOS XMAPT4B0P0909), bitland_mifs_wmi_suspend() intermittently fails to read the platform profile and returns -EINVAL to the PM core, which aborts the entire system suspend:

bitland-mifs-wmi B60BFB48-...-4: PM: dpm_run_callback():
bitland_mifs_wmi_suspend [bitland_mifs_wmi] returns -22
bitland-mifs-wmi B60BFB48-...-4: PM: failed to suspend: error -22
PM: Some devices failed to suspend, or early wake event detected

The WMI method call itself succeeds. The WMI core reports its own failures as -EIO, -ENOMSG, -ENODATA or -EPROTO, so the -EINVAL comes from laptop_profile_get() when the SystemPerMode value returned by the firmware is not one of the four documented modes.

How often this happens depends on firmware state. On a v7.2.2 boot the query failed persistently on battery: /sys/power/suspend_stats reported 4628 failures against 2 successes, both of those on AC. Because logind re-issues the suspend while the lid stays closed, the machine looped awake and drained half the battery over one 8h42m lid-closed period at ~7-8 W. On v7.3-rc2, whose driver, WMI core and platform_profile code are unchanged from v7.2, the query has so far failed only right after boot, when power-profiles-daemon first reads the profile.

Commit d3666875c75e ("platform/x86: bitland-mifs-wmi: Fix NULL pointer dereference during suspend/resume") added a !data->pp_dev guard, but that only covers the event device. On the control device pp_dev is valid, so the guard does not apply and the error is returned verbatim.

Saving the platform profile is best-effort. Failing to read it should not keep the system awake, so warn and continue instead, in line with that commit skipping profile operations rather than failing the transition. Do the same on resume: laptop_profile_set() returns -EOPNOTSUPP for the performance and full-speed modes without DC power (see Documentation/wmi/devices/bitland-mifs-wmi.rst), so a profile saved on DC power cannot be restored after unplugging during suspend, which would otherwise mark the resume as failed.

Fixes dc1ec4fa86b2 ("platform/x86: bitland-mifs-wmi: Add new Bitland MIFS WMI driver")

---
Tested on the same machine with v7.3-rc2 plus this patch: 11 s2idle suspend cycles, including lid-closed suspends on battery of 7h42m, 4.9h and 75.8h, and repeated pm_test=devices cycles on battery all completed without errors. The failing firmware state could not be reproduced on demand on v7.3-rc2, so the new warnings have not been observed firing during a suspend; the failures described above were recorded on v7.2.2 without this patch.

 drivers/platform/x86/bitland-mifs-wmi.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)
drivers/platform/x86/bitland-mifs-wmi.c +13 -4
index 3a37318..788ef14 100644
@@ -305,22 +305,31 @@ static int bitland_mifs_wmi_suspend(struct device *dev)
 		return 0;
 
 	ret = laptop_profile_get(data->pp_dev, &profile);
-	if (ret == 0)
-		data->saved_profile = profile;
+	if (ret) {
+		dev_warn(dev, "Failed to save platform profile: %d\n", ret);
+		return 0;
+	}
 
-	return ret;
+	data->saved_profile = profile;
+
+	return 0;
 }
 
 static int bitland_mifs_wmi_resume(struct device *dev)
 {
 	struct bitland_mifs_wmi_data *data = dev_get_drvdata(dev);
+	int ret;
 
 	/* Skip event device */
 	if (!data->pp_dev)
 		return 0;
 
 	dev_dbg(dev, "Resuming, restoring profile %d\n", data->saved_profile);
-	return laptop_profile_set(dev, data->saved_profile);
+	ret = laptop_profile_set(dev, data->saved_profile);
+	if (ret)
+		dev_warn(dev, "Failed to restore platform profile: %d\n", ret);
+
+	return 0;
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(bitland_mifs_wmi_pm_ops,
Signature · 1 line
2.55.0
#1 lore raw

Am 11.09.26 um 20:21 schrieb Chris Taraszka:

Quoted 28 lines

On a Xiaomi Book Pro 14 (BIOS XMAPT4B0P0909), bitland_mifs_wmi_suspend() intermittently fails to read the platform profile and returns -EINVAL to the PM core, which aborts the entire system suspend:

bitland-mifs-wmi B60BFB48-...-4: PM: dpm_run_callback():
bitland_mifs_wmi_suspend [bitland_mifs_wmi] returns -22
bitland-mifs-wmi B60BFB48-...-4: PM: failed to suspend: error -22
PM: Some devices failed to suspend, or early wake event detected

The WMI method call itself succeeds. The WMI core reports its own failures as -EIO, -ENOMSG, -ENODATA or -EPROTO, so the -EINVAL comes from laptop_profile_get() when the SystemPerMode value returned by the firmware is not one of the four documented modes.

How often this happens depends on firmware state. On a v7.2.2 boot the query failed persistently on battery: /sys/power/suspend_stats reported 4628 failures against 2 successes, both of those on AC. Because logind re-issues the suspend while the lid stays closed, the machine looped awake and drained half the battery over one 8h42m lid-closed period at ~7-8 W. On v7.3-rc2, whose driver, WMI core and platform_profile code are unchanged from v7.2, the query has so far failed only right after boot, when power-profiles-daemon first reads the profile.

Commit d3666875c75e ("platform/x86: bitland-mifs-wmi: Fix NULL pointer dereference during suspend/resume") added a !data->pp_dev guard, but that only covers the event device. On the control device pp_dev is valid, so the guard does not apply and the error is returned verbatim.

Quoted 8 lines

Saving the platform profile is best-effort. Failing to read it should not keep the system awake, so warn and continue instead, in line with that commit skipping profile operations rather than failing the transition. Do the same on resume: laptop_profile_set() returns -EOPNOTSUPP for the performance and full-speed modes without DC power (see Documentation/wmi/devices/bitland-mifs-wmi.rst), so a profile saved on DC power cannot be restored after unplugging during suspend, which would otherwise mark the resume as failed.

Hi,

i think the root cause for this is that your device uses a different set of platform profiles. Can you share the output of "acpidump"?

Thanks,
Armin Wolf

Quoted 55 lines
Fixes dc1ec4fa86b2 ("platform/x86: bitland-mifs-wmi: Add new Bitland MIFS WMI driver")

---
Tested on the same machine with v7.3-rc2 plus this patch: 11 s2idle suspend cycles, including lid-closed suspends on battery of 7h42m, 4.9h and 75.8h, and repeated pm_test=devices cycles on battery all completed without errors. The failing firmware state could not be reproduced on demand on v7.3-rc2, so the new warnings have not been observed firing during a suspend; the failures described above were recorded on v7.2.2 without this patch.

  drivers/platform/x86/bitland-mifs-wmi.c | 17 +++++++++++++----
  1 file changed, 13 insertions(+), 4 deletions(-)
drivers/platform/x86/bitland-mifs-wmi.c +13 -4
index 3a37318..788ef14 100644
@@ -305,22 +305,31 @@ static int bitland_mifs_wmi_suspend(struct device *dev)
  		return 0;
 
  	ret = laptop_profile_get(data->pp_dev, &profile);
-	if (ret == 0)
-		data->saved_profile = profile;
+	if (ret) {
+		dev_warn(dev, "Failed to save platform profile: %d\n", ret);
+		return 0;
+	}
 
-	return ret;
+	data->saved_profile = profile;
+
+	return 0;
  }
 
  static int bitland_mifs_wmi_resume(struct device *dev)
  {
  	struct bitland_mifs_wmi_data *data = dev_get_drvdata(dev);
+	int ret;
 
  	/* Skip event device */
  	if (!data->pp_dev)
  		return 0;
 
  	dev_dbg(dev, "Resuming, restoring profile %d\n", data->saved_profile);
-	return laptop_profile_set(dev, data->saved_profile);
+	ret = laptop_profile_set(dev, data->saved_profile);
+	if (ret)
+		dev_warn(dev, "Failed to restore platform profile: %d\n", ret);
+
+	return 0;
  }
 
  static DEFINE_SIMPLE_DEV_PM_OPS(bitland_mifs_wmi_pm_ops,
#2 lore raw

Hi,

gzip attached (~702K rather than 3.6M)

Xiaomi Book Pro 14 (sys_vendor XIAOMI), BIOS XMAPT4B0P0909 /sys/firmware/acpi/platform_profile_choices: low-power balanced performance

Quoted 110 lines · Armin Wolf

Am 11.09.26 um 20:21 schrieb Chris Taraszka:

Quoted 28 lines

On a Xiaomi Book Pro 14 (BIOS XMAPT4B0P0909), bitland_mifs_wmi_suspend() intermittently fails to read the platform profile and returns -EINVAL to the PM core, which aborts the entire system suspend:

bitland-mifs-wmi B60BFB48-...-4: PM: dpm_run_callback():
bitland_mifs_wmi_suspend [bitland_mifs_wmi] returns -22
bitland-mifs-wmi B60BFB48-...-4: PM: failed to suspend: error -22
PM: Some devices failed to suspend, or early wake event detected

The WMI method call itself succeeds. The WMI core reports its own failures as -EIO, -ENOMSG, -ENODATA or -EPROTO, so the -EINVAL comes from laptop_profile_get() when the SystemPerMode value returned by the firmware is not one of the four documented modes.

How often this happens depends on firmware state. On a v7.2.2 boot the query failed persistently on battery: /sys/power/suspend_stats reported 4628 failures against 2 successes, both of those on AC. Because logind re-issues the suspend while the lid stays closed, the machine looped awake and drained half the battery over one 8h42m lid-closed period at ~7-8 W. On v7.3-rc2, whose driver, WMI core and platform_profile code are unchanged from v7.2, the query has so far failed only right after boot, when power-profiles-daemon first reads the profile.

Commit d3666875c75e ("platform/x86: bitland-mifs-wmi: Fix NULL pointer dereference during suspend/resume") added a !data->pp_dev guard, but that only covers the event device. On the control device pp_dev is valid, so the guard does not apply and the error is returned verbatim.

Quoted 8 lines

Saving the platform profile is best-effort. Failing to read it should not keep the system awake, so warn and continue instead, in line with that commit skipping profile operations rather than failing the transition. Do the same on resume: laptop_profile_set() returns -EOPNOTSUPP for the performance and full-speed modes without DC power (see Documentation/wmi/devices/bitland-mifs-wmi.rst), so a profile saved on DC power cannot be restored after unplugging during suspend, which would otherwise mark the resume as failed.

Hi,

i think the root cause for this is that your device uses a different set of platform profiles.
Can you share the output of "acpidump"?

Thanks,
Armin Wolf

Quoted 1 line
Fixes dc1ec4fa86b2 ("platform/x86: bitland-mifs-wmi: Add new Bitland

MIFS WMI driver")

Quoted 11 lines

---
Tested on the same machine with v7.3-rc2 plus this patch: 11 s2idle suspend cycles, including lid-closed suspends on battery of 7h42m, 4.9h and 75.8h, and repeated pm_test=devices cycles on battery all completed without errors. The failing firmware state could not be reproduced on demand on v7.3-rc2, so the new warnings have not been observed firing during a suspend; the failures described above were recorded on v7.2.2 without this patch.

Review

Quoted 4 lines
  drivers/platform/x86/bitland-mifs-wmi.c | 17 +++++++++++++----
  1 file changed, 13 insertions(+), 4 deletions(-)
(quoted diff) +0 -0

b/drivers/platform/x86/bitland-mifs-wmi.c

drivers/platform/x86/bitland-mifs-wmi.c static int bitland_mifs_wmi_suspend(struct device

Quoted 4 lines

index 3a37318..788ef14 100644

drivers/platform/x86/bitland-mifs-wmi.c +0 -0
@@ -305,22 +305,31 @@ static int bitland_mifs_wmi_suspend(struct device

*dev)

drivers/platform/x86/bitland-mifs-wmi.c static int bitland_mifs_wmi_suspend(struct device *dev)

Quoted 7 lines
drivers/platform/x86/bitland-mifs-wmi.c +2 -2
              return 0;
 
      ret = laptop_profile_get(data->pp_dev, &profile);
-     if (ret == 0)
-             data->saved_profile = profile;
+     if (ret) {
+             dev_warn(dev, "Failed to save platform profile: %d\n",

ret);

Quoted 9 lines
drivers/platform/x86/bitland-mifs-wmi.c +5 -1
+             return 0;
+     }
 
-     return ret;
+     data->saved_profile = profile;
+
+     return 0;
  }
 

drivers/platform/x86/bitland-mifs-wmi.c static int bitland_mifs_wmi_suspend(struct device *dev)

Quoted 10 lines
drivers/platform/x86/bitland-mifs-wmi.c +1 -0
  static int bitland_mifs_wmi_resume(struct device *dev)
  {
      struct bitland_mifs_wmi_data *data = dev_get_drvdata(dev);
+     int ret;
 
      /* Skip event device */
      if (!data->pp_dev)
              return 0;
 
      dev_dbg(dev, "Resuming, restoring profile %d\n",

data->saved_profile);

drivers/platform/x86/bitland-mifs-wmi.c static int bitland_mifs_wmi_suspend(struct device *dev)

Quoted 4 lines
drivers/platform/x86/bitland-mifs-wmi.c +3 -1
-     return laptop_profile_set(dev, data->saved_profile);
+     ret = laptop_profile_set(dev, data->saved_profile);
+     if (ret)
+             dev_warn(dev, "Failed to restore platform profile: %d\n",

ret);

Quoted 5 lines
drivers/platform/x86/bitland-mifs-wmi.c +2 -0
+
+     return 0;
  }
 
  static DEFINE_SIMPLE_DEV_PM_OPS(bitland_mifs_wmi_pm_ops,
#3 lore raw

Am 12.09.26 um 20:16 schrieb Chris Taraszka:

Quoted 7 lines

Hi,

gzip attached (~702K rather than 3.6M)

Xiaomi Book Pro 14 (sys_vendor XIAOMI), BIOS XMAPT4B0P0909 /sys/firmware/acpi/platform_profile_choices: low-power balanced performance

Thanks, it seems that the root cause for all of this is that your device uses different values for communicating the desired platform profile to the underlying ACPI firmware. I strongly suspect that the platform profile mechanism will currently not work on your device at all.

Some other users have already posted patch to work around this by implementing a DMI whitelist, you can find those patches in the archives. I currently have a patch series pending that fixes an issue inside the underlying WMI interface, after that i can take care of this problem.

For now i suggest that you blacklist the driver until the DMI whitelist has been integrated.

Thanks,
Armin Wolf

Quoted 133 lines · Armin Wolf
    Am 11.09.26 um 20:21 schrieb Chris Taraszka:

    > On a Xiaomi Book Pro 14 (BIOS XMAPT4B0P0909),
    bitland_mifs_wmi_suspend()
    > intermittently fails to read the platform profile and returns
    -EINVAL to
    > the PM core, which aborts the entire system suspend:
    >
    >    bitland-mifs-wmi B60BFB48-...-4: PM: dpm_run_callback():
    >      bitland_mifs_wmi_suspend [bitland_mifs_wmi] returns -22
    >    bitland-mifs-wmi B60BFB48-...-4: PM: failed to suspend: error -22
    >    PM: Some devices failed to suspend, or early wake event detected
    >
    > The WMI method call itself succeeds. The WMI core reports its own
    > failures as -EIO, -ENOMSG, -ENODATA or -EPROTO, so the -EINVAL
    comes from
    > laptop_profile_get() when the SystemPerMode value returned by the
    > firmware is not one of the four documented modes.
    >
    > How often this happens depends on firmware state. On a v7.2.2
    boot the
    > query failed persistently on battery: /sys/power/suspend_stats
    reported
    > 4628 failures against 2 successes, both of those on AC. Because
    logind
    > re-issues the suspend while the lid stays closed, the machine looped
    > awake and drained half the battery over one 8h42m lid-closed
    period at
    > ~7-8 W. On v7.3-rc2, whose driver, WMI core and platform_profile
    code are
    > unchanged from v7.2, the query has so far failed only right
    after boot,
    > when power-profiles-daemon first reads the profile.
    >
    > Commit d3666875c75e ("platform/x86: bitland-mifs-wmi: Fix NULL
    pointer
    > dereference during suspend/resume") added a !data->pp_dev guard, but
    > that only covers the event device. On the control device pp_dev is
    > valid, so the guard does not apply and the error is returned
    verbatim.
    >
    > Saving the platform profile is best-effort. Failing to read it
    should
    > not keep the system awake, so warn and continue instead, in line
    with
    > that commit skipping profile operations rather than failing the
    > transition. Do the same on resume: laptop_profile_set() returns
    > -EOPNOTSUPP for the performance and full-speed modes without DC
    power
    > (see Documentation/wmi/devices/bitland-mifs-wmi.rst), so a
    profile saved
    > on DC power cannot be restored after unplugging during suspend,
    which
    > would otherwise mark the resume as failed.

    Hi,

    i think the root cause for this is that your device uses a
    different set of platform profiles.
    Can you share the output of "acpidump"?

    Thanks,
    Armin Wolf

    > Fixes: dc1ec4fa86b2 ("platform/x86: bitland-mifs-wmi: Add new
    Bitland MIFS WMI driver")
    > Cc: [email protected]
    > Signed-off-by: Chris Taraszka <[email protected]>
    > ---
    > Tested on the same machine with v7.3-rc2 plus this patch: 11 s2idle
    > suspend cycles, including lid-closed suspends on battery of
    7h42m, 4.9h
    > and 75.8h, and repeated pm_test=devices cycles on battery all
    completed
    > without errors. The failing firmware state could not be
    reproduced on
    > demand on v7.3-rc2, so the new warnings have not been observed
    firing
    > during a suspend; the failures described above were recorded on
    v7.2.2
    > without this patch.
    >
    >   drivers/platform/x86/bitland-mifs-wmi.c | 17 +++++++++++++----
    >   1 file changed, 13 insertions(+), 4 deletions(-)
    >
    > diff --git a/drivers/platform/x86/bitland-mifs-wmi.c
    b/drivers/platform/x86/bitland-mifs-wmi.c
    > index 3a37318..788ef14 100644
    > --- a/drivers/platform/x86/bitland-mifs-wmi.c
    > +++ b/drivers/platform/x86/bitland-mifs-wmi.c
    > @@ -305,22 +305,31 @@ static int bitland_mifs_wmi_suspend(struct
    device *dev)
    >               return 0;
    >
    >       ret = laptop_profile_get(data->pp_dev, &profile);
    > -     if (ret == 0)
    > -             data->saved_profile = profile;
    > +     if (ret) {
    > +             dev_warn(dev, "Failed to save platform profile:
    %d\n", ret);
    > +             return 0;
    > +     }
    >
    > -     return ret;
    > +     data->saved_profile = profile;
    > +
    > +     return 0;
    >   }
    >
    >   static int bitland_mifs_wmi_resume(struct device *dev)
    >   {
    >       struct bitland_mifs_wmi_data *data = dev_get_drvdata(dev);
    > +     int ret;
    >
    >       /* Skip event device */
    >       if (!data->pp_dev)
    >               return 0;
    >
    >       dev_dbg(dev, "Resuming, restoring profile %d\n",
    data->saved_profile);
    > -     return laptop_profile_set(dev, data->saved_profile);
    > +     ret = laptop_profile_set(dev, data->saved_profile);
    > +     if (ret)
    > +             dev_warn(dev, "Failed to restore platform profile:
    %d\n", ret);
    > +
    > +     return 0;
    >   }
    >
    >   static DEFINE_SIMPLE_DEV_PM_OPS(bitland_mifs_wmi_pm_ops,