Re: [PATCH 2.6.29] eeepc-laptop: report brightness control events via the input layer

From: Darren Salt
Date: Sat Apr 04 2009 - 18:56:46 EST


I demand that Corentin Chary may or may not have written...

>>> How can brn be -2?
>> If notify_brn() wasn't called, it will be.

> Oh, yeah, I miss the if() before notify_brn()

Easily missed. ;-)

>>> And why -2?
>> Because notify_brn() won't return it (and if it ever does, it needs to be
>> fixed). (Yes, I know, "magic numbers" and all that...)

> Maybe a negative known error code could be used here

I see the following:

* read_acpi_int() returns 0 and writes the brightness setting to *val, or
returns -1 and writes -1 to *val if the ACPI call failed.

* get_acpi() returns whatever was written to value by read_acpi_int(), or
-ENODEV if there's no ACPI method which can be called.

* read_brightness is a trivial wrapper for get_acpi().

* notify_brn() stores the result of read_brightness and returns the
previously-stored value, so it can store then later return -ENODEV, -1 or
the brightness setting.

This makes -ENODEV a suitable value. Replacing that -1 with something other
than -ENODEV might be good, but I don't think that that really matters right
now (though I've replaced the "!= -1" in the original version of the patch
with "< 0").

Revised patch follows...

---
eeepc-laptop: report brightness control events via the input layer

This maps the brightness control events to one of two keys, either
KEY_BRIGHTNESSDOWN or KEY_BRIGHTNESSUP, as needed.

Some mapping has to be done due to the fact that the BIOS reports them as
<base value> + <current brightness index>; the selection is done according to
the sign of the change in brightness (if this is 0, no keypress is reported).

(Ref. http://lists.alioth.debian.org/pipermail/debian-eeepc-devel/2009-April/002001.html)

Signed-off-by: Darren Salt <linux@xxxxxxxxxxxxxxxxxxxxxxxxxxx>

--- a/drivers/platform/x86/eeepc-laptop.c 2009-03-24 17:32:56.000000000 +0000
+++ b/drivers/platform/x86/eeepc-laptop.c 2009-04-03 23:37:34.000000000 +0100
@@ -166,6 +166,8 @@
{KE_KEY, 0x1b, KEY_ZOOM },
{KE_KEY, 0x1c, KEY_PROG2 },
{KE_KEY, 0x1d, KEY_PROG3 },
+ {KE_KEY, NOTIFY_BRN_MIN, KEY_BRIGHTNESSDOWN },
+ {KE_KEY, NOTIFY_BRN_MIN + 2, KEY_BRIGHTNESSUP },
{KE_KEY, 0x30, KEY_SWITCHVIDEOMODE },
{KE_KEY, 0x31, KEY_SWITCHVIDEOMODE },
{KE_KEY, 0x32, KEY_SWITCHVIDEOMODE },
@@ -512,11 +514,17 @@
return 0;
}

-static void notify_brn(void)
+static int notify_brn(void)
{
+ /* returns the *previous* brightness, or -1 */
struct backlight_device *bd = eeepc_backlight_device;
if (bd)
+ {
+ int old = bd->props.brightness;
bd->props.brightness = read_brightness(bd);
+ return old;
+ }
+ return -1;
}

static void eeepc_rfkill_notify(acpi_handle handle, u32 event, void *data)
@@ -558,17 +566,33 @@
{
static struct key_entry *key;
u16 count;
+ int brn = -ENODEV;

if (!ehotk)
return;
if (event >= NOTIFY_BRN_MIN && event <= NOTIFY_BRN_MAX)
- notify_brn();
+ brn = notify_brn();
count = ehotk->event_count[event % 128]++;
acpi_bus_generate_proc_event(ehotk->device, event, count);
acpi_bus_generate_netlink_event(ehotk->device->pnp.device_class,
dev_name(&ehotk->device->dev), event,
count);
if (ehotk->inputdev) {
+ if (brn != -ENODEV) {
+ /* brightness-change events need special
+ * handling for conversion to key events
+ */
+ if (brn < 0)
+ brn = event;
+ else
+ brn += NOTIFY_BRN_MIN;
+ if (event < brn)
+ event = NOTIFY_BRN_MIN; /* brightness down */
+ else if (event > brn)
+ event = NOTIFY_BRN_MIN + 2; /* ... up */
+ else
+ event = NOTIFY_BRN_MIN + 1; /* ... unchanged */
+ }
key = eepc_get_entry_by_scancode(event);
if (key) {
switch (key->type) {

--
| Darren Salt | linux or ds at | nr. Ashington, | Toon
| RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army
| + Burn less waste. Use less packaging. Waste less. USE FEWER RESOURCES.

Worse things happen in C.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/