Re: The fatal logic error in EC write transaction(EC transaction is done in interrupt context)

[Alexey Starikovskiy <astarikovskiy@suse.de>]

Hi Yakui,

Just don't know how I would have lived without your valuable analisys...
Do you remember that ec_transaction starts by waiting for IBF to become 0?
Thus not any new transaction should begin before previous write or burst-disable command completes.
So, fatal logic error exists only in your head.

Now, let me explain why it is better to wait for last IBF=0 event at beginning of transaction, rather than at the end of previous one.
Look at the flow chart from different angle -- as fast transaction patch does it:
EC sends interrupt to OS then it is ready for us to proceed. It will send us interrupt marking IBF=0 transition if it expects us to write byte and it will send us interrupt marking OBF=1 transition if it expects us to read byte. It does not ever send us IBF=1 or OBF=0 interrupts just because there is nothing we could of should do in such a case.
After we completed write sequence, we don't need to wait for last IBF=0 transition, as we don't know if we will have anything else to write -- it might be last transaction for quite a while. But next transaction knows that it should not start before IBF is cleared, thus it waits 
for it (and expects possible interrupt).

Regards,
Alex.
 
Zhao, Yakui wrote:
> Hi, Alexey
>      After checking the EC working flowchart I find a fatal logic error that exists in EC write transaction. The issue happens on most laptops. The following is the detailed explanation about this issue:
>      According to the ACPI the EC write transaction is divided into the following three phases: (Seen in the section 12.6.2 of ACPI 3.0b)
>      a. Byte #1: Host Writes the EC command to EC CMD I/O PORT. Interrupt is triggered after EC reads the command byte 
>      b. Byte #2: Host writes the address index to EC data I/O port. Interrupt is triggered after EC reads the address index byte
>      c. Byte #3: Host writes the Data to EC data I/O port. Interrupt is triggered after EC reads the data byte
>    
>    When the function of acpi_ec_transaction_unlocked is called for EC write transaction, the wlen variable is initialized as 2 and the rlen variable is initialized as zero. Then EC transaction follows the below three steps.
>      1. OS writes the EC command to EC CMD I/O port
>      2. EC GPE interrupt is triggered and then the address index is written into EC data I/O port. (The wlen is decreased to 1.This is executed in EC GPE handler)
>      3. EC GPE interrupt is triggered again and then the data is written into the EC data I/O port. The wlen is decreased to 0.(This is also executed in EC GPE handler). As the wlen is decreased to zero, it means that the EC transaction is finished(True is returned by the function of ec_transaction_done). 
>       >status = acpi_ec_read_status(ec);
>       >gpe_transaction(ec, status);
>       > if (ec_transaction_done(ec) && (status & ACPI_EC_FLAG_IBF) == 0)
>                 wake_up(&ec->wait); 
>      In this step the status variable only indicates that EC controller already reads the address index written by OS(It can’t indicate that EC controller already reads the data byte written by Host). As the IBF flag is zero, the waiting process is waked up and EC mutex lock is released. In such case it means that OS can begin another EC transaction. 
>      But in fact in such case EC transaction is not really finished. After the EC data is written into EC Data I/O port, no flag indicates whether EC transaction is finished. Only when the IBF bit becomes zero again, we can say that EC transaction is really finished. If OS begins another EC transaction before previous EC transaction is really finished, we can't imagine what will happen.
> 　　IMO Based on the above analysis there exists the fatal logic error in the EC write transaction after the patch from Alexey is shipped. Although now no one reports this issue, it is still an unstable factor. After all it is the fatal logic error. 
> 　　 
> 　　 At the same time there exists the similar logic error when OS begins an EC transaction to disable the EC burst mode.
>      
> 　　Welcome the comments.
>      
> 　　Best regards
> 　　     Yakui
> 
>     
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html