udev keymaps: support for force_release quirk

udev keymaps: support for force_release quirk

[Johannes Stezenbach]

Hi,

(please keep me Cc'd, I'm not subscribed)

following existing practice I wanted to add a quirk
for the hotkeys of the Samsung N130 in the same way
as it was done for the NC10, but was informed that
this method is no longer accepted since the quirk can
be handled in userspace using the force_release
sysfs attribute introduced in 2.6.32-rc.

http://lkml.org/lkml/2009/11/16/243

Usually it is /sys/devices/platform/i8042/serio0/force_release,
with keycodes in decimal. The default is "369-370"
(HANGEUL + HANJA keys).  The correct value for N130
would be "130-132,134,136-137,179,247,249,369-370".

(If I just add N130 to the regex in the NC10 line
in /lib/udev/rules.d/95-keymap.rules, pressing
some of the hotkeys generates an endless stream
of key events which causes the keyboard to go
dead in X. It can be brought back to life by
switching to a Linux console with Ctrl-Alt-F1 and back.)


I guess it would make sense to add support for
the forced release attribute into
extras/keymap/keymap.c in such a way that
the keymap files (e.g. samsung-other)
can have an optional third column with flags.
Since many models share samsung-other, but only
three had the force_release handled in to the kernel
I'd also add a flag to keymap.c so that the
forced_release flag is only applied when the --force-release/-f
switch is present.  The we can add two lines to 95-keymap.rules,
one for models which need the quirk and one fo the others.
On an older kernel which doesn't support the force_release
sysfs attribute the flag would be silently ignored.

Entries in samsung-other would then look like this:

0x82 switchvideomode force_release # Fn+F4 CRT/LCD (high keycode: "displaytoggle")
0x83 battery force_release,some_other_flag # Fn+F2
0x84 prog1 # Fn+F5 backlight on/off

(if there is a need for some_other_flag in the future)


Comments?  Should I go forward and try to implement it?


Thanks
Johannes

Re: udev keymaps: support for force_release quirk

[Martin Pitt]

Hello Johannes,

Johannes Stezenbach [2009-11-26 18:39 +0100]:
> I guess it would make sense to add support for the forced release
> attribute into extras/keymap/keymap.c

Indeed, that'd need to happen now that it gets pushed out of the
kernel.

> in such a way that the keymap files (e.g. samsung-other) can have an
> optional third column with flags.

This sounds good.

> Since many models share samsung-other, but only
> three had the force_release handled in to the kernel
> I'd also add a flag to keymap.c so that the
> forced_release flag is only applied when the --force-release/-f
> switch is present.

I don't understand this. We need separate rules for the quirked vs.
non-quirked models anyway, so why would we need this -f switch if we
already specify it in the rules? I think I see what you try to do, but
IMHO it would just make matters more confusing, and not really help to
reduce rules either.

> The we can add two lines to 95-keymap.rules,
> one for models which need the quirk and one fo the others.
> On an older kernel which doesn't support the force_release
> sysfs attribute the flag would be silently ignored.
> 
> Entries in samsung-other would then look like this:
> 
> 0x82 switchvideomode force_release # Fn+F4 CRT/LCD (high keycode: "displaytoggle")
> 0x83 battery force_release,some_other_flag # Fn+F2
> 0x84 prog1 # Fn+F5 backlight on/off

This looks good.

> Should I go forward and try to implement it?

That would be great!

Thanks,

Martin

-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)

Re: udev keymaps: support for force_release quirk

[Johannes Stezenbach]

On Fri, Nov 27, 2009 at 04:33:51PM +0100, Martin Pitt wrote:
> Johannes Stezenbach [2009-11-26 18:39 +0100]:
> 
> > Since many models share samsung-other, but only
> > three had the force_release handled in to the kernel
> > I'd also add a flag to keymap.c so that the
> > forced_release flag is only applied when the --force-release/-f
> > switch is present.
> 
> I don't understand this. We need separate rules for the quirked vs.
> non-quirked models anyway, so why would we need this -f switch if we
> already specify it in the rules? I think I see what you try to do, but
> IMHO it would just make matters more confusing, and not really help to
> reduce rules either.

Well, 95-keymap.rules currently has:

ENV{DMI_VENDOR}="[sS][aA][mM][sS][uU][nN][gG]*", ATTR{[dmi/id]product_name}="*NC10*|*NC20*|*SP55S*|*SQ45S70S*|*SX60P*|*SX22S*|*SX30S*|*R59P/R60P/R61P*|*Q210*|*Q310*|*X05*|*P560*|*R560*", RUN+="keymap $name samsung-other"

But the kernel drivers/input/keyboard/atkbd.c only has the
force_release quirk for NC10, NC20 and SQ45S70S.  I simply
don't know if at does harm to apply the force_release quirk
to the other models which share samsung-other.

Another solution than the proposed -f switch would be
to copy samsung-other to samsung-other-force-release and
add the force_release flag only to the latter.

> > Should I go forward and try to implement it?
> 
> That would be great!

OK, I'll try to work something out over the weekend.


Thanks
Johannes

Re: udev keymaps: support for force_release quirk

[Martin Pitt]

Hello Johannes,

Johannes Stezenbach [2009-11-27 16:48 +0100]:
> I simply don't know if at does harm to apply the force_release quirk
> to the other models which share samsung-other.

I don't know for sure either, but if it was harmless, the kernel could
just unconditionally do it for all keys and not bother about this
mechanism at all. So I guess it is important after all.

> Another solution than the proposed -f switch would be to copy
> samsung-other to samsung-other-force-release and add the
> force_release flag only to the latter.

Right, that's what I had in mind.

Thanks, Martin
-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)

Re: udev keymaps: support for force_release quirk

[Greg KH]

On Thu, Nov 26, 2009 at 06:39:12PM +0100, Johannes Stezenbach wrote:
> Hi,
> 
> (please keep me Cc'd, I'm not subscribed)
> 
> following existing practice I wanted to add a quirk
> for the hotkeys of the Samsung N130 in the same way
> as it was done for the NC10, but was informed that
> this method is no longer accepted since the quirk can
> be handled in userspace using the force_release
> sysfs attribute introduced in 2.6.32-rc.

What specific quirk are you needing for the N130?  I have one here and
it seems to work just fine.

thanks,

greg k-h

Re: udev keymaps: support for force_release quirk

[Johannes Stezenbach]

On Sat, Nov 28, 2009 at 11:14:54AM -0800, Greg KH wrote:
> On Thu, Nov 26, 2009 at 06:39:12PM +0100, Johannes Stezenbach wrote:
> > 
> > (please keep me Cc'd, I'm not subscribed)
> > 
> > following existing practice I wanted to add a quirk
> > for the hotkeys of the Samsung N130 in the same way
> > as it was done for the NC10, but was informed that
> > this method is no longer accepted since the quirk can
> > be handled in userspace using the force_release
> > sysfs attribute introduced in 2.6.32-rc.
> 
> What specific quirk are you needing for the N130?  I have one here and
> it seems to work just fine.

Well, by default most of the Fn keys have no keycode assigned,
so no problem except that the Fn keys don't work.

[  163.138031] atkbd.c: Unknown key pressed (translated set 2, code 0xf9 on isa0060/serio0).
[  163.138052] atkbd.c: Use 'setkeycodes e079 <keycode>' to make it known.
[  164.721926] atkbd.c: Unknown key pressed (translated set 2, code 0xf7 on isa0060/serio0).
[  164.721947] atkbd.c: Use 'setkeycodes e077 <keycode>' to make it known.
etc.

When I add the N130 in /lib/udev/rules.d/95-keymap.rules then
they keycodes get assigned, but some of the keys don't
generate release events, which can cause an endless stream
of keyevents in X, you can see them with xev (happens only
the first time a key is pressed, after that that key is dead
since there is no release event in between; the stream of events
stops when any opther key is pressed).  This makes e.g. the
backlight control keys useless.

For Fn-F4 the behaviour is that the keyboard goes dead in X,
you need to switch to the Linux console and back to X to revive it.
Maybe because it generates FocusOut events, no idea what
X is doing here:

FocusOut event, serial 33, synthetic NO, window 0x2800001,
    mode NotifyGrab, detail NotifyAncestor

FocusIn event, serial 33, synthetic NO, window 0x2800001,
    mode NotifyUngrab, detail NotifyAncestor

KeymapNotify event, serial 33, synthetic NO, window 0x0,
    keys:  33  0   0   0   0   0   0   0   0   0   0   0   0   0   0   0   
           0   0   0   0   0   0   0   0   0   0   0   0   0   0   0   0   

(the keycode assigned to Fn-F4 is KEY_SWITCHVIDEOMODE)

All this isn't new, the same issues existed for the NC10 and were
fixed by adding the force_release quirk.


Thanks
Johannes

Re: udev keymaps: support for force_release quirk

[Johannes Stezenbach]

On Fri, Nov 27, 2009 at 04:57:48PM +0100, Martin Pitt wrote:
> Johannes Stezenbach [2009-11-27 16:48 +0100]:
> > I simply don't know if at does harm to apply the force_release quirk
> > to the other models which share samsung-other.
> 
> I don't know for sure either, but if it was harmless, the kernel could
> just unconditionally do it for all keys and not bother about this
> mechanism at all. So I guess it is important after all.
> 
> > Another solution than the proposed -f switch would be to copy
> > samsung-other to samsung-other-force-release and add the
> > force_release flag only to the latter.
> 
> Right, that's what I had in mind.

Below is a first cut. It works but it has the following
potential problems:

- I used %n in sscanf even though the man page warns:

  "The  C  standard says:  "Execution  of  a %n directive
  does not increment the assignment count returned at the
  completion of execution" but the Corrigendum seems to
  contradict this.  Probably it  is  wise not to make any
  assumptions on the effect of %n conversions on the return value."
  http://www.kernel.org/doc/man-pages/online/pages/man3/sscanf.3.html

  But IMHO the C standard (C99 as well as N1336) is pretty
  clear. I've no idea if all libcs implement it correctly
  though. It works with glibc.

- For the force_release attribute we need DEVPATH.
  I used getenv() and added a hardcoded /sys prefix
  (like done e.g. in input_id.c or firmware.sh).
  Alternatively we could pass $sys$devpath on the
  commandline. Or we could use libudev.

- To get from the DEVPATH to the force_release attribute
  we need to go two parents up. I just used the
  "device" symlinks:
    DEVPATH:  eventX
    DEVPATH/device: inputX
    DEVPATH/device/device: serioY
    (e.g. /sys/devices/platform/i8042/serio0/input/input2/event2/device/device/)
  Alternatively we could use udev_device_get_parent()
  from libudev.

- For setting the force_release attr I did not bother to
  parse the previous setting. I just add potential new
  scancodes and let the kernel sort it out.

Let me know what you think.


Thanks
Johannes

diff --git a/extras/keymap/keymap.c b/extras/keymap/keymap.c
index b565c33..0a5523b 100644
--- a/extras/keymap/keymap.c
+++ b/extras/keymap/keymap.c
@@ -41,6 +41,86 @@ const struct key* lookup_key (const char *str, unsigned int len);
 
 #define MAX_SCANCODES 1024
 
+static int n_quirks, n_quirks_alloc;
+static int *quirks;
+
+static void add_force_release_quirk(int scancode)
+{
+	if (n_quirks >= n_quirks_alloc) {
+		n_quirks_alloc = n_quirks_alloc ? n_quirks_alloc * 2 : 32;
+		quirks = realloc(quirks, n_quirks_alloc);
+		if (!quirks) {
+			fprintf(stderr, "error: cannot allocate %d bytes\n", n_quirks_alloc);
+			exit(1);
+		}
+	}
+	quirks[n_quirks++] = scancode;
+}
+
+static int force_relase_attr_open(void)
+{
+	int fd;
+	char fn[PATH_MAX], *env;
+
+	strcpy(fn, "/sys");
+	env = getenv("DEVPATH");
+	if (!env) {
+		fprintf(stderr, "error: DEVPATH not set\n");
+		return -1;
+	}
+	strncat(fn, env, sizeof(fn));
+	strncat(fn, "/device/device/force_release", sizeof(fn));
+
+	if ((fd = open(fn, O_RDWR)) < 0) {
+		fprintf(stderr, "error: open('%s'): %m\n", fn);
+		return -1;
+	}
+	return fd;
+}
+
+static int set_quirks(void)
+{
+	int q, fd, bytes;
+	size_t i;
+	char force_release[1024], scancode[20];;
+
+	if (!n_quirks)
+		return 0;
+
+	fd = force_relase_attr_open();
+	if (fd = -1)
+		return -1;
+	bytes = read(fd, force_release, sizeof(force_release) - 1);
+	if (bytes = -1) {
+		fprintf(stderr, "error: read force_release attribute %m\n");
+		close(fd);
+		return -1;
+	}
+	i = bytes;
+	force_release[i] = '\0';
+	if (i && force_release[i - 1] = '\n')
+		force_release[--i] = '\0';
+	fprintf(stderr, "info: old force_release attribute '%s'\n", force_release);
+	for (q = 0; q < n_quirks; q++) {
+		fprintf(stderr, "adding force_release quirk for scancode %d\n", quirks[q]);
+		if (i && (i + 2 < sizeof(force_release)))
+			strcat(force_release + i++, ",");
+		snprintf(scancode, sizeof(scancode), "%d", quirks[q]);
+		strncat(force_release + i, scancode, sizeof(force_release) - i);
+		i += strlen(scancode);
+	}
+	fprintf(stderr, "info: new force_release attribute '%*s'\n", i, force_release);
+	lseek(fd, 0, SEEK_SET);
+	bytes = write(fd, force_release, i);
+	if (bytes = -1) {
+		fprintf(stderr, "error: write force_release attribute %m\n");
+		close(fd);
+		return -1;
+	}
+	close(fd);
+	return 0;
+}
+
 static int evdev_open(const char *dev)
 {
 	int fd;
@@ -194,8 +274,8 @@ static int merge_table(int fd, const char *filename) {
 	}
 
 	while (!feof(f)) {
-		char s[256], *p;
-		int scancode, new_keycode, old_keycode;
+		char s[256], flags[100], *p;
+		int scancode, new_keycode, old_keycode, n;
 
 		if (!fgets(s, sizeof(s), f))
 			break;
@@ -205,11 +285,11 @@ static int merge_table(int fd, const char *filename) {
 		if (*p = '#' || *p = '\n')
 			continue;
 
-		if (sscanf(p, "%i %i", &scancode, &new_keycode) != 2) {
+		if (sscanf(p, "%i %i %n", &scancode, &new_keycode, &n) != 2) {
 			char t[105] = "KEY_UNKNOWN";
 			const struct key *k;
 
-			if (sscanf(p, "%i %100s", &scancode, t+4) != 2) {
+			if (sscanf(p, "%i %100s %n", &scancode, t+4, &n) != 2) {
 				fprintf(stderr, "WARNING: Parse failure at line %i, ignoring.\n", line);
 				r = -1;
 				continue;
@@ -224,6 +304,10 @@ static int merge_table(int fd, const char *filename) {
 			new_keycode = k->id;
 		}
 
+		if (sscanf(p+n, "%99s", flags) = 1) {
+			if (strcmp(flags, "force_release") = 0)
+				add_force_release_quirk(scancode);
+		}
 
 		if ((old_keycode = evdev_get_keycode(fd, scancode, 0)) < 0) {
 			r = -1;
@@ -369,6 +453,7 @@ int main(int argc, char **argv)
 	/* two arguments (device, mapfile): set map file */
 	if (argc = optind+2) {
 		merge_table(fd, default_keymap_path(argv[optind+1]));
+		set_quirks();
 		return 0;
 	}
 
diff --git a/extras/keymap/keymaps/samsung-other-force-release b/extras/keymap/keymaps/samsung-other-force-release
new file mode 100644
index 0000000..67decb2
--- /dev/null
+++ b/extras/keymap/keymaps/samsung-other-force-release
@@ -0,0 +1,14 @@
+0x74 prog1 # User key
+0x75 www
+0x78 mail
+0x82 switchvideomode force_release # Fn+F4 CRT/LCD (high keycode: "displaytoggle")
+0x83 battery force_release # Fn+F2
+0x84 prog1 force_release # Fn+F5 backlight on/off
+0x86 wlan force_release # Fn+F9
+0x88 brightnessup force_release # Fn-Up
+0x89 brightnessdown force_release # Fn-Down
+0xB1 prog2 # Fn+F7 run Samsung Magic Doctor (keypressed event is generated twice)
+0xB3 prog3 force_release # Fn+F8 switch power mode (battery/dynamic/performance)
+0xB4 wlan # Fn+F9 (X60P)
+0xF7 f22 force_release # Fn+F10 Touchpad on
+0xF9 f22 force_release # Fn+F10 Touchpad off

Re: udev keymaps: support for force_release quirk

[Kay Sievers]

On Tue, Dec 1, 2009 at 00:27, Johannes Stezenbach <js@sig21.net> wrote:
> +       strncat(fn, "/device/device/force_release", sizeof(fn));

You can only select a parent device by it's subsystem/devtype. Any
hardcoded assumption about the order of parents is wrong, and the
"device" link is deprecated, it must not appear anywhere in udev code.

Thanks,
Kay

Re: udev keymaps: support for force_release quirk

[Dmitry Torokhov]

On Tue, Dec 01, 2009 at 02:28:06AM +0100, Kay Sievers wrote:
> On Tue, Dec 1, 2009 at 00:27, Johannes Stezenbach <js@sig21.net> wrote:
> > +       strncat(fn, "/device/device/force_release", sizeof(fn));
> 
> You can only select a parent device by it's subsystem/devtype. Any
> hardcoded assumption about the order of parents is wrong, and the
> "device" link is deprecated, it must not appear anywhere in udev code.
> 

Maybe the quirk should fire up for /sys/device/platform/i8043/serioX
devices to begin with and not go backwards from the child event device?

-- 
Dmitry

Re: udev keymaps: support for force_release quirk

[Kay Sievers]

On Tue, Dec 1, 2009 at 02:37, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Tue, Dec 01, 2009 at 02:28:06AM +0100, Kay Sievers wrote:
>> On Tue, Dec 1, 2009 at 00:27, Johannes Stezenbach <js@sig21.net> wrote:
>> > +       strncat(fn, "/device/device/force_release", sizeof(fn));
>>
>> You can only select a parent device by it's subsystem/devtype. Any
>> hardcoded assumption about the order of parents is wrong, and the
>> "device" link is deprecated, it must not appear anywhere in udev code.
>>
>
> Maybe the quirk should fire up for /sys/device/platform/i8043/serioX
> devices to begin with and not go backwards from the child event device?

The hierarchy (/sys/devices/) of devices may change, for some device
types even at runtime. It's usually not safe to make assumptions about
the hierarchy. In this case it will probably never change for the
platform devices, but it's still safer to lookup devices from the flat
classification directories like: /sys/bus/serio/devices/serioX.

Reaching the device from the child device should be as easy as:
udev_device_get_parent_with_subsystem_devtype(..., "serio", NULL).

Thanks,
Kay

Re: udev keymaps: support for force_release quirk

[Johannes Stezenbach]

On Mon, Nov 30, 2009 at 05:37:04PM -0800, Dmitry Torokhov wrote:
> On Tue, Dec 01, 2009 at 02:28:06AM +0100, Kay Sievers wrote:
> > On Tue, Dec 1, 2009 at 00:27, Johannes Stezenbach <js@sig21.net> wrote:
> > > +       strncat(fn, "/device/device/force_release", sizeof(fn));
> > 
> > You can only select a parent device by it's subsystem/devtype. Any
> > hardcoded assumption about the order of parents is wrong, and the
> > "device" link is deprecated, it must not appear anywhere in udev code.

I have to admit I have no clue how else to do it.  I'd be glad
to hear your advice.

> Maybe the quirk should fire up for /sys/device/platform/i8043/serioX
> devices to begin with and not go backwards from the child event device?

Good point.  Maybe it doesn't make sense to handle the quirk in
keymap.c then, but rather add its own utility to be called
from a seperate udev rule for SUBSYSTEM="serio"?

If yes, does it still make sense to add the force_release
flags to the keymap files, or is it better to use a seperate
file which just lists the keys which should be added to
the force_release quirk?


Thanks
Johannes

Re: udev keymaps: support for force_release quirk

[Kay Sievers]

On Tue, Dec 1, 2009 at 02:52, Johannes Stezenbach <js@sig21.net> wrote:
> On Mon, Nov 30, 2009 at 05:37:04PM -0800, Dmitry Torokhov wrote:
>> On Tue, Dec 01, 2009 at 02:28:06AM +0100, Kay Sievers wrote:
>> > On Tue, Dec 1, 2009 at 00:27, Johannes Stezenbach <js@sig21.net> wrote:
>> > > +       strncat(fn, "/device/device/force_release", sizeof(fn));
>> >
>> > You can only select a parent device by it's subsystem/devtype. Any
>> > hardcoded assumption about the order of parents is wrong, and the
>> > "device" link is deprecated, it must not appear anywhere in udev code.
>
> I have to admit I have no clue how else to do it.  I'd be glad
> to hear your advice.

For udev it would be something like:
  udev_device_get_parent_with_subsystem_devtype(..., "serio", NULL);

To find the first parent device which is a device on the "serio" bus.

Kay

Re: udev keymaps: support for force_release quirk

[Johannes Stezenbach]

On Tue, Dec 01, 2009 at 02:55:27AM +0100, Kay Sievers wrote:
> On Tue, Dec 1, 2009 at 02:52, Johannes Stezenbach <js@sig21.net> wrote:
> > On Mon, Nov 30, 2009 at 05:37:04PM -0800, Dmitry Torokhov wrote:
> >> On Tue, Dec 01, 2009 at 02:28:06AM +0100, Kay Sievers wrote:
> >> > On Tue, Dec 1, 2009 at 00:27, Johannes Stezenbach <js@sig21.net> wrote:
> >> > > +       strncat(fn, "/device/device/force_release", sizeof(fn));
> >> >
> >> > You can only select a parent device by it's subsystem/devtype. Any
> >> > hardcoded assumption about the order of parents is wrong, and the
> >> > "device" link is deprecated, it must not appear anywhere in udev code.
> >
> > I have to admit I have no clue how else to do it.  I'd be glad
> > to hear your advice.
> 
> For udev it would be something like:
>   udev_device_get_parent_with_subsystem_devtype(..., "serio", NULL);
> 
> To find the first parent device which is a device on the "serio" bus.

I saw your other mail the moment after I sent mine...

I'll wait for Martin's comments but at the moment I'm thinking
it might be easier and cleaner with the seperate utility
and a rule for SUBSYSTEM="serio".  Because, on second thought,
in theory there could be keys which need the force_release quirk
but no keycode remapping?  No idea if that's the case in practice,
and it could be handled with a dummy keycode assignment, but
that's unclean. Maybe

  ./force_release /sys/devices/platform/i8042/serio0 samsung-other-force-release

with samsung-other-force-release looking like

  0x82 # Fn+F4 CRT/LCD (high keycode: "displaytoggle")
  0x83 # Fn+F2
  0x84 # Fn+F5 backlight on/off
  ...

and rules like

  ACTION!="add", GOTO="force_release_end"
  SUBSYSTEM!="serio", GOTO="force_release_end"
  KERNEL!="serio*", GOTO="force_release_end"
  DRIVERS!="atkbd", GOTO="force_release_end"

  ENV{DMI_VENDOR}="[sS][aA][mM][sS][uU][nN][gG]*", ATTR{[dmi/id]product_name}="*N130*", RUN+"force_release $sys$devname samsung-other-force-release"

  LABEL="force_release_end"

Would that work?


Thanks
Johannes

Re: udev keymaps: support for force_release quirk

[Dmitry Torokhov]

On Tue, Dec 01, 2009 at 03:21:20AM +0100, Johannes Stezenbach wrote:
> On Tue, Dec 01, 2009 at 02:55:27AM +0100, Kay Sievers wrote:
> > On Tue, Dec 1, 2009 at 02:52, Johannes Stezenbach <js@sig21.net> wrote:
> > > On Mon, Nov 30, 2009 at 05:37:04PM -0800, Dmitry Torokhov wrote:
> > >> On Tue, Dec 01, 2009 at 02:28:06AM +0100, Kay Sievers wrote:
> > >> > On Tue, Dec 1, 2009 at 00:27, Johannes Stezenbach <js@sig21.net> wrote:
> > >> > > +       strncat(fn, "/device/device/force_release", sizeof(fn));
> > >> >
> > >> > You can only select a parent device by it's subsystem/devtype. Any
> > >> > hardcoded assumption about the order of parents is wrong, and the
> > >> > "device" link is deprecated, it must not appear anywhere in udev code.
> > >
> > > I have to admit I have no clue how else to do it.  I'd be glad
> > > to hear your advice.
> > 
> > For udev it would be something like:
> >   udev_device_get_parent_with_subsystem_devtype(..., "serio", NULL);
> > 
> > To find the first parent device which is a device on the "serio" bus.
> 
> I saw your other mail the moment after I sent mine...
> 
> I'll wait for Martin's comments but at the moment I'm thinking
> it might be easier and cleaner with the seperate utility
> and a rule for SUBSYSTEM="serio".  Because, on second thought,
> in theory there could be keys which need the force_release quirk
> but no keycode remapping?  No idea if that's the case in practice,
> and it could be handled with a dummy keycode assignment, but
> that's unclean. Maybe
> 
>   ./force_release /sys/devices/platform/i8042/serio0 samsung-other-force-release
> 
> with samsung-other-force-release looking like
> 
>   0x82 # Fn+F4 CRT/LCD (high keycode: "displaytoggle")
>   0x83 # Fn+F2
>   0x84 # Fn+F5 backlight on/off
>   ...
> 
> and rules like
> 
>   ACTION!="add", GOTO="force_release_end"
>   SUBSYSTEM!="serio", GOTO="force_release_end"
>   KERNEL!="serio*", GOTO="force_release_end"
>   DRIVERS!="atkbd", GOTO="force_release_end"
> 
>   ENV{DMI_VENDOR}="[sS][aA][mM][sS][uU][nN][gG]*", ATTR{[dmi/id]product_name}="*N130*", RUN+"force_release $sys$devname samsung-other-force-release"
> 
>   LABEL="force_release_end"
> 
> Would that work?
>

Yes, I would go with separate utilities and separate maps. They are
conceptually different, work on different devices and map is not
necessarily the same. It will be much cleaner this way.

-- 
Dmitry

Re: udev keymaps: support for force_release quirk

[Johannes Stezenbach]

Hi,

On Wed, Dec 02, 2009 at 07:12:51PM -0800, Dmitry Torokhov wrote:
> 
> Yes, I would go with separate utilities and separate maps. They are
> conceptually different, work on different devices and map is not
> necessarily the same. It will be much cleaner this way.

Below is a version that seems to work, not yet tested thouroughly
but I wanted to find out if I'm on the right track.

There are two issues:
- I used shell arithmetic expansion to convert from hex to dec
  (works with busybox ash, dash and bash, but not guaranteed by POSIX).
  Is it OK?  Or should I use bc or rewrite as awk script?
- I hardcoded /lib/udev/keymaps/ path, probably needs LIBEXEC
  substition by autoconf

Let me know what you think.


Thanks
Johannes


From efa08417aa9487dbc4d5381aeff4892b1321f29b Mon Sep 17 00:00:00 2001
From: Johannes Stezenbach <js@sig21.net>
Date: Sun, 6 Dec 2009 22:28:28 +0100
Subject: [PATCH 1/1] Add support for the atkbd force_release quirk.

Some laptop keyboards don't generate release events
for some hotkeys.  Since linux-2.6.32 the list of scancodes
for which to enable the force_release quirk can be set
via sysfs.
Apply this to Samsung N130.

Signed-off-by: Johannes Stezenbach <js@sig21.net>
---
 extras/keymap/95-keyboard-force-release.rules     |   24 +++++++++++++++++++++
 extras/keymap/keyboard-force-release.sh           |   22 +++++++++++++++++++
 extras/keymap/keymaps/samsung-other-force-release |   10 ++++++++
 3 files changed, 56 insertions(+), 0 deletions(-)
 create mode 100644 extras/keymap/95-keyboard-force-release.rules
 create mode 100644 extras/keymap/keyboard-force-release.sh
 create mode 100644 extras/keymap/keymaps/samsung-other-force-release

diff --git a/extras/keymap/95-keyboard-force-release.rules b/extras/keymap/95-keyboard-force-release.rules
new file mode 100644
index 0000000..f231af9
--- /dev/null
+++ b/extras/keymap/95-keyboard-force-release.rules
@@ -0,0 +1,24 @@
+# Set model specific atkbd force_release quirk
+#
+# Serveral laptops have hotkeys which don't generate release events,
+# which can cause problems with software key repeat.
+# The atkbd driver has a quirk handler for generating synthetic
+# release events, which can be configured via sysfs since 2.6.32.
+# Simply add a file with a list of scancodes for your laptop model
+# in /lib/udev/keymaps, and add a rule here.
+# If the hotkeys also need a keymap assignment you can copy the
+# scancodes from the keymap file, otherwise you can run
+# /lib/udev/keymap -i /dev/input/eventX
+# on a Linux vt to find out.
+
+ACTION!="add", GOTO="force_release_end"
+SUBSYSTEM!="serio", GOTO="force_release_end"
+KERNEL!="serio*", GOTO="force_release_end"
+DRIVER!="atkbd", GOTO="force_release_end"
+
+ENV{DMI_VENDOR}="$attr{[dmi/id]sys_vendor}"
+
+
+ENV{DMI_VENDOR}="[sS][aA][mM][sS][uU][nN][gG]*", ATTR{[dmi/id]product_name}="*N130*", RUN+="keyboard-force-release.sh $sys$devpath samsung-other-force-release"
+
+LABEL="force_release_end"
diff --git a/extras/keymap/keyboard-force-release.sh b/extras/keymap/keyboard-force-release.sh
new file mode 100644
index 0000000..c76ef42
--- /dev/null
+++ b/extras/keymap/keyboard-force-release.sh
@@ -0,0 +1,22 @@
+#!/bin/sh -e
+# read list of scancodes, convert hex to decimal and
+# append to the atkbd force_release sysfs attribute
+# $1 sysfs devpath for serioX
+# $2 file with scancode list (hex or dec)
+
+case "$2" in
+	/*) scf="$2" ;;
+	*)  scf="/lib/udev/keymaps/$2" ;;
+esac
+
+attr=`cat "$1/force_release"`
+while read scancode dummy; do
+	case "$scancode" in
+		\#*) ;;
+		*)
+			scancode=$(($scancode))
+			[ -n "$attr" ] && attr="$attr,$scancode" || attr="$scancode"
+			;;
+	esac
+done <"$scf"
+echo "$attr" >"$1/force_release"
diff --git a/extras/keymap/keymaps/samsung-other-force-release b/extras/keymap/keymaps/samsung-other-force-release
new file mode 100644
index 0000000..c51123a
--- /dev/null
+++ b/extras/keymap/keymaps/samsung-other-force-release
@@ -0,0 +1,10 @@
+# list of scancodes (hex or decimal), optional comment
+0x82 # Fn+F4 CRT/LCD
+0x83 # Fn+F2 battery
+0x84 # Fn+F5 backlight on/off
+0x86 # Fn+F9 WLAN
+0x88 # Fn-Up brightness up
+0x89 # Fn-Down brightness down
+0xB3 # Fn+F8 switch power mode (battery/dynamic/performance)
+0xF7 # Fn+F10 Touchpad on
+0xF9 # Fn+F10 Touchpad off
-- 
1.6.5.4