[PATCH 1/5] atkbd: Fix key release for Fn keys on Samsung series 5 ultra 540UC

[PATCH 1/5] atkbd: Fix key release for Fn keys on Samsung series 5 ultra 540UC

[Mauro Carvalho Chehab]

Samsung series 5 ultra notebooks don't produce release events for certain
keys (Fn+F1, Fn+F11, Fn+F12 and Fn Lock). Add those keys at the release
fixup table.

Note: Fn Lock actually produces two scancodes: 0xa8 and 0xa9.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/input/keyboard/atkbd.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 2626773..a18bf03 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -943,6 +943,13 @@ static unsigned int atkbd_samsung_forced_release_keys[] = {
 };
 
 /*
+ * Samsung 530U3C/530U4C/532U3C/540U3C Fn release keys not working
+ */
+static unsigned int atkbd_samsung_530u3c_forced_release_keys[] = {
+	0xa8, 0xa9, 0xb3, 0xce, 0xd5, -1U
+};
+
+/*
  * Amilo Pi 3525 key release for Fn+Volume keys not working
  */
 static unsigned int atkbd_amilo_pi3525_forced_release_keys[] = {
@@ -1732,6 +1739,15 @@ static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
 		.driver_data = atkbd_samsung_forced_release_keys,
 	},
 	{
+		/* Samsung series 5 Ultra (530U3C/530U4C/532U3C/540U3C) */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "530U3C"),
+		},
+		.callback = atkbd_setup_forced_release,
+		.driver_data = atkbd_samsung_530u3c_forced_release_keys,
+	},
+	{
 		/* Fujitsu Amilo PA 1510 */
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU SIEMENS"),
-- 
1.8.3.1

[PATCH 2/5] atkbd: Fix key release for Fn keys on Samsung series 5 550P5C

[Mauro Carvalho Chehab]

Fix key release for Fn keys with Samsung series 5

Samsung series 5 notebooks don't produce release events for certain
keys (Fn+F1, Fn+F11, Fn+F12 and Fn Lock). Add those keys at the release
fixup table.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/input/keyboard/atkbd.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index a18bf03..298ac1d 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -950,6 +950,13 @@ static unsigned int atkbd_samsung_530u3c_forced_release_keys[] = {
 };
 
 /*
+ * Samsung 550P5C/550P7C Fn release keys not working
+ */
+static unsigned int atkbd_samsung_550p5c_forced_release_keys[] = {
+	0xa8, 0xa9, 0xc5, 0xce, 0xd5, -1U
+};
+
+/*
  * Amilo Pi 3525 key release for Fn+Volume keys not working
  */
 static unsigned int atkbd_amilo_pi3525_forced_release_keys[] = {
@@ -1748,6 +1755,15 @@ static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
 		.driver_data = atkbd_samsung_530u3c_forced_release_keys,
 	},
 	{
+		/* Samsung series 5 (550P5C/550P7C) */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "550P5C"),
+		},
+		.callback = atkbd_setup_forced_release,
+		.driver_data = atkbd_samsung_550p5c_forced_release_keys,
+	},
+	{
 		/* Fujitsu Amilo PA 1510 */
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU SIEMENS"),
-- 
1.8.3.1

Re: [PATCH 2/5] atkbd: Fix key release for Fn keys on Samsung series 5 550P5C

[Dmitry Torokhov]

Hi Mauro,

On Mon, Jul 29, 2013 at 12:59:36AM -0300, Mauro Carvalho Chehab wrote:
> Fix key release for Fn keys with Samsung series 5
> 
> Samsung series 5 notebooks don't produce release events for certain
> keys (Fn+F1, Fn+F11, Fn+F12 and Fn Lock). Add those keys at the release
> fixup table.

As I mentioned, the force release should be handled by udev instead of
adding kernel driver quirk.

Thanks.

-- 
Dmitry

[PATCH 3/5] input.h: add keycodes for Fn Lock

[Mauro Carvalho Chehab]

Samsung notebooks have a FN LOCK key. It works like CAPS LOCK or NUM
LOCK keys.

When FN LOCK key is pressed, any further press to a key with a blue label
on it (Fn keys) will produce the alternate code.

Another press makes the keyboard to return to its normal state.

On the notebooks where such feature were found, a FN LOCK on event
produces scancode 0xa8, and a FN LOCK off event produces scancode 0xa9.

Yet, it is better to reserve some space at the keymap to allow some
different implementation of this feature where the same keycode might
be used.

Also, as this is actually a switch, add a switch indicator to report
when this switch is set/reset.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 include/uapi/linux/input.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index d584047..4622c34 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -716,6 +716,10 @@ struct input_keymap_entry {
 #define BTN_DPAD_LEFT		0x222
 #define BTN_DPAD_RIGHT		0x223
 
+#define KEY_FNLOCK_TOGGLE	0x224	/* Request switch Fn on or off */
+#define KEY_FNLOCK_ON		0x225
+#define KEY_FNLOCK_OFF		0x226
+
 #define BTN_TRIGGER_HAPPY		0x2c0
 #define BTN_TRIGGER_HAPPY1		0x2c0
 #define BTN_TRIGGER_HAPPY2		0x2c1
@@ -853,6 +857,7 @@ struct input_keymap_entry {
 #define SW_FRONT_PROXIMITY	0x0b  /* set = front proximity sensor active */
 #define SW_ROTATE_LOCK		0x0c  /* set = rotate locked/disabled */
 #define SW_LINEIN_INSERT	0x0d  /* set = inserted */
+#define SW_FNLOCK		0x0e  /* set = Fn locked */
 #define SW_MAX			0x0f
 #define SW_CNT			(SW_MAX+1)
 
-- 
1.8.3.1

Re: [PATCH 3/5] input.h: add keycodes for Fn Lock

[Dmitry Torokhov]

Hi Mauro,

On Mon, Jul 29, 2013 at 12:59:37AM -0300, Mauro Carvalho Chehab wrote:
> Samsung notebooks have a FN LOCK key. It works like CAPS LOCK or NUM
> LOCK keys.
> 
> When FN LOCK key is pressed, any further press to a key with a blue label
> on it (Fn keys) will produce the alternate code.
> 
> Another press makes the keyboard to return to its normal state.
> 
> On the notebooks where such feature were found, a FN LOCK on event
> produces scancode 0xa8, and a FN LOCK off event produces scancode 0xa9.
> 
> Yet, it is better to reserve some space at the keymap to allow some
> different implementation of this feature where the same keycode might
> be used.
> 
> Also, as this is actually a switch, add a switch indicator to report
> when this switch is set/reset.
> 
> Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> ---
>  include/uapi/linux/input.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index d584047..4622c34 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -716,6 +716,10 @@ struct input_keymap_entry {
>  #define BTN_DPAD_LEFT		0x222
>  #define BTN_DPAD_RIGHT		0x223
>  
> +#define KEY_FNLOCK_TOGGLE	0x224	/* Request switch Fn on or off */
> +#define KEY_FNLOCK_ON		0x225
> +#define KEY_FNLOCK_OFF		0x226
> +
>  #define BTN_TRIGGER_HAPPY		0x2c0
>  #define BTN_TRIGGER_HAPPY1		0x2c0
>  #define BTN_TRIGGER_HAPPY2		0x2c1
> @@ -853,6 +857,7 @@ struct input_keymap_entry {
>  #define SW_FRONT_PROXIMITY	0x0b  /* set = front proximity sensor active */
>  #define SW_ROTATE_LOCK		0x0c  /* set = rotate locked/disabled */
>  #define SW_LINEIN_INSERT	0x0d  /* set = inserted */
> +#define SW_FNLOCK		0x0e  /* set = Fn locked */

I am not sure if we need both the keys and the switch, so I would
probably simply go with the keys, and not bother with switch. Then we do
not need to touch the atkbd driver at all and rely on udev to set up the
keymap and force release keys. 

-- 
Dmitry

Re: [PATCH 3/5] input.h: add keycodes for Fn Lock

[Mauro Carvalho Chehab]

Hi Dmitry,

Em Sun, 28 Jul 2013 21:53:58 -0700
Dmitry Torokhov <dmitry.torokhov@gmail.com> escreveu:

> Hi Mauro,
> 
> On Mon, Jul 29, 2013 at 12:59:37AM -0300, Mauro Carvalho Chehab wrote:
> > Samsung notebooks have a FN LOCK key. It works like CAPS LOCK or NUM
> > LOCK keys.
> > 
> > When FN LOCK key is pressed, any further press to a key with a blue label
> > on it (Fn keys) will produce the alternate code.
> > 
> > Another press makes the keyboard to return to its normal state.
> > 
> > On the notebooks where such feature were found, a FN LOCK on event
> > produces scancode 0xa8, and a FN LOCK off event produces scancode 0xa9.
> > 
> > Yet, it is better to reserve some space at the keymap to allow some
> > different implementation of this feature where the same keycode might
> > be used.
> > 
> > Also, as this is actually a switch, add a switch indicator to report
> > when this switch is set/reset.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> > ---
> >  include/uapi/linux/input.h | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> > index d584047..4622c34 100644
> > --- a/include/uapi/linux/input.h
> > +++ b/include/uapi/linux/input.h
> > @@ -716,6 +716,10 @@ struct input_keymap_entry {
> >  #define BTN_DPAD_LEFT		0x222
> >  #define BTN_DPAD_RIGHT		0x223
> >  
> > +#define KEY_FNLOCK_TOGGLE	0x224	/* Request switch Fn on or off */
> > +#define KEY_FNLOCK_ON		0x225
> > +#define KEY_FNLOCK_OFF		0x226
> > +
> >  #define BTN_TRIGGER_HAPPY		0x2c0
> >  #define BTN_TRIGGER_HAPPY1		0x2c0
> >  #define BTN_TRIGGER_HAPPY2		0x2c1
> > @@ -853,6 +857,7 @@ struct input_keymap_entry {
> >  #define SW_FRONT_PROXIMITY	0x0b  /* set = front proximity sensor active */
> >  #define SW_ROTATE_LOCK		0x0c  /* set = rotate locked/disabled */
> >  #define SW_LINEIN_INSERT	0x0d  /* set = inserted */
> > +#define SW_FNLOCK		0x0e  /* set = Fn locked */
> 
> I am not sure if we need both the keys and the switch, so I would
> probably simply go with the keys, and not bother with switch. Then we do
> not need to touch the atkbd driver at all and rely on udev to set up the
> keymap and force release keys. 

The hole idea of doing those patches is to have an userspace tool that will
be able to show software LEDs, like mate-applet-lockeys, that will query
the input driver to know the current status. So, the better is to keep 
control of it as soon as kernel starts controlling the Keyboard, as, 
otherwise, the software LED indicators may be wrong.

If you think that having both keycodes and a switch is an overkill, then
the better is to just keep the switch.

-- 

Cheers,
Mauro

Re: [PATCH 3/5] input.h: add keycodes for Fn Lock

[Mauro Carvalho Chehab]

Em Mon, 29 Jul 2013 07:03:46 -0300
Mauro Carvalho Chehab <m.chehab@samsung.com> escreveu:

> Hi Dmitry,
> 
> Em Sun, 28 Jul 2013 21:53:58 -0700
> Dmitry Torokhov <dmitry.torokhov@gmail.com> escreveu:
> 
> > Hi Mauro,
> > 
> > On Mon, Jul 29, 2013 at 12:59:37AM -0300, Mauro Carvalho Chehab wrote:
> > > Samsung notebooks have a FN LOCK key. It works like CAPS LOCK or NUM
> > > LOCK keys.
> > > 
> > > When FN LOCK key is pressed, any further press to a key with a blue label
> > > on it (Fn keys) will produce the alternate code.
> > > 
> > > Another press makes the keyboard to return to its normal state.
> > > 
> > > On the notebooks where such feature were found, a FN LOCK on event
> > > produces scancode 0xa8, and a FN LOCK off event produces scancode 0xa9.
> > > 
> > > Yet, it is better to reserve some space at the keymap to allow some
> > > different implementation of this feature where the same keycode might
> > > be used.
> > > 
> > > Also, as this is actually a switch, add a switch indicator to report
> > > when this switch is set/reset.
> > > 
> > > Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> > > ---
> > >  include/uapi/linux/input.h | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> > > index d584047..4622c34 100644
> > > --- a/include/uapi/linux/input.h
> > > +++ b/include/uapi/linux/input.h
> > > @@ -716,6 +716,10 @@ struct input_keymap_entry {
> > >  #define BTN_DPAD_LEFT		0x222
> > >  #define BTN_DPAD_RIGHT		0x223
> > >  
> > > +#define KEY_FNLOCK_TOGGLE	0x224	/* Request switch Fn on or off */
> > > +#define KEY_FNLOCK_ON		0x225
> > > +#define KEY_FNLOCK_OFF		0x226
> > > +
> > >  #define BTN_TRIGGER_HAPPY		0x2c0
> > >  #define BTN_TRIGGER_HAPPY1		0x2c0
> > >  #define BTN_TRIGGER_HAPPY2		0x2c1
> > > @@ -853,6 +857,7 @@ struct input_keymap_entry {
> > >  #define SW_FRONT_PROXIMITY	0x0b  /* set = front proximity sensor active */
> > >  #define SW_ROTATE_LOCK		0x0c  /* set = rotate locked/disabled */
> > >  #define SW_LINEIN_INSERT	0x0d  /* set = inserted */
> > > +#define SW_FNLOCK		0x0e  /* set = Fn locked */
> > 
> > I am not sure if we need both the keys and the switch, so I would
> > probably simply go with the keys, and not bother with switch. Then we do
> > not need to touch the atkbd driver at all and rely on udev to set up the
> > keymap and force release keys. 
> 
> The hole idea of doing those patches is to have an userspace tool that will
> be able to show software LEDs, like mate-applet-lockeys, that will query
> the input driver to know the current status. So, the better is to keep 
> control of it as soon as kernel starts controlling the Keyboard, as, 
> otherwise, the software LED indicators may be wrong.
> 
> If you think that having both keycodes and a switch is an overkill, then
> the better is to just keep the switch.

After thinking for a little bit, I can see a few alternatives:

1) add patch 3/5 and patch 4/5 as is. No need to apply patch 5/5. The
Fn Lock switch support will start work as soon as udev initializes the
keytables.

2) Add patches 3/5 and patch 4/5 as is, and patch 5/5 rebased, as:
	http://git.infradead.org/users/mchehab/samsung.git/commitdiff/96171bbd42d8ab9dfc3f36e0d4796080a8989344

This way, all keyboards with FN LOCK will require a quirk at atkbd.
The key handling will only start after udev starts.

3) Modify patch 3/5 to only have there a switch (or a LED). That will
require a change at patch 5/5 to use the scancodes to handle the
switch directly.

IMHO, (3) is better, as it will start handling the FN LOCK earlier,
reducing the risk of someone press the key after keyboard reset and
before udev to load the keytable, and providing a more reliable
software LED indication.

Please let me know what works best for you, and I'll redo the patches.

Thanks!
Mauro

Re: [PATCH 3/5] input.h: add keycodes for Fn Lock

[Dmitry Torokhov]

On Mon, Jul 29, 2013 at 07:03:46AM -0300, Mauro Carvalho Chehab wrote:
> Hi Dmitry,
> 
> Em Sun, 28 Jul 2013 21:53:58 -0700
> Dmitry Torokhov <dmitry.torokhov@gmail.com> escreveu:
> 
> > Hi Mauro,
> > 
> > On Mon, Jul 29, 2013 at 12:59:37AM -0300, Mauro Carvalho Chehab wrote:
> > > Samsung notebooks have a FN LOCK key. It works like CAPS LOCK or NUM
> > > LOCK keys.
> > > 
> > > When FN LOCK key is pressed, any further press to a key with a blue label
> > > on it (Fn keys) will produce the alternate code.
> > > 
> > > Another press makes the keyboard to return to its normal state.
> > > 
> > > On the notebooks where such feature were found, a FN LOCK on event
> > > produces scancode 0xa8, and a FN LOCK off event produces scancode 0xa9.
> > > 
> > > Yet, it is better to reserve some space at the keymap to allow some
> > > different implementation of this feature where the same keycode might
> > > be used.
> > > 
> > > Also, as this is actually a switch, add a switch indicator to report
> > > when this switch is set/reset.
> > > 
> > > Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> > > ---
> > >  include/uapi/linux/input.h | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> > > index d584047..4622c34 100644
> > > --- a/include/uapi/linux/input.h
> > > +++ b/include/uapi/linux/input.h
> > > @@ -716,6 +716,10 @@ struct input_keymap_entry {
> > >  #define BTN_DPAD_LEFT		0x222
> > >  #define BTN_DPAD_RIGHT		0x223
> > >  
> > > +#define KEY_FNLOCK_TOGGLE	0x224	/* Request switch Fn on or off */
> > > +#define KEY_FNLOCK_ON		0x225
> > > +#define KEY_FNLOCK_OFF		0x226
> > > +
> > >  #define BTN_TRIGGER_HAPPY		0x2c0
> > >  #define BTN_TRIGGER_HAPPY1		0x2c0
> > >  #define BTN_TRIGGER_HAPPY2		0x2c1
> > > @@ -853,6 +857,7 @@ struct input_keymap_entry {
> > >  #define SW_FRONT_PROXIMITY	0x0b  /* set = front proximity sensor active */
> > >  #define SW_ROTATE_LOCK		0x0c  /* set = rotate locked/disabled */
> > >  #define SW_LINEIN_INSERT	0x0d  /* set = inserted */
> > > +#define SW_FNLOCK		0x0e  /* set = Fn locked */
> > 
> > I am not sure if we need both the keys and the switch, so I would
> > probably simply go with the keys, and not bother with switch. Then we do
> > not need to touch the atkbd driver at all and rely on udev to set up the
> > keymap and force release keys. 
> 
> The hole idea of doing those patches is to have an userspace tool that will
> be able to show software LEDs, like mate-applet-lockeys, that will query
> the input driver to know the current status. So, the better is to keep 
> control of it as soon as kernel starts controlling the Keyboard, as, 
> otherwise, the software LED indicators may be wrong.
> 
> If you think that having both keycodes and a switch is an overkill, then
> the better is to just keep the switch.

Yes, we should have either the keys or the switch.

I am a bit concerned with the behavior of this FN key and whether the state
can be reported reliably. Have you tested the behavior of keyboard when you
press the FN key before atkbd driver had a chance to bind to it? What
about suspending with FN engaged and then resuming? Suspend-to-disk
behavior?

Thanks.

-- 
Dmitry

Re: [PATCH 3/5] input.h: add keycodes for Fn Lock

[Mauro Carvalho Chehab]

Em Tue, 30 Jul 2013 00:14:13 -0700
Dmitry Torokhov <dmitry.torokhov@gmail.com> escreveu:

> On Mon, Jul 29, 2013 at 07:03:46AM -0300, Mauro Carvalho Chehab wrote:
> > Hi Dmitry,
> > 
> > Em Sun, 28 Jul 2013 21:53:58 -0700
> > Dmitry Torokhov <dmitry.torokhov@gmail.com> escreveu:
> > 
> > > Hi Mauro,
> > > 
> > > On Mon, Jul 29, 2013 at 12:59:37AM -0300, Mauro Carvalho Chehab wrote:
> > > > Samsung notebooks have a FN LOCK key. It works like CAPS LOCK or NUM
> > > > LOCK keys.
> > > > 
> > > > When FN LOCK key is pressed, any further press to a key with a blue label
> > > > on it (Fn keys) will produce the alternate code.
> > > > 
> > > > Another press makes the keyboard to return to its normal state.
> > > > 
> > > > On the notebooks where such feature were found, a FN LOCK on event
> > > > produces scancode 0xa8, and a FN LOCK off event produces scancode 0xa9.
> > > > 
> > > > Yet, it is better to reserve some space at the keymap to allow some
> > > > different implementation of this feature where the same keycode might
> > > > be used.
> > > > 
> > > > Also, as this is actually a switch, add a switch indicator to report
> > > > when this switch is set/reset.
> > > > 
> > > > Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> > > > ---
> > > >  include/uapi/linux/input.h | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > > 
> > > > diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> > > > index d584047..4622c34 100644
> > > > --- a/include/uapi/linux/input.h
> > > > +++ b/include/uapi/linux/input.h
> > > > @@ -716,6 +716,10 @@ struct input_keymap_entry {
> > > >  #define BTN_DPAD_LEFT		0x222
> > > >  #define BTN_DPAD_RIGHT		0x223
> > > >  
> > > > +#define KEY_FNLOCK_TOGGLE	0x224	/* Request switch Fn on or off */
> > > > +#define KEY_FNLOCK_ON		0x225
> > > > +#define KEY_FNLOCK_OFF		0x226
> > > > +
> > > >  #define BTN_TRIGGER_HAPPY		0x2c0
> > > >  #define BTN_TRIGGER_HAPPY1		0x2c0
> > > >  #define BTN_TRIGGER_HAPPY2		0x2c1
> > > > @@ -853,6 +857,7 @@ struct input_keymap_entry {
> > > >  #define SW_FRONT_PROXIMITY	0x0b  /* set = front proximity sensor active */
> > > >  #define SW_ROTATE_LOCK		0x0c  /* set = rotate locked/disabled */
> > > >  #define SW_LINEIN_INSERT	0x0d  /* set = inserted */
> > > > +#define SW_FNLOCK		0x0e  /* set = Fn locked */
> > > 
> > > I am not sure if we need both the keys and the switch, so I would
> > > probably simply go with the keys, and not bother with switch. Then we do
> > > not need to touch the atkbd driver at all and rely on udev to set up the
> > > keymap and force release keys. 
> > 
> > The hole idea of doing those patches is to have an userspace tool that will
> > be able to show software LEDs, like mate-applet-lockeys, that will query
> > the input driver to know the current status. So, the better is to keep 
> > control of it as soon as kernel starts controlling the Keyboard, as, 
> > otherwise, the software LED indicators may be wrong.
> > 
> > If you think that having both keycodes and a switch is an overkill, then
> > the better is to just keep the switch.
> 
> Yes, we should have either the keys or the switch.

OK.

> I am a bit concerned with the behavior of this FN key and whether the state
> can be reported reliably. Have you tested the behavior of keyboard when you
> press the FN key before atkbd driver had a chance to bind to it?

If I press FN LOCK at grub2, or before that, it simply doesn't work.

I suspect that something at the atkbd initialization (or, more likely, at ACPI
initialization) makes the BIOS to enable it.

I did a quick inspection at ACPI DSDT table, but I wasn't able to discover
anything there. The BIOS don't have explicit support for Linux.

> What
> about suspending with FN engaged and then resuming? Suspend-to-disk
> behavior?

I tested calling both pm-suspend and pm-hibernate here: Fn Lock state was
recovered properly. On a normal reboot, Fn Lock behavior resets.

So, it seems that there's something already in resume code that restores
Fn Lock to the state before suspend.

Regards,
Mauro

[PATCH 4/5] atkbd: add support for handling KEY_FNLOCK

[Mauro Carvalho Chehab]

Samsung keyboards have a key called FN LOCK. This key acts like
CAPS LOCK and NUM LOCK. When pressed, the keyboard switches to a state
where the produced scancode are the blue ones (the Fn keycodes).

Add support for it at atkbd.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/input/keyboard/atkbd.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 298ac1d..7f88132 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -218,6 +218,7 @@ struct atkbd {
 	bool softraw;
 	bool scroll;
 	bool enabled;
+	bool fnlock_set;
 
 	/* Accessed only from interrupt */
 	unsigned char emul;
@@ -453,6 +454,28 @@ static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
 	if (keycode != ATKBD_KEY_NULL)
 		input_event(dev, EV_MSC, MSC_SCAN, code);
 
+	/*
+	 * Handles special Fn Lock switch.
+	 *
+	 * This switch can't be merged with the next one, as we also want
+	 * to report those keycodes to userspace, via input_event, and
+	 * call input_sync().
+	 */
+	switch (keycode) {
+	case KEY_FNLOCK_OFF:
+		atkbd->fnlock_set = false;
+		input_report_switch(dev, SW_FNLOCK, atkbd->fnlock_set);
+		break;
+	case KEY_FNLOCK_ON:
+		atkbd->fnlock_set = true;
+		input_report_switch(dev, SW_FNLOCK, atkbd->fnlock_set);
+		break;
+	case KEY_FNLOCK_TOGGLE:
+		atkbd->fnlock_set = !atkbd->fnlock_set;
+		input_report_switch(dev, SW_FNLOCK, atkbd->fnlock_set);
+		break;
+	}
+
 	switch (keycode) {
 	case ATKBD_KEY_NULL:
 		break;
@@ -1126,6 +1149,10 @@ static void atkbd_set_device_attrs(struct atkbd *atkbd)
 			__set_bit(atkbd->keycode[i], input_dev->keybit);
 		}
 	}
+
+	/* FIXME: Unconditionqally sets FN LOCK switch */
+	__set_bit(SW_FNLOCK, atkbd->dev->swbit);
+	input_dev->evbit[0] |= BIT_MASK(EV_SW);
 }
 
 /*
-- 
1.8.3.1

[PATCH 5/5] atkbd: only enable SW_FNLOCK on keyboards that have FN LOCK key

[Mauro Carvalho Chehab]

Instead of unconditionally enabling the switch, only make it available
where the key actually exists.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/input/keyboard/atkbd.c | 58 ++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 25 deletions(-)

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 7f88132..3ec7d67 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -240,6 +240,7 @@ struct atkbd {
 /*
  * System-specific keymap fixup routine
  */
+static bool atkbd_platform_has_fnlock;
 static void (*atkbd_platform_fixup)(struct atkbd *, const void *data);
 static void *atkbd_platform_fixup_data;
 static unsigned int (*atkbd_platform_scancode_fixup)(struct atkbd *, unsigned int);
@@ -454,26 +455,22 @@ static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
 	if (keycode != ATKBD_KEY_NULL)
 		input_event(dev, EV_MSC, MSC_SCAN, code);
 
-	/*
-	 * Handles special Fn Lock switch.
-	 *
-	 * This switch can't be merged with the next one, as we also want
-	 * to report those keycodes to userspace, via input_event, and
-	 * call input_sync().
-	 */
-	switch (keycode) {
-	case KEY_FNLOCK_OFF:
-		atkbd->fnlock_set = false;
-		input_report_switch(dev, SW_FNLOCK, atkbd->fnlock_set);
-		break;
-	case KEY_FNLOCK_ON:
-		atkbd->fnlock_set = true;
-		input_report_switch(dev, SW_FNLOCK, atkbd->fnlock_set);
-		break;
-	case KEY_FNLOCK_TOGGLE:
-		atkbd->fnlock_set = !atkbd->fnlock_set;
-		input_report_switch(dev, SW_FNLOCK, atkbd->fnlock_set);
-		break;
+	/* Handles special Fn Lock switch found on some notebooks */
+	if (test_bit(SW_FNLOCK, dev->swbit)) {
+		switch (keycode) {
+		case KEY_FNLOCK_OFF:
+			atkbd->fnlock_set = false;
+			input_report_switch(dev, SW_FNLOCK, atkbd->fnlock_set);
+			break;
+		case KEY_FNLOCK_ON:
+			atkbd->fnlock_set = true;
+			input_report_switch(dev, SW_FNLOCK, atkbd->fnlock_set);
+			break;
+		case KEY_FNLOCK_TOGGLE:
+			atkbd->fnlock_set = !atkbd->fnlock_set;
+			input_report_switch(dev, SW_FNLOCK, atkbd->fnlock_set);
+			break;
+		}
 	}
 
 	switch (keycode) {
@@ -1150,9 +1147,10 @@ static void atkbd_set_device_attrs(struct atkbd *atkbd)
 		}
 	}
 
-	/* FIXME: Unconditionqally sets FN LOCK switch */
-	__set_bit(SW_FNLOCK, atkbd->dev->swbit);
-	input_dev->evbit[0] |= BIT_MASK(EV_SW);
+	if (atkbd_platform_has_fnlock) {
+		__set_bit(SW_FNLOCK, atkbd->dev->swbit);
+		input_dev->evbit[0] |= BIT_MASK(EV_SW);
+	}
 }
 
 /*
@@ -1672,6 +1670,16 @@ static int __init atkbd_setup_forced_release(const struct dmi_system_id *id)
 	return 1;
 }
 
+static int __init
+atkbd_setup_forced_release_and_fnlock(const struct dmi_system_id *id)
+{
+	atkbd_platform_fixup = atkbd_apply_forced_release_keylist;
+	atkbd_platform_fixup_data = id->driver_data;
+	atkbd_platform_has_fnlock = true;
+
+	return 1;
+}
+
 static int __init atkbd_setup_scancode_fixup(const struct dmi_system_id *id)
 {
 	atkbd_platform_scancode_fixup = id->driver_data;
@@ -1778,7 +1786,7 @@ static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
 			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
 			DMI_MATCH(DMI_PRODUCT_NAME, "530U3C"),
 		},
-		.callback = atkbd_setup_forced_release,
+		.callback = atkbd_setup_forced_release_and_fnlock,
 		.driver_data = atkbd_samsung_530u3c_forced_release_keys,
 	},
 	{
@@ -1787,7 +1795,7 @@ static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
 			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
 			DMI_MATCH(DMI_PRODUCT_NAME, "550P5C"),
 		},
-		.callback = atkbd_setup_forced_release,
+		.callback = atkbd_setup_forced_release_and_fnlock,
 		.driver_data = atkbd_samsung_550p5c_forced_release_keys,
 	},
 	{
-- 
1.8.3.1

Re: [PATCH 1/5] atkbd: Fix key release for Fn keys on Samsung series 5 ultra 540UC

[Dmitry Torokhov]

Hi Mauro,


On Mon, Jul 29, 2013 at 12:59:35AM -0300, Mauro Carvalho Chehab wrote:
> Samsung series 5 ultra notebooks don't produce release events for certain
> keys (Fn+F1, Fn+F11, Fn+F12 and Fn Lock). Add those keys at the release
> fixup table.
> 

As I mentioned, the force release should be handled by udev instead of
adding kernel driver quirk.

Thanks.

-- 
Dmitry

Re: [PATCH 1/5] atkbd: Fix key release for Fn keys on Samsung series 5 ultra 540UC

[Mauro Carvalho Chehab]

Em Sun, 28 Jul 2013 21:49:09 -0700
Dmitry Torokhov <dmitry.torokhov@gmail.com> escreveu:

> Hi Mauro,
> 
> 
> On Mon, Jul 29, 2013 at 12:59:35AM -0300, Mauro Carvalho Chehab wrote:
> > Samsung series 5 ultra notebooks don't produce release events for certain
> > keys (Fn+F1, Fn+F11, Fn+F12 and Fn Lock). Add those keys at the release
> > fixup table.
> > 
> 
> As I mentioned, the force release should be handled by udev instead of
> adding kernel driver quirk.

Ah, OK. I'll drop both patches 1 and 2 on a next patchset.

-- 

Cheers,
Mauro