Re: [ibm-acpi-devel] [PATCH] ACPI: thinkpad-acpi: add thinkpad keys to input.h
On Sunday 27 May 2007 08:15, Henrique de Moraes Holschuh wrote: > On Sat, 26 May 2007, Dmitry Torokhov wrote: > > On Saturday 26 May 2007 13:31, Henrique de Moraes Holschuh wrote: > > > Add hotkeys available in almost all ThinkPads manufactured in the last > > > five > > > years (more than one million machines given the ammount of batteries > > > recalled) to input.h, and make thinkpad-acpi use those instead of issuing > > > KEY_UNKNOWN. > > > > > > KEY_FN_PAGEDOWN is not ever reported by the ThinkPad firmware due to > > > random > > > bogon-induced stupidity at IBM some years ago, but it is provided because > > > it doesn't make sense to define KEY_FN_PAGEUP and not define > > > KEY_FN_PAGEDOWN at the same time. > > > > > > > I am unconvinced that we need new keycodes. Isn't there a better default > > keycodes for these keys? You mentioned that fn+f5 controls radio on many > > thinkpads, why don't you use KEY_WLAN in your keymap? > > No can do the KEY-WLAN thing, sorry. I *don't* have a way to know, unless I > add a model-specific map table to the kernel, and I have been told by > numerous people to don't even try, unless it is for quirks, etc. > Why not? It thinkpad-acpi is a box-specific driver and you could try to setup proper keymap depending on models. We do that in wistron_btns and it doers not even need alot of memory (keymaps and dmi data is marked __initdata and is discarded). > Really, what are we to do with that input.h scancode map? It *IS* supposed > to be absolute, i.e. one is not supposed to reuse keys in there if the > functionality *or* the generic description is not an exact match. Are there any markings on those keys? > This is > extremely clear, and it makes complete sense from the point of view of > userland: HAL and others can properly assign functions to all scan codes and > it will be always correct. > Are you arguing for KEY_THINKPAD_FN_F1, etc? And it being different from KEY_ACER_FN_F1? I don't think it is a good idea... > But then, it is expensive memory-wise, so it is near the current KEY_MAX, > and people are very, very relutant to add another bit to it. > Not really expensive. Right now keys cost 128 bytes per input device, absolute axis data cost much more. If we really need it we could increase KEY_MAX. > This is definately a ridiculous and aggravating situation, that deserves a > proper fix. If increasing the scancode table cannot be done (why?), it is > time to stop denying reality, and remove everything after KEY_FN (0x1d0), > and instead give us a block of KEY_HOSTSPECIFIC_* scancodes, from 0x1d0 to > at least 0x1ef. > > Given the way that scancode table is being used, it is one way or the other. > Either increase it to KEY_MAX=0x3ff, or do exactly what UNICODE did, give us > a decently sized block of host-specific scancodes, and shunt the problem to > userspace to clean up after. > In this case I better do nothing and leave everything as KEY_RESERVED and let userspace load proper keymap for the device. -- Dmitry - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH] ACPI: thinkpad-acpi: allow hotkey to input event map to be modified
Hi, On Sunday 27 May 2007 08:03, Henrique de Moraes Holschuh wrote: > On Sat, 26 May 2007, Dmitry Torokhov wrote: > > On Saturday 26 May 2007 13:31, Henrique de Moraes Holschuh wrote: > > > Add a sysfs interface to allow userspace to modify the mapping between > > > ThinkPad hotkeys and the keycode input events they generate. > > > > No, please do not do that. We have a standard way to adjust keymap for > > an input device via EVIOCGKEYCODE/EVIOCSKEYCODE ioctls on corresponding > > event device; there is no need to invent another interface. Just define > > getkeycodes() and setkeycode() methods for your input device and be done > > with it. > > Sure, I will change to the IOCTLs. Such stuff is exactly why I sent out a > partway-done "don't merge it yet" patch set: I had a hunch that the code > would need some changes as the documentation on how to use the input device > in-kernel API is worth very little. > > IMO if there is an API that is dedicated to drivers (and not, say, kernel > core), full documentation and keeping it up-to-date should be non-negotiable > requirements for the initial merge in mainline, and any subsequent patches > that touch it. Oh well. I am reading the corgikbd driver now, it looks > sane enough to use as documentation. > Documentation/input/input-programming.txt gives some pointers. > On that topic, am I to send SYNC events between key-press and key-release > events? Yes. The application is allowed to "accumulate" input events until it gets EV_SYN/SYN_REPORT so if you don't send sync in-between some applications may not see a button press. > Just after a key-press+key release event? Or not at all for a > EV_KEY ? And exactly what should go in the hardware port descriptor? I used > module name/input device number relative to the module. Are you talking about "phys"? Then it is supposed to have an unique path to the input device. For devices on BUS_HOST module name + relative input device numbers seems to be the best choice. -- Dmitry - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] nvram polling for hardware mixer
On Mon, 28 May 2007, Richard Hughes wrote:
> I've been hacking to make the nvram hardware buttons on thinkpad exposed
> as a hardware mixer device. This requires the nvram to be polled in
> kernel space, and then examined to see if the volume and mute status has
> changed. I have attached code to export a simple mono tuner, but this
> needs further testing and integration. I've also prepared a preliminary
> patch to add the polling to the thinkpad_acpi driver.
Thanks. As usual, idea accepted, and let's work together to get it into
shape for merging.
> Please review, and make sure I'm on the right lines. I also can't see
> the input patches in the ibm-acpi-2.6 tree, so I've done this against
> linus. This isn't for merging, it's just a preview for comments.
The tree you need is at:
http://repo.or.cz/w/linux-2.6/linux-acpi-2.6/ibm-acpi-2.6.git input-hotkeys
It is being updated as I fix the input-hotkey stuff, so it will get
rewinded and you will need to rebase from time to time. BTW, please read
the patch descriptions, and suggest any changes you might like on how I
should credit your work on them.
The above tree is based on 2.6.20, but I will need one based on 2.6.22-rc3
(input API changed) for merging, so I will forward-port it as soon as it is
nearly done. Still, 2.6.20 is what I run right now, so it is my main
development platform. You *are* welcome to work in any kernel version you
see fit, we can change whatever is needed when merging.
> +#include
Stay away from the freeer if you can help it. Can we use the standard
worker thread interface for this? Using lax timers if possible, so that we
are no-tick kernel friendly? Nothing in thinkpad-acpi (so far) needs strict
timings for polling or firing, not even the fan watchdog.
Although we do need at least 2Hz (and 4Hz seems better) for the nvram
polling. I don't know if the no-tick friendly timers can do that.
The open/close hooks should go into the git tree tonight. When it does,
please use them to enable/disable polling: if nothing is reading us, we
don't poll.
> +/* NVRAM is a simple data structure that is polled, parsed and then compared
> + * to a previous version.
> + * Any differences are exposed as bit differences in the struct. */
> +struct tp_nvram_state_struct {
> + char thinkpad_toggle; /* ThinkPad button */
> + char zoom_toggle; /* zoom toggle */
> + char display_toggle;/* display toggle */
> + char home_toggle; /* Home button */
> + char search_toggle; /* Search button */
> + char mail_toggle; /* Mail button */
> + char thinklight_toggle; /* ThinkLight */
> + char hibernate_toggle; /* hibernation/suspend toggle */
> + char display_state; /* display state */
> + char expand_toggle; /* hv expansion state */
> + char brightness_level; /* brightness level */
> + char brightness_toggle; /* brightness toggle */
> + char volume_level; /* volume level */
> + char volume_toggle; /* volume toggle */
> + char mute_toggle; /* mute toggle */
> +};
Make this a u32 bitfield, and use 0 and 1 for values, please.
> +static void thinkpad_read_nvram (struct tp_nvram_state_struct *state)
> +{
> + char data[6];
> +
> + /* read 6 bytes of nvram */
> + data[0] = nvram_read_byte(0x56);
> + data[1] = nvram_read_byte(0x57);
> + data[2] = nvram_read_byte(0x58);
> + data[3] = nvram_read_byte(0x59);
> + data[4] = nvram_read_byte(0x5e);
> + data[5] = nvram_read_byte(0x60);
All magic constants need an appropriate #define, please.
> + /* process and extract states */
> + state->home_toggle = ( data[0] & 0x01);
> + state->search_toggle = ( data[0] & 0x02) >> 1;
> + state->mail_toggle = ( data[0] & 0x04) >> 2;
> + state->thinkpad_toggle = ( data[1] & 0x08) >> 3;
> + state->zoom_toggle = (~data[1] & 0x20) >> 5;
> + state->display_toggle= ( data[1] & 0x40) >> 6;
> + state->thinklight_toggle = ( data[2] & 0x10) >> 4;
> + state->expand_toggle = ( data[3] & 0x10) >> 4;
> + state->brightness_level = ( data[4] & 0x07);
> + state->brightness_toggle = ( data[4] & 0x20) >> 5;
> + state->volume_level = ( data[5] & 0x0f);
> + state->volume_toggle = ( data[5] & 0x80) >> 7;
> + state->mute_toggle = ( data[5] & 0x40) >> 6;
More magic constants...
> + /* find new button presses that are not acpi events */
> + if (new_nvram_state.thinkpad_toggle !=
> last_nvram_state.thinkpad_toggle) {
> + emit_keycode (KEY_COMPOSE); // thinkpad key
We need to get this through the input driver keycode map, see above git
tree...
> /*
> * Thinkpad soundcard mixer interface
> * Copyright (c) by Richard Hughes <[EMAIL PROTECTED]>
I won't go into this one right now (I also have something like it around,
but EC based), and I'd rather go over both codes later and merge a mix with
the best ideas from both.
But my bandwid
[ibm-acpi-devel] nvram polling for hardware mixer
Hi,
I've been hacking to make the nvram hardware buttons on thinkpad exposed
as a hardware mixer device. This requires the nvram to be polled in
kernel space, and then examined to see if the volume and mute status has
changed. I have attached code to export a simple mono tuner, but this
needs further testing and integration. I've also prepared a preliminary
patch to add the polling to the thinkpad_acpi driver.
Please review, and make sure I'm on the right lines. I also can't see
the input patches in the ibm-acpi-2.6 tree, so I've done this against
linus. This isn't for merging, it's just a preview for comments.
Thanks.
Richard.
--- origin/thinkpad_acpi.h 2007-05-28 22:59:03.0 +0100
+++ thinkpad_acpi.h 2007-05-28 23:26:09.0 +0100
@@ -39,6 +39,9 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include
@@ -169,6 +172,7 @@ static int experimental;
static u32 dbg_level;
static int force_load;
static char *ibm_thinkpad_ec_found;
+static int nvram_hz;
static char* check_dmi_for_ec(void);
static int thinkpad_acpi_module_init(void);
@@ -234,6 +238,27 @@ static struct {
u16 fan_ctrl_status_undef:1;
} tp_features;
+/* NVRAM is a simple data structure that is polled, parsed and then compared
+ * to a previous version.
+ * Any differences are exposed as bit differences in the struct. */
+struct tp_nvram_state_struct {
+ char thinkpad_toggle; /* ThinkPad button */
+ char zoom_toggle; /* zoom toggle */
+ char display_toggle;/* display toggle */
+ char home_toggle; /* Home button */
+ char search_toggle; /* Search button */
+ char mail_toggle; /* Mail button */
+ char thinklight_toggle; /* ThinkLight */
+ char hibernate_toggle; /* hibernation/suspend toggle */
+ char display_state; /* display state */
+ char expand_toggle; /* hv expansion state */
+ char brightness_level; /* brightness level */
+ char brightness_toggle; /* brightness toggle */
+ char volume_level; /* volume level */
+ char volume_toggle; /* volume toggle */
+ char mute_toggle; /* mute toggle */
+};
+
static struct list_head tpacpi_all_drivers;
static struct ibm_init_struct ibms_init[];
--- origin/thinkpad_acpi.c 2007-05-28 22:59:03.0 +0100
+++ thinkpad_acpi.c 2007-05-28 23:42:19.0 +0100
@@ -92,6 +92,11 @@ MODULE_LICENSE("GPL");
/* Please remove this in year 2009 */
MODULE_ALIAS("ibm_acpi");
+static struct semaphore thread_sem;
+static int thread_should_die;
+static int using_nvram_thread;
+static struct tp_nvram_state_struct last_nvram_state;
+
#define __unused __attribute__ ((unused))
/
@@ -3871,6 +3876,131 @@ static struct ibm_struct fan_driver_data
.exit = fan_exit,
};
+/*
+ * NVRAM polling
+ */
+
+static void thinkpad_read_nvram (struct tp_nvram_state_struct *state)
+{
+ char data[6];
+
+ /* read 6 bytes of nvram */
+ data[0] = nvram_read_byte(0x56);
+ data[1] = nvram_read_byte(0x57);
+ data[2] = nvram_read_byte(0x58);
+ data[3] = nvram_read_byte(0x59);
+ data[4] = nvram_read_byte(0x5e);
+ data[5] = nvram_read_byte(0x60);
+
+ /* clear existing */
+ memset (state, 0, sizeof (struct tp_nvram_state_struct));
+
+ /* process and extract states */
+ state->home_toggle = ( data[0] & 0x01);
+ state->search_toggle = ( data[0] & 0x02) >> 1;
+ state->mail_toggle = ( data[0] & 0x04) >> 2;
+ state->thinkpad_toggle = ( data[1] & 0x08) >> 3;
+ state->zoom_toggle = (~data[1] & 0x20) >> 5;
+ state->display_toggle= ( data[1] & 0x40) >> 6;
+ state->thinklight_toggle = ( data[2] & 0x10) >> 4;
+ state->expand_toggle = ( data[3] & 0x10) >> 4;
+ state->brightness_level = ( data[4] & 0x07);
+ state->brightness_toggle = ( data[4] & 0x20) >> 5;
+ state->volume_level = ( data[5] & 0x0f);
+ state->volume_toggle = ( data[5] & 0x80) >> 7;
+ state->mute_toggle = ( data[5] & 0x40) >> 6;
+}
+
+static void set_mute (int enable)
+{
+ if (enable)
+ printk(IBM_INFO "Mute\n");
+ else
+ printk(IBM_INFO "Unmute\n");
+}
+
+static void set_brightness_level (int level)
+{
+ printk(IBM_INFO "Brightness level %i\n", level);
+}
+
+static void set_volume_level (int level)
+{
+ printk(IBM_INFO "Volume level %i\n", level);
+}
+
+static void emit_keycode (int keycode)
+{
+ printk(IBM_INFO "Keycode %i\n", keycode);
+}
+
+static void thinkpad_thread_poll_nvram(void)
+{
+ struct tp_nvram_state_struct new_nvram_state;
+
+ thinkpad_read_nvram (&new_nvram_state);
+
+ /* are they the same? */
+ if (memcmp (&new_nvram_state, &last_nvram_state, sizeof (struct tp_nvram_state_struct)) == 0)
+ return;
+
+ /* brightness has changed */
+ if (new_nvram_state.brightness_level != last_nvram_state.brightness_level)
+ set_brightness_level (new_nvram_state.brightness_level);
+
+ /* volume has changed */
+ if (new_nvram_state.volume_level != last_nvram_state.volume_level)
