[RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[Joshua C.]

I rebased this on the latest kernel-3.3-rc5 so that it can be properly applied.
------------------

>From 36e15b8d9d491817a3bada5ef9375aabe9439d9b Mon Sep 17 00:00:00 2012
From: Joshua Cov <joshuacov@googlemail.com>
Date: Mon, 27 Feb 2012 20:49:18 +0100
Subject: [PATCH] Use BIOS Keyboard variable to set Numlock

The PC BIOS does provide a NUMLOCK flag containing the desired state
of this LED. Bit 0x417 has the current keyboard modifier state. This
patch sets the current state according to the data in the bios and
introduces a module parameter "numlock" which can be used to
explicitely disable the NumLock (1 = enable, 0 = disable).

See first discussion back in 2007 at:
http://lkml.indiana.edu/hypermail/linux/kernel/0707.1/1834.html

Signed-Off-By: Joshua Cov <joshuacov@googlemail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Bodo Eggert <7eggert@gmx.de>

---
 drivers/input/keyboard/Kconfig |   14 ++++++++++++++
 drivers/tty/vt/keyboard.c      |   21 +++++++++++++++++++--
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index cdc385b..1dd1965 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -84,6 +84,20 @@ config KEYBOARD_ATKBD
 	  To compile this driver as a module, choose M here: the
 	  module will be called atkbd.

+config KBD_DEFLEDS_PCBIOS
+	bool "Enable Num-Lock based on BIOS settings"
+	depends on KEYBOARD_ATKBD && X86
+	default y
+	help
+	  Turns on Numlock depending on the BIOS settings.
+	  This works by reading the BIOS data area as defined for IBM PCs (1981).
+	  You can also controll the NumLock state with the kernel parameter
+	  numlock.
+
+	  If you have an alternative firmware like OpenFirmware or LinuxBios,
+	  this flag might not be set correctly, which results in a random state
+	  of the Numlock key.
+
 config KEYBOARD_ATKBD_HP_KEYCODES
 	bool "Use HP keyboard scancodes"
 	depends on PARISC && KEYBOARD_ATKBD
diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index a605549..d254396 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -24,6 +24,7 @@

 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

+#include <asm/io.h>
 #include <linux/consolemap.h>
 #include <linux/module.h>
 #include <linux/sched.h>
@@ -59,7 +60,13 @@ extern void ctrl_alt_del(void);
  * to be used for numbers.
  */

-#if defined(CONFIG_PARISC) && (defined(CONFIG_KEYBOARD_HIL) ||
defined(CONFIG_KEYBOARD_HIL_OLD))
+#ifdef CONFIG_KBD_DEFLEDS_PCBIOS
+/* KBD_DEFLEDS is a variable */
+#undef KBD_DEFLEDS
+	static int numlock = 1;
+	MODULE_PARM_DESC(numlock, "Toggle Numlock (1 = enable, 0 = disable)");
+	module_param_named(numlock, numlock, int, 0400);
+#elif defined(CONFIG_PARISC) && (defined(CONFIG_KEYBOARD_HIL) ||
defined(CONFIG_KEYBOARD_HIL_OLD))
 #define KBD_DEFLEDS (1 << VC_NUMLOCK)
 #else
 #define KBD_DEFLEDS 0
@@ -1432,7 +1439,17 @@ int __init kbd_init(void)
 {
 	int i;
 	int error;
-
+	
+#ifdef CONFIG_KBD_DEFLEDS_PCBIOS
+	int KBD_DEFLEDS = 0;
+	/* address 0x40:0x17 */
+	char * bios_kbd_status=xlate_dev_mem_ptr(0x417);
+	
+	/* Numlock status bit set? */
+	if ((*bios_kbd_status & 0x20) && numlock)
+	KBD_DEFLEDS = 1 << VC_NUMLOCK;
+#endif
+	
         for (i = 0; i < MAX_NR_CONSOLES; i++) {
 		kbd_table[i].ledflagstate = KBD_DEFLEDS;
 		kbd_table[i].default_ledflagstate = KBD_DEFLEDS;
-- 
1.7.7.6

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[H. Peter Anvin]

On 02/27/2012 11:23 AM, Joshua C. wrote:
> I rebased this on the latest kernel-3.3-rc5 so that it can be properly applied.
> ------------------
> 
> From 36e15b8d9d491817a3bada5ef9375aabe9439d9b Mon Sep 17 00:00:00 2012
> From: Joshua Cov <joshuacov@googlemail.com>
> Date: Mon, 27 Feb 2012 20:49:18 +0100
> Subject: [PATCH] Use BIOS Keyboard variable to set Numlock
> 
> The PC BIOS does provide a NUMLOCK flag containing the desired state
> of this LED. Bit 0x417 has the current keyboard modifier state. This
> patch sets the current state according to the data in the bios and
> introduces a module parameter "numlock" which can be used to
> explicitely disable the NumLock (1 = enable, 0 = disable).
> 
> See first discussion back in 2007 at:
> http://lkml.indiana.edu/hypermail/linux/kernel/0707.1/1834.html
> 
> Signed-Off-By: Joshua Cov <joshuacov@googlemail.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Bodo Eggert <7eggert@gmx.de>
> 

A better idea might be to query the status in the BIOS bootstrap code --
then non-BIOS boots can do the equivalent.  It also has the side benefit
of making people running Grub2 perhaps realize that they are f****ng
themselves over by using the "linux" command and not "linux16", because
of course policy in Grub2 is that if there is a sane way to do it, make
sure it is NOT the default.

	-hpa

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[Joshua C.]

2012/2/27 H. Peter Anvin <hpa@zytor.com>:
> On 02/27/2012 11:23 AM, Joshua C. wrote:
>> I rebased this on the latest kernel-3.3-rc5 so that it can be properly applied.
>> ------------------
>>
>> From 36e15b8d9d491817a3bada5ef9375aabe9439d9b Mon Sep 17 00:00:00 2012
>> From: Joshua Cov <joshuacov@googlemail.com>
>> Date: Mon, 27 Feb 2012 20:49:18 +0100
>> Subject: [PATCH] Use BIOS Keyboard variable to set Numlock
>>
>> The PC BIOS does provide a NUMLOCK flag containing the desired state
>> of this LED. Bit 0x417 has the current keyboard modifier state. This
>> patch sets the current state according to the data in the bios and
>> introduces a module parameter "numlock" which can be used to
>> explicitely disable the NumLock (1 = enable, 0 = disable).
>>
>> See first discussion back in 2007 at:
>> http://lkml.indiana.edu/hypermail/linux/kernel/0707.1/1834.html
>>
>> Signed-Off-By: Joshua Cov <joshuacov@googlemail.com>
>> Cc: H. Peter Anvin <hpa@zytor.com>
>> Cc: Bodo Eggert <7eggert@gmx.de>
>>
>
> A better idea might be to query the status in the BIOS bootstrap code --
> then non-BIOS boots can do the equivalent.  It also has the side benefit
> of making people running Grub2 perhaps realize that they are f****ng
> themselves over by using the "linux" command and not "linux16", because
> of course policy in Grub2 is that if there is a sane way to do it, make
> sure it is NOT the default.
>
>        -hpa
>

Do you mind querying the state in Grub? This will mean that we'll have
to remove the code that sets this bit from the kernel and leave it the
in the state reported by the grub?

If so I'm not sure about it. We check the BIOS data area as defined
for IBM PCs (1981), so a fair amount of user should benefit from the
change. Those non-BIOS boots can set the numlock=0 and won't be
affected by this. I think this isa lot easier to implement than doing
it in the BIOS bootstrap code.

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[H. Peter Anvin]

> 
> Do you mind querying the state in Grub? This will mean that we'll have
> to remove the code that sets this bit from the kernel and leave it the
> in the state reported by the grub?
> 

Doing anything in Grub is idiotic at best.  The place to do it is in the
kernel BIOS stub code.

> If so I'm not sure about it. We check the BIOS data area as defined
> for IBM PCs (1981), so a fair amount of user should benefit from the
> change. Those non-BIOS boots can set the numlock=0 and won't be
> affected by this. I think this isa lot easier to implement than doing
> it in the BIOS bootstrap code.

Not really, and your patch really shows it.  It probably could be done
in less than 20 lines, total.

	-hpa

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[H. Peter Anvin]

[-- Attachment #1: Type: text/plain, Size: 502 bytes --]

On 02/27/2012 04:08 PM, Joshua C. wrote:
> 
> If so I'm not sure about it. We check the BIOS data area as defined
> for IBM PCs (1981), so a fair amount of user should benefit from the
> change. Those non-BIOS boots can set the numlock=0 and won't be
> affected by this. I think this isa lot easier to implement than doing
> it in the BIOS bootstrap code.
> 

Here is a patch to query the BIOS state properly; if you could fill out
the rest of the patch then we can merge this in easily enough.

	-hpa

[-- Attachment #2: diff --]
[-- Type: text/plain, Size: 1753 bytes --]

diff --git a/arch/x86/boot/main.c b/arch/x86/boot/main.c
index 40358c8..cf6083d 100644
--- a/arch/x86/boot/main.c
+++ b/arch/x86/boot/main.c
@@ -57,14 +57,20 @@ static void copy_boot_params(void)
 }
 
 /*
- * Set the keyboard repeat rate to maximum.  Unclear why this
+ * Query the keyboard lock status as given by the BIOS, and
+ * set the keyboard repeat rate to maximum.  Unclear why the latter
  * is done here; this might be possible to kill off as stale code.
  */
-static void keyboard_set_repeat(void)
+static void keyboard_init(void)
 {
-	struct biosregs ireg;
+	struct biosregs ireg, oreg;
 	initregs(&ireg);
-	ireg.ax = 0x0305;
+
+	ireg.ah = 0x02;		/* Get keyboard status */
+	intcall(0x16, &ireg, &oreg);
+	boot_params.kbd_status = oreg.al;
+
+	ireg.ax = 0x0305;	/* Set keyboard repeat rate */
 	intcall(0x16, &ireg, NULL);
 }
 
@@ -151,8 +157,8 @@ void main(void)
 	/* Detect memory layout */
 	detect_memory();
 
-	/* Set keyboard repeat rate (why?) */
-	keyboard_set_repeat();
+	/* Set keyboard repeat rate (why?) and query the lock flags */
+	keyboard_init();
 
 	/* Query MCA information */
 	query_mca();
diff --git a/arch/x86/include/asm/bootparam.h b/arch/x86/include/asm/bootparam.h
index 2f90c51..eb45aa6 100644
--- a/arch/x86/include/asm/bootparam.h
+++ b/arch/x86/include/asm/bootparam.h
@@ -112,7 +112,8 @@ struct boot_params {
 	__u8  e820_entries;				/* 0x1e8 */
 	__u8  eddbuf_entries;				/* 0x1e9 */
 	__u8  edd_mbr_sig_buf_entries;			/* 0x1ea */
-	__u8  _pad6[6];					/* 0x1eb */
+	__u8  kbd_status;				/* 0x1eb */
+	__u8  _pad6[5];					/* 0x1ec */
 	struct setup_header hdr;    /* setup header */	/* 0x1f1 */
 	__u8  _pad7[0x290-0x1f1-sizeof(struct setup_header)];
 	__u32 edd_mbr_sig_buffer[EDD_MBR_SIG_MAX];	/* 0x290 */

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[Joshua C.]

2012/2/28 H. Peter Anvin <hpa@zytor.com>:
> On 02/27/2012 04:08 PM, Joshua C. wrote:
>>
>> If so I'm not sure about it. We check the BIOS data area as defined
>> for IBM PCs (1981), so a fair amount of user should benefit from the
>> change. Those non-BIOS boots can set the numlock=0 and won't be
>> affected by this. I think this isa lot easier to implement than doing
>> it in the BIOS bootstrap code.
>>
>
> Here is a patch to query the BIOS state properly; if you could fill out
> the rest of the patch then we can merge this in easily enough.
>
>        -hpa

I think this should be the missing part:

@@ -1432,7 +1439,17 @@ int __init kbd_init(void)
 {
 	int i;
 	int error;
-
+	
+#ifdef CONFIG_KBD_DEFLEDS_PCBIOS
+	int KBD_DEFLEDS = 0;
+	char * bios_kbd_lock_status=boot_params.kbd_status
+	
+	/* Numlock status bit set? */
+	if ((*bios_kbd_lock_status & 0x20) && numlock)
+	KBD_DEFLEDS = 1 << VC_NUMLOCK;
+#endif
+	
         for (i = 0; i < MAX_NR_CONSOLES; i++) {
 		kbd_table[i].ledflagstate = KBD_DEFLEDS;
 		kbd_table[i].default_ledflagstate = KBD_DEFLEDS;

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[Bodo Eggert]

On Mon, 27 Feb 2012, H. Peter Anvin wrote:
> On 02/27/2012 04:08 PM, Joshua C. wrote:

>> If so I'm not sure about it. We check the BIOS data area as defined
>> for IBM PCs (1981), so a fair amount of user should benefit from the
>> change. Those non-BIOS boots can set the numlock=0 and won't be
>> affected by this. I think this isa lot easier to implement than doing
>> it in the BIOS bootstrap code.
>>
>
> Here is a patch to query the BIOS state properly; if you could fill out
> the rest of the patch then we can merge this in easily enough.

Asking the BIOS is as correct as querying the memory location (defined to 
have same result), but more expensive.

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[H. Peter Anvin]

On 02/28/2012 10:32 AM, Bodo Eggert wrote:
> Asking the BIOS is as correct as querying the memory location (defined
> to have same result), but more expensive.

Not quite, in reality; it is more likely to work on systems which 
implement various kinds of bypass schemes.  The key aspect of this, 
though, is that this is done on a BIOS path and not by groping a memory 
location which may not even be initialized on non-BIOS systems.

	-hpa

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[Joshua C.]

2012/2/28 H. Peter Anvin <hpa@zytor.com>:
> On 02/28/2012 10:32 AM, Bodo Eggert wrote:
>>
>> Asking the BIOS is as correct as querying the memory location (defined
>> to have same result), but more expensive.
>
>
> Not quite, in reality; it is more likely to work on systems which implement
> various kinds of bypass schemes.  The key aspect of this, though, is that
> this is done on a BIOS path and not by groping a memory location which may
> not even be initialized on non-BIOS systems.
>
>        -hpa

I got the idea that we should somehow check the kbd_state and set the
numlock accordingly but this is behind my capabilities. I tried
several times to read those boot_params but I have no idea how to do
it. Where are they stored, how to access them? Help anyone?

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[Bodo Eggert]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1389 bytes --]

On Wed, 29 Feb 2012, Joshua C. wrote:
> 2012/2/28 H. Peter Anvin <hpa@zytor.com>:
>> On 02/28/2012 10:32 AM, Bodo Eggert wrote:

>>> Asking the BIOS is as correct as querying the memory location (defined
>>> to have same result), but more expensive.
>>
>>
>> Not quite, in reality; it is more likely to work on systems which implement
>> various kinds of bypass schemes.  The key aspect of this, though, is that
>> this is done on a BIOS path and not by groping a memory location which may
>> not even be initialized on non-BIOS systems.

> I got the idea that we should somehow check the kbd_state and set the
> numlock accordingly but this is behind my capabilities. I tried
> several times to read those boot_params but I have no idea how to do
> it. Where are they stored, how to access them? Help anyone?

There are two bytes in the BIOS data area, one (0x417 + 0x418) containing 
the settings that are supposed to be set and one that contains the values 
sent to the keyboard most recently (0x496 + 0x497). Both values are 
compared on each int16 call and the LED are set accordingly if the 
corresponding bits differ.

You can change the LED states by setting the according byte and calling 
any keyboard interrupt function, e.g. querying the availability of 
characters in the buffer. I don't know about any other way to do the same 
using only the BIOS.

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[H. Peter Anvin]

On 02/29/2012 03:43 AM, Joshua C. wrote:
>
> I got the idea that we should somehow check the kbd_state and set the
> numlock accordingly but this is behind my capabilities. I tried
> several times to read those boot_params but I have no idea how to do
> it. Where are they stored, how to access them? Help anyone?
>

There is a global variable called boot_params; the prototype for it is 
defined in <asm/setup.h>.

So as long as you are on an x86 architecture you can just #include 
<asm/setup.h> and just access the boot_params structure directly (with 
the patch provided to add the new field.)

	-hpa

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[Joshua C.]

2012/2/29 H. Peter Anvin <hpa@zytor.com>:
> On 02/29/2012 03:43 AM, Joshua C. wrote:
>>
>>
>> I got the idea that we should somehow check the kbd_state and set the
>> numlock accordingly but this is behind my capabilities. I tried
>> several times to read those boot_params but I have no idea how to do
>> it. Where are they stored, how to access them? Help anyone?
>>
>
> There is a global variable called boot_params; the prototype for it is
> defined in <asm/setup.h>.
>
> So as long as you are on an x86 architecture you can just #include
> <asm/setup.h> and just access the boot_params structure directly (with the
> patch provided to add the new field.)
>
>        -hpa

I think I figured it out. This is the patch I tested on a desktop and
a laptop and it works the way it should.
--------------

>From 0e3f1393132fd61f43e950065ae888b5ee103a96 Mon Sep 17 00:00:00 2001
From: Joshua Cov <joshuacov@googlemail.com>
Date: Thu, 1 Mar 2012 00:21:55 +0100
Subject: [PATCH] Use BIOS Keyboard variable to set Numlock

The PC BIOS does provide a NUMLOCK flag containing the desired state
of the LED. This patch sets the current state according to the data in
the bios and introduces a module parameter "numlock" which can be used
to explicitely ignore the BIOS Setting (1 = use BIOS Setting, 0 =
don't use it).

Signed-Off-By: Joshua Cov <joshuacov@googlemail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Bodo Eggert <7eggert@gmx.de>

---
 arch/x86/boot/main.c             |   18 ++++++++++++------
 arch/x86/include/asm/bootparam.h |    3 ++-
 drivers/tty/vt/keyboard.c        |   37 +++++++++++++++++++++++--------------
 3 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/arch/x86/boot/main.c b/arch/x86/boot/main.c
index 40358c8..cf6083d 100644
--- a/arch/x86/boot/main.c
+++ b/arch/x86/boot/main.c
@@ -57,14 +57,20 @@ static void copy_boot_params(void)
 }

 /*
- * Set the keyboard repeat rate to maximum.  Unclear why this
+ * Query the keyboard lock status as given by the BIOS, and
+ * set the keyboard repeat rate to maximum.  Unclear why the latter
  * is done here; this might be possible to kill off as stale code.
  */
-static void keyboard_set_repeat(void)
+static void keyboard_init(void)
 {
-	struct biosregs ireg;
+	struct biosregs ireg, oreg;
 	initregs(&ireg);
-	ireg.ax = 0x0305;
+
+	ireg.ah = 0x02;		/* Get keyboard status */
+	intcall(0x16, &ireg, &oreg);
+	boot_params.kbd_status = oreg.al;
+
+	ireg.ax = 0x0305;	/* Set keyboard repeat rate */
 	intcall(0x16, &ireg, NULL);
 }

@@ -151,8 +157,8 @@ void main(void)
 	/* Detect memory layout */
 	detect_memory();

-	/* Set keyboard repeat rate (why?) */
-	keyboard_set_repeat();
+	/* Set keyboard repeat rate (why?) and query the lock flags */
+	keyboard_init();

 	/* Query MCA information */
 	query_mca();
diff --git a/arch/x86/include/asm/bootparam.h b/arch/x86/include/asm/bootparam.h
index 2f90c51..eb45aa6 100644
--- a/arch/x86/include/asm/bootparam.h
+++ b/arch/x86/include/asm/bootparam.h
@@ -112,7 +112,8 @@ struct boot_params {
 	__u8  e820_entries;				/* 0x1e8 */
 	__u8  eddbuf_entries;				/* 0x1e9 */
 	__u8  edd_mbr_sig_buf_entries;			/* 0x1ea */
-	__u8  _pad6[6];					/* 0x1eb */
+	__u8  kbd_status;				/* 0x1eb */
+	__u8  _pad6[5];					/* 0x1ec */
 	struct setup_header hdr;    /* setup header */	/* 0x1f1 */
 	__u8  _pad7[0x290-0x1f1-sizeof(struct setup_header)];
 	__u32 edd_mbr_sig_buffer[EDD_MBR_SIG_MAX];	/* 0x290 */
diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index a605549..40a33bf 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -24,6 +24,8 @@

 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

+#include <asm/io.h>
+#include <asm/setup.h>
 #include <linux/consolemap.h>
 #include <linux/module.h>
 #include <linux/sched.h>
@@ -52,19 +54,6 @@ extern void ctrl_alt_del(void);

 #define KBD_DEFMODE ((1 << VC_REPEAT) | (1 << VC_META))

-/*
- * Some laptops take the 789uiojklm,. keys as number pad when NumLock is on.
- * This seems a good reason to start with NumLock off. On HIL keyboards
- * of PARISC machines however there is no NumLock key and everyone
expects the keypad
- * to be used for numbers.
- */
-
-#if defined(CONFIG_PARISC) && (defined(CONFIG_KEYBOARD_HIL) ||
defined(CONFIG_KEYBOARD_HIL_OLD))
-#define KBD_DEFLEDS (1 << VC_NUMLOCK)
-#else
-#define KBD_DEFLEDS 0
-#endif
-
 #define KBD_DEFLOCK 0

 void compute_shiftstate(void);
@@ -1428,12 +1417,32 @@ static struct input_handler kbd_handler = {
 	.id_table	= kbd_ids,
 };

+/* Let the user decide if we should use the value from the BIOS. */
+static int numlock = 1;
+MODULE_PARM_DESC(numlock, "Should we use the NumLock state returned
by the BIOS? \
+				(1 = use BIOS Setting, 0 = don't use it)");
+module_param_named(numlock, numlock, int, 0400);
+
 int __init kbd_init(void)
 {
 	int i;
 	int error;

-        for (i = 0; i < MAX_NR_CONSOLES; i++) {
+	/*
+	* Some laptops take the 789uiojklm,. keys as number pad when NumLock is on.
+	* This seems a good reason to start with NumLock off. On HIL keyboards
+	* of PARISC machines however there is no NumLock key and everyone
expects the keypad
+	* to be used for numbers. That's why we start with NumLock off and
ask the bios
+	* for the correct state.
+	*/
+
+	int KBD_DEFLEDS = 0 << VC_NUMLOCK;
+
+	/* Numlock status bit set? */
+	if ((boot_params.kbd_status & 0x20) && numlock)
+		KBD_DEFLEDS = 1 << VC_NUMLOCK;
+
+	for (i = 0; i < MAX_NR_CONSOLES; i++) {
 		kbd_table[i].ledflagstate = KBD_DEFLEDS;
 		kbd_table[i].default_ledflagstate = KBD_DEFLEDS;
 		kbd_table[i].ledmode = LED_SHOW_FLAGS;
-- 
1.7.7.6

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[H. Peter Anvin]

Looks good, *except* that this will break the compile for all non-x86
architectures.  It might be worthwhile to factor out the
architecture-dependent bits for cleanliness.

	-hpa

On 02/29/2012 02:56 PM, Joshua C. wrote:
> diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
> index a605549..40a33bf 100644
> --- a/drivers/tty/vt/keyboard.c
> +++ b/drivers/tty/vt/keyboard.c
> @@ -24,6 +24,8 @@
> 
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> 
> +#include <asm/io.h>
> +#include <asm/setup.h>
>  #include <linux/consolemap.h>
>  #include <linux/module.h>
>  #include <linux/sched.h>
> @@ -52,19 +54,6 @@ extern void ctrl_alt_del(void);
> 
>  #define KBD_DEFMODE ((1 << VC_REPEAT) | (1 << VC_META))
> 
> -/*
> - * Some laptops take the 789uiojklm,. keys as number pad when NumLock is on.
> - * This seems a good reason to start with NumLock off. On HIL keyboards
> - * of PARISC machines however there is no NumLock key and everyone
> expects the keypad
> - * to be used for numbers.
> - */
> -
> -#if defined(CONFIG_PARISC) && (defined(CONFIG_KEYBOARD_HIL) ||
> defined(CONFIG_KEYBOARD_HIL_OLD))
> -#define KBD_DEFLEDS (1 << VC_NUMLOCK)
> -#else
> -#define KBD_DEFLEDS 0
> -#endif
> -
>  #define KBD_DEFLOCK 0
> 
>  void compute_shiftstate(void);
> @@ -1428,12 +1417,32 @@ static struct input_handler kbd_handler = {
>  	.id_table	= kbd_ids,
>  };
> 
> +/* Let the user decide if we should use the value from the BIOS. */
> +static int numlock = 1;
> +MODULE_PARM_DESC(numlock, "Should we use the NumLock state returned
> by the BIOS? \
> +				(1 = use BIOS Setting, 0 = don't use it)");
> +module_param_named(numlock, numlock, int, 0400);
> +
>  int __init kbd_init(void)
>  {
>  	int i;
>  	int error;
> 
> -        for (i = 0; i < MAX_NR_CONSOLES; i++) {
> +	/*
> +	* Some laptops take the 789uiojklm,. keys as number pad when NumLock is on.
> +	* This seems a good reason to start with NumLock off. On HIL keyboards
> +	* of PARISC machines however there is no NumLock key and everyone
> expects the keypad
> +	* to be used for numbers. That's why we start with NumLock off and
> ask the bios
> +	* for the correct state.
> +	*/
> +
> +	int KBD_DEFLEDS = 0 << VC_NUMLOCK;
> +
> +	/* Numlock status bit set? */
> +	if ((boot_params.kbd_status & 0x20) && numlock)
> +		KBD_DEFLEDS = 1 << VC_NUMLOCK;
> +
> +	for (i = 0; i < MAX_NR_CONSOLES; i++) {
>  		kbd_table[i].ledflagstate = KBD_DEFLEDS;
>  		kbd_table[i].default_ledflagstate = KBD_DEFLEDS;
>  		kbd_table[i].ledmode = LED_SHOW_FLAGS;


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

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[Joshua C.]

2012/3/1 H. Peter Anvin <hpa@zytor.com>:
> Looks good, *except* that this will break the compile for all non-x86
> architectures.  It might be worthwhile to factor out the
> architecture-dependent bits for cleanliness.
>
>        -hpa
>


Will this work?

+#if (defined(__i386__) || defined(__x86_64__))
+#include <asm/io.h>
+#include <asm/setup.h>
+#else
+extern struct boot_params boot_params;
+#endif

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[H. Peter Anvin]

On 02/29/2012 03:51 PM, Joshua C. wrote:
> 
> Will this work?
> 
> +#if (defined(__i386__) || defined(__x86_64__))
> +#include <asm/io.h>
> +#include <asm/setup.h>
> +#else
> +extern struct boot_params boot_params;
> +#endif
> 

A much better way to do this is probably something like

#ifdef CONFIG_X86

#include <asm/setup.h>

static inline int kbd_defleds(void)
{
	return boot_param.kbd_status & 0x20 ? (1 << VC_NUMLOCK) : 0;
}

#elif defined(CONFIG_PARISC)
static inline int kbd_defleds(void)
{
# if defined(CONFIG_KEYBOARD_HIL) || defined(CONFIG_KEYBOARD_HIL_OLD)
	return 1 << VC_NUMLOCK;
# else
	return 0;
# endif
}

#else

static inline int kbd_defleds(void)
{
	return 0;
}

#endif

... then arguably this should be moved into the arch/* directories, in a
header file or by making this a callable function.


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

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[Joshua C.]

2012/3/1 H. Peter Anvin <hpa@zytor.com>:
> On 02/29/2012 03:51 PM, Joshua C. wrote:
>>
>> Will this work?
>>
>> +#if (defined(__i386__) || defined(__x86_64__))
>> +#include <asm/io.h>
>> +#include <asm/setup.h>
>> +#else
>> +extern struct boot_params boot_params;
>> +#endif
>>
>
> A much better way to do this is probably something like
>
> #ifdef CONFIG_X86
>
> #include <asm/setup.h>
>
> static inline int kbd_defleds(void)
> {
>        return boot_param.kbd_status & 0x20 ? (1 << VC_NUMLOCK) : 0;
> }
>
> #elif defined(CONFIG_PARISC)
> static inline int kbd_defleds(void)
> {
> # if defined(CONFIG_KEYBOARD_HIL) || defined(CONFIG_KEYBOARD_HIL_OLD)
>        return 1 << VC_NUMLOCK;
> # else
>        return 0;
> # endif
> }
>
> #else
>
> static inline int kbd_defleds(void)
> {
>        return 0;
> }
>
> #endif
>
> ... then arguably this should be moved into the arch/* directories, in a
> header file or by making this a callable function.
>
>
> --
> H. Peter Anvin, Intel Open Source Technology Center
> I work for Intel.  I don't speak on their behalf.
>

Thanks for the help. You can better figure out where this code should
go into. Can you prepare the patch for merging in the mainline kernel?

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[H. Peter Anvin]

On 02/29/2012 04:21 PM, Joshua C. wrote:
>
> Thanks for the help. You can better figure out where this code should
> go into. Can you prepare the patch for merging in the mainline kernel?
>

Yes, although "when" would be a very open question.

I realize it is frustrating to do these kinds of things for the first 
time and getting what feels like a lot of very picky feedback.  The hope 
is that the learning experience makes the second, and third, and fourth 
time a lot less painful.

	-hpa

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[Joshua C.]

2012/3/1 H. Peter Anvin <hpa@zytor.com>:
> On 02/29/2012 04:21 PM, Joshua C. wrote:
>>
>>
>> Thanks for the help. You can better figure out where this code should
>> go into. Can you prepare the patch for merging in the mainline kernel?
>>
>
> Yes, although "when" would be a very open question.
>
> I realize it is frustrating to do these kinds of things for the first time
> and getting what feels like a lot of very picky feedback.  The hope is that
> the learning experience makes the second, and third, and fourth time a lot
> less painful.
>
>        -hpa

Your're right. I wasn't sure about some parts of the code but it
finally worked out. I haven't worked with any non x86 system and I was
guessing what should be fixed. In the meantime I'll ask for this to be
merged in the fedora kernel. Thanks once again.

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[Joshua C.]

2012/3/1 Joshua C. <joshuacov@googlemail.com>:
> 2012/3/1 H. Peter Anvin <hpa@zytor.com>:
>> On 02/29/2012 04:21 PM, Joshua C. wrote:
>>>
>>>
>>> Thanks for the help. You can better figure out where this code should
>>> go into. Can you prepare the patch for merging in the mainline kernel?
>>>
>>
>> Yes, although "when" would be a very open question.
>>
>> I realize it is frustrating to do these kinds of things for the first time
>> and getting what feels like a lot of very picky feedback.  The hope is that
>> the learning experience makes the second, and third, and fourth time a lot
>> less painful.
>>
>>        -hpa
>
> Your're right. I wasn't sure about some parts of the code but it
> finally worked out. I haven't worked with any non x86 system and I was
> guessing what should be fixed. In the meantime I'll ask for this to be
> merged in the fedora kernel. Thanks once again.

I think this is the final version of the patch:

---
 arch/x86/boot/main.c             |   18 +++++++++++-----
 arch/x86/include/asm/bootparam.h |    3 +-
 arch/x86/include/asm/kbdleds.h   |   41 ++++++++++++++++++++++++++++++++++++++
 drivers/tty/vt/keyboard.c        |   13 +++--------
 4 files changed, 59 insertions(+), 16 deletions(-)
 create mode 100644 arch/x86/include/asm/kbdleds.h

diff --git a/arch/x86/boot/main.c b/arch/x86/boot/main.c
index 40358c8..cf6083d 100644
--- a/arch/x86/boot/main.c
+++ b/arch/x86/boot/main.c
@@ -57,14 +57,20 @@ static void copy_boot_params(void)
 }

 /*
- * Set the keyboard repeat rate to maximum.  Unclear why this
+ * Query the keyboard lock status as given by the BIOS, and
+ * set the keyboard repeat rate to maximum.  Unclear why the latter
  * is done here; this might be possible to kill off as stale code.
  */
-static void keyboard_set_repeat(void)
+static void keyboard_init(void)
 {
-	struct biosregs ireg;
+	struct biosregs ireg, oreg;
 	initregs(&ireg);
-	ireg.ax = 0x0305;
+
+	ireg.ah = 0x02;		/* Get keyboard status */
+	intcall(0x16, &ireg, &oreg);
+	boot_params.kbd_status = oreg.al;
+
+	ireg.ax = 0x0305;	/* Set keyboard repeat rate */
 	intcall(0x16, &ireg, NULL);
 }

@@ -151,8 +157,8 @@ void main(void)
 	/* Detect memory layout */
 	detect_memory();

-	/* Set keyboard repeat rate (why?) */
-	keyboard_set_repeat();
+	/* Set keyboard repeat rate (why?) and query the lock flags */
+	keyboard_init();

 	/* Query MCA information */
 	query_mca();
diff --git a/arch/x86/include/asm/bootparam.h b/arch/x86/include/asm/bootparam.h
index 2f90c51..eb45aa6 100644
--- a/arch/x86/include/asm/bootparam.h
+++ b/arch/x86/include/asm/bootparam.h
@@ -112,7 +112,8 @@ struct boot_params {
 	__u8  e820_entries;				/* 0x1e8 */
 	__u8  eddbuf_entries;				/* 0x1e9 */
 	__u8  edd_mbr_sig_buf_entries;			/* 0x1ea */
-	__u8  _pad6[6];					/* 0x1eb */
+	__u8  kbd_status;				/* 0x1eb */
+	__u8  _pad6[5];					/* 0x1ec */
 	struct setup_header hdr;    /* setup header */	/* 0x1f1 */
 	__u8  _pad7[0x290-0x1f1-sizeof(struct setup_header)];
 	__u32 edd_mbr_sig_buffer[EDD_MBR_SIG_MAX];	/* 0x290 */
diff --git a/arch/x86/include/asm/kbdleds.h b/arch/x86/include/asm/kbdleds.h
new file mode 100644
index 0000000..1446a65
--- /dev/null
+++ b/arch/x86/include/asm/kbdleds.h
@@ -0,0 +1,41 @@
+#ifndef _ASM_X86_KBDLEDS_H
+#define _ASM_X86_KBDLEDS_H
+
+/*
+ * Some laptops take the 789uiojklm,. keys as number pad when NumLock is on.
+ * This seems a good reason to start with NumLock off. On HIL keyboards
+ * of PARISC machines however there is no NumLock key and everyone expects
+ * the keypad to be used for numbers. That's why on X86 we ask the bios for
+ * the correct state.
+ */
+
+#ifdef CONFIG_X86
+
+#include <asm/setup.h>
+
+static inline int kbd_defleds(void)
+{
+	return boot_params.kbd_status & 0x20 ? (1 << VC_NUMLOCK) : 0;
+}
+
+#elif defined(CONFIG_PARISC)
+
+static inline int kbd_defleds(void)
+{
+#if defined(CONFIG_KEYBOARD_HIL) || defined(CONFIG_KEYBOARD_HIL_OLD)
+	return 1 << VC_NUMLOCK;
+#else
+	return 0;
+#endif
+}
+
+#else
+
+static inline int kbd_defleds(void)
+{
+	return 0;
+}
+
+#endif /* CONFIG_X86 */
+
+#endif /* _ASM_X86_KBDLEDS_H */
diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index a605549..4474d18 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -43,6 +43,7 @@
 #include <linux/jiffies.h>

 #include <asm/irq_regs.h>
+#include <asm/kbdleds.h>

 extern void ctrl_alt_del(void);

@@ -59,12 +60,6 @@ extern void ctrl_alt_del(void);
  * to be used for numbers.
  */

-#if defined(CONFIG_PARISC) && (defined(CONFIG_KEYBOARD_HIL) ||
defined(CONFIG_KEYBOARD_HIL_OLD))
-#define KBD_DEFLEDS (1 << VC_NUMLOCK)
-#else
-#define KBD_DEFLEDS 0
-#endif
-
 #define KBD_DEFLOCK 0

 void compute_shiftstate(void);
@@ -1433,9 +1428,9 @@ int __init kbd_init(void)
 	int i;
 	int error;

-        for (i = 0; i < MAX_NR_CONSOLES; i++) {
-		kbd_table[i].ledflagstate = KBD_DEFLEDS;
-		kbd_table[i].default_ledflagstate = KBD_DEFLEDS;
+	for (i = 0; i < MAX_NR_CONSOLES; i++) {
+		kbd_table[i].ledflagstate = kbd_defleds();
+		kbd_table[i].default_ledflagstate = kbd_defleds();
 		kbd_table[i].ledmode = LED_SHOW_FLAGS;
 		kbd_table[i].lockstate = KBD_DEFLOCK;
 		kbd_table[i].slockstate = 0;
-- 
1.7.7.6

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[H. Peter Anvin]

On 03/01/2012 11:42 AM, Joshua C. wrote:
> diff --git a/arch/x86/include/asm/kbdleds.h b/arch/x86/include/asm/kbdleds.h
> new file mode 100644
> index 0000000..1446a65
> --- /dev/null
> +++ b/arch/x86/include/asm/kbdleds.h
> @@ -0,0 +1,41 @@
> +#ifndef _ASM_X86_KBDLEDS_H
> +#define _ASM_X86_KBDLEDS_H
> +
> +/*
> + * Some laptops take the 789uiojklm,. keys as number pad when NumLock is on.
> + * This seems a good reason to start with NumLock off. On HIL keyboards
> + * of PARISC machines however there is no NumLock key and everyone expects
> + * the keypad to be used for numbers. That's why on X86 we ask the bios for
> + * the correct state.
> + */
> +
> +#ifdef CONFIG_X86
> +
> +#include<asm/setup.h>
> +
> +static inline int kbd_defleds(void)
> +{
> +	return boot_params.kbd_status&  0x20 ? (1<<  VC_NUMLOCK) : 0;
> +}
> +
> +#elif defined(CONFIG_PARISC)
                  ^^^^^^^^^^^^^

Sorry, you can't put non-x86 code in arch/x86.  There are two way to 
distribute this kind of stuff into arch directories:

1. You can define an ARCH_ symbol which indicates that <asm/kbdleds.h> 
is available; otherwise use the fallback routine;

2. You can define it in a .c file as an actual function in the affected 
architectures (x86 and parisc here) and then define it as a weak symbol 
(grep for __weak) in the common code.

If you don't want to bite off this distribution I can understand it 
(although it's not up to me, technically, since I'm not the maintainer 
of the vt subsystem) but if so leave this as an inline in a common place.

	-hpa

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[Joshua C.]

2012/3/1 H. Peter Anvin <hpa@zytor.com>:
             ^^^^^^^^^^^^^
>
> Sorry, you can't put non-x86 code in arch/x86.  There are two way to
> distribute this kind of stuff into arch directories:
>
> 1. You can define an ARCH_ symbol which indicates that <asm/kbdleds.h> is
> available; otherwise use the fallback routine;
>
> 2. You can define it in a .c file as an actual function in the affected
> architectures (x86 and parisc here) and then define it as a weak symbol
> (grep for __weak) in the common code.
>
> If you don't want to bite off this distribution I can understand it
> (although it's not up to me, technically, since I'm not the maintainer of
> the vt subsystem) but if so leave this as an inline in a common place.
>
>        -hpa

I thought about your idea for separating the platform specific code in
another file. It will make the code more modular but at the end we're
talking about a single function that flips a single bit. I really
don't see any real benefit from defining it in a separate file. It
makes (from my point of view) reading the file just more difficult.
That's why I decided to return the platform specific code back where
it originally was and only modifiy the x86 code. So I can keep the
changes at minimum. I'll post the patch later today and cc the tty
maintainer. Thanks for the help and the ideas.

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[H. Peter Anvin]

On 03/02/2012 09:53 AM, Joshua C. wrote:
> 
> I thought about your idea for separating the platform specific code in
> another file. It will make the code more modular but at the end we're
> talking about a single function that flips a single bit. I really
> don't see any real benefit from defining it in a separate file. It
> makes (from my point of view) reading the file just more difficult.
> That's why I decided to return the platform specific code back where
> it originally was and only modifiy the x86 code. So I can keep the
> changes at minimum. I'll post the patch later today and cc the tty
> maintainer. Thanks for the help and the ideas.
> 

Yeah, it seems a bit excessive in this case.

	-hpa

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

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[Joshua C.]

2012/3/2 H. Peter Anvin <hpa@zytor.com>:
>
> Yeah, it seems a bit excessive in this case.
>
>        -hpa
>
> --
> H. Peter Anvin, Intel Open Source Technology Center
> I work for Intel.  I don't speak on their behalf.
>

This is the final version. It works fine here on one laptop and a desktop.

@Alan
Can you, please, queue this for the mainline and also push it to
stable? The patch is straight forward and set the NumLock on a
keyboard according to the data in the BIOS.

---
>From f061c2caeb98b34be41dc1d5ddda03658a7b3555 Mon Sep 17 00:00:00 2001
From: Joshua Cov <joshuacov@googlemail.com>
Date: Fri, 2 Mar 2012 22:44:33 +0100
Subject: [PATCH] Use BIOS Keyboard variable to set Numlock

The PC BIOS does provide a NUMLOCK flag containing the desired state
of this LED. This patch sets the current state according to the data
in the bios.

Signed-Off-By: Joshua Cov <joshuacov@googlemail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Bodo Eggert <7eggert@gmx.de>
Cc: Alan Cox <alan@linux.intel.com>

---
 arch/x86/boot/main.c             |   18 ++++++++++++------
 arch/x86/include/asm/bootparam.h |    3 ++-
 drivers/tty/vt/keyboard.c        |    9 ++++++---
 3 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/arch/x86/boot/main.c b/arch/x86/boot/main.c
index 40358c8..cf6083d 100644
--- a/arch/x86/boot/main.c
+++ b/arch/x86/boot/main.c
@@ -57,14 +57,20 @@ static void copy_boot_params(void)
 }

 /*
- * Set the keyboard repeat rate to maximum.  Unclear why this
+ * Query the keyboard lock status as given by the BIOS, and
+ * set the keyboard repeat rate to maximum.  Unclear why the latter
  * is done here; this might be possible to kill off as stale code.
  */
-static void keyboard_set_repeat(void)
+static void keyboard_init(void)
 {
-	struct biosregs ireg;
+	struct biosregs ireg, oreg;
 	initregs(&ireg);
-	ireg.ax = 0x0305;
+
+	ireg.ah = 0x02;		/* Get keyboard status */
+	intcall(0x16, &ireg, &oreg);
+	boot_params.kbd_status = oreg.al;
+
+	ireg.ax = 0x0305;	/* Set keyboard repeat rate */
 	intcall(0x16, &ireg, NULL);
 }

@@ -151,8 +157,8 @@ void main(void)
 	/* Detect memory layout */
 	detect_memory();

-	/* Set keyboard repeat rate (why?) */
-	keyboard_set_repeat();
+	/* Set keyboard repeat rate (why?) and query the lock flags */
+	keyboard_init();

 	/* Query MCA information */
 	query_mca();
diff --git a/arch/x86/include/asm/bootparam.h b/arch/x86/include/asm/bootparam.h
index 2f90c51..eb45aa6 100644
--- a/arch/x86/include/asm/bootparam.h
+++ b/arch/x86/include/asm/bootparam.h
@@ -112,7 +112,8 @@ struct boot_params {
 	__u8  e820_entries;				/* 0x1e8 */
 	__u8  eddbuf_entries;				/* 0x1e9 */
 	__u8  edd_mbr_sig_buf_entries;			/* 0x1ea */
-	__u8  _pad6[6];					/* 0x1eb */
+	__u8  kbd_status;				/* 0x1eb */
+	__u8  _pad6[5];					/* 0x1ec */
 	struct setup_header hdr;    /* setup header */	/* 0x1f1 */
 	__u8  _pad7[0x290-0x1f1-sizeof(struct setup_header)];
 	__u32 edd_mbr_sig_buffer[EDD_MBR_SIG_MAX];	/* 0x290 */
diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index a605549..7a95faf 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -56,10 +56,13 @@ extern void ctrl_alt_del(void);
  * Some laptops take the 789uiojklm,. keys as number pad when NumLock is on.
  * This seems a good reason to start with NumLock off. On HIL keyboards
  * of PARISC machines however there is no NumLock key and everyone
expects the keypad
- * to be used for numbers.
+ * to be used for numbers. That's why on X86 we ask the bios for the
correct state.
  */

-#if defined(CONFIG_PARISC) && (defined(CONFIG_KEYBOARD_HIL) ||
defined(CONFIG_KEYBOARD_HIL_OLD))
+#ifdef CONFIG_X86
+#include <asm/setup.h>
+#define KBD_DEFLEDS (boot_params.kbd_status & 0x20 ? (1 << VC_NUMLOCK) : 0)
+#elif defined(CONFIG_PARISC) && (defined(CONFIG_KEYBOARD_HIL) ||
defined(CONFIG_KEYBOARD_HIL_OLD))
 #define KBD_DEFLEDS (1 << VC_NUMLOCK)
 #else
 #define KBD_DEFLEDS 0
@@ -1433,7 +1436,7 @@ int __init kbd_init(void)
 	int i;
 	int error;

-        for (i = 0; i < MAX_NR_CONSOLES; i++) {
+	for (i = 0; i < MAX_NR_CONSOLES; i++) {
 		kbd_table[i].ledflagstate = KBD_DEFLEDS;
 		kbd_table[i].default_ledflagstate = KBD_DEFLEDS;
 		kbd_table[i].ledmode = LED_SHOW_FLAGS;
-- 
1.7.7.6

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[H. Peter Anvin]

On 03/02/2012 01:21 PM, Joshua C. wrote:
> 2012/3/2 H. Peter Anvin <hpa@zytor.com>:
>>
>> Yeah, it seems a bit excessive in this case.
>>
>>        -hpa
>>
>> --
>> H. Peter Anvin, Intel Open Source Technology Center
>> I work for Intel.  I don't speak on their behalf.
>>
> 
> This is the final version. It works fine here on one laptop and a desktop.
> 
> @Alan
> Can you, please, queue this for the mainline and also push it to
> stable? The patch is straight forward and set the NumLock on a
> keyboard according to the data in the BIOS.
> 

I still prefer the inline function instead of a macro for KBD_DEFLEDS,
but this is in deep nitpick territory at this point, and the macro is
already there.

Reviewed-by: H. Peter Anvin <hpa@zytor.com>


	-hpa

Re: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

[Joshua C.]

2012/3/21 Joshua C. <joshuacov@googlemail.com>:
> 2012/3/21 H. Peter Anvin <hpa@zytor.com>:
>>
>> He referring to myself and Ingo.  Note that what he's asking for is the
>> same thing I originally asked for as well.
>>
>>        -hpa

I re-worked the patch and moved the code around the way both of you
wanted it. It should be ready now for mainline, shouldn't it?

--joshua

---
 arch/parisc/include/asm/kbdleds.h |   19 +++++++++++++++++++
 arch/x86/boot/main.c              |   18 ++++++++++++------
 arch/x86/include/asm/bootparam.h  |    3 ++-
 arch/x86/include/asm/kbdleds.h    |   19 +++++++++++++++++++
 drivers/tty/vt/keyboard.c         |   21 +++++++--------------
 5 files changed, 59 insertions(+), 21 deletions(-)
 create mode 100644 arch/parisc/include/asm/kbdleds.h
 create mode 100644 arch/x86/include/asm/kbdleds.h

diff --git a/arch/parisc/include/asm/kbdleds.h
b/arch/parisc/include/asm/kbdleds.h
new file mode 100644
index 0000000..2a23ca9
--- /dev/null
+++ b/arch/parisc/include/asm/kbdleds.h
@@ -0,0 +1,19 @@
+#ifndef _ASM_PARISC_KBDLEDS_H
+#define _ASM_PARISC_KBDLEDS_H
+
+/*
+ * On HIL keyboards of PARISC machines there is no NumLock key and
+ * everyone expects the keypad to be used for numbers. That's why
+ * we can safely turn on the NUMLOCK bit.
+ */
+
+inline int kbd_defleds(void)
+{
+#if defined(CONFIG_KEYBOARD_HIL) || defined(CONFIG_KEYBOARD_HIL_OLD)
+	return 1 << VC_NUMLOCK;
+#else
+	return 0;
+#endif
+}
+
+#endif /* _ASM_PARISC_KBDLEDS_H */
diff --git a/arch/x86/boot/main.c b/arch/x86/boot/main.c
index 40358c8..cf6083d 100644
--- a/arch/x86/boot/main.c
+++ b/arch/x86/boot/main.c
@@ -57,14 +57,20 @@ static void copy_boot_params(void)
 }

 /*
- * Set the keyboard repeat rate to maximum.  Unclear why this
+ * Query the keyboard lock status as given by the BIOS, and
+ * set the keyboard repeat rate to maximum.  Unclear why the latter
  * is done here; this might be possible to kill off as stale code.
  */
-static void keyboard_set_repeat(void)
+static void keyboard_init(void)
 {
-	struct biosregs ireg;
+	struct biosregs ireg, oreg;
 	initregs(&ireg);
-	ireg.ax = 0x0305;
+
+	ireg.ah = 0x02;		/* Get keyboard status */
+	intcall(0x16, &ireg, &oreg);
+	boot_params.kbd_status = oreg.al;
+
+	ireg.ax = 0x0305;	/* Set keyboard repeat rate */
 	intcall(0x16, &ireg, NULL);
 }

@@ -151,8 +157,8 @@ void main(void)
 	/* Detect memory layout */
 	detect_memory();

-	/* Set keyboard repeat rate (why?) */
-	keyboard_set_repeat();
+	/* Set keyboard repeat rate (why?) and query the lock flags */
+	keyboard_init();

 	/* Query MCA information */
 	query_mca();
diff --git a/arch/x86/include/asm/bootparam.h b/arch/x86/include/asm/bootparam.h
index 2f90c51..eb45aa6 100644
--- a/arch/x86/include/asm/bootparam.h
+++ b/arch/x86/include/asm/bootparam.h
@@ -112,7 +112,8 @@ struct boot_params {
 	__u8  e820_entries;				/* 0x1e8 */
 	__u8  eddbuf_entries;				/* 0x1e9 */
 	__u8  edd_mbr_sig_buf_entries;			/* 0x1ea */
-	__u8  _pad6[6];					/* 0x1eb */
+	__u8  kbd_status;				/* 0x1eb */
+	__u8  _pad6[5];					/* 0x1ec */
 	struct setup_header hdr;    /* setup header */	/* 0x1f1 */
 	__u8  _pad7[0x290-0x1f1-sizeof(struct setup_header)];
 	__u32 edd_mbr_sig_buffer[EDD_MBR_SIG_MAX];	/* 0x290 */
diff --git a/arch/x86/include/asm/kbdleds.h b/arch/x86/include/asm/kbdleds.h
new file mode 100644
index 0000000..7639eb7
--- /dev/null
+++ b/arch/x86/include/asm/kbdleds.h
@@ -0,0 +1,19 @@
+#ifndef _ASM_X86_KBDLEDS_H
+#define _ASM_X86_KBDLEDS_H
+
+/*
+ * Some laptops take the 789uiojklm,. keys as number pad when NumLock is on.
+ * This seems a good reason to start with NumLock off. On HIL keyboards
+ * of PARISC machines however there is no NumLock key and everyone expects
+ * the keypad to be used for numbers. That's why on X86 we ask the bios for
+ * the correct state.
+ */
+
+#include <asm/setup.h>
+
+inline int kbd_defleds(void)
+{
+	return boot_params.kbd_status & 0x20 ? (1 << VC_NUMLOCK) : 0;
+}
+
+#endif /* _ASM_X86_KBDLEDS_H */
diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index a605549..25f8200 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -52,19 +52,12 @@ extern void ctrl_alt_del(void);

 #define KBD_DEFMODE ((1 << VC_REPEAT) | (1 << VC_META))

-/*
- * Some laptops take the 789uiojklm,. keys as number pad when NumLock is on.
- * This seems a good reason to start with NumLock off. On HIL keyboards
- * of PARISC machines however there is no NumLock key and everyone
expects the keypad
- * to be used for numbers.
- */
-
-#if defined(CONFIG_PARISC) && (defined(CONFIG_KEYBOARD_HIL) ||
defined(CONFIG_KEYBOARD_HIL_OLD))
-#define KBD_DEFLEDS (1 << VC_NUMLOCK)
-#else
-#define KBD_DEFLEDS 0
+#if defined(CONFIG_X86) || defined(CONFIG_PARISC)
+#include <asm/kbdleds.h>
 #endif

+extern __attribute__((weak)) int kbd_defleds(void);
+
 #define KBD_DEFLOCK 0

 void compute_shiftstate(void);
@@ -1433,9 +1426,9 @@ int __init kbd_init(void)
 	int i;
 	int error;

-        for (i = 0; i < MAX_NR_CONSOLES; i++) {
-		kbd_table[i].ledflagstate = KBD_DEFLEDS;
-		kbd_table[i].default_ledflagstate = KBD_DEFLEDS;
+	for (i = 0; i < MAX_NR_CONSOLES; i++) {
+		kbd_table[i].ledflagstate = kbd_defleds();
+		kbd_table[i].default_ledflagstate = kbd_defleds();
 		kbd_table[i].ledmode = LED_SHOW_FLAGS;
 		kbd_table[i].lockstate = KBD_DEFLOCK;
 		kbd_table[i].slockstate = 0;
-- 
1.7.7.6


-- joshua