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

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

[Mattia Dongili]

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 <marco@absence.it>
> ---
> 
> --- 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!

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

[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 
platform driver (thinkpad_acpi) is using it.
What is right and what is wrong? Should it be removed from thinkpad_acpi 
too?

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

[Matthew Garrett]

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 | mjg59@srcf.ucam.org

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

[Marco Chiappero]

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.

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

[Joey Lee]

於 六，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

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

[Marco Chiappero]

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?

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

[Mattia Dongili]

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!

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

[Marco Chiappero]

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.

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

[Mattia Dongili]

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!