Bug 13803

Summary: "ThinkPad Extra Buttons" device not detected on ThinkPad X61
Product: hal Reporter: Ben Liblit <liblit>
Component: haldAssignee: David Zeuthen (not reading bugmail) <zeuthen>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium Keywords: patch
Version: unspecified   
Hardware: x86 (IA32)   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments: complete output of "hald --verbose=yes --daemon=no --exit-after-probing"
add support for SW_RADIO switches

Description Ben Liblit 2007-12-24 01:42:28 UTC
My ThinkPad X61 has the "thinkpad_acpi" kernel module loaded, which correctly detects a "ThinkPad Extra Buttons" input device and creates the expected structures under /sys/class/input/input<n> for some <n>.  For example, at present:

        % cat /sys/class/input/input6/name

        ThinkPad Extra Buttons

However, hald does not include this input device in its list.  It appears that hald initially finds this device, but rejects it because of an "Unknown property type 29548" error:

        % hald --verbose=yes --daemon=no --exit-after-probing

        03:37:46.947 [I] device.c:3751: device removed due to prober fail
        device udi = /org/freedesktop/Hal/devices/temp/76
          button.state.value = false  (bool)
          linux.device_file = '/dev/input/event6'  (string)
          button.has_state = true  (bool)
          input.product = 'ThinkPad Extra Buttons'  (string)
        03:37:46.947 [W] device.c:1374: Unknown property type 29548
          linux.subsystem = 'input'  (string)
          info.product = 'ThinkPad Extra Buttons'  (string)
          linux.hotplug_type = 2  0x2  (int)
          linux.sysfs_path = '/sys/class/input/input6/event6'  (string)
          info.category = 'input'  (string)
          input.device = '/dev/input/event6'  (string)
          info.parent = '/org/freedesktop/Hal/devices/computer'  (string)

This in turn means that the rules in "30-keymap-module-thinkpad-acpi.fdi" are not applied, and hardware utopia remains just out of reach.  :-(

I'm running Fedora 8 with the following Fedora-provided packages installed:

    kernel-2.6.23.9-85.fc8
    hal-0.5.10-1.fc8
    hal-info-20071030-1.fc8
    hal-libs-0.5.10-1.fc8
Comment 1 Ben Liblit 2007-12-24 01:46:37 UTC
Created attachment 13337 [details]
complete output of "hald --verbose=yes --daemon=no --exit-after-probing"

I'm attaching the complete "hald --verbose=yes --daemon=no --exit-after-probing" output, just in case the fragment I included in my original message omitted anything important.
Comment 2 Ben Liblit 2007-12-24 03:37:00 UTC
I think that the "Unknown property type 29548" message is a red herring.  This occurs when printing details about an input device which has *already* been rejected.  So things have already gone wrong by the time we get there.

From what I can tell using strace, it looks like hald-probe-input is being called for this input device but is exiting with an error because there is no $HAL_PROP_BUTTON_TYPE set in its environment.

Setting the button type might be the responsibility of input_test_switch() in "hald/linux/device.c".  It sets "button.type" based on the "capabilities/sw" bitmask for the device being tested.  (I assume that "button.type" turns into $HAL_PROP_BUTTON_TYPE somewhere else.)

The code in input_test_switch() checks for three possible bits: SW_LID (bit 0), SW_TABLET_MODE (bit 1), and SW_HEADPHONE_INSERT (bit 2).  However, this input device actually has "8" in "capabilities/sw".  That's bit 3, which is not checked by input_test_switch().  Therefore, "button.type" is never set.

If input_test_switch() were checking bit 3, that would be SW_RADIO.  Is that a sensible value?  I'm not sure.  Perhaps it refers to Fn+F5, which toggles Bluetooth.  Or perhaps it refers to the radio kill switch.  Anyway, "capabilities/sw" is definitely 8 and that's definitely not what input_test_switch() was expecting.

So either input_test_switch() needs to check for SW_RADIO or else this device should not have been considered a switch in the first place.  This input device is already granted the "input.keys" capability by input_test_key(), so I'm not sure whether it makes sense for it to be both "input.keys" and "input.switch" at the same time.

Note: if input_test_switch() is changed to check for the SW_RADIO bit, then "hald/linux/probing/probe-input.c" will also need to be changed accordingly.
Comment 3 Ben Liblit 2007-12-28 04:31:03 UTC
Created attachment 13396 [details] [review]
add support for SW_RADIO switches

Further research and experimentation shows that the SW_RADIO aspect of this device does correspond to the hardware RF kill switch.  So the right thing to do is to teach input_test_switch() and other related code how to cope with SW_RADIO and button.type "radio".  The attached patch does exactly that.  It's been tested with Linux kernel version 2.6.23.9-85.fc8 and thinkpad_acpi module version 0.18-20071203.

SW_RADIO was added to the Linux kernel "input.h" header in version 2.6.22, but presumable we want HAL to build against earlier kernel versions too.  The patch uses #ifdefs to check that SW_RADIO exists and omits the new code if it does not.
Comment 4 Danny Kukawka 2008-02-05 13:44:01 UTC
commited to git master.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.