Created attachment 213626 [details] dmesg on r360134/HP Spectre x360 13-ap0053dx On a HP Spectre x360 13-ap0053dx running 13-CURRENT r360134, I get this when running acpiconf -i 0: root@spectre:/home/neel # acpiconf -i 0 Design capacity: 0 mWh Last full capacity: 0 mWh Technology: primary (non-rechargeable) Design voltage: 0 mV Capacity (warn): 0 mWh Capacity (low): 0 mWh Cycle Count: 0 Mesurement Accuracy: 0 % Max Sampling Time: 0 ms Min Sampling Time: 0 ms Max Average Interval: 0 ms Min Average Interval: 0 ms Low/warn granularity: 0 mWh Warn/full granularity: 0 mWh Model number: Serial number: Type: OEM info: State: not present Present voltage: unknown root@spectre:/home/neel # Also, hw.acpi.acline appears if my system is plugged in when it isn't: root@spectre:/home/neel # sysctl hw.acpi.acline hw.acpi.acline: 1 root@spectre:/home/neel # I have attached a dmesg and will upload a DSDT log.
Created attachment 213627 [details] DSDT for HP Spectre x360 13-ap0053dx
Using my "old" kernel with build r359837 works.
I think r360131 (Bug 245757) broke this, but can't confirm. The HP x360 13-ap0053dx gives many ACPI errors on bootup, and r360131 suppresses ACPI probes on an "error" which prevents the OS (and GNOME) from getting the battery status.
Can you provide a dmesg from the working configuration? There are no acpi_ec errors in the original report. Additionally, please attach the output of 'acpidump -dt'; the raw DSDT table doesn't seem to be parseable. > r360131 suppresses ACPI probes on an "error" This is not an especially accurate summary of 360131.
Created attachment 213660 [details] Working dmesg on HP Spectre x360 13-ap0053dx Here's the dmesg of the working system. acpidump -dt will be posted shortly.
Created attachment 213661 [details] acpidump -dt of the HP Spectre x360 13-ap0053dx Here's the acpidump output.
If I manually revert r360131 myself in my own /usr/src, battery status and backlight control hotkeys works normally. Post-r360131, acpi_ec does not load on my Spectre.
Thanks Neel for the rapid report and information. I will try to take a look today.
Created attachment 213705 [details] battery_sysctls_debug_messages_lenovoA485 Hello, I believe I also identified the same issue running 360179, traced it back to the patch for PR 245757 as well. If I go back to r360081, battery still works fine. The hardware in question is a Lenovo A485 (AMD Ryzen CPU) running the latest BIOS (nothing changed besides a new OS revision). The attached file contains the battery-related sysctls, build information and the debugging messages that now appear on dmesg. Would happily help test patches and provide more information.
Created attachment 213706 [details] acpidump_dt_lenovoA485
(In reply to Neel Chauhan from comment #6) The acpidump seems to be missing disassembled DSDT, unfortunately. @Neel, @Evilham, please try the following change. --- a/sys/dev/acpica/acpi_ec.c +++ b/sys/dev/acpica/acpi_ec.c @@ -443,6 +443,8 @@ acpi_ec_probe(device_t dev) if (buf.Pointer) AcpiOsFree(buf.Pointer); + + ret = rc; out: if (ret <= 0) { snprintf(desc, sizeof(desc), "Embedded Controller: GPE %#x%s%s", In the case where there was no error when we got to 'out', r360131 inadvertently left 'ret' with the initial ENXIO value. The patch above restores the pre-r360131 return value if no errors occurred. If that change doesn't work, here's one with further debugging in all of the error cases: --- a/sys/dev/acpica/acpi_ec.c +++ b/sys/dev/acpica/acpi_ec.c @@ -397,6 +397,7 @@ acpi_ec_probe(device_t dev) */ peer = devclass_get_device(acpi_ec_devclass, params->uid); if (peer != NULL && device_is_alive(peer)) { + device_printf(dev, "XXX duplicate case\n"); device_disable(dev); goto out; } @@ -417,8 +418,10 @@ acpi_ec_probe(device_t dev) } obj = (ACPI_OBJECT *)buf.Pointer; - if (obj == NULL) + if (obj == NULL) { + device_printf(dev, "XXX _GPE NULL result\n"); goto out; + } switch (obj->Type) { case ACPI_TYPE_INTEGER: @@ -426,12 +429,16 @@ acpi_ec_probe(device_t dev) params->gpe_bit = obj->Integer.Value; break; case ACPI_TYPE_PACKAGE: - if (!ACPI_PKG_VALID(obj, 2)) + if (!ACPI_PKG_VALID(obj, 2)) { + device_printf(dev, "XXX _GPE invalid PKG\n"); goto out; + } params->gpe_handle = acpi_GetReference(NULL, &obj->Package.Elements[0]); if (params->gpe_handle == NULL || - acpi_PkgInt32(obj, 1, ¶ms->gpe_bit) != 0) + acpi_PkgInt32(obj, 1, ¶ms->gpe_bit) != 0) { + device_printf(dev, "XXX _GPE NULL handle or bad bit\n"); goto out; + } break; default: device_printf(dev, "_GPE has invalid type %d\n", obj->Type); @@ -443,6 +450,8 @@ acpi_ec_probe(device_t dev) if (buf.Pointer) AcpiOsFree(buf.Pointer); + + ret = rc; out: if (ret <= 0) { snprintf(desc, sizeof(desc), "Embedded Controller: GPE %#x%s%s",
The first patch (ret = rc;) fixed it for me, and the debug messages also disappeared for this instance of ACPI things. I take it they'll still be around for the cases in which they were intended. Thank you :-).
Thanks for the quick test! This was a total brain-o on my part in r360131; sorry.
A commit references this bug: Author: cem Date: Thu Apr 23 17:30:03 UTC 2020 New revision: 360224 URL: https://svnweb.freebsd.org/changeset/base/360224 Log: acpi_ec(4): Don't probe erroneously if success occurred In r360131, acpi_ec probe was changed to not clobber an error status prior to several error cases that did not explicitly set the error variable before goto'ing the exit path. However, I did not notice that the error variable was not set to success in the success path. That caused all successful probes to fail, which is obviously undesirable. PR: 245778 Reported by: Neel Chauhan <neel AT neelc.org>, Evilham <contact AT evilham.com> Tested by: Evilham X-MFC-With: r360131 Changes: head/sys/dev/acpica/acpi_ec.c