Re: [RFC] [Patch 0/4] ACPI : several patches for EC

[Alexey Starikovskiy <aystarik@gmail.com>]

Zhao Yakui wrote:
> On Thu, 2008-09-25 at 15:05 +0400, Alexey Starikovskiy wrote:
>   
>> Zhao Yakui wrote:
>>     
>>> But I think that the spin_lock is overkill in the updated patch.
>>> Assuming that 1000 EC transactions are done per second, the CPU
>>> interrupt is disabled for 1ms. It is important that the normal laptops
>>> will be affected by this.
>>>   
>>>       
>> How do you arrive with these numbers? Where do you get this 1ms?
>> Spinlock is around single inb/outb instruction plus several even simpler
>> instructions. Do you claim it is going to take 1us? Do you claim that it 
>> will
>>     
> It is noted that inb/outb instruction is not memory read/write
> operation. It will take some time to read/write the external peripheral
> device.
> For example: 
> On intel platform : The EC is connected with ICH device through LPC bus.
> For every LCP I/O read/write operation, it will take almost 0.7us.
>
> When OS reads the SBS battery info, there will a lot of SCI interrupts,
> most of which are related with EC transaction. 
>   
You seem to forget, that SBS driver is also written by me... Don't
explain to me how it works. Thanks.
>> add anything to interrupts-disabled time of ACPI SCI interrupt handler
>> itself?
>>     
> Not included.
>
> Why not explain the following ? The local function variable resides on
> the process stack space. If the process that is doing EC transaction is
> killed before it is finished, what will happen? Maybe the system will be
> kernel panic.
Ever tried to kill process, which entered syscall (e.g. in kernel)?
Please read some books first.
> I only want to know why you raise the following commit?
>     >commit 9e197219605513c14d3eae41039ecf1b82d1920d
>     >Author: Alexey Starikovskiy <alexey.y.starikovskiy@intel.com>
>     > Date:   Wed Mar 7 18:29:35 2007 -0500
>       >ACPI: ec: fix race in status register access
>    
>    If your understanding is correct, why push it?
>    Why give two different explanations about the same issue?
> At the same time please confirm whether the laptop of bug 8110 is broken
> again by your patch if the GPE storm happens ?
>   
I just explained everything in the above mail. If you don't understand --
this is not my problem, but _yours_. I am not going to explain same things
to you several times.

My patch does not break machine in bug report 8110.