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

[Alexey Starikovskiy <astarikovskiy@suse.de>]

Alan,

Could you please uncomment DEBUG and send dmesg after application of the patch.
I put the "storm detected" message under debug, so it is not visible in normal situation.
I think, that interrupt storm is still detected on your machine, and workaround works.

Regards,
Alex.

Alan Jenkins wrote:
> 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