Re: [PATCH] ACPI: EC: do transaction from interrupt context

Alan Jenkins
2008-09-26T09:03:49Z

Alexey Starikovskiy wrote:
> Sitsofe Wheeler wrote:
>> Zhao Yakui wrote:
>>> I think that the problem on the asus-EEEPC can't be resolved really
>>> by the Alexey's patch. IMO it is only lucky.
>>
>> How lucky? It really does go from excruciatingly slow to quite fast
>> on that laptop. What will happen should my luck fail? What are the
>> odds of my luck failing?
>>
>>> In fact the main problem on Asus-EEEPC is related with the broken
>>> EC.
>>> Before an EC notification event is processed, another EC notification
>>> event arrives again.
>>> If EC driver check whether the SCI_EVT bit is set after processing
>>> one EC notification event, the problem will be resolved.
>>> http://bugzilla.kernel.org/show_bug.cgi?id=11089
>>> Alan Jenkins already sent a patch about how to fix the issue on the
>>> Asus-EEEPC.

I submitted alternative patches in the past, but they were not up to
scratch. The first patch stopped the EeePC dropping events, but left
them arriving in bursts every 0.5 seconds - which is still a UI
regression; it looks even more weird when you hold down a brightness
key. My other patches fixed the EeePC, but caused severe regressions on
other hardware.

So I have stopped pushing any patches of my own. In the interest of
removing this regression as soon as reasonably possible, I will not
object to changes that actually work.

>> I'll cc Alan and see what he makes of this patch.
>>

> Alan already tested my patch and it works for him.

Yup. I agree with Zhao, in that it is not crystal clear why this new
patch helps.

The new patch includes a fallback which requires polling-driven
transactions. That's a different type of polling fallback to the
previous one. However, from the experience of testing a range of
patches, I believe the new polling fallback would also drop events. At
face value, the trigger for the polling fallback is the same - 20
"false" (surplus to requirements) interrupts, so it shouldn't work :-).

I can think of two possibilities

1. Doing more in the interrupt handler leaves the interrupt disabled for
longer, hiding some of the false interrupts (which arrive in bursts).

2. Maybe when the device is serviced (e.g. a data byte is read), it
stops the current burst of false interrupts.

>>> But if the above patch is merged , maybe it will break some laptops.
>>
>> Fair enough but can those people with such laptops test the patch too
>> and report back? Can you say in what form the breakages will take?
>> From what you are saying it sounds like this patch shouldn't have had
>> any affect for me but it does. What happens when they try it? What
>> happens when you try it?
>>
>

Alan