Re: [PATCH v4 1/1] platform/x86: add Acer battery control driver

Hi,

On 07/06/2026 22:46, Armin Wolf wrote:
> Am 31.05.26 um 11:05 schrieb Jelle van der Waa:
> 
>> Some Acer laptops can configure battery related features through Acer
>> Care Center on Windows. This driver uses the power supply extension to
>> set a battery charge limit and exposes the battery
>> temperature.
>>
>> This driver is based on the existing acer-wmi-battery project on GitHub
>> and was tested on an Acer Aspire A315-510P.
> 
> Good work, i tested it on my old Aspire notebook (v1 interface) and i 
> can confirm
> that the temperature reading and DMI whitelist works as intended.
> 
> I found a couple of minor issues, once those are addressed:
> 
> Reviewed-by: Armin Wolf <[email protected]>

Sorry for the long hiatus, I send a v5 yesterday and forgot to include 
this. If it is needed to send a v6 I will include it and send that as 
soon as possible.
> [WMI, Dynamic, Provider("WmiProv"), Locale("MS\\0x409"), 
> Description("Class used to control smart battery, Version 2.88"), 
> guid("{79772EC5-04B1-4bfd-843C-61E7F77B6CC9}")]
> class BatteryControl {
>    [key, read] string InstanceName;
>    [read] boolean Active;
> 
>    [WmiMethodId(19), Implemented, read, write, Description("Get battery 
> Information Interface.")] void GetBattInfoInterface([in] uint32 
> uBatteryInfoIndex, [in] uint32 uBatteryNo, [out] uint32 uReturn);
>    [WmiMethodId(20), Implemented, read, write, Description("Get Battery 
> Health Control Status.")] void GetBatteryHealthControlStatus([in] uint8 
> uBatteryNo, [in] uint8 uFunctionQuery, [in] uint8 uReserved[2], [out] 
> uint8 uFunctionList, [out] uint8 uReturn[2], [out] uint8 
> uFunctionStatus[5]);
>    [WmiMethodId(21), Implemented, read, write, Description("Set Battery 
> Health Control.")] void SetBatteryHealthControl([in] uint8 uBatteryNo, 
> [in] uint8 uFunctionMask, [in] uint8 uFunctionStatus, [in] uint8 
> uReservedIn[5], [out] uint16 uReturn, [out] uint16 uReservedOut);
>    [WmiMethodId(22), Implemented, read, write, Description("Get Battery 
> Function Data.")] void GetBatteryFunctionData([in] uint8 uFunctionMask, 
> [in] uint8 uReservedIn[3], [out] uint8 uReturnCode[2], [out] uint8 
> uBACStartTime[2], [out] uint8 uBACStopTime[2], [out] uint8 uBACStatus, 
> [out] uint8 uReservedOut[9]);
>    [WmiMethodId(23), Implemented, read, write, Description("Set Battery 
> Function Data.")] void SetBatteryFunctionData([in] uint8 uFunctionMask, 
> [in] uint8 uBACSwitch, [in] uint8 uReservedIn[6], [out] uint16 
> uReturnCode, [out] uint8 uReservedOut[2]);
> };
> 
> I suggest you write a tiny bit of documentation under Documentation/wmi/ 
> devices/ using this MOF description.
> This way future developers can better understand the WMI interface used 
> by this driver.

Thanks for poking me to write this, this was the first time writing a 
WMI driver and writing a bit of documentation definitively made 
everything click in my head.

It also sparked my curiosity. I know you added the temperature reading 
and referenced "Smart Battery Data Specification", did you try any of 
the other fields (such as manufacturer?)  [1]

[1] https://sbs-forum.org/specs/sbdat110.pdf

Thanks for the review,

Jelle van der Waa