Comment 258 for bug 1247723

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=0 OBF=1
[ 42.656424] ACPI : EC: EC_DATA(R) = 0x5e
[ 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
I can see this happens in test A after:
[ 42.656442] ACPI: Waking up from system sleep state S3
Perhaps one of the resuming steps in acpi_pm_finish() (most likely in acpi_hw_legacy_wake()) has cleared the SCI_EVT.
If it is caused by register accesses occurred in acpi_hw_legacy_wake(), this is an ACPICA bug.
If it happens in _WAK evaluation, then this is most likely a firmware bug. Samsung guys need to take care of such firmware bugs.

IMO it is not worthy to track down to the root cause there, given the fact that it would require very complex debugging steps to root cause it and I cannot do it myself. So I'm going to mark this issue as resolved.

Thanks and best regards