Re: [PATCH]: ACPI Cleanup :Initialize EC global lock based on the return status

[Alexey Starikovskiy <aystarik@gmail.com>]

Zhao Yakui wrote:
> On Tue, 2008-11-04 at 17:38 +0800, Alexey Starikovskiy wrote:
>   
>> Zhao Yakui wrote:
>>     
>>> On Tue, 2008-11-04 at 16:05 +0800, Alexey Starikovskiy wrote:
>>>       
>>>> NAK
>>>>         
>>> Will you please describe the detailed reason?
>>>
>>> In the bug 11917 the regression is related with the following commit:
>>>     >commit 27663c5855b10af9ec67bc7dfba001426ba21222
>>>     >Author: Matthew Wilcox <willy@linux.intel.com>
>>>     >Date:   Fri Oct 10 02:22:59 2008 -0400
>>>     >ACPI: Change acpi_evaluate_integer to support 64-bit on 32-bit
>>> kernels
>>>
>>>     But IMO the main reason is that EC driver misuses the Linux-ACPI
>>> utility interface.(acpi_evaluate_integer).  
>>>       
>> Did you _read_ the interface specification?
>> It explicitly states that if any function call does not succeed it will not
>> change the data passed to it.
>> So it is again only your not-so-humble opinion.
>>     
> What do you mean "not-so-humble"?
Did you ever noticed the difference between IMO and IMHO?
>       
>   
>>>     It will be better to determine whether the return value of ACPI
>>> object is effective according to the return status. In such case the
>>> code still can work well even after the Linux-ACPI utility interface is
>>> changed again.
>>>       
>> Code works fine until someone tries to optimize it...
>>     
>      I agree that your patch can work well. But it depends on the
> internal realization of Linux-ACPI utility interface. If Linux-ACPI
> utility interface is changed, maybe it will be broken. If so, why not to
> determine whether the return value is effective based on the return
> status of Linux-ACPI utility?
>   
Once again, my code is based not on internal realization, but on the 
ACPI CA spec.
ACPI code is already littered with un-needed checks, transitions, etc;
I see no reason to spread it to whole Linux.
>       
>   
>> Regards,
>> Alex.
>>
>>