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

Am 06.08.26 um 17:18 schrieb Jelle van der Waa:

> 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.

No problem, just include this tag with the v6.

Thanks,
Armin Wolf

>> [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
>