Bug 203563 - Battery Information not read on Toshiba Laptop
Summary: Battery Information not read on Toshiba Laptop
Status: CLOSED ANSWERED
Alias: None
Product: ACPI
Classification: Unclassified
Component: ACPICA-Core (show other bugs)
Hardware: Intel Linux
: P1 high
Assignee: Zhang Rui
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-05-10 00:10 UTC by Bjarki Geir Benediktsson
Modified: 2021-08-09 07:30 UTC (History)
4 users (show)

See Also:
Kernel Version: 4.20 and later
Subsystem:
Regression: Yes
Bisected commit-id:


Attachments
acpidump (510.08 KB, text/plain)
2019-05-11 01:14 UTC, Bjarki Geir Benediktsson
Details
dmesg when not working (59.11 KB, text/plain)
2019-05-11 01:18 UTC, Bjarki Geir Benediktsson
Details
dmesg when working (59.56 KB, text/plain)
2019-05-11 01:20 UTC, Bjarki Geir Benediktsson
Details
dmesg with debug info (59.61 KB, text/plain)
2019-05-14 00:06 UTC, Bjarki Geir Benediktsson
Details
dmesg with debug info when working (58.92 KB, text/plain)
2019-05-14 00:15 UTC, Bjarki Geir Benediktsson
Details
Proposed fix (1.16 KB, patch)
2020-12-01 20:11 UTC, Bjarki Geir Benediktsson
Details | Diff
Minimal patch (912 bytes, patch)
2020-12-05 23:27 UTC, Bjarki Geir Benediktsson
Details | Diff
dmidecode (18.21 KB, text/plain)
2021-06-01 06:35 UTC, Bjarki Geir Benediktsson
Details
patch: force evaluate EC _REG (2.89 KB, patch)
2021-06-02 03:11 UTC, Zhang Rui
Details | Diff
debug patch to dump dmi info (818 bytes, patch)
2021-07-13 15:14 UTC, Zhang Rui
Details | Diff
dmesg (130.45 KB, text/plain)
2021-07-14 13:18 UTC, Bjarki Geir Benediktsson
Details

Description Bjarki Geir Benediktsson 2019-05-10 00:10:00 UTC
First time reporting a bug (Hope I have identified the correct subsystem)

ACPI doesn't read the battery information in kernel versions 4.20 and later on Toshiba Satellite C870-13D 

By bisecting the problem I found that the first bad commit on the linux kernel repository is:
     8b1cafdcb4b75c5027c52f1e82b47ebe727ad7ed

The problem is not just in the front en as the information is unavailable or incorrect (default 0) in /sys/class/power_supply/
Comment 1 Erik Kaneda 2019-05-10 17:08:41 UTC
Please post an acpidump of your machine https://01.org/linux-acpi/utilities as well as the dmesg
Comment 2 Bjarki Geir Benediktsson 2019-05-11 01:14:24 UTC
Created attachment 282715 [details]
acpidump
Comment 3 Bjarki Geir Benediktsson 2019-05-11 01:18:11 UTC
Created attachment 282717 [details]
dmesg when not working

dmesg was run as the first command after reboot on the latest build of arch linux (5.0.13)
the bug exists in this version
Comment 4 Bjarki Geir Benediktsson 2019-05-11 01:20:48 UTC
Created attachment 282719 [details]
dmesg when working

dmesg was run as the first command after reboot using arch linux lts 4.19.41-1
the bug does not occur in this version, added for a comparison
Comment 5 Erik Kaneda 2019-05-13 20:12:31 UTC
(In reply to Bjarki Geir Benediktsson from comment #0)
> First time reporting a bug (Hope I have identified the correct subsystem)
> 
> ACPI doesn't read the battery information in kernel versions 4.20 and later
> on Toshiba Satellite C870-13D 
> 
> By bisecting the problem I found that the first bad commit on the linux
> kernel repository is:
>      8b1cafdcb4b75c5027c52f1e82b47ebe727ad7ed

For my own notes, here is the commit log for this commit ID:

    ACPICA: Never run _REG on system_memory and system_IO

    These address spaces are defined by the ACPI spec to be
    "always available", and thus _REG should never be run on them.
    Provides compatibility with other ACPI implementations.


> 
> The problem is not just in the front en as the information is unavailable or
> incorrect (default 0) in /sys/class/power_supply/
Comment 6 Erik Kaneda 2019-05-13 21:50:07 UTC
Thanks for the info. I need a little more...

Please build both old and new with the kernel with CONFIG_ACPI_DEBUG=y and boot with the following command line parameter:

"acpi.debug_layer=0xffffffff acpi.debug_level=0x400 log_buf_len=10M"
Comment 7 Erik Kaneda 2019-05-13 21:50:40 UTC
and I forgot to mention: please post both dmesg logs with the above command.
Comment 8 Bjarki Geir Benediktsson 2019-05-14 00:06:07 UTC
Created attachment 282749 [details]
dmesg with debug info

Hope I have managed to pass these parameters correctly
Comment 9 Bjarki Geir Benediktsson 2019-05-14 00:15:26 UTC
Created attachment 282751 [details]
dmesg with debug info when working

dmesg log whith acpi debug info

I forgot to mention for the previous upload, that was dmesg from the latest kernel build where the bug occurs, 
this file is from linux-lts where the bug does not occur
Comment 10 Zhang Rui 2019-09-03 07:42:59 UTC
I believe the problem has been fixed. Bug closed.

Bjarki, please feel free to re-open it if the problem still exists in the latest upstream kernel.
Comment 11 Zhang Rui 2019-09-03 07:43:05 UTC
I believe the problem has been fixed. Bug closed.

Bjarki, please feel free to re-open it if the problem still exists in the latest upstream kernel.
Comment 12 Bjarki Geir Benediktsson 2019-09-04 13:49:04 UTC
Just built and tested latest upstream kernel, problem exists
Comment 13 Zhang Rui 2020-06-29 08:09:06 UTC
can you please check if the problem still exists in the latest upstream kernel, say 5.7 or 5.8-rc?
Comment 14 Bjarki Geir Benediktsson 2020-06-30 12:47:46 UTC
Just built and tested v5.8-rc3
more specificially commit 9ebcfadb0610322ac537dd7aa5d9cbc2b2894c68
Problem exists in that version.


When I was configuring the Kernel I accepted all default options but there where a couple that cought my eye as possibly relevant and I thought I'd mention these where all turned off by default.

ACPI_HMAT
BATTERY_CW2015
SYSTEM76_ACPI



are any of them likely to affect the situation ?
Comment 15 Bjarki Geir Benediktsson 2020-12-01 20:11:51 UTC
Created attachment 293893 [details]
Proposed fix

I tried digging further into the commit that introduced the bug and wrote a patch that fixes the issue under kernel version 5.10 by reverting part of it.
Comment 16 Bjarki Geir Benediktsson 2020-12-05 23:27:30 UTC
Created attachment 293953 [details]
Minimal patch

Minimal code change that fixes the bug.
Comment 17 Zhang Rui 2021-05-07 02:51:08 UTC
TBH, I'm confused,

$ grep _REG *.dsl
dsdt.dsl:                    Method (_REG, 2, NotSerialized)  // _REG: Region Availability

                    Method (_REG, 2, NotSerialized)  // _REG: Region Availability
                    {
                        ECFL = One
                        Local1 = RRAM (0x04A0)
                        Local1 |= 0x08
                        WRAM (0x04A0, Local1)
                        TTPH ()
                        ECPH ()
                    }


1. There is only one _REG method.
2. The _REG method is located under the EC device node, \_SB.PCI0.LPCB.EC0._REG.
3. The _REG method does not check Arg0 (address space ID)
4. There is no EmbeddedControl OperationRegion defined under EC device node.

This explains why you patch makes a difference.
First of all, _REG needs to be evaluated.
And usually, EC._REG will be evaluated when installing the EC address space handler.
But on this specific platform, there is no EC operation region, which means this _REG does not have a chance to get evaluated.

Currently, I'm not sure how to fix this. Let me sync with the ACPICA experts and get back later.
Comment 18 Rafael J. Wysocki 2021-05-10 15:51:30 UTC
It gets evaluated with the patch, though, so I'm wondering how that happens.

It can be either via acpi_install_address_space_handler() or through acpi_ev_initialize_op_regions(), so I guess the latter?
Comment 19 Zhang Rui 2021-05-11 02:04:54 UTC
Bjarki,
As this is actually a firmware issue, which is exposed by the commit you bisected, would you please check if there is any newer BIOS available?
and if yes, please check if the problem still exists after upgrading the BIOS.
Comment 20 Zhang Rui 2021-05-11 02:14:43 UTC
And please attach the dmidecode output.
Comment 21 Bjarki Geir Benediktsson 2021-05-14 01:37:30 UTC
As the laptop model is not released for international markets finding BIOS wasn't easy.
I found a newer BIOS (released in 2014) but since the laptop is mission critical for my PhD thesis I will not be risking flashing the bios until after I submit, hopefully later this summer. 

Do I understand correctly that the patch isn't being considered for adoption?
Comment 22 Zhang Rui 2021-05-27 13:36:16 UTC
No, we won't apply the proposed fix in this report because the original patch is the right direction, and the issue in this bug report is caused by a BIOS that violates the ACPI spec.

And for the current issue, we'd like to generate a quirk patch to force run the _REG method under EC namespace for this specific model, which also solves the problem for you. And that is why I asked for dmidecode of this laptop. Please attach it.
Comment 23 Zhang Rui 2021-06-01 05:56:24 UTC
Hi, Bjarki,

can you please attach the dmidecode of this laptop so that I can propose a quirk patch for this system?
Comment 24 Bjarki Geir Benediktsson 2021-06-01 06:35:14 UTC
Created attachment 297099 [details]
dmidecode

attached the dmidecode
Comment 25 Zhang Rui 2021-06-02 03:11:06 UTC
Created attachment 297115 [details]
patch: force evaluate EC _REG

please apply the patch on top of latest upstream Linux kernel and check if the problem is gone or not.
Comment 26 Bjarki Geir Benediktsson 2021-06-09 18:19:15 UTC
Patch applied, problem persists
Comment 27 Zhang Rui 2021-06-10 02:28:13 UTC
Can you see this line "Detected system needing force evaluating EC _REG." in dmesg after boot, after enabling the dynamic debug for ec.c?
Comment 28 Bjarki Geir Benediktsson 2021-06-10 12:16:04 UTC
As I have never used dynamic debug before, and found the instructions I could find confusing, I just wanted to make sure I did this correctly

I mounted debugfs

as root ran 
"echo -n 'file acpi/ec.c +p' > debugfs/dynamic_debug/control"

then rebooted the machine and checked DMESG and the message wasn't there
Comment 29 Zhang Rui 2021-06-11 05:10:42 UTC
No, below is how I did this.

add this line in drivers/acpi/Makefile

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 700b41adf2db..1597109367d4 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -4,6 +4,7 @@
 #
 
 ccflags-$(CONFIG_ACPI_DEBUG)   += -DACPI_DEBUG_OUTPUT
+ccflags-$(CONFIG_ACPI_DEBUG)   += -DDEBUG
 
 #
 # ACPI Boot-Time Table Parsing

And then reboot.
Comment 30 Bjarki Geir Benediktsson 2021-06-14 17:34:22 UTC
Thanks for the instructions.

The message does not appear in DMESG after adding this line and rebooting
Comment 31 Zhang Rui 2021-06-15 01:53:18 UTC
I probably also added dyndbg="module acpi +p" in my kernel commandline, but I'm not pretty sure right now.

IMO, a simple way is to just change
   pr_debug("Detected system needing force evaluating EC _REG.\n");
to
   printk("Detected system needing force evaluating EC _REG.\n");
and confirm.
Comment 32 Zhang Rui 2021-07-01 08:54:42 UTC
ping...
Comment 33 Bjarki Geir Benediktsson 2021-07-13 01:14:44 UTC
Tried adding dyndbg="module acpi +p"  to the command line, and still couldn't find the message in dmesg
Comment 34 Zhang Rui 2021-07-13 01:57:19 UTC
a simple way is to just change
   pr_debug("Detected system needing force evaluating EC _REG.\n");
to
   printk("Detected system needing force evaluating EC _REG.\n");
and confirm.

I want to make sure if the quirk is applied first, and if yes, I'd be curious why the quirk does not work for you.
Comment 35 Bjarki Geir Benediktsson 2021-07-13 14:48:30 UTC
I changed it from pr_debug to printk built, intstalled and rebooted but the line does not appear in dmesg
Comment 36 Zhang Rui 2021-07-13 15:13:32 UTC
Good to know.
This means that the quirk is not applied successfully for some unknown reason, not the idea of the patch is not working, so it is good news.

so below is your system dmidecode
Handle 0x0001, DMI type 1, 27 bytes
System Information
	Manufacturer: TOSHIBA
	Product Name: SATELLITE C870-13D
	Version: PSC8AE-019005N5
	Serial Number: 5C041590R
	UUID: c7ccae00-4c0a-81e2-2fcf-e840f2dfcd08
	Wake-up Type: Power Switch
	SKU Number: PSC8AE-019005N5
	Family: Type1Family

And this is the code in my patch.
+	{
+	/* https://bugzilla.kernel.org/show_bug.cgi?id=203563 */
+	ec_force_reg, "TOSHIBA SATELLITE C870-13D", {
+	DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+	DMI_MATCH(DMI_PRODUCT_NAME, "SATELLITE C870-13D"),}, NULL},

TBH, I don't see anything wrong here.
Can you double check your dmidecode output?
Comment 37 Zhang Rui 2021-07-13 15:14:21 UTC
Created attachment 297815 [details]
debug patch to dump dmi info

Please apply this patch on top, and attach the dmesg output after boot.
Comment 38 Bjarki Geir Benediktsson 2021-07-14 13:18:19 UTC
Created attachment 297847 [details]
dmesg

Added the dmesg after applying your patch
The print commands don't appear, there is clearly something not right with acpi, it complains about a lack of context,
Comment 39 Zhang Rui 2021-07-15 08:28:02 UTC
I'd say the important messages are flushed by these error messages.

Do you see these messages in your previous kernel?
Better rebuild your kernel and boot with the previous changes reverted, including
+ccflags-$(CONFIG_ACPI_DEBUG)   += -DDEBUG
and kernel commandline
dyndbg="module acpi +p"
Comment 40 Bjarki Geir Benediktsson 2021-08-05 14:05:03 UTC
I have now had a chance to flash the latest BIOS, and the issue no longer presents under Linux 5.13.8-arch-1 (the latest version packaged by Arch Linux)

I changed the status to resolved, should we still pursue the quirk patch or leave it as is ?

Note You need to log in before you can comment on or make changes to this bug.