Bug 13865

Summary: Can only resume with HP_WMI selected on compaq nc6000 when 4c395bdd3f2ca8f7e8efad881e16071182c3b8ca is reverted
Product: Power Management Reporter: cedric (cedric)
Component: Hibernation/SuspendAssignee: power-management_other
Status: CLOSED CODE_FIX    
Severity: normal CC: elendil, lenb, rjw
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 2.6.30 Subsystem:
Regression: Yes Bisected commit-id:
Bug Depends on:    
Bug Blocks: 7216, 13070    

Description cedric 2009-07-29 12:47:14 UTC
Just for testing I enabled HP_WMI. But from then, I could not resume anymore.
I just reverted the last git commit on the 'drivers/platform/x86/hp-wmi.c' code (4c395bdd3f2ca8f7e8efad881e16071182c3b8ca) and now I can resume.
The problem still persists on .31-rc serie
Comment 1 Rafael J. Wysocki 2009-07-29 20:58:38 UTC
On Wednesday 29 July 2009, you wrote:
> http://bugzilla.kernel.org/show_bug.cgi?id=13865
> 
>            Summary: Can only resume with HP_WMI selected on compaq nc6000
>                     when 4c395bdd3f2ca8f7e8efad881e16071182c3b8ca is
>                     reverted
>            Product: Power Management
>            Version: 2.5
>     Kernel Version: 2.6.30
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: Hibernation/Suspend
>         AssignedTo: power-management_other@kernel-bugs.osdl.org
>         ReportedBy: cedric@belbone.be
>         Regression: No
> 
> 
> Just for testing I enabled HP_WMI. But from then, I could not resume anymore.
> I just reverted the last git commit on the 'drivers/platform/x86/hp-wmi.c'
> code
> (4c395bdd3f2ca8f7e8efad881e16071182c3b8ca) and now I can resume.
> The problem still persists on .31-rc serie

Frans, that's something for you to take care of.

Best,
Rafael
Comment 2 Frans Pop 2009-07-29 22:57:52 UTC
On Wednesday 29 July 2009, Frans Pop wrote:
> The strange thing is that the resume function does not do anything
> that's not also done in hp_wmi_input_setup(). The only thing I can
> think of is that we might need to test for
> wmi_has_guid(HPWMI_EVENT_GUID) because that model simply does not
> support input events.

If that is the problem, the following patch will hopefully fix it.
This patch is against .31-rc4 but will need only minor changes to
apply against .30.

Subject: hp-wmi: check that an input device exists in resume handler

Some systems may not support input events, or registering the input
handler may have failed. So check that an input device exists before
trying to set the docking and tablet mode state during resume.

Fixes: http://bugzilla.kernel.org/show_bug.cgi?id=13865

Reported-by: cedric@belbone.be
Signed-off-by: Frans Pop <elendil@planet.nl>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>

diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index 369bd43..63c9214 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -540,11 +540,13 @@ static int hp_wmi_resume_handler(struct device *device)
 	 * the input layer will only actually pass it on if the state
 	 * changed.
 	 */
-
-	input_report_switch(hp_wmi_input_dev, SW_DOCK, hp_wmi_dock_state());
-	input_report_switch(hp_wmi_input_dev, SW_TABLET_MODE,
-			    hp_wmi_tablet_state());
-	input_sync(hp_wmi_input_dev);
+	if (hp_wmi_input_dev) {
+		input_report_switch(hp_wmi_input_dev, SW_DOCK,
+				    hp_wmi_dock_state());
+		input_report_switch(hp_wmi_input_dev, SW_TABLET_MODE,
+				    hp_wmi_tablet_state());
+		input_sync(hp_wmi_input_dev);
+	}
 
 	if (wifi_rfkill)
 		rfkill_set_states(wifi_rfkill,
Comment 3 cedric 2009-07-30 09:20:29 UTC
Frans Pop wrote:
> On Wednesday 29 July 2009, Rafael J. Wysocki wrote:
>   
>> On Wednesday 29 July 2009, you wrote:
>>     
>>> Just for testing I enabled HP_WMI. But from then, I could not resume
>>> anymore. I just reverted the last git commit on the
>>> 'drivers/platform/x86/hp-wmi.c' code
>>> (4c395bdd3f2ca8f7e8efad881e16071182c3b8ca) and now I can resume. The
>>> problem still persists on .31-rc serie
>>>       
>> Frans, that's something for you to take care of.
>>     
>
> I'll at least give it a try.
>
> Cedric, as a start, can you please provide the output of the following 
> commands after the system is booted and with hp-wmi loaded:
> $ grep . /sys/devices/platform/hp-wmi/*
> $ ls -l /dev/input/*
> $ cat /proc/bus/input/devices
>
> Does an unload and reload of the module work correctly? What gets output 
> in dmesg by that?
>
> The strange thing is that the resume function does not do anything that's 
> not also done in hp_wmi_input_setup(). The only thing I can think of is 
> that we might need to test for wmi_has_guid(HPWMI_EVENT_GUID) because 
> that model simply does not support input events.
>
> Cheers,
> FJP
>   
I tested your patch (from buzilla) and now I can resume.

Thanks ! You have my tested-by ;-)

PS: I suppose you don't need this infos anymore ?
Comment 4 Frans Pop 2009-07-30 09:56:06 UTC
On Thursday 30 July 2009, Cédric Godin wrote:
> > The strange thing is that the resume function does not do anything
> > that's not also done in hp_wmi_input_setup(). The only thing I can
> > think of is that we might need to test for
> > wmi_has_guid(HPWMI_EVENT_GUID) because that model simply does not
> > support input events.
>
> I tested your patch (from buzilla) and now I can resume.

Great.

> Thanks ! You have my tested-by ;-)
>
> PS: I suppose you don't need this infos anymore ?

I'd still like to see the following if it's not too much trouble, 
basically to confirm that the theory was indeed correct.

$ grep . /sys/devices/platform/hp-wmi/*
$ cat /proc/bus/input/devices

Thanks,
FJP
Comment 5 cedric 2009-07-30 10:07:38 UTC
i'm here to test and help find bug, so no trouble at all.

cedric@enea ~ $ grep . /sys/devices/platform/hp-wmi/*
grep: /sys/devices/platform/hp-wmi/als: Invalid argument
grep: /sys/devices/platform/hp-wmi/display: Invalid argument
grep: /sys/devices/platform/hp-wmi/dock: Invalid argument
grep: /sys/devices/platform/hp-wmi/hddtemp: Invalid argument
/sys/devices/platform/hp-wmi/modalias:platform:hp-wmi
grep: /sys/devices/platform/hp-wmi/tablet: Invalid argument
/sys/devices/platform/hp-wmi/uevent:DRIVER=hp-wmi
/sys/devices/platform/hp-wmi/uevent:MODALIAS=platform:hp-wmi


cedric@enea ~ $ cat /proc/bus/input/devices
I: Bus=0019 Vendor=0000 Product=0001 Version=0000
N: Name="Power Button"                           
P: Phys=LNXPWRBN/button/input0                   
S: Sysfs=/devices/LNXSYSTM:00/LNXPWRBN:00/input/input0
U: Uniq=                                              
H: Handlers=kbd event0                                
B: EV=3                                               
B: KEY=100000 0 0 0                                   

I: Bus=0019 Vendor=0000 Product=0003 Version=0000
N: Name="Sleep Button"                           
P: Phys=PNP0C0E/button/input0                    
S: Sysfs=/devices/LNXSYSTM:00/device:00/PNP0C0E:00/input/input1
U: Uniq=                                                       
H: Handlers=kbd event1                                         
B: EV=3                                                        
B: KEY=4000 0 0 0 0                                            

I: Bus=0019 Vendor=0000 Product=0005 Version=0000
N: Name="Lid Switch"                             
P: Phys=PNP0C0D/button/input0                    
S: Sysfs=/devices/LNXSYSTM:00/device:00/PNP0C0D:00/input/input2
U: Uniq=                                                       
H: Handlers=event2                                             
B: EV=21                                                       
B: SW=1                                                        

I: Bus=0011 Vendor=0001 Product=0001 Version=ab41
N: Name="AT Translated Set 2 keyboard"           
P: Phys=isa0060/serio0/input0                    
S: Sysfs=/devices/platform/i8042/serio0/input/input3
U: Uniq=
H: Handlers=kbd event3
B: EV=120013
B: KEY=20000 0 20 0 0 0 0 500f 2100002 3803078 f800d401 feffffdf ffefffff ffffffff ffffffff
B: MSC=10
B: LED=7

I: Bus=0011 Vendor=0002 Product=0007 Version=1bb1
N: Name="SynPS/2 Synaptics TouchPad"
P: Phys=isa0060/serio4/input0
S: Sysfs=/devices/platform/i8042/serio4/input/input4
U: Uniq=
H: Handlers=mouse0 event4
B: EV=b
B: KEY=6420 0 70000 0 0 0 0 0 0 0 0
B: ABS=11000003

I: Bus=0011 Vendor=0002 Product=0001 Version=0000
N: Name="PS/2 Generic Mouse"
P: Phys=synaptics-pt/serio0/input0
S: Sysfs=/devices/platform/i8042/serio4/serio5/input/input5
U: Uniq=
H: Handlers=mouse1 event5
B: EV=7
B: KEY=70000 0 0 0 0 0 0 0 0
B: REL=3

I: Bus=0003 Vendor=046d Product=c016 Version=0110
N: Name="Logitech Optical USB Mouse"
P: Phys=usb-0000:00:1d.0-2/input0
S: Sysfs=/devices/pci0000:00/0000:00:1d.0/usb1/1-2/1-2:1.0/input/input6
U: Uniq=
H: Handlers=mouse2 event6
B: EV=17
B: KEY=70000 0 0 0 0 0 0 0 0
B: REL=103
B: MSC=10

Cedric

PS: I had to apply the patch by hand on .31-rc4. Maybe you have changes in your tree not yet commited ?
Comment 6 Frans Pop 2009-07-30 10:42:11 UTC
On Wednesday 29 July 2009, Rafael J. Wysocki wrote:
> On Wednesday 29 July 2009, you wrote:
> > http://bugzilla.kernel.org/show_bug.cgi?id=13865
> >
> > Just for testing I enabled HP_WMI. But from then, I could not resume
> > anymore. I just reverted the last git commit on the
> > 'drivers/platform/x86/hp-wmi.c' code
> > (4c395bdd3f2ca8f7e8efad881e16071182c3b8ca) and now I can resume. The
> > problem still persists on .31-rc serie

Here's the final patch, tested by Cédric. (And properly against mainline.)

From: Frans Pop <elendil@planet.nl>
Subject: hp-wmi: check that an input device exists in resume handler

Some systems may not support input events, or registering the input
handler may have failed. So check that an input device exists before
trying to set the docking and tablet mode state during resume.

Fixes: http://bugzilla.kernel.org/show_bug.cgi?id=13865

Reported-and-tested-by: Cédric Godin <cedric@belbone.be>
Signed-off-by: Frans Pop <elendil@planet.nl>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>

diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index ca50856..a2ad53e 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -520,11 +520,13 @@ static int hp_wmi_resume_handler(struct platform_device *device)
 	 * the input layer will only actually pass it on if the state
 	 * changed.
 	 */
-
-	input_report_switch(hp_wmi_input_dev, SW_DOCK, hp_wmi_dock_state());
-	input_report_switch(hp_wmi_input_dev, SW_TABLET_MODE,
-			    hp_wmi_tablet_state());
-	input_sync(hp_wmi_input_dev);
+	if (hp_wmi_input_dev) {
+		input_report_switch(hp_wmi_input_dev, SW_DOCK,
+				    hp_wmi_dock_state());
+		input_report_switch(hp_wmi_input_dev, SW_TABLET_MODE,
+				    hp_wmi_tablet_state());
+		input_sync(hp_wmi_input_dev);
+	}
 
 	return 0;
 }
Comment 7 Frans Pop 2009-07-30 10:48:57 UTC
> cedric@enea ~ $ grep . /sys/devices/platform/hp-wmi/*
> grep: /sys/devices/platform/hp-wmi/als: Invalid argument
> grep: /sys/devices/platform/hp-wmi/display: Invalid argument
> grep: /sys/devices/platform/hp-wmi/dock: Invalid argument
> grep: /sys/devices/platform/hp-wmi/hddtemp: Invalid argument
> /sys/devices/platform/hp-wmi/modalias:platform:hp-wmi
> grep: /sys/devices/platform/hp-wmi/tablet: Invalid argument
> /sys/devices/platform/hp-wmi/uevent:DRIVER=hp-wmi
> /sys/devices/platform/hp-wmi/uevent:MODALIAS=platform:hp-wmi

Thanks, that does indeed confirm the theory. In fact, it looks as if the 
driver basically does not support your notebook at all. Is that correct?

However, the patch is IMO still correct as it does make the driver more 
stable.

> PS: I had to apply the patch by hand on .31-rc4. Maybe you have changes
> in your tree not yet commited ?

Yes, sorry about that. The final patch I just sent should be correct.
Comment 8 cedric 2009-07-30 11:31:31 UTC
bugzilla-daemon@bugzilla.kernel.org wrote:
> http://bugzilla.kernel.org/show_bug.cgi?id=13865
>
>
>
>
>
> --- Comment #7 from Frans Pop <elendil@planet.nl>  2009-07-30 10:48:57 ---
>   
>> cedric@enea ~ $ grep . /sys/devices/platform/hp-wmi/*
>> grep: /sys/devices/platform/hp-wmi/als: Invalid argument
>> grep: /sys/devices/platform/hp-wmi/display: Invalid argument
>> grep: /sys/devices/platform/hp-wmi/dock: Invalid argument
>> grep: /sys/devices/platform/hp-wmi/hddtemp: Invalid argument
>> /sys/devices/platform/hp-wmi/modalias:platform:hp-wmi
>> grep: /sys/devices/platform/hp-wmi/tablet: Invalid argument
>> /sys/devices/platform/hp-wmi/uevent:DRIVER=hp-wmi
>> /sys/devices/platform/hp-wmi/uevent:MODALIAS=platform:hp-wmi
>>     
>
> Thanks, that does indeed confirm the theory. In fact, it looks as if the 
> driver basically does not support your notebook at all. Is that correct?
>
>   
In fact, I don't know. I thought it was related to my wifi and bluetooth
button on my laptop so that I can shut the wifi (and the led) down when
I don't want wireless connection but it seems I was confused.
> However, the patch is IMO still correct as it does make the driver more 
> stable.
>
>   
>> PS: I had to apply the patch by hand on .31-rc4. Maybe you have changes
>> in your tree not yet commited ?
>>     
>
> Yes, sorry about that. The final patch I just sent should be correct.
>
>
Comment 9 Frans Pop 2009-07-30 11:49:18 UTC
On Thursday 30 July 2009, you wrote:
> In fact, I don't know. I thought it was related to my wifi and
> bluetooth button on my laptop so that I can shut the wifi (and the led)
> down when I don't want wireless connection but it seems I was confused.

Possibly wifi rfkill is supported, but AFAICT not the button.

I don't know if the driver could be extended to support your laptop: I 
don't know ACPI that well. The person who might be able to help with that 
is Matthew Garrett, the original author of the driver.

There is a patch in the pipeline for .32 that extends wifi rfkill support 
in the driver somewhat, but again I'm not sure if that would be relevant 
for your laptop.

Have you tried the rfkill utility?
http://git.sipsolutions.net/?p=rfkill.git
Comment 10 cedric 2009-07-30 11:51:41 UTC
bugzilla-daemon@bugzilla.kernel.org wrote:
> http://bugzilla.kernel.org/show_bug.cgi?id=13865
>
>
>
>
>
> --- Comment #9 from Frans Pop <elendil@planet.nl>  2009-07-30 11:49:18 ---
> On Thursday 30 July 2009, you wrote:
>   
>> In fact, I don't know. I thought it was related to my wifi and
>> bluetooth button on my laptop so that I can shut the wifi (and the led)
>> down when I don't want wireless connection but it seems I was confused.
>>     
>
> Possibly wifi rfkill is supported, but AFAICT not the button.
>
> I don't know if the driver could be extended to support your laptop: I 
> don't know ACPI that well. The person who might be able to help with that 
> is Matthew Garrett, the original author of the driver.
>
> There is a patch in the pipeline for .32 that extends wifi rfkill support 
> in the driver somewhat, but again I'm not sure if that would be relevant 
> for your laptop.
>
> Have you tried the rfkill utility?
> http://git.sipsolutions.net/?p=rfkill.git
>
>   
not yet, but installing it now !

many thanks
Comment 11 Anonymous Emailer 2009-07-30 12:37:17 UTC
Reply-To: mjg@redhat.com

On Thu, Jul 30, 2009 at 12:42:05PM +0200, Frans Pop wrote:
> From: Frans Pop <elendil@planet.nl>
> Subject: hp-wmi: check that an input device exists in resume handler
> 
> Some systems may not support input events, or registering the input
> handler may have failed. So check that an input device exists before
> trying to set the docking and tablet mode state during resume.
> 
> Fixes: http://bugzilla.kernel.org/show_bug.cgi?id=13865
> 
> Reported-and-tested-by: Cédric Godin <cedric@belbone.be>
> Signed-off-by: Frans Pop <elendil@planet.nl>
> Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Acked-by: Matthew Garrett <mjg@redhat.com>
Comment 12 Anonymous Emailer 2009-07-30 21:17:28 UTC
Reply-To: lenb@kernel.org

> > Reported-and-tested-by: Cédric Godin <cedric@belbone.be>
> > Signed-off-by: Frans Pop <elendil@planet.nl>
> > Cc: Matthew Garrett <mjg59@srcf.ucam.org>

> Acked-by: Matthew Garrett <mjg@redhat.com>

Applied -- and i'll put it on my queue for 2.6.30.stable.

thanks,
Len Brown, Intel Open Source Technology Center
Comment 13 Rafael J. Wysocki 2009-08-03 15:17:01 UTC
Fixed by commit daed953721850381673687c59f3a0df553eb6626 .
Comment 14 Len Brown 2009-08-28 23:23:37 UTC
hp-wmi: check that an input device exists in resume handler
shipped in linux 2.6.31-rc6