[PATCH v2 0/2] platform/x86: hp-wmi: Spectre charge behaviour

[PATCH v2 0/2] platform/x86: hp-wmi: Spectre charge behaviour

[Shang En Sim]

Address review of the Spectre charge_behaviour patch.

The hwmon keep-alive work is switched to devm_delayed_work_autocancel()
first, so the new error return from charge-control setup does not leave
that work pending after a failed probe.

v1: https://lore.kernel.org/r/20260812062113.27741-1-sim@shangen.org/

Changes in v2:
- Switch the hwmon keep-alive delayed work to devm_delayed_work_autocancel()
  and register it before the hwmon device (Ilpo Järvinen)
- Name the GBCO status field with GENMASK() and read it with FIELD_GET()
  (Ilpo Järvinen)
- Add HP Spectre x360 14-eu0xxx, board 8C15 (Alain Cousinie)

Shang En Sim (2):
  platform/x86: hp-wmi: Fix hwmon keep-alive teardown
  platform/x86: hp-wmi: add charge_behaviour support for Spectre
    8C15/8C17

 drivers/platform/x86/hp/hp-wmi.c | 265 ++++++++++++++++++++++++++++++-
 1 file changed, 258 insertions(+), 7 deletions(-)


base-commit: fe5030c8cc7156223f48530e9b49aa87c0305bcd
-- 
2.56.0

[PATCH v2 1/2] platform/x86: hp-wmi: Fix hwmon keep-alive teardown

[Shang En Sim]

hp_wmi_hwmon_init() initialises keep_alive_dwork with INIT_DELAYED_WORK()
and relies on hp_wmi_bios_remove() to cancel it. Nothing cancels the work
if probe fails after hp_wmi_hwmon_init() has run, so the devm-managed
priv can be freed while the work is still pending.

The ordering is also wrong on both ends. The work is initialised only
after the hwmon device is registered, so a sysfs write to pwm1_enable
can schedule an uninitialised work item. On removal, the work is
cancelled in .remove(), before devres unregisters the hwmon device, so a
sysfs write can requeue it after the cancel.

Use devm_delayed_work_autocancel() and set it up before registering the
hwmon device. devres then cancels the work on every teardown path, after
the hwmon sysfs interface has been removed and before priv and its mutex
are released. Drop the now-unneeded cancel in hp_wmi_bios_remove() and
the platform drvdata that was only used for it.

Fixes: c203c59fb5de ("platform/x86: hp-wmi: implement fan keep-alive")
Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Shang En Sim <sim@shangen.org>
---
 drivers/platform/x86/hp/hp-wmi.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index 7ab81ce5f8d4..2bf0bb04dcec 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -18,6 +18,7 @@
 #include <linux/bits.h>
 #include <linux/cleanup.h>
 #include <linux/compiler_attributes.h>
+#include <linux/devm-helpers.h>
 #include <linux/dmi.h>
 #include <linux/fixp-arith.h>
 #include <linux/hwmon.h>
@@ -2571,7 +2572,6 @@ static int __init hp_wmi_bios_setup(struct platform_device *device)
 static void __exit hp_wmi_bios_remove(struct platform_device *device)
 {
 	int i;
-	struct hp_wmi_hwmon_priv *priv;
 
 	for (i = 0; i < rfkill2_count; i++) {
 		rfkill_unregister(rfkill2[i].rfkill);
@@ -2590,10 +2590,6 @@ static void __exit hp_wmi_bios_remove(struct platform_device *device)
 		rfkill_unregister(wwan_rfkill);
 		rfkill_destroy(wwan_rfkill);
 	}
-
-	priv = platform_get_drvdata(device);
-	if (priv)
-		cancel_delayed_work_sync(&priv->keep_alive_dwork);
 }
 
 static int hp_wmi_resume_handler(struct device *device)
@@ -2941,6 +2937,17 @@ static int hp_wmi_hwmon_init(void)
 	ret = hp_wmi_setup_fan_settings(priv);
 	if (ret)
 		return ret;
+
+	/*
+	 * Set up the work before registering hwmon so that, on teardown,
+	 * it is cancelled only after the sysfs writers that schedule it
+	 * are gone.
+	 */
+	ret = devm_delayed_work_autocancel(dev, &priv->keep_alive_dwork,
+					   hp_wmi_hwmon_keep_alive_handler);
+	if (ret)
+		return ret;
+
 	hwmon = devm_hwmon_device_register_with_info(dev, "hp", priv,
 						     &chip_info, NULL);
 
@@ -2949,8 +2956,6 @@ static int hp_wmi_hwmon_init(void)
 		return PTR_ERR(hwmon);
 	}
 
-	INIT_DELAYED_WORK(&priv->keep_alive_dwork, hp_wmi_hwmon_keep_alive_handler);
-	platform_set_drvdata(hp_wmi_platform_dev, priv);
 	ret = hp_wmi_apply_fan_settings(priv);
 	if (ret)
 		dev_warn(dev, "Failed to apply initial fan settings: %d\n", ret);
-- 
2.56.0

[PATCH v2 2/2] platform/x86: hp-wmi: add charge_behaviour support for Spectre 8C15/8C17

[Shang En Sim]

HP Spectre x360 16-aa0xxx (board 8C17) exposes battery charge mode control
through WMI command 0x2b (GBCO/SBCO). Inhibit charge, force discharge, and
restoring automatic charging all work through that command. The
Spectre x360 14-eu0xxx (board 8C15) shares the same platform and the same
GBCO/SBCO implementation.

Unlike some other HP designs that restore automatic charging through WMI
command 0x1f (SBCC), that path fails on this BIOS. Keep the quirk limited
to boards 8C15 and 8C17 and drive all three behaviours through 0x2b only.

Expose auto / inhibit-charge / force-discharge through the standard
power_supply charge_behaviour property. Seed the reported policy from
GBCO at setup because firmware retains inhibit across reboot. Cache that
policy afterwards: GBCO also encodes live status, so 0x01 means actively
charging and 0x02 is ambiguous between force-discharge and running on
battery alone.

Observed firmware mapping on this board:
- write 0x00 -> auto; write 0x02 -> force-discharge; write 0x05 -> inhibit
- read 0x00/0x01 -> auto (0x01 = actively charging)
- read 0x02 -> force-discharge (also seen on battery alone)
- read 0x04/0x05/0x06 -> inhibit

Tested on HP Spectre x360 16-aa0xxx (board 8C17, BIOS F.16):
- inhibit stops charging on AC; GBCO reads 0x04
- force-discharge discharges on AC; GBCO reads 0x02
- restore-auto via 0x2b mode 0 resumes charging; GBCO reads 0x01
- inhibit persists across reboot (status Not charging, GBCO 0x04)

Board 8C15 was reported working by Alain Cousinie.

Suggested-by: Alain Cousinie <alain.cousinie@laposte.net>
Signed-off-by: Shang En Sim <sim@shangen.org>
---
Changes in v2:
- Read GBCO status with FIELD_GET(HPWMI_CHARGE_MODE_READ_MASK, val)
- Add board 8C15

v1: https://lore.kernel.org/r/20260812062113.27741-1-sim@shangen.org/

 drivers/platform/x86/hp/hp-wmi.c | 246 +++++++++++++++++++++++++++++++
 1 file changed, 246 insertions(+)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index 2bf0bb04dcec..ff503c058995 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -15,6 +15,7 @@
 
 #include <linux/acpi.h>
 #include <linux/array_size.h>
+#include <linux/bitfield.h>
 #include <linux/bits.h>
 #include <linux/cleanup.h>
 #include <linux/compiler_attributes.h>
@@ -40,6 +41,8 @@
 #include <linux/types.h>
 #include <linux/workqueue.h>
 
+#include <acpi/battery.h>
+
 MODULE_AUTHOR("Matthew Garrett <mjg59@srcf.ucam.org>");
 MODULE_DESCRIPTION("HP laptop WMI driver");
 MODULE_LICENSE("GPL");
@@ -428,6 +431,7 @@ enum hp_wmi_commandtype {
 	HPWMI_FEATURE2_QUERY		= 0x0d,
 	HPWMI_WIRELESS2_QUERY		= 0x1b,
 	HPWMI_POSTCODEERROR_QUERY	= 0x2a,
+	HPWMI_BATTERY_CHARGE_MODE	= 0x2b,
 	HPWMI_SYSTEM_DEVICE_MODE	= 0x40,
 	HPWMI_THERMAL_PROFILE_QUERY	= 0x4c,
 	HPWMI_GRAPHICS_MUX_QUERY	= 0x52,
@@ -497,6 +501,30 @@ enum hp_wireless2_bits {
 #define IS_HWBLOCKED(x) ((x & HPWMI_POWER_FW_OR_HW) != HPWMI_POWER_FW_OR_HW)
 #define IS_SWBLOCKED(x) !(x & HPWMI_POWER_SOFT)
 
+#if IS_REACHABLE(CONFIG_ACPI_BATTERY)
+#define HPWMI_CHARGE_BEHAVIOURS \
+	(BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO) | \
+	 BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE) | \
+	 BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE))
+
+/*
+ * SBCO reads the requested mode from bits 15:8 of the input value.
+ * GBCO returns literal status values in bits 7:0.
+ */
+#define HPWMI_CHARGE_MODE_MASK			GENMASK(15, 8)
+#define HPWMI_CHARGE_MODE_AUTO			0x00
+#define HPWMI_CHARGE_MODE_FORCE_DISCHARGE	0x02
+#define HPWMI_CHARGE_MODE_INHIBIT		0x05
+
+#define HPWMI_CHARGE_MODE_READ_MASK		GENMASK(7, 0)
+#define HPWMI_CHARGE_MODE_READ_AUTO		0x00
+#define HPWMI_CHARGE_MODE_READ_CHARGING		0x01
+#define HPWMI_CHARGE_MODE_READ_FORCE_DISCHARGE	0x02
+#define HPWMI_CHARGE_MODE_READ_INHIBIT		0x04
+#define HPWMI_CHARGE_MODE_READ_INHIBIT_SHPM	0x05
+#define HPWMI_CHARGE_MODE_READ_INHIBIT_EXT	0x06
+#endif
+
 struct bios_rfkill2_device_state {
 	u8 radio_type;
 	u8 bus_type;
@@ -584,6 +612,26 @@ static const char * const tablet_chassis_types[] = {
 	"32"  /* Detachable */
 };
 
+#if IS_REACHABLE(CONFIG_ACPI_BATTERY)
+static const struct dmi_system_id hp_wmi_charge_control_quirks[] __initconst = {
+	{
+		/* HP Spectre x360 16-aa0xxx */
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "HP"),
+			DMI_MATCH(DMI_BOARD_NAME, "8C17"),
+		},
+	},
+	{
+		/* HP Spectre x360 14-eu0xxx */
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "HP"),
+			DMI_MATCH(DMI_BOARD_NAME, "8C15"),
+		},
+	},
+	{}
+};
+#endif
+
 #define DEVICE_MODE_TABLET	0x06
 
 #define CPU_FAN 0
@@ -813,6 +861,200 @@ static int hp_wmi_read_int(int query)
 	return val;
 }
 
+#if IS_REACHABLE(CONFIG_ACPI_BATTERY)
+/* Protects hp_wmi_charge_behaviour */
+static DEFINE_MUTEX(hp_wmi_charge_lock);
+static enum power_supply_charge_behaviour hp_wmi_charge_behaviour =
+	POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO;
+
+static int hp_wmi_charge_mode_to_behaviour(int mode,
+					   enum power_supply_charge_behaviour *behaviour)
+{
+	switch (mode) {
+	case HPWMI_CHARGE_MODE_READ_AUTO:
+	case HPWMI_CHARGE_MODE_READ_CHARGING:
+		/*
+		 * GBCO 0x01 reports active charging, rather than a separate
+		 * charge policy. Treat it as automatic charging.
+		 */
+		*behaviour = POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO;
+		return 0;
+	case HPWMI_CHARGE_MODE_READ_FORCE_DISCHARGE:
+		/*
+		 * Firmware also returns 0x02 while running on battery alone.
+		 * Treat it as force-discharge so a persisted AC policy is not
+		 * lost across reboot.
+		 */
+		*behaviour = POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE;
+		return 0;
+	case HPWMI_CHARGE_MODE_READ_INHIBIT:
+	case HPWMI_CHARGE_MODE_READ_INHIBIT_SHPM:
+	case HPWMI_CHARGE_MODE_READ_INHIBIT_EXT:
+		*behaviour = POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int hp_wmi_charge_mode_read(void)
+{
+	int val = 0, ret;
+
+	ret = hp_wmi_perform_query(HPWMI_BATTERY_CHARGE_MODE, HPWMI_READ,
+				   &val, zero_if_sup(val), sizeof(val));
+	if (ret < 0)
+		return ret;
+	if (ret > 0)
+		return -EINVAL;
+
+	return FIELD_GET(HPWMI_CHARGE_MODE_READ_MASK, val);
+}
+
+static int hp_wmi_charge_mode_write(u32 mode)
+{
+	int ret;
+
+	mode = FIELD_PREP(HPWMI_CHARGE_MODE_MASK, mode);
+	ret = hp_wmi_perform_query(HPWMI_BATTERY_CHARGE_MODE, HPWMI_WRITE,
+				   &mode, sizeof(mode), 0);
+	if (ret < 0)
+		return ret;
+	if (ret > 0)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int hp_wmi_charge_set_behaviour(enum power_supply_charge_behaviour behaviour)
+{
+	int ret;
+
+	guard(mutex)(&hp_wmi_charge_lock);
+
+	switch (behaviour) {
+	case POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO:
+		ret = hp_wmi_charge_mode_write(HPWMI_CHARGE_MODE_AUTO);
+		break;
+	case POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE:
+		ret = hp_wmi_charge_mode_write(HPWMI_CHARGE_MODE_INHIBIT);
+		break;
+	case POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE:
+		ret = hp_wmi_charge_mode_write(HPWMI_CHARGE_MODE_FORCE_DISCHARGE);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (ret)
+		return ret;
+
+	hp_wmi_charge_behaviour = behaviour;
+
+	return 0;
+}
+
+static int hp_wmi_charge_get_property(struct power_supply *psy,
+				      const struct power_supply_ext *ext,
+				      void *data,
+				      enum power_supply_property psp,
+				      union power_supply_propval *val)
+{
+	if (psp != POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR)
+		return -EINVAL;
+
+	guard(mutex)(&hp_wmi_charge_lock);
+	val->intval = hp_wmi_charge_behaviour;
+
+	return 0;
+}
+
+static int hp_wmi_charge_set_property(struct power_supply *psy,
+				      const struct power_supply_ext *ext,
+				      void *data,
+				      enum power_supply_property psp,
+				      const union power_supply_propval *val)
+{
+	if (psp != POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR)
+		return -EINVAL;
+
+	return hp_wmi_charge_set_behaviour(val->intval);
+}
+
+static int hp_wmi_charge_property_is_writeable(struct power_supply *psy,
+					       const struct power_supply_ext *ext,
+					       void *data,
+					       enum power_supply_property psp)
+{
+	return psp == POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR;
+}
+
+static const enum power_supply_property hp_wmi_charge_properties[] = {
+	POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR,
+};
+
+static const struct power_supply_ext hp_wmi_charge_extension = {
+	.name			= "hp-wmi-charge-control",
+	.properties		= hp_wmi_charge_properties,
+	.num_properties		= ARRAY_SIZE(hp_wmi_charge_properties),
+	.charge_behaviours	= HPWMI_CHARGE_BEHAVIOURS,
+	.get_property		= hp_wmi_charge_get_property,
+	.set_property		= hp_wmi_charge_set_property,
+	.property_is_writeable	= hp_wmi_charge_property_is_writeable,
+};
+
+static int hp_wmi_charge_add_battery(struct power_supply *battery,
+				     struct acpi_battery_hook *hook)
+{
+	return power_supply_register_extension(battery, &hp_wmi_charge_extension,
+					       &hp_wmi_platform_dev->dev, NULL);
+}
+
+static int hp_wmi_charge_remove_battery(struct power_supply *battery,
+					struct acpi_battery_hook *hook)
+{
+	power_supply_unregister_extension(battery, &hp_wmi_charge_extension);
+	return 0;
+}
+
+static struct acpi_battery_hook hp_wmi_charge_battery_hook = {
+	.name			= "HP WMI charge control",
+	.add_battery		= hp_wmi_charge_add_battery,
+	.remove_battery		= hp_wmi_charge_remove_battery,
+};
+
+static int __init hp_wmi_charge_control_setup(struct device *dev)
+{
+	enum power_supply_charge_behaviour behaviour;
+	int ret;
+
+	if (!dmi_check_system(hp_wmi_charge_control_quirks))
+		return 0;
+
+	/*
+	 * Firmware retains inhibit across reboot, so seed from GBCO rather
+	 * than assuming auto.
+	 */
+	ret = hp_wmi_charge_mode_read();
+	if (ret < 0)
+		return 0;
+
+	ret = hp_wmi_charge_mode_to_behaviour(ret, &behaviour);
+	if (ret)
+		return 0;
+
+	scoped_guard(mutex, &hp_wmi_charge_lock)
+		hp_wmi_charge_behaviour = behaviour;
+
+	return devm_battery_hook_register(dev, &hp_wmi_charge_battery_hook);
+}
+#else
+static int __init hp_wmi_charge_control_setup(struct device *dev)
+{
+	return 0;
+}
+#endif
+
 static int hp_wmi_get_dock_state(void)
 {
 	int state = hp_wmi_read_int(HPWMI_HARDWARE_QUERY);
@@ -2564,6 +2806,10 @@ static int __init hp_wmi_bios_setup(struct platform_device *device)
 	if (err < 0)
 		return err;
 
+	err = hp_wmi_charge_control_setup(&device->dev);
+	if (err)
+		return err;
+
 	thermal_profile_setup(device);
 
 	return 0;
-- 
2.56.0