Re: [PATCH] Input: document the proper usage of EV_KEY and KEY_UNKNOWN

2007-05-31 Thread Dmitry Torokhov
On Friday 01 June 2007 00:08, Matthew Garrett wrote:
> On Thu, May 31, 2007 at 11:33:10PM -0400, Dmitry Torokhov wrote:
> > On Thursday 31 May 2007 21:44, Matthew Garrett wrote:
> > > It's not trivial at all. You need to introduce a mechanism for noting a 
> > > KEY_UNKNOWN keypress. It then needs to signal the user (dbus is probably 
> > > the best layer for this), but you need to ensure that you only signal 
> > > the user who is currently at the keyboard. This needs to be presented to 
> > > the user via some sort of UI, which will then need to signal some sort 
> > > of privileged process to actually change the keymap.
> > 
> > Not necessarily priveleged - you most likely already change ownership
> > of event devices to user who is logged at console (so your force feedback
> > joysticks work).
> 
> If you let users alter the kernel keymap, then you need to implement 
> support for resetting the kernel keymap on exit. Otherwise it's a 
> trivial DoS.
> 

You already do - do you let your users play games with force-feedback
joysticks? To load force feedback effect you need write permissions for
corresponding event device.

And we are talking about console owner here - with current desktop-oriented
distributions they already get access to host of otherwise restricted
devices.

> > > When the user logs  
> > > out, you'll then need to unmap the key again and repeat as necessary for 
> > > any new user who logs in.
> > 
> > I think we should aim at the most common case - when there are no multiple
> > users on the box. Then the utility that detects KEY_UNKNOWN just saves the
> > mapping user chose and automatically reload keymap upon next reboot.
> 
> The standard setup for home machines tends to be an account per family 
> member.

That could be... Although it is desktops that are usually shared.
Hmm, I am trying to remember setup of the people I know... I think
the most common setup is a desktop in an easily accessible place
(kitchen) with a single account. Older kids/parents may have their
own desktop/laptops that are not shared.

> The standard setup in an office environment is likely to be  
> multiuser.

Huh? In my limited experience everyone in the office gets its own box.
And I am not talking about software shop.

> 
> > Note that KEY_UNKNOWN solution does not preculde futher customization on
> > per-user base once default action is established.
> 
> No, but it makes it significantly more confusing. User 1 chooses a 
> setup. This gets saved. User 2 remaps keys based on User 1's settings 
> (which have been restored at bootup). User 1 alters key mapping. User 2 
> suddenly becomes hugely confused.

One user is an administrator. He can alter the global keymap. If there
are multiple users he may need to be cautious.
 
> > > 
> > > Alternatively, we could generate a keycode and then let the user map 
> > > that to an X keysym. We've even already got code to do this.
> > >
> > 
> > There is world outside of X.
> 
> On machines like we're discussing (laptops, basically) it's a tiny 
> world. Optimise for the common case, not the rare one.
> 
> > > That's a ridiculously niche case, and can be handled in userspace. Just 
> > > have udev do remapping when it detects multiple keyboards that both have 
> > > KEY_PROG* layers, or let X have different keymaps for different input 
> > > devices. We shouldn't make the (by far) common case significantly more 
> > > difficult to deal with this one.
> > > 
> > 
> > No, it is not a niche case. I think it is much more common than the case 
> > where
> > you have multiple users for the same box using different keymaps. Even if 
> > box
> > is shared there most likely will be one person setting it up in the 
> > beginning
> > and the rest will follow his/her setup.
> 
> How many users plug external keyboards with unlabelled keys into a 
> laptop? No, I really don't think that's a common case at all.

I think quite a few people use external keyboards. I know that in my office
everyone with a laptop has a docking station and uses full keyboard with
it. I use external AT keyboard at home...

As far as unlabeled goes - they may be labeled but we may not know their
labels.

> The solution that satisfies the largest number of users with the 
> smallest amount of work is the one where pressing a key on the keyboard 
> results in X events being generated. Right now, that requires that the 
> key generate a real keycode.
> 

Again, it is not only about X. What if X is not running (or running but
nobody is logged in)? There are number of events (SUSPEND, WLAN switch,
undock request, etc) that should be handled by daemons not depending
on X.

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


Re: [PATCH] Input: document the proper usage of EV_KEY and KEY_UNKNOWN

2007-05-31 Thread Matthew Garrett
On Thu, May 31, 2007 at 11:33:10PM -0400, Dmitry Torokhov wrote:
> On Thursday 31 May 2007 21:44, Matthew Garrett wrote:
> > It's not trivial at all. You need to introduce a mechanism for noting a 
> > KEY_UNKNOWN keypress. It then needs to signal the user (dbus is probably 
> > the best layer for this), but you need to ensure that you only signal 
> > the user who is currently at the keyboard. This needs to be presented to 
> > the user via some sort of UI, which will then need to signal some sort 
> > of privileged process to actually change the keymap.
> 
> Not necessarily priveleged - you most likely already change ownership
> of event devices to user who is logged at console (so your force feedback
> joysticks work).

If you let users alter the kernel keymap, then you need to implement 
support for resetting the kernel keymap on exit. Otherwise it's a 
trivial DoS.

> > When the user logs  
> > out, you'll then need to unmap the key again and repeat as necessary for 
> > any new user who logs in.
> 
> I think we should aim at the most common case - when there are no multiple
> users on the box. Then the utility that detects KEY_UNKNOWN just saves the
> mapping user chose and automatically reload keymap upon next reboot.

The standard setup for home machines tends to be an account per family 
member. The standard setup in an office environment is likely to be 
multiuser.

> Note that KEY_UNKNOWN solution does not preculde futher customization on
> per-user base once default action is established.

No, but it makes it significantly more confusing. User 1 chooses a 
setup. This gets saved. User 2 remaps keys based on User 1's settings 
(which have been restored at bootup). User 1 alters key mapping. User 2 
suddenly becomes hugely confused.

> > 
> > Alternatively, we could generate a keycode and then let the user map 
> > that to an X keysym. We've even already got code to do this.
> >
> 
> There is world outside of X.

On machines like we're discussing (laptops, basically) it's a tiny 
world. Optimise for the common case, not the rare one.

> > That's a ridiculously niche case, and can be handled in userspace. Just 
> > have udev do remapping when it detects multiple keyboards that both have 
> > KEY_PROG* layers, or let X have different keymaps for different input 
> > devices. We shouldn't make the (by far) common case significantly more 
> > difficult to deal with this one.
> > 
> 
> No, it is not a niche case. I think it is much more common than the case where
> you have multiple users for the same box using different keymaps. Even if box
> is shared there most likely will be one person setting it up in the beginning
> and the rest will follow his/her setup.

How many users plug external keyboards with unlabelled keys into a 
laptop? No, I really don't think that's a common case at all.

The solution that satisfies the largest number of users with the 
smallest amount of work is the one where pressing a key on the keyboard 
results in X events being generated. Right now, that requires that the 
key generate a real keycode.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Input: document the proper usage of EV_KEY and KEY_UNKNOWN

2007-05-31 Thread Dmitry Torokhov
On Thursday 31 May 2007 21:44, Matthew Garrett wrote:
> On Thu, May 31, 2007 at 10:29:28PM -0300, Henrique de Moraes Holschuh wrote:
> > On Fri, 01 Jun 2007, Matthew Garrett wrote:
> > > Given existing userspace, it's never useful to generate KEY_UNKNOWN. 
> > > Adding extra information to the event doesn't alter that.
> > 
> > It will not break anything, and it is trivial to write an application to
> > intelligently handle KEY_UNKNOWN+scancode events.  This really is not a
> > reason to not do it, at all.
> 
> It's not trivial at all. You need to introduce a mechanism for noting a 
> KEY_UNKNOWN keypress. It then needs to signal the user (dbus is probably 
> the best layer for this), but you need to ensure that you only signal 
> the user who is currently at the keyboard. This needs to be presented to 
> the user via some sort of UI, which will then need to signal some sort 
> of privileged process to actually change the keymap.

Not necessarily priveleged - you most likely already change ownership
of event devices to user who is logged at console (so your force feedback
joysticks work).

> When the user logs  
> out, you'll then need to unmap the key again and repeat as necessary for 
> any new user who logs in.

I think we should aim at the most common case - when there are no multiple
users on the box. Then the utility that detects KEY_UNKNOWN just saves the
mapping user chose and automatically reload keymap upon next reboot.

Note that KEY_UNKNOWN solution does not preculde futher customization on
per-user base once default action is established.

> 
> Alternatively, we could generate a keycode and then let the user map 
> that to an X keysym. We've even already got code to do this.
>

There is world outside of X.
 
> > > I think using positional keycodes would also be a mistake. We just need 
> > > a slightly larger set of keycodes representing user-definable keys. 
> > > There's 4 of them already - I really can't imagine there being many 
> > > keyboards with a significantly larger set of unlabelled keys.
> > 
> > I had this exact PoV, too, until Dmitry reminded me that keycodes are
> > *global* to the system in practice, and that different keys (as in keys that
> > have no correlation between their position, labels or lack thereof, and
> > function) in different input devices would end up mapped to KEY_PROGx by
> > default.
> 
> That's a ridiculously niche case, and can be handled in userspace. Just 
> have udev do remapping when it detects multiple keyboards that both have 
> KEY_PROG* layers, or let X have different keymaps for different input 
> devices. We shouldn't make the (by far) common case significantly more 
> difficult to deal with this one.
> 

No, it is not a niche case. I think it is much more common than the case where
you have multiple users for the same box using different keymaps. Even if box
is shared there most likely will be one person setting it up in the beginning
and the rest will follow his/her setup.

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


[PATCH] ACPI: poke display on lid open

2007-05-31 Thread Daniel Drake
Many Dell laptops have the DSDT coded to power down the display when the lid
is closed, and leave it off when it is opened.

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

Based on ideas from Len Brown, this patch creates an internal event so
that the button driver can notify the video driver when the lid has been
opened. The video driver then reactivates the LCD.

Signed-off-by: Daniel Drake <[EMAIL PROTECTED]>

---

After all 3 of these patches, my Dell laptop display is correctly powered back
up when I open the lid, without any userspace or DSDT hacks. Is the ievent
idea acceptable or does it clutter things too much?

Index: linux/drivers/acpi/button.c
===
--- linux.orig/drivers/acpi/button.c
+++ linux/drivers/acpi/button.c
@@ -259,10 +259,15 @@ static void acpi_button_notify(acpi_hand
if (button->type == ACPI_BUTTON_TYPE_LID) {
struct acpi_handle *handle = button->device->handle;
unsigned long state;
+   acpi_status status;
 
-   if (!ACPI_FAILURE(acpi_evaluate_integer(handle, "_LID",
-   NULL, &state)))
+   status = acpi_evaluate_integer(handle, "_LID", NULL,
+  &state);
+
+   if (!ACPI_FAILURE(status)) {
+   acpi_ievent_notify(ACPI_IEVENT_LID, &state);
input_report_switch(input, SW_LID, !state);
+   }
 
} else {
int keycode = test_bit(KEY_SLEEP, input->keybit) ?
Index: linux/drivers/acpi/video.c
===
--- linux.orig/drivers/acpi/video.c
+++ linux/drivers/acpi/video.c
@@ -131,6 +131,7 @@ struct acpi_video_bus {
struct semaphore sem;
struct list_head video_device_list;
struct proc_dir_entry *dir;
+   struct acpi_ievent_handle *lid_event;
 };
 
 struct acpi_video_device_flags {
@@ -1730,6 +1731,29 @@ static int acpi_video_bus_stop_devices(s
return acpi_video_bus_DOS(video, 0, 1);
 }
 
+/* Upon lid open event, poke DSS to wake up display. Needed for many Dell
+ * laptops. */
+static void acpi_video_lid_event(enum acpi_ievent_code event, void *event_data,
+void *user_data)
+{
+   struct acpi_video_bus *video = user_data;
+   struct acpi_video_device *dev, *tmp;
+   unsigned long state = *((unsigned long *) event_data);
+
+   if (!state)
+   return;
+
+   list_for_each_entry_safe(dev, tmp, &video->video_device_list, entry) {
+   if (!dev->flags.lcd)
+   continue;
+   ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+ "Waking up %s.%s on lid event\n",
+ acpi_device_bid(video->device),
+ acpi_device_bid(dev->dev)));
+   acpi_video_device_set_state(dev, 0x8001);
+   }
+}
+
 static void acpi_video_bus_notify(acpi_handle handle, u32 event, void *data)
 {
struct acpi_video_bus *video = data;
@@ -1825,11 +1849,11 @@ static int acpi_video_bus_add(struct acp
acpi_video_bus_find_cap(video);
result = acpi_video_bus_check(video);
if (result)
-   goto end;
+   goto out;
 
result = acpi_video_bus_add_fs(device);
if (result)
-   goto end;
+   goto out;
 
init_MUTEX(&video->sem);
INIT_LIST_HEAD(&video->video_device_list);
@@ -1843,12 +1867,17 @@ static int acpi_video_bus_add(struct acp
if (ACPI_FAILURE(status)) {
ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
  "Error installing notify handler\n"));
-   acpi_video_bus_stop_devices(video);
-   acpi_video_bus_put_devices(video);
-   kfree(video->attached_array);
-   acpi_video_bus_remove_fs(device);
result = -ENODEV;
-   goto end;
+   goto out_stop;
+   }
+
+   video->lid_event = acpi_install_ievent_handler(ACPI_IEVENT_LID,
+   acpi_video_lid_event, video);
+   if (!video->lid_event) {
+   ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
+ "Error installing lid handler\n"));
+   result = -ENODEV;
+   goto out_remove_notify;
}
 
printk(KERN_INFO PREFIX "%s [%s] (multi-head: %s  rom: %s  post: %s)\n",
@@ -1856,11 +1885,19 @@ static int acpi_video_bus_add(struct acp
   video->flags.multihead ? "yes" : "no",
   video->flags.rom ? "yes" : "no",
   video->flags.post ? "yes" : "no");
+   return result;
 
-  end:
+out_remove_notify:
+   acpi_remove_notify_handler(device->han

[PATCH] ACPI: add internal event mechanism

2007-05-31 Thread Daniel Drake
This patch creates a new event system for communication between in-kernel
ACPI drivers ("ievent"). It is simple - it should only be used to infrequently
pass simple messages to a small audience.

This is used in an upcoming patch which makes the ACPI video driver listen for
lid open events from the button driver. I hope it is generic enough to be
useful to other ACPI drivers in the future.

Signed-off-by: Daniel Drake <[EMAIL PROTECTED]>

Index: linux/drivers/acpi/bus.c
===
--- linux.orig/drivers/acpi/bus.c
+++ linux/drivers/acpi/bus.c
@@ -533,6 +533,100 @@ static void acpi_bus_notify(acpi_handle 
 }
 
 /* --
+ Internal events
+   -- 
*/
+
+/*
+ * This is an event implementation designed for simple communication between
+ * ACPI drivers. It is intended for infrequent events that do not have
+ * many listeners.
+ *
+ * Drivers install an ievent handler to a certain event, and unregister it
+ * later. Drivers may raise (notify) events from any point except from the
+ * context of an event handler function.
+ */
+
+static struct list_head ievent_list;
+static spinlock_t ievent_lock;
+
+struct acpi_ievent_handle {
+   struct list_head list;
+   enum acpi_ievent_code event;
+   acpi_ievent_handler handler;
+   void *user_data;
+};
+
+/* Subscribe to a specific internal event.
+ *
+ * The handler function takes 3 parameters:
+ * 1. The acpi_ievent_code of the event that was raised
+ * 2. Some event data (set when the event was raised by the code which
+ *raised the event)
+ * 3. Some user data which you can specify in the data parameter below.
+ *
+ * This function returns a handle which you must save so that you can
+ * unsubscribe later.
+ */
+struct acpi_ievent_handle *
+acpi_install_ievent_handler(enum acpi_ievent_code event,
+   acpi_ievent_handler handler, void *data)
+{
+   unsigned long flags;
+   struct acpi_ievent_handle *handle
+   = kmalloc(sizeof(*handle), GFP_KERNEL);
+   if (!handle)
+   return NULL;
+
+   handle->event = event;
+   handle->handler = handler;
+   handle->user_data = data;
+   INIT_LIST_HEAD(&handle->list);
+
+   spin_lock_irqsave(&ievent_lock, flags);
+   list_add_tail(&handle->list, &ievent_list);
+   spin_unlock_irqrestore(&ievent_lock, flags);
+
+   return handle;
+}
+EXPORT_SYMBOL_GPL(acpi_install_ievent_handler);
+
+/*
+ * Unsubscribe from an internal event, using a handle that was returned from
+ * acpi_install_ievent_handler() earlier.
+ */
+void acpi_remove_ievent_handler(struct acpi_ievent_handle *handle)
+{
+   unsigned long flags;
+
+   if (!handle)
+   return;
+
+   spin_lock_irqsave(&ievent_lock, flags);
+   list_del(&handle->list);
+   spin_unlock_irqrestore(&ievent_lock, flags);
+   kfree(handle);
+}
+EXPORT_SYMBOL_GPL(acpi_remove_ievent_handler);
+
+/*
+ * Raise an event with some data.
+ */
+void acpi_ievent_notify(enum acpi_ievent_code event, void *event_data)
+{
+   struct acpi_ievent_handle *handle;
+   unsigned long flags;
+
+   spin_lock_irqsave(&ievent_lock, flags);
+   list_for_each_entry(handle, &ievent_list, list) {
+   if (handle->event != event)
+   continue;
+   handle->handler(event, event_data, handle->user_data);
+   }
+   spin_unlock_irqrestore(&ievent_lock, flags);
+}
+EXPORT_SYMBOL_GPL(acpi_ievent_notify);
+
+/* --
  Initialization/Cleanup
-- 
*/
 
@@ -741,6 +835,8 @@ static int __init acpi_init(void)
 {
int result = 0;
 
+   spin_lock_init(&ievent_lock);
+   INIT_LIST_HEAD(&ievent_list);
 
if (acpi_disabled) {
printk(KERN_INFO PREFIX "Interpreter disabled.\n");
Index: linux/include/acpi/acpi_bus.h
===
--- linux.orig/include/acpi/acpi_bus.h
+++ linux/include/acpi/acpi_bus.h
@@ -323,6 +323,26 @@ struct acpi_bus_event {
 extern struct kset acpi_subsys;
 
 /*
+ * Internal events
+ */
+
+enum acpi_ievent_code {
+   ACPI_IEVENT_LID,
+};
+
+struct acpi_ievent_handle;
+
+typedef
+void (*acpi_ievent_handler)(enum acpi_ievent_code event, void *event_data,
+   void *user_data);
+
+struct acpi_ievent_handle *
+acpi_install_ievent_handler(enum acpi_ievent_code event,
+   acpi_ievent_handler handler, void *data);
+void acpi_remove_ievent_handler(struct acpi_ievent_handle *handle);
+void acpi_ievent_notify(enum acpi_ievent_code event, void *event_data);
+
+/*
  * External Functions
  */
 
-

[PATCH] ACPI video: guess flags for devices with non-standard addressing

2007-05-31 Thread Daniel Drake
Dell laptops seem to address video devices without the device_id_scheme bit,
which means that Linux doesn't know the device types of the video devices.

This patch makes the ACPI video driver guess the device type based on the
device name for devices which do not have the device_id_scheme set. This
fixes the flags field for all 4 of my video devices on this laptop
(Inspiron 640m).

Signed-off-by: Daniel Drake <[EMAIL PROTECTED]>

Index: linux/drivers/acpi/video.c
===
--- linux.orig/drivers/acpi/video.c
+++ linux/drivers/acpi/video.c
@@ -1343,26 +1343,46 @@ acpi_video_bus_get_one_device(struct acp
 
attribute = acpi_video_get_device_attr(video, device_id);
 
-   if((attribute != NULL) && attribute->device_id_scheme) {
-   switch (attribute->display_type) {
-   case ACPI_VIDEO_DISPLAY_CRT:
-   data->flags.crt = 1;
-   break;
-   case ACPI_VIDEO_DISPLAY_TV:
-   data->flags.tvout = 1;
-   break;
-   case ACPI_VIDEO_DISPLAY_DVI:
-   data->flags.dvi = 1;
-   break;
-   case ACPI_VIDEO_DISPLAY_LCD:
-   data->flags.lcd = 1;
-   break;
-   default:
-   data->flags.unknown = 1;
-   break;
+   if (attribute) {
+   if (!attribute->device_id_scheme) {
+   char *name = acpi_device_bid(device);
+
+   /* Dell laptops seem to follow standard video
+* device addressing but do not set the
+* device_id_scheme bit.
+* Guess the flags based on the device names.
+*/
+   if (strcmp(name, "CRT") == 0)
+   data->flags.crt = 1;
+   else if (strcmp(name, "TV") == 0)
+   data->flags.tvout = 1;
+   else if (strcmp(name, "DVI") == 0)
+   data->flags.dvi = 1;
+   else if (strcmp(name, "LCD") == 0)
+   data->flags.lcd = 1;
+   else
+   data->flags.unknown = 1;
+   } else {
+   switch (attribute->display_type) {
+   case ACPI_VIDEO_DISPLAY_CRT:
+   data->flags.crt = 1;
+   break;
+   case ACPI_VIDEO_DISPLAY_TV:
+   data->flags.tvout = 1;
+   break;
+   case ACPI_VIDEO_DISPLAY_DVI:
+   data->flags.dvi = 1;
+   break;
+   case ACPI_VIDEO_DISPLAY_LCD:
+   data->flags.lcd = 1;
+   break;
+   default:
+   data->flags.unknown = 1;
+   break;
+   }
+   if(attribute->bios_can_detect)
+   data->flags.bios = 1;
}
-   if(attribute->bios_can_detect)
-   data->flags.bios = 1;
} else
data->flags.unknown = 1;
 
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Laptop screen doesn't come on when lid is reopened

2007-05-31 Thread Daniel Drake

Len Brown wrote:

Then the button driver should take the lid event and
cause the code in the video driver to be invoked.
I'm not sure what the best method for inter-driver
event like this, since both of them can be modules.


I have taken a stab at this approach and it is working nicely. I'm 
sending patches to the linux-acpi list shortly.


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


Re: 2.6.22-rc1-mm1 [cannot change thermal trip points]

2007-05-31 Thread Len Brown
On Monday 21 May 2007 08:11, Pavel Machek wrote:
> On Thu 2007-05-17 18:42:43, Len Brown wrote:
> > > Something similar happened to me on XE3, yes.
> > > 
> > > (Actual values were different; BIOS specified critical temperature at
> > > cca 95C, but hw killed the power at cca 83C. Setting critical trip
> > > point at 80C made the problem go away.)
> > 
> > Great, please file a bug and include the acpidump from the XE3
> > and we'll fix it, rather than supporting a bogus (manual) workaround for it.
> 
> It is few years since I do not have that XE3 machine.
> 
> > Of course if your system is running at 80*C and the hardware shuts
> > off at 83*C, you may have a broken fan, or one clogged with dust...
> 
> It _did_ have broken fan. It also had broken trip points.

Thanks for clarifying this, Pavel.
If you come upon an XE3 where Linux-2.6.22 doesn't work as well
as Windows, please let me know.

Given that the justification for this ill-conceived workaround
seems to have diminished to the memory of broken hardware,
it is clear that we should stay the course of removing it
so that it doesn't further confuse future users.

If SuSE violently disagrees with me, you are certainly empowered
to restore the workaround in your distribution staring at 2.6.22
as part of your value add.  However, given its history of confusing
users, it seems that it might increase your support burden rather
than decrease it.

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


[patch] Hook acpi D-state method to pnp suspend/resume

2007-05-31 Thread Shaohua Li
Hook acpi D-state method to pnp suspend/resume, so we have a chance to poweroff 
the device.

Signed-off-by: Shaohua Li <[EMAIL PROTECTED]>

Index: 2.6.22-rc2/include/linux/pnp.h
===
--- 2.6.22-rc2.orig/include/linux/pnp.h 2007-05-23 09:15:14.0 +0800
+++ 2.6.22-rc2/include/linux/pnp.h  2007-05-24 16:14:17.0 +0800
@@ -335,6 +335,10 @@ struct pnp_protocol {
int (*set)(struct pnp_dev *dev, struct pnp_resource_table *res);
int (*disable)(struct pnp_dev *dev);
 
+   /* protocol specific suspend/resume */
+   int (*suspend)(struct pnp_dev *dev, pm_message_t state);
+   int (*resume)(struct pnp_dev *dev);
+
/* used by pnp layer only (look but don't touch) */
unsigned char   number; /* protocol number*/
struct device   dev;/* link to driver model */
Index: 2.6.22-rc2/drivers/pnp/driver.c
===
--- 2.6.22-rc2.orig/drivers/pnp/driver.c2007-05-23 09:15:14.0 
+0800
+++ 2.6.22-rc2/drivers/pnp/driver.c 2007-05-24 16:15:29.0 +0800
@@ -167,6 +167,8 @@ static int pnp_bus_suspend(struct device
return error;
}
 
+   if (pnp_dev->protocol && pnp_dev->protocol->suspend)
+   pnp_dev->protocol->suspend(pnp_dev, state);
return 0;
 }
 
@@ -179,6 +181,9 @@ static int pnp_bus_resume(struct device 
if (!pnp_drv)
return 0;
 
+   if (pnp_dev->protocol && pnp_dev->protocol->resume)
+   pnp_dev->protocol->resume(pnp_dev);
+
if (!(pnp_drv->flags & PNP_DRIVER_RES_DO_NOT_CHANGE)) {
error = pnp_start_dev(pnp_dev);
if (error)
Index: 2.6.22-rc2/drivers/pnp/pnpacpi/core.c
===
--- 2.6.22-rc2.orig/drivers/pnp/pnpacpi/core.c  2007-05-23 09:15:14.0 
+0800
+++ 2.6.22-rc2/drivers/pnp/pnpacpi/core.c   2007-06-01 10:35:13.0 
+0800
@@ -119,11 +119,23 @@ static int pnpacpi_disable_resources(str
return ACPI_FAILURE(status) ? -ENODEV : 0;
 }
 
+static int pnpacpi_suspend(struct pnp_dev *dev, pm_message_t state)
+{
+   return acpi_bus_set_power((acpi_handle)dev->data, 3);
+}
+
+static int pnpacpi_resume(struct pnp_dev *dev)
+{
+   return acpi_bus_set_power((acpi_handle)dev->data, 0);
+}
+
 static struct pnp_protocol pnpacpi_protocol = {
.name   = "Plug and Play ACPI",
.get= pnpacpi_get_resources,
.set= pnpacpi_set_resources,
.disable = pnpacpi_disable_resources,
+   .suspend = pnpacpi_suspend,
+   .resume = pnpacpi_resume,
 };
 
 static int __init pnpacpi_add_device(struct acpi_device *device)
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Input: document the proper usage of EV_KEY and KEY_UNKNOWN

2007-05-31 Thread Henrique de Moraes Holschuh
On Fri, 01 Jun 2007, Matthew Garrett wrote:
> On Thu, May 31, 2007 at 10:29:28PM -0300, Henrique de Moraes Holschuh wrote:
> > On Fri, 01 Jun 2007, Matthew Garrett wrote:
> > > Given existing userspace, it's never useful to generate KEY_UNKNOWN. 
> > > Adding extra information to the event doesn't alter that.
> > 
> > It will not break anything, and it is trivial to write an application to
> > intelligently handle KEY_UNKNOWN+scancode events.  This really is not a
> > reason to not do it, at all.
> 
> It's not trivial at all. You need to introduce a mechanism for noting a 
> KEY_UNKNOWN keypress. It then needs to signal the user (dbus is probably 

That would be trivial, but...

> the best layer for this), but you need to ensure that you only signal 
> the user who is currently at the keyboard. This needs to be presented to 
> the user via some sort of UI, which will then need to signal some sort 
> of privileged process to actually change the keymap. When the user logs 
> out, you'll then need to unmap the key again and repeat as necessary for 
> any new user who logs in.

... this is not.  You're correct.  So using KEY_UNKNOWN should not be the
preferred way, then.

> Alternatively, we could generate a keycode and then let the user map 
> that to an X keysym. We've even already got code to do this.

Well, I also would appreciate bumping up KEY_MAX and a nice shiny set of
KEY_FN that covered all the missing ones for ThinkPads.  It is Dmitry you
have to convince about it.

That still doesn't make KEY_UNKNOWN without a scan code useful, so either
way, I think KEY_UNKNOWN should be limited to drivers that can remap
keycodes *and* which send MSC_SCAN events along with KEY_UNKNOWN, so that
you know what key to remap.  I don't care if it is easy or not to be nice to
the user and ask for a key remap, at least it will not be *impossible* like
it currently is.  Otherwise, we would be better off removing KEY_UNKNOWN
altogether (which I wouldn't mind much, either).

> > > I think using positional keycodes would also be a mistake. We just need 
> > > a slightly larger set of keycodes representing user-definable keys. 
> > > There's 4 of them already - I really can't imagine there being many 
> > > keyboards with a significantly larger set of unlabelled keys.
> > 
> > I had this exact PoV, too, until Dmitry reminded me that keycodes are
> > *global* to the system in practice, and that different keys (as in keys that
> > have no correlation between their position, labels or lack thereof, and
> > function) in different input devices would end up mapped to KEY_PROGx by
> > default.
> 
> That's a ridiculously niche case, and can be handled in userspace. Just 
> have udev do remapping when it detects multiple keyboards that both have 
> KEY_PROG* layers, or let X have different keymaps for different input 
> devices. We shouldn't make the (by far) common case significantly more 
> difficult to deal with this one.

Dmitry?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Input: document the proper usage of EV_KEY and KEY_UNKNOWN

2007-05-31 Thread Matthew Garrett
On Thu, May 31, 2007 at 10:29:28PM -0300, Henrique de Moraes Holschuh wrote:
> On Fri, 01 Jun 2007, Matthew Garrett wrote:
> > Given existing userspace, it's never useful to generate KEY_UNKNOWN. 
> > Adding extra information to the event doesn't alter that.
> 
> It will not break anything, and it is trivial to write an application to
> intelligently handle KEY_UNKNOWN+scancode events.  This really is not a
> reason to not do it, at all.

It's not trivial at all. You need to introduce a mechanism for noting a 
KEY_UNKNOWN keypress. It then needs to signal the user (dbus is probably 
the best layer for this), but you need to ensure that you only signal 
the user who is currently at the keyboard. This needs to be presented to 
the user via some sort of UI, which will then need to signal some sort 
of privileged process to actually change the keymap. When the user logs 
out, you'll then need to unmap the key again and repeat as necessary for 
any new user who logs in.

Alternatively, we could generate a keycode and then let the user map 
that to an X keysym. We've even already got code to do this.

> > I think using positional keycodes would also be a mistake. We just need 
> > a slightly larger set of keycodes representing user-definable keys. 
> > There's 4 of them already - I really can't imagine there being many 
> > keyboards with a significantly larger set of unlabelled keys.
> 
> I had this exact PoV, too, until Dmitry reminded me that keycodes are
> *global* to the system in practice, and that different keys (as in keys that
> have no correlation between their position, labels or lack thereof, and
> function) in different input devices would end up mapped to KEY_PROGx by
> default.

That's a ridiculously niche case, and can be handled in userspace. Just 
have udev do remapping when it detects multiple keyboards that both have 
KEY_PROG* layers, or let X have different keymaps for different input 
devices. We shouldn't make the (by far) common case significantly more 
difficult to deal with this one.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Input: document the proper usage of EV_KEY and KEY_UNKNOWN

2007-05-31 Thread Henrique de Moraes Holschuh
On Fri, 01 Jun 2007, Matthew Garrett wrote:
> On Thu, May 31, 2007 at 09:13:04PM -0300, Henrique de Moraes Holschuh wrote:
> > Well, we already produce KEY_UNKNOWN anyway, and the stuff you quoted above
> > just makes KEY_UNKNOWN useful for something instead of keeping it as an
> > useless notice to the user that some key (which one? who knows!) was
> > pressed.
> 
> Given existing userspace, it's never useful to generate KEY_UNKNOWN. 
> Adding extra information to the event doesn't alter that.

It will not break anything, and it is trivial to write an application to
intelligently handle KEY_UNKNOWN+scancode events.  This really is not a
reason to not do it, at all.

> > Perhaps what you dislike re. KEY_UNKNOWN is the part where KEY_UNKNOWN+scan
> > code is declared to be the prefered way to report keys that do not have a
> > specific function?  Your reply seems to indicate this, but I am not sure I
> > really understood what you meant.
> 
> Yes.

That could easily be removed or switched around but...

> > I am not exactly in love with the idea of using KEY_UNKNOWN in place of
> > stuff like KEY_FN_F1 either (I'd prefer to just bump up KEY_MAX and have
> > more posicional keycodes), but Dmitry is being quite clear that he does not
> > want to increase KEY_MAX to add more positional keycodes.
> 
> I think using positional keycodes would also be a mistake. We just need 
> a slightly larger set of keycodes representing user-definable keys. 
> There's 4 of them already - I really can't imagine there being many 
> keyboards with a significantly larger set of unlabelled keys.

I had this exact PoV, too, until Dmitry reminded me that keycodes are
*global* to the system in practice, and that different keys (as in keys that
have no correlation between their position, labels or lack thereof, and
function) in different input devices would end up mapped to KEY_PROGx by
default.

And in that scenario, KEY_UNKNOWN (i.e. "please remap me to what you want
this key to do for real") makes a lot more sense, given that we don't have
all the positional keycodes we need, and more are not being added.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC][PATCH 0/3] A Dynticks Aware Processor Idle PM Governor

2007-05-31 Thread Len Brown
Applied this series to acpi-test
in anticipation that Venki is about to send some incremental
patches to address the feedback on it.

thanks,
-Len

On Saturday 24 March 2007 03:46, Adam Belay wrote:
> Hi All,
> 
> Here is my first take at implementing an idle PM governor that takes
> full advantage of NO_HZ.  I call it the 'menu' governor because it
> considers the full list of idle states before each entry.
> 
> I've kept the implementation fairly simple.  It attempts to guess the
> next residency time and then chooses a state that would meet at least
> the break-even point between power savings and entry cost.  To this end,
> it selects the deepest idle state that satisfies the following
> constraints:
>  1. If the idle time elapsed since bus master activity was detected
> is below a threshold (currently 20 ms), then limit the selection
> to C2-type or above.
>  2. Do not choose a state with a break-even residency that exceeds
> the expected time remaining until the next timer interrupt.
>  3. Do not choose a state with a break-even residency that exceeds
> the elapsed time between the last pair of break events,
> excluding timer interrupts.
> 
> This governor has an advantage over "ladder" governor because it
> proactively checks how much time remains until the next timer interrupt
> using the tick infrastructure.  Also, it handles device interrupt
> activity more intelligently by not including timer interrupts in break
> event calculations.  Finally, it doesn't make policy decisions using the
> number of state entries, which can have variable residency times (NO_HZ
> makes these potentially very large), and instead only considers sleep
> time deltas.
> 
> The menu governor can be selected during runtime using the cpuidle sysfs
> interface like so:
> "echo "menu" > /sys/devices/system/cpu/cpuidle/current_governor"
> 
> This patchset applies against 2.6.21-rc4 plus the latest from the acpi
> testing tree, which is available here:
> ftp://ftp.kernel.org/pub/linux/kernel/people/lenb/acpi/patches/test/2.6.21/acpi-test-20070126-2.6.21-rc4.diff.bz2
> 
> I'd really appreciate any comments, benchmarks, or suggestions.
> 
> Cheers,
> Adam
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: scary warnings

2007-05-31 Thread Len Brown
Please open a sighting at bugzilla.kernel.org against ACPI/tables
and attach the output from acpidump

thanks,
-Len

On Thursday 31 May 2007 20:11, Sergio Monteiro Basto wrote:
> I got this messages warnings, I just found it, after had other problems
> not related with ACPI, I am been running with this kernels one or two
> months ago, and I don't had problems, just reporting the situation,
> could be useful anyway. 
> Thanks,
> 
> ACPI Warning (tbfadt-0360): Ignoring BIOS FADT r2 C-state control [20070126]
> (...)
> pci_hotplug: PCI Hot Plug PCI Core version: 0.5
> ACPI Warning (tbutils-0158): Incorrect checksum in table [  ð] -  00, should 
> be E7 [20070126]
> ACPI Error (tbinstal-0134): Table has invalid signature [  ð], must be SSDT 
> or PSDT [20070126]
> ACPI Error (psparse-0537): Method parse/execution failed [\_PR_.CPU1._PDC] 
> (Node 810037cc3b50), AE_BAD_SIGNATURE
> ACPI Warning (tbutils-0158): Incorrect checksum in table [  ð] -  00, should 
> be E7 [20070126]
> ACPI Error (tbinstal-0134): Table has invalid signature [  ð], must be SSDT 
> or PSDT [20070126]
> ACPI Error (psparse-0537): Method parse/execution failed [\_PR_.CPU2._PDC] 
> (Node 810037cc3a50), AE_BAD_SIGNATURE
> ACPI Exception (processor_core-0783): AE_NOT_FOUND, Processor Device is not 
> present [20070126]
> ACPI Exception (processor_core-0783): AE_NOT_FOUND, Processor Device is not 
> present [20070126]
> 
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Input: document the proper usage of EV_KEY and KEY_UNKNOWN

2007-05-31 Thread Matthew Garrett
On Thu, May 31, 2007 at 09:13:04PM -0300, Henrique de Moraes Holschuh wrote:

> Well, we already produce KEY_UNKNOWN anyway, and the stuff you quoted above
> just makes KEY_UNKNOWN useful for something instead of keeping it as an
> useless notice to the user that some key (which one? who knows!) was
> pressed.

Given existing userspace, it's never useful to generate KEY_UNKNOWN. 
Adding extra information to the event doesn't alter that.

> Perhaps what you dislike re. KEY_UNKNOWN is the part where KEY_UNKNOWN+scan
> code is declared to be the prefered way to report keys that do not have a
> specific function?  Your reply seems to indicate this, but I am not sure I
> really understood what you meant.

Yes.

> I am not exactly in love with the idea of using KEY_UNKNOWN in place of
> stuff like KEY_FN_F1 either (I'd prefer to just bump up KEY_MAX and have
> more posicional keycodes), but Dmitry is being quite clear that he does not
> want to increase KEY_MAX to add more positional keycodes.

I think using positional keycodes would also be a mistake. We just need 
a slightly larger set of keycodes representing user-definable keys. 
There's 4 of them already - I really can't imagine there being many 
keyboards with a significantly larger set of unlabelled keys.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


scary warnings

2007-05-31 Thread Sergio Monteiro Basto
I got this messages warnings, I just found it, after had other problems
not related with ACPI, I am been running with this kernels one or two
months ago, and I don't had problems, just reporting the situation,
could be useful anyway. 
Thanks,

ACPI Warning (tbfadt-0360): Ignoring BIOS FADT r2 C-state control [20070126]
(...)
pci_hotplug: PCI Hot Plug PCI Core version: 0.5
ACPI Warning (tbutils-0158): Incorrect checksum in table [  ð] -  00, should be 
E7 [20070126]
ACPI Error (tbinstal-0134): Table has invalid signature [  ð], must be SSDT or 
PSDT [20070126]
ACPI Error (psparse-0537): Method parse/execution failed [\_PR_.CPU1._PDC] 
(Node 810037cc3b50), AE_BAD_SIGNATURE
ACPI Warning (tbutils-0158): Incorrect checksum in table [  ð] -  00, should be 
E7 [20070126]
ACPI Error (tbinstal-0134): Table has invalid signature [  ð], must be SSDT or 
PSDT [20070126]
ACPI Error (psparse-0537): Method parse/execution failed [\_PR_.CPU2._PDC] 
(Node 810037cc3a50), AE_BAD_SIGNATURE
ACPI Exception (processor_core-0783): AE_NOT_FOUND, Processor Device is not 
present [20070126]
ACPI Exception (processor_core-0783): AE_NOT_FOUND, Processor Device is not 
present [20070126]

-- 
Sérgio M.B.
 BIOS-e820: 3ffb - 3ffc (ACPI data)
 BIOS-e820: 3ffc - 3fff (ACPI NVS)
ACPI: RSDP 000F7CA0, 0014 (r0 ACPIAM)
ACPI: RSDT 3FFB, 0034 (r1 A M I  OEMRSDT   6000616 MSFT   97)
ACPI: FACP 3FFB0200, 0084 (r2 A M I  OEMFACP   6000616 MSFT   97)
ACPI Warning (tbfadt-0360): Ignoring BIOS FADT r2 C-state control [20070126]
ACPI: DSDT 3FFB0450, 3CD7 (r1  75D8P 75D8P0044 INTL  2002026)
ACPI: FACS 3FFC, 0040
ACPI: APIC 3FFB0390, 0078 (r1 A M I  OEMAPIC   6000616 MSFT   97)
ACPI: MCFG 3FFB0410, 003C (r1 A M I  OEMMCFG   6000616 MSFT   97)
ACPI: OEMB 3FFC0040, 0046 (r1 A M I  AMI_OEM   6000616 MSFT   97)
ACPI: PM-Timer IO Port: 0x808
ACPI: Local APIC address 0xfee0
ACPI: LAPIC (acpi_id[0x01] lapic_id[0x00] enabled)
ACPI: LAPIC (acpi_id[0x02] lapic_id[0x01] enabled)
ACPI: LAPIC (acpi_id[0x03] lapic_id[0x82] disabled)
ACPI: LAPIC (acpi_id[0x04] lapic_id[0x83] disabled)
ACPI: IOAPIC (id[0x02] address[0xfec0] gsi_base[0])
ACPI: IOAPIC (id[0x03] address[0xfecc] gsi_base[24])
ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level)
ACPI: IRQ0 used by override.
ACPI: IRQ2 used by override.
ACPI: IRQ9 used by override.
Using ACPI (MADT) for SMP configuration information
ACPI: Core revision 20070126
ACPI: bus type pci registered
ACPI: Interpreter enabled
ACPI: (supports S0 S1 S3 S4 S5)
ACPI: Using IOAPIC for interrupt routing
ACPI: PCI Root Bridge [PCI0] (:00)
ACPI: PCI Interrupt Routing Table [\_SB_.PCI0._PRT]
ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.NBPG._PRT]
ACPI: PCI Interrupt Link [LNKA] (IRQs 3 4 5 6 7 10 *11 12 14 15)
ACPI: PCI Interrupt Link [LNKB] (IRQs *3 4 5 6 7 10 11 12 14 15)
ACPI: PCI Interrupt Link [LNKC] (IRQs 3 4 *5 6 7 10 11 12 14 15)
ACPI: PCI Interrupt Link [LNKD] (IRQs 3 4 *5 6 7 10 11 12 14 15)
ACPI: PCI Interrupt Link [LNKE] (IRQs 3 4 5 6 7 10 11 12 14 15) *0, disabled.
ACPI: PCI Interrupt Link [LNKF] (IRQs 3 4 5 6 7 10 11 12 14 15) *0, disabled.
ACPI: PCI Interrupt Link [LNKG] (IRQs 3 4 5 6 7 10 11 12 14 15) *0, disabled.
ACPI: PCI Interrupt Link [LNKH] (IRQs 3 4 5 6 7 *10 11 12 14 15)
pnp: PnP ACPI init
pnp: PnP ACPI: found 15 devices
PCI: Using ACPI for IRQ routing
Time: acpi_pm clocksource has been installed.
ACPI: PCI Interrupt :00:02.0[A] -> GSI 27 (level, low) -> IRQ 27
ACPI Warning (tbutils-0158): Incorrect checksum in table [  ð] -  00, should 
be E7 [20070126]
ACPI Error (tbinstal-0134): Table has invalid signature [  ð], must be SSDT or 
PSDT [20070126]
ACPI Error (psparse-0537): Method parse/execution failed [\_PR_.CPU1._PDC] 
(Node 810037cc3b50), AE_BAD_SIGNATURE
ACPI Warning (tbutils-0158): Incorrect checksum in table [  ð] -  00, should 
be E7 [20070126]
ACPI Error (tbinstal-0134): Table has invalid signature [  ð], must be SSDT or 
PSDT [20070126]
ACPI Error (psparse-0537): Method parse/execution failed [\_PR_.CPU2._PDC] 
(Node 810037cc3a50), AE_BAD_SIGNATURE
ACPI Exception (processor_core-0783): AE_NOT_FOUND, Processor Device is not 
present [20070126]
ACPI Exception (processor_core-0783): AE_NOT_FOUND, Processor Device is not 
present [20070126]
ACPI: PCI Interrupt :00:10.0[A] -> GSI 21 (level, low) -> IRQ 21
ACPI: PCI Interrupt :00:10.1[A] -> GSI 21 (level, low) -> IRQ 21
ACPI: PCI Interrupt :00:10.2[B] -> GSI 21 (level, low) -> IRQ 21
ACPI: PCI Interrupt :00:10.3[B] -> GSI 21 (level, low) -> IRQ 21
ACPI: PCI Interrupt :00:10.4[C] -> GSI 21 (level, low) -> IRQ 21
ACPI: PCI Interrupt :00:0f.0[B] -> GSI 20 (level, low) -> IRQ 20
ACPI: PCI Interrupt :00:0f.1[A] -> GSI 20 (level, low) -> IRQ 20
ACPI: PCI Interrupt :00:12.0[A] -> GSI 23 (level, low) -> IRQ 23
ACPI: PCI Interrupt :00:0b.0[A] -> GSI 19 (l

Re: [PATCH] Input: document the proper usage of EV_KEY and KEY_UNKNOWN

2007-05-31 Thread Henrique de Moraes Holschuh
On Fri, 01 Jun 2007, Matthew Garrett wrote:
> On Thu, May 31, 2007 at 07:28:14PM -0300, Henrique de Moraes Holschuh wrote:
> > We have all the pieces needed to have sane, generic userland keyboard 
> > handling
> > in place for a while now, but it was not sufficiently documented (or used!).
> > 
> > If EV_KEY input drivers always generate scan codes that can be used to
> > reprogram their keycode maps, and always generate EV_MSC MSC_SCAN events 
> > when
> > they output an EV_KEY KEY_UNKNOWN event, userspace can trap those and feed 
> > it
> > to a generic helper that can ask the user to assign a key code and function 
> > to
> > that key.
> 
> I still disagree that this is the best approach. Userspace already has 
> the functionality to map keys if they produce a keycode. Producing 
> KEY_UNKNOWN would require the implementation of a stack of extra code.

Well, we already produce KEY_UNKNOWN anyway, and the stuff you quoted above
just makes KEY_UNKNOWN useful for something instead of keeping it as an
useless notice to the user that some key (which one? who knows!) was
pressed.

Drivers already have KEY_RESERVED to mark positions in the keycode map for
keys that should generate no events, so we are not forcing anyone to always
generate useless events, either.

Perhaps what you dislike re. KEY_UNKNOWN is the part where KEY_UNKNOWN+scan
code is declared to be the prefered way to report keys that do not have a
specific function?  Your reply seems to indicate this, but I am not sure I
really understood what you meant.

I am not exactly in love with the idea of using KEY_UNKNOWN in place of
stuff like KEY_FN_F1 either (I'd prefer to just bump up KEY_MAX and have
more posicional keycodes), but Dmitry is being quite clear that he does not
want to increase KEY_MAX to add more positional keycodes.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Input: document the proper usage of EV_KEY and KEY_UNKNOWN

2007-05-31 Thread Matthew Garrett
On Thu, May 31, 2007 at 07:28:14PM -0300, Henrique de Moraes Holschuh wrote:
> We have all the pieces needed to have sane, generic userland keyboard handling
> in place for a while now, but it was not sufficiently documented (or used!).
> 
> If EV_KEY input drivers always generate scan codes that can be used to
> reprogram their keycode maps, and always generate EV_MSC MSC_SCAN events when
> they output an EV_KEY KEY_UNKNOWN event, userspace can trap those and feed it
> to a generic helper that can ask the user to assign a key code and function to
> that key.

I still disagree that this is the best approach. Userspace already has 
the functionality to map keys if they produce a keycode. Producing 
KEY_UNKNOWN would require the implementation of a stack of extra code.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Add INPUT support to toshiba_acpi

2007-05-31 Thread Richard Hughes
On Thu, 2007-05-31 at 18:46 +0200, Andreas Mohr wrote:
> HCI_EMPTY is *by far* the most frequent state to occur I think
> (users won't press keys all the time), thus it's probably better(?)
> for branch prediction to have this placed first, right?
> Not that it matters too much instruction-wise, but still...

Sure.

> Apart from that I'm very happy to see progress on this front
> (speaking as a "proud" owner of an old Toshiba notebook requiring
> this stuff).

Good - this stuff should just work :-)

> And I'd definitely move the multiple identical "Re-enabled hotkeys" parts
> into one single non-inlined(!) function for the same reason.
> Not to mention that it's BUTT UGLY to have the *same* fat
> multi-line comment duplicated bazillion times.

Agree. New patch (untested) attached.

Thanks for review,

Richard.

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 139f41f..38835b6 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -222,6 +222,7 @@ config ACPI_TOSHIBA
 	tristate "Toshiba Laptop Extras"
 	depends on X86
 	select BACKLIGHT_CLASS_DEVICE
+	select INPUT_POLLDEV
 	---help---
 	  This driver adds support for access to certain system settings
 	  on "legacy free" Toshiba laptops.  These laptops can be recognized by
diff --git a/drivers/acpi/toshiba_acpi.c b/drivers/acpi/toshiba_acpi.c
index 3906d47..f802609 100644
--- a/drivers/acpi/toshiba_acpi.c
+++ b/drivers/acpi/toshiba_acpi.c
@@ -27,13 +27,13 @@
  *		engineering the Windows drivers
  *	Yasushi Nagato - changes for linux kernel 2.4 -> 2.5
  *	Rob Miller - TV out and hotkeys help
- *
+ *	Richard Hughes - emit INPUT events for hotkeys
  *
  *  TODO
  *
  */
 
-#define TOSHIBA_ACPI_VERSION	"0.18"
+#define TOSHIBA_ACPI_VERSION	"0.19"
 #define PROC_INTERFACE_VERSION	1
 
 #include 
@@ -42,6 +42,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -99,6 +100,16 @@ MODULE_LICENSE("GPL");
 #define HCI_VIDEO_OUT_CRT		0x2
 #define HCI_VIDEO_OUT_TV		0x4
 
+/* key definitions */
+#define HCI_HKEY_MUTE			0x0101
+#define HCI_HKEY_LOCK			0x013b
+#define HCI_HKEY_SEARCH			0x013c
+#define HCI_HKEY_SUSPEND		0x013d
+#define HCI_HKEY_HIBERNATE		0x013e
+#define HCI_HKEY_BRIGHTNESSDOWN		0x0140
+#define HCI_HKEY_BRIGHTNESSUP		0x0141
+#define HCI_HKEY_WLAN			0x0142
+
 /* utility
  */
 
@@ -213,9 +224,15 @@ static acpi_status hci_read1(u32 reg, u32 * out1, u32 * result)
 
 static struct proc_dir_entry *toshiba_proc_dir /*= 0*/ ;
 static struct backlight_device *toshiba_backlight_device;
+static struct input_polled_dev *toshiba_poll_dev;
 static int force_fan;
 static int last_key_event;
 static int key_event_valid;
+static int hotkeys_over_input = 1;
+static int hotkeys_check_per_sec = 2;
+
+module_param(hotkeys_over_input, uint, 0400);
+module_param(hotkeys_check_per_sec, uint, 0400);
 
 typedef struct _ProcItem {
 	const char *name;
@@ -438,34 +455,60 @@ static unsigned long write_fan(const char *buffer, unsigned long count)
 	return count;
 }
 
-static char *read_keys(char *p)
+/* returns zero if hci event is invalid */
+static u32 get_hci_system_event_value(void)
 {
 	u32 hci_result;
 	u32 value;
+	u32 retval = 0;
+
+	/* read a system event from the queue */
+	hci_read1(HCI_SYSTEM_EVENT, &value, &hci_result);
+
+	switch (hci_result) {
+	case HCI_EMPTY:
+		/* better luck next time */
+		break;
+	case HCI_SUCCESS:
+		/* value is the data in the queue */
+		retval = value;
+		break;
+	case HCI_NOT_SUPPORTED:
+		/* This is a workaround for an unresolved issue on
+		 * some machines where system events sporadically
+		 * become disabled. */
+		hci_write1(HCI_SYSTEM_EVENT, 1, &hci_result);
+		printk(MY_NOTICE "Re-enabled hotkeys\n");
+		break;
+	default:
+		/* there was an unknown error */
+		printk(MY_ERR "Error reading hotkey status\n");
+	}
+	return retval;
+}
 
-	if (!key_event_valid) {
-		hci_read1(HCI_SYSTEM_EVENT, &value, &hci_result);
-		if (hci_result == HCI_SUCCESS) {
-			key_event_valid = 1;
-			last_key_event = value;
-		} else if (hci_result == HCI_EMPTY) {
-			/* better luck next time */
-		} else if (hci_result == HCI_NOT_SUPPORTED) {
-			/* This is a workaround for an unresolved issue on
-			 * some machines where system events sporadically
-			 * become disabled. */
-			hci_write1(HCI_SYSTEM_EVENT, 1, &hci_result);
-			printk(MY_NOTICE "Re-enabled hotkeys\n");
-		} else {
-			printk(MY_ERR "Error reading hotkey status\n");
-			goto end;
+/* deprecate this interface... */
+static char *read_keys(char *p)
+{
+	u32 value;
+
+	if (hotkeys_over_input) {
+		key_event_valid = 0;
+		last_key_event = 0;
+	} else {
+		if (!key_event_valid) {
+			value = get_hci_system_event_value();
+			if (value != 0) {
+key_event_valid = 1;
+last_key_event = value;
+			}
 		}
 	}
 
 	p += sprintf(p, "hotkey_ready:%d\n", key_event_valid);
 	p += sprintf(p, "hotkey:  0x%04x\n", last_key_event);
+	p += sprintf(p, "hotkeys_via_input:   %d\n", hotkeys_over_input);
 
-  end:
 	return p;
 }
 
@@ -534,15 +577

RE: acpi exception with current -mm lineup (HPET)

2007-05-31 Thread Pallipadi, Venkatesh
 

>-Original Message-
>From: Len Brown [mailto:[EMAIL PROTECTED] 
>Sent: Wednesday, May 30, 2007 11:02 PM
>To: Andrew Morton; Pallipadi, Venkatesh
>Cc: linux-acpi@vger.kernel.org
>Subject: Re: acpi exception with current -mm lineup (HPET)
>
>
>> I put a copy of /proc/acpi/dsdt at 
>http://userweb.kernel.org/~akpm/dsdt
>> if that's any help.
>> 
>> Full dmesg at http://userweb.kernel.org/~akpm/dmesg-x.txt
>
>Okay, this BIOS actually has a real HPET table:
>
>ACPI: HPET 7FFD7460, 0038 (r1 A M I  OEMHPET   5000427 MSFT   97)
>
>and the hpet is actually found:
>
>Calling initcall 0x80652488: late_hpet_init+0x0/0xd1()
>hpet0: at MMIO 0xfed0, IRQs 2, 8, 0
>hpet0: 3 64-bit timers, 14318180 Hz
>initcall 0x80652488: late_hpet_init+0x0/0xd1() returned 0.
>initcall 0x80652488 ran for 0 msecs: late_hpet_init+0x0/0xd1()
>...
>
>Perhaps Venki can comment on if the resource conflict is expected
>to be fatal or not.
>


The issue seems to be HPET at two different places in BIOS/ACPI.
One listing is in MADT which is used to set up kernel timer and
which reserves this resource earlier

hpet0: at MMIO 0xfed0, IRQs 2, 8, 0

And later hpet character driver is finding HPET listed in _CRS
And finds it is the same HPET as the one looked at before
Causing this conflict. Totally nonfatal.
Just correcting the return type of hpet_resources should be fine here.

Thanks,
Venki 
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Input: document the proper usage of EV_KEY and KEY_UNKNOWN

2007-05-31 Thread Henrique de Moraes Holschuh
We have all the pieces needed to have sane, generic userland keyboard handling
in place for a while now, but it was not sufficiently documented (or used!).

If EV_KEY input drivers always generate scan codes that can be used to
reprogram their keycode maps, and always generate EV_MSC MSC_SCAN events when
they output an EV_KEY KEY_UNKNOWN event, userspace can trap those and feed it
to a generic helper that can ask the user to assign a key code and function to
that key.

This patch documents the requirements and best practices for EV_KEY input
drivers.

Signed-off-by: Henrique de Moraes Holschuh <[EMAIL PROTECTED]>
Cc: Dmitry Torokhov <[EMAIL PROTECTED]>
---

 Dmitry, how is that as a first approximation of the text?

 Documentation/input/input-programming.txt |   53 -
 1 files changed, 52 insertions(+), 1 deletions(-)

diff --git a/Documentation/input/input-programming.txt 
b/Documentation/input/input-programming.txt
index d9d5230..836f0bb 100644
--- a/Documentation/input/input-programming.txt
+++ b/Documentation/input/input-programming.txt
@@ -272,7 +272,58 @@ present, it is broken sometimes (at keyboards: Toshiba 
notebooks). To enable
 autorepeat for your device, just set EV_REP in dev->evbit. All will be
 handled by the input system.
 
-1.9 Other event types, handling output events
+1.9 Being friendly to userspace when implementing EV_KEY drivers
+
+
+Input drivers that generate EV_KEY events should always support either
+dev->getkeycode()/dev->setkeycode(), or keycode, keycodemax and keycodesize,
+so as to allow userspace to reprogram the keycodes as needed.  They should
+also either always generate EV_MSC MSC_SCAN events along with every EV_KEY
+event, or special case EV_KEY KEY_UNKNOWN as described below.
+
+If the input driver doesn't support any of the generic methods to manipulate
+the keycode map, it must never issue a EV_KEY KEY_UNKNOWN event.  If you
+need to issue EV_KEY KEY_UNKNOWN events, please implement the code in your
+driver to manipulate its keycode map.  KEY_UNKNOWN is meant to be something
+that can be replaced by the user with a functional keycode.
+
+If the input driver generates an EV_KEY KEY_UNKNOWN event, it should also
+generate *in the same event block* (i.e. before it issues an EV_SYN) an
+EV_MSC MSC_SCAN event, with the scan code for the "unknown key".  This
+should be done both in "press" and "release" EV_KEY events.  The EV_MSC
+MSC_SCAN event allows a generic userspace keyboard helper daemon to ask the
+user if he would like to map a key in a input device to a valid keycode, and
+assign a function to it.
+
+The scan code of a key (as informed in a EV_MSC MSC_SCAN event) must be its
+index in the keycode map, as implemented by dev->getkeycode() /
+dev->setkeycode(), or keycode, keycodemax and keycodesize for the device.
+
+If a key has a specific function that is known to the driver, it should
+generate the appropriate keycode for that function by default.  E.g., in a
+laptop where the FN+F1 key combination is always marked "HELP" in the
+keyboard, the driver is to generate KEY_HELP and not KEY_FN_F1.
+
+Unmarked keys that do not have a set function, or whose functions are
+unknown, should usually generate by default an EV_KEY KEY_UNKNOWN event.  If
+a positional keycode for that key already exists in input.h (e.g. KEY_FN_F1
+for FN+F1), it can also be used for backwards compatibility.  KEY_UNKNOWN is
+preferred for all new drivers, however.
+
+Non-positional keycodes like KEY_PROG1 should never be used by default.
+
+As an example, a ThinkPad T43 laptop hot key keyboard has FN+F1 unmarked,
+and FN+F5 marked with a "RF transmitter" symbol.  The driver for this
+keyboard is to generate EV_KEY KEY_UNKNOWN (plus EV_MSC MSC_SCAN ) if the user presses or releases FN+F1.  It could generate EV_KEY
+KEY_FN_F1 *instead* of EV_KEY KEY_UNKNOWN as well.
+
+If the ThinkPad T43 user presses FN+F5, however, the driver is to generate
+EV_KEY KEY_WLAN, and not KEY_FN_F5 or KEY_UNKNOWN (this assumes the driver
+does know it is running on a ThinkPad T43, and that it has a model-specific
+key table for the T43).
+
+1.10 Other event types, handling output events
 ~
 
 The other event types up to now are:
-- 
1.5.1.6


-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ACPI Debug - for test, devel and possibly even for production kernels

2007-05-31 Thread Len Brown
On Thursday 31 May 2007 14:57, Pekka Enberg wrote:
> On 5/31/07, Thomas Renninger <[EMAIL PROTECTED]> wrote:
> > (This should efficiently be the same as the proposed big patch a year
> > ago from Pekka Enberg, just a bit smaller and should make ACPICA and
> > kernel/linux people happy:
> > http://marc.info/?l=linux-kernel&m=113699535303722&w=2)
> 
> No, you're keeping these obfuscating macros around:
> 
> +#define return_VOID return
> +#define return_ACPI_STATUS(s)   return(s)
> +#define return_VALUE(s) return(s)
> +#define return_UINT8(s) return(s)
> 
> Making the ACPI code look like regular Linux kernel code (or even
> regular C for that matter) was the whole point of my patch. Your patch
> doesn't change that.

I think that Thomas's point is that he is optimally removing
function tracing via #ifdef.
Your 600KB patch, on the other hand, permanently removed the feature
and touched every file in ACPICA.

The net effect to the user is the same, the ability to enable
ACPI_DEBUG and not enable ACPICA function tracing.

As I probably wrote a year ago, it isn't viable to completely
remove the tracing code --
until Linux reaches a point where vendors certify that their
BIOS is compatible with Linux before they ship, rather than the Linux
community having to debug some Windows-compatible systems into
Linux-compatibility well after they have shipped into the field.

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


Re: [PATCH] ACPI Debug - for test, devel and possibly even for production kernels

2007-05-31 Thread Pekka Enberg

On 5/31/07, Thomas Renninger <[EMAIL PROTECTED]> wrote:

(This should efficiently be the same as the proposed big patch a year
ago from Pekka Enberg, just a bit smaller and should make ACPICA and
kernel/linux people happy:
http://marc.info/?l=linux-kernel&m=113699535303722&w=2)


No, you're keeping these obfuscating macros around:

+#define return_VOID return
+#define return_ACPI_STATUS(s)   return(s)
+#define return_VALUE(s) return(s)
+#define return_UINT8(s) return(s)

Making the ACPI code look like regular Linux kernel code (or even
regular C for that matter) was the whole point of my patch. Your patch
doesn't change that.
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Making KEY_UNKNOWN really useful to userland

2007-05-31 Thread Dmitry Torokhov
On Wednesday 30 May 2007 20:53, Henrique de Moraes Holschuh wrote:
> On Wed, 30 May 2007, Dmitry Torokhov wrote:
> > On 5/30/07, Henrique de Moraes Holschuh <[EMAIL PROTECTED]> wrote:
> > >It is trivial to guarantee that KEY_PROG is unique for a single input
> > >device (keyboard), but it certainly won't work across multiple devices.
> > >Userspace has to know what kind of input device it is talking to to map
> > >them to something, but since the input layer already provides this
> > >information, I was not going to raise a fuss about it.  I figure it is
> > >the price of not increasing even more the keycode table.
> > 
> > Actually I try to discourage people from using input_id during device
> > discovery but rather tell them to analyze capability bits and then decide
> > if their application is interested in that particular input device. I
> > think input_id should pretty much only used by udev & co. to adjust
> > default kernel setup for the needs of local installation (fix keymap,
> > adjust absmax, etc).
> 
> It is likely going to be used by something like udev, or an init script, or
> a thinkpad-configurator helper, or whatever.  Sounds like the sort of stuff
> that should be paying attention to input_id anyway.

Right.

> 
> We don't appear to have deployed a good kernel-userland interface that
> allows us to have generic "press an unknown key and tell me what you want it
> to do" application in userland, but if we do please correct me.  If
> KEY_UNKNOWN was always required to send a EV_MSC MSC_RAW/MSC_SCAN, then we
> would have one.
>

Yes, I think this is a resonable requrement.

> Maybe we can add that requirement and driver changes (if any, for all I know
> most drivers might already be generating such events) for 2.6.23?

I think that we should start with drivers that allow adjusting keymaps
via EVIOCGKEYCODE/EVIOCSKEYCODE. If driver does not allow changing keymap
then it does not really matter (although I will try to convert drivers
now that we have getkeycode/setkeycode per-device methods).
 
> I bet 
> Richard would like that one a lot.  Richard?
> 
> Such functionality makes most of my requests for new keycodes irrelevant, so
> I will just trim that down.
> 
> > >That said, how is one to know which hardware key was translated to
> > >KEY_UNKNOWN, so that he can inform the user to remap that key?  Should I
> > >send another event together with KEY_UNKNOWN that has the raw keycode?
> > >Which one?
> > 
> > You may try using EV_MSC/MSC_SCAN. You can even send it all the time, not
> > only for KEY_UNKNOWN.
> 
> Will do, thanks.  And I will send it all the time, that will force people to
> properly implement code that can deal with them in userland.
> 
> What is the exact difference between MSC_RAW and MSC_SCAN?  Which one can be
> used as an index to reprogram the keymap using the IOCTLs?
>

MSC_RAW is just raw data from device. It may carry make/break data encoded
in it. MSC_SCAN is what one need.

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


Re: [PATCH] Section mismatch ... acpi_map_pxm_to_node

2007-05-31 Thread Len Brown
Applied.

thanks,
-Len

On Thursday 24 May 2007 16:57, Luck, Tony wrote:
> Last of the "Section mismatch" errors from ia64 builds! acpi_map_pxm_to_node()
> is defined with attribute __cpuinit, but is called by "normal" kernel 
> functions
> acpi_getnode() and acpi_map_cpu2node().
> 
> Commit f363d16fbb9374c0bd7f2757d412c287169094c9 moved the data structures on
> which this routine operates from __cpuinitdata to regular memory, so this
> routine can also move out of init space.
> 
> Signed-off-by: Tony Luck <[EMAIL PROTECTED]>
> 
> ---
> 
> diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
> index a2efae8..0c9f15c 100644
> --- a/drivers/acpi/numa.c
> +++ b/drivers/acpi/numa.c
> @@ -59,7 +59,7 @@ int node_to_pxm(int node)
>   return node_to_pxm_map[node];
>  }
>  
> -int __cpuinit acpi_map_pxm_to_node(int pxm)
> +int acpi_map_pxm_to_node(int pxm)
>  {
>   int node = pxm_to_node_map[pxm];
>  
> diff --git a/include/acpi/acpi_numa.h b/include/acpi/acpi_numa.h
> index b62cd36..e2fcee2 100644
> --- a/include/acpi/acpi_numa.h
> +++ b/include/acpi/acpi_numa.h
> @@ -13,7 +13,7 @@
>  
>  extern int pxm_to_node(int);
>  extern int node_to_pxm(int);
> -extern int __cpuinit acpi_map_pxm_to_node(int);
> +extern int acpi_map_pxm_to_node(int);
>  extern void __cpuinit acpi_unmap_pxm_to_node(int);
>  
>  #endif   /* CONFIG_ACPI_NUMA */
> -
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ACPI: thinkpad-acpi: do not use named sysfs groups

2007-05-31 Thread Len Brown
Applied.

thanks,
-Len

On Wednesday 30 May 2007 19:50, Henrique de Moraes Holschuh wrote:
> The initial version of the thinkpad-acpi sysfs interface (not yet released
> in any stable mainline kernel) made liberal use of named sysfs groups, in
> order to get the attributes more organized.
> 
> This proved to be a really bad design decision.  Maybe if attribute groups
> were as flexible as a real directory, and if binary attributes were not
> second-class citizens, the idea of subdirs and named groups would not have
> been so bad.
> 
> This patch makes all the thinkpad-acpi sysfs groups anonymous (thus
> removing the subdirs), adds the former group names as a prefix (so that
> hotkey/enable becomes hotkey_enable for example), and updates the
> documentation.
> 
> These changes will make the thinkpad-acpi sysfs ABI a lot easier to
> maintain.
> 
> Signed-off-by: Henrique de Moraes Holschuh <[EMAIL PROTECTED]>
> ---
> 
>  It gets easier to apply if I actually get git to include the patch :)
>  Sorry about that!
> 
>  Documentation/thinkpad-acpi.txt |   25 +++--
>  drivers/misc/thinkpad_acpi.c|   17 +++--
>  drivers/misc/thinkpad_acpi.h|6 --
>  3 files changed, 18 insertions(+), 30 deletions(-)
> 
> diff --git a/Documentation/thinkpad-acpi.txt b/Documentation/thinkpad-acpi.txt
> index 2d48033..9e6b94f 100644
> --- a/Documentation/thinkpad-acpi.txt
> +++ b/Documentation/thinkpad-acpi.txt
> @@ -138,7 +138,7 @@ Hot keys
>  
>  
>  procfs: /proc/acpi/ibm/hotkey
> -sysfs device attribute: hotkey/*
> +sysfs device attribute: hotkey_*
>  
>  Without this driver, only the Fn-F4 key (sleep button) generates an
>  ACPI event. With the driver loaded, the hotkey feature enabled and the
> @@ -196,10 +196,7 @@ The following commands can be written to the 
> /proc/acpi/ibm/hotkey file:
>  
>  sysfs notes:
>  
> - The hot keys attributes are in a hotkey/ subdirectory off the
> - thinkpad device.
> -
> - bios_enabled:
> + hotkey_bios_enabled:
>   Returns the status of the hot keys feature when
>   thinkpad-acpi was loaded.  Upon module unload, the hot
>   key feature status will be restored to this value.
> @@ -207,19 +204,19 @@ sysfs notes:
>   0: hot keys were disabled
>   1: hot keys were enabled
>  
> - bios_mask:
> + hotkey_bios_mask:
>   Returns the hot keys mask when thinkpad-acpi was loaded.
>   Upon module unload, the hot keys mask will be restored
>   to this value.
>  
> - enable:
> + hotkey_enable:
>   Enables/disables the hot keys feature, and reports
>   current status of the hot keys feature.
>  
>   0: disables the hot keys feature / feature disabled
>   1: enables the hot keys feature / feature enabled
>  
> - mask:
> + hotkey_mask:
>   bit mask to enable ACPI event generation for each hot
>   key (see above).  Returns the current status of the hot
>   keys mask, and allows one to modify it.
> @@ -229,7 +226,7 @@ Bluetooth
>  -
>  
>  procfs: /proc/acpi/ibm/bluetooth
> -sysfs device attribute: bluetooth/enable
> +sysfs device attribute: bluetooth_enable
>  
>  This feature shows the presence and current state of a ThinkPad
>  Bluetooth device in the internal ThinkPad CDC slot.
> @@ -244,7 +241,7 @@ If Bluetooth is installed, the following commands can be 
> used:
>  Sysfs notes:
>  
>   If the Bluetooth CDC card is installed, it can be enabled /
> - disabled through the "bluetooth/enable" thinkpad-acpi device
> + disabled through the "bluetooth_enable" thinkpad-acpi device
>   attribute, and its current status can also be queried.
>  
>   enable:
> @@ -252,7 +249,7 @@ Sysfs notes:
>   1: enables Bluetooth / Bluetooth is enabled.
>  
>   Note: this interface will be probably be superseeded by the
> - generic rfkill class.
> + generic rfkill class, so it is NOT to be considered stable yet.
>  
>  Video output control -- /proc/acpi/ibm/video
>  
> @@ -898,7 +895,7 @@ EXPERIMENTAL: WAN
>  -
>  
>  procfs: /proc/acpi/ibm/wan
> -sysfs device attribute: wwan/enable
> +sysfs device attribute: wwan_enable
>  
>  This feature is marked EXPERIMENTAL because the implementation
>  directly accesses hardware registers and may not work as expected. USE
> @@ -921,7 +918,7 @@ If the W-WAN card is installed, the following commands 
> can be used:
>  Sysfs notes:
>  
>   If the W-WAN card is installed, it can be enabled /
> - disabled through the "wwan/enable" thinkpad-acpi device
> + disabled through the "wwan_enable" thinkpad-acpi device
>   attribute, and its current status can also be queried.
>  
>   enable:
> @@ -929,7 +926,7 @@ Sysfs notes:
>   1: enables WWAN card / WWAN card is enabled.
>  
>   Note: thi

Re: [PATCH] ACPI Debug - for test, devel and possibly even for production kernels

2007-05-31 Thread Len Brown
The name of this patch is really "split ACPI function tracing
out of CONFIG_ACPI_DEBUG to a new Kconfig build option"

I agree that tracing should be its own build option.

I don't agree that enabling CONFIG_ACPI_DEBUG by default
is what you want to do in a production kernel, but that
isn't what this patch does, and I'm not going to stand
in your way if you want to ship debug kernels by default.

> we only make use of a small subset of current ACPI tables.
> ACPI spreads more and more over different parts of the kernel and it's
> likely that this will hold on.

There are two types of "ACPI tables", those with contents defined by
the ACPI spec, and those where ACPI is just acting as the "envelope"
to deliver a table from another spec to the OS.

We already use all the tables defined by the ACPI spec.
It is unlikely that this patch will have any effect on the "envelope" tables.

So I don't think you're going to see growing utility of CONFIG_ACPI_DEBUG
over time, you're going to see what it does immediately.

> ACPI_DEBUG=y is a powerful debug facility, that can ease up bug
> communication with less (ACPI) experienced bug submitters and can shed
> some light behind ACPI magic.
> 
> I've done some measurements and found out that most of the performance
> costs are due the function tracing support in the ACPICA debug
> subsystem.
> 
> This patch offers the possibility to compile out ACPI function tracing
> support which is of none use for ordinary kernel debugging (is only used
> for Intel internal debugging AFAIK), but takes most of the performance
> costs.

I agree that function tracing is never used by people who can't build a kernel 
--
and so it is fine to make it an additional build option.

> (This should efficiently be the same as the proposed big patch a year
> ago from Pekka Enberg, just a bit smaller and should make ACPICA and
> kernel/linux people happy:
> http://marc.info/?l=linux-kernel&m=113699535303722&w=2
> )
> 
> 
> Kernel image size (compiled on x86_64):
> Full acpi debug:
> 1618277 bytes (+3.1%, +48k)
> acpi debug function trace compiled out:
> 1605359 bytes (+2.3%, +35k)
> acpi debug not compiled in at all
> 1569803 bytes (100%)
> 
> 
> Speed measures on one specific machine (x86_64, laptop):
> 
> A) ACPI_DEBUG compiled in
> B) ACPI_DEBUG compiled in, but Function trace compiled out
> C) ACPI_DEBUG compiled out
> D) ACPI_DEBUG compiled out (control run)
> 
> The time has been taken at following functions:
> 
> 1) drivers/acpi/bus.c:acpi_early_init (load tables and initialize
>ACPI namespace)
> 2) drivers/acpi/bus.c:acpi_init (some more initialisation
>  -> enable ACPI HW mode
>  -> read PIC/APIC table, ...)
> 3) drivers/acpi/scan.c:scan_init (Enumerate devices in the
>   ACPI namespace)
> 
> register driver for acpi drivers:
> 4)  ec
> 5)  pci_root
> 6)  pci_link
> 
> ---
>   |  A |   B|   C| D  | 
> ---
> 1 | 434711 (+1.0%) | 432652 (+0.4%) |  430522 (100%) |  430512 (-0.00%)
> ---
> 2 |  69156 (+3.7%) |  67007 (+0.4%) |   66696 (100%) |   66703 (+0.01%)
> ---
> 3 |   8769(+63.3%) |   7363(+37.1%) |5369 (100%) |5369 ( 0.00%)
> ---

37% sounds like a lot.

The ACPI interpreter is not supposed to be in performace critical sections,
but at the same time, it isn't supposed to be a pig either.

> 4 |  15999 (+0.7%) |  15950 (+0.4%) |   15879 (100%) |   15878 (-0.01%)
> ---
> 5 |   9492(+75.4%) |   7657(+41.5%) |5411 (100%) |5405 (-0.11%)
> ---
> 6 |   1717(+64.0%) |   1494(+42.6%) |1047 (100%) |1046 (-0.10%)
> ---
> All measured values are in us. TSC was used to obtain the values.
> do_gettimeofday did only provide valid values after hpet got setup
> (after pci_link).
> 
> I wonder why only the short time taking measures show that extreme
> performance loss, maybe someone has an idea. However if you summarize
> all additional time with ACPI_DEBUG without function trace, you get
> about +5 ms boot time (which may vary from machine to machine...).
> This should be acceptable for test and develop kernels.
> If nothing bad happens in Beta phase I'd like to include this one into
> the final OpenSUSE kernel to get detailed bug reports.
> It would be great if others with their latest test/devel kernels follow,
> so that even ordinary users can easily provide essen

Re: Add INPUT support to toshiba_acpi

2007-05-31 Thread Andreas Mohr
Hi,

On Thu, May 31, 2007 at 04:46:56PM +0100, Richard Hughes wrote:

+   if (!hotkeys_over_input) {
+   if (!key_event_valid) {
+   hci_read1(HCI_SYSTEM_EVENT, &value, &hci_result);
+   if (hci_result == HCI_SUCCESS) {
+   key_event_valid = 1;
+   last_key_event = value;
+   } else if (hci_result == HCI_EMPTY) {
+   /* better luck next time */

HCI_EMPTY is *by far* the most frequent state to occur I think
(users won't press keys all the time), thus it's probably better(?)
for branch prediction to have this placed first, right?
Not that it matters too much instruction-wise, but still...

Apart from that I'm very happy to see progress on this front
(speaking as a "proud" owner of an old Toshiba notebook requiring
this stuff).

Oh, and maybe merge the sprintf()s into a single one to reduce code size.

And I'd definitely move the multiple identical "Re-enabled hotkeys" parts
into one single non-inlined(!) function for the same reason.
Not to mention that it's BUTT UGLY to have the *same* fat
multi-line comment duplicated bazillion times.

Thanks a lot!

Andreas Mohr
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Add INPUT support to toshiba_acpi

2007-05-31 Thread Richard Hughes
On Thu, 2007-05-31 at 14:44 +0100, Richard Hughes wrote:
> Nope, impossible AFAICS. The hardware is just broken. Windows XP has an
> toshiba supplied daemon that polls, so I think we have to just bite the
> bullet.

... adding in reply in different thread...

On Thu, 2007-05-31 at 10:37 -0400, Dmitry Torokhov wrote:
> Please use input-polldev to set up polled devices. It
> will create work queue for you and will make sure that polling is
> stopped when device is closed. 

Okay, I had never heard of this. I've done the suggested changes in the
attached patch. Please can you have a look and make sure I'm being sane.

> Also I don't think you want to use
> KEY_BREAK. What is the expected function of that key?

It's a "lock" key, I really want KEY_LOCK added to input.h, but that
might prove difficult. For now I've used KEY_CLEAR, yell if you think
there's a better one to substitute or if you guys want me to add get a
constant added to input.h.

Thanks for the review.

Richard.

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 139f41f..38835b6 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -222,6 +222,7 @@ config ACPI_TOSHIBA
 	tristate "Toshiba Laptop Extras"
 	depends on X86
 	select BACKLIGHT_CLASS_DEVICE
+	select INPUT_POLLDEV
 	---help---
 	  This driver adds support for access to certain system settings
 	  on "legacy free" Toshiba laptops.  These laptops can be recognized by
diff --git a/drivers/acpi/toshiba_acpi.c b/drivers/acpi/toshiba_acpi.c
index 3906d47..2b1a3d9 100644
--- a/drivers/acpi/toshiba_acpi.c
+++ b/drivers/acpi/toshiba_acpi.c
@@ -27,13 +27,13 @@
  *		engineering the Windows drivers
  *	Yasushi Nagato - changes for linux kernel 2.4 -> 2.5
  *	Rob Miller - TV out and hotkeys help
- *
+ *	Richard Hughes - emit INPUT events for hotkeys
  *
  *  TODO
  *
  */
 
-#define TOSHIBA_ACPI_VERSION	"0.18"
+#define TOSHIBA_ACPI_VERSION	"0.19"
 #define PROC_INTERFACE_VERSION	1
 
 #include 
@@ -42,6 +42,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -99,6 +100,16 @@ MODULE_LICENSE("GPL");
 #define HCI_VIDEO_OUT_CRT		0x2
 #define HCI_VIDEO_OUT_TV		0x4
 
+/* key definitions */
+#define HCI_HKEY_MUTE			0x0101
+#define HCI_HKEY_BREAK			0x013b
+#define HCI_HKEY_SEARCH			0x013c
+#define HCI_HKEY_SUSPEND		0x013d
+#define HCI_HKEY_HIBERNATE		0x013e
+#define HCI_HKEY_BRIGHTNESSDOWN		0x0140
+#define HCI_HKEY_BRIGHTNESSUP		0x0141
+#define HCI_HKEY_WLAN			0x0142
+
 /* utility
  */
 
@@ -213,9 +224,15 @@ static acpi_status hci_read1(u32 reg, u32 * out1, u32 * result)
 
 static struct proc_dir_entry *toshiba_proc_dir /*= 0*/ ;
 static struct backlight_device *toshiba_backlight_device;
+static struct input_polled_dev *toshiba_poll_dev;
 static int force_fan;
 static int last_key_event;
 static int key_event_valid;
+static int hotkeys_over_input = 1;
+static int hotkeys_check_per_sec = 2;
+
+module_param(hotkeys_over_input, uint, 0400);
+module_param(hotkeys_check_per_sec, uint, 0400);
 
 typedef struct _ProcItem {
 	const char *name;
@@ -443,27 +460,33 @@ static char *read_keys(char *p)
 	u32 hci_result;
 	u32 value;
 
-	if (!key_event_valid) {
-		hci_read1(HCI_SYSTEM_EVENT, &value, &hci_result);
-		if (hci_result == HCI_SUCCESS) {
-			key_event_valid = 1;
-			last_key_event = value;
-		} else if (hci_result == HCI_EMPTY) {
-			/* better luck next time */
-		} else if (hci_result == HCI_NOT_SUPPORTED) {
-			/* This is a workaround for an unresolved issue on
-			 * some machines where system events sporadically
-			 * become disabled. */
-			hci_write1(HCI_SYSTEM_EVENT, 1, &hci_result);
-			printk(MY_NOTICE "Re-enabled hotkeys\n");
-		} else {
-			printk(MY_ERR "Error reading hotkey status\n");
-			goto end;
+	if (!hotkeys_over_input) {
+		if (!key_event_valid) {
+			hci_read1(HCI_SYSTEM_EVENT, &value, &hci_result);
+			if (hci_result == HCI_SUCCESS) {
+key_event_valid = 1;
+last_key_event = value;
+			} else if (hci_result == HCI_EMPTY) {
+/* better luck next time */
+			} else if (hci_result == HCI_NOT_SUPPORTED) {
+/* This is a workaround for an unresolved issue on
+ * some machines where system events sporadically
+ * become disabled. */
+hci_write1(HCI_SYSTEM_EVENT, 1, &hci_result);
+printk(MY_NOTICE "Re-enabled hotkeys\n");
+			} else {
+printk(MY_ERR "Error reading hotkey status\n");
+goto end;
+			}
 		}
+	} else {
+		key_event_valid = 0;
+		last_key_event = 0;
 	}
 
 	p += sprintf(p, "hotkey_ready:%d\n", key_event_valid);
 	p += sprintf(p, "hotkey:  0x%04x\n", last_key_event);
+	p += sprintf(p, "hotkeys_via_input:   %d\n", hotkeys_over_input);
 
   end:
 	return p;
@@ -534,15 +557,127 @@ static acpi_status __exit remove_device(void)
 }
 
 static struct backlight_ops toshiba_backlight_data = {
-.get_brightness = get_lcd,
-.update_status  = set_lcd_status,
+	.get_brightness = get_lcd,
+	.update_status  = set_lcd_status,
 };
 
+static void toshiba_deliver_button_event(struct

[PATCH] ACPI Debug - for test, devel and possibly even for production kernels

2007-05-31 Thread Thomas Renninger
Hi,

we only make use of a small subset of current ACPI tables.
ACPI spreads more and more over different parts of the kernel and it's
likely that this will hold on.

ACPI_DEBUG=y is a powerful debug facility, that can ease up bug
communication with less (ACPI) experienced bug submitters and can shed
some light behind ACPI magic.

I've done some measurements and found out that most of the performance
costs are due the function tracing support in the ACPICA debug
subsystem.

This patch offers the possibility to compile out ACPI function tracing
support which is of none use for ordinary kernel debugging (is only used
for Intel internal debugging AFAIK), but takes most of the performance
costs.
(This should efficiently be the same as the proposed big patch a year
ago from Pekka Enberg, just a bit smaller and should make ACPICA and
kernel/linux people happy:
http://marc.info/?l=linux-kernel&m=113699535303722&w=2
)


Kernel image size (compiled on x86_64):
Full acpi debug:
1618277 bytes (+3.1%, +48k)
acpi debug function trace compiled out:
1605359 bytes (+2.3%, +35k)
acpi debug not compiled in at all
1569803 bytes (100%)


Speed measures on one specific machine (x86_64, laptop):

A) ACPI_DEBUG compiled in
B) ACPI_DEBUG compiled in, but Function trace compiled out
C) ACPI_DEBUG compiled out
D) ACPI_DEBUG compiled out (control run)

The time has been taken at following functions:

1) drivers/acpi/bus.c:acpi_early_init (load tables and initialize
   ACPI namespace)
2) drivers/acpi/bus.c:acpi_init (some more initialisation
 -> enable ACPI HW mode
 -> read PIC/APIC table, ...)
3) drivers/acpi/scan.c:scan_init (Enumerate devices in the
  ACPI namespace)

register driver for acpi drivers:
4)  ec
5)  pci_root
6)  pci_link

---
  |  A |   B|   C| D  | 
---
1 | 434711 (+1.0%) | 432652 (+0.4%) |  430522 (100%) |  430512 (-0.00%)
---
2 |  69156 (+3.7%) |  67007 (+0.4%) |   66696 (100%) |   66703 (+0.01%)
---
3 |   8769(+63.3%) |   7363(+37.1%) |5369 (100%) |5369 ( 0.00%)
---
4 |  15999 (+0.7%) |  15950 (+0.4%) |   15879 (100%) |   15878 (-0.01%)
---
5 |   9492(+75.4%) |   7657(+41.5%) |5411 (100%) |5405 (-0.11%)
---
6 |   1717(+64.0%) |   1494(+42.6%) |1047 (100%) |1046 (-0.10%)
---
All measured values are in us. TSC was used to obtain the values.
do_gettimeofday did only provide valid values after hpet got setup
(after pci_link).

I wonder why only the short time taking measures show that extreme
performance loss, maybe someone has an idea. However if you summarize
all additional time with ACPI_DEBUG without function trace, you get
about +5 ms boot time (which may vary from machine to machine...).
This should be acceptable for test and develop kernels.
If nothing bad happens in Beta phase I'd like to include this one into
the final OpenSUSE kernel to get detailed bug reports.
It would be great if others with their latest test/devel kernels follow,
so that even ordinary users can easily provide essential, detailed debug
information easily(e.g. Linus' Compaq and Andrew's Vaio are well known
on the acpi list, others do not have the luck to get that much of
support and they do not have the knowledge to compile kernel and so
on...).

I can also add a file "Documentation/acpi_debug" and add some debug
hints if this one gets accepted...

This one should not be really dangerous. If the patch is ok, can Len
still merge this in .22 time?
If not, can you take it for some testing, Andrew?

Thanks,

   Thomas



Split ACPI_DEBUG into function trace enabled and not enabled.

Function trace is most of the ACPI_DEBUG costs, but is
not much of use for kernel ACPI debugging.

Size of kernel image increased on test compile:
+ 48k  (Full ACPI_DEBUG)
+ 35k  (ACPI_DEBUG with function trace compiled out)

Performance without function trace is also much better.

Also remove ACPI_LV_DEBUG_OBJECT from default debug level as
a lot vendors let Store (value, debug) in their code and this
might confuse users when it pops up in syslog.

---
 drivers/acpi/Kconfig|8 
 include/acpi/acmacros.h |   23 +++
 include/acpi/acoutput.h |4 ++--
 3 files changed, 33 insertions(+), 2 deletions(-)

Index: linux-2.6.22-rc3/drivers/acpi/Kconfig

Re: Making KEY_UNKNOWN really useful to userland

2007-05-31 Thread Dmitry Torokhov

On 5/31/07, Henrique de Moraes Holschuh <[EMAIL PROTECTED]> wrote:

On Thu, 31 May 2007, Richard Hughes wrote:

I am on it on the thinkpad-acpi side, so at least for that, you don't have
to worry.  I am still waiting an answer about which event is the correct one
to output scancodes, but the thinkpad-acpi GIT tree is already updated with
the above, tentively using MSC_SCAN to report scan codes.



I thought I sent out email saying MSC_SCAN is what you want to use,
MSC_RAW is raw data from device, potentially having make/break codes
included/encoded. Apparently my mail broke even more than I thought.

Highjacking the thread somewhat - Richard, I saw your patch for
toshiba_acpi. Please use input-polldev to set up polled devices. It
will create work queue for you and will make sure that polling is
stopped when device is closed. Also I don't think you want to use
KEY_BREAK. What is the expected function of that key?

Please copy me on input-related changes in the tree. Thank you.

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


Re: Add INPUT support to toshiba_acpi

2007-05-31 Thread Bastien Nocera
On Thu, 2007-05-31 at 13:36 +0100, Richard Hughes wrote:
> Attached patch adds a kernel thread to do polling on Toshiba hardware.
> 
> Toshiba hardware is a little oddball, and does not provide ACPI events
> on some key presses, typically Fn hotkey buttons. The key interface is
> now polled, and events now matched to a list of toshiba specific
> scancodes, and are squirted to userspace using the INPUT subsystem.
> 
> This means that toshiba laptops buttons "just work" without any
> userspace daemon (using uinput) such as fnfx or bodges such as using a
> userspace hal addon. Doing the polling in kernel is more efficient, and
> makes stuff just work out of the box. You can assign the keys using
> standard X keymaps, or using tools such as gnome-keybinding-properties.
> 
> This is similar to other patches sent for the thinkpad_acpi driver, and
> is part of the "Unf*ck my keyboard" initiative to make multimedia keys
> just work.
> 
> Changes from the first patch involve switching to a workqueue for the
> polling, not breaking the spaces in "hotkeys_via_input" and also masking
> out the fn key button up.

A couple of things:
- use a switch statement in toshiba_deliver_button_event(), would look a
bit cleaner.
- make the magic number #defines
- not sure if it's possible, but you should disable the workqueue
altogether if nothing has the input device opened.

Hopefully we'll find a way to receive an interrupt, or some kind of
event when the keys are pressed in the future, and completely avoid the
polling. In the meantime, it should be minimised.

Cheers

-- 
Bastien Nocera <[EMAIL PROTECTED]> 

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


Re: Add INPUT support to toshiba_acpi

2007-05-31 Thread Richard Hughes
On Thu, 2007-05-31 at 13:53 +0100, Bastien Nocera wrote:
> On Thu, 2007-05-31 at 13:36 +0100, Richard Hughes wrote:
> > Attached patch adds a kernel thread to do polling on Toshiba hardware.
> > 
> > Toshiba hardware is a little oddball, and does not provide ACPI events
> > on some key presses, typically Fn hotkey buttons. The key interface is
> > now polled, and events now matched to a list of toshiba specific
> > scancodes, and are squirted to userspace using the INPUT subsystem.
> > 
> > This means that toshiba laptops buttons "just work" without any
> > userspace daemon (using uinput) such as fnfx or bodges such as using a
> > userspace hal addon. Doing the polling in kernel is more efficient, and
> > makes stuff just work out of the box. You can assign the keys using
> > standard X keymaps, or using tools such as gnome-keybinding-properties.
> > 
> > This is similar to other patches sent for the thinkpad_acpi driver, and
> > is part of the "Unf*ck my keyboard" initiative to make multimedia keys
> > just work.
> > 
> > Changes from the first patch involve switching to a workqueue for the
> > polling, not breaking the spaces in "hotkeys_via_input" and also masking
> > out the fn key button up.
> 
> A couple of things:
> - use a switch statement in toshiba_deliver_button_event(), would look a
> bit cleaner.

Agree, done.

> - make the magic number #defines

Agree, done.

> - not sure if it's possible, but you should disable the workqueue
> altogether if nothing has the input device opened.

Do we get an event when the [input]->users value changes? If not, we
could do 1Hz (users > 0) checking when the input device is unclaimed,
and then switch to 4Hz polling when the input device is open.

> Hopefully we'll find a way to receive an interrupt, or some kind of
> event when the keys are pressed in the future, and completely avoid the
> polling. In the meantime, it should be minimised.

Nope, impossible AFAICS. The hardware is just broken. Windows XP has an
toshiba supplied daemon that polls, so I think we have to just bite the
bullet.

New patch attached.

Richard.

diff --git a/drivers/acpi/toshiba_acpi.c b/drivers/acpi/toshiba_acpi.c
index 3906d47..cf4ab76 100644
--- a/drivers/acpi/toshiba_acpi.c
+++ b/drivers/acpi/toshiba_acpi.c
@@ -27,13 +27,13 @@
  *		engineering the Windows drivers
  *	Yasushi Nagato - changes for linux kernel 2.4 -> 2.5
  *	Rob Miller - TV out and hotkeys help
- *
+ *	Richard Hughes - emit INPUT events, based on a patch from Daniel Silverstone
  *
  *  TODO
  *
  */
 
-#define TOSHIBA_ACPI_VERSION	"0.18"
+#define TOSHIBA_ACPI_VERSION	"0.19"
 #define PROC_INTERFACE_VERSION	1
 
 #include 
@@ -42,6 +42,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include 
 
@@ -99,6 +101,16 @@ MODULE_LICENSE("GPL");
 #define HCI_VIDEO_OUT_CRT		0x2
 #define HCI_VIDEO_OUT_TV		0x4
 
+/* key definitions */
+#define HCI_HKEY_MUTE			0x0101
+#define HCI_HKEY_BREAK			0x013b
+#define HCI_HKEY_SEARCH			0x013c
+#define HCI_HKEY_SUSPEND		0x013d
+#define HCI_HKEY_HIBERNATE		0x013e
+#define HCI_HKEY_BRIGHTNESSDOWN		0x0140
+#define HCI_HKEY_BRIGHTNESSUP		0x0141
+#define HCI_HKEY_WLAN			0x0142
+
 /* utility
  */
 
@@ -213,9 +225,15 @@ static acpi_status hci_read1(u32 reg, u32 * out1, u32 * result)
 
 static struct proc_dir_entry *toshiba_proc_dir /*= 0*/ ;
 static struct backlight_device *toshiba_backlight_device;
+static struct input_dev *toshiba_input;
 static int force_fan;
 static int last_key_event;
 static int key_event_valid;
+static int hotkeys_over_input = 1;
+static int hotkeys_check_per_sec = 2;
+
+module_param(hotkeys_over_input, uint, 0400);
+module_param(hotkeys_check_per_sec, uint, 0400);
 
 typedef struct _ProcItem {
 	const char *name;
@@ -443,27 +461,33 @@ static char *read_keys(char *p)
 	u32 hci_result;
 	u32 value;
 
-	if (!key_event_valid) {
-		hci_read1(HCI_SYSTEM_EVENT, &value, &hci_result);
-		if (hci_result == HCI_SUCCESS) {
-			key_event_valid = 1;
-			last_key_event = value;
-		} else if (hci_result == HCI_EMPTY) {
-			/* better luck next time */
-		} else if (hci_result == HCI_NOT_SUPPORTED) {
-			/* This is a workaround for an unresolved issue on
-			 * some machines where system events sporadically
-			 * become disabled. */
-			hci_write1(HCI_SYSTEM_EVENT, 1, &hci_result);
-			printk(MY_NOTICE "Re-enabled hotkeys\n");
-		} else {
-			printk(MY_ERR "Error reading hotkey status\n");
-			goto end;
+	if (!hotkeys_over_input) {
+		if (!key_event_valid) {
+			hci_read1(HCI_SYSTEM_EVENT, &value, &hci_result);
+			if (hci_result == HCI_SUCCESS) {
+key_event_valid = 1;
+last_key_event = value;
+			} else if (hci_result == HCI_EMPTY) {
+/* better luck next time */
+			} else if (hci_result == HCI_NOT_SUPPORTED) {
+/* This is a workaround for an unresolved issue on
+ * some machines where system events sporadically
+ * become disabled. */
+hci_write1(HCI_SYSTEM_EVENT, 1, &hci_result);
+printk(MY_NOTICE "Re-enabled hotkeys\n");
+			} else {
+prin

Re: Making KEY_UNKNOWN really useful to userland

2007-05-31 Thread Henrique de Moraes Holschuh
On Thu, 31 May 2007, Richard Hughes wrote:
> On Wed, 2007-05-30 at 21:53 -0300, Henrique de Moraes Holschuh wrote:
> > Maybe we can add that requirement and driver changes (if any, for all
> > I know most drivers might already be generating such events) for
> > 2.6.23?  I bet Richard would like that one a lot.  Richard?
> 
> To be honest, I've got lost in all the discussion about hotkeys. What is
> the plan for a typical acpi driver?
> 
> i.e. what do you want a driver to emit given two scenarios:
> 
> * A acpi event that is always the same on all models?
>KEY_BRIGHTNESSUP?

When one should be sent, yes.  It has the same problem as a volume
up/down/mute key that "just works" in firmware: you don't want to have two
things trying to mess with the display brightness every time a brightness
key is pressed.

SOME keys have the added aggravation that they really, really should be
sending ACPI events instead (brightness up/down is one: it is an ACPI 3.0a
event), but to really get that to work, we need to improve how ACPICA talks
to other ACPI modules to selectively disable some functions.

But that's not a problem that affects every hot key in every platform out
there, so I consider them to be the exceptions that define the rule :-p

> * an acpi event that is different on every laptop model?
>KEY_PROG1? KEY_UNKNOWN?

EV_KEY KEY_UNKNOWN + EV_MSC MSC_SCAN (or MSC_RAW??) + EV_SYN for *unmarked*
keys.  Then you just ask the user if he wants to map that scan code to some
other keycode in userspace, if you want.

For thinkpad-acpi, I don't send ACPI events for stuff I will be sending a
meaningful input event, just like you asked me to (and I don't send events
if nothing has the input device open).  So you get ACPI events if nothing is
listening to the input device, OR if the key is unmapped or mapped to
KEY_UNKNOWN.

I will send the scan code always in thinkpad-acpi, but that is hardly
necessary I suppose.

Note that the scan code is the same for press and release (it is the same
key).  So anything that wants to know if a key was pressed or released
better use the EV_KEY to know it (as it should, since it *is* in the same
event block for a damn good reason :p ).

Oh, and the thinkpad firmware 'weirdness' of needing one to enable/disable
OS hot key support still applies, of course (the hotkey mask).

> Tell me what do to, and I'll resync my trees and resubmit patches.

I am on it on the thinkpad-acpi side, so at least for that, you don't have
to worry.  I am still waiting an answer about which event is the correct one
to output scancodes, but the thinkpad-acpi GIT tree is already updated with
the above, tentively using MSC_SCAN to report scan codes.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Add INPUT support to toshiba_acpi

2007-05-31 Thread Richard Hughes
Attached patch adds a kernel thread to do polling on Toshiba hardware.

Toshiba hardware is a little oddball, and does not provide ACPI events
on some key presses, typically Fn hotkey buttons. The key interface is
now polled, and events now matched to a list of toshiba specific
scancodes, and are squirted to userspace using the INPUT subsystem.

This means that toshiba laptops buttons "just work" without any
userspace daemon (using uinput) such as fnfx or bodges such as using a
userspace hal addon. Doing the polling in kernel is more efficient, and
makes stuff just work out of the box. You can assign the keys using
standard X keymaps, or using tools such as gnome-keybinding-properties.

This is similar to other patches sent for the thinkpad_acpi driver, and
is part of the "Unf*ck my keyboard" initiative to make multimedia keys
just work.

Changes from the first patch involve switching to a workqueue for the
polling, not breaking the spaces in "hotkeys_via_input" and also masking
out the fn key button up.

 toshiba_acpi.c |  228 +++--
 1 file changed, 206 insertions(+), 22 deletions(-)

Signed-off-by: Richard Hughes <[EMAIL PROTECTED]>

--- origin/toshiba_acpi.c	2007-05-18 09:56:03.0 +0100
+++ toshiba_acpi.c	2007-05-31 13:24:02.0 +0100
@@ -27,13 +27,13 @@
  *		engineering the Windows drivers
  *	Yasushi Nagato - changes for linux kernel 2.4 -> 2.5
  *	Rob Miller - TV out and hotkeys help
- *
+ *	Richard Hughes - emit INPUT events, based on a patch from Daniel Silverstone
  *
  *  TODO
  *
  */
 
-#define TOSHIBA_ACPI_VERSION	"0.18"
+#define TOSHIBA_ACPI_VERSION	"0.19"
 #define PROC_INTERFACE_VERSION	1
 
 #include 
@@ -42,6 +42,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -213,9 +214,15 @@ static acpi_status hci_read1(u32 reg, u3
 
 static struct proc_dir_entry *toshiba_proc_dir /*= 0*/ ;
 static struct backlight_device *toshiba_backlight_device;
+static struct input_dev *toshiba_input;
 static int force_fan;
 static int last_key_event;
 static int key_event_valid;
+static int hotkeys_over_input = 1;
+static int hotkeys_check_per_sec = 2;
+
+module_param(hotkeys_over_input, uint, 0400);
+module_param(hotkeys_check_per_sec, uint, 0400);
 
 typedef struct _ProcItem {
 	const char *name;
@@ -443,27 +450,33 @@ static char *read_keys(char *p)
 	u32 hci_result;
 	u32 value;
 
-	if (!key_event_valid) {
-		hci_read1(HCI_SYSTEM_EVENT, &value, &hci_result);
-		if (hci_result == HCI_SUCCESS) {
-			key_event_valid = 1;
-			last_key_event = value;
-		} else if (hci_result == HCI_EMPTY) {
-			/* better luck next time */
-		} else if (hci_result == HCI_NOT_SUPPORTED) {
-			/* This is a workaround for an unresolved issue on
-			 * some machines where system events sporadically
-			 * become disabled. */
-			hci_write1(HCI_SYSTEM_EVENT, 1, &hci_result);
-			printk(MY_NOTICE "Re-enabled hotkeys\n");
-		} else {
-			printk(MY_ERR "Error reading hotkey status\n");
-			goto end;
+	if (!hotkeys_over_input) {
+		if (!key_event_valid) {
+			hci_read1(HCI_SYSTEM_EVENT, &value, &hci_result);
+			if (hci_result == HCI_SUCCESS) {
+key_event_valid = 1;
+last_key_event = value;
+			} else if (hci_result == HCI_EMPTY) {
+/* better luck next time */
+			} else if (hci_result == HCI_NOT_SUPPORTED) {
+/* This is a workaround for an unresolved issue on
+ * some machines where system events sporadically
+ * become disabled. */
+hci_write1(HCI_SYSTEM_EVENT, 1, &hci_result);
+printk(MY_NOTICE "Re-enabled hotkeys\n");
+			} else {
+printk(MY_ERR "Error reading hotkey status\n");
+goto end;
+			}
 		}
+	} else {
+		key_event_valid = 0;
+		last_key_event = 0;
 	}
 
 	p += sprintf(p, "hotkey_ready:%d\n", key_event_valid);
 	p += sprintf(p, "hotkey:  0x%04x\n", last_key_event);
+	p += sprintf(p, "hotkeys_via_input:   %d\n", hotkeys_over_input);
 
   end:
 	return p;
@@ -534,15 +547,121 @@ static acpi_status __exit remove_device(
 }
 
 static struct backlight_ops toshiba_backlight_data = {
-.get_brightness = get_lcd,
-.update_status  = set_lcd_status,
+	.get_brightness = get_lcd,
+	.update_status  = set_lcd_status,
 };
 
+static void toshiba_keys_fire(struct work_struct *work);
+static struct workqueue_struct *toshiba_keys_wq;
+static DECLARE_DELAYED_WORK(toshiba_keys_work, toshiba_keys_fire);
+
+static void toshiba_deliver_button_event(u32 value)
+{
+	int keycode = KEY_UNKNOWN;
+	int key_down = 1;
+
+	if (!toshiba_input)
+		return;
+
+	/* translate MSB to key up */
+	if (value & 0x80) {
+		value &= ~0x80;
+		key_down = 0;
+	}
+
+	/* ignore FN on its own */
+	if (value == 0x0100)
+		return;
+
+	/* translate keys to keycodes */
+	if (value == 0x101)
+		keycode = KEY_MUTE;
+	else if (value == 0x13b)
+		keycode = KEY_BREAK;
+	else if (value == 0x13c)
+		keycode = KEY_SEARCH;
+	else if (value == 0x13d)
+		keycode = KEY_SLEEP;
+	else if (value == 0x13e)
+		keycode = KEY_SUSPEND

Re: Making KEY_UNKNOWN really useful to userland

2007-05-31 Thread Richard Hughes
On Wed, 2007-05-30 at 21:53 -0300, Henrique de Moraes Holschuh wrote:
> Maybe we can add that requirement and driver changes (if any, for all
> I know most drivers might already be generating such events) for
> 2.6.23?  I bet Richard would like that one a lot.  Richard?

To be honest, I've got lost in all the discussion about hotkeys. What is
the plan for a typical acpi driver?

i.e. what do you want a driver to emit given two scenarios:

* A acpi event that is always the same on all models?
   KEY_BRIGHTNESSUP?

* an acpi event that is different on every laptop model?
   KEY_PROG1? KEY_UNKNOWN?

Tell me what do to, and I'll resync my trees and resubmit patches.

Thanks.


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


Re: [KJ] [PATCH] drivers/acpi: sizeof/sizeof array size calculations replaced with ARRAY_SIZE

2007-05-31 Thread Christoph Hellwig
On Wed, May 30, 2007 at 03:25:05PM -0400, Len Brown wrote:
>  
> > > Any reason to not just replace ACPI_RSD_TABLE_SIZE with ARRAY_SIZE?
> 
> Probably because ARRAY_SIZE doesn't exist in ACPICA, which is
> where this code comes from...
> 
> When we change syntax in ACPICA files in Linux to make it more "beautiful",
> then it creates more work for me -- as forever on, that syntax difference
> must be manually compared to upstream ACPICA and Linux -- and that syntax
> difference causes upstream patches to no longer apply and require
> hand merging.

Or we could stop that ACPCICA crap ASAP.  The acpi code not only looks
like crap because of that but it's buggy as hell now.

Intel is claiming how great linux is and how supportive is to it, but
can't even write proper code for their abomination of a firmware standard.
This is really more than dishonest.  Please take patches to get acpi into
shape and stop this complaining.

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


Re: smartbattery status on Laptop

2007-05-31 Thread Alexey Starikovskiy
Please open bug at bugzilla.kernel.org and attach acpidump output.

Thanks,
Alex.

John Floyd пишет:
> Hi,
> 
> Have a specialised weatherproof laptop with a smartbattery running FC6
>  latest kernel. I cannot get battey monitoring going. Have loaded
>  modules i2c_ec and sbs with out any errors.
> 
> log/messages indicate that the EC driver loads with
> ACPI: EC HC smbus [SMB1]
> 
> There is nothing from the sbs module though it is loaded and
> the /proc/acpi/battery directory is empty.
> 
> The smartbattery can be recognized. If I run the following command ...
> 
> smartbattery 1 (1 is to specify the i2c dev,
> needs i2c-dev module loaded)
> 
> then I get battery status, which behaves properly (changes if I unplug
>  AC etc). It shows Battery name, status , charge or discharge current,
>  time to full/left, temperature etc.  The smartbattery command came
>  with the initial code exploring the ec-smbus interface a couple of
>  years ago by Bruno Ducrot.
> 
> However nothing shows up in the proc/acpi/battery directory under the
>  sbs module, and therefore the gui battery indicators do not work
> 
> Any suggestions?
> 
> 
> Love to get this working in the gui.
> 
> 
> John
> --
> John Floyd
> Senior Natural Resource Officer
> Department of Environment and Climate Change
> Phone +61-2-9895-5956
> Fax   +61-2-9895-7263
> [EMAIL PROTECTED]
> 
> ---
> -
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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


smartbattery status on Laptop

2007-05-31 Thread John Floyd
Hi,

Have a specialised weatherproof laptop with a smartbattery running FC6
 latest kernel. I cannot get battey monitoring going. Have loaded
 modules i2c_ec and sbs with out any errors.

log/messages indicate that the EC driver loads with
ACPI: EC HC smbus [SMB1]

There is nothing from the sbs module though it is loaded and
the /proc/acpi/battery directory is empty.

The smartbattery can be recognized. If I run the following command ...

smartbattery 1 (1 is to specify the i2c dev,
needs i2c-dev module loaded)

then I get battery status, which behaves properly (changes if I unplug
 AC etc). It shows Battery name, status , charge or discharge current,
 time to full/left, temperature etc.  The smartbattery command came
 with the initial code exploring the ec-smbus interface a couple of
 years ago by Bruno Ducrot.

However nothing shows up in the proc/acpi/battery directory under the
 sbs module, and therefore the gui battery indicators do not work

Any suggestions?


Love to get this working in the gui.


John
--
John Floyd
Senior Natural Resource Officer
Department of Environment and Climate Change
Phone +61-2-9895-5956
Fax   +61-2-9895-7263
[EMAIL PROTECTED]

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