[Samsung NP900X3C-A02US] Lid state changes not detected

[Սահակ]

Neither lid open nor lid close events are detected on Samsung 900X3C laptop.

ProblemType: Bug
DistroRelease: Ubuntu 13.10
Package: linux-image-3.11.0-12-generic 3.11.0-12.19 [modified: boot/vmlinuz-3.11.0-12-generic]
ProcVersionSignature: Ubuntu 3.11.0-12.19-generic 3.11.3
Uname: Linux 3.11.0-12-generic x86_64
ApportVersion: 2.12.5-0ubuntu2.1
Architecture: amd64
AudioDevicesInUse:
 USER PID ACCESS COMMAND
 /dev/snd/controlC0: sahak 1482 F.... pulseaudio
Date: Sun Nov 3 23:11:48 2013
InstallationDate: Installed on 2013-10-26 (8 days ago)
InstallationMedia: Ubuntu 13.10 "Saucy Salamander" - Release amd64 (20131016.1)
MachineType: SAMSUNG ELECTRONICS CO., LTD. 900X3C/900X3D/900X4C/900X4D
MarkForUpload: True
ProcFB: 0 inteldrmfb
ProcKernelCmdLine: BOOT_IMAGE=/boot/vmlinuz-3.11.0-12-generic root=UUID=366ff628-3319-488f-b6f2-7b1872de5733 ro quiet splash vt.handoff=7
RelatedPackageVersions:
 linux-restricted-modules-3.11.0-12-generic N/A
 linux-backports-modules-3.11.0-12-generic N/A
 linux-firmware 1.116
SourcePackage: linux
UpgradeStatus: No upgrade log present (probably fresh install)
WifiSyslog: Nov 3 23:05:26 samsung kernel: [ 5958.261114] perf samples too long (2517 > 2500), lowering kernel.perf_event_max_sample_rate to 50000
dmi.bios.date: 02/08/2013
dmi.bios.vendor: Phoenix Technologies Ltd.
dmi.bios.version: P06AAC
dmi.board.asset.tag: Base Board Asset Tag
dmi.board.name: SAMSUNG_NP1234567890
dmi.board.vendor: SAMSUNG ELECTRONICS CO., LTD.
dmi.board.version: FAB1
dmi.chassis.asset.tag: No Asset Tag
dmi.chassis.type: 9
dmi.chassis.vendor: SAMSUNG ELECTRONICS CO., LTD.
dmi.chassis.version: 0.1
dmi.modalias: dmi:bvnPhoenixTechnologiesLtd.:bvrP06AAC:bd02/08/2013:svnSAMSUNGELECTRONICSCO.,LTD.:pn900X3C/900X3D/900X4C/900X4D:pvr0.1:rvnSAMSUNGELECTRONICSCO.,LTD.:rnSAMSUNG_NP1234567890:rvrFAB1:cvnSAMSUNGELECTRONICSCO.,LTD.:ct9:cvr0.1:
dmi.product.name: 900X3C/900X3D/900X4C/900X4D
dmi.product.version: 0.1
dmi.sys.vendor: SAMSUNG ELECTRONICS CO., LTD.

[email (email-linux-kernel-bugs)]

While the power supply device (ADP1) seems to always understand when the cord is plugged in and removed, the battery (BAT1) does not seem to know when it is discharging or not.

Samsung Series 9 np900x4b with Fedora 17

$ uname -a
Linux Lethe 3.4.4-3.fc17.x86_64 #1 SMP Tue Jun 26 20:54:56 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux

(Results of acpi -i -b -a)
*** AC PLUGGED IN ***
Battery 0: Full, 100%
Battery 0: design capacity 8400 mAh, last full capacity 8500 mAh = 100%
Adapter 0: on-line

*** AC REMOVED ***
Battery 0: Charging, 100%, until charged
Battery 0: design capacity 8400 mAh, last full capacity 8500 mAh = 100%
Adapter 0: off-line

*** System Suspended and woken back up ***

*** AC STILL REMOVED ***
Battery 0: Discharging, 100%, 06:33:49 remaining
Battery 0: design capacity 8400 mAh, last full capacity 8500 mAh = 100%
Adapter 0: off-line

*** AC PLUGGED IN ***
Battery 0: Unknown, 98%
Battery 0: design capacity 8400 mAh, last full capacity 8500 mAh = 100%
Adapter 0: on-line

*** SUSPEND AND WAKE ***

*** AC PLUGGED IN ***
Battery 0: Unknown, 98%
Battery 0: design capacity 8400 mAh, last full capacity 8500 mAh = 100%
Adapter 0: on-line

*** AC REMOVED ***
Battery 0: Charging, 98%, charging at zero rate - will never fully charge.
Battery 0: design capacity 8400 mAh, last full capacity 8500 mAh = 100%
Adapter 0: off-line

The same information is reflected when looking directly at the sysfs enteries.
/sys/class/power_supply/ADP1/online shows the correct status all the time. (1 for online, 0 offline)

/sys/class/power_supply/BAT1/status will show the correct status after a fresh suspend, reflecting the above results.

[email (email-linux-kernel-bugs)]

Created attachment 74581
dmesg

[email (email-linux-kernel-bugs)]

Created attachment 74591
lsmod

[email (email-linux-kernel-bugs)]

Created attachment 74601
lspci

[email (email-linux-kernel-bugs)]

Created attachment 74611
upower -d

[email (email-linux-kernel-bugs)]

Created attachment 74621
ACPI DSDT DSL

[email (email-linux-kernel-bugs)]

The effect this has on my system is that upowerd does not always know if the laptop is running on AC or Battery. This also means that things like the battery indicator in Gnome 3 and tuned do not end up in the right state.

[bryan+lk (bryan+lk-linux-kernel-bugs)]

Also affects new Ivy Bridge Samsung Series 9 models (observed on 900X4C). Also tested this using mainline kernels without vendor changes and got the same behavior.

[mark (mark-linux-kernel-bugs)]

Just in case the NP400x4C (Ivy Bridge version) give different values, here are the results of acpi -i -b -a on this system.

** AC Off **

Battery 0: Discharging, 70%, 03:46:06 remaining
Battery 0: design capacity 8400 mAh, last full capacity 8700 mAh = 100%
Adapter 0: off-line

** AC On **

Battery 0: Discharging, 70%, 01:55:16 remaining
Battery 0: design capacity 8400 mAh, last full capacity 8700 mAh = 100%
Adapter 0: on-line

** Suspend and wake **

** AC Still On **

Battery 0: Charging, 70%, 00:50:05 until charged
Battery 0: design capacity 8400 mAh, last full capacity 8700 mAh = 100%
Adapter 0: on-line

** AC Off **

Battery 0: Charging, 70%, 02:11:35 until charged
Battery 0: design capacity 8400 mAh, last full capacity 8700 mAh = 100%
Adapter 0: off-line

** Suspend and wake in this state **

** AC Off **

Battery 0: Discharging, 70%, 05:06:01 remaining
Battery 0: design capacity 8400 mAh, last full capacity 8700 mAh = 100%
Adapter 0: off-line

** AC On **

Battery 0: Discharging, 70%, 406:00:00 remaining
Battery 0: design capacity 8400 mAh, last full capacity 8700 mAh = 100%
Adapter 0: on-line

What I find quite interesting about these is that the times change as the AC state changes and seem to reflect the "correct" time, i.e. time to discharge when disconnected and time to charged when connected. So something is detecting the change, it's just not propagating to all properties.

[mark (mark-linux-kernel-bugs)]

Should have said, the above came from an Ubuntu 12.04 machine using mainline kernel packages.

Linux XXXXXX 3.4.0-030400-generic #201205210521 SMP Mon May 21 09:22:02 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux

[mail (mail-linux-kernel-bugs)]

Also affected, Samsung 5 Series NP530U3C laptop, same symptoms:

Plugged in:
Battery 0: Charging, 79%, 00:27:20 until charged
Battery 0: design capacity 6100 mAh, last full capacity 5900 mAh = 96%
Adapter 0: on-line

Not plugged in:
Battery 0: Charging, 79%, 01:26:32 until charged
Battery 0: design capacity 6100 mAh, last full capacity 5900 mAh = 96%
Adapter 0: off-line

uname -a
Linux jaytec 3.2.0-27-generic #43-Ubuntu SMP Fri Jul 6 14:25:57 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux

Also had Ubuntu Quantal 3.5 kernel before wiping and reinstalling 3.2. The bug existed there also.

[mail (mail-linux-kernel-bugs)]

Created attachment 75961
dmesg

[mail (mail-linux-kernel-bugs)]

Created attachment 75971
lshw

[mail (mail-linux-kernel-bugs)]

Created attachment 75981
lsmod

[mail (mail-linux-kernel-bugs)]

Created attachment 75991
lspci

[mail (mail-linux-kernel-bugs)]

Created attachment 76001
upower

[mail (mail-linux-kernel-bugs)]

This for me seems fixed, although my Samsung laptop brightness buttons cause the screen to flicker and thus are useless now :)

uname -a
Linux jaytec 3.2.0-27-generic #43-Ubuntu SMP Fri Jul 6 14:25:57 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux

Kernel has not updated, not quite sure what update fixed it, possibly a driver update from Ubuntu http://ppa.launchpad.net/ubuntu-x-swat/x-updates/ Ubuntu PPA which offers updated X drivers.

[email (email-linux-kernel-bugs)]

Interesting... though I don't see how it could be x drivers that would fix things. I can say though that the status updates seemed to work at *some* point for me... I remember it working quite clearly... but then it stopped working again for some reason and has not worked since. Almost makes me think it's an ACPI issue on the laptop itself... though things do work in windows so I am really not sure.

(Actually it *may* be broken in windows too, but Windows may be seeing the fact it's discharging and updating the icon based on that vs the ac being unplugged)

[Brad Figg (brad-figg)]

This change was made by a bot.

[Սահակ (petrosyan)]

NP900X3C-A02US

[Սահակ (petrosyan)]

Hi Christopher,

I checked and my current BIOS ( P06AAC ) is the latest version of the BIOS.

[penalvch (penalvch)]

Սահակ, could you please test the latest upstream kernel available (not the daily folder, but the one all the way at the bottom) following https://wiki.ubuntu.com/KernelMainlineBuilds ? It will allow additional upstream developers to examine the issue. Once you've tested the upstream kernel, please comment on which kernel version specifically you tested. If this bug is fixed in the mainline kernel, please add the following tags:
kernel-fixed-upstream
kernel-fixed-upstream-VERSION-NUMBER

where VERSION-NUMBER is the version number of the kernel you tested. For example:
kernel-fixed-upstream-v3.12

This can be done by clicking on the yellow circle with a black pencil icon next to the word Tags located at the bottom of the bug description. As well, please remove the tag:
needs-upstream-testing

If the mainline kernel does not fix this bug, please add the following tags:
kernel-bug-exists-upstream
kernel-bug-exists-upstream-VERSION-NUMBER

As well, please remove the tag:
needs-upstream-testing

Once testing of the upstream kernel is complete, please mark this bug's Status as Confirmed. Please let us know your results. Thank you for your understanding.

[Սահակ (petrosyan)]

I tested it with kernel

Linux samsung 3.12.0-999-generic #201311050511 SMP Tue Nov 5 10:13:10 UTC 2013 x
86_64 x86_64 x86_64 GNU/Linux

and I am still seeing this bug.

[penalvch (penalvch)]

Սահակ, could you please test the latest upstream kernel available (not the daily folder) but http://kernel.ubuntu.com/~kernel-ppa/mainline/v3.12-saucy/ ?

[Սահակ (petrosyan)]

I also tested on 3.12.0 kernel and the bug is still there.

Linux samsung 3.12.0-031200-generic #201311031935 SMP Mon Nov 4 00:36:54 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux

[penalvch (penalvch)]

Սահակ, did this problem not occur in a release prior to Saucy?

[Սահակ (petrosyan)]

this problem did occur in Ubuntu 13.04 as well

[penalvch (penalvch)]

Սահակ, for regression testing purposes could you please test http://old-releases.ubuntu.com/releases/lucid/ and advise to the results?

[Սահակ (petrosyan)]

Christopher, how can I install Ubuntu 10.04.4?

Since this laptop was released in 2013 it is very unlikely that Ubuntu 10.04.4 would have support for this laptop.

[penalvch (penalvch)]

"Christopher, how can I install Ubuntu 10.04.4?"

You wouldn't need to install it, but just test functionality in a Live environment.

"Since this laptop was released in 2013 it is very unlikely that Ubuntu 10.04.4 would have support for this laptop."

I wouldn't base likelihood solely on when a model laptop was released. What would be more accurate is the components within the laptop.

[Սահակ (petrosyan)]

Hi Christopher,

I downloaded Ubuntu 10.04.04 iso image but "Startup Disk Creator" crashes when trying to create a live USB image. I am running it on Ubuntu 13.10.

Having wasted a lot of time on satisfying you requests I feel compelled to leave here some feedback for you.

A lot of people who open bug reports on the launchpad, myself included, are Ubuntu developers. By making unreasonable requests, such as testing on ancient Ubuntu releases, you are wasting our valuable time that could be spent on development. In other words your contribution to Ubuntu community is negative.

[penalvch (penalvch)]

Սահակ, to maintain a respectful atmosphere, please follow the code of conduct - http://www.ubuntu.com/project/about-ubuntu/conduct . Bug reports are handled by humans, the majority of whom are volunteers, so please bear this in mind.

With this in mind, as skilled developers know, the purpose of a regression test is to find out if this is simply a matter of reversing a commit, build on the foundation of the regression commit, or this has happened as far back as one could test (typically Lucid). This type of information is vital to a developer in diagnosing and fixing the issue.

As well, I've seen regressions go unfixed as back as far as Jaunty/9.04, going into current kernel releases.

Hence, presuming asking for a regression test is unreasonable is counter productive to getting a bug solved.

Regarding usb-creator-gtk crashing, this is a known issue -> https://bugs.launchpad.net/ubuntu/+source/usb-creator/+bug/915626 .

A WORKAROUND to burning an ISO to USB is:
sudo dd if=custom-backup.iso of=/dev/sdb ibs=10M obs=20M

where the following is replaced by your ISO name:
custom-backup.iso

and the following is replaced with your USB drive location:
/dev/sdb

[Սահակ (petrosyan)]

sudo dd if=custom-backup.iso of=/dev/sdb ibs=10M obs=20M workaround created a non-bootable USB image.

I tried unetbootin tool to create a USB boot image and the computer started booting from the USB key but the bootup process never finished.

[penalvch (penalvch)]

Սահակ, the next newer releases may be found at http://old-releases.ubuntu.com/releases/ .

[Launchpad Janitor (janitor)]

[Expired for linux (Ubuntu) because there has been no activity for 60 days.]

[mmalmeida (mmalmeida)]

For those affected by this bug, please consider the solution posted on https://bugs.launchpad.net/ubuntu/+source/acpi/+bug/971061- it might be what you need.

[Սահակ (petrosyan)]

thanks a lot mmalmeida!!!!!

the patch indeed fixes this bug!!!!

let's hope it gets incorporated into the kernel.

[lv.zheng (lv.zheng-linux-kernel-bugs)]

Hi,

The test results are very useful for us to understand this issue. :-)

(In reply to Andrea Fagiani from comment #227)
> > B. We want to see some proofs that Samsung EC firmware acts exactly as what
> > we've inferred from the reversion requirements. So could you:
> > 1. using linux-pm/linux-next with patches from Comment 219.
> > 2. Uncomment the following line in the drivers/acpi/ec.c:
> > /* #define DEBUG */
> > 3. build/boot the kernel
> > 4. do some LID state changes and make sure they are captured by
> acpi_listen.
> > 5. upload the dmesg output here.
>
> Opened/closed 5 times, all of them showed up in acpi_listen.
> Attaching dmesg.

Yes. I also found the 0x5E (5 times) and 0x5F (4 times) in the dmesg.
TBH, I didn't find proofs for comment 210 in it.
So the issue isn't as what I imagined before.

> > C. We want to confirm if the timely polling support is really required by
> > Samsung EC firmware:
> > 1. using linux-pm/linux-next
> > 3. git am gpe-handle21.patch (attachment 156591 [details])
> > 4. git am ec-flush1.patch (attachment 155351 [details])
> > 5. git am ec-flush2.patch (attachment 155371 [details])
> > 6. git am ec-flush3.patch (attachment 155381 [details])
> > 7. git am ec-flush4.patch (attachment 155391 [details])
> > 8. git am ec-flush5.patch (attachment 155411 [details])
> > 9. git am ec-flush6.patch (attachment 155421 [details])
> > Do not apply the ec-flush7.patch, can power plug and LID switch still work,
> > and can still work even after resuming?
>
> Both work fine, even after a suspend/resume.

Thus, in order to fix Samsung issue.
Either we need timely polling quirk (since test A works)
Or we need old CLEAR_ON_RESUME quirk (since test C works).

> Note that the ec-flush patches depend on the reverts, so I applied them even
> if they weren't explicitly listed. I can test without them later today if
> required.

Not necessary :-). I only want the kernel test results __wih__ the reverts (as they conflict with the patches posted here) so I don't need to post another set of rebased patches for current linux-pm/linux-next.

> > D. We want to confirm if this is really a regression and current fix can:
> > 1. Using linux-pm/linux-next
> > 2. git revert 79149001
> > 3. git revert df9ff918
> > Do not apply any of the posted patches, can power plug and LID switch still
> > work? And
>
> Upon boot, it works correctly. After a suspend/resume, power plug detection
> does not work anymore (it is greatly delayed) and neither does the LID
> switch (doesn't trigger at all).

Thus the regression seems to be: "the 2 commits have broken the CLEAR_ON_RESUME quirk".

I guess SCI_EVT is set after resuming but GPE doesn't fire.
If this is the case, this isn't a silicon/firmware bug but a driver bug.
As EC GPE is edge triggered and GPE might have already been fired before suspending.
If so, we need to improve the driver and shouldn't fix the driver bug using any kind of quirk.

So I want to check if the SCI_EVT is set after resuming.
Let me think about a way to confirm this.
I'll add another test later.

> > 1. Using linux-pm/linux-next
> > Do not revert the "regression fix" commits and do not apply any of the
> > posted patches...

[lv.zheng (lv.zheng-linux-kernel-bugs)]

Hi, Andrea

Please use either of the following working combinations:
1. test A
2. test C
3. test D2 (which you haven't done yet)

If you'll do test D2 for us, you can just do it and capture the confirmation information at same time.

Please do:
1. suspend
2. do some LID opening/closing to trigger old issue
3. resume
4. dmesg > dmesg-right-after-resume.txt

And post the dmesg-right-after-resume.txt here.
If the kernel log buffer size is not big enough to contain all messages right after resuming, please increase it using the kernel boot parameter - log_buf_len=1M.

Thanks and best regards

[lv.zheng (lv.zheng-linux-kernel-bugs)]

Hi, Andrea

Sorry that I forgot to ask you to enable the EC log to capture the debug information.

2. Uncomment the following line in the drivers/acpi/ec.c:
     /* #define DEBUG */

(In reply to Lv Zheng from comment #230)
> Hi, Andrea
>
> Please use either of the following working combinations:
> 1. test A
> 2. test C
> 3. test D2 (which you haven't done yet)
>
> If you'll do test D2 for us, you can just do it and capture the confirmation
> information at same time.
>
> Please do:
> 1. suspend
> 2. do some LID opening/closing to trigger old issue
> 3. resume
> 4. dmesg > dmesg-right-after-resume.txt
>
> And post the dmesg-right-after-resume.txt here.
> If the kernel log buffer size is not big enough to contain all messages
> right after resuming, please increase it using the kernel boot parameter -
> log_buf_len=1M.

Thanks
-Lv

[andfagiani (andfagiani-linux-kernel-bugs)]

Created attachment 157351
dmesg-right-after-resume_A

[andfagiani (andfagiani-linux-kernel-bugs)]

Created attachment 157361
dmesg-right-after-resume_C

[andfagiani (andfagiani-linux-kernel-bugs)]

Created attachment 157371
dmesg-right-after-resume_D2

[andfagiani (andfagiani-linux-kernel-bugs)]

Hi Lv,
I captured and uploaded the dmesg as requested.

Concerning test D2, both power plug and LID switch seem to work fine even after a suspend/resume. You'll notice I've played with it a little bit before performing the suspend test.

Hope it helps,
Andrea

[lv.zheng (lv.zheng-linux-kernel-bugs)]

Hi, Andrea

The results make something clear to me.
Thanks for the great job.

For the 3 test results, SCI_EVT is always flagged after resuming:
Test A (regression fixes + cleanup + timely polling quirk):
[ 42.608720] PM: Restoring platform NVS memory
...
[ 42.649355] ACPI : EC: EC_SC(R) = 0x28 SCI_EVT=1 BURST=0 CMD=1 IBF=0 OBF=0
Test C (regression fixes + cleanup + CLEAR_ON_RESUME quirk):
[ 21.764941] PM: Restoring platform NVS memory
...
[ 21.811916] ACPI : EC: EC_SC(R) = 0x28 SCI_EVT=1 BURST=0 CMD=1 IBF=0 OBF=0
Test D2 (regression fixes + CLEAR_ON_RESUME quirk):
[ 172.102165] PM: Restoring platform NVS memory
...
[ 172.149860] ACPI : EC: EC_SC(R) = 0x28 SCI_EVT=1 BURST=0 CMD=1 IBF=0 OBF=0
This can explain the cause of the original bug identified in comment 129.
The EC behavior is not broekn here. We shouldn't implement a quirk for it since this is a driver bug - if the EC GPE is not fired, we don't have means to poll the events.

For the regression reported in comment 184, I only find the proof in test A that we need both the 2 commits to be reverted:
Test A (regression fixes + cleanup + timely polling quirk):
[ 42.662695] ACPI : EC: ***** Command(QR_EC) started *****
[ 42.662697] ACPI : EC: ===== TASK (2) =====
[ 42.662702] ACPI : EC: EC_SC(R) = 0x00 SCI_EVT=0 BURST=0 CMD=0 IBF=0 OBF=0
[ 42.662703] ACPI : EC: EC_SC(W) = 0x84
[ 42.662707] ACPI : EC: ***** Event running *****
[ 42.662827] ACPI : EC: ##### Query(0x5e) stopped #####
[ 42.666037] ACPI : EC: ===== TASK (2) =====
[ 42.666042] ACPI : EC: EC_SC(R) = 0x09 SCI_EVT=0 BURST=0 CMD=1 IBF=0 OBF=1
[ 42.666045] ACPI : EC: EC_DATA(R) = 0x5f
[ 42.666046] ACPI : EC: ***** Command(QR_EC) hardware completion *****
[ 42.666047] ACPI : EC: ***** Command(QR_EC) stopped *****
The event of 0x5F is returned when SCI_EVT=0.
So we need to revert the 2 commits to allow QR_EC to be sent when SCI_EVT=0 and there is really non 0x00 events need to be drained when SCI_EVT=0.
TBH, the EC behavior is broekn here, thus IMO this is a different issue than the one identified in comment 129.
It seems both the quirks (drain way of CLEAR_ON_RESUME, timely polling way) can work it around. If this issue can be reproduced during runtime, then the CLEAR_ON_RESUME can not work it around at that time. According to your tests, the problem cannot be easily reproduced during runtime. So we can stay unchanged until someone else blaims.

If we have to assume this is only a problem happened during resuming and if we want to root cause why the SCI_EVT is suddently cleared after resuming, we will have to assume that something during the resuming steps has caused this issue because:
1. In test C, 2 stale events are returned with SCI_EVT=1:
[ 21.814019] ACPI : EC: EC_SC(R) = 0x29 SCI_EVT=1 BURST=0 CMD=1 IBF=0 OBF=1
[ 21.814022] ACPI : EC: EC_DATA(R) = 0x5e
[ 21.817333] ACPI : EC: EC_SC(R) = 0x29 SCI_EVT=1 BURST=0 CMD=1 IBF=0 OBF=1
[ 21.817351] ACPI : EC: EC_DATA(R) = 0x5f
So SCI_EVT will not be spontaneously cleared during acpi_ec_unblock_transactions().
2. In test A, 2 stale events are returned with different SCI_EVT value:
[ 42.656421] ACPI : EC: EC_SC(R) = 0x29 SCI_EVT=1 BURST=0 CMD=1 IBF...

[lv.zheng (lv.zheng-linux-kernel-bugs)]

Hi, Andrea

I checked the log again. And the following seems to be wrong.

> It seems both the quirks (drain way of CLEAR_ON_RESUME, timely polling way)
> can work it around. If this issue can be reproduced during runtime, then the
> CLEAR_ON_RESUME can not work it around at that time. According to your
> tests, the problem cannot be easily reproduced during runtime. So we can
> stay unchanged until someone else blaims.

Though you didn't face real functional issues, there are following entries in the test D2 log:
[ 31.072607] ACPI : EC: EC_SC(R) = 0x09 SCI_EVT=0 BURST=0 CMD=1 IBF=0 OBF=1
[ 31.072613] ACPI : EC: EC_DATA(R) = 0x66
[ 77.835639] ACPI : EC: EC_SC(R) = 0x09 SCI_EVT=0 BURST=0 CMD=1 IBF=0 OBF=1
[ 77.835647] ACPI : EC: EC_DATA(R) = 0x66
[ 79.939044] ACPI : EC: EC_SC(R) = 0x09 SCI_EVT=0 BURST=0 CMD=1 IBF=0 OBF=1
[ 79.939053] ACPI : EC: EC_DATA(R) = 0x66
[ 82.006084] ACPI : EC: EC_SC(R) = 0x29 SCI_EVT=1 BURST=0 CMD=1 IBF=0 OBF=1
[ 82.006087] ACPI : EC: EC_DATA(R) = 0x66
Thanks for you long time test or we won't find them out.
The _Q66 is as follows:
                    Method (_Q66, 0, NotSerialized)
                    {
                        P8XH (Zero, 0x66)
                        If (LEqual (B1EX, One))
                        {
                            Notify (BAT1, 0x80)
                        }
                    }
It updates battery status.
Then it means the assumption in comment 210 might be real and we need the "timely polling quirk" for samsung to have everything wroked around...
So I'll prepare the whole patchset posted here for upstream according to this result.

In test A, I can even see such log entries:
[ 1.788647] ACPI : EC: ***** Command(QR_EC) started *****
[ 1.788651] ACPI : EC: ===== TASK (1) =====
[ 1.788656] ACPI : EC: EC_SC(R) = 0x08 SCI_EVT=0 BURST=0 CMD=1 IBF=0 OBF=0
[ 1.788659] ACPI : EC: EC_SC(W) = 0x84
...
[ 1.800289] ACPI : EC: ===== IRQ (0) =====
[ 1.800295] ACPI : EC: EC_SC(R) = 0x29 SCI_EVT=1 BURST=0 CMD=1 IBF=0 OBF=1
[ 1.800300] ACPI : EC: EC_DATA(R) = 0x70
...
[ 1.800385] ACPI : EC: ***** Command(QR_EC) stopped *****
It seems for Samsung EC, the firmware can even return event that happens after we send the query command. There can really be a big time slot between driver pushes the command byte and firmware fetches it.

Thanks and best regards

[clancy.kieran+kernel (clancy.kieran+kernel-linux-kernel-bugs)]

Hi Lv,

Thanks for all your work on this issue. It's certainly not easy, especially if you don't have the hardware available to test on yourself.

You are probably right about the racy behaviour. One important point though, is that you would need to miss either 8 (or was it 16?) events in a row during run time for the buffer to fill and for SCI_EVT to stop being given entirely.

Under normal usage, I think there's almost always going to be an event regularly enough that this "full" condition never occurs. It was only found to be a problem during suspend, where the EC controller seemed to continue logging, and after enough battery level or AC adapter changes it would stop triggering events even after resume.

My HDD was too full recently to build kernels, but I have just freed some space (a bit late, sorry) so if you want me to test anything please let me know.

[lv.zheng (lv.zheng-linux-kernel-bugs)]

(In reply to Kieran Clancy from comment #238)
> Hi Lv,
>
> Thanks for all your work on this issue. It's certainly not easy, especially
> if you don't have the hardware available to test on yourself.
>
> You are probably right about the racy behaviour. One important point though,
> is that you would need to miss either 8 (or was it 16?) events in a row
> during run time for the buffer to fill and for SCI_EVT to stop being given
> entirely.
>
> Under normal usage, I think there's almost always going to be an event
> regularly enough that this "full" condition never occurs. It was only found
> to be a problem during suspend, where the EC controller seemed to continue
> logging, and after enough battery level or AC adapter changes it would stop
> triggering events even after resume.
>
> My HDD was too full recently to build kernels, but I have just freed some
> space (a bit late, sorry) so if you want me to test anything please let me
> know.

Hi,

Thanks a lot!
We failed to find this platform selling on China market.

I was thinking this issue doesn't exist during runtime. That's why I said in comment 236 that it might be a BIOS _WAK issue or something in acpi_hw_legacy_wake() has cleared SCI_EVT and we may need further investigation. But finally in comment 237, I found it occurred not only after resuming, but also runtime, so further investigation doesn't matter any more because this is definitely an EC firmware behavior according to the facts.

In our driver, we will queue 2nd QR_EC after sending the 1st QR_EC and before fetching the returned event. At that time, SCI_EVT should still be 1, so we can always send 2 QR_EC for 1 SCI_EVT=1 indication. This somewhat can reduce the reproduction ratio of the Samsung EC issue and that's why during runtime it can hardly be reproduced as we can drain events faster than they are accumulated in the firmware.

But the above code can only make a happen to work environment. If the host acts a bit slower than the target, this issue might still occurr during runtime when there are more than 3 events queued up and the driver will be unable to queue 3rd QR_EC if SCI_EVT is cleared after fetching the 1st event.

It seems the best way for Samsung is always sending QR_EC whatever SCI_EVT is until 0x00 returned. This driver behavior is reasonable from ACPI spec's point of view. The CLEAR_ON_RESUME quirk provided by you did this after resuming, we may need to extend this behavior to runtime.

But we do have many platforms act differently:
1. Some platforms never return 0x00 even when SCI_EVT=0, they return certain event value. Some of them even return the event value for which there is no _Qxx prepared in the namespace. So if we continously send QR_EC until 0x00 is returned, the process will never end on such platforms. The users of such platforms will sure blame us.
2. Some platforms (Acer) do not return anything if SCI_EVT=0, the EC query transaction will be blocked, and our driver cannot issue further EC transaction unless previous one is completed. So if we allow QR_EC to be sent without checking SCI_EVT, users of such platforms will complain.

What I'm going to do is:
1. extending the draining behavior - poll...

[lv.zheng (lv.zheng-linux-kernel-bugs)]

Hi, Kieran

Can you:
1. use current linux-pm/linux-next branch
2. enable EC debugging
3. disable quirk
4. trigger the bug
5. capture the dmesg right after resume
6. upload the dmesg here
for investigation?

Thanks in advance

[clancy.kieran+kernel (clancy.kieran+kernel-linux-kernel-bugs)]

Created attachment 157601
dmesg on linux-next without quirk showing EC working before but not after triggering bug

(In reply to Lv Zheng from comment #240)
> Hi, Kieran
>
> Can you:
> 1. use current linux-pm/linux-next branch
> 2. enable EC debugging

Is it enough to just uncomment the #define DEBUG?

> 3. disable quirk

Is it enough to prevent EC_FLAGS_CLEAR_ON_RESUME from being set? Is this what you meant?

> 4. trigger the bug
> 5. capture the dmesg right after resume
> 6. upload the dmesg here

See attached. I annotated the dmesg with a few extra > /dev/kmsg lines starting with 'KC'. They are:

[ 107.787915] KC just booted and logged in
[ 137.240937] KC screen close/open works as intended
[ 203.943414] KC about to suspend, but not trigger bug this time
[ 226.584995] KC back from suspend
[ 265.000194] KC screen close/open works as intended
[ 298.608956] KC AC change detected as intended
[ 318.874279] KC about to suspend and trigger bug
[ 348.834217] KC back from suspend
[ 374.777099] KC screen open/close not detected
[ 398.000040] KC AC change not detected
[ 425.733615] KC about to manually clear EC events with script
[ 442.663539] KC EC events cleared
[ 469.766095] KC AC change detected again
[ 496.219180] KC screen open/close detected again

So between 0 and 318 you can see the EC operating as intended including one suspend cycle.

After 318 I trigger the bug (by unplugging AC a bunch of times), and after that you can see that there is nothing at all logged when I either closed the screen or unplugged AC.

At 425 I manually simulated the quirk by sending 0x84 EC command queries and reading the data until the data is 0x00 using Juan Manuel's userspace program.

After that, EC events are detected properly again.

----

My analysis is below:

After the bug is triggered, SCI_EVT=1 is set just ONE time, immediately after resume:

[ 337.319012] ACPI : EC: EC_SC(R) = 0x28 SCI_EVT=1 BURST=0 CMD=1 IBF=0 OBF=0

It does not seem as though we ever handle this event properly though. Namely, there seems to be no corresponding "EC_SC(W) = 0x84". There is a couple of "EC_DATA(W) = 0x84" but I'm pretty sure these are totally different?

----

Further testing shows that this SCI_EVT=1 happens for the first resume after the bug is triggered, but not for the second or subsequent resumes.

That means that we are still going to need a wakeup quirk because if for some reason we fail to clear the EC state before the next suspend, we will never get another SCI_EVT=1 (even after a power cycle, I believe).

[clancy.kieran+kernel (clancy.kieran+kernel-linux-kernel-bugs)]

Created attachment 157611
dmesg on linux-next without quirk showing bug persisting over several suspend cycles

Of the three suspend/resume cycles shown in this dmesg, I trigger the EC bug during the first suspend time.

It comes back with SCI_EVT=1 set the first time, but the 2nd and 3rd resumes do not have this. In a moment I will confirm if this persists across power cycles.

It seems the right behaviour for affected Samsung machines is to send QR_EC until data is 0x00 not just if we get SCI_EVT=1, but additionally on boot or resume.

[clancy.kieran+kernel (clancy.kieran+kernel-linux-kernel-bugs)]

Created attachment 157631
dmesg on linux-next without quirk showing bug persisting over power cycle

Confirming that once the bug is initially triggered, we don't get SCI_EVT=1 even on a power cycle, so we still need the boot/resume quirk. At least, that's my interpretation.

I hope the dmesgs were helpful. Unfortunately I'm not going to have internet access for the next week (going to the outback where there is not even any mobile reception), but I'm happy to test more things when I return.

[lv.zheng (lv.zheng-linux-kernel-bugs)]

(In reply to Kieran Clancy from comment #241)
> Created attachment 157601 [details]
> dmesg on linux-next without quirk showing EC working before but not after
> triggering bug
>
> (In reply to Lv Zheng from comment #240)
> > Hi, Kieran
> >
> > Can you:
> > 1. use current linux-pm/linux-next branch
> > 2. enable EC debugging
>
> Is it enough to just uncomment the #define DEBUG?

Yes.

> > 3. disable quirk
>
> Is it enough to prevent EC_FLAGS_CLEAR_ON_RESUME from being set? Is this
> what you meant?

Yes.

> > 4. trigger the bug
> > 5. capture the dmesg right after resume
> > 6. upload the dmesg here
>
> See attached. I annotated the dmesg with a few extra > /dev/kmsg lines
> starting with 'KC'. They are:
>
> [ 107.787915] KC just booted and logged in
> [ 137.240937] KC screen close/open works as intended
> [ 203.943414] KC about to suspend, but not trigger bug this time
> [ 226.584995] KC back from suspend
> [ 265.000194] KC screen close/open works as intended
> [ 298.608956] KC AC change detected as intended
> [ 318.874279] KC about to suspend and trigger bug
> [ 348.834217] KC back from suspend
> [ 374.777099] KC screen open/close not detected
> [ 398.000040] KC AC change not detected
> [ 425.733615] KC about to manually clear EC events with script
> [ 442.663539] KC EC events cleared
> [ 469.766095] KC AC change detected again
> [ 496.219180] KC screen open/close detected again

Great information!
Thanks.

> So between 0 and 318 you can see the EC operating as intended including one
> suspend cycle.
>
> After 318 I trigger the bug (by unplugging AC a bunch of times), and after
> that you can see that there is nothing at all logged when I either closed
> the screen or unplugged AC.
>
> At 425 I manually simulated the quirk by sending 0x84 EC command queries and
> reading the data until the data is 0x00 using Juan Manuel's userspace
> program.
>
> After that, EC events are detected properly again.

Great test cases!
Thanks.

> My analysis is below:
>
> After the bug is triggered, SCI_EVT=1 is set just ONE time, immediately
> after resume:
>
> [ 337.319012] ACPI : EC: EC_SC(R) = 0x28 SCI_EVT=1 BURST=0 CMD=1 IBF=0 OBF=0
>
> It does not seem as though we ever handle this event properly though.
> Namely, there seems to be no corresponding "EC_SC(W) = 0x84". There is a
> couple of "EC_DATA(W) = 0x84" but I'm pretty sure these are totally
> different?

This is a bug, in ec_poll(), there is no code to check SCI_EVT.
And event will thus be lost.

I think this has been fixed in the ec-flush6.patch.
+out:
+ if (status & ACPI_EC_FLAG_SCI &&
+ (!t || t->flags & ACPI_EC_COMMAND_COMPLETE))
+ __acpi_ec_set_event(ec);

We will have this flag checked in the advance_transaction.
So you won't see this with ec-flush[1-6].patch applied.

> Further testing shows that this SCI_EVT=1 happens for the first resume after
> the bug is triggered, but not for the second or subsequent resumes.
>
> That means that we are still going to need a wakeup quirk because if for
> some reason we fail to clear the EC state before the next suspend, we will
> never get another SCI_EVT=1 (even after a power cycle, I believe).

Yes, I think this is requi...

[lv.zheng (lv.zheng-linux-kernel-bugs)]

(In reply to Kieran Clancy from comment #242)
> Created attachment 157611 [details]
> dmesg on linux-next without quirk showing bug persisting over several
> suspend cycles
>
> Of the three suspend/resume cycles shown in this dmesg, I trigger the EC bug
> during the first suspend time.
>
> It comes back with SCI_EVT=1 set the first time, but the 2nd and 3rd resumes
> do not have this. In a moment I will confirm if this persists across power
> cycles.
>
> It seems the right behaviour for affected Samsung machines is to send QR_EC
> until data is 0x00 not just if we get SCI_EVT=1, but additionally on boot or
> resume.

Yes.
I was planning to support this in this way.

Thanks

[lv.zheng (lv.zheng-linux-kernel-bugs)]

(In reply to Kieran Clancy from comment #243)
> Created attachment 157631 [details]
> dmesg on linux-next without quirk showing bug persisting over power cycle
>
> Confirming that once the bug is initially triggered, we don't get SCI_EVT=1
> even on a power cycle, so we still need the boot/resume quirk. At least,
> that's my interpretation.
>
> I hope the dmesgs were helpful. Unfortunately I'm not going to have internet
> access for the next week (going to the outback where there is not even any
> mobile reception), but I'm happy to test more things when I return.

Yes, it's very helpful.
Thanks for the help!

Best regards
-Lv

[lv.zheng (lv.zheng-linux-kernel-bugs)]

(In reply to Lv Zheng from comment #244)
> (In reply to Kieran Clancy from comment #241)
> > My analysis is below:
> >
> > After the bug is triggered, SCI_EVT=1 is set just ONE time, immediately
> > after resume:
> >
> > [ 337.319012] ACPI : EC: EC_SC(R) = 0x28 SCI_EVT=1 BURST=0 CMD=1 IBF=0
> OBF=0
> >
> > It does not seem as though we ever handle this event properly though.
> > Namely, there seems to be no corresponding "EC_SC(W) = 0x84". There is a
> > couple of "EC_DATA(W) = 0x84" but I'm pretty sure these are totally
> > different?
>
> This is a bug, in ec_poll(), there is no code to check SCI_EVT.
> And event will thus be lost.
>
> I think this has been fixed in the ec-flush6.patch.
> +out:
> + if (status & ACPI_EC_FLAG_SCI &&
> + (!t || t->flags & ACPI_EC_COMMAND_COMPLETE))
> + __acpi_ec_set_event(ec);
>
> We will have this flag checked in the advance_transaction.
> So you won't see this with ec-flush[1-6].patch applied.

I should take this back. :-)
This still need to be improved to indicate SCI_EVT even when there is a transaction running.

Thanks
-Lv

[lenb (lenb-linux-kernel-bugs)]

commit 74443bbed72ab22ee005ecb6ecdc657a8018e1db
Author: Lv Zheng <email address hidden>
Date: Wed Jan 14 19:28:47 2015 +0800

    ACPI / EC: Fix issues related to the SCI_EVT handling

shipped in Linux-4.0-rc1
closed.

[el (el-linux-kernel-bugs)]

commit 4c237371f290d1ed3b2071dd43554362137b1cce
Author: Lv Zheng <email address hidden>
Date: Wed Jan 4 11:17:17 2017 +0800

    ACPI / EC: Remove old CLEAR_ON_RESUME quirk

The above reintroduced this bug on my NP900x4c. Reverting the commit makes things work again.

[balazs4web (balazs4web-linux-kernel-bugs)]

(In reply to Elvis Pranskevichus from comment #249)
> commit 4c237371f290d1ed3b2071dd43554362137b1cce
> Author: Lv Zheng <email address hidden>
> Date: Wed Jan 4 11:17:17 2017 +0800
>
> ACPI / EC: Remove old CLEAR_ON_RESUME quirk
>
> The above reintroduced this bug on my NP900x4c. Reverting the commit makes
> things work again.

Confirmed, the bug is back again on samsung latops :-(

https://bbs.archlinux.org/viewtopic.php?pid=1767061#p1767061

[mark (mark-linux-kernel-bugs)]

Not exactly surprising when the change that fixed then gets reverted out of the kernel. Seems like Intel trying to force obsolescence of these systems.

[balazs4web (balazs4web-linux-kernel-bugs)]

(In reply to Mark Syms from comment #251)
> Not exactly surprising when the change that fixed then gets reverted out of
> the kernel. Seems like Intel trying to force obsolescence of these systems.

what has to be done to bring the fix back into the kernel code?
I mean at the first glance it was only for `Samsung hardware`, so I guess it does not affect any other vendors/hardwares.
The mentioned commit looks like just a `code cleanup` changes. but I'm not familiar with the kernel code.

Shall I file a new issue or could this be opened again?

[rui.zhang (rui.zhang-linux-kernel-bugs)]

Lv is not working on this now, and I'm new to the EC code, first let's confirm a quick revert patch.

[rui.zhang (rui.zhang-linux-kernel-bugs)]

Created attachment 274121
revert patch

please confirm
1. the problem exists with the latest upstream kernel, say, 4.16-rc1
2. the problem is gone after applying the patch attached

[odi (odi-linux-kernel-bugs)]

I can confirm that the regression exists with 4.15.3 and that your patch fixes it. 4.16 not tested.

[cribari (cribari-linux-kernel-bugs)]

I am experiencing the same problem (Arch Linux, kernel 4.15.3, KDE). Hardware: Samsung laptop model 900X3L. Will the patch be included in the Linux kernel?

[balazs4web (balazs4web-linux-kernel-bugs)]

Created attachment 274447
revert-patch works

confirmed, pacth is working on top of `4.15`

[cribari (cribari-linux-kernel-bugs)]

I am still facing this problem with Arch Linux + kernel 4.16.0-2. Hardware is a Samsung laptop (model: NP900X3L-KW1BR). Suggestions are welcome.

[cribari (cribari-linux-kernel-bugs)]

I compiled kernels 4.16.0 and 4.16.2 (in Arch Linux) using the patch in Comment #254 and I confirm that the patch fixes the problem. (Hardware: Samsung model NP900X3L-KW1BR.)

[yu.c.chen (yu.c.chen-linux-kernel-bugs)]

Hi @Rui,
may I know if the patch #Comment 254 will be pushed upstream ?

[penalvch (penalvch)]

Could you please advise what supported release you have reproduced this in?

[jonas (jonas-linux-kernel-bugs)]

Hi

Poll on status for this bug. Does anyone know if/when it will be fixed upstream?

[cribari (cribari-linux-kernel-bugs)]

Has this been fixed upstream? Despite what we read at

https://www.systutorials.com/linux-kernels/59801/acpi-ec-fix-regression-related-to-triggering-source-of-ec-event-handling-linux-4-14-3/

some people still face the problem. Thank you.

[rui.zhang (rui.zhang-linux-kernel-bugs)]

sorry that I thought I have already sent this patch upstream, but apparently I didn't.

(In reply to Francisco Cribari from comment #263)
> Has this been fixed upstream? Despite what we read at
>
> https://www.systutorials.com/linux-kernels/59801/acpi-ec-fix-regression-
> related-to-triggering-source-of-ec-event-handling-linux-4-14-3/
>
I can not access this page.
But if you can confirm the problem still exists in 4.19-rc1, and the revert indeed fixes it, I will send the revert patch out.

[rui.zhang (rui.zhang-linux-kernel-bugs)]

Created attachment 278797
revert patch on top of 4.19-rc4

please confirm this revert patch works for you on top of 4.19-rc kernel.
I will send it out after got your confirmation.

[odi (odi-linux-kernel-bugs)]

(In reply to Zhang Rui from comment #265)
> please confirm this revert patch works for you on top of 4.19-rc kernel.
> I will send it out after got your confirmation.

It does! Also the CPU max frequency is now back to 2.4GHz after unplug/replug cycle.

VANILLA 4.19-rc4

plugged:
bat ~ # cat /sys/class/power_supply/ADP1/online
1
bat ~ # cat /sys/class/power_supply/BAT1/status
Charging

unplugged:
bat ~ # cat /sys/class/power_supply/ADP1/online
0
bat ~ # cat /sys/class/power_supply/BAT1/status
Charging

PATCHED 4.19-rc4

plugged:
bat ~ # cat /sys/class/power_supply/BAT1/status
Charging

unplugged:
bat ~ # cat /sys/class/power_supply/ADP1/online
0
bat ~ # cat /sys/class/power_supply/BAT1/status
Discharging

replugged:
bat ~ # cat /sys/class/power_supply/ADP1/online
1
bat ~ # cat /sys/class/power_supply/BAT1/status
Charging

[jonas (jonas-linux-kernel-bugs)]

(In reply to Zhang Rui from comment #265)
> Created attachment 278797 [details]
> revert patch on top of 4.19-rc4
>
> please confirm this revert patch works for you on top of 4.19-rc kernel.
> I will send it out after got your confirmation.

Hi Rui,

Any chance this will hit 4.19 rc before final release?