[PATCH 0/3] toshiba_acpi: Hotkey handling and keymap updates

[PATCH 0/3] toshiba_acpi: Hotkey handling and keymap updates

[Azael Avalos]

These patches add support to a new function that queries the supported Hotkey
Event Type, making the use of the DMI matching unnecessary and also fixes
the "Special Functions" mode on some laptops.

Azael Avalos (3):
  toshiba_acpi: Add Hotkey Event Type function and definitions
  toshiba_acpi: Use the Hotkey Event Type function for keymap choosing
  toshiba_acpi: Fix the enabling of the Special Functions

 drivers/platform/x86/toshiba_acpi.c | 97 ++++++++++++++++++++++++-------------
 1 file changed, 64 insertions(+), 33 deletions(-)

-- 
2.2.2

[PATCH 1/3] toshiba_acpi: Add Hotkey Event Type function and definitions

[Azael Avalos]

This patch adds support to query the "Hotkey Event Type" the system
supports.

There are two main event types (so far), 0x10 and 0x11, with the
first beign all those laptops that have the old keyboard layout, and
the latter all those new laptops with the new keyboard layout.

Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
---
 drivers/platform/x86/toshiba_acpi.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index dbcb7a8..e6aa8f9 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -116,6 +116,7 @@ MODULE_LICENSE("GPL");
 #define HCI_KBD_ILLUMINATION		0x0095
 #define HCI_ECO_MODE			0x0097
 #define HCI_ACCELEROMETER2		0x00a6
+#define HCI_HOTKEY_EVENT_TYPE		0xc000
 #define SCI_PANEL_POWER_ON		0x010d
 #define SCI_ILLUMINATION		0x014e
 #define SCI_USB_SLEEP_CHARGE		0x0150
@@ -127,8 +128,11 @@ MODULE_LICENSE("GPL");
 
 /* field definitions */
 #define HCI_ACCEL_MASK			0x7fff
+#define HCI_HOTKEY_EVENT_NORMAL		0x10
+#define HCI_HOTKEY_EVENT_SPECIAL	0x11
 #define HCI_HOTKEY_DISABLE		0x0b
 #define HCI_HOTKEY_ENABLE		0x09
+#define HCI_HOTKEY_ENABLE_SPECIAL	0x10
 #define HCI_LCD_BRIGHTNESS_BITS		3
 #define HCI_LCD_BRIGHTNESS_SHIFT	(16-HCI_LCD_BRIGHTNESS_BITS)
 #define HCI_LCD_BRIGHTNESS_LEVELS	(1 << HCI_LCD_BRIGHTNESS_BITS)
@@ -1149,6 +1153,28 @@ static int toshiba_usb_three_set(struct toshiba_acpi_dev *dev, u32 state)
 	return 0;
 }
 
+/* Hotkey event type */
+static int toshiba_hotkey_event_type_get(struct toshiba_acpi_dev *dev,
+					 u32 *hotkey_event_type)
+{
+	u32 val1 = 0x03;
+	u32 val2 = 0;
+	u32 result;
+
+	result = hci_read2(dev, HCI_HOTKEY_EVENT_TYPE, &val1, &val2);
+	if (result == TOS_FAILURE) {
+		pr_err("ACPI callto get Hotkey Event type failed\n");
+		return -EIO;
+	} else if (result == TOS_NOT_SUPPORTED) {
+		pr_info("Hotkey Event type not supported\n");
+		return -ENODEV;
+	}
+
+	*hotkey_event_type = val2;
+
+	return 0;
+}
+
 /* Bluetooth rfkill handlers */
 
 static u32 hci_get_bt_present(struct toshiba_acpi_dev *dev, bool *present)
-- 
2.2.2

[PATCH 2/3] toshiba_acpi: Use the Hotkey Event Type function for keymap choosing

[Azael Avalos]

With the previous patch adding support to "Hotkey Event Type", we can
now use the type to distinguish which keymap to use.

This patch changes the toshiba_acpi_setup_keyboard function to make
use of the hotkey event type to choose the correct keymap without the
need to use the DMI matching list.

Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
---
 drivers/platform/x86/toshiba_acpi.c | 51 +++++++++++++------------------------
 1 file changed, 18 insertions(+), 33 deletions(-)

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index e6aa8f9..10e0773 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -49,7 +49,6 @@
 #include <linux/workqueue.h>
 #include <linux/i8042.h>
 #include <linux/acpi.h>
-#include <linux/dmi.h>
 #include <linux/uaccess.h>
 
 MODULE_AUTHOR("John Belmonte");
@@ -178,6 +177,7 @@ struct toshiba_acpi_dev {
 	int kbd_mode;
 	int kbd_time;
 	int usbsc_bat_level;
+	int hotkey_event_type;
 
 	unsigned int illumination_supported:1;
 	unsigned int video_supported:1;
@@ -247,29 +247,6 @@ static const struct key_entry toshiba_acpi_keymap[] = {
 	{ KE_END, 0 },
 };
 
-/* alternative keymap */
-static const struct dmi_system_id toshiba_alt_keymap_dmi[] = {
-	{
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "Satellite M840"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "Qosmio X75-A"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "TECRA A50-A"),
-		},
-	},
-	{}
-};
-
 static const struct key_entry toshiba_acpi_alt_keymap[] = {
 	{ KE_KEY, 0x157, { KEY_MUTE } },
 	{ KE_KEY, 0x102, { KEY_ZOOMOUT } },
@@ -2460,10 +2437,22 @@ static void toshiba_acpi_process_hotkeys(struct toshiba_acpi_dev *dev)
 
 static int toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
 {
+	const struct key_entry *keymap = NULL;
 	acpi_handle ec_handle;
-	int error;
+	u32  events_type;
 	u32 hci_result;
-	const struct key_entry *keymap = toshiba_acpi_keymap;
+	int error;
+
+	error = toshiba_acpi_enable_hotkeys(dev);
+	if (error)
+		return error;
+
+	error = toshiba_hotkey_event_type_get(dev, &events_type);
+	if (error) {
+		pr_err("Unable to query Hotkey Event Type\n");
+		return error;
+	}
+	dev->hotkey_event_type = events_type;
 
 	dev->hotkey_dev = input_allocate_device();
 	if (!dev->hotkey_dev)
@@ -2473,7 +2462,9 @@ static int toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
 	dev->hotkey_dev->phys = "toshiba_acpi/input0";
 	dev->hotkey_dev->id.bustype = BUS_HOST;
 
-	if (dmi_check_system(toshiba_alt_keymap_dmi))
+	if (events_type == HCI_HOTKEY_EVENT_NORMAL)
+		keymap = toshiba_acpi_keymap;
+	else if (events_type == HCI_HOTKEY_EVENT_SPECIAL)
 		keymap = toshiba_acpi_alt_keymap;
 	error = sparse_keymap_setup(dev->hotkey_dev, keymap, NULL);
 	if (error)
@@ -2516,12 +2507,6 @@ static int toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
 		goto err_remove_filter;
 	}
 
-	error = toshiba_acpi_enable_hotkeys(dev);
-	if (error) {
-		pr_info("Unable to enable hotkeys\n");
-		goto err_remove_filter;
-	}
-
 	error = input_register_device(dev->hotkey_dev);
 	if (error) {
 		pr_info("Unable to register input device\n");
-- 
2.2.2

[PATCH 3/3] toshiba_acpi: Fix the enabling of the Special Functions

[Azael Avalos]

Some Toshiba laptops with the "Special Functions" feature enabled
fail to properly enable such feature unless a specific value is
used to enable the hotkey events.

This patch adds a new function called "*_enable_special_functions",
that simply makes a call to the HCI_HOTKEY_EVENT call, but this time
we are using a different parameter to make the "Special Functions"
mode work as expected.

Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
---
 drivers/platform/x86/toshiba_acpi.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 10e0773..09c6a2f 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -2336,6 +2336,20 @@ static int toshiba_acpi_enable_hotkeys(struct toshiba_acpi_dev *dev)
 	return 0;
 }
 
+static void toshiba_acpi_enable_special_functions(struct toshiba_acpi_dev *dev)
+{
+	u32 result;
+
+	/*
+	 * Re-activate the hotkeys, but this time, we are using the
+	 * "Special Functions" mode.
+	 */
+	result = hci_write1(dev, HCI_HOTKEY_EVENT,
+		    HCI_HOTKEY_ENABLE_SPECIAL);
+	if (result != TOS_SUCCESS)
+		pr_err("Could not enable the Special Function mode\n");
+}
+
 static bool toshiba_acpi_i8042_filter(unsigned char data, unsigned char str,
 				      struct serio *port)
 {
@@ -2738,6 +2752,12 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
 
 	ret = toshiba_function_keys_get(dev, &dummy);
 	dev->kbd_function_keys_supported = !ret;
+	/*
+	 * Enable the "Special Functions" mode only if they are
+	 * supported and if they are activated.
+	 */
+	if (dev->kbd_function_keys_supported && dummy == 1)
+		toshiba_acpi_enable_special_functions(dev);
 
 	ret = toshiba_panel_power_on_get(dev, &dummy);
 	dev->panel_power_on_supported = !ret;
-- 
2.2.2

Re: [PATCH 1/3] toshiba_acpi: Add Hotkey Event Type function and definitions

[Darren Hart]

On Thu, Feb 26, 2015 at 10:57:12AM -0700, Azael Avalos wrote:

Hi Azael,

I'm pretty behind on this one, apologies.

> This patch adds support to query the "Hotkey Event Type" the system
> supports.
> 
> There are two main event types (so far), 0x10 and 0x11, with the
> first beign all those laptops that have the old keyboard layout, and

being

> the latter all those new laptops with the new keyboard layout.
> 

Some concern about this binary "new" and "old". What happens in 2 years when
they decide to break^W change our keyboards again?

> Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
> ---
>  drivers/platform/x86/toshiba_acpi.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index dbcb7a8..e6aa8f9 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -116,6 +116,7 @@ MODULE_LICENSE("GPL");
>  #define HCI_KBD_ILLUMINATION		0x0095
>  #define HCI_ECO_MODE			0x0097
>  #define HCI_ACCELEROMETER2		0x00a6
> +#define HCI_HOTKEY_EVENT_TYPE		0xc000
>  #define SCI_PANEL_POWER_ON		0x010d
>  #define SCI_ILLUMINATION		0x014e
>  #define SCI_USB_SLEEP_CHARGE		0x0150
> @@ -127,8 +128,11 @@ MODULE_LICENSE("GPL");
>  
>  /* field definitions */
>  #define HCI_ACCEL_MASK			0x7fff
> +#define HCI_HOTKEY_EVENT_NORMAL		0x10
> +#define HCI_HOTKEY_EVENT_SPECIAL	0x11

NORMAL and SPECIAL? Hrm... what comes after special? EXTRASPECIAL? ;-)

Or am I not looking at this right? Is there reason to expect these two to be
sufficient?

>  #define HCI_HOTKEY_DISABLE		0x0b
>  #define HCI_HOTKEY_ENABLE		0x09
> +#define HCI_HOTKEY_ENABLE_SPECIAL	0x10
>  #define HCI_LCD_BRIGHTNESS_BITS		3
>  #define HCI_LCD_BRIGHTNESS_SHIFT	(16-HCI_LCD_BRIGHTNESS_BITS)
>  #define HCI_LCD_BRIGHTNESS_LEVELS	(1 << HCI_LCD_BRIGHTNESS_BITS)
> @@ -1149,6 +1153,28 @@ static int toshiba_usb_three_set(struct toshiba_acpi_dev *dev, u32 state)
>  	return 0;
>  }
>  
> +/* Hotkey event type */
> +static int toshiba_hotkey_event_type_get(struct toshiba_acpi_dev *dev,
> +					 u32 *hotkey_event_type)
> +{
> +	u32 val1 = 0x03;
> +	u32 val2 = 0;
> +	u32 result;
> +
> +	result = hci_read2(dev, HCI_HOTKEY_EVENT_TYPE, &val1, &val2);
> +	if (result == TOS_FAILURE) {
> +		pr_err("ACPI callto get Hotkey Event type failed\n");

call to

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 2/3] toshiba_acpi: Use the Hotkey Event Type function for keymap choosing

[Darren Hart]

On Thu, Feb 26, 2015 at 10:57:13AM -0700, Azael Avalos wrote:
> With the previous patch adding support to "Hotkey Event Type", we can
> now use the type to distinguish which keymap to use.
> 
> This patch changes the toshiba_acpi_setup_keyboard function to make
> use of the hotkey event type to choose the correct keymap without the
> need to use the DMI matching list.

Yay!

Pending discussion on 1/3.

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 3/3] toshiba_acpi: Fix the enabling of the Special Functions

[Darren Hart]

On Thu, Feb 26, 2015 at 10:57:14AM -0700, Azael Avalos wrote:
> Some Toshiba laptops with the "Special Functions" feature enabled
> fail to properly enable such feature unless a specific value is
> used to enable the hotkey events.

The specific value being... HCI_HOTKEY_ENABLE_SPECIAL? And we do this for ALL
systems that support special keys right (and have them enabled)? Not just the
ones that fail to properly enable them?

The above makes it sound like a work around, but nothing in the implementation
suggests that to me. Is this a workaround?

> This patch adds a new function called "*_enable_special_functions",
> that simply makes a call to the HCI_HOTKEY_EVENT call, but this time
> we are using a different parameter to make the "Special Functions"
> mode work as expected.
> 
> Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
> ---
>  drivers/platform/x86/toshiba_acpi.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index 10e0773..09c6a2f 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -2336,6 +2336,20 @@ static int toshiba_acpi_enable_hotkeys(struct toshiba_acpi_dev *dev)
>  	return 0;
>  }
>  
> +static void toshiba_acpi_enable_special_functions(struct toshiba_acpi_dev *dev)
> +{
> +	u32 result;
> +
> +	/*
> +	 * Re-activate the hotkeys, but this time, we are using the
> +	 * "Special Functions" mode.
> +	 */
> +	result = hci_write1(dev, HCI_HOTKEY_EVENT,
> +		    HCI_HOTKEY_ENABLE_SPECIAL);

One line.


-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 0/3] toshiba_acpi: Hotkey handling and keymap updates

[Darren Hart]

On Thu, Feb 26, 2015 at 10:57:11AM -0700, Azael Avalos wrote:
> These patches add support to a new function that queries the supported Hotkey
> Event Type, making the use of the DMI matching unnecessary and also fixes
> the "Special Functions" mode on some laptops.
> 
> Azael Avalos (3):
>   toshiba_acpi: Add Hotkey Event Type function and definitions
>   toshiba_acpi: Use the Hotkey Event Type function for keymap choosing
>   toshiba_acpi: Fix the enabling of the Special Functions
> 
>  drivers/platform/x86/toshiba_acpi.c | 97 ++++++++++++++++++++++++-------------
>  1 file changed, 64 insertions(+), 33 deletions(-)
> 

Would you also include a MAINTAINERS update, adding yourself as the
thoshiba-acpi and possibly the toshiba-haps drivers? This helps ensure you get
Cc'd. I'll also look for you to pre-screen any other submission to these drivers
and provide your Reviewed-by before I merge them.

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 1/3] toshiba_acpi: Add Hotkey Event Type function and definitions

[Azael Avalos]

Hi Darren,

2015-03-06 11:21 GMT-07:00 Darren Hart <dvhart@infradead.org>:
> On Thu, Feb 26, 2015 at 10:57:12AM -0700, Azael Avalos wrote:
>
> Hi Azael,
>
> I'm pretty behind on this one, apologies.
>

No wories, we still have plenty of time for 4.1 ;-)
I simply wanted to send these early to get as much tests as possible.

>> This patch adds support to query the "Hotkey Event Type" the system
>> supports.
>>
>> There are two main event types (so far), 0x10 and 0x11, with the
>> first beign all those laptops that have the old keyboard layout, and
>
> being
>
>> the latter all those new laptops with the new keyboard layout.
>>
>
> Some concern about this binary "new" and "old". What happens in 2 years when
> they decide to break^W change our keyboards again?

Well, a new key mapping will need to be added, and hopefully
a new value (other than 0x10 and 0x11) to identify the "newer"
layout, only time will tell.

>
>> Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
>> ---
>>  drivers/platform/x86/toshiba_acpi.c | 26 ++++++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>>
>> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
>> index dbcb7a8..e6aa8f9 100644
>> --- a/drivers/platform/x86/toshiba_acpi.c
>> +++ b/drivers/platform/x86/toshiba_acpi.c
>> @@ -116,6 +116,7 @@ MODULE_LICENSE("GPL");
>>  #define HCI_KBD_ILLUMINATION         0x0095
>>  #define HCI_ECO_MODE                 0x0097
>>  #define HCI_ACCELEROMETER2           0x00a6
>> +#define HCI_HOTKEY_EVENT_TYPE                0xc000
>>  #define SCI_PANEL_POWER_ON           0x010d
>>  #define SCI_ILLUMINATION             0x014e
>>  #define SCI_USB_SLEEP_CHARGE         0x0150
>> @@ -127,8 +128,11 @@ MODULE_LICENSE("GPL");
>>
>>  /* field definitions */
>>  #define HCI_ACCEL_MASK                       0x7fff
>> +#define HCI_HOTKEY_EVENT_NORMAL              0x10
>> +#define HCI_HOTKEY_EVENT_SPECIAL     0x11
>
> NORMAL and SPECIAL? Hrm... what comes after special? EXTRASPECIAL? ;-)

Hehe, well, those can be changed to something like
HCI_HOTKEY_EVENT_TYPE{1, 2}
I just named them after the "special events" feature, as the value
returned from HCI_HOTKEY_EVENT_TYPE is related to.

>
> Or am I not looking at this right? Is there reason to expect these two to be
> sufficient?

We can use the SCI_KBD_FUNCTION_KEYS too, all newer laptops
that now come with the new layout support this feature.
I can test for its presence and also for the value returned by
HCI_HOTKEY_EVENT_TYPE.

>
>>  #define HCI_HOTKEY_DISABLE           0x0b
>>  #define HCI_HOTKEY_ENABLE            0x09
>> +#define HCI_HOTKEY_ENABLE_SPECIAL    0x10
>>  #define HCI_LCD_BRIGHTNESS_BITS              3
>>  #define HCI_LCD_BRIGHTNESS_SHIFT     (16-HCI_LCD_BRIGHTNESS_BITS)
>>  #define HCI_LCD_BRIGHTNESS_LEVELS    (1 << HCI_LCD_BRIGHTNESS_BITS)
>> @@ -1149,6 +1153,28 @@ static int toshiba_usb_three_set(struct toshiba_acpi_dev *dev, u32 state)
>>       return 0;
>>  }
>>
>> +/* Hotkey event type */
>> +static int toshiba_hotkey_event_type_get(struct toshiba_acpi_dev *dev,
>> +                                      u32 *hotkey_event_type)
>> +{
>> +     u32 val1 = 0x03;
>> +     u32 val2 = 0;
>> +     u32 result;
>> +
>> +     result = hci_read2(dev, HCI_HOTKEY_EVENT_TYPE, &val1, &val2);
>> +     if (result == TOS_FAILURE) {
>> +             pr_err("ACPI callto get Hotkey Event type failed\n");
>
> call to
>

Caught this one after I sent the patches :-P

> Thanks,
>
> --
> Darren Hart
> Intel Open Source Technology Center


Cheers
Azael


-- 
-- El mundo apesta y vosotros apestais tambien --

Re: [PATCH 3/3] toshiba_acpi: Fix the enabling of the Special Functions

[Azael Avalos]

Hi Darren,

2015-03-06 11:28 GMT-07:00 Darren Hart <dvhart@infradead.org>:
> On Thu, Feb 26, 2015 at 10:57:14AM -0700, Azael Avalos wrote:
>> Some Toshiba laptops with the "Special Functions" feature enabled
>> fail to properly enable such feature unless a specific value is
>> used to enable the hotkey events.
>
> The specific value being... HCI_HOTKEY_ENABLE_SPECIAL? And we do this for ALL
> systems that support special keys right (and have them enabled)? Not just the
> ones that fail to properly enable them?

For all of them supporting "special functions" and have them enabled.

>
> The above makes it sound like a work around, but nothing in the implementation
> suggests that to me. Is this a workaround?

This is more a fix than a workaround, on the laptops I tested this value
is needed in order to properly activate the "special functions", if not,
they will behave like "normal" even if the "special functions"are enabled.

Tho' I'm not sure if it's only the models I tested or all of them, as I don't
have that much hardware to test.

>
>> This patch adds a new function called "*_enable_special_functions",
>> that simply makes a call to the HCI_HOTKEY_EVENT call, but this time
>> we are using a different parameter to make the "Special Functions"
>> mode work as expected.
>>
>> Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
>> ---
>>  drivers/platform/x86/toshiba_acpi.c | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
>> index 10e0773..09c6a2f 100644
>> --- a/drivers/platform/x86/toshiba_acpi.c
>> +++ b/drivers/platform/x86/toshiba_acpi.c
>> @@ -2336,6 +2336,20 @@ static int toshiba_acpi_enable_hotkeys(struct toshiba_acpi_dev *dev)
>>       return 0;
>>  }
>>
>> +static void toshiba_acpi_enable_special_functions(struct toshiba_acpi_dev *dev)
>> +{
>> +     u32 result;
>> +
>> +     /*
>> +      * Re-activate the hotkeys, but this time, we are using the
>> +      * "Special Functions" mode.
>> +      */
>> +     result = hci_write1(dev, HCI_HOTKEY_EVENT,
>> +                 HCI_HOTKEY_ENABLE_SPECIAL);
>
> One line.

Somehow I thought this was gonna pass the 80 chars limit, seems not.
Will change in v2.

>
>
> --
> Darren Hart
> Intel Open Source Technology Center


Cheers
Azael


-- 
-- El mundo apesta y vosotros apestais tambien --

Re: [PATCH 0/3] toshiba_acpi: Hotkey handling and keymap updates

[Azael Avalos]

Hi Darren,

2015-03-06 11:34 GMT-07:00 Darren Hart <dvhart@infradead.org>:
> On Thu, Feb 26, 2015 at 10:57:11AM -0700, Azael Avalos wrote:
>> These patches add support to a new function that queries the supported Hotkey
>> Event Type, making the use of the DMI matching unnecessary and also fixes
>> the "Special Functions" mode on some laptops.
>>
>> Azael Avalos (3):
>>   toshiba_acpi: Add Hotkey Event Type function and definitions
>>   toshiba_acpi: Use the Hotkey Event Type function for keymap choosing
>>   toshiba_acpi: Fix the enabling of the Special Functions
>>
>>  drivers/platform/x86/toshiba_acpi.c | 97 ++++++++++++++++++++++++-------------
>>  1 file changed, 64 insertions(+), 33 deletions(-)
>>
>
> Would you also include a MAINTAINERS update, adding yourself as the
> thoshiba-acpi and possibly the toshiba-haps drivers? This helps ensure you get
> Cc'd. I'll also look for you to pre-screen any other submission to these drivers
> and provide your Reviewed-by before I merge them.

Sure, will do, I have a pending patch updating the *_notify events,
I will send a new patch alongside.

>
> --
> Darren Hart
> Intel Open Source Technology Center


Cheers
Azael


-- 
-- El mundo apesta y vosotros apestais tambien --

Re: [PATCH 1/3] toshiba_acpi: Add Hotkey Event Type function and definitions

[Darren Hart]

On Fri, Mar 06, 2015 at 11:42:50AM -0700, Azael Avalos wrote:
> Hi Darren,
> 
> 2015-03-06 11:21 GMT-07:00 Darren Hart <dvhart@infradead.org>:
> > On Thu, Feb 26, 2015 at 10:57:12AM -0700, Azael Avalos wrote:
> >
> > Hi Azael,
> >
> > I'm pretty behind on this one, apologies.
> >
> 
> No wories, we still have plenty of time for 4.1 ;-)
> I simply wanted to send these early to get as much tests as possible.
> 
> >> This patch adds support to query the "Hotkey Event Type" the system
> >> supports.
> >>
> >> There are two main event types (so far), 0x10 and 0x11, with the
> >> first beign all those laptops that have the old keyboard layout, and
> >
> > being
> >
> >> the latter all those new laptops with the new keyboard layout.
> >>
> >
> > Some concern about this binary "new" and "old". What happens in 2 years when
> > they decide to break^W change our keyboards again?
> 
> Well, a new key mapping will need to be added, and hopefully
> a new value (other than 0x10 and 0x11) to identify the "newer"
> layout, only time will tell.

OK, anything else is just idle speculation I guess. So just fix the typos and
resend please.

--
Darren

> 
> >
> >> Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
> >> ---
> >>  drivers/platform/x86/toshiba_acpi.c | 26 ++++++++++++++++++++++++++
> >>  1 file changed, 26 insertions(+)
> >>
> >> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> >> index dbcb7a8..e6aa8f9 100644
> >> --- a/drivers/platform/x86/toshiba_acpi.c
> >> +++ b/drivers/platform/x86/toshiba_acpi.c
> >> @@ -116,6 +116,7 @@ MODULE_LICENSE("GPL");
> >>  #define HCI_KBD_ILLUMINATION         0x0095
> >>  #define HCI_ECO_MODE                 0x0097
> >>  #define HCI_ACCELEROMETER2           0x00a6
> >> +#define HCI_HOTKEY_EVENT_TYPE                0xc000
> >>  #define SCI_PANEL_POWER_ON           0x010d
> >>  #define SCI_ILLUMINATION             0x014e
> >>  #define SCI_USB_SLEEP_CHARGE         0x0150
> >> @@ -127,8 +128,11 @@ MODULE_LICENSE("GPL");
> >>
> >>  /* field definitions */
> >>  #define HCI_ACCEL_MASK                       0x7fff
> >> +#define HCI_HOTKEY_EVENT_NORMAL              0x10
> >> +#define HCI_HOTKEY_EVENT_SPECIAL     0x11
> >
> > NORMAL and SPECIAL? Hrm... what comes after special? EXTRASPECIAL? ;-)
> 
> Hehe, well, those can be changed to something like
> HCI_HOTKEY_EVENT_TYPE{1, 2}
> I just named them after the "special events" feature, as the value
> returned from HCI_HOTKEY_EVENT_TYPE is related to.
> 
> >
> > Or am I not looking at this right? Is there reason to expect these two to be
> > sufficient?
> 
> We can use the SCI_KBD_FUNCTION_KEYS too, all newer laptops
> that now come with the new layout support this feature.
> I can test for its presence and also for the value returned by
> HCI_HOTKEY_EVENT_TYPE.
> 
> >
> >>  #define HCI_HOTKEY_DISABLE           0x0b
> >>  #define HCI_HOTKEY_ENABLE            0x09
> >> +#define HCI_HOTKEY_ENABLE_SPECIAL    0x10
> >>  #define HCI_LCD_BRIGHTNESS_BITS              3
> >>  #define HCI_LCD_BRIGHTNESS_SHIFT     (16-HCI_LCD_BRIGHTNESS_BITS)
> >>  #define HCI_LCD_BRIGHTNESS_LEVELS    (1 << HCI_LCD_BRIGHTNESS_BITS)
> >> @@ -1149,6 +1153,28 @@ static int toshiba_usb_three_set(struct toshiba_acpi_dev *dev, u32 state)
> >>       return 0;
> >>  }
> >>
> >> +/* Hotkey event type */
> >> +static int toshiba_hotkey_event_type_get(struct toshiba_acpi_dev *dev,
> >> +                                      u32 *hotkey_event_type)
> >> +{
> >> +     u32 val1 = 0x03;
> >> +     u32 val2 = 0;
> >> +     u32 result;
> >> +
> >> +     result = hci_read2(dev, HCI_HOTKEY_EVENT_TYPE, &val1, &val2);
> >> +     if (result == TOS_FAILURE) {
> >> +             pr_err("ACPI callto get Hotkey Event type failed\n");
> >
> > call to
> >
> 
> Caught this one after I sent the patches :-P
> 
> > Thanks,
> >
> > --
> > Darren Hart
> > Intel Open Source Technology Center
> 
> 
> Cheers
> Azael
> 
> 
> -- 
> -- El mundo apesta y vosotros apestais tambien --
> 

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 3/3] toshiba_acpi: Fix the enabling of the Special Functions

[Darren Hart]

On Fri, Mar 06, 2015 at 11:52:41AM -0700, Azael Avalos wrote:
> Hi Darren,
> 
> 2015-03-06 11:28 GMT-07:00 Darren Hart <dvhart@infradead.org>:
> > On Thu, Feb 26, 2015 at 10:57:14AM -0700, Azael Avalos wrote:
> >> Some Toshiba laptops with the "Special Functions" feature enabled
> >> fail to properly enable such feature unless a specific value is
> >> used to enable the hotkey events.
> >
> > The specific value being... HCI_HOTKEY_ENABLE_SPECIAL? And we do this for ALL
> > systems that support special keys right (and have them enabled)? Not just the
> > ones that fail to properly enable them?
> 
> For all of them supporting "special functions" and have them enabled.
> 
> >
> > The above makes it sound like a work around, but nothing in the implementation
> > suggests that to me. Is this a workaround?
> 
> This is more a fix than a workaround, on the laptops I tested this value
> is needed in order to properly activate the "special functions", if not,
> they will behave like "normal" even if the "special functions"are enabled.
> 
> Tho' I'm not sure if it's only the models I tested or all of them, as I don't
> have that much hardware to test.

OK, I'll wait for v2 and queue for 4.1.

-- 
Darren Hart
Intel Open Source Technology Center