2022-05-02 15:38:36

by Maximilian Luz

[permalink] [raw]
Subject: [PATCH] ACPI: battery: Add "Not Charging" quirk for Microsoft Surface devices

Microsoft Surface devices have a limiter that sets a fixed maximum
charge capacity for the battery. When that maximum capacity has been
reached, charging stops. In that case, _BST returns a battery state
field with both "charging" and "discharging" bits cleared. The battery
driver, however, returns "unknown" as status.

This seems to be the same behavior as observed on the ThinkPads, so
let's use the same quirk to handle that.

Signed-off-by: Maximilian Luz <[email protected]>
---

For what it's worth, I don't think the ACPI spec explicitly states that
any of the status bits need to be set, or that there are only the
"charging" and "discharging" states. As far as I can tell, ACPI only
states:

Notice that the Charging bit and the Discharging bit are mutually
exclusive and must not both be set at the same time. Even in
critical state, hardware should report the corresponding
charging/discharging state.

But that does not exclude the case that no bit is set. So, strictly
going by spec, I don't think it's necessary to put all of this behind a
quirk.

---
drivers/acpi/battery.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index dc208f5f5a1f..1c88504aae5b 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -1152,6 +1152,19 @@ static const struct dmi_system_id bat_dmi_table[] __initconst = {
DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad"),
},
},
+ {
+ /*
+ * Microsoft Surface devices have an optional "battery
+ * limiter". Due to this, there is a "Not Charging" state
+ * similar to the one on the Lenovo ThinkPads, described above.
+ */
+ .callback = battery_quirk_not_charging,
+ .ident = "Microsoft Surface",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Surface"),
+ },
+ },
{
/* Microsoft Surface Go 3 */
.callback = battery_notification_delay_quirk,
--
2.36.0


2022-05-09 07:13:17

by Maximilian Luz

[permalink] [raw]
Subject: Re: [PATCH] ACPI: battery: Add "Not Charging" quirk for Microsoft Surface devices

On 5/6/22 20:43, Rafael J. Wysocki wrote:
> On Fri, Apr 29, 2022 at 7:41 PM Maximilian Luz <[email protected]> wrote:
>>
>> Microsoft Surface devices have a limiter that sets a fixed maximum
>> charge capacity for the battery. When that maximum capacity has been
>> reached, charging stops. In that case, _BST returns a battery state
>> field with both "charging" and "discharging" bits cleared. The battery
>> driver, however, returns "unknown" as status.
>>
>> This seems to be the same behavior as observed on the ThinkPads, so
>> let's use the same quirk to handle that.
>>
>> Signed-off-by: Maximilian Luz <[email protected]>
>> ---
>>
>> For what it's worth, I don't think the ACPI spec explicitly states that
>> any of the status bits need to be set, or that there are only the
>> "charging" and "discharging" states. As far as I can tell, ACPI only
>> states:
>>
>> Notice that the Charging bit and the Discharging bit are mutually
>> exclusive and must not both be set at the same time. Even in
>> critical state, hardware should report the corresponding
>> charging/discharging state.
>>
>> But that does not exclude the case that no bit is set. So, strictly
>> going by spec, I don't think it's necessary to put all of this behind a
>> quirk.
>
> I think that this should be covered by the patch I've just applied:
>
> https://patchwork.kernel.org/project/linux-acpi/patch/[email protected]/
>
> Shouldn't it?

It does, thank you!

Sorry for having missed that one.

Regards,
Max

2022-05-09 08:20:47

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] ACPI: battery: Add "Not Charging" quirk for Microsoft Surface devices

On Fri, Apr 29, 2022 at 7:41 PM Maximilian Luz <[email protected]> wrote:
>
> Microsoft Surface devices have a limiter that sets a fixed maximum
> charge capacity for the battery. When that maximum capacity has been
> reached, charging stops. In that case, _BST returns a battery state
> field with both "charging" and "discharging" bits cleared. The battery
> driver, however, returns "unknown" as status.
>
> This seems to be the same behavior as observed on the ThinkPads, so
> let's use the same quirk to handle that.
>
> Signed-off-by: Maximilian Luz <[email protected]>
> ---
>
> For what it's worth, I don't think the ACPI spec explicitly states that
> any of the status bits need to be set, or that there are only the
> "charging" and "discharging" states. As far as I can tell, ACPI only
> states:
>
> Notice that the Charging bit and the Discharging bit are mutually
> exclusive and must not both be set at the same time. Even in
> critical state, hardware should report the corresponding
> charging/discharging state.
>
> But that does not exclude the case that no bit is set. So, strictly
> going by spec, I don't think it's necessary to put all of this behind a
> quirk.

I think that this should be covered by the patch I've just applied:

https://patchwork.kernel.org/project/linux-acpi/patch/[email protected]/

Shouldn't it?

> ---
> drivers/acpi/battery.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index dc208f5f5a1f..1c88504aae5b 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -1152,6 +1152,19 @@ static const struct dmi_system_id bat_dmi_table[] __initconst = {
> DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad"),
> },
> },
> + {
> + /*
> + * Microsoft Surface devices have an optional "battery
> + * limiter". Due to this, there is a "Not Charging" state
> + * similar to the one on the Lenovo ThinkPads, described above.
> + */
> + .callback = battery_quirk_not_charging,
> + .ident = "Microsoft Surface",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "Surface"),
> + },
> + },
> {
> /* Microsoft Surface Go 3 */
> .callback = battery_notification_delay_quirk,
> --
> 2.36.0
>