[PATCH v2 0/3] toshiba_acpi: Hotkeys and event handling changes

[PATCH v2 0/3] toshiba_acpi: Hotkeys and event handling changes

[Azael Avalos]

These patches change the current code to accomodate the handling
of more events, since so far, it only handles hotkey events (0x80),
move the hotkey enabling code to a sub function to avoid duplication,
and add event 0x92 which indicates a change in the keyboard backlight
mode.

Changes since V1:
- Split patch 02 in two patches
- Some misc changes in the description of the patches (a bit wider now)

Azael Avalos (3):
  toshiba_acpi: Move hotkey enabling code to own function
  toshiba_acpi: Change notify funtion to handle more events
  toshiba_acpi: Add keyboard backlight mode change event

 drivers/platform/x86/toshiba_acpi.c | 139 ++++++++++++++++++++++--------------
 1 file changed, 86 insertions(+), 53 deletions(-)

-- 
2.1.2

[PATCH v2 1/3] toshiba_acpi: Move hotkey enabling code to own function

[Azael Avalos]

The hotkey enabling code is being used by toshiba_acpi_setup_keyboard
and also by toshiba_acpi_resume.

This patch creates a new function called toshiba_acpi_enable_hotkeys
to be used by these two functions to avoid duplicating code.

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

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index a329469..8bb07c7 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -1573,6 +1573,28 @@ static umode_t toshiba_sysfs_is_visible(struct kobject *kobj,
 	return exists ? attr->mode : 0;
 }
 
+/*
+ * Hotkeys
+ */
+static int toshiba_acpi_enable_hotkeys(struct toshiba_acpi_dev *dev)
+{
+	acpi_status status;
+	u32 result;
+
+	status = acpi_evaluate_object(dev->acpi_dev->handle,
+				      "ENAB", NULL, NULL);
+	if (ACPI_FAILURE(status))
+		return -ENODEV;
+
+	result = hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_ENABLE);
+	if (result == TOS_FAILURE)
+		return -EIO;
+	else if (result == TOS_NOT_SUPPORTED)
+		return -ENODEV;
+
+	return 0;
+}
+
 static bool toshiba_acpi_i8042_filter(unsigned char data, unsigned char str,
 				      struct serio *port)
 {
@@ -1637,7 +1659,6 @@ static void toshiba_acpi_report_hotkey(struct toshiba_acpi_dev *dev,
 
 static int toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
 {
-	acpi_status status;
 	acpi_handle ec_handle;
 	int error;
 	u32 hci_result;
@@ -1664,7 +1685,6 @@ static int toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
 	 * supported, so if it's present set up an i8042 key filter
 	 * for this purpose.
 	 */
-	status = AE_ERROR;
 	ec_handle = ec_get_handle();
 	if (ec_handle && acpi_has_method(ec_handle, "NTFY")) {
 		INIT_WORK(&dev->hotkey_work, toshiba_acpi_hotkey_work);
@@ -1695,10 +1715,9 @@ static int toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
 		goto err_remove_filter;
 	}
 
-	status = acpi_evaluate_object(dev->acpi_dev->handle, "ENAB", NULL, NULL);
-	if (ACPI_FAILURE(status)) {
+	error = toshiba_acpi_enable_hotkeys(dev);
+	if (error) {
 		pr_info("Unable to enable hotkeys\n");
-		error = -ENODEV;
 		goto err_remove_filter;
 	}
 
@@ -1708,7 +1727,6 @@ static int toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
 		goto err_remove_filter;
 	}
 
-	hci_result = hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_ENABLE);
 	return 0;
 
  err_remove_filter:
@@ -2006,16 +2024,12 @@ static int toshiba_acpi_suspend(struct device *device)
 static int toshiba_acpi_resume(struct device *device)
 {
 	struct toshiba_acpi_dev *dev = acpi_driver_data(to_acpi_device(device));
-	u32 result;
-	acpi_status status;
+	int error;
 
 	if (dev->hotkey_dev) {
-		status = acpi_evaluate_object(dev->acpi_dev->handle, "ENAB",
-				NULL, NULL);
-		if (ACPI_FAILURE(status))
+		error = toshiba_acpi_enable_hotkeys(dev);
+		if (error)
 			pr_info("Unable to re-enable hotkeys\n");
-
-		result = hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_ENABLE);
 	}
 
 	return 0;
-- 
2.1.2

[PATCH v2 2/3] toshiba_acpi: Change notify funtion to handle more events

[Azael Avalos]

Currently the function toshiba_acpi_notify only takes care of hotkeys,
however, the TOSXXXX devices receive more events that can be useful.

This patch changes the function to be able to handle more events,
and in the process, move all hotkey related code residing in it to
a new function called toshiba_acpi_process_hotkeys.

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

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 8bb07c7..21ac11f 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -1657,6 +1657,43 @@ static void toshiba_acpi_report_hotkey(struct toshiba_acpi_dev *dev,
 		pr_info("Unknown key %x\n", scancode);
 }
 
+static void toshiba_acpi_process_hotkeys(struct toshiba_acpi_dev *dev)
+{
+	u32 hci_result, value;
+	int retries = 3;
+	int scancode;
+
+	if (dev->info_supported) {
+		scancode = toshiba_acpi_query_hotkey(dev);
+		if (scancode < 0)
+			pr_err("Failed to query hotkey event\n");
+		else if (scancode != 0)
+			toshiba_acpi_report_hotkey(dev, scancode);
+	} else if (dev->system_event_supported) {
+		do {
+			hci_result = hci_read1(dev, HCI_SYSTEM_EVENT, &value);
+			switch (hci_result) {
+			case TOS_SUCCESS:
+				toshiba_acpi_report_hotkey(dev, (int)value);
+				break;
+			case TOS_NOT_SUPPORTED:
+				/*
+				 * This is a workaround for an unresolved
+				 * issue on some machines where system events
+				 * sporadically become disabled.
+				 */
+				hci_result =
+					hci_write1(dev, HCI_SYSTEM_EVENT, 1);
+				pr_notice("Re-enabled hotkeys\n");
+				/* fall through */
+			default:
+				retries--;
+				break;
+			}
+		} while (retries && hci_result != TOS_FIFO_EMPTY);
+	}
+}
+
 static int toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
 {
 	acpi_handle ec_handle;
@@ -1971,41 +2008,22 @@ error:
 static void toshiba_acpi_notify(struct acpi_device *acpi_dev, u32 event)
 {
 	struct toshiba_acpi_dev *dev = acpi_driver_data(acpi_dev);
-	u32 hci_result, value;
-	int retries = 3;
-	int scancode;
-
-	if (event != 0x80)
-		return;
+	int ret;
 
-	if (dev->info_supported) {
-		scancode = toshiba_acpi_query_hotkey(dev);
-		if (scancode < 0)
-			pr_err("Failed to query hotkey event\n");
-		else if (scancode != 0)
-			toshiba_acpi_report_hotkey(dev, scancode);
-	} else if (dev->system_event_supported) {
-		do {
-			hci_result = hci_read1(dev, HCI_SYSTEM_EVENT, &value);
-			switch (hci_result) {
-			case TOS_SUCCESS:
-				toshiba_acpi_report_hotkey(dev, (int)value);
-				break;
-			case TOS_NOT_SUPPORTED:
-				/*
-				 * This is a workaround for an unresolved
-				 * issue on some machines where system events
-				 * sporadically become disabled.
-				 */
-				hci_result =
-					hci_write1(dev, HCI_SYSTEM_EVENT, 1);
-				pr_notice("Re-enabled hotkeys\n");
-				/* fall through */
-			default:
-				retries--;
-				break;
-			}
-		} while (retries && hci_result != TOS_FIFO_EMPTY);
+	switch (event) {
+	case 0x80: /* Hotkeys and some system events */
+		toshiba_acpi_process_hotkeys(dev);
+		break;
+	case 0x81: /* Unknown */
+	case 0x82: /* Unknown */
+	case 0x83: /* Unknown */
+	case 0x8c: /* Unknown */
+	case 0x8e: /* Unknown */
+	case 0x8f: /* Unknown */
+	case 0x90: /* Unknown */
+	default:
+		pr_info("Unknown event received %x\n", event);
+		break;
 	}
 }
 
-- 
2.1.2

Re: [PATCH v2 2/3] toshiba_acpi: Change notify funtion to handle more events

[Darren Hart]

On Wed, Dec 03, 2014 at 11:42:04PM -0700, Azael Avalos wrote:
> @@ -1971,41 +2008,22 @@ error:
>  static void toshiba_acpi_notify(struct acpi_device *acpi_dev, u32 event)
>  {
>  	struct toshiba_acpi_dev *dev = acpi_driver_data(acpi_dev);
> -	u32 hci_result, value;
> -	int retries = 3;
> -	int scancode;
> -
> -	if (event != 0x80)
> -		return;
> +	int ret;

  CC      drivers/platform/x86/toshiba_acpi.o
drivers/platform/x86/toshiba_acpi.c: In function ‘toshiba_acpi_notify’:
drivers/platform/x86/toshiba_acpi.c:2012:6: warning: unused variable ‘ret’ [-Wunused-variable]
    int ret;
          ^

Please compile check each patch.

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH v2 2/3] toshiba_acpi: Change notify funtion to handle more events

[Azael Avalos]

Hi Darren,

2014-12-03 6:12 GMT-07:00 Darren Hart <dvhart@infradead.org>:
> On Wed, Dec 03, 2014 at 11:42:04PM -0700, Azael Avalos wrote:
>> @@ -1971,41 +2008,22 @@ error:
>>  static void toshiba_acpi_notify(struct acpi_device *acpi_dev, u32 event)
>>  {
>>       struct toshiba_acpi_dev *dev = acpi_driver_data(acpi_dev);
>> -     u32 hci_result, value;
>> -     int retries = 3;
>> -     int scancode;
>> -
>> -     if (event != 0x80)
>> -             return;
>> +     int ret;
>
>   CC      drivers/platform/x86/toshiba_acpi.o
> drivers/platform/x86/toshiba_acpi.c: In function ‘toshiba_acpi_notify’:
> drivers/platform/x86/toshiba_acpi.c:2012:6: warning: unused variable ‘ret’ [-Wunused-variable]
>     int ret;
>           ^
>
> Please compile check each patch.

Sorry about that, was a left over from the split.
Want me to send a V3? I can send them in few minutes.

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



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

Re: [PATCH v2 2/3] toshiba_acpi: Change notify funtion to handle more events

[Darren Hart]

On Thu, Dec 04, 2014 at 08:10:49PM -0700, Azael Avalos wrote:
> Hi Darren,
> 
> 2014-12-03 6:12 GMT-07:00 Darren Hart <dvhart@infradead.org>:
> > On Wed, Dec 03, 2014 at 11:42:04PM -0700, Azael Avalos wrote:
> >> @@ -1971,41 +2008,22 @@ error:
> >>  static void toshiba_acpi_notify(struct acpi_device *acpi_dev, u32 event)
> >>  {
> >>       struct toshiba_acpi_dev *dev = acpi_driver_data(acpi_dev);
> >> -     u32 hci_result, value;
> >> -     int retries = 3;
> >> -     int scancode;
> >> -
> >> -     if (event != 0x80)
> >> -             return;
> >> +     int ret;
> >
> >   CC      drivers/platform/x86/toshiba_acpi.o
> > drivers/platform/x86/toshiba_acpi.c: In function ‘toshiba_acpi_notify’:
> > drivers/platform/x86/toshiba_acpi.c:2012:6: warning: unused variable ‘ret’ [-Wunused-variable]
> >     int ret;
> >           ^
> >
> > Please compile check each patch.
> 
> Sorry about that, was a left over from the split.
> Want me to send a V3? I can send them in few minutes.

Yes please.

-- 
Darren Hart
Intel Open Source Technology Center

[PATCH v2 3/3] toshiba_acpi: Add keyboard backlight mode change event

[Azael Avalos]

A previous patch added support to handle more events.

This patch adds support to update the sysfs group whenever we receive
a 0x92 event, which indicates a change in the keyboard backlight mode.

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

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 21ac11f..5e9b298 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -1393,12 +1393,6 @@ static ssize_t toshiba_kbd_bl_mode_store(struct device *dev,
 		if (ret)
 			return ret;
 
-		/* Update sysfs entries on successful mode change*/
-		ret = sysfs_update_group(&toshiba->acpi_dev->dev.kobj,
-					 &toshiba_attr_group);
-		if (ret)
-			return ret;
-
 		toshiba->kbd_mode = mode;
 	}
 
@@ -2014,6 +2008,13 @@ static void toshiba_acpi_notify(struct acpi_device *acpi_dev, u32 event)
 	case 0x80: /* Hotkeys and some system events */
 		toshiba_acpi_process_hotkeys(dev);
 		break;
+	case 0x92: /* Keyboard backlight mode changed */
+		/* Update sysfs entries */
+		ret = sysfs_update_group(&acpi_dev->dev.kobj,
+					 &toshiba_attr_group);
+		if (ret)
+			pr_err("Unable to update sysfs entries\n");
+		break;
 	case 0x81: /* Unknown */
 	case 0x82: /* Unknown */
 	case 0x83: /* Unknown */
-- 
2.1.2