2012-02-27 19:23:13

by tip-bot for Joshua Cov

[permalink] [raw]
Subject: [RESUBMIT] [PATCH] Use BIOS Keyboard variable to set Numlock

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 <[email protected]>
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 <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Bodo Eggert <[email protected]>

---
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


2012-02-27 21:10:04

by H. Peter Anvin

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

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 <[email protected]>
> 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 <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Bodo Eggert <[email protected]>
>

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

2012-02-28 00:08:38

by tip-bot for Joshua Cov

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

2012/2/27 H. Peter Anvin <[email protected]>:
> 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 <[email protected]>
>> 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 <[email protected]>
>> Cc: H. Peter Anvin <[email protected]>
>> Cc: Bodo Eggert <[email protected]>
>>
>
> 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.

2012-02-28 00:15:03

by H. Peter Anvin

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

>
> 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


2012-02-28 00:27:44

by H. Peter Anvin

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

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


Attachments:
diff (1.71 kB)

2012-02-28 09:14:10

by tip-bot for Joshua Cov

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

2012/2/28 H. Peter Anvin <[email protected]>:
> 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;

2012-02-28 18:32:59

by Bodo Eggert

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

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.

2012-02-28 18:35:41

by H. Peter Anvin

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

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

2012-02-29 11:43:07

by tip-bot for Joshua Cov

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

2012/2/28 H. Peter Anvin <[email protected]>:
> 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?

2012-02-29 17:54:06

by Bodo Eggert

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

On Wed, 29 Feb 2012, Joshua C. wrote:
> 2012/2/28 H. Peter Anvin <[email protected]>:
>> 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.

2012-02-29 18:17:15

by H. Peter Anvin

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

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

2012-02-29 22:56:53

by tip-bot for Joshua Cov

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

2012/2/29 H. Peter Anvin <[email protected]>:
> 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 <[email protected]>
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 <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Bodo Eggert <[email protected]>

---
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

2012-02-29 23:12:15

by H. Peter Anvin

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

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.

2012-02-29 23:51:22

by tip-bot for Joshua Cov

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

2012/3/1 H. Peter Anvin <[email protected]>:
> 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

2012-03-01 00:14:07

by H. Peter Anvin

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

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.

2012-03-01 00:21:09

by tip-bot for Joshua Cov

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

2012/3/1 H. Peter Anvin <[email protected]>:
> 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?

2012-03-01 00:24:11

by H. Peter Anvin

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

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

2012-03-01 00:28:23

by tip-bot for Joshua Cov

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

2012/3/1 H. Peter Anvin <[email protected]>:
> 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.