mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] platform/x86: hp-wmi: add charge_behaviour support for Spectre 8C17
@ 2026-08-12  6:20 Shang En Sim
  2026-09-08 12:19 ` Alain Cousinie
  2026-09-17 11:22 ` Ilpo Järvinen
  0 siblings, 2 replies; 3+ messages in thread
From: Shang En Sim @ 2026-08-12  6:20 UTC (permalink / raw)
  To: Hans de Goede, Ilpo Järvinen
  Cc: platform-driver-x86, linux-kernel, 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.

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 board 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)

Signed-off-by: Shang En Sim <sim@shangen.org>
---
 drivers/platform/x86/hp/hp-wmi.c | 238 +++++++++++++++++++++++++++++++
 1 file changed, 238 insertions(+)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index 3c737ef38..1f9d11427 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>
@@ -39,6 +40,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");
@@ -392,6 +395,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,
@@ -461,6 +465,29 @@ 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_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;
@@ -548,6 +575,19 @@ 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"),
+		},
+	},
+	{}
+};
+#endif
+
 #define DEVICE_MODE_TABLET	0x06
 
 #define CPU_FAN 0
@@ -777,6 +817,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 val & 0xff;
+}
+
+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);
@@ -2528,6 +2762,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.55.0


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

* Re: [PATCH] platform/x86: hp-wmi: add charge_behaviour support for Spectre 8C17
  2026-08-12  6:20 [PATCH] platform/x86: hp-wmi: add charge_behaviour support for Spectre 8C17 Shang En Sim
@ 2026-09-08 12:19 ` Alain Cousinie
  2026-09-17 11:22 ` Ilpo Järvinen
  1 sibling, 0 replies; 3+ messages in thread
From: Alain Cousinie @ 2026-09-08 12:19 UTC (permalink / raw)
  To: Shang En Sim
  Cc: Hans de Goede, Ilpo Järvinen, platform-driver-x86, linux-kernel

Le 12/08/2026 à 08:20, Shang En Sim a écrit :
> 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.
>
>   
> +#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"),
> +		},
> +	},
> +	{}
> +};
> +#endif
> +

Hello Shang,

I am using an HP Spectre 14-eu0xxx (board 8C15), which appears to be identical to the 8C17 model regarding `hp-wmi`.
(See also https://github.com/aigilea/hp_spectre_x360_14_eu0xxx, which also covers the 16-aa0xxx model.)
I found the same elements in the DSDT, and my tests seem to work after modifying `hp-wmi.c`.
Is it possible to add 8C15 to your patch?

+    {
+        /* HP Spectre x360 14-eu0xxx */
+        .matches = {
+            DMI_MATCH(DMI_BOARD_VENDOR, "HP"),
+            DMI_MATCH(DMI_BOARD_NAME, "8C15"),
+        },
+    },

Thanks in advance.
Alain


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

* Re: [PATCH] platform/x86: hp-wmi: add charge_behaviour support for Spectre 8C17
  2026-08-12  6:20 [PATCH] platform/x86: hp-wmi: add charge_behaviour support for Spectre 8C17 Shang En Sim
  2026-09-08 12:19 ` Alain Cousinie
@ 2026-09-17 11:22 ` Ilpo Järvinen
  1 sibling, 0 replies; 3+ messages in thread
From: Ilpo Järvinen @ 2026-09-17 11:22 UTC (permalink / raw)
  To: Shang En Sim; +Cc: Hans de Goede, platform-driver-x86, LKML

On Tue, 11 Aug 2026, Shang En Sim wrote:

> 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.
> 
> 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 board 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)
> 
> Signed-off-by: Shang En Sim <sim@shangen.org>
> ---
>  drivers/platform/x86/hp/hp-wmi.c | 238 +++++++++++++++++++++++++++++++
>  1 file changed, 238 insertions(+)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index 3c737ef38..1f9d11427 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>
> @@ -39,6 +40,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");
> @@ -392,6 +395,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,
> @@ -461,6 +465,29 @@ 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_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;
> @@ -548,6 +575,19 @@ 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"),
> +		},
> +	},
> +	{}
> +};
> +#endif
> +
>  #define DEVICE_MODE_TABLET	0x06
>  
>  #define CPU_FAN 0
> @@ -777,6 +817,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 val & 0xff;

Hi,

Thanks for the patch.

Can the mask literal be named with a define (and make define use 
GENMASK()) and FIELD_GET() be used here?

> +}
> +
> +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);
> @@ -2528,6 +2762,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;

Sashiko found out there's a pre-existing issue in hp_wmi_hwmon_init() not 
properly cancelling work on error, and now with this adding more error 
returns, the same problem occurs in them as well.

So I think it would be useful to preceed this change with another patch
that converts INIT_DELAYED_WORK() in hp_wmi_hwmon_init() into 
devm_delayed_work_autocancel() so we don't end up adding into that 
problem here but fix it instead.

-- 
 i.


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

end of thread, other threads:[~2026-09-17 11:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-12  6:20 [PATCH] platform/x86: hp-wmi: add charge_behaviour support for Spectre 8C17 Shang En Sim
2026-09-08 12:19 ` Alain Cousinie
2026-09-17 11:22 ` Ilpo Järvinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®