Comment 233 for bug 1283589

Hi, Kieran and Ortwin

The 2nd commit need to be reverted means Samsung firmware behaves like:
[ +event 1 ] SCI_EVT set
[ +event 2 ] SCI_EVT set
                           QR_EC
                           event returned
[ -event 1 ] SCI_EVT clear
Then without commit 2, Samsung can work:
[ +event 1 ] SCI_EVT set
[ +event 2 ] SCI_EVT set
                           QR_EC
                           can prepare next QR_EC because SCI_EVT is set
                           event returned
[ -event 1 ] SCI_EVT clear
                           QR_EC
                           event returned
[ -event 2 ] SCI_EVT clear
And with commit 2, Samsung cannot work:
[ +event 1 ] SCI_EVT set
[ +event 2 ] SCI_EVT set
                           QR_EC
                           event returned
[ -event 1 ] SCI_EVT clear
                           cannot prepare next QR_EC because SCI_EVT is cleared
The Samsung specific behavior conflicts with Acer one:
[ +event 1 ] SCI_EVT set
[ +event 2 ]
                           QR_EC
                           event returned
[ -event 1 ] SCI_EVT clear
[ +event 2 ] SCI_EVT set
Without commit 2, Acer cannot work when there is only 1 event:
[ +event 1 ] SCI_EVT set
                           QR_EC
                           can prepared next QR_EC because SCI_EVT is set
                           event returned
[ -event 1 ] SCI_EVT clear
                           QR_EC (but SCI_EVT is cleared!!!)
And with commit 2, Acer can work:
[ +event 1 ] SCI_EVT set
[ +event 2 ]
                           QR_EC
                           event returned
[ -event 1 ] SCI_EVT set
                           can prepare next QR_EC because SCI_EVT is cleared

For Samsung behavior, reverting commit 2 will still have chances to have events lost:
[ +event 1 ] SCI_EVT set
[ +event 2 ] SCI_EVT set
                           QR_EC
---
                           event returned
[ -event 1 ] SCI_EVT clear
                           can prepare next QR_EC because SCI_EVT is set
---
[ -event 2 ] SCI_EVT clear
The --- marked period is racy, after sending the QR_EC command, firmware can behave faster than driver, and we still cannot prepare another QR_EC when this happens.
So I believe this is a Samsung EC firmware specific bug. And the better solution for Samsung is to implement timely event polling on top of the polling thread commit so that when this race happens the lost events can still be found back.

But the polling thread is a new feature, and we need to be more careful with new features, it shouldn't be merged as a regression fix. So I'll propose them to the upstream after proposing a regression fix and seeing more test reports here.

As a regression fix, I'll propose 2 simpler patches:
The first patch will revert the 2nd commit - 558e4736f2e1b0e6323adf7a5e4df77ed6cfc1a4, since it is not required by Acer as Acer platform can be fixed the 1st commit.
The second patch will add a Acer specific quirk flag to the 1st commit - 3afcf2ece453e1a8c2c6de19cdf06da3772a1b08.

Thanks