Patch AML or patch acpi driver?

[Moore, Robert]

-----Original Message-----
From: owner-freebsd-acpi at freebsd.org [mailto:owner-freebsd-acpi at freebsd.org] On Behalf Of Dan Kotowski
Sent: Monday, September 16, 2019 4:44 AM
To: freebsd-acpi at freebsd.org
Subject: Patch AML or patch acpi driver?

The _BST method in my laptop's AML only returns the present state, present capacity, and present charge/discharge rate of the battery, but does not return the present voltage across the terminals. Is it better to patch the AML or the acpi driver to account for this missing DWORD?

As per the ACPI 2 spec (here: https://uefi.org/sites/default/files/resources/ACPI_2.pdf), the BST method should return the following struct:

Package {
    Battery State //DWORD
    Battery Present Rate //DWORD
    Battery Remaining Capacity //DWORD
    Battery Present Voltage //DWORD
}

However, when I look at the AML dumped from my controller, I can see that PKG0 as returned by the _BST method (line 10557 here: https://gist.github.com/agrajag9/f91a09cc181c9df404a93d119316aa18) only has the first 3 values and will never return the present voltage.

So the options are:

1. Patch the AML: This should be simple enough by copying either the DWORD or logic for getting the Design Voltage in the _BIF method. I can imagine scenarios where this causes issues for code attempting to compare the design and present voltages checking for failing batteries, but right now the other 3 values just get ignored completely which is also less-than-useful.

If possible, I would do this, even if the actual value is unavailable; Otherwise, the ACPICA code will always flag this as an error, since the length of the package is incorrect.


2. Patch the driver: This would allow more platforms to handle incomplete BST data, but it would complicate the driver logic some. In sys/dev/acpica/acpi_battery.c the acpi_battery_bst_valid function checks for state, capacity, and voltage, but not rate. Even if the design philosophy is "all or nothing", then this is also bad logic. I think it would be better to return true if ANY _BST DWORD is valid (not 0xFFFFFFFF) rather than failing if some are invalid. This would at least expose the valid fields to the rest of the system.

Dan Kotowski
_______________________________________________
freebsd-acpi at freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-acpi
To unsubscribe, send any mail to "freebsd-acpi-unsubscribe at freebsd.org"