[PATCH v2 0/4] toshiba_acpi: Expanded hotkey support

[PATCH v2 0/4] toshiba_acpi: Expanded hotkey support

[Seth Forshee]

This is an update to the patches I sent previously to enable hotkeys on
many Toshiba models that aren't supported by the code currently in
toshiba_acpi. Once agains, here's the high-level summary:

 * Add support for reading scancodes using the INFO method when
   avaialable and for generating hotkey notifications by filtering Fn
   key presses from the keyboard
 * Adds a new EC interface allowing toshiba_acpi to get a handle to the
   EC, which is needed because the NTFY method used to generate the
   hotkey notifications is in the EC namespace
 * Support some additional hotkey scancodes
 * Blacklist hotkey support for some Satellite models due to a buggy
   implementation of the INFO method on this model

The only change since v1 is to add additional machines to the hotkey
blacklist. These machines were identified by scraping bug reports to
identify machines with the same INFO method implementation in their
DSDTs.

Thanks,
Seth


Azael Avalos (1):
  toshiba_acpi: Support additional hotkey scancodes

Seth Forshee (3):
  ACPI: EC: Add ec_get_handle()
  toshiba_acpi: Support alternate hotkey interfaces
  toshiba_acpi: Add blacklist for models with hotkey problems

 drivers/acpi/ec.c                   |   10 ++
 drivers/platform/x86/toshiba_acpi.c |  275 ++++++++++++++++++++++++++++++-----
 include/linux/acpi.h                |    1 +
 3 files changed, 249 insertions(+), 37 deletions(-)

[PATCH v2 1/4] ACPI: EC: Add ec_get_handle()

[Seth Forshee]

toshiba_acpi needs to execute an AML method within the EC namespace
to make hotkeys work on some platforms. Provide an interface to
allow it to easily get a handle to the EC namespace for this purpose.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/acpi/ec.c    |   10 ++++++++++
 include/linux/acpi.h |    1 +
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index b19a18d..e37615f 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -445,6 +445,16 @@ int ec_transaction(u8 command,
 
 EXPORT_SYMBOL(ec_transaction);
 
+/* Get the handle to the EC device */
+acpi_handle ec_get_handle(void)
+{
+	if (!first_ec)
+		return NULL;
+	return first_ec->handle;
+}
+
+EXPORT_SYMBOL(ec_get_handle);
+
 void acpi_ec_block_transactions(void)
 {
 	struct acpi_ec *ec = first_ec;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 6001b4da..ecccbb7 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -151,6 +151,7 @@ extern int ec_write(u8 addr, u8 val);
 extern int ec_transaction(u8 command,
                           const u8 *wdata, unsigned wdata_len,
                           u8 *rdata, unsigned rdata_len);
+extern acpi_handle ec_get_handle(void);
 
 #if defined(CONFIG_ACPI_WMI) || defined(CONFIG_ACPI_WMI_MODULE)
 
-- 
1.7.5.4

Re: [PATCH v2 1/4] ACPI: EC: Add ec_get_handle()

[Matthew Garrett]

On Tue, Jan 03, 2012 at 01:02:35PM -0600, Seth Forshee wrote:
> toshiba_acpi needs to execute an AML method within the EC namespace
> to make hotkeys work on some platforms. Provide an interface to
> allow it to easily get a handle to the EC namespace for this purpose.
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

Looks good to me - Len, do you want to take this through your tree or 
should I?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v2 1/4] ACPI: EC: Add ec_get_handle()

[Seth Forshee]

On Thu, Jan 05, 2012 at 01:27:08PM -0500, Konrad Rzeszutek Wilk wrote:
> 8
> On Jan 3, 2012 2:05 PM, "Seth Forshee" <seth.forshee@canonical.com> wrote:
> >
> > toshiba_acpi needs to execute an AML method within the EC namespace
> > to make hotkeys work on some platforms. Provide an interface to
> > allow it to easily get a handle to the EC namespace for this purpose.
> >
> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> > ---
> >  drivers/acpi/ec.c    |   10 ++++++++++
> >  include/linux/acpi.h |    1 +
> >  2 files changed, 11 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> > index b19a18d..e37615f 100644
> > --- a/drivers/acpi/ec.c
> > +++ b/drivers/acpi/ec.c
> > @@ -445,6 +445,16 @@ int ec_transaction(u8 command,
> >
> >  EXPORT_SYMBOL(ec_transaction);
> >
> > +/* Get the handle to the EC device */
> > +acpi_handle ec_get_handle(void)
> > +{
> > +       if (!first_ec)
> > +               return NULL;
> > +       return first_ec->handle;
> > +}
> > +
> > +EXPORT_SYMBOL(ec_get_handle);
> 
> shouldn't this be _GPL?

I don't know. All the other ec_* interfaces are EXPORT_SYMBOL, so I did
likewise. I'm happy to change it if _GPL is what ought to be used here.

Re: [PATCH v2 1/4] ACPI: EC: Add ec_get_handle()

[Konrad Rzeszutek Wilk]

On Thu, Jan 5, 2012 at 2:34 PM, Seth Forshee <seth.forshee@canonical.com> wrote:
> On Thu, Jan 05, 2012 at 01:27:08PM -0500, Konrad Rzeszutek Wilk wrote:
>> 8
>> On Jan 3, 2012 2:05 PM, "Seth Forshee" <seth.forshee@canonical.com> wrote:
>> >
>> > toshiba_acpi needs to execute an AML method within the EC namespace
>> > to make hotkeys work on some platforms. Provide an interface to
>> > allow it to easily get a handle to the EC namespace for this purpose.
>> >
>> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
>> > ---
>> >  drivers/acpi/ec.c    |   10 ++++++++++
>> >  include/linux/acpi.h |    1 +
>> >  2 files changed, 11 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
>> > index b19a18d..e37615f 100644
>> > --- a/drivers/acpi/ec.c
>> > +++ b/drivers/acpi/ec.c
>> > @@ -445,6 +445,16 @@ int ec_transaction(u8 command,
>> >
>> >  EXPORT_SYMBOL(ec_transaction);
>> >
>> > +/* Get the handle to the EC device */
>> > +acpi_handle ec_get_handle(void)
>> > +{
>> > +       if (!first_ec)
>> > +               return NULL;
>> > +       return first_ec->handle;
>> > +}
>> > +
>> > +EXPORT_SYMBOL(ec_get_handle);
>>
>> shouldn't this be _GPL?
>
> I don't know. All the other ec_* interfaces are EXPORT_SYMBOL, so I did
> likewise. I'm happy to change it if _GPL is what ought to be used here.

My understanding (and this is from reading Greg KH's patches) is that
any new interface should use _GPL variant unless there is an absolute
need for it. Say, a binary driver that uses this function and there is
no other way around it.

>

Re: [PATCH v2 1/4] ACPI: EC: Add ec_get_handle()

[Matthew Garrett]

On Sat, Jan 07, 2012 at 03:55:35PM -0500, Konrad Rzeszutek Wilk wrote:

> My understanding (and this is from reading Greg KH's patches) is that
> any new interface should use _GPL variant unless there is an absolute
> need for it. Say, a binary driver that uses this function and there is
> no other way around it.

A binary driver could implement the same functionality using exported 
interfaces from the ACPI core, so I don't see any benefit in insisting 
on _GPL here. _GPL is intended to be a hint that use of the interface is 
definitely an indication that you're creating a derived work.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

[PATCH v2 2/4] toshiba_acpi: Support alternate hotkey interfaces

[Seth Forshee]

There are two types of problems that prevent hotkeys from working
on many of the machines supported by toshiba_acpi. The first of
these is the lack of a functioning SCI for hotkey events. For these
machines it is possible to filter the Fn keypresses from the
keyboard and generate a notification by executing the ACPI NTFY
method.

The second problem is a lack of support for HCI_SYSTEM_EVENT, which
is used for reading the hotkey scancodes. On these machines the
scancodes can be read by executing the ACPI NTFY method.

This patch fixes both problems by installing an i8042 filter when
the NTFY method is present to generate notifications and by
detecting which of INFO or HCI_SYSTEM_EVENT is supported for
reading scancodes. If neither method of reading scancodes is
supported, the hotkey input device is not registered.

Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/platform/x86/toshiba_acpi.c |  231 +++++++++++++++++++++++++++++------
 1 files changed, 195 insertions(+), 36 deletions(-)

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index dcdc1f4..1d83884 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -52,6 +52,8 @@
 #include <linux/input/sparse-keymap.h>
 #include <linux/leds.h>
 #include <linux/slab.h>
+#include <linux/workqueue.h>
+#include <linux/i8042.h>
 
 #include <asm/uaccess.h>
 
@@ -61,6 +63,9 @@ MODULE_AUTHOR("John Belmonte");
 MODULE_DESCRIPTION("Toshiba Laptop ACPI Extras Driver");
 MODULE_LICENSE("GPL");
 
+/* Scan code for Fn key on TOS1900 models */
+#define TOS1900_FN_SCAN		0x6e
+
 /* Toshiba ACPI method paths */
 #define METHOD_VIDEO_OUT	"\\_SB_.VALX.DSSX"
 
@@ -95,6 +100,8 @@ MODULE_LICENSE("GPL");
 #define HCI_WIRELESS			0x0056
 
 /* field definitions */
+#define HCI_HOTKEY_DISABLE		0x0b
+#define HCI_HOTKEY_ENABLE		0x09
 #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)
@@ -111,6 +118,7 @@ struct toshiba_acpi_dev {
 	const char *method_hci;
 	struct rfkill *bt_rfk;
 	struct input_dev *hotkey_dev;
+	struct work_struct hotkey_work;
 	struct backlight_device *backlight_dev;
 	struct led_classdev led_dev;
 
@@ -122,10 +130,14 @@ struct toshiba_acpi_dev {
 	int video_supported:1;
 	int fan_supported:1;
 	int system_event_supported:1;
+	int ntfy_supported:1;
+	int info_supported:1;
 
 	struct mutex mutex;
 };
 
+static struct toshiba_acpi_dev *toshiba_acpi;
+
 static const struct acpi_device_id toshiba_device_ids[] = {
 	{"TOS6200", 0},
 	{"TOS6208", 0},
@@ -847,10 +859,78 @@ static const struct backlight_ops toshiba_backlight_data = {
         .update_status  = set_lcd_status,
 };
 
+static bool toshiba_acpi_i8042_filter(unsigned char data, unsigned char str,
+				      struct serio *port)
+{
+	if (str & 0x20)
+		return false;
+
+	if (unlikely(data == 0xe0))
+		return false;
+
+	if ((data & 0x7f) == TOS1900_FN_SCAN) {
+		schedule_work(&toshiba_acpi->hotkey_work);
+		return true;
+	}
+
+	return false;
+}
+
+static void toshiba_acpi_hotkey_work(struct work_struct *work)
+{
+	acpi_handle ec_handle = ec_get_handle();
+	acpi_status status;
+
+	if (!ec_handle)
+		return;
+
+	status = acpi_evaluate_object(ec_handle, "NTFY", NULL, NULL);
+	if (ACPI_FAILURE(status))
+		pr_err("ACPI NTFY method execution failed\n");
+}
+
+/*
+ * Returns hotkey scancode, or < 0 on failure.
+ */
+static int toshiba_acpi_query_hotkey(struct toshiba_acpi_dev *dev)
+{
+	struct acpi_buffer buf;
+	union acpi_object out_obj;
+	acpi_status status;
+
+	buf.pointer = &out_obj;
+	buf.length = sizeof(out_obj);
+
+	status = acpi_evaluate_object(dev->acpi_dev->handle, "INFO",
+				      NULL, &buf);
+	if (ACPI_FAILURE(status) || out_obj.type != ACPI_TYPE_INTEGER) {
+		pr_err("ACPI INFO method execution failed\n");
+		return -EIO;
+	}
+
+	return out_obj.integer.value;
+}
+
+static void toshiba_acpi_report_hotkey(struct toshiba_acpi_dev *dev,
+				       int scancode)
+{
+	if (scancode == 0x100)
+		return;
+
+	/* act on key press; ignore key release */
+	if (scancode & 0x80)
+		return;
+
+	if (!sparse_keymap_report_event(dev->hotkey_dev, scancode, 1, true))
+		pr_info("Unknown key %x\n", scancode);
+}
+
 static int __devinit toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
 {
 	acpi_status status;
+	acpi_handle ec_handle, handle;
 	int error;
+	u32 hci_result;
 
 	dev->hotkey_dev = input_allocate_device();
 	if (!dev->hotkey_dev) {
@@ -866,21 +946,67 @@ static int __devinit toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
 	if (error)
 		goto err_free_dev;
 
+	/*
+	 * For some machines the SCI responsible for providing hotkey
+	 * notification doesn't fire. We can trigger the notification
+	 * whenever the Fn key is pressed using the NTFY method, if
+	 * 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)
+		status = acpi_get_handle(ec_handle, "NTFY", &handle);
+
+	if (ACPI_SUCCESS(status)) {
+		INIT_WORK(&dev->hotkey_work, toshiba_acpi_hotkey_work);
+
+		error = i8042_install_filter(toshiba_acpi_i8042_filter);
+		if (error) {
+			pr_err("Error installing key filter\n");
+			goto err_free_keymap;
+		}
+
+		dev->ntfy_supported = 1;
+	}
+
+	/*
+	 * Determine hotkey query interface. Prefer using the INFO
+	 * method when it is available.
+	 */
+	status = acpi_get_handle(dev->acpi_dev->handle, "INFO", &handle);
+	if (ACPI_SUCCESS(status)) {
+		dev->info_supported = 1;
+	} else {
+		hci_write1(dev, HCI_SYSTEM_EVENT, 1, &hci_result);
+		if (hci_result == HCI_SUCCESS)
+			dev->system_event_supported = 1;
+	}
+
+	if (!dev->info_supported && !dev->system_event_supported) {
+		pr_warn("No hotkey query interface found\n");
+		goto err_remove_filter;
+	}
+
 	status = acpi_evaluate_object(dev->acpi_dev->handle, "ENAB", NULL, NULL);
 	if (ACPI_FAILURE(status)) {
 		pr_info("Unable to enable hotkeys\n");
 		error = -ENODEV;
-		goto err_free_keymap;
+		goto err_remove_filter;
 	}
 
 	error = input_register_device(dev->hotkey_dev);
 	if (error) {
 		pr_info("Unable to register input device\n");
-		goto err_free_keymap;
+		goto err_remove_filter;
 	}
 
+	hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_ENABLE, &hci_result);
 	return 0;
 
+ err_remove_filter:
+	if (dev->ntfy_supported)
+		i8042_remove_filter(toshiba_acpi_i8042_filter);
  err_free_keymap:
 	sparse_keymap_free(dev->hotkey_dev);
  err_free_dev:
@@ -895,6 +1021,11 @@ static int toshiba_acpi_remove(struct acpi_device *acpi_dev, int type)
 
 	remove_toshiba_proc_entries(dev);
 
+	if (dev->ntfy_supported) {
+		i8042_remove_filter(toshiba_acpi_i8042_filter);
+		cancel_work_sync(&dev->hotkey_work);
+	}
+
 	if (dev->hotkey_dev) {
 		input_unregister_device(dev->hotkey_dev);
 		sparse_keymap_free(dev->hotkey_dev);
@@ -911,6 +1042,9 @@ static int toshiba_acpi_remove(struct acpi_device *acpi_dev, int type)
 	if (dev->illumination_supported)
 		led_classdev_unregister(&dev->led_dev);
 
+	if (toshiba_acpi)
+		toshiba_acpi = NULL;
+
 	kfree(dev);
 
 	return 0;
@@ -936,12 +1070,14 @@ static int __devinit toshiba_acpi_add(struct acpi_device *acpi_dev)
 {
 	struct toshiba_acpi_dev *dev;
 	const char *hci_method;
-	u32 hci_result;
 	u32 dummy;
 	bool bt_present;
 	int ret = 0;
 	struct backlight_properties props;
 
+	if (toshiba_acpi)
+		return -EBUSY;
+
 	pr_info("Toshiba Laptop ACPI Extras version %s\n",
 	       TOSHIBA_ACPI_VERSION);
 
@@ -963,11 +1099,6 @@ static int __devinit toshiba_acpi_add(struct acpi_device *acpi_dev)
 
 	mutex_init(&dev->mutex);
 
-	/* enable event fifo */
-	hci_write1(dev, HCI_SYSTEM_EVENT, 1, &hci_result);
-	if (hci_result == HCI_SUCCESS)
-		dev->system_event_supported = 1;
-
 	props.type = BACKLIGHT_PLATFORM;
 	props.max_brightness = HCI_LCD_BRIGHTNESS_LEVELS - 1;
 	dev->backlight_dev = backlight_device_register("toshiba",
@@ -1024,6 +1155,8 @@ static int __devinit toshiba_acpi_add(struct acpi_device *acpi_dev)
 
 	create_toshiba_proc_entries(dev);
 
+	toshiba_acpi = dev;
+
 	return 0;
 
 error:
@@ -1036,40 +1169,64 @@ 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 (!dev->system_event_supported || event != 0x80)
+	if (event != 0x80)
 		return;
 
-	do {
-		hci_read1(dev, HCI_SYSTEM_EVENT, &value, &hci_result);
-		switch (hci_result) {
-		case HCI_SUCCESS:
-			if (value == 0x100)
-				continue;
-			/* act on key press; ignore key release */
-			if (value & 0x80)
-				continue;
-
-			if (!sparse_keymap_report_event(dev->hotkey_dev,
-							value, 1, true)) {
-				pr_info("Unknown key %x\n",
-				       value);
+	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_read1(dev, HCI_SYSTEM_EVENT, &value, &hci_result);
+			switch (hci_result) {
+			case HCI_SUCCESS:
+				toshiba_acpi_report_hotkey(dev, (int)value);
+				break;
+			case HCI_NOT_SUPPORTED:
+				/*
+				 * This is a workaround for an unresolved
+				 * issue on some machines where system events
+				 * sporadically become disabled.
+				 */
+				hci_write1(dev, HCI_SYSTEM_EVENT, 1,
+					   &hci_result);
+				pr_notice("Re-enabled hotkeys\n");
+				/* fall through */
+			default:
+				retries--;
+				break;
 			}
-			break;
-		case HCI_NOT_SUPPORTED:
-			/* This is a workaround for an unresolved issue on
-			 * some machines where system events sporadically
-			 * become disabled. */
-			hci_write1(dev, HCI_SYSTEM_EVENT, 1, &hci_result);
-			pr_notice("Re-enabled hotkeys\n");
-			/* fall through */
-		default:
-			retries--;
-			break;
-		}
-	} while (retries && hci_result != HCI_EMPTY);
+		} while (retries && hci_result != HCI_EMPTY);
+	}
 }
 
+static int toshiba_acpi_suspend(struct acpi_device *acpi_dev,
+				pm_message_t state)
+{
+	struct toshiba_acpi_dev *dev = acpi_driver_data(acpi_dev);
+	u32 result;
+
+	if (dev->hotkey_dev)
+		hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_DISABLE, &result);
+
+	return 0;
+}
+
+static int toshiba_acpi_resume(struct acpi_device *acpi_dev)
+{
+	struct toshiba_acpi_dev *dev = acpi_driver_data(acpi_dev);
+	u32 result;
+
+	if (dev->hotkey_dev)
+		hci_write1(dev, HCI_HOTKEY_EVENT, HCI_HOTKEY_ENABLE, &result);
+
+	return 0;
+}
 
 static struct acpi_driver toshiba_acpi_driver = {
 	.name	= "Toshiba ACPI driver",
@@ -1080,6 +1237,8 @@ static struct acpi_driver toshiba_acpi_driver = {
 		.add		= toshiba_acpi_add,
 		.remove		= toshiba_acpi_remove,
 		.notify		= toshiba_acpi_notify,
+		.suspend	= toshiba_acpi_suspend,
+		.resume		= toshiba_acpi_resume,
 	},
 };
 
-- 
1.7.5.4

Re: [PATCH v2 2/4] toshiba_acpi: Support alternate hotkey interfaces

[Matthew Garrett]

On Tue, Jan 03, 2012 at 01:02:36PM -0600, Seth Forshee wrote:
> There are two types of problems that prevent hotkeys from working
> on many of the machines supported by toshiba_acpi. The first of
> these is the lack of a functioning SCI for hotkey events. For these
> machines it is possible to filter the Fn keypresses from the
> keyboard and generate a notification by executing the ACPI NTFY
> method.
> 
> The second problem is a lack of support for HCI_SYSTEM_EVENT, which
> is used for reading the hotkey scancodes. On these machines the
> scancodes can be read by executing the ACPI NTFY method.
> 
> This patch fixes both problems by installing an i8042 filter when
> the NTFY method is present to generate notifications and by
> detecting which of INFO or HCI_SYSTEM_EVENT is supported for
> reading scancodes. If neither method of reading scancodes is
> supported, the hotkey input device is not registered.
> 
> Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

It's still an awful way to implement things, but it doesn't seem like 
there's any alternative, so sure.

> +		error = i8042_install_filter(toshiba_acpi_i8042_filter);

If this is only needed for TOS1900 machines then I think I'd prefer to 
see it only done on them. Just add a flag to the data field of the IDs 
and check that.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v2 2/4] toshiba_acpi: Support alternate hotkey interfaces

[Seth Forshee]

On Thu, Jan 05, 2012 at 06:25:22PM +0000, Matthew Garrett wrote:
> On Tue, Jan 03, 2012 at 01:02:36PM -0600, Seth Forshee wrote:
> > There are two types of problems that prevent hotkeys from working
> > on many of the machines supported by toshiba_acpi. The first of
> > these is the lack of a functioning SCI for hotkey events. For these
> > machines it is possible to filter the Fn keypresses from the
> > keyboard and generate a notification by executing the ACPI NTFY
> > method.
> > 
> > The second problem is a lack of support for HCI_SYSTEM_EVENT, which
> > is used for reading the hotkey scancodes. On these machines the
> > scancodes can be read by executing the ACPI NTFY method.
> > 
> > This patch fixes both problems by installing an i8042 filter when
> > the NTFY method is present to generate notifications and by
> > detecting which of INFO or HCI_SYSTEM_EVENT is supported for
> > reading scancodes. If neither method of reading scancodes is
> > supported, the hotkey input device is not registered.
> > 
> > Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> 
> It's still an awful way to implement things, but it doesn't seem like 
> there's any alternative, so sure.

I agree. If there's an alternative I couldn't find it, and not for a
lack of trying.

> > +		error = i8042_install_filter(toshiba_acpi_i8042_filter);
> 
> If this is only needed for TOS1900 machines then I think I'd prefer to 
> see it only done on them. Just add a flag to the data field of the IDs 
> and check that.

I checked all the Toshiba DSDTs I have, and every machine with NTFY is a
TOS1900 machine, but only a subset of the TOS1900 machines have NTFY. So
I think we need to still limit it to only machines with NTFY. I can add
an additional check for TOS1900 if that's what you want.

Re: [PATCH v2 2/4] toshiba_acpi: Support alternate hotkey interfaces

[Matthew Garrett]

Oh, if NTFY is only present on TOS1900, that'll be fine.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

[PATCH v2 3/4] toshiba_acpi: Support additional hotkey scancodes

[Seth Forshee]

From: Azael Avalos <coproscefalo@gmail.com>

These scancodes are used by many of the models now supported with
the addition of TOS1900 device support.

Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/platform/x86/toshiba_acpi.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 1d83884..6824bf7 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -150,6 +150,8 @@ static const struct key_entry toshiba_acpi_keymap[] __devinitconst = {
 	{ KE_KEY, 0x101, { KEY_MUTE } },
 	{ KE_KEY, 0x102, { KEY_ZOOMOUT } },
 	{ KE_KEY, 0x103, { KEY_ZOOMIN } },
+	{ KE_KEY, 0x12c, { KEY_KBDILLUMTOGGLE } },
+	{ KE_KEY, 0x139, { KEY_ZOOMRESET } },
 	{ KE_KEY, 0x13b, { KEY_COFFEE } },
 	{ KE_KEY, 0x13c, { KEY_BATTERY } },
 	{ KE_KEY, 0x13d, { KEY_SLEEP } },
@@ -158,7 +160,7 @@ static const struct key_entry toshiba_acpi_keymap[] __devinitconst = {
 	{ KE_KEY, 0x140, { KEY_BRIGHTNESSDOWN } },
 	{ KE_KEY, 0x141, { KEY_BRIGHTNESSUP } },
 	{ KE_KEY, 0x142, { KEY_WLAN } },
-	{ KE_KEY, 0x143, { KEY_PROG1 } },
+	{ KE_KEY, 0x143, { KEY_TOUCHPAD_TOGGLE } },
 	{ KE_KEY, 0x17f, { KEY_FN } },
 	{ KE_KEY, 0xb05, { KEY_PROG2 } },
 	{ KE_KEY, 0xb06, { KEY_WWW } },
@@ -168,6 +170,7 @@ static const struct key_entry toshiba_acpi_keymap[] __devinitconst = {
 	{ KE_KEY, 0xb32, { KEY_NEXTSONG } },
 	{ KE_KEY, 0xb33, { KEY_PLAYPAUSE } },
 	{ KE_KEY, 0xb5a, { KEY_MEDIA } },
+	{ KE_IGNORE, 0x1430, { KEY_RESERVED } },
 	{ KE_END, 0 },
 };
 
-- 
1.7.5.4

[PATCH v2 4/4] toshiba_acpi: Add blacklist for models with hotkey problems

[Seth Forshee]

Several Satellite models have a buggy implementation of the INFO method
that causes ACPI exceptions when executed:

 ACPI Error: Result stack is empty! State=ffff88012d70f800 (20110413/dswstate-98)
 ACPI Exception: AE_AML_NO_RETURN_VALUE, Missing or null operand (20110413/dsutils-646)
 ACPI Exception: AE_AML_NO_RETURN_VALUE, While creating Arg 0 (20110413/dsutils-763)
 ACPI Error: Method parse/execution failed [\_SB_.VALZ.GETE] (Node ffff880131175eb0), AE_AML_NO_RETURN_VALUE (20110413/psparse-536)
 ACPI Error: Method parse/execution failed [\_SB_.VALZ.INFO] (Node ffff880131175ed8), AE_AML_NO_RETURN_VALUE (20110413/psparse-536)
 toshiba_acpi: ACPI INFO method execution failed
 toshiba_acpi: Failed to query hotkey event

To work around this, add a blacklist of models with hotkey problems and
disable the use of hotkeys on these machines.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/platform/x86/toshiba_acpi.c |   39 +++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 6824bf7..fbb8318 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -54,6 +54,7 @@
 #include <linux/slab.h>
 #include <linux/workqueue.h>
 #include <linux/i8042.h>
+#include <linux/dmi.h>
 
 #include <asm/uaccess.h>
 
@@ -174,6 +175,39 @@ static const struct key_entry toshiba_acpi_keymap[] __devinitconst = {
 	{ KE_END, 0 },
 };
 
+/* Machines for which hotkeys should not be enabled */
+static struct dmi_system_id __devinitdata toshiba_hotkey_blacklist[] = {
+	{
+		.ident = "Toshiba Satellite C670-10V",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "SATELLITE C670-10V"),
+		},
+	},
+	{
+		.ident = "Toshiba Satellite C670-12E",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "SATELLITE C670-12E"),
+		},
+	},
+	{
+		.ident = "Toshiba Satellite U500",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "SATELLITE U500"),
+		},
+	},
+	{
+		.ident = "Toshiba Satellite Pro L770-116",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "SATELLITE PRO L770-116"),
+		},
+	},
+	{}
+};
+
 /* utility
  */
 
@@ -935,6 +969,11 @@ static int __devinit toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
 	int error;
 	u32 hci_result;
 
+	if (dmi_check_system(toshiba_hotkey_blacklist)) {
+		pr_info("Model has known hotkey problems, hotkeys will be disabled\n");
+		return 0;
+	}
+
 	dev->hotkey_dev = input_allocate_device();
 	if (!dev->hotkey_dev) {
 		pr_info("Unable to register input device\n");
-- 
1.7.5.4

Re: [PATCH v2 4/4] toshiba_acpi: Add blacklist for models with hotkey problems

[Matthew Garrett]

On Tue, Jan 03, 2012 at 01:02:38PM -0600, Seth Forshee wrote:
> Several Satellite models have a buggy implementation of the INFO method
> that causes ACPI exceptions when executed:
> 
>  ACPI Error: Result stack is empty! State=ffff88012d70f800 (20110413/dswstate-98)
>  ACPI Exception: AE_AML_NO_RETURN_VALUE, Missing or null operand (20110413/dsutils-646)
>  ACPI Exception: AE_AML_NO_RETURN_VALUE, While creating Arg 0 (20110413/dsutils-763)
>  ACPI Error: Method parse/execution failed [\_SB_.VALZ.GETE] (Node ffff880131175eb0), AE_AML_NO_RETURN_VALUE (20110413/psparse-536)
>  ACPI Error: Method parse/execution failed [\_SB_.VALZ.INFO] (Node ffff880131175ed8), AE_AML_NO_RETURN_VALUE (20110413/psparse-536)
>  toshiba_acpi: ACPI INFO method execution failed
>  toshiba_acpi: Failed to query hotkey event

Ugh, in several ways. The hotkeys on these machines are presumably 
supposed to work - do we have any idea what we should be doing?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v2 4/4] toshiba_acpi: Add blacklist for models with hotkey problems

[Seth Forshee]

On Thu, Jan 05, 2012 at 06:26:27PM +0000, Matthew Garrett wrote:
> On Tue, Jan 03, 2012 at 01:02:38PM -0600, Seth Forshee wrote:
> > Several Satellite models have a buggy implementation of the INFO method
> > that causes ACPI exceptions when executed:
> > 
> >  ACPI Error: Result stack is empty! State=ffff88012d70f800 (20110413/dswstate-98)
> >  ACPI Exception: AE_AML_NO_RETURN_VALUE, Missing or null operand (20110413/dsutils-646)
> >  ACPI Exception: AE_AML_NO_RETURN_VALUE, While creating Arg 0 (20110413/dsutils-763)
> >  ACPI Error: Method parse/execution failed [\_SB_.VALZ.GETE] (Node ffff880131175eb0), AE_AML_NO_RETURN_VALUE (20110413/psparse-536)
> >  ACPI Error: Method parse/execution failed [\_SB_.VALZ.INFO] (Node ffff880131175ed8), AE_AML_NO_RETURN_VALUE (20110413/psparse-536)
> >  toshiba_acpi: ACPI INFO method execution failed
> >  toshiba_acpi: Failed to query hotkey event
> 
> Ugh, in several ways. The hotkeys on these machines are presumably 
> supposed to work - do we have any idea what we should be doing?

Here's a run-down of why this happens. First, the relevant sections of
the DSDT:

    Name (EVCD, Package (0x64) {})
    Name (EVCT, 0x00)

    Method (PUTE, 1, Serialized)
    {
        <snip>

        If (LLess (EVCT, 0x64))
        {
            Store (Arg0, Index (EVCD, EVCT))
            Increment (EVCT)
        }

	<snip>

        Notify (VALZ, 0x80)
        Sleep (0x64)
        Return (0x00)
    }

    Method (GETE, 0, Serialized)
    {
        <snip>

        If (LEqual (EVCT, 0x00))
        {
            Release (MUEV)
            Return (0x00)
        }

        Store (DerefOf (Index (EVCD, 0x00)), Local0)
        Store (0x00, Local1)
        While (LLess (Local1, Subtract (0x64, 0x01)))
        {
            Store (DerefOf (Index (EVCD, Add (Local1, 0x01))), Index (
                EVCD, Local1))
            Increment (Local1)
        }

        Decrement (EVCT)
        Release (MUEV)
        If (LNotEqual (EVCT, 0x00))
        {
            And (HKEV, 0x02, Local0)
            If (LEqual (Local0, 0x02))
            {
                Return (0x00)
            }

            Notify (VALZ, 0x80)
            Sleep (0x64)
        }

        Return (Local0)
    }

    Method (INFO, 0, Serialized)
    {
        Store (GETE (), Local0)
        Return (Local0)
    }

So EVCD is a queue of events, and EVCT is the number of events in the
queue. NTFY calls PUTE, which puts an event in the queue. INFO calls
GETE to read an event from the queue. Reading an event consists of
copying out the first object out of EVCD, then copying each subsequent
object to the previous element in EVCD. Inefficient, but it should work.

Except for that fact that EVCD will initially contain uninitialized
objects. So until/unless PUTE writes every element in EVCD that copy
loop is going to dereference unintialized objects, which causes the
exceptions.

GETE/PUTE are the only methods which write to EVCD, so there's not some
initialization method or something like that which we're forgetting to
call. We could do something like call PUTE 64 times during
initialization, but that's relying on a BIOS implementation detail which
just seems like a bad idea.

My only guess is that Windows is more permissive about this sort of
thing than ACPICA is, but by my reading of the spec throwing a fatal
error is what's supposed to happen.

Also a side note of interest: The machines that suffer from this all
also have a WMI interface with code to support hotkeys, and that code
shouldn't suffer from this problem. Unfortunately these models are the
only ones I've seen with this WMI interface, and the code to generate
the hotkey events isn't executed if the OS reports itself as Vista or
newer.

Re: [PATCH v2 4/4] toshiba_acpi: Add blacklist for models with hotkey problems

[Matthew Garrett]

On Thu, Jan 05, 2012 at 01:32:28PM -0600, Seth Forshee wrote:

> Also a side note of interest: The machines that suffer from this all
> also have a WMI interface with code to support hotkeys, and that code
> shouldn't suffer from this problem. Unfortunately these models are the
> only ones I've seen with this WMI interface, and the code to generate
> the hotkey events isn't executed if the OS reports itself as Vista or
> newer.

In that case, can you check for the presence of the WMI interface and 
then refuse to bind? That's seem far simpler than adding an unknown 
number of systems to the blacklist.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v2 4/4] toshiba_acpi: Add blacklist for models with hotkey problems

[Seth Forshee]

On Thu, Jan 05, 2012 at 07:34:31PM +0000, Matthew Garrett wrote:
> On Thu, Jan 05, 2012 at 01:32:28PM -0600, Seth Forshee wrote:
> 
> > Also a side note of interest: The machines that suffer from this all
> > also have a WMI interface with code to support hotkeys, and that code
> > shouldn't suffer from this problem. Unfortunately these models are the
> > only ones I've seen with this WMI interface, and the code to generate
> > the hotkey events isn't executed if the OS reports itself as Vista or
> > newer.
> 
> In that case, can you check for the presence of the WMI interface and 
> then refuse to bind? That's seem far simpler than adding an unknown 
> number of systems to the blacklist.

Hmm, I suppose I could look for the specific WMI event guid, although
the relationship between the two seems tenuous at best. It would make
sense to me if we were refusing to bind because there was a better
interface available, but Toshiba is obviously treating hotkeys on the
WMI interface as legacy since it's disabled in the BIOS for Vista or
later. INFO seems to be the preferred interface.

Re: [PATCH v2 4/4] toshiba_acpi: Add blacklist for models with hotkey problems

[Matthew Garrett]

On Thu, Jan 05, 2012 at 02:04:32PM -0600, Seth Forshee wrote:
> On Thu, Jan 05, 2012 at 07:34:31PM +0000, Matthew Garrett wrote:
> > In that case, can you check for the presence of the WMI interface and 
> > then refuse to bind? That's seem far simpler than adding an unknown 
> > number of systems to the blacklist.
> 
> Hmm, I suppose I could look for the specific WMI event guid, although
> the relationship between the two seems tenuous at best. It would make
> sense to me if we were refusing to bind because there was a better
> interface available, but Toshiba is obviously treating hotkeys on the
> WMI interface as legacy since it's disabled in the BIOS for Vista or
> later. INFO seems to be the preferred interface.

...but doesn't work?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v2 4/4] toshiba_acpi: Add blacklist for models with hotkey problems

[Seth Forshee]

On Thu, Jan 05, 2012 at 08:09:26PM +0000, Matthew Garrett wrote:
> On Thu, Jan 05, 2012 at 02:04:32PM -0600, Seth Forshee wrote:
> > On Thu, Jan 05, 2012 at 07:34:31PM +0000, Matthew Garrett wrote:
> > > In that case, can you check for the presence of the WMI interface and 
> > > then refuse to bind? That's seem far simpler than adding an unknown 
> > > number of systems to the blacklist.
> > 
> > Hmm, I suppose I could look for the specific WMI event guid, although
> > the relationship between the two seems tenuous at best. It would make
> > sense to me if we were refusing to bind because there was a better
> > interface available, but Toshiba is obviously treating hotkeys on the
> > WMI interface as legacy since it's disabled in the BIOS for Vista or
> > later. INFO seems to be the preferred interface.
> 
> ...but doesn't work?

Yes. But that's not because of the WMI interface. They both just belong
to the same BIOS implementation. There's no reason why a future machine
couldn't have this WMI interface along with a working INFO
implementation.

For now that's theoretical though, so I'll change it to use the guid
instead of the blacklist. If such a machine appears in the future we can
deal with it then.

RE: [PATCH v2 4/4] toshiba_acpi: Add blacklist for models with hotkey problems

[Moore, Robert]

> My only guess is that Windows is more permissive about this sort of
> thing than ACPICA is, but by my reading of the spec throwing a fatal
> error is what's supposed to happen.

If it is true that Windows will execute this code without error, then ACPICA should be changed to match the Windows behavior. ACPICA is now a "Windows compatible" implementation of ACPI, but that of course means we are attempting to be compatible with a black box ACPI implementation. On the other hand, it may be the case that Windows never executes this code, so the error is not seen on Windows.

We would like to investigate this further. Please send or point me to the acpidump for this machine.

Thanks,
Bob

> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Seth Forshee
> Sent: Thursday, January 05, 2012 11:32 AM
> To: Matthew Garrett
> Cc: Len Brown; Azael Avalos; Thomas Renninger; platform-driver-
> x86@vger.kernel.org; linux-acpi@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v2 4/4] toshiba_acpi: Add blacklist for models with
> hotkey problems
> 
> On Thu, Jan 05, 2012 at 06:26:27PM +0000, Matthew Garrett wrote:
> > On Tue, Jan 03, 2012 at 01:02:38PM -0600, Seth Forshee wrote:
> > > Several Satellite models have a buggy implementation of the INFO
> method
> > > that causes ACPI exceptions when executed:
> > >
> > >  ACPI Error: Result stack is empty! State=ffff88012d70f800
> (20110413/dswstate-98)
> > >  ACPI Exception: AE_AML_NO_RETURN_VALUE, Missing or null operand
> (20110413/dsutils-646)
> > >  ACPI Exception: AE_AML_NO_RETURN_VALUE, While creating Arg 0
> (20110413/dsutils-763)
> > >  ACPI Error: Method parse/execution failed [\_SB_.VALZ.GETE] (Node
> ffff880131175eb0), AE_AML_NO_RETURN_VALUE (20110413/psparse-536)
> > >  ACPI Error: Method parse/execution failed [\_SB_.VALZ.INFO] (Node
> ffff880131175ed8), AE_AML_NO_RETURN_VALUE (20110413/psparse-536)
> > >  toshiba_acpi: ACPI INFO method execution failed
> > >  toshiba_acpi: Failed to query hotkey event
> >
> > Ugh, in several ways. The hotkeys on these machines are presumably
> > supposed to work - do we have any idea what we should be doing?
> 
> Here's a run-down of why this happens. First, the relevant sections of
> the DSDT:
> 
>     Name (EVCD, Package (0x64) {})
>     Name (EVCT, 0x00)
> 
>     Method (PUTE, 1, Serialized)
>     {
>         <snip>
> 
>         If (LLess (EVCT, 0x64))
>         {
>             Store (Arg0, Index (EVCD, EVCT))
>             Increment (EVCT)
>         }
> 
> 	<snip>
> 
>         Notify (VALZ, 0x80)
>         Sleep (0x64)
>         Return (0x00)
>     }
> 
>     Method (GETE, 0, Serialized)
>     {
>         <snip>
> 
>         If (LEqual (EVCT, 0x00))
>         {
>             Release (MUEV)
>             Return (0x00)
>         }
> 
>         Store (DerefOf (Index (EVCD, 0x00)), Local0)
>         Store (0x00, Local1)
>         While (LLess (Local1, Subtract (0x64, 0x01)))
>         {
>             Store (DerefOf (Index (EVCD, Add (Local1, 0x01))), Index (
>                 EVCD, Local1))
>             Increment (Local1)
>         }
> 
>         Decrement (EVCT)
>         Release (MUEV)
>         If (LNotEqual (EVCT, 0x00))
>         {
>             And (HKEV, 0x02, Local0)
>             If (LEqual (Local0, 0x02))
>             {
>                 Return (0x00)
>             }
> 
>             Notify (VALZ, 0x80)
>             Sleep (0x64)
>         }
> 
>         Return (Local0)
>     }
> 
>     Method (INFO, 0, Serialized)
>     {
>         Store (GETE (), Local0)
>         Return (Local0)
>     }
> 
> So EVCD is a queue of events, and EVCT is the number of events in the
> queue. NTFY calls PUTE, which puts an event in the queue. INFO calls
> GETE to read an event from the queue. Reading an event consists of
> copying out the first object out of EVCD, then copying each subsequent
> object to the previous element in EVCD. Inefficient, but it should
> work.
> 
> Except for that fact that EVCD will initially contain uninitialized
> objects. So until/unless PUTE writes every element in EVCD that copy
> loop is going to dereference unintialized objects, which causes the
> exceptions.
> 
> GETE/PUTE are the only methods which write to EVCD, so there's not some
> initialization method or something like that which we're forgetting to
> call. We could do something like call PUTE 64 times during
> initialization, but that's relying on a BIOS implementation detail
> which
> just seems like a bad idea.
> 
> My only guess is that Windows is more permissive about this sort of
> thing than ACPICA is, but by my reading of the spec throwing a fatal
> error is what's supposed to happen.
> 
> Also a side note of interest: The machines that suffer from this all
> also have a WMI interface with code to support hotkeys, and that code
> shouldn't suffer from this problem. Unfortunately these models are the
> only ones I've seen with this WMI interface, and the code to generate
> the hotkey events isn't executed if the OS reports itself as Vista or
> newer.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2 4/4] toshiba_acpi: Add blacklist for models with hotkey problems

[Moore, Robert]

> My only guess is that Windows is more permissive about this sort of
> thing than ACPICA is, but by my reading of the spec throwing a fatal
> error is what's supposed to happen.

If it is true that Windows will execute this code without error, then ACPICA should be changed to match the Windows behavior. ACPICA is now a "Windows compatible" implementation of ACPI, but that of course means we are attempting to be compatible with a black box ACPI implementation. On the other hand, it may be the case that Windows never executes this code, so the error is not seen on Windows.

We would like to investigate this further. Please send or point me to the acpidump for this machine.

Thanks,
Bob

> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Seth Forshee
> Sent: Thursday, January 05, 2012 11:32 AM
> To: Matthew Garrett
> Cc: Len Brown; Azael Avalos; Thomas Renninger; platform-driver-
> x86@vger.kernel.org; linux-acpi@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v2 4/4] toshiba_acpi: Add blacklist for models with
> hotkey problems
> 
> On Thu, Jan 05, 2012 at 06:26:27PM +0000, Matthew Garrett wrote:
> > On Tue, Jan 03, 2012 at 01:02:38PM -0600, Seth Forshee wrote:
> > > Several Satellite models have a buggy implementation of the INFO
> method
> > > that causes ACPI exceptions when executed:
> > >
> > >  ACPI Error: Result stack is empty! State=ffff88012d70f800
> (20110413/dswstate-98)
> > >  ACPI Exception: AE_AML_NO_RETURN_VALUE, Missing or null operand
> (20110413/dsutils-646)
> > >  ACPI Exception: AE_AML_NO_RETURN_VALUE, While creating Arg 0
> (20110413/dsutils-763)
> > >  ACPI Error: Method parse/execution failed [\_SB_.VALZ.GETE] (Node
> ffff880131175eb0), AE_AML_NO_RETURN_VALUE (20110413/psparse-536)
> > >  ACPI Error: Method parse/execution failed [\_SB_.VALZ.INFO] (Node
> ffff880131175ed8), AE_AML_NO_RETURN_VALUE (20110413/psparse-536)
> > >  toshiba_acpi: ACPI INFO method execution failed
> > >  toshiba_acpi: Failed to query hotkey event
> >
> > Ugh, in several ways. The hotkeys on these machines are presumably
> > supposed to work - do we have any idea what we should be doing?
> 
> Here's a run-down of why this happens. First, the relevant sections of
> the DSDT:
> 
>     Name (EVCD, Package (0x64) {})
>     Name (EVCT, 0x00)
> 
>     Method (PUTE, 1, Serialized)
>     {
>         <snip>
> 
>         If (LLess (EVCT, 0x64))
>         {
>             Store (Arg0, Index (EVCD, EVCT))
>             Increment (EVCT)
>         }
> 
> 	<snip>
> 
>         Notify (VALZ, 0x80)
>         Sleep (0x64)
>         Return (0x00)
>     }
> 
>     Method (GETE, 0, Serialized)
>     {
>         <snip>
> 
>         If (LEqual (EVCT, 0x00))
>         {
>             Release (MUEV)
>             Return (0x00)
>         }
> 
>         Store (DerefOf (Index (EVCD, 0x00)), Local0)
>         Store (0x00, Local1)
>         While (LLess (Local1, Subtract (0x64, 0x01)))
>         {
>             Store (DerefOf (Index (EVCD, Add (Local1, 0x01))), Index (
>                 EVCD, Local1))
>             Increment (Local1)
>         }
> 
>         Decrement (EVCT)
>         Release (MUEV)
>         If (LNotEqual (EVCT, 0x00))
>         {
>             And (HKEV, 0x02, Local0)
>             If (LEqual (Local0, 0x02))
>             {
>                 Return (0x00)
>             }
> 
>             Notify (VALZ, 0x80)
>             Sleep (0x64)
>         }
> 
>         Return (Local0)
>     }
> 
>     Method (INFO, 0, Serialized)
>     {
>         Store (GETE (), Local0)
>         Return (Local0)
>     }
> 
> So EVCD is a queue of events, and EVCT is the number of events in the
> queue. NTFY calls PUTE, which puts an event in the queue. INFO calls
> GETE to read an event from the queue. Reading an event consists of
> copying out the first object out of EVCD, then copying each subsequent
> object to the previous element in EVCD. Inefficient, but it should
> work.
> 
> Except for that fact that EVCD will initially contain uninitialized
> objects. So until/unless PUTE writes every element in EVCD that copy
> loop is going to dereference unintialized objects, which causes the
> exceptions.
> 
> GETE/PUTE are the only methods which write to EVCD, so there's not some
> initialization method or something like that which we're forgetting to
> call. We could do something like call PUTE 64 times during
> initialization, but that's relying on a BIOS implementation detail
> which
> just seems like a bad idea.
> 
> My only guess is that Windows is more permissive about this sort of
> thing than ACPICA is, but by my reading of the spec throwing a fatal
> error is what's supposed to happen.
> 
> Also a side note of interest: The machines that suffer from this all
> also have a WMI interface with code to support hotkeys, and that code
> shouldn't suffer from this problem. Unfortunately these models are the
> only ones I've seen with this WMI interface, and the code to generate
> the hotkey events isn't executed if the OS reports itself as Vista or
> newer.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/4] toshiba_acpi: Add blacklist for models with hotkey problems

[Seth Forshee]

On Thu, Jan 05, 2012 at 08:46:43PM +0000, Moore, Robert wrote:
> > My only guess is that Windows is more permissive about this sort of
> > thing than ACPICA is, but by my reading of the spec throwing a fatal
> > error is what's supposed to happen.
> 
> If it is true that Windows will execute this code without error, then ACPICA should be changed to match the Windows behavior. ACPICA is now a "Windows compatible" implementation of ACPI, but that of course means we are attempting to be compatible with a black box ACPI implementation. On the other hand, it may be the case that Windows never executes this code, so the error is not seen on Windows.
> 
> We would like to investigate this further. Please send or point me to the acpidump for this machine.

I posted the ACPI tables at the following link. Let me know if there's
anything I can do to help.

http://people.canonical.com/~sforshee/toshiba-acpi/nb505-acpi-tables.txt

Thanks,
Seth

Re: [PATCH v2 4/4] toshiba_acpi: Add blacklist for models with hotkey problems

[Seth Forshee]

On Thu, Jan 05, 2012 at 08:46:43PM +0000, Moore, Robert wrote:
> > My only guess is that Windows is more permissive about this sort of
> > thing than ACPICA is, but by my reading of the spec throwing a fatal
> > error is what's supposed to happen.
> 
> If it is true that Windows will execute this code without error, then ACPICA should be changed to match the Windows behavior. ACPICA is now a "Windows compatible" implementation of ACPI, but that of course means we are attempting to be compatible with a black box ACPI implementation. On the other hand, it may be the case that Windows never executes this code, so the error is not seen on Windows.
> 
> We would like to investigate this further. Please send or point me to the acpidump for this machine.

I posted the ACPI tables at the following link. Let me know if there's
anything I can do to help.

http://people.canonical.com/~sforshee/toshiba-acpi/nb505-acpi-tables.txt

Thanks,
Seth