Bug 1920

OK, My reading comprehension was a bit off last night. I thought that this was 
the behavior defined by the spec, but it's not. This contradicts the spec. So, 
it would be great for me if this were the default behavior, but if it would 
break other systems, then we wouldn't want that. I'll see if I can modify the 
summary to reflect the questionable nature of this change.

If the FF button is supposed to take precedence, though, is it supposed to 
generate ACPI events?

Thanks,
Greg

Created attachment 1944 [details]
Patch for boot parameter to ignore FF buttons

Hi,

I'm not sure whether it would be proper to change this behavior. So, at the
risk of being presumptuous, I've decided to give a patch a try.

I have attached a patch that allows you to pass a boot parameter to tell the
kernel to ignore the FF buttons. The parameter is called ignore_ff_buttons.
Valid values are "PWRF", "SLPF" or "BOTH". They basically do what you would
expect.

For example, in grub.conf, you would add one of the following to the kernel
line:

ignore_ff_buttons=PWRF	 (ignore only the FF power button)
ignore_ff_buttons=SLPF	 (ignore only the FF sleep button)
ignore_ff_buttons=BOTH	 (ignore both the FF power and sleep buttons)

I've never done this before, so if anyone decides to use the patch, please do
so with care. I have tried it on my system with all three settings, and
unspecified, and everything seems to work. I don't have an FF sleep button,
though, so I can't verify that it really does ignore that successfully.

Also, if anyone would like to tell me if something about this is horribly
wrong, I'd appreciate that, too.

Thanks,
Greg

Does the box have a single physical power button and a single physical sleep button? 
It seems odd that the system defines both FF and CM access to the same power button. 
 
Please attach the output from acpidmp, available in /usr/sbin/, or in pmtools: 
http://ftp.kernel.org/pub/linux/kernel/people/lenb/acpi/utils/ 
plus the output of dmesg -s40000 and /proc/interrupts for the unpatched kernel. 
 
re: patch in comment #2 -- boot flags are typically string-compared at boot-time, setting an 
integer flag for run-time comparison -- rather than copying the actual string for run-time 
comparison.  see examples throughout the kernel. 
 
thanks, 
-Len 
 

Thanks, Len.

The machine has a single physical power button and uses a function key for the
sleep button (Fn+F2 as I recall, but I don't have it with me here at work, so
I'm not 100% sure of the exact key).

I'll attach the output that you requested tonight when I get home. I do have a
couple of questions about that, since I am using three patches on this kernel:

1. initrd DSDT override patch
2. Embedded Control patch from bug 1690
3. power button patch above

Should I use my overridden DSDT for this purpose, or would you rather the
original DSDT? My DSDT had an error in the Embedded Control's ECR region method;
it is addressed at bug 1744. Also, should I use a completely clean kernel for
the output, or should I just remove the power button patch? I assume the latter,
since we're just looking into the power button here.

Thanks for the guidance on the patch. I'll modify it appropriately.

Thanks,
Greg

Created attachment 2025 [details]
Output of acpidmp

Created attachment 2026 [details]
Outout od dmesg -s40000

Created attachment 2027 [details]
/proc/interrupts

For acpidmp, dmesg and /proc/interrupts, I used an unpatched vanilla 2.6.2
kernel. The errors in dmesg go away if I use my fake DSDT and the Embedded
Control patch from bug 1690.

Thanks,
 Greg

upcoming GPE re-write should address this

GPE update completed in 20040514

While the FADT.PWR_BUTTON definition in the ACPI spec
seems to say that 1 means CM, 0 means FF -- the definitions
of PM1 Status SLPBTN/PWRBTN_EN/STS say that if the CM device
is defined, the OS should ignore the FF bits; no matter
what the setting of FADT.SLP/PWR_BUTTON.

So perhaps we can fix this w/o any manual workarounds...

Does recent kernel work for you? We now always enable button GPE.

Created attachment 4123 [details]
Patch to choose CM button over FF if both are found.

Could you please try this patch?

This patch makes my button in Toshiba laptop failed. I can't receive any 
button interrupts more.
Here is the definition of the power button in my laptop:
Device(PBTN)
{
    Name(_HID, EidsID("PNP0C0C"))
}
No other methods here, and no 'notify(PBTN, 0x80)' in the dmesg. That means 
PBTN can't act as a power button. 
From the dmesg, I got:
>ACPI:Power Button (FF) [PWRF]
>ACPI:Power Button (CM) [PBTN]
But the /proc/acpi/ is correct, only has PBTN. But anyway, the dmesg will 
mislead users.
Is this the BIOS error or did you test the patch in other systems?
If not, I suggest use a module paramter to override FF button with CM button 
and doesn't break other systems.

Created attachment 4268 [details]
Patch to allow all buttons to be used

David, could you try this patch?

Toshiba seems to be the strange case here. 
On my satellite pro: 
 
FADT.Flags.POWER_BUTTON = 0 -- suggesting FF, 
but the existence of a PNP0C0C device in the DSDT sugges CM: 
 
        Device (PWRB) 
        { 
            Name (_HID, EisaId ("PNP0C0C")) 
            Name (_PRW, Package (0x02) 
            { 
                0x08, 
                0x04 
            }) 
            Name (_STA, 0x0B) 
        } 
 
The GPE handler for 0x08 is in the SSDT: 
    Scope (\_GPE) 
    { 
        Method (_L08, 0, Serialized) 
        { 
         <many other devices sharing this GPE> 
        If (\_SB.MEM.GP91) 
        { 
            Store (0x00, \_SB.MEM.GP91) 
            If (LEqual (Arg0, 0x04)) 
            { 
                Notify (\_SB.PWRB, 0x02) 
            } 
        } 
 
So it appears that the CM capability is only used for the system wake notification (0x2), 
it is not used for the system power-down button-pressed notification (0x80). 
This is why trying to use this CM button for poweroff fails. 
 
Looks like Shaohua's Toshiba has an even more sparse definition of PNP0C0C, 
and without any GPE, it is a mystery to me what that device is supposed to do. 
Shaohua, can you attach the acpidmp for your toshiba? 

For the record, Greg's system has 
FADT.Flags:            0x00000225 
PWR_BUTTON is bit 4, so it is 0 here, suggesting FF. 
But PNP0C0C is defined in the DSDT like so: 
 
        Device (PWRB) 
        { 
            Name (_HID, EisaId ("PNP0C0C")) 
            Name (_PRW, Package (0x02) 
            { 
                0x1D, 
                0x04 
            }) 
        } 
 
There is a GPE notify handler for wakup on 1D: 
    Scope (\_GPE) 
    { 
... 
        Method (_L1D, 0, NotSerialized) 
        { 
            Store (\_SB.PHS1 (0x8E, 0x00), Local0) 
            If (LEqual (Local0, 0x01)) 
            { 
                Notify (\_SB.PWRB, 0x02) 
            } 
 
            If (LEqual (Local0, 0x02)) 
            { 
                Notify (\_SB.PWRB, 0x02) 
            } 
        } 
 
And there is also a button-pressed notify method defined in the embedded controller device: 
 
                    Method (_Q50, 0, NotSerialized) 
                    { 
                        Notify (\_SB.PWRB, 0x80) 
                    } 
So it looks like SMBUS device 50 kicks this one off. 
 

Created attachment 4285 [details]
acpidmp from my toshiba laptop

This is the acpidmp output from my toshiba laptop.

no response from submitter since 2/2004 -- 
we don't have a failing system to test fixes on.

closing as UNREPRODUCIBLE -- re-open if you find
a system that fails.

Hi,

Just to let you know that problem is still present, and Greg's original solution
still working (which means power button does nothing with stock kernel, and
triggers shutdown when acpi_bus_scan_fixed() is modified).

Hardware:  Gateway 200x
Bios:  newer version than Greg's, could find exact versions if relevant
Kernel:
- 2.6.11.2
- custom DSDT (home-made, not Greg's)
- patch for synaptics wheel
- (optional) Greg's "three lines commented out" power button patch

Just drop me a line if I can help (more info, try patches, ...)

Jean-Marc Ranger

It will help a lot if you try patch http://bugme.osdl.org/attachment.cgi?id=4268

Output of dmesg|grep Button on stock 2.6.11 kernel:
   ACPI: Power Button (FF) [PWRF]
   ACPI: Sleep Button (CM) [SLPB]
Power button doing nothing.

Output of dmesg|grep Button on 2.6.11 + patch 4268
   ACPI: Power Button (FF) [PWRF]
   ACPI: Sleep Button (CM) [SLPB]
   ACPI: Power Button (CM) [PWRB]
Power button triggers shutdown.

Looks perfect to me.  Do you wish any specific log ?

JMR

woops...

Add "custom DSDT" to both kernel descriptions in previous post.

JMR

Thanks for verification of the patch

applying the patch makes the additional CM button appear on my Toshiba Satellite Pro, 
and it doesn't hurt the (already functioning) button. 
 
However, this patch fails the "rmmod button" test: 
 
Badness in remove_proc_entry at fs/proc/generic.c:688 
 [<c0187fdb>] remove_proc_entry+0x11b/0x130 
 [<e081e8a8>] acpi_button_exit+0x0/0xa7 [button] 
 [<e081e93f>] acpi_button_exit+0x97/0xa7 [button] 
 [<c013459e>] sys_delete_module+0x16e/0x180 
 [<c014d753>] do_munmap+0x113/0x170 
 [<c014d7e4>] sys_munmap+0x34/0x50 
 [<c0102d99>] sysenter_past_esp+0x52/0x79 
 
--- 
Looking over the driver, I'm leaning towards deleting the bloated /proc code 
instead of fixing it.  The existence of the buttons is given in dmesg, 
and the LID state is the only real run-time object -- and that is 
already handled by events...  We'll get it back when we add 
the acpi devices to sysfs. 

Created attachment 4756 [details]
expanded patch v2.6.10

Took Alexey's patch and then
deleted /proc/acpi/button and all the code supporting for it (40% of the driver
source).

shipped in 2.6.13-rc3 - closing
(though apparently we'll need to restore the /proc stuff...:-(