2011-06-04 08:07:14

by Mattia Dongili

[permalink] [raw]
Subject: Re: [PATCH 12/25] sony-laptop: input core improvements improvements

On Fri, Jun 03, 2011 at 05:43:42PM +0200, Marco Chiappero wrote:
> The code is now forwarding the SW_RFKILL_ALL event; added a couple

I don't think we should do this.
4eeb50220a4efd8c33598a228d03aff203a7ad07 removed the notification for
the SPIC part of the module and I'd rather not add it now for SNC. The
notification has proven to cause race conditions (see the bug referenced
in the commit message).
Moreover it should be left to the rfkill subsystem to notify userspace
for consistency with all the other drivers implementing rfkill switches.

> of Fn combos too.

the additional key codes are fine though.

> Signed-off-by: Marco Chiappero <[email protected]>
> ---
>
> --- a/include/linux/sonypi.h
> +++ b/include/linux/sonypi.h
> @@ -114,6 +114,7 @@
> #define SONYPI_EVENT_BRIGHTNESS_PRESSED 71
> #define SONYPI_EVENT_MEDIA_PRESSED 72
> #define SONYPI_EVENT_VENDOR_PRESSED 73
> +#define SONYPI_EVENT_RFKILL_ALL 74
>
> /* get/set brightness */
> #define SONYPI_IOCGBRT _IOR('v', 0, __u8)
> --- a/drivers/platform/x86/sony-laptop.c
> +++ b/drivers/platform/x86/sony-laptop.c
> @@ -142,6 +142,7 @@ MODULE_PARM_DESC(kbd_backlight_timeout,
>
>
> static int sony_rfkill_handle = -1;
> +static int sony_nc_get_rfkill_hwblock(void);
>
> /*********** Input Devices ***********/
>
> @@ -241,7 +242,8 @@ static int sony_laptop_input_index[] = {
> 57, /* 70 SONYPI_EVENT_VOLUME_DEC_PRESSED */
> -1, /* 71 SONYPI_EVENT_BRIGHTNESS_PRESSED */
> 58, /* 72 SONYPI_EVENT_MEDIA_PRESSED */
> - 59, /* 72 SONYPI_EVENT_VENDOR_PRESSED */
> + 59, /* 73 SONYPI_EVENT_VENDOR_PRESSED */
> + -1, /* 74 SONYPI_EVENT_RFKILL_ALL */
> };
>
> static int sony_laptop_input_keycode_map[] = {
> @@ -335,6 +337,7 @@ static void sony_laptop_report_input_eve
> struct input_dev *jog_dev = sony_laptop_input.jog_dev;
> struct input_dev *key_dev = sony_laptop_input.key_dev;
> struct sony_laptop_keypress kp = { NULL };
> + int rfk_switch;
>
> if (event == SONYPI_EVENT_FNKEY_RELEASED ||
> event == SONYPI_EVENT_ANYBUTTON_RELEASED) {
> @@ -363,6 +366,14 @@ static void sony_laptop_report_input_eve
> kp.dev = jog_dev;
> break;
>
> + case SONYPI_EVENT_RFKILL_ALL:
> + rfk_switch = sony_nc_get_rfkill_hwblock();
> + if (!(rfk_switch < 0)) {
> + input_report_switch(key_dev, SW_RFKILL_ALL, rfk_switch);
> + input_sync(key_dev);
> + }
> + return;
> +
> default:
> if (event >= ARRAY_SIZE(sony_laptop_input_index)) {
> dprintk("sony_laptop_report_input_event, event not known: %d\n",
> event);
> @@ -438,6 +449,14 @@ static int sony_laptop_setup_input(struc
> __set_bit(sony_laptop_input_keycode_map[i], key_dev->keybit);
> __clear_bit(KEY_RESERVED, key_dev->keybit);
>
> + if (sony_rfkill_handle != -1) {
> + int rfk_switch;
> +
> + rfk_switch = sony_nc_get_rfkill_hwblock();
> + input_set_capability(key_dev, EV_SW, SW_RFKILL_ALL);
> + input_report_switch(key_dev, SW_RFKILL_ALL, rfk_switch);
> + }
> +
> error = input_register_device(key_dev);
> if (error)
> goto err_free_keydev;
> @@ -1079,10 +1098,6 @@ struct sony_nc_event {
> };
>
> static struct sony_nc_event sony_100_events[] = {
> - { 0x90, SONYPI_EVENT_PKEY_P1 },
> - { 0x10, SONYPI_EVENT_ANYBUTTON_RELEASED },
> - { 0x91, SONYPI_EVENT_PKEY_P2 },
> - { 0x11, SONYPI_EVENT_ANYBUTTON_RELEASED },
> { 0x81, SONYPI_EVENT_FNKEY_F1 },
> { 0x01, SONYPI_EVENT_FNKEY_RELEASED },
> { 0x82, SONYPI_EVENT_FNKEY_F2 },
> @@ -1097,12 +1112,20 @@ static struct sony_nc_event sony_100_eve
> { 0x06, SONYPI_EVENT_FNKEY_RELEASED },
> { 0x87, SONYPI_EVENT_FNKEY_F7 },
> { 0x07, SONYPI_EVENT_FNKEY_RELEASED },
> + { 0x88, SONYPI_EVENT_FNKEY_F8 },
> + { 0x08, SONYPI_EVENT_FNKEY_RELEASED },
> { 0x89, SONYPI_EVENT_FNKEY_F9 },
> { 0x09, SONYPI_EVENT_FNKEY_RELEASED },
> { 0x8A, SONYPI_EVENT_FNKEY_F10 },
> { 0x0A, SONYPI_EVENT_FNKEY_RELEASED },
> + { 0x8B, SONYPI_EVENT_FNKEY_F11 },
> + { 0x0B, SONYPI_EVENT_FNKEY_RELEASED },
> { 0x8C, SONYPI_EVENT_FNKEY_F12 },
> { 0x0C, SONYPI_EVENT_FNKEY_RELEASED },
> + { 0x90, SONYPI_EVENT_PKEY_P1 },
> + { 0x10, SONYPI_EVENT_ANYBUTTON_RELEASED },
> + { 0x91, SONYPI_EVENT_PKEY_P2 },
> + { 0x11, SONYPI_EVENT_ANYBUTTON_RELEASED },
> { 0x9d, SONYPI_EVENT_ZOOM_PRESSED },
> { 0x1d, SONYPI_EVENT_ANYBUTTON_RELEASED },
> { 0x9f, SONYPI_EVENT_CD_EJECT_PRESSED },
--
mattia
:wq!


2011-06-04 16:40:21

by Mattia Dongili

[permalink] [raw]
Subject: Re: [PATCH 12/25] sony-laptop: input core improvements improvements

On Sat, Jun 04, 2011 at 05:21:11PM +0200, Marco Chiappero wrote:
> Il 04/06/2011 10:07, Mattia Dongili ha scritto:
> >>of Fn combos too.
> >
> >the additional key codes are fine though.
>
> There is one thing (IIRC you already heard of) I'd like to discuss here.
> It seems that on recent models the hotkeys layout hasn't changed for
> some time now, and it's likely to stay the same for a long time.
> However it is different from the one mapped in the driver, and some
> changes are required. For example:
>
> - Fn-F1 (Touchpad togle enable) to KEY_TOUCHPAD_TOGGLE instead of KEY_FN_F1
> - Fn-F5 (bright. down) to KEY_BRIGHTNESSDOWN instead of KEY_FN_F5
> - Fn-F9 (zoom out) to KEY_ZOOMOUT instead of KEY_FN_F9
> and so on.
>
> To preserve support for older models we can use udev keymaps or a
> fallback map for any notebook not starting with the "VPC" model
> prefix (since a couple of years this is the starting model string).
> Is this solution acceptable? Having a correct mapping at the source
> instead of remapping for every model is better, is it possible?

sure it's possible but it's going to be a source of headache once sony
changes their keyboard layout again (and it will happen).
Using generic key codes and remapping them in userspace where necessary
worked for more than 10 years and is still working fine.

--
mattia
:wq!

2011-06-05 22:24:07

by Mattia Dongili

[permalink] [raw]
Subject: Re: [PATCH 12/25] sony-laptop: input core improvements improvements

On Sat, Jun 04, 2011 at 06:58:53PM +0200, Marco Chiappero wrote:
> Il 04/06/2011 18:40, Mattia Dongili ha scritto:
...
> >Using generic key codes and remapping them in userspace where necessary
> >worked for more than 10 years and is still working fine.
>
> Sure and I consider to be a good alternative, but it looks to me
> like being another workaround, it is fine for particular exceptions
> that need a few remappings, but it should not be the rule, the
> driver extension. That's why I was asking about this change.

what is a better solution? having keymaps for every different layout
that Sony implemented throught the years and adding more when things
change? This already exists in userspace, it makes not much sense to
have it in the kernel as well.

--
mattia
:wq!

2011-06-04 12:44:55

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 12/25] sony-laptop: input core improvements improvements

On Sat, Jun 04, 2011 at 12:42:47PM +0200, Marco Chiappero wrote:

> As far as I know this event should be forwarded and the rfkill-input
> code removed as soon as there is enough userspace support. Another
> platform driver (thinkpad_acpi) is using it.
> What is right and what is wrong? Should it be removed from
> thinkpad_acpi too?

This event should be sent if hitting the switch is a request to
userspace. If pressing it causes the hardware to perform the change and
this is merely a notification that the change has been made, it
shouldn't be sent. Just update the rfkill state and let userspace notice
the uevents from rfkill.

--
Matthew Garrett | [email protected]

2011-06-04 13:11:30

by Marco Chiappero

[permalink] [raw]
Subject: Re: [PATCH 12/25] sony-laptop: input core improvements improvements

Il 04/06/2011 14:44, Matthew Garrett ha scritto:
> This event should be sent if hitting the switch is a request to
> userspace. If pressing it causes the hardware to perform the change and
> this is merely a notification that the change has been made, it
> shouldn't be sent.

Ok, I didn't know about this difference, I'll remove it and update the
patch.


2011-06-04 15:21:15

by Marco Chiappero

[permalink] [raw]
Subject: Re: [PATCH 12/25] sony-laptop: input core improvements improvements

Il 04/06/2011 10:07, Mattia Dongili ha scritto:
>> of Fn combos too.
>
> the additional key codes are fine though.

There is one thing (IIRC you already heard of) I'd like to discuss here.
It seems that on recent models the hotkeys layout hasn't changed for
some time now, and it's likely to stay the same for a long time. However
it is different from the one mapped in the driver, and some changes are
required. For example:

- Fn-F1 (Touchpad togle enable) to KEY_TOUCHPAD_TOGGLE instead of KEY_FN_F1
- Fn-F5 (bright. down) to KEY_BRIGHTNESSDOWN instead of KEY_FN_F5
- Fn-F9 (zoom out) to KEY_ZOOMOUT instead of KEY_FN_F9
and so on.

To preserve support for older models we can use udev keymaps or a
fallback map for any notebook not starting with the "VPC" model prefix
(since a couple of years this is the starting model string). Is this
solution acceptable? Having a correct mapping at the source instead of
remapping for every model is better, is it possible?

2011-06-08 08:26:30

by Joey Lee

[permalink] [raw]
Subject: Re: [PATCH 12/25] sony-laptop: input core improvements improvements

於 六,2011-06-04 於 12:42 +0200,Marco Chiappero 提到:
> Il 04/06/2011 10:07, Mattia Dongili ha scritto:
> > On Fri, Jun 03, 2011 at 05:43:42PM +0200, Marco Chiappero wrote:
> >> The code is now forwarding the SW_RFKILL_ALL event; added a couple
> >
> > I don't think we should do this.
>
> As far as I know this event should be forwarded and the rfkill-input
> code removed as soon as there is enough userspace support. Another

There already have a urfkill daemon on userland has good support
killswitch:
http://www.freedesktop.org/wiki/Software/urfkill

But,
rfkill-input will not removed:
http://www.spinics.net/lists/linux-acpi/msg32465.html

The above information for you reference.


Thank's a lot!
Joey Lee



2011-06-04 10:50:38

by Marco Chiappero

[permalink] [raw]
Subject: Re: [PATCH 12/25] sony-laptop: input core improvements improvements

Il 04/06/2011 10:07, Mattia Dongili ha scritto:
> On Fri, Jun 03, 2011 at 05:43:42PM +0200, Marco Chiappero wrote:
>> The code is now forwarding the SW_RFKILL_ALL event; added a couple
>
> I don't think we should do this.

As far as I know this event should be forwarded and the rfkill-input
code removed as soon as there is enough userspace support. Another
platform driver (thinkpad_acpi) is using it.
What is right and what is wrong? Should it be removed from thinkpad_acpi
too?

2011-06-04 16:58:58

by Marco Chiappero

[permalink] [raw]
Subject: Re: [PATCH 12/25] sony-laptop: input core improvements improvements

Il 04/06/2011 18:40, Mattia Dongili ha scritto:
> sure it's possible but it's going to be a source of headache once sony
> changes their keyboard layout again (and it will happen).

Well, it depends on when, if it will happen in 2020 that's absolutely
fine. ;)

> Using generic key codes and remapping them in userspace where necessary
> worked for more than 10 years and is still working fine.

Sure and I consider to be a good alternative, but it looks to me like
being another workaround, it is fine for particular exceptions that need
a few remappings, but it should not be the rule, the driver extension.
That's why I was asking about this change.