the errors about two EC patches

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Len
    
    The patch named acpi-ec-dont-degrade-to-poll-mode-at-storm-automatically is 
included by 2.6.27-rc5-mm1 tree. In fact this patch can't fix the problem of 
bug 11089 & 10724. 
    >Not all users of semi-broken EC devices want to degrade to poll mode, so
give them right to choose.
    >This fixes a regression in 2.6.26 (from 2.6.25.3).  Initially reported as
"Asus Eee PC hotkeys stop working if pressed quickly" in bugzilla
<http://bugzilla.kernel.org/show_bug.cgi?id=11089>.

    At the same time the content in the patch is totally discarded by the 
following patch in ACPI test git tree:
>commit 50c0f43ff1199bbea5f0418d40df32f4d4029b13
>Author: Andi Kleen <ak@xxxxxxxxxxxxxxx>
>Date:   Thu Sep 4 15:13:29 2008 +0200

    >ACPI: EC: do transaction from interrupt context

   In the above patch there exist the following problems:
   a. In the following code the GPE_STORM bit will always be set regardless 
of whether there exists the EC GPE storm.
    >if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) {
    >            /* check if we received SCI during transaction */
    >           ec_check_sci(ec, acpi_ec_read_status(ec));
    >           /* it is safe to enable GPE outside of transaction */
    >            acpi_enable_gpe(NULL, ec->gpe, ACPI_NOT_ISR);
    >    } else if (test_bit(EC_FLAGS_GPE_MODE, &ec->flags) &&
    >               atomic_read(&ec->irq_count) > ACPI_EC_STORM_THRESHOLD)
    >           pr_debug(PREFIX "GPE storm detected\n");
    >           set_bit(EC_FLAGS_GPE_STORM, &ec->flags);
    b. When EC GPE storm is detected, the EC GPE will be disabled and 
EC will work in polling mode while doing EC transaction. In such case 
maybe EC transaction is not finished. But when timeout happens, it 
will still be regarded as finishing the EC transaction. 
       This error is related with the following source code:
           > {
           >    delay = jiffies + msecs_to_jiffies(ACPI_EC_DELAY); 
 //Maybe the preemptible schedule happens here.If the current jiffies
 is greater than the predefined jiffies after the process is returned 
from preempt_schedule, OS will have no opportunity to do the EC transaction.
 It means that EC transaction is not finished. But it is still regarded as 
finishing EC transaction. It is incorrect.
           >     while (time_before(jiffies, delay)) {
            >            gpe_transaction(ec, acpi_ec_read_status(ec));
            >            msleep(1);
//as msleep is realized by using the function of schedule_timeout, 
maybe the current jiffies is already greater than the predefined jiffies 
before finishing EC transaction. In such case the EC transaction is
not finished. But it is also regarded as finishing. It is incorrect.
            >            if (ec_transaction_done(ec))
            >                    goto end;
            >    }
     c. There is no detailed change log although a lot is changed.(
Including the EC work mode, the detect mechanism of EC GPE storm). 
It is not easy to understand.     

     At the same time this commit can't handle the EC notification
event in the following case:
         EC GPE Interrupt storm is detected and EC GPE will be disabled
when doing EC transaction. If EC notification event happens while doing
EC transaction(EC GPE is disabled), the SCI_EVT bit of EC status
register is cleared before OS issuing query command. In such case the EC
notification event will be lost.

  Based on the above analysis ,IMO the two patches are not appropriate.

Best regards.
    Yakui


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

[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux