[PATCH 0/4] toshiba_acpi: Expanded hotkey support

[PATCH 0/4] toshiba_acpi: Expanded hotkey support

[Seth Forshee]

Hi Matthew,

The following patches expand toshiba_acpi's hotkey support to include
many additional models. 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 the Satellite C670-10V due to a buggy
   implementation of the INFO method on this model

More details can be found in the individual patch descriptions.

Cc-ing the ACPI folks on the full series to provide context regarding
how toshiba_acpi will use the new EC interface.

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 devices with hotkey problems

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

[PATCH 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 1/4] ACPI: EC: Add ec_get_handle()

[Thomas Renninger]

On Thursday 15 December 2011 19:06:08 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.
I first liked the idea of making the ec device more global for others
to access.
But thinkpad_acpi is doing the same already in another way.

I think best is to move the thinkpad implementation of getting
ACPI handles based on HIDs to osl.c and make it global.
I'll send patches.
Please review them carefully, they are only compile tested.

      Thomas

> 
> 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
> 
> --
> 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 1/4] ACPI: EC: Add ec_get_handle()

[Matthew Garrett]

On Fri, Dec 16, 2011 at 01:22:35AM +0100, Thomas Renninger wrote:

> I think best is to move the thinkpad implementation of getting
> ACPI handles based on HIDs to osl.c and make it global.
> I'll send patches.
> Please review them carefully, they are only compile tested.

The ec driver is already finding the hardware on the basis of the HID - 
is there any reason to do this twice rather than just exporting the 
information the ec driver already has?

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

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

[Thomas Renninger]

On Friday 16 December 2011 01:33:33 Matthew Garrett wrote:
> On Fri, Dec 16, 2011 at 01:22:35AM +0100, Thomas Renninger wrote:
> 
> > I think best is to move the thinkpad implementation of getting
> > ACPI handles based on HIDs to osl.c and make it global.
> > I'll send patches.
> > Please review them carefully, they are only compile tested.
> 
> The ec driver is already finding the hardware on the basis of the HID - 
> is there any reason to do this twice rather than just exporting the 
> information the ec driver already has?
Exporting the EC handle is static, you do not want to export all
kind of ACPI devices as soon as other drivers need them.

The question is how long it takes to do:
acpi_ns_walk_namespace(ACPI_TYPE_DEVICE, ...
in worst case (not found).

Ok, the EC may be somewhat special because it's the most likely
candidate to get shared (or say needed by other drivers)
and its driver has to be compiled in.
.
thinkpad_acpi should at least also make use of Seth's approach and get the
ec_handle quickly/directly.

And my patches should still be added, because others could use
them as well. Beside thinkpad_acpi grabbing the video handle, there
is another re-implementation of my acpi_handle_locate (eeepc_wmi.c):
static acpi_status eeepc_wmi_parse_device(acpi_handle handle, u32 level,
                                                 void *context, void **retval)
{
        pr_warn("Found legacy ATKD device (%s)\n", EEEPC_ACPI_HID);
        *(bool *)context = true;
        return AE_CTRL_TERMINATE;
}

static int eeepc_wmi_check_atkd(void)
{
        acpi_status status;
        bool found = false;

        status = acpi_get_devices(EEEPC_ACPI_HID, eeepc_wmi_parse_device,
                                  &found, NULL);

        if (ACPI_FAILURE(status) || !found)
                return 0;
        return -1;                                                                                                                                                    
}


If you agree I can send something tomorrow.


    Thomas

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

[Thomas Renninger]

On Friday, December 16, 2011 01:33:33 AM Matthew Garrett wrote:
> On Fri, Dec 16, 2011 at 01:22:35AM +0100, Thomas Renninger wrote:
> 
> > I think best is to move the thinkpad implementation of getting
> > ACPI handles based on HIDs to osl.c and make it global.
> > I'll send patches.
> > Please review them carefully, they are only compile tested.
> 
> The ec driver is already finding the hardware on the basis of the HID - 
> is there any reason to do this twice rather than just exporting the 
> information the ec driver already has?

I don't object to export ec_handle, but thinkpad_acpi.c should get
adjusted as well.

I had a closer look and can come up with a replacement for acpi_get_devices
using bus.c matching function bus_find_device() avoiding the namespace
walk.

What do you think about this one?
I found these occurences of acpi_get_devices, which I would
adjust in follow up patches if it's worth it:
drivers/char/agp/hp-agp.c:      acpi_get_devices("HWP0003", zx1_gart_probe, "HWP0003", NULL);
drivers/char/agp/hp-agp.c:      acpi_get_devices("HWP0007", zx1_gart_probe, "HWP0007", NULL);
drivers/acpi/processor_core.c:  acpi_get_devices("ACPI0007", early_init_pdc, NULL, NULL);
drivers/platform/x86/eeepc-wmi.c:       status = acpi_get_devices(EEEPC_ACPI_HID, eeepc_wmi_parse_device,
drivers/platform/x86/thinkpad_acpi.c:   status = acpi_get_devices(hid, tpacpi_acpi_handle_locate_callback,


Argh, these are called before ACPI objects are initialized
(or parsed at all) and won't work:
drivers/acpi/ec.c:      status = acpi_get_devices(ec_device_ids[0].id, ec_parse_device,
arch/ia64/kernel/acpi.c:        acpi_get_devices(NULL, acpi_map_iosapic, NULL, NULL);
drivers/pnp/pnpacpi/core.c:     acpi_get_devices(NULL, pnpacpi_add_device_handler, NULL, NULL);
arch/x86/pci/mmconfig-shared.c: acpi_get_devices("PNP0C01", find_mboard_resource, &mcfg_res, NULL);
arch/x86/pci/mmconfig-shared.c:         acpi_get_devices("PNP0C02", find_mboard_resource, &mcfg_res,
arch/ia64/sn/kernel/io_acpi_init.c:     acpi_get_devices("SGIHUB", sn_acpi_hubdev_init, NULL, NULL);
arch/ia64/sn/kernel/io_acpi_init.c:     acpi_get_devices("SGITIO", sn_acpi_hubdev_init, NULL, NULL);

Looks like it's not worth it.
I paste my patch anyway, if there is any use-case,
it's already tested and works...:

---
ACPI: Provide an acpi device search on acpi bus registered devices

These should replace unnecessary acpi_get_devices invokations
which ends in a namespace walk.

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: Matthew Garrett <mjg@redhat.com>
CC: Len Brown <lenb@kernel.org>
CC: linux-acpi@vger.kernel.org
CC: seth.forshee@canonical.com
CC: Azael Avalos <coproscefalo@gmail.com>
CC: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

---
 drivers/acpi/scan.c  |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/acpi.h |    4 +++
 2 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 8ab80ba..efd97ea 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1606,3 +1606,69 @@ int __init acpi_scan_init(void)
 
 	return result;
 }
+
+static int acpi_bus_find_cb(struct device *dev, void *data)
+{
+	struct acpi_device_id *ids = data;
+
+	if (acpi_match_device_ids(to_acpi_device(dev), ids) == 0)
+		return 1;
+	return 0;
+}
+
+/**
+ * acpi_find_devices - find acpi devices of a given HID and execute callback
+ * @ids: The Hardware IDs for matching devices
+ * @match: A callback function which is executed for matching HIDs
+ *
+ * This is the counterpart of acpi_get_devices from ACPICA sources which
+ * should not get executed because it performs a namespace walk.
+ * This function instead iterates only over all ACPI devices found by initially
+ * parsing the ACPI namespace (and thus got added to the ACPI bus).
+ * One difference to acpi_get_devices is that it only finds active devices
+ * which were successfully added to the acpi bus and show up in
+ * /sys/devices/LNXSYSTM:00 and for which an ACPI driver's .add function would
+ * get called.
+ *
+ * ACPI devices with a matching HID or CID are recognized.
+ *
+ * If match returns zero, no further, possibly matching devices are processed.
+ */
+int acpi_find_devices(struct acpi_device_id *ids, void *data,
+		      int (*match) (struct acpi_device *dev, void *data))
+{
+	struct device *hit_dev = NULL;
+
+	while(1) {
+		hit_dev = bus_find_device(&acpi_bus_type, hit_dev, ids,
+					  acpi_bus_find_cb);
+		if (!hit_dev)
+			break;
+		if (!match(to_acpi_device(hit_dev), data))
+			break;
+	}
+	return 0;		
+}
+EXPORT_SYMBOL_GPL(acpi_find_devices);
+
+/**
+ * acpi_find_device - find an acpi device of a given HID
+ * @ids: The Hardware IDs for matching devices
+ *
+ * Same as acpi_find_devices, but the search will stop on the first found
+ * device and its structure is returned.
+ *
+ * Use this one if you are sure that only 1 ACPI device of a given HID can
+ * exist in ACPI namespace.
+ *
+ * Returns NULL if no device is found.
+ */
+struct acpi_device *acpi_find_device(struct acpi_device_id *ids)
+{
+	struct device *dev = bus_find_device(&acpi_bus_type, NULL, ids,
+					     acpi_bus_find_cb);
+	if (dev)
+		return to_acpi_device(dev);
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(acpi_find_device);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 6001b4da..2f9e09e 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -146,6 +146,10 @@ struct acpi_pci_driver {
 int acpi_pci_register_driver(struct acpi_pci_driver *driver);
 void acpi_pci_unregister_driver(struct acpi_pci_driver *driver);
 
+extern int acpi_find_devices(struct acpi_device_id *ids, void *data,
+			     int (*match) (struct acpi_device *dev, void *data));
+extern struct acpi_device *acpi_find_device(struct acpi_device_id *ids);
+
 extern int ec_read(u8 addr, u8 *val);
 extern int ec_write(u8 addr, u8 val);
 extern int ec_transaction(u8 command,

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

[Corentin Chary]

On Fri, Dec 16, 2011 at 2:19 PM, Thomas Renninger <trenn@suse.de> wrote:
> On Friday, December 16, 2011 01:33:33 AM Matthew Garrett wrote:
>> On Fri, Dec 16, 2011 at 01:22:35AM +0100, Thomas Renninger wrote:
>>
>> > I think best is to move the thinkpad implementation of getting
>> > ACPI handles based on HIDs to osl.c and make it global.
>> > I'll send patches.
>> > Please review them carefully, they are only compile tested.
>>
>> The ec driver is already finding the hardware on the basis of the HID -
>> is there any reason to do this twice rather than just exporting the
>> information the ec driver already has?
>
> I don't object to export ec_handle, but thinkpad_acpi.c should get
> adjusted as well.
>
> I had a closer look and can come up with a replacement for acpi_get_devices
> using bus.c matching function bus_find_device() avoiding the namespace
> walk.
>
> What do you think about this one?
> I found these occurences of acpi_get_devices, which I would
> adjust in follow up patches if it's worth it:
> drivers/char/agp/hp-agp.c:      acpi_get_devices("HWP0003", zx1_gart_probe, "HWP0003", NULL);
> drivers/char/agp/hp-agp.c:      acpi_get_devices("HWP0007", zx1_gart_probe, "HWP0007", NULL);
> drivers/acpi/processor_core.c:  acpi_get_devices("ACPI0007", early_init_pdc, NULL, NULL);
> drivers/platform/x86/eeepc-wmi.c:       status = acpi_get_devices(EEEPC_ACPI_HID, eeepc_wmi_parse_device,

I like the idea, that would remove some code from eeepc-wmi (and others) :).

> drivers/platform/x86/thinkpad_acpi.c:   status = acpi_get_devices(hid, tpacpi_acpi_handle_locate_callback,
>
>
> Argh, these are called before ACPI objects are initialized
> (or parsed at all) and won't work:
> drivers/acpi/ec.c:      status = acpi_get_devices(ec_device_ids[0].id, ec_parse_device,
> arch/ia64/kernel/acpi.c:        acpi_get_devices(NULL, acpi_map_iosapic, NULL, NULL);
> drivers/pnp/pnpacpi/core.c:     acpi_get_devices(NULL, pnpacpi_add_device_handler, NULL, NULL);
> arch/x86/pci/mmconfig-shared.c: acpi_get_devices("PNP0C01", find_mboard_resource, &mcfg_res, NULL);
> arch/x86/pci/mmconfig-shared.c:         acpi_get_devices("PNP0C02", find_mboard_resource, &mcfg_res,
> arch/ia64/sn/kernel/io_acpi_init.c:     acpi_get_devices("SGIHUB", sn_acpi_hubdev_init, NULL, NULL);
> arch/ia64/sn/kernel/io_acpi_init.c:     acpi_get_devices("SGITIO", sn_acpi_hubdev_init, NULL, NULL);
>
> Looks like it's not worth it.
> I paste my patch anyway, if there is any use-case,
> it's already tested and works...:
>
> ---
> ACPI: Provide an acpi device search on acpi bus registered devices
>
> These should replace unnecessary acpi_get_devices invokations
> which ends in a namespace walk.
>
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> CC: Matthew Garrett <mjg@redhat.com>
> CC: Len Brown <lenb@kernel.org>
> CC: linux-acpi@vger.kernel.org
> CC: seth.forshee@canonical.com
> CC: Azael Avalos <coproscefalo@gmail.com>
> CC: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
>
> ---
>  drivers/acpi/scan.c  |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/acpi.h |    4 +++
>  2 files changed, 70 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 8ab80ba..efd97ea 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1606,3 +1606,69 @@ int __init acpi_scan_init(void)
>
>        return result;
>  }
> +
> +static int acpi_bus_find_cb(struct device *dev, void *data)
> +{
> +       struct acpi_device_id *ids = data;
> +
> +       if (acpi_match_device_ids(to_acpi_device(dev), ids) == 0)
> +               return 1;
> +       return 0;
> +}
> +
> +/**
> + * acpi_find_devices - find acpi devices of a given HID and execute callback
> + * @ids: The Hardware IDs for matching devices
> + * @match: A callback function which is executed for matching HIDs
> + *
> + * This is the counterpart of acpi_get_devices from ACPICA sources which
> + * should not get executed because it performs a namespace walk.
> + * This function instead iterates only over all ACPI devices found by initially
> + * parsing the ACPI namespace (and thus got added to the ACPI bus).
> + * One difference to acpi_get_devices is that it only finds active devices
> + * which were successfully added to the acpi bus and show up in
> + * /sys/devices/LNXSYSTM:00 and for which an ACPI driver's .add function would
> + * get called.
> + *
> + * ACPI devices with a matching HID or CID are recognized.
> + *
> + * If match returns zero, no further, possibly matching devices are processed.
> + */
> +int acpi_find_devices(struct acpi_device_id *ids, void *data,
> +                     int (*match) (struct acpi_device *dev, void *data))
> +{
> +       struct device *hit_dev = NULL;
> +
> +       while(1) {
> +               hit_dev = bus_find_device(&acpi_bus_type, hit_dev, ids,
> +                                         acpi_bus_find_cb);
> +               if (!hit_dev)
> +                       break;
> +               if (!match(to_acpi_device(hit_dev), data))
> +                       break;
> +       }
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(acpi_find_devices);
> +
> +/**
> + * acpi_find_device - find an acpi device of a given HID
> + * @ids: The Hardware IDs for matching devices
> + *
> + * Same as acpi_find_devices, but the search will stop on the first found
> + * device and its structure is returned.
> + *
> + * Use this one if you are sure that only 1 ACPI device of a given HID can
> + * exist in ACPI namespace.
> + *
> + * Returns NULL if no device is found.
> + */
> +struct acpi_device *acpi_find_device(struct acpi_device_id *ids)
> +{
> +       struct device *dev = bus_find_device(&acpi_bus_type, NULL, ids,
> +                                            acpi_bus_find_cb);
> +       if (dev)
> +               return to_acpi_device(dev);
> +       return NULL;
> +}
> +EXPORT_SYMBOL_GPL(acpi_find_device);
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 6001b4da..2f9e09e 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -146,6 +146,10 @@ struct acpi_pci_driver {
>  int acpi_pci_register_driver(struct acpi_pci_driver *driver);
>  void acpi_pci_unregister_driver(struct acpi_pci_driver *driver);
>
> +extern int acpi_find_devices(struct acpi_device_id *ids, void *data,
> +                            int (*match) (struct acpi_device *dev, void *data));
> +extern struct acpi_device *acpi_find_device(struct acpi_device_id *ids);
> +
>  extern int ec_read(u8 addr, u8 *val);
>  extern int ec_write(u8 addr, u8 val);
>  extern int ec_transaction(u8 command,
> --
> 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



-- 
Corentin Chary
http://xf.iksaif.net
--
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 1/4] ACPI: EC: Add ec_get_handle()

[Seth Forshee]

On Fri, Dec 16, 2011 at 02:19:32PM +0100, Thomas Renninger wrote:
> On Friday, December 16, 2011 01:33:33 AM Matthew Garrett wrote:
> > On Fri, Dec 16, 2011 at 01:22:35AM +0100, Thomas Renninger wrote:
> > 
> > > I think best is to move the thinkpad implementation of getting
> > > ACPI handles based on HIDs to osl.c and make it global.
> > > I'll send patches.
> > > Please review them carefully, they are only compile tested.
> > 
> > The ec driver is already finding the hardware on the basis of the HID - 
> > is there any reason to do this twice rather than just exporting the 
> > information the ec driver already has?
> 
> I don't object to export ec_handle, but thinkpad_acpi.c should get
> adjusted as well.
> 
> I had a closer look and can come up with a replacement for acpi_get_devices
> using bus.c matching function bus_find_device() avoiding the namespace
> walk.

My thought was the same as Matthew's, why bother redoing the work that
the EC driver has already done. I'm open to doing it either way though.
What your proposing does look useful independant of whether or not we
use it for finding the EC.

[PATCH 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 2/4] toshiba_acpi: Support alternate hotkey interfaces

[Thomas Renninger]

On Thursday 15 December 2011 19:06:09 Seth Forshee wrote:
...  
> +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;
> +	}
What have you tried to check whether some other kind of ACPI event
is happening?
Do any acpi/SCI interrupts happen?:
watch -n1 "cat /proc/interrupts |grep acpi"

Could it by chance be an EC or other device GPE/SCI?

> +
> +	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");
Why is calling NTFY needed?

...

> +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;
> +}
What are the suspend/resume funcs for?
What bad things happen without them?


     Thomas

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

[Azael Avalos]

2011/12/17 Thomas Renninger <trenn@suse.de>:
> On Thursday 15 December 2011 19:06:09 Seth Forshee wrote:
> ...
>> +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;
>> +     }
> What have you tried to check whether some other kind of ACPI event
> is happening?
> Do any acpi/SCI interrupts happen?:
> watch -n1 "cat /proc/interrupts |grep acpi"

I already did this, no events whatsoever, I was using a Satellite X205
at the time

>
> Could it by chance be an EC or other device GPE/SCI?
>

Seth mentioned me something about this, but w/o proper docs from
Toshiba, we are blindly shooting.

Seth?

>> +
>> +     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");
> Why is calling NTFY needed?

The NTFY method triggers a 0x80 notify event on TOS1900 device,
and thus being trapped by toshiba_acpi_notify, here are the methods

Method (NTFY, 0, NotSerialized)
{
  Store (One, ^^^^VALZ.TECF)
  Notify (VALZ, 0x80)
  Return (0xAA)
}

And then

Method (INFO, 0, NotSerialized)
{
  If (TECF)
  {
    Store (Zero, TECF)
    Store (^^PCI0.LPCB.EC0.TOHK, Local0)
    Store (Zero, ^^PCI0.LPCB.EC0.TOHK)
  }
  Else
  {
    Store (Zero, Local0)
  }

  Return (Local0)
}


>
> ...
>
>> +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;
>> +}
> What are the suspend/resume funcs for?
> What bad things happen without them?

Some models (NB500 among others) stop sending hotkey events
when resumed, and even activating the hotkeys again don't work,
the suspend/resume functions do the trick ;)

>
>
>     Thomas

Saludos
Azael


-- 
-- El mundo apesta y vosotros apestais tambien --
--
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 2/4] toshiba_acpi: Support alternate hotkey interfaces

[Seth Forshee]

On Sat, Dec 17, 2011 at 04:32:14AM -0700, Azael Avalos wrote:
> 2011/12/17 Thomas Renninger <trenn@suse.de>:
> > On Thursday 15 December 2011 19:06:09 Seth Forshee wrote:
> > ...
> >> +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;
> >> +     }
> > What have you tried to check whether some other kind of ACPI event
> > is happening?
> > Do any acpi/SCI interrupts happen?:
> > watch -n1 "cat /proc/interrupts |grep acpi"
> 
> I already did this, no events whatsoever, I was using a Satellite X205
> at the time

I've done similar checks on the NB505, no events.

> >
> > Could it by chance be an EC or other device GPE/SCI?
> >
> 
> Seth mentioned me something about this, but w/o proper docs from
> Toshiba, we are blindly shooting.
> 
> Seth?

In the DSDTs I've inspected there is an EC query method that looks like
it handles events for the hotkeys, but I've never been able to find
anything that will cause the GPE to trigger when the hotkeys are
pressed.

Interestingly, I also saw that the Windows hotkey driver for the NB505
logs some messages that indicate it's also filtering Fn key presses, and
I also found that the binary contains the NTFY string. That's not proof
of anything, but it does suggest that the Windows driver might be doing
something similar to support hotkeys. Which makes me wonder if the GPE
works at all.

> 
> >> +
> >> +     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");
> > Why is calling NTFY needed?
> 
> The NTFY method triggers a 0x80 notify event on TOS1900 device,
> and thus being trapped by toshiba_acpi_notify, here are the methods
> 
> Method (NTFY, 0, NotSerialized)
> {
>   Store (One, ^^^^VALZ.TECF)
>   Notify (VALZ, 0x80)
>   Return (0xAA)
> }
> 
> And then
> 
> Method (INFO, 0, NotSerialized)
> {
>   If (TECF)
>   {
>     Store (Zero, TECF)
>     Store (^^PCI0.LPCB.EC0.TOHK, Local0)
>     Store (Zero, ^^PCI0.LPCB.EC0.TOHK)
>   }
>   Else
>   {
>     Store (Zero, Local0)
>   }
> 
>   Return (Local0)
> }

The NB505 is very similar. We might be able to get by without NTFY if
all it was doing was providing the notification, but as you can see it
also sets a flag that's needed to read the hotkey event with the INFO
method.

> >
> > ...
> >
> >> +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;
> >> +}
> > What are the suspend/resume funcs for?
> > What bad things happen without them?
> 
> Some models (NB500 among others) stop sending hotkey events
> when resumed, and even activating the hotkeys again don't work,
> the suspend/resume functions do the trick ;)

The NB505 (which is what I've been testing with) requires this as well.

Seth

--
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 2/4] toshiba_acpi: Support alternate hotkey interfaces

[Thomas Renninger]

On Saturday 17 December 2011 16:07:42 Seth Forshee wrote:
> On Sat, Dec 17, 2011 at 04:32:14AM -0700, Azael Avalos wrote:
> > 2011/12/17 Thomas Renninger <trenn@suse.de>:
> > > On Thursday 15 December 2011 19:06:09 Seth Forshee wrote:
> > > ...
> > >> +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;
> > >> +     }
> > > What have you tried to check whether some other kind of ACPI event
> > > is happening?
> > > Do any acpi/SCI interrupts happen?:
> > > watch -n1 "cat /proc/interrupts |grep acpi"
> > 
> > I already did this, no events whatsoever, I was using a Satellite X205
> > at the time
> 
> I've done similar checks on the NB505, no events.
Ok.

> > > Could it by chance be an EC or other device GPE/SCI?
> > >
> > 
> > Seth mentioned me something about this, but w/o proper docs from
> > Toshiba, we are blindly shooting.
> > 
> > Seth?
> 
> In the DSDTs I've inspected there is an EC query method that looks like
> it handles events for the hotkeys, but I've never been able to find
> anything that will cause the GPE to trigger when the hotkeys are
> pressed.
Be careful, those EC event notifications do not show up in:
/sys/firmware/acpi/interrupts/gpe*
If no acpi irqs are happening (you said you've tried already):
watch -n1 "cat /proc/interrupts |grep acpi"
filtering the key events sounds appropriate.
I see you dig quite a bit already:

> Interestingly, I also saw that the Windows hotkey driver for the NB505
> logs some messages that indicate it's also filtering Fn key presses, and
> I also found that the binary contains the NTFY string. That's not proof
> of anything, but it does suggest that the Windows driver might be doing
> something similar to support hotkeys. Which makes me wonder if the GPE
> works at all.

Nice work.

   Thomas

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

[Seth Forshee]

On Sun, Dec 18, 2011 at 03:01:35PM +0100, Thomas Renninger wrote:
> > In the DSDTs I've inspected there is an EC query method that looks like
> > it handles events for the hotkeys, but I've never been able to find
> > anything that will cause the GPE to trigger when the hotkeys are
> > pressed.
> Be careful, those EC event notifications do not show up in:
> /sys/firmware/acpi/interrupts/gpe*
> If no acpi irqs are happening (you said you've tried already):
> watch -n1 "cat /proc/interrupts |grep acpi"
> filtering the key events sounds appropriate.
> I see you dig quite a bit already:

I just went back and tried this specifically again to verify my memory
is correct. Without toshiba_acpi loaded, I see nothing. With it loaded
(with this patch series applied, which turns out to be important) I do
see the interrupt count increase. Further investigation leads me to
conclude that this activity appears to be associated with execution of
the INFO method -- reading and writing a variable in the EC address
space.

Seth

[PATCH 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 4/4] toshiba_acpi: Add blacklist for devices with hotkey problems

[Seth Forshee]

The Satellite C670-10V has 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
add this model to that list.

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

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 6824bf7..a2f7cce 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,18 @@ 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"),
+		},
+	},
+	{}
+};
+
 /* utility
  */
 
@@ -935,6 +948,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