Date: Thu, 19 Sep 2019 17:02:01 +0000 From: "Moore, Robert" <robert.moore@intel.com> To: Dan Kotowski <dan.kotowski@a9development.com>, "freebsd-acpi@freebsd.org" <freebsd-acpi@freebsd.org> Subject: RE: Patch AML or patch acpi driver? Message-ID: <94F2FBAB4432B54E8AACC7DFDE6C92E3B9685421@ORSMSX110.amr.corp.intel.com> In-Reply-To: <dwW0Sn1sZD3iXxaTqYs3XoSiyGl5c_KO0-z9jNg5WkZGoGqWPj6I4kzlXmN8OWZrW3Y6Y5HokDxGYpKAhHA2rGidDJXxWzqW47O8SvAIIXg=@a9development.com> References: <dwW0Sn1sZD3iXxaTqYs3XoSiyGl5c_KO0-z9jNg5WkZGoGqWPj6I4kzlXmN8OWZrW3Y6Y5HokDxGYpKAhHA2rGidDJXxWzqW47O8SvAIIXg=@a9development.com>
next in thread | previous in thread | raw e-mail | index | archive | help
-----Original Message----- From: owner-freebsd-acpi@freebsd.org [mailto:owner-freebsd-acpi@freebsd.org= ] On Behalf Of Dan Kotowski Sent: Monday, September 16, 2019 4:44 AM To: freebsd-acpi@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 re= turn the present voltage across the terminals. Is it better to patch the AM= L or the acpi driver to account for this missing DWORD? As per the ACPI 2 spec (here: https://uefi.org/sites/default/files/resource= s/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 P= KG0 as returned by the _BST method (line 10557 here: https://gist.github.co= m/agrajag9/f91a09cc181c9df404a93d119316aa18) only has the first 3 values an= d 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 s= cenarios where this causes issues for code attempting to compare the design= and present voltages checking for failing batteries, but right now the oth= er 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; Othe= rwise, 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 B= ST data, but it would complicate the driver logic some. In sys/dev/acpica/a= cpi_battery.c the acpi_battery_bst_valid function checks for state, capacit= y, and voltage, but not rate. Even if the design philosophy is "all or noth= ing", then this is also bad logic. I think it would be better to return tru= e if ANY _BST DWORD is valid (not 0xFFFFFFFF) rather than failing if some a= re invalid. This would at least expose the valid fields to the rest of the = system. Dan Kotowski _______________________________________________ freebsd-acpi@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-acpi To unsubscribe, send any mail to "freebsd-acpi-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?94F2FBAB4432B54E8AACC7DFDE6C92E3B9685421>