Re: [PATCH v2] hp-wmi: add battery threshold support

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Mon, 8 Jun 2026, yahia wrote:

> Hello, Ilpo
> Thanks for your review,
> This V2 patch implements Battery threshold support via ACPI methods
> SBCO and GBCO,This Implementation adds battery_get function to retrieve
> the current battery threshold mode and use it to identify capabilities
> to simplify code instead of another function specifically for detection

As already stated, please write proper English sentences.

Anything you want to say that isn't part of the changelog should appear 
under --- line.

> Signed-off-by: yahia ahmed <yahia.a.abdrabou@gmail.com>
> ---

Missing version history.

>  drivers/platform/x86/hp/hp-wmi.c | 61 ++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index f63bc00d9a9b..f2180b61b84e 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -11,6 +11,7 @@
>   * Copyright (C) 2005 Dmitry Torokhov <dtor@mail.ru>
>   */
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
>  #include <linux/acpi.h>

Eh?

> @@ -292,6 +293,18 @@ enum hp_wmi_event_ids {
>  	HPWMI_SMART_EXPERIENCE_APP	= 0x21,
>  };
>  
> +/*
> + * Profiles for hp battery threshold profile 0x05 and 0x06 map to 4 and 5 respectively
> + */
> +enum hp_battery_modes {
> +	HP_BATTERY_MODE_NORMAL = 0x00,
> +	HP_BATTERY_MODE_ADAPTIVE = 0x02,
> +	HP_BATTERY_THRESHOLD_4 = 0x05,
> +	HP_BATTERY_THRESHOLD_5 = 0x06,

The values should be aligned.

> +};
> +
> +#define HPWMI_BATTERY_THRESHOLD_SBCO 0x2B // SBCO Command id
> +#define HPWMI_AC_REQUIRED 0x35 // obscure edge case where the bios requires the user to be charged while setting a profile

???

>  /*
>   * struct bios_args buffer is dynamically allocated.  New WMI command types
>   * were introduced that exceeds 128-byte data size.  Changes to handle
> @@ -448,6 +461,7 @@ static struct notifier_block platform_power_source_nb;
>  static enum platform_profile_option active_platform_profile;
>  static bool platform_profile_support;
>  static bool zero_insize_support;
> +static bool battery_threshold_support;
>  
>  static struct rfkill *wifi_rfkill;
>  static struct rfkill *bluetooth_rfkill;
> @@ -1085,6 +1099,50 @@ static ssize_t postcode_store(struct device *dev, struct device_attribute *attr,
>  	return count;
>  }
>  
> +static int battery_get(void) {
> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	int mode = -1;
> +	if (ACPI_SUCCESS(acpi_evaluate_object(NULL, "\\_SB.WMID.GBCO", NULL, &output))) {
> +		union acpi_object *obj = (union acpi_object *)output.pointer;
> +		if (obj && obj->type == ACPI_TYPE_PACKAGE && obj->package.count >= 3) { // standard package sent by bios
> +			mode = (int)obj->package.elements[2].buffer.pointer[0]; // bios returns the result in the third buffer 
> +		}
> +		kfree(output.pointer);
> +	} else 
> +		mode = -EIO;
> +	return mode;
> +}
> +
> +static int battery_set(int profile) 
> +{
> +	if (!battery_threshold_support) {
> +		pr_err("Battery threshold is not supported");
> +		return -EOPNOTSUPP;
> +	}
> +	union acpi_object args[4];
> +	acpi_status status;
> +	struct acpi_object_list arg_list = { .count = 4, .pointer = args };
> +	memset(args, 0, sizeof(args));
> +	args[0].type = ACPI_TYPE_INTEGER;
> +	args[0].integer.value = 0;
> +	args[1].type = ACPI_TYPE_INTEGER;
> +	args[1].integer.value = profile;
> +	args[2].type = ACPI_TYPE_INTEGER; // padding required by bios
> +	args[2].integer.value = 0;
> +	args[3].type = ACPI_TYPE_INTEGER;
> +	args[3].integer.value = 0;
> +	status = acpi_evaluate_object(NULL, "\\_SB.WMID.SBCO", &arg_list, NULL);
> +	if (unlikely(status == HPWMI_AC_REQUIRED)) {
> +		pr_err("You have to be plugged in while setting a battery profile"); // random edge case in the bios
> +		return -EIO;
> +	}
> +	if (ACPI_FAILURE(status)) {
> +		pr_info("Write failed with Error code 0x%08x\n", status);
> +		return -EIO;
> +	}
> +	return status;
> +}
> +
>  static int camera_shutter_input_setup(void)
>  {
>  	int err;
> @@ -2292,6 +2350,9 @@ static int __init hp_wmi_bios_setup(struct platform_device *device)
>  	if (err < 0)
>  		return err;
>  
> +	int ret = battery_get();
> +	if (ret >= 0)
> +		battery_threshold_support = true;
>  	thermal_profile_setup(device);
>  
>  	return 0;
> 

Unfortunately, this submission doesn't match kernel coding style 
guidelines you can find under Documentation/.

Please review the patch yourself before submitting the next version and correct 
all error you find before the submission.

-- 
 i.