[PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[Alexey Dobriyan]

Steps to reproduce:

	[log into console (not xterm)]
	[load non-trivial keymap]
	[turn on CapsLock]
	[type something]

Symbols won't be capital despite CapsLock and despite Shift+* working
as expected.

Note: patch relies on keymap being consistent wrt SMALL/CAPITAL symbols.
Though extracting SMALL <=> CAPITAL mapping from unicode tables and
putting it into kernel may be more correct.

Fix long-standing http://bugzilla.kernel.org/show_bug.cgi?id=7063

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 drivers/char/keyboard.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

--- a/drivers/char/keyboard.c
+++ b/drivers/char/keyboard.c
@@ -1261,8 +1261,14 @@ static void kbd_keycode(unsigned int keycode, int down, int hw_raw)
 		param.value = keysym;
 		if (atomic_notifier_call_chain(&keyboard_notifier_list, KBD_UNICODE, &param) == NOTIFY_STOP)
 			return;
-		if (down && !raw_mode)
+		if (down && !raw_mode) {
+			if (vc_kbd_led(kbd, VC_CAPSLOCK)) {
+				key_map = key_maps[shift_final ^ (1 << KG_SHIFT)];
+				if (key_map)
+					keysym = key_map[keycode];
+			}
 			to_utf8(vc, keysym);
+		}
 		return;
 	}
 

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[Alan Cox]

On Mon, 16 Nov 2009 16:51:15 +0300
Alexey Dobriyan <adobriyan@gmail.com> wrote:

> Steps to reproduce:
> 
> 	[log into console (not xterm)]
> 	[load non-trivial keymap]
> 	[turn on CapsLock]
> 	[type something]
> 
> Symbols won't be capital despite CapsLock and despite Shift+* working
> as expected.
> 
> Note: patch relies on keymap being consistent wrt SMALL/CAPITAL symbols.
> Though extracting SMALL <=> CAPITAL mapping from unicode tables and
> putting it into kernel may be more correct.
> 
> Fix long-standing http://bugzilla.kernel.org/show_bug.cgi?id=7063
> 
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>

Acked-by: Alan Cox <alan@linux.intel.com>

The assumptions seem reasonable and if not we'll find out. Either way its
going to be an improvement.

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[Samuel Thibault]

Alan Cox, le Mon 16 Nov 2009 14:40:03 +0000, a écrit :
> The assumptions seem reasonable and if not we'll find out. Either way its
> going to be an improvement.

Nope, it turns the caps lock into a shift lock, which is really not the
same.

Samuel

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[Samuel Thibault]

Alexey Dobriyan, le Mon 16 Nov 2009 16:51:15 +0300, a écrit :
> Steps to reproduce:
> 
> 	[log into console (not xterm)]
> 	[load non-trivial keymap]
> 	[turn on CapsLock]
> 	[type something]
> 
> Symbols won't be capital despite CapsLock and despite Shift+* working
> as expected.

Fix your keymap, it should use KT_LETTER instead of KT_LATIN.

> Note: patch relies on keymap being consistent wrt SMALL/CAPITAL symbols.

And that's not true for a lot of keyboard symbols.  Strictly speaking,
caps lock is caps lock, not shift lock.  If you really want a shift
lock, then set your caps lock key to produce shift lock.  Applying your
patch would turn the existing capslock behavior into shift lock, we
_don't_ want that.

> Though extracting SMALL <=> CAPITAL mapping from unicode tables and
> putting it into kernel may be more correct.

That's what console-setup does by using various symbol levels and it
just _works_.  One issue however is that then the capslock keyboard
led doesn't light up while in caps mode.  Maybe we should rethink the
interface to light keyboard leds instead.

Nacked-By: Samuel Thibault <samuel.thibault@ens-lyon.org>

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[Alexey Dobriyan]

On Mon, Nov 16, 2009 at 08:07:39PM +0100, Samuel Thibault wrote:
> Alexey Dobriyan, le Mon 16 Nov 2009 16:51:15 +0300, a écrit :
> > Steps to reproduce:
> > 
> > 	[log into console (not xterm)]
> > 	[load non-trivial keymap]
> > 	[turn on CapsLock]
> > 	[type something]
> > 
> > Symbols won't be capital despite CapsLock and despite Shift+* working
> > as expected.
> 
> Fix your keymap, it should use KT_LETTER instead of KT_LATIN.

You have read bugzilla and patch, haven't you?

My keymap contains

				keycode 44 = +z
		shift           keycode 44 = +Z
	altgr                   keycode 44 = U+044F        # CYRILLIC SMALL LETTER YA
	altgr   shift           keycode 44 = U+042F        # CYRILLIC CAPITAL LETTER YA

> > Note: patch relies on keymap being consistent wrt SMALL/CAPITAL symbols.
> 
> And that's not true for a lot of keyboard symbols.

That's why patch implies keymap is not fucked up.

> Strictly speaking, caps lock is caps lock, not shift lock.  If you really
> want a shift lock, then set your caps lock key to produce shift lock.
> Applying your patch would turn the existing capslock behavior into shift
> lock, we _don't_ want that.
> 
> > Though extracting SMALL <=> CAPITAL mapping from unicode tables and
> > putting it into kernel may be more correct.
> 
> That's what console-setup

What is it?

	$ sudo emerge -s console-setup
	Searching...
	[ Results for search key : console-setup ]
	[ Applications found : 0 ]

> does by using various symbol levels and it just _works_.

Ubuntu user?

> One issue however is that then the capslock keyboard
> led doesn't light up while in caps mode.

Interesting breakage you have.

[presses CapsLock several times]

> Maybe we should rethink the interface to light keyboard leds instead.

Oh, and there no need to reply at every place as if Linus is going to
grab it from bugzilla and apply in hurry.

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[Samuel Thibault]

Alexey Dobriyan, le Mon 16 Nov 2009 22:53:13 +0300, a écrit :
> > Fix your keymap, it should use KT_LETTER instead of KT_LATIN.
> 
> You have read bugzilla and patch, haven't you?

Yes.

And you could probably also read bug #7746 which is probably now a dup
(yes, the original report mixes several issues).

> My keymap contains
> 
> 				keycode 44 = +z
> 		shift           keycode 44 = +Z
> 	altgr                   keycode 44 = U+044F        # CYRILLIC SMALL LETTER YA
> 	altgr   shift           keycode 44 = U+042F        # CYRILLIC CAPITAL LETTER YA

And U+044F / U+042F is not KT_LETTER.

Yes, there's no way you can express a unicode character in KT_LETTER.
Limited interface, but that's not a reason to break other interfaces.

> > > Note: patch relies on keymap being consistent wrt SMALL/CAPITAL symbols.
> > 
> > And that's not true for a lot of keyboard symbols.
> 
> That's why patch implies keymap is not fucked up.

However you can't changed the "fucked up" keyboard of people.  They have
bought it and it's printed like that on it...

> > Strictly speaking, caps lock is caps lock, not shift lock.  If you really
> > want a shift lock, then set your caps lock key to produce shift lock.
> > Applying your patch would turn the existing capslock behavior into shift
> > lock, we _don't_ want that.
> > 
> > > Though extracting SMALL <=> CAPITAL mapping from unicode tables and
> > > putting it into kernel may be more correct.
> > 
> > That's what console-setup
> 
> What is it?

google tells packages.debian.org/console-setup

> 	$ sudo emerge -s console-setup
> 	Searching...
> 	[ Results for search key : console-setup ]
> 	[ Applications found : 0 ]

emerge is not universal, that's all.

> > does by using various symbol levels and it just _works_.
> 
> Ubuntu user?

Nope.

> > One issue however is that then the capslock keyboard led doesn't
> > light up while in caps mode.
> 
> Interesting breakage you have.

It's not breakage.  It's because instead of using the KT_LETTER way to
get the caps lock behavior, console-setup uses a modifier, since it's
much more powerful (you just decide what exactly will be the upper case,
and not have to rely on the shifted keysym to be the expected one), but
the kernel doesn't permit to assign a modifier to a keyboard LED.

> [presses CapsLock several times]

No need to be sarcastic.

> > Maybe we should rethink the interface to light keyboard leds instead.
> 
> Oh, and there no need to reply at every place

People posting in the bugzilla usually don't read linux-kernel, that's
why I posted a sum up there.

Samuel

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[Alexey Dobriyan]

On Mon, Nov 16, 2009 at 11:27:39PM +0100, Samuel Thibault wrote:
> Alexey Dobriyan, le Mon 16 Nov 2009 22:53:13 +0300, a écrit :
> And you could probably also read bug #7746 which is probably now a dup
> (yes, the original report mixes several issues).
> 
> > My keymap contains
> > 
> > 				keycode 44 = +z
> > 		shift           keycode 44 = +Z
> > 	altgr                   keycode 44 = U+044F        # CYRILLIC SMALL LETTER YA
> > 	altgr   shift           keycode 44 = U+042F        # CYRILLIC CAPITAL LETTER YA
> 
> And U+044F / U+042F is not KT_LETTER.
> 
> Yes, there's no way you can express a unicode character in KT_LETTER.
> Limited interface, but that's not a reason to break other interfaces.

What other interfaces?

Here, U+044F is sent, now (correctly) U+042F.

> > > > Note: patch relies on keymap being consistent wrt SMALL/CAPITAL symbols.
> > > 
> > > And that's not true for a lot of keyboard symbols.
> > 
> > That's why patch implies keymap is not fucked up.
> 
> However you can't changed the "fucked up" keyboard of people.  They have
> bought it and it's printed like that on it...

keymap is loadable, what are you talking about?
/usr/share/keymaps/i386/qwerty/ru.map.gz

> > > One issue however is that then the capslock keyboard led doesn't
> > > light up while in caps mode.
> > 
> > Interesting breakage you have.
> 
> It's not breakage.  It's because instead of using the KT_LETTER way to
> get the caps lock behavior, console-setup uses a modifier, since it's
> much more powerful (you just decide what exactly will be the upper case,
> and not have to rely on the shifted keysym to be the expected one), but
> the kernel doesn't permit to assign a modifier to a keyboard LED.

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[Samuel Thibault]

Alexey Dobriyan, le Tue 17 Nov 2009 01:53:51 +0300, a écrit :
> On Mon, Nov 16, 2009 at 11:27:39PM +0100, Samuel Thibault wrote:
> > Alexey Dobriyan, le Mon 16 Nov 2009 22:53:13 +0300, a écrit :
> > And you could probably also read bug #7746 which is probably now a dup
> > (yes, the original report mixes several issues).
> > 
> > > My keymap contains
> > > 
> > > 				keycode 44 = +z
> > > 		shift           keycode 44 = +Z
> > > 	altgr                   keycode 44 = U+044F        # CYRILLIC SMALL LETTER YA
> > > 	altgr   shift           keycode 44 = U+042F        # CYRILLIC CAPITAL LETTER YA
> > 
> > And U+044F / U+042F is not KT_LETTER.
> > 
> > Yes, there's no way you can express a unicode character in KT_LETTER.
> > Limited interface, but that's not a reason to break other interfaces.
> 
> What other interfaces?

The caps lock interface. As I said, caps lock is caps lock, it's not
shift lock.  For now Linux uses caps lock.  If you make it use shift
lock it completely changes the behavior.

> Here, U+044F is sent, now (correctly) U+042F.

Yes, that's shift lock.  But then any other key will also be shifted,
not only the KT_LETTER ones, while it's exactly the purpose of KT_LETTER
and caps lock.

> > > > > Note: patch relies on keymap being consistent wrt SMALL/CAPITAL symbols.
> > > > 
> > > > And that's not true for a lot of keyboard symbols.
> > > 
> > > That's why patch implies keymap is not fucked up.
> > 
> > However you can't changed the "fucked up" keyboard of people.  They have
> > bought it and it's printed like that on it...
> 
> keymap is loadable, what are you talking about?
> /usr/share/keymaps/i386/qwerty/ru.map.gz

It seems we don't understand each other, I'll repeat what I have
understood and explain more:

- you say
> Note: patch relies on keymap being consistent wrt SMALL/CAPITAL symbols.

- I understand
patch relies that keymaps always have the capital variant of letters at
the shifted position of each keys.

- I answer
> And that's not true for a lot of keyboard symbols.

- I mean
Some keyboard (physical, bought at the local store, not just keymaps
from CrazyHacker) do _not_ always have the capital variant of letters at
the shifted position of each keys.  On a french keyboard for instance, É
is not at shift+é, but altgr+shift+é.  So your patch won't work and
actually do harm.

- You answer
> That's why patch implies keymap is not fucked up.

- I understand
That's why patch implies that french keyboard is not fucked up.

- I answer
> However you can't changed the "fucked up" keyboard of people.  They have
> bought it and it's printed like that on it...

- I mean
Maybe it's fucked up, but all the french keyboards are like that, you
can't change that fact.

- You answer
> keymap is loadable, what are you talking about?
> /usr/share/keymaps/i386/qwerty/ru.map.gz

- I didn't understand anything.

Samuel

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[Samuel Thibault]

Samuel Thibault, le Mon 16 Nov 2009 23:27:38 +0100, a écrit :
> > My keymap contains
> > 
> > 				keycode 44 = +z
> > 		shift           keycode 44 = +Z
> > 	altgr                   keycode 44 = U+044F        # CYRILLIC SMALL LETTER YA
> > 	altgr   shift           keycode 44 = U+042F        # CYRILLIC CAPITAL LETTER YA
> 
> And U+044F / U+042F is not KT_LETTER.
> 
> Yes, there's no way you can express a unicode character in KT_LETTER.
> Limited interface, but that's not a reason to break other interfaces.

One way to go would be to decrete that keysyms between 0xD800 and 0xE000
(unused anyway) are "KT_LETTER" versions of the unicode 0x0000 - 0x0800.
That however covers only part of Unicode and doesn't solve the case of
keyboards where the upper case of a letter is not simply at the shifted
position.

The real correct solution is really to have kbd use modifiers just like
console-setup and provide a way for them to configure which leds should
be lit when some modifier is locked.  That way building a keymap becomes
just orthogonal, no need to upload a table of lower/upper pairs (which
depend on the locale see for instance i/I vs i/İ).

Samuel

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[Alexey Dobriyan]

On Mon, Nov 16, 2009 at 11:54:29PM +0100, Samuel Thibault wrote:
> Samuel Thibault, le Mon 16 Nov 2009 23:27:38 +0100, a écrit :
> > > My keymap contains
> > > 
> > > 				keycode 44 = +z
> > > 		shift           keycode 44 = +Z
> > > 	altgr                   keycode 44 = U+044F        # CYRILLIC SMALL LETTER YA
> > > 	altgr   shift           keycode 44 = U+042F        # CYRILLIC CAPITAL LETTER YA
> > 
> > And U+044F / U+042F is not KT_LETTER.
> > 
> > Yes, there's no way you can express a unicode character in KT_LETTER.
> > Limited interface, but that's not a reason to break other interfaces.
> 
> One way to go would be to decrete that keysyms between 0xD800 and 0xE000

And this is going to help me with U+042F/U+044F how?

> (unused anyway) are "KT_LETTER" versions of the unicode 0x0000 - 0x0800.
> That however covers only part of Unicode and doesn't solve the case of
> keyboards where the upper case of a letter is not simply at the shifted
> position.
> 
> The real correct solution is really to have kbd use modifiers just like
> console-setup and provide a way for them to configure which leds should
> be lit when some modifier is locked.  That way building a keymap becomes
> just orthogonal, no need to upload a table of lower/upper pairs (which
> depend on the locale see for instance i/I vs i/İ).

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[Samuel Thibault]

Alexey Dobriyan, le Tue 17 Nov 2009 02:05:23 +0300, a écrit :
> On Mon, Nov 16, 2009 at 11:54:29PM +0100, Samuel Thibault wrote:
> > Samuel Thibault, le Mon 16 Nov 2009 23:27:38 +0100, a écrit :
> > > > My keymap contains
> > > > 
> > > > 				keycode 44 = +z
> > > > 		shift           keycode 44 = +Z
> > > > 	altgr                   keycode 44 = U+044F        # CYRILLIC SMALL LETTER YA
> > > > 	altgr   shift           keycode 44 = U+042F        # CYRILLIC CAPITAL LETTER YA
> > > 
> > > And U+044F / U+042F is not KT_LETTER.
> > > 
> > > Yes, there's no way you can express a unicode character in KT_LETTER.
> > > Limited interface, but that's not a reason to break other interfaces.
> > 
> > One way to go would be to decrete that keysyms between 0xD800 and 0xE000
> 
> And this is going to help me with U+042F/U+044F how?

This way:

diff --git a/drivers/char/keyboard.c b/drivers/char/keyboard.c
index 737be95..264db17 100644
--- a/drivers/char/keyboard.c
+++ b/drivers/char/keyboard.c
@@ -1258,6 +1258,15 @@ static void kbd_keycode(unsigned int keycode, int down, int hw_raw)
 	type = KTYP(keysym);
 
 	if (type < 0xf0) {
+		if (keysym >= 0xD800 && keysym < 0xE000) {
+			/* Surrogates in Unicode, here KT_LETTER variants of unicode U+0000-U+07FF */
+			keysym -= 0xD800;
+			if (vc_kbd_led(kbd, VC_CAPSLOCK)) {
+				key_map = key_maps[shift_final ^ (1 << KG_SHIFT)];
+				if (key_map)
+					keysym = key_map[keycode];
+			}
+		}
 		param.value = keysym;
 		if (atomic_notifier_call_chain(&keyboard_notifier_list, KBD_UNICODE, &param) == NOTIFY_STOP)
 			return;

Which BTW is correct while the proposed patch earlier wasn't: the
param.value needs to be the final keysym.

But again, that only solves the problem of the limited range
U+0000-U+0800 and doesn't solve the é/É french keyboard problem.

Adding an interface to change the modifier lock / LED assignation would
on the other hand permit kbd and console-setup to properly do proper
capslock processing correctly.

Samuel

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[Alexey Dobriyan]

On Tue, Nov 17, 2009 at 12:15:20AM +0100, Samuel Thibault wrote:
> Alexey Dobriyan, le Tue 17 Nov 2009 02:05:23 +0300, a écrit :
> > On Mon, Nov 16, 2009 at 11:54:29PM +0100, Samuel Thibault wrote:
> > > Samuel Thibault, le Mon 16 Nov 2009 23:27:38 +0100, a écrit :
> > > > > My keymap contains
> > > > > 
> > > > > 				keycode 44 = +z
> > > > > 		shift           keycode 44 = +Z
> > > > > 	altgr                   keycode 44 = U+044F        # CYRILLIC SMALL LETTER YA
> > > > > 	altgr   shift           keycode 44 = U+042F        # CYRILLIC CAPITAL LETTER YA
> > > > 
> > > > And U+044F / U+042F is not KT_LETTER.
> > > > 
> > > > Yes, there's no way you can express a unicode character in KT_LETTER.
> > > > Limited interface, but that's not a reason to break other interfaces.
> > > 
> > > One way to go would be to decrete that keysyms between 0xD800 and 0xE000
> > 
> > And this is going to help me with U+042F/U+044F how?

> --- a/drivers/char/keyboard.c
> +++ b/drivers/char/keyboard.c
> @@ -1258,6 +1258,15 @@ static void kbd_keycode(unsigned int keycode, int down, int hw_raw)
>  	type = KTYP(keysym);
>  
>  	if (type < 0xf0) {
> +		if (keysym >= 0xD800 && keysym < 0xE000) {

keysym is 0x044F at this point.

> +			/* Surrogates in Unicode, here KT_LETTER variants of unicode U+0000-U+07FF */
> +			keysym -= 0xD800;
> +			if (vc_kbd_led(kbd, VC_CAPSLOCK)) {
> +				key_map = key_maps[shift_final ^ (1 << KG_SHIFT)];
> +				if (key_map)
> +					keysym = key_map[keycode];
> +			}
> +		}
>  		param.value = keysym;
>  		if (atomic_notifier_call_chain(&keyboard_notifier_list, KBD_UNICODE, &param) == NOTIFY_STOP)
>  			return;
> 
> Which BTW is correct while the proposed patch earlier wasn't: the
> param.value needs to be the final keysym.

OK, this I should fix.

> But again, that only solves the problem of the limited range
> U+0000-U+0800 and doesn't solve the é/É french keyboard problem.
> 
> Adding an interface to change the modifier lock / LED assignation would
> on the other hand permit kbd and console-setup to properly do proper
> capslock processing correctly.

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[Samuel Thibault]

Alexey Dobriyan, le Tue 17 Nov 2009 14:55:03 +0300, a écrit :
> On Tue, Nov 17, 2009 at 12:15:20AM +0100, Samuel Thibault wrote:
> > Alexey Dobriyan, le Tue 17 Nov 2009 02:05:23 +0300, a écrit :
> > > On Mon, Nov 16, 2009 at 11:54:29PM +0100, Samuel Thibault wrote:
> > > > Samuel Thibault, le Mon 16 Nov 2009 23:27:38 +0100, a écrit :
> > > > > > My keymap contains
> > > > > > 
> > > > > > 				keycode 44 = +z
> > > > > > 		shift           keycode 44 = +Z
> > > > > > 	altgr                   keycode 44 = U+044F        # CYRILLIC SMALL LETTER YA
> > > > > > 	altgr   shift           keycode 44 = U+042F        # CYRILLIC CAPITAL LETTER YA
> > > > > 
> > > > > And U+044F / U+042F is not KT_LETTER.
> > > > > 
> > > > > Yes, there's no way you can express a unicode character in KT_LETTER.
> > > > > Limited interface, but that's not a reason to break other interfaces.
> > > > 
> > > > One way to go would be to decrete that keysyms between 0xD800 and 0xE000
> > > 
> > > And this is going to help me with U+042F/U+044F how?
> 
> > --- a/drivers/char/keyboard.c
> > +++ b/drivers/char/keyboard.c
> > @@ -1258,6 +1258,15 @@ static void kbd_keycode(unsigned int keycode, int down, int hw_raw)
> >  	type = KTYP(keysym);
> >  
> >  	if (type < 0xf0) {
> > +		if (keysym >= 0xD800 && keysym < 0xE000) {
> 
> keysym is 0x044F at this point.

I'm precisely suggesting that instead of 0x044F, the keymap provides
0xdc4f, just like for +z the keysym is not K(KT_LATIN,0x0079), but
K(KT_LETTER,0x79) (that's the difference between just z and +z, i.e.
whether it's KT_LATIN or KT_LETTER, i.e. whether capslock acts on it or
not, that's the whole point of capslock vs shift lock!).

More precisely, in the kbd source code, in the add_capslock function, in
the unicode case, instead of ignoring the '+', add 0xD800 to the unicode
value if it is below 0x0800.

But again, that's a very limited fix and just fixing the LED interface
would allow to just use modifiers and permit much more powerful keymaps.

Samuel

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[Alexey Dobriyan]

On Tue, Nov 17, 2009 at 02:23:58PM +0100, Samuel Thibault wrote:
> Alexey Dobriyan, le Tue 17 Nov 2009 14:55:03 +0300, a écrit :
> > On Tue, Nov 17, 2009 at 12:15:20AM +0100, Samuel Thibault wrote:
> > > Alexey Dobriyan, le Tue 17 Nov 2009 02:05:23 +0300, a écrit :
> > > > On Mon, Nov 16, 2009 at 11:54:29PM +0100, Samuel Thibault wrote:
> > > > > Samuel Thibault, le Mon 16 Nov 2009 23:27:38 +0100, a écrit :
> > > > > > > My keymap contains
> > > > > > > 
> > > > > > > 				keycode 44 = +z
> > > > > > > 		shift           keycode 44 = +Z
> > > > > > > 	altgr                   keycode 44 = U+044F        # CYRILLIC SMALL LETTER YA
> > > > > > > 	altgr   shift           keycode 44 = U+042F        # CYRILLIC CAPITAL LETTER YA
> > > > > > 
> > > > > > And U+044F / U+042F is not KT_LETTER.
> > > > > > 
> > > > > > Yes, there's no way you can express a unicode character in KT_LETTER.
> > > > > > Limited interface, but that's not a reason to break other interfaces.
> > > > > 
> > > > > One way to go would be to decrete that keysyms between 0xD800 and 0xE000
> > > > 
> > > > And this is going to help me with U+042F/U+044F how?
> > 
> > > --- a/drivers/char/keyboard.c
> > > +++ b/drivers/char/keyboard.c
> > > @@ -1258,6 +1258,15 @@ static void kbd_keycode(unsigned int keycode, int down, int hw_raw)
> > >  	type = KTYP(keysym);
> > >  
> > >  	if (type < 0xf0) {
> > > +		if (keysym >= 0xD800 && keysym < 0xE000) {
> > 
> > keysym is 0x044F at this point.
> 
> I'm precisely suggesting that instead of 0x044F, the keymap provides
> 0xdc4f, just like for +z the keysym is not K(KT_LATIN,0x0079), but
> K(KT_LETTER,0x79) (that's the difference between just z and +z, i.e.
> whether it's KT_LATIN or KT_LETTER, i.e. whether capslock acts on it or
> not, that's the whole point of capslock vs shift lock!).
> 
> More precisely, in the kbd source code, in the add_capslock function, in
> the unicode case, instead of ignoring the '+', add 0xD800 to the unicode
> value if it is below 0x0800.

You suggest to change kernel and keymap and kbd and introduce 0xD800 hack.
This is not going to fly.

> But again, that's a very limited fix and just fixing the LED interface
> would allow to just use modifiers and permit much more powerful keymaps.

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[Samuel Thibault]

Alexey Dobriyan, le Thu 19 Nov 2009 16:18:54 +0300, a écrit :
> > More precisely, in the kbd source code, in the add_capslock function, in
> > the unicode case, instead of ignoring the '+', add 0xD800 to the unicode
> > value if it is below 0x0800.
> 
> You suggest to change kernel and keymap and kbd and introduce 0xD800 hack.
> This is not going to fly.

Sure, that's precisely what I said here:

> > But again, that's a very limited fix and just fixing the LED interface
> > would allow to just use modifiers and permit much more powerful keymaps.

Just to make sure you understand my point: your bug is _not_ in the
kernel, but in kbd, see what the add_capslock() function does when in
unicode mode:

	fprintf(stderr, _("plus before %s ignored\n"), p);

No wonder capslocked keys don't work, kbd doesn't even tell the kernel
which keys should get capslock behavior. Breaking the capslock behavior
to make it a shiftlock to compensate kbd's bug is not going to fly
either.

Now, as I said, kbd's issue is that it doesn't have any way to express
that a unicode keysym should get capslock behavior, due to kernel
interface limitation.  But as I said, it can do like console-setup: use
a modifier instead of the limited capslock interface. A side issue is
that modifier locks don't get advertised through keyboard leds. That can
be solved by adding an interface to make that happen.

Instead of breaking an interface (capslock by making it a shiftlock
instead), add one that makes sense (being able to configure how modifier
locks are advertised through LEDs). _That_ will fly.

Samuel

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[Samuel Thibault]

Just to make sure you really understand.

Samuel Thibault, le Thu 19 Nov 2009 14:28:28 +0100, a écrit :
> Now, as I said, kbd's issue is that it doesn't have any way to express
> that a unicode keysym should get capslock behavior, due to kernel
> interface limitation.

And that's precisely why I just showed how such limitation could be
lifted:

> > > More precisely, in the kbd source code, in the add_capslock function, in
> > > the unicode case, instead of ignoring the '+', add 0xD800 to the unicode
> > > value if it is below 0x0800.
> > 
> > You suggest to change kernel and keymap and kbd and introduce 0xD800 hack.
> > This is not going to fly.
> 
> Sure, that's precisely what I said here:
> 
> > > But again, that's a very limited fix and just fixing the LED interface
> > > would allow to just use modifiers and permit much more powerful keymaps.

So we agree, and so the right solution is to either completely rework
the interface (ugh), or just add LED routing (which can be very useful,
actually, I for instance don't care at all about the num lock state, but
I _do_ care about the lock that lets me shift between different keyboard
layouts).

Samuel

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[H. Peter Anvin]

On 11/19/2009 05:37 AM, Samuel Thibault wrote:
> 
> So we agree, and so the right solution is to either completely rework
> the interface (ugh), or just add LED routing (which can be very useful,
> actually, I for instance don't care at all about the num lock state, but
> I _do_ care about the lock that lets me shift between different keyboard
> layouts).
> 

Adding LED routing and using keyboard levels is so clearly The Right Thing.

We already allow the LEDs to be programmed; we should probably just
have, for each LED, the ability to program it off/on/flashing/connected
to modifier #X.

The current KDSETLED/KDGETLED interface is of course too limited for
this -- we need an interface with at least a byte per LED.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[Pavel Machek]

Hi!

> > So we agree, and so the right solution is to either completely rework
> > the interface (ugh), or just add LED routing (which can be very useful,
> > actually, I for instance don't care at all about the num lock state, but
> > I _do_ care about the lock that lets me shift between different keyboard
> > layouts).
> > 
> 
> Adding LED routing and using keyboard levels is so clearly The Right Thing.
> 
> We already allow the LEDs to be programmed; we should probably just
> have, for each LED, the ability to program it off/on/flashing/connected
> to modifier #X.
> 
> The current KDSETLED/KDGETLED interface is of course too limited for
> this -- we need an interface with at least a byte per LED.

Actually, we already have very nice /sys/class/led interface, and we
should just use it for keyboard leds, too. Actually I have a patch,
somewhere...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[Pavel Machek]

Hi!

> > > So we agree, and so the right solution is to either completely rework
> > > the interface (ugh), or just add LED routing (which can be very useful,
> > > actually, I for instance don't care at all about the num lock state, but
> > > I _do_ care about the lock that lets me shift between different keyboard
> > > layouts).
> > > 
> > 
> > Adding LED routing and using keyboard levels is so clearly The Right Thing.
> > 
> > We already allow the LEDs to be programmed; we should probably just
> > have, for each LED, the ability to program it off/on/flashing/connected
> > to modifier #X.
> > 
> > The current KDSETLED/KDGETLED interface is of course too limited for
> > this -- we need an interface with at least a byte per LED.
> 
> Actually, we already have very nice /sys/class/led interface, and we
> should just use it for keyboard leds, too. Actually I have a patch,
> somewhere...

Like this... But it should probably be slightly more complex -- like
making numlock/capslock/scrollock leds into trigers, and then
move input LED lowlevels into drivers/led...

Signed-off-by: Pavel Machek <pavel@ucw.cz> (but...)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index e4f599f..c80fee8 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -96,6 +96,13 @@ config LEDS_COBALT_RAQ
 	help
 	  This option enables support for the Cobalt Raq series LEDs.
 
+config LEDS_INPUT
+	tristate "LED Support for input layer keyboards"
+	depends on LEDS_CLASS
+	help
+	  This option enables support for LEDs on keyboards handled by
+	  input layer.
+
 config LEDS_SUNFIRE
 	tristate "LED support for SunFire servers."
 	depends on LEDS_CLASS && SPARC64
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 46d7270..71bde81 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_LEDS_NET48XX)		+= leds-net48xx.o
 obj-$(CONFIG_LEDS_WRAP)			+= leds-wrap.o
 obj-$(CONFIG_LEDS_ALIX2)		+= leds-alix2.o
 obj-$(CONFIG_LEDS_H1940)		+= leds-h1940.o
+obj-$(CONFIG_LEDS_INPUT)		+= leds-input.o
 obj-$(CONFIG_LEDS_COBALT_QUBE)		+= leds-cobalt-qube.o
 obj-$(CONFIG_LEDS_COBALT_RAQ)		+= leds-cobalt-raq.o
 obj-$(CONFIG_LEDS_SUNFIRE)		+= leds-sunfire.o
diff --git a/drivers/leds/leds-input.c b/drivers/leds/leds-input.c
new file mode 100644
index 0000000..8ffb4fc
--- /dev/null
+++ b/drivers/leds/leds-input.c
@@ -0,0 +1,151 @@
+/*
+ * LED <-> input subsystem glue
+ *
+ * Copyright 2007 Pavel Machek <pavel@ucw.cz>
+ * Copyright 2007 Dmitry Torokhov
+ * Copyright 2005-2006 Openedhand Ltd.
+ *
+ * Author: Pavel Machek <pavel@ucw.cz>
+ * Based on code by: Richard Purdie <rpurdie@openedhand.com>
+ * 		     
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/leds.h>
+
+#include <linux/module.h>
+#include <linux/input.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
+#include <linux/init.h>
+
+struct blinker {
+	struct work_struct work;
+	struct input_handle handle;
+	int state;
+
+	struct led_classdev dev;
+};
+
+struct blinker *blinker;
+
+static void inputled_set(struct led_classdev *led_cdev, enum led_brightness value)
+{
+	struct blinker *blinker = container_of(led_cdev, struct blinker, dev);
+	blinker->state = value;
+	schedule_work(&blinker->work);
+}
+
+static void blink_task_handler(struct work_struct *work)
+{
+	struct blinker *blinker = container_of(work, struct blinker, work);
+	input_inject_event(&blinker->handle, EV_LED, LED_CAPSL, !!blinker->state);
+}
+
+static void blink_event(struct input_handle *handle, unsigned int type,
+		        unsigned int code, int down)
+{
+	/*
+	 * This is a very rare handler that does not process any input
+	 * events; just injects them.
+	 */
+}
+
+static int blink_connect(struct input_handler *handler, struct input_dev *dev,
+			  const struct input_device_id *id)
+{
+	struct input_handle *handle;
+	struct led_classdev *led_dev;
+	static int counter;
+	int error;
+
+	blinker = kzalloc(sizeof(struct blinker), GFP_KERNEL);
+	if (!blinker) {
+		return -ENOMEM;
+	}
+
+	INIT_WORK(&blinker->work, blink_task_handler);
+
+	led_dev = &blinker->dev;
+	led_dev->name = kmalloc(10, GFP_KERNEL);
+	sprintf(led_dev->name, "input%d", counter++);
+	led_dev->brightness_set = inputled_set;
+
+	handle = &blinker->handle;
+	handle->dev = dev;
+	handle->handler = handler;
+	handle->name = "blink";
+	handle->private = blinker;
+
+	error = input_register_handle(handle);
+	if (error)
+		goto err_free_handle;
+
+	error = input_open_device(handle);
+	if (error)
+		goto err_unregister_handle;
+
+	error = led_classdev_register(NULL, led_dev);
+	if (error < 0)
+		goto err_input_close_device;
+
+	return 0;
+
+ err_input_close_device:
+	input_close_device(handle);
+ err_unregister_handle:
+	input_unregister_handle(handle);
+ err_free_handle:
+	kfree(handle);
+	return error;
+}
+
+static void blink_disconnect(struct input_handle *handle)
+{
+	struct blinker *blinker = handle->private;
+
+	led_classdev_unregister(&blinker->dev);
+	input_close_device(handle);
+	input_unregister_handle(handle);
+	kfree(blinker);
+}
+
+static const struct input_device_id blink_ids[] = {
+	{
+		.flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_LEDBIT,
+		.evbit = { BIT(EV_LED) },
+		.ledbit = { [LED_CAPSL] = BIT(LED_CAPSL) },
+	},
+	{ }
+};
+
+static struct input_handler blink_handler = {
+	.event		= blink_event,
+	.connect	= blink_connect,
+	.disconnect	= blink_disconnect,
+	.name		= "blink",
+	.id_table	= blink_ids,
+};
+
+static int __init blink_handler_init(void)
+{
+	return input_register_handler(&blink_handler);
+}
+
+static void __exit blink_handler_exit(void)
+{
+	input_unregister_handler(&blink_handler);
+	flush_scheduled_work();
+}
+
+module_init(blink_handler_init);
+module_exit(blink_handler_exit);
+
+


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII

[H. Peter Anvin]

On 11/20/2009 12:46 PM, Pavel Machek wrote:
> 
> Like this... But it should probably be slightly more complex -- like
> making numlock/capslock/scrollock leds into trigers, and then
> move input LED lowlevels into drivers/led...
> 

In particular, all keyboard modifiers should be available triggers.

	-hpa

[RFC,PATCH] Route kbd leds through the generic leds layer (Was: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII)

[Samuel Thibault]

Route keyboard leds through the generic leds layer.

This permits to reassign keyboard LEDs to something else than keyboard
"leds" state, and also permits to fix #7063 by using a modifier to
implement proper CapsLock behavior and have the keyboard caps lock led
show that caps lock state.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---

Hello,

H. Peter Anvin, le Fri 20 Nov 2009 13:27:48 -0800, a écrit :
> On 11/20/2009 12:46 PM, Pavel Machek wrote:
> > Like this... But it should probably be slightly more complex -- like
> > making numlock/capslock/scrollock leds into trigers, and then
> > move input LED lowlevels into drivers/led...
> > 
> 
> In particular, all keyboard modifiers should be available triggers.

Here is a patch to be commented on.  It does work for me, and the very
nice thing I like is

echo phy0assoc > /sys/class/leds/input::scrolllock/trigger

because I don't have a wifi led on my laptop.

The thing I like less is that dmesg now always contains

Registered led device: input::numlock
etc.

I could perhaps inspect the set of leds supported by all the connected
input devices and only register triggers for those?  Not so easy, but
should be doable.  In the meanwhile, please comment on the attached
patch.

Samuel


diff -ur linux-2.6.32-orig/Documentation/leds-class.txt linux-2.6.32-perso/Documentation/leds-class.txt
--- linux-2.6.32-orig/Documentation/leds-class.txt	2009-12-03 13:41:42.000000000 +0100
+++ linux-2.6.32-perso/Documentation/leds-class.txt	2010-02-21 04:12:59.000000000 +0100
@@ -2,9 +2,6 @@
 LED handling under Linux
 ========================
 
-If you're reading this and thinking about keyboard leds, these are
-handled by the input subsystem and the led class is *not* needed.
-
 In its simplest form, the LED class just allows control of LEDs from
 userspace. LEDs appear in /sys/class/leds/. The maximum brightness of the
 LED is defined in max_brightness file. The brightness file will set the brightness
diff -ur linux-2.6.32-orig/drivers/char/keyboard.c linux-2.6.32-perso/drivers/char/keyboard.c
--- linux-2.6.32-orig/drivers/char/keyboard.c	2009-12-03 13:42:46.000000000 +0100
+++ linux-2.6.32-perso/drivers/char/keyboard.c	2010-02-21 05:59:26.000000000 +0100
@@ -34,6 +34,7 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/irq.h>
+#include <linux/leds.h>
 
 #include <linux/kbd_kern.h>
 #include <linux/kbd_diacr.h>
@@ -140,6 +141,9 @@
 static char rep;					/* flag telling character repeat */
 
 static unsigned char ledstate = 0xff;			/* undefined */
+#ifdef CONFIG_LEDS_INPUT
+static unsigned char lockstate = 0xff;			/* undefined */
+#endif
 static unsigned char ledioctl;
 
 static struct ledptr {
@@ -997,6 +1001,23 @@
 	return leds;
 }
 
+#ifdef CONFIG_LEDS_INPUT
+/* When input-based leds are enabled, we route keyboard "leds" through triggers
+ */
+DEFINE_LED_TRIGGER(ledtrig_scrolllock);
+DEFINE_LED_TRIGGER(ledtrig_numlock);
+DEFINE_LED_TRIGGER(ledtrig_capslock);
+DEFINE_LED_TRIGGER(ledtrig_kanalock);
+DEFINE_LED_TRIGGER(ledtrig_shiftlock);
+DEFINE_LED_TRIGGER(ledtrig_altgrlock);
+DEFINE_LED_TRIGGER(ledtrig_ctrllock);
+DEFINE_LED_TRIGGER(ledtrig_altlock);
+DEFINE_LED_TRIGGER(ledtrig_shiftllock);
+DEFINE_LED_TRIGGER(ledtrig_shiftrlock);
+DEFINE_LED_TRIGGER(ledtrig_ctrlllock);
+DEFINE_LED_TRIGGER(ledtrig_ctrlrlock);
+#endif
+
 /*
  * This routine is the bottom half of the keyboard interrupt
  * routine, and runs with all interrupts enabled. It does
@@ -1013,19 +1034,63 @@
 
 static void kbd_bh(unsigned long dummy)
 {
-	struct list_head *node;
 	unsigned char leds = getleds();
 
 	if (leds != ledstate) {
+#ifdef CONFIG_LEDS_INPUT
+		led_trigger_event(ledtrig_scrolllock,
+				leds & (1 << VC_SCROLLOCK) ? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_numlock,
+				leds & (1 << VC_NUMLOCK)   ? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_capslock,
+				leds & (1 << VC_CAPSLOCK)  ? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_kanalock,
+				leds & (1 << VC_KANALOCK)  ? INT_MAX : LED_OFF);
+#else
+		struct list_head *node;
 		list_for_each(node, &kbd_handler.h_list) {
 			struct input_handle *handle = to_handle_h(node);
-			input_inject_event(handle, EV_LED, LED_SCROLLL, !!(leds & 0x01));
-			input_inject_event(handle, EV_LED, LED_NUML,    !!(leds & 0x02));
-			input_inject_event(handle, EV_LED, LED_CAPSL,   !!(leds & 0x04));
+			input_inject_event(handle, EV_LED, LED_SCROLLL,
+					!!(leds & (1 << VC_SCROLLOCK)));
+			input_inject_event(handle, EV_LED, LED_NUML,
+					!!(leds & (1 << VC_NUMLOCK)));
+			input_inject_event(handle, EV_LED, LED_CAPSL,
+					!!(leds & (1 << VC_CAPSLOCK)));
 			input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
 		}
+#endif
 	}
 
+#ifdef CONFIG_LEDS_INPUT
+	if (kbd->lockstate != lockstate) {
+		led_trigger_event(ledtrig_shiftlock,
+			kbd->lockstate & (1<<VC_SHIFTLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_altgrlock,
+			kbd->lockstate & (1<<VC_ALTGRLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_ctrllock,
+			kbd->lockstate & (1<<VC_CTRLLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_altlock,
+			kbd->lockstate & (1<<VC_ALTLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_shiftllock,
+			kbd->lockstate & (1<<VC_SHIFTLLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_shiftrlock,
+			kbd->lockstate & (1<<VC_SHIFTRLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_ctrlllock,
+			kbd->lockstate & (1<<VC_CTRLLLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_ctrlrlock,
+			kbd->lockstate & (1<<VC_CTRLRLOCK)
+				? INT_MAX : LED_OFF);
+	}
+	lockstate = kbd->lockstate;
+#endif
+
 	ledstate = leds;
 }
 
@@ -1357,6 +1422,7 @@
 	kfree(handle);
 }
 
+#ifndef CONFIG_LEDS_INPUT
 /*
  * Start keyboard handler on the new keyboard by refreshing LED state to
  * match the rest of the system.
@@ -1367,13 +1433,17 @@
 
 	tasklet_disable(&keyboard_tasklet);
 	if (leds != 0xff) {
-		input_inject_event(handle, EV_LED, LED_SCROLLL, !!(leds & 0x01));
-		input_inject_event(handle, EV_LED, LED_NUML,    !!(leds & 0x02));
-		input_inject_event(handle, EV_LED, LED_CAPSL,   !!(leds & 0x04));
+		input_inject_event(handle, EV_LED, LED_SCROLLL,
+				!!(leds & (1 << VC_SCROLLOCK)));
+		input_inject_event(handle, EV_LED, LED_NUML,
+				!!(leds & (1 << VC_NUMLOCK)));
+		input_inject_event(handle, EV_LED, LED_CAPSL,
+				!!(leds & (1 << VC_CAPSLOCK)));
 		input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
 	}
 	tasklet_enable(&keyboard_tasklet);
 }
+#endif
 
 static const struct input_device_id kbd_ids[] = {
 	{
@@ -1395,7 +1465,9 @@
 	.event		= kbd_event,
 	.connect	= kbd_connect,
 	.disconnect	= kbd_disconnect,
+#ifndef CONFIG_LEDS_INPUT
 	.start		= kbd_start,
+#endif
 	.name		= "kbd",
 	.id_table	= kbd_ids,
 };
@@ -1419,6 +1491,21 @@
 	if (error)
 		return error;
 
+#ifdef CONFIG_LEDS_INPUT
+	led_trigger_register_simple("scrolllock", &ledtrig_scrolllock);
+	led_trigger_register_simple("numlock", &ledtrig_numlock);
+	led_trigger_register_simple("capslock", &ledtrig_capslock);
+	led_trigger_register_simple("kanalock", &ledtrig_kanalock);
+	led_trigger_register_simple("shiftlock", &ledtrig_shiftlock);
+	led_trigger_register_simple("altgrlock", &ledtrig_altgrlock);
+	led_trigger_register_simple("ctrllock", &ledtrig_ctrllock);
+	led_trigger_register_simple("altlock", &ledtrig_altlock);
+	led_trigger_register_simple("shiftllock", &ledtrig_shiftllock);
+	led_trigger_register_simple("shiftrlock", &ledtrig_shiftrlock);
+	led_trigger_register_simple("ctrlllock", &ledtrig_ctrlllock);
+	led_trigger_register_simple("ctrlrlock", &ledtrig_ctrlrlock);
+#endif
+
 	tasklet_enable(&keyboard_tasklet);
 	tasklet_schedule(&keyboard_tasklet);
 
diff -ur linux-2.6.32-orig/drivers/char/vt.c linux-2.6.32-perso/drivers/char/vt.c
--- linux-2.6.32-orig/drivers/char/vt.c	2009-12-03 13:42:47.000000000 +0100
+++ linux-2.6.32-perso/drivers/char/vt.c	2010-02-20 17:21:58.000000000 +0100
@@ -1625,6 +1625,7 @@
 	clr_kbd(vc, lnm);
 	kbd_table[vc->vc_num].lockstate = 0;
 	kbd_table[vc->vc_num].slockstate = 0;
+	/* FIXME */
 	kbd_table[vc->vc_num].ledmode = LED_SHOW_FLAGS;
 	kbd_table[vc->vc_num].ledflagstate = kbd_table[vc->vc_num].default_ledflagstate;
 	/* do not do set_leds here because this causes an endless tasklet loop
diff -ur linux-2.6.32-orig/drivers/leds/Kconfig linux-2.6.32-perso/drivers/leds/Kconfig
--- linux-2.6.32-orig/drivers/leds/Kconfig	2009-12-03 13:42:57.000000000 +0100
+++ linux-2.6.32-perso/drivers/leds/Kconfig	2010-02-21 04:17:07.000000000 +0100
@@ -4,9 +4,6 @@
 	  Say Y to enable Linux LED support.  This allows control of supported
 	  LEDs from both userspace and optionally, by kernel events (triggers).
 
-	  This is not related to standard keyboard LEDs which are controlled
-	  via the input system.
-
 if NEW_LEDS
 
 config LEDS_CLASS
@@ -17,6 +14,13 @@
 
 comment "LED drivers"
 
+config LEDS_INPUT
+	tristate "LED Support using input keyboards"
+	depends on LEDS_CLASS
+	help
+	  This option enables support for the LEDs on keyboard managed
+	  by the input layer.
+
 config LEDS_ATMEL_PWM
 	tristate "LED Support using Atmel PWM outputs"
 	depends on LEDS_CLASS && ATMEL_PWM
diff -ur linux-2.6.32-orig/drivers/leds/leds-input.c linux-2.6.32-perso/drivers/leds/leds-input.c
--- linux-2.6.32-orig/drivers/leds/leds-input.c	2010-02-21 04:13:41.000000000 +0100
+++ linux-2.6.32-perso/drivers/leds/leds-input.c	2010-02-21 05:59:58.000000000 +0100
@@ -0,0 +1,172 @@
+/*
+ * LED support for the input layer
+ *
+ * Copyright 2010 Samuel Thibault <samuel.thibault@ens-lyon.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/input.h>
+
+static DEFINE_SPINLOCK(input_led_lock);
+static int input_led_leds = -1;
+static struct led_classdev input_leds[];
+static struct input_handler input_led_handler;
+
+/* Led state change, update all keyboards */
+static void input_led_set(struct led_classdev *cdev,
+			  enum led_brightness brightness)
+{
+	int led = cdev - input_leds;
+	unsigned long flags;
+	struct input_handle *handle;
+
+	spin_lock_irqsave(&input_led_lock, flags);
+	list_for_each_entry(handle, &input_led_handler.h_list, h_node) {
+		input_inject_event(handle, EV_LED, led, !!brightness);
+		input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
+	}
+	if (brightness)
+		input_led_leds |= 1 << led;
+	else
+		input_led_leds &= ~(1 << led);
+	spin_unlock_irqrestore(&input_led_lock, flags);
+}
+
+/* Array of all the input leds */
+static struct led_classdev input_leds[] = {
+#define DEFINE_INPUT_LED(input_led, nam, deftrig) \
+	[input_led] = { \
+		.name = "input::"nam, \
+		.max_brightness = 1, \
+		.brightness_set = input_led_set, \
+		.default_trigger = deftrig, \
+	}
+DEFINE_INPUT_LED(LED_NUML, "numlock", "numlock"),
+DEFINE_INPUT_LED(LED_CAPSL, "capslock", "capslock"),
+DEFINE_INPUT_LED(LED_SCROLLL, "scrolllock", "scrolllock"),
+DEFINE_INPUT_LED(LED_COMPOSE, "compose", NULL),
+DEFINE_INPUT_LED(LED_KANA, "kana", NULL),
+DEFINE_INPUT_LED(LED_SLEEP, "sleep", NULL),
+DEFINE_INPUT_LED(LED_SUSPEND, "suspend", NULL),
+DEFINE_INPUT_LED(LED_MUTE, "mute", NULL),
+DEFINE_INPUT_LED(LED_MISC, "misc", NULL),
+DEFINE_INPUT_LED(LED_MAIL, "mail", NULL),
+DEFINE_INPUT_LED(LED_CHARGING, "charging", NULL),
+};
+
+static int input_led_connect(struct input_handler *handler,
+			      struct input_dev *dev,
+			      const struct input_device_id *id)
+{
+	struct input_handle *handle;
+	int error;
+
+	if (!test_bit(EV_LED, dev->keybit))
+		return -ENODEV;
+
+	handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL);
+	if (!handle)
+		return -ENOMEM;
+
+	handle->dev = dev;
+	handle->handler = handler;
+	handle->name = "input leds";
+
+	error = input_register_handle(handle);
+	if (error) {
+		kfree(handle);
+		return error;
+	}
+
+	return 0;
+}
+
+static void input_led_disconnect(struct input_handle *handle)
+{
+	input_unregister_handle(handle);
+	kfree(handle);
+}
+
+/* New keyboard, update its leds */
+static void input_led_start(struct input_handle *handle)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&input_led_lock, flags);
+	if (input_led_leds != -1) {
+		int i;
+		for (i = 0; i < sizeof(input_leds) / sizeof(input_leds[0]); i++)
+			if (input_leds[i].name)
+				input_inject_event(handle, EV_LED, i,
+						!!(input_led_leds & (1 << i)));
+		input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
+	}
+	spin_unlock_irqrestore(&input_led_lock, flags);
+}
+
+static const struct input_device_id input_led_ids[] = {
+	{
+		.flags = INPUT_DEVICE_ID_MATCH_EVBIT,
+		.evbit = { BIT_MASK(EV_LED) },
+	},
+
+	{ },	/* Terminating entry */
+};
+
+static struct input_handler input_led_handler = {
+	.connect	= input_led_connect,
+	.disconnect	= input_led_disconnect,
+	.start		= input_led_start,
+	.name		= "input leds",
+	.id_table	= input_led_ids,
+};
+
+static int __init input_led_init(void)
+{
+	int i;
+	int error;
+
+	error = input_register_handler(&input_led_handler);
+	if (error)
+		return error;
+
+	for (i = 0; i < sizeof(input_leds) / sizeof(input_leds[0]); i++)
+		if (input_leds[i].name) {
+			error = led_classdev_register(NULL, &input_leds[i]);
+			if (error)
+				break;
+		}
+
+	if (error) {
+		for (i--; i >= 0; i--)
+			if (input_leds[i].name)
+				led_classdev_unregister(&input_leds[i]);
+		return error;
+	}
+
+	return 0;
+}
+
+static void __exit input_led_exit(void)
+{
+	int i;
+
+	input_unregister_handler(&input_led_handler);
+
+	for (i = 0; i < sizeof(input_leds) / sizeof(input_leds[0]); i++)
+		if (input_leds[i].name)
+			led_classdev_unregister(&input_leds[i]);
+}
+
+module_init(input_led_init);
+module_exit(input_led_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("User LED support for input layer");
+MODULE_AUTHOR("Samuel Thibault <samuel.thibault@ens-lyon.org>");
diff -ur linux-2.6.32-orig/drivers/leds/Makefile linux-2.6.32-perso/drivers/leds/Makefile
--- linux-2.6.32-orig/drivers/leds/Makefile	2009-12-03 13:42:57.000000000 +0100
+++ linux-2.6.32-perso/drivers/leds/Makefile	2010-02-21 03:37:08.000000000 +0100
@@ -5,6 +5,7 @@
 obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
 
 # LED Platform Drivers
+obj-$(CONFIG_LEDS_INPUT)		+= leds-input.o
 obj-$(CONFIG_LEDS_ATMEL_PWM)		+= leds-atmel-pwm.o
 obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
 obj-$(CONFIG_LEDS_LOCOMO)		+= leds-locomo.o
diff -ur linux-2.6.32-orig/lib/Kconfig.debug linux-2.6.32-perso/lib/Kconfig.debug
--- linux-2.6.32-orig/lib/Kconfig.debug	2009-12-03 13:44:04.000000000 +0100
+++ linux-2.6.32-perso/lib/Kconfig.debug	2010-02-21 04:36:23.000000000 +0100
@@ -103,7 +103,7 @@
 
 config DEBUG_SECTION_MISMATCH
 	bool "Enable full Section mismatch analysis"
-	depends on UNDEFINED
+	#depends on UNDEFINED
 	# This option is on purpose disabled for now.
 	# It will be enabled when we are down to a resonable number
 	# of section mismatch warnings (< 10 for an allyesconfig build)

Re: [RFC,PATCH] Route kbd leds through the generic leds layer (Was: [PATCH] kbd: (#7063) make CapsLock work as expected even for non-ASCII)

[Pavel Machek]

Hi!

> Route keyboard leds through the generic leds layer.
> 
> This permits to reassign keyboard LEDs to something else than keyboard
> "leds" state, and also permits to fix #7063 by using a modifier to
> implement proper CapsLock behavior and have the keyboard caps lock led
> show that caps lock state.

Yes please.

> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

Looks good. ACK.

> --- linux-2.6.32-orig/lib/Kconfig.debug	2009-12-03 13:44:04.000000000 +0100
> +++ linux-2.6.32-perso/lib/Kconfig.debug	2010-02-21 04:36:23.000000000 +0100
> @@ -103,7 +103,7 @@
>  
>  config DEBUG_SECTION_MISMATCH
>  	bool "Enable full Section mismatch analysis"
> -	depends on UNDEFINED
> +	#depends on UNDEFINED
>  	# This option is on purpose disabled for now.
>  	# It will be enabled when we are down to a resonable number
>  	# of section mismatch warnings (< 10 for an allyesconfig build)

You probably want to leave this bit.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html