Bug 203829
| Summary: | [Bisected] 9bcf15f75cac3c6a00d8f8083a635de9c8537799: Battery status no longer properly working on Lenovo Ideapad 100S | ||
|---|---|---|---|
| Product: | Drivers | Reporter: | klappnase (klappnase) |
| Component: | Other | Assignee: | Hans de Goede (jwrdegoede) |
| Status: | CLOSED CODE_FIX | ||
| Severity: | normal | CC: | rui.zhang |
| Priority: | P1 | ||
| Hardware: | Intel | ||
| OS: | Linux | ||
| Kernel Version: | 4.19.22 | Subsystem: | |
| Regression: | Yes | Bisected commit-id: | |
| Attachments: |
dmidecode info
syslog lsmod info acpidump output [PATCH] iio: adc: axp288: Override TS pin bias current for some models |
||
Created attachment 283125 [details]
syslog
A syslog excerpt that shows two subsequent boots into 4.19.21 and 4.19.22
Created attachment 283127 [details]
lsmod info
Sorted output of lsmod
I tried something else and recompiled 4.19.22 with the file drivers/iio/adc/axp288_adc.c replaced with the one from 4.19.21 . Surprisingly from a quick test this seems to fix the issue here, so it looks like the changes applied to the axp288_adc module seem to be the problem. Of course I have no idea why that is so, but I hope that maybe this gives you something to work with. Best regards Michael I should add that I only noticed yesterday that there is an updated BIOS firmware available from Lenovo. So I upgraded the laptop's BIOS to the latest version (E2CN15WW), still the behavior remains the same. The output of dmidecode now looks (apart from the BIOS version) exactly the same as the one I attached earlier. Best regards Michael there is only one change in drivers/iio/adc/axp288_adc.c recently. commit 9bcf15f75cac3c6a00d8f8083a635de9c8537799 Author: Hans de Goede <hdegoede@redhat.com> AuthorDate: Sat Jan 5 19:36:18 2019 +0100 Commit: Jonathan Cameron <Jonathan.Cameron@huawei.com> CommitDate: Sat Jan 12 17:09:55 2019 +0000 iio: adc: axp288: Fix TS-pin handling please try revert this commit in the latest upstream kernel, and see if things improve or not. Thanks for the feedback! I tried with 5.3-rc7. Just to double-check I compiled 5.3-rc7 in its original form and found that the bug still persists (same behavior as described above). Compiled again with the commit reverted, then again the battery status works fine, just as expected. Best regards Michael good to know, thanks for the confirmation. Reassign to Hans. :) Hi, klappnase, thank you for the bug report. Zhang Rui thank you for the initial triaging of the bug. klappnase, to figure out a fix for this I need some more info. First of all please install the i2c-tools, or whatever the package providing the i2cdump command is called on your distro and then do: sudo modprobe i2c-dev # note the filename below may be slightly different, e.g. :01 at the end ls -l /sys/bus/i2c/devices/i2c-INT33F4:00 This will print something like this: /sys/bus/i2c/devices/i2c-INT33F4:00 -> ../../../devices/pci0000:00/0000:00:1d.1/0000:03:00.0/0000:04:00.0/i2c-5/i2c-INT33F4:00 Notice the i2c-5 just before the i2c-INT33F4:00, the 5 is the number we are interested in, now do: sudo i2cdump -y -f # 0x34 Replace the # with the number you found by doing the ls -l /sys/bus/i2c/devices/i2c-INT33F4:00 and then copy and paste the output here. Please paste the i2cdump output for both a boot with a working kernel and a boot with a kernel with the troublesome 9bcf15f75cac3c6a00d8f8083a635de9c8537799 commit. Thanks, Hans Note to self make sure whatever fix I come up with does not regress: https://bugzilla.redhat.com/show_bug.cgi?id=1610545 Hi Hans,
thanks for the the quick response!
I did what you suggested with the two versions of 5.3-rc7 I built the other day and got the following output:
"broken" kernel 5.3-rc7 (original sources):
# i2cdump -y -f 4 0x34
No size specified (using byte-data access)
0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
00: 00 30 e0 51 00 00 3b 01 00 00 00 00 00 00 00 00 .0?Q..;?........
10: 7f 00 7a ff 48 16 16 1a 1a 00 16 00 0b 0b 00 00 ?.z.H????.?.??..
20: 02 9f b2 9e ac a8 a8 f4 15 1a 1a 00 01 30 40 00 ???????????.?0@.
30: 29 03 4b 4d 23 78 08 00 a5 1f 80 08 e8 09 00 00 )?KM#x?.??????..
40: 1c 3c ff 03 60 03 00 00 00 00 00 04 00 00 00 00 ?<.?`?.....?....
50: d0 00 00 00 65 06 b2 0a 1c 03 00 01 00 00 00 00 ?...e?????.?....
60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
70: 00 00 00 00 00 00 00 00 d1 0f 00 00 51 02 00 00 ........??..Q?..
80: 00 80 f1 00 93 b4 00 00 00 00 00 00 00 00 40 0c .??.??........@?
90: 07 1a 03 1a 00 00 00 00 00 00 42 32 22 64 0a 00 ????......B2"d?.
a0: d0 03 00 00 65 07 21 0c 00 00 00 00 00 00 00 00 ??..e?!?........
b0: 00 00 00 00 00 00 00 00 e0 b2 c0 63 d9 0c 00 00 ........???c??..
c0: 00 00 00 00 00 00 01 01 02 03 04 0c 13 20 2b 30 ......??????? +0
d0: 35 39 3b 41 45 4b 4e 51 58 5e 62 64 64 64 64 64 59;AEKNQX^bddddd
e0: 96 51 8b 17 b4 b2 d2 00 00 00 00 00 00 00 50 00 ?Q?????.......P.
f0: 01 00 80 03 00 00 00 00 00 00 00 00 00 00 00 00 ?.??............
"working" kernel 5.3-rc7 (commit reverted):
# i2cdump -y -f 4 0x34
No size specified (using byte-data access)
0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
00: 00 30 e0 51 00 00 3b 01 00 00 00 00 00 00 00 00 .0?Q..;?........
10: 7f 00 7a ff 48 16 16 1a 1a 00 16 00 0b 0b 00 00 ?.z.H????.?.??..
20: 02 9e b2 9d ac a8 a8 f4 15 1a 1a 00 01 30 40 00 ???????????.?0@.
30: 29 03 4b 4d 23 78 08 00 a5 1f 80 08 e8 09 00 00 )?KM#x?.??????..
40: 1c 3c ff 03 60 03 00 00 00 00 00 04 00 00 00 00 ?<.?`?.....?....
50: cf 07 00 00 67 0c b3 00 39 04 00 02 00 00 00 00 ??..g??.9?.?....
60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
70: 00 00 00 00 00 00 00 00 d1 0f 00 00 4d 07 00 00 ........??..M?..
80: 00 80 f1 00 f3 b4 00 00 00 00 00 00 00 00 40 0c .??.??........@?
90: 07 1a 03 1a 00 00 00 00 00 00 42 32 22 64 0a 00 ????......B2"d?.
a0: cf 04 00 00 64 08 22 00 00 00 00 00 00 00 00 00 ??..d?".........
b0: 00 00 00 00 00 00 00 00 e0 b1 c0 63 d9 07 00 00 ........???c??..
c0: 00 00 00 00 00 00 01 01 02 03 04 0c 13 20 2b 30 ......??????? +0
d0: 35 39 3b 41 45 4b 4e 51 58 5e 62 64 64 64 64 64 59;AEKNQX^bddddd
e0: 96 51 8a e4 b2 b1 d2 00 00 00 00 00 00 00 50 00 ?Q?????.......P.
f0: 01 00 80 03 00 00 00 00 00 00 00 00 00 00 00 00 ?.??............
I hope this helps (and thanks for working on this!)
Michael
Ok, so the onyl 2 registers impacted by the commit causing this problem are: #define AXP20X_ADC_EN1 0x82 #define AXP288_ADC_TS_PIN_CTRL 0x84 Register 0x82 is 0xf1 in both cases, with the 0x01 part indicating that the TS pin is enabled. 0x84 changes from 0x93 (non working kernel) to 0xf3 (working kernel). This is caused by the change described by this part of the commit msg of the troublesome commit: "Problem 2 is we were writing hardcoded values to the ADC TS pin-ctrl register, overwriting various other unrelated bits. Specifically we were overwriting the current-source setting for the TS and GPIO0 pins, forcing it to 80ųA independent of its original setting. On a Chuwi Vi10 tablet this was causing us to get a too high adc value (due to a too high current-source) resulting in the following errors being logged: ACPI Error: AE_ERROR, Returned by Handler for [UserDefinedRegion] ACPI Error: Method parse/execution failed \_SB.SXP1._TMP, AE_ERROR This commit fixes this by using regmap_update_bits to change only the relevant bits." I believe that that part of the commit is pretty much full on correct. So what the difference for reg 0x84 changing from 0x93 (non working kernel) to 0xf3 (working kernel) means is that the bias current for the TS and GPIO0 pins are being configured differently, specificly the bias current for the TS pin is 40ųA on the non working kernel and 80ųA on the working kernel. IMHO this is a firmware bug, the firmware should really initialize the bias current correctly since there is no way for us to know what the correct bias current is. So It looks like we will need to fix thus using a DMI based quirk specificly for the Lenovo Ideapad 100S. To confirm that this really is the problem, can you try executing the following command with the *non* working kernel and confirm that this fixes things? : sudo i2cset -y -f 4 0x34 0x84 0xf3 If this does not fix things, please run i2cdump again, the value may not stick as it gets cached by the regmap subsys in the kernel. If the value does not stick I will just need to prepare the fix assuming it will work. Also please do: sudo acpidump -o acpidump.lenovo-ideapad-100s And attach the generated acpidump.lenovo-ideapad-100s file here. I want to check if maybe the ACPI code is changing the bias current on the fly, in that case there might be a cleaner fix. Created attachment 284865 [details]
acpidump output
Thanks for the explanation. As far as I can see, calling the i2cset command immediately fixes things (battery light stops blinking, battery status gets updated when charging). According to i2cdump the setting of 0xf3 for the 0x84 register appears to stick, at least for the 15 minutes or so that I tried. Ok, the ACPI tables do not seem to touch the 0x84 register. So it looks like a DMI based quirk is going to be the answer here. I see you've already attached dmidecode info :) Before I write a patch for this, the power-on-reset value of register 0x84 is 0xf2 (according to the datasheet), this suggests that the BIOS does actually program the bias current, but with a wrong setting, which is weird. So 2 questions: 1) Setting 0x84 to 0xf3 changes both the GPIO0 and the TS pin currents, can you try using: sudo i2cset -y -f 4 0x34 0x84 0xb3 To change just the TS pin bias current and see if that fixes things ? I would prefer for the patch for this to only change the TS pin bias and to leave the GPIO0 bias alone. 2) Did you perhaps replace the battery of the laptop? That might explain why the BIOS set bias current is not correct... Setting the register to 0xb3 seems to fix things just as setting it to 0xf3 did. The battery is still the original one (but maybe those ideapads are equipped with cheap replacement batteries from the start :) Created attachment 284885 [details]
[PATCH] iio: adc: axp288: Override TS pin bias current for some models
klappnase,
Here is a patch which should fix this by overriding the TS pin bias current on the Ideapad 100S, please give this a try with a recent kernel.
Once you confirm that this fixes things I will submit this patch upstream.
Thanks,
Hans
Hi Hans, I applied this patch to 5.3-rc7. Battery status works as expected again. According to i2cdump the 0x84 register is set to 0xb3, as intended. Excellent work, thank you so much! :) (In reply to klappnase from comment #17) > I applied this patch to 5.3-rc7. Battery status works as expected again. > According to i2cdump the 0x84 register is set to 0xb3, as intended. Thank you for testing, I've submitted the patch upstream, it has a Cc: stable so it should show up in the stable kernels series eventually. |
Created attachment 283123 [details] dmidecode info With the Lenovo Ideapd 100S beginning with Kernel 4.19.22 the battery status no longer works properly. The bug still persists with 5.2-rc3. Unfortunately I have not been able to find useful messages in the syslog or dmesg output; I will try to describe the symptoms the best I can. When booting the system without the charger, during the boot process for some reason the laptop's battery light starts blinking white. Apart from this the battery status seems to be reported correctly. When the charger is plugged in, the battery light still blinks instead of glowing steadily. The battery appears to be charged, however the status (resp. the percentage) reported by upower never changes. When the charger is disconnected again, the reported percentage remains at the incorrect "before charging" value until the battery is discharged to a lower value. When the laptop is shut down, the battery light keeps blinking, however its color changes to red. When the laptop is being rebooted after charging, the incorrect percentage value wil still be reported, regardless of which kernel is used for the reboot. None of the above symptoms occurs with 4.19.21, both the battery light and the reported battery status work as expected. I attached dmidecode info of the Ideapad. Please let me know, if you need more information. Best regards Michael