Message-ID: <CAFqH_51w213x3W9PTDWu1Qa+fqsGqUtrAFwZqX5zX4Y7kspteg@mail.gmail.com>
Date:   Tue, 22 Jan 2019 16:01:55 +0100
From:   Enric Balletbo Serra <eballetbo@...il.com>
To:     Nick Crews <ncrews@...omium.org>
Cc:     linux-kernel <linux-kernel@...r.kernel.org>,
        Guenter Roeck <groeck@...omium.org>,
        Simon Glass <sjg@...omium.org>,
        Daniel Kurtz <djkurtz@...gle.com>, dlaurie@...omium.org,
        Duncan Laurie <dlaurie@...gle.com>,
        Nick Crews <ncrews@...gle.com>,
        Enric Balletbo i Serra <enric.balletbo@...labora.com>,
        Benson Leung <bleung@...omium.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>
Subject: Re: [PATCH v3 6/9] platform/chrome: Add event handling

Hi Nick,

I'd like to have some feedback from input subsystem if it's possible,
so adding linux-input@...r.kernel.org and Dmitry. Don't forget to add
them for the next versions.

Missatge de Nick Crews <ncrews@...omium.org> del dia ds., 19 de gen.
2019 a les 1:16:
>
> From: Duncan Laurie <dlaurie@...gle.com>
>
> The Wilco Embedded Controller can return extended events that
> are not handled by standard ACPI objects.  These events can
> include hotkeys which map to standard functions like brightness
> controls, or information about EC controlled features like the
> charger or battery.
>
> These events are triggered with an ACPI Notify(0x90) and the
> event data buffer is read through an ACPI method provided by
> the BIOS which reads the event buffer from EC RAM.
>
> These events are then processed, with hotkey events being sent
> to the input subsystem and other events put into a queue which
> can be read by a userspace daemon via a sysfs attribute.
>
> > evtest /dev/input/event6
> Input driver version is 1.0.1
> Input device ID: bus 0x19 vendor 0x0 product 0x0 version 0x0
> Input device name: "Wilco EC hotkeys"
> Supported events:
>   Event type 0 (EV_SYN)
>   Event type 1 (EV_KEY)
>     Event code 224 (KEY_BRIGHTNESSDOWN)
>     Event code 225 (KEY_BRIGHTNESSUP)
>     Event code 240 (KEY_UNKNOWN)
>   Event type 4 (EV_MSC)
>     Event code 4 (MSC_SCAN)
> Properties:
> Testing ... (interrupt to exit)
> Event: type 4 (EV_MSC), code 4 (MSC_SCAN), value 57
> Event: type 1 (EV_KEY), code 224 (KEY_BRIGHTNESSDOWN), value 1
> Event: -------------- SYN_REPORT ------------
> Event: type 1 (EV_KEY), code 224 (KEY_BRIGHTNESSDOWN), value 0
> Event: -------------- SYN_REPORT ------------
> Event: type 4 (EV_MSC), code 4 (MSC_SCAN), value 58
> Event: type 1 (EV_KEY), code 225 (KEY_BRIGHTNESSUP), value 1
> Event: -------------- SYN_REPORT ------------
> Event: type 1 (EV_KEY), code 225 (KEY_BRIGHTNESSUP), value 0
> Event: -------------- SYN_REPORT ------------
>
> Signed-off-by: Duncan Laurie <dlaurie@...gle.com>
> Signed-off-by: Nick Crews <ncrews@...omium.org>
> ---
>
> Changes in v3:
> - change err check from "if (ret < 0)" to "if (ret)"
>
> Changes in v2:
> - rm "wilco_ec_event -" prefix from docstring
> - rm license boiler plate
> - Add sysfs directory documentation
> - Fix cosmetics
> - events are init()ed before subdrivers now
>
>  drivers/platform/chrome/wilco_ec/Makefile |   2 +-
>  drivers/platform/chrome/wilco_ec/core.c   |  14 +-
>  drivers/platform/chrome/wilco_ec/event.c  | 347 ++++++++++++++++++++++
>  include/linux/platform_data/wilco-ec.h    |  34 +++
>  4 files changed, 395 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/platform/chrome/wilco_ec/event.c
>
> diff --git a/drivers/platform/chrome/wilco_ec/Makefile b/drivers/platform/chrome/wilco_ec/Makefile
> index 84ad2772247c..18070d0f8b5e 100644
> --- a/drivers/platform/chrome/wilco_ec/Makefile
> +++ b/drivers/platform/chrome/wilco_ec/Makefile
> @@ -1,5 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
>
>  wilco_ec-objs                          := core.o mailbox.o debugfs.o sysfs.o \
> -                                          legacy.o
> +                                          legacy.o event.o
>  obj-$(CONFIG_WILCO_EC)                 += wilco_ec.o
> diff --git a/drivers/platform/chrome/wilco_ec/core.c b/drivers/platform/chrome/wilco_ec/core.c
> index dd8b896b3833..65588cfcc7a2 100644
> --- a/drivers/platform/chrome/wilco_ec/core.c
> +++ b/drivers/platform/chrome/wilco_ec/core.c
> @@ -77,6 +77,13 @@ static int wilco_ec_probe(struct platform_device *pdev)
>                 goto rm_debugfs;
>         }
>
> +       /* Prepare to handle events */
> +       ret = wilco_ec_event_init(ec);
> +       if (ret) {
> +               dev_err(dev, "Failed to setup event handling\n");
> +               goto remove_sysfs;
> +       }
> +
>         /*
>          * Register a RTC platform device that will get picked up by the RTC
>          * subsystem. This platform device will get freed when its parent device
> @@ -88,11 +95,13 @@ static int wilco_ec_probe(struct platform_device *pdev)
>         if (IS_ERR(child_pdev)) {
>                 dev_err(dev, "Failed to create RTC platform device\n");
>                 ret = PTR_ERR(child_pdev);
> -               goto remove_sysfs;
> +               goto remove_events;
>         }
>
>         return 0;
>
> +remove_events:
> +       wilco_ec_event_remove(ec);
>  remove_sysfs:
>         wilco_ec_sysfs_remove(ec);
>  rm_debugfs:
> @@ -106,6 +115,9 @@ static int wilco_ec_remove(struct platform_device *pdev)
>  {
>         struct wilco_ec_device *ec = platform_get_drvdata(pdev);
>
> +       /* Stop handling EC events */
> +       wilco_ec_event_remove(ec);
> +
>         /* Remove sysfs attributes */
>         wilco_ec_sysfs_remove(ec);
>
> diff --git a/drivers/platform/chrome/wilco_ec/event.c b/drivers/platform/chrome/wilco_ec/event.c
> new file mode 100644
> index 000000000000..7601c02dffe8
> --- /dev/null
> +++ b/drivers/platform/chrome/wilco_ec/event.c
> @@ -0,0 +1,347 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Event handling for Wilco Embedded Controller
> + *
> + * Copyright 2018 Google LLC
> + *
> + * The Wilco Embedded Controller can return extended events that
> + * are not handled by standard ACPI objects.  These events can
> + * include hotkeys which map to standard functions like brightness
> + * controls, or information about EC controlled features like the
> + * charger or battery.
> + *
> + * These events are triggered with an ACPI Notify(0x90) and the
> + * event data buffer is read through an ACPI method provided by
> + * the BIOS which reads the event buffer from EC RAM.
> + *
> + * These events are then processed, with hotkey events being sent
> + * to the input subsystem and other events put into a queue which
> + * can be read by a userspace daemon via a sysfs attribute.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/device.h>
> +#include <linux/input.h>
> +#include <linux/input/sparse-keymap.h>
> +#include <linux/list.h>
> +#include <linux/platform_data/wilco-ec.h>
> +#include <linux/platform_device.h>
> +
> +/* ACPI Notify event code indicating event data is available */
> +#define EC_ACPI_NOTIFY_EVENT           0x90
> +
> +/* ACPI Method to execute to retrieve event data buffer from the EC */
> +#define EC_ACPI_GET_EVENT              "^QSET"
> +
> +/* Maximum number of words in event data returned by the EC */
> +#define EC_ACPI_MAX_EVENT_DATA         6
> +
> +/* Keep at most 100 events in the queue */
> +#define EC_EVENT_QUEUE_MAX             100
> +
> +/**
> + * enum ec_event_type - EC event categories.
> + * @EC_EVENT_TYPE_HOTKEY: Hotkey event for handling special keys.
> + * @EC_EVENT_TYPE_NOTIFY: EC feature state changes.
> + * @EC_EVENT_TYPE_SYSTEM: EC system messages.
> + */
> +enum ec_event_type {
> +       EC_EVENT_TYPE_HOTKEY = 0x10,
> +       EC_EVENT_TYPE_NOTIFY = 0x11,
> +       EC_EVENT_TYPE_SYSTEM = 0x12,
> +};
> +
> +/**
> + * struct ec_event - Extended event returned by the EC.
> + * @size: Number of words in structure after the size word.
> + * @type: Extended event type from &enum ec_event_type.
> + * @event: Event data words.  Max count is %EC_ACPI_MAX_EVENT_DATA.
> + */
> +struct ec_event {
> +       u16 size;
> +       u16 type;
> +       u16 event[0];
> +} __packed;
> +
> +/**
> + * struct ec_event_entry - Event queue entry.
> + * @list: List node.
> + * @size: Number of bytes in event structure.
> + * @event: Extended event returned by the EC.  This should be the last
> + *         element because &struct ec_event includes a zero length array.
> + */
> +struct ec_event_entry {
> +       struct list_head list;
> +       size_t size;
> +       struct ec_event event;
> +};
> +
> +static const struct key_entry wilco_ec_keymap[] = {
> +       { KE_KEY, 0x0057, { KEY_BRIGHTNESSDOWN } },
> +       { KE_KEY, 0x0058, { KEY_BRIGHTNESSUP } },
> +       { KE_END, 0 }
> +};
> +
> +/**
> + * wilco_ec_handle_events() - Handle Embedded Controller events.
> + * @ec: EC device.
> + * @buf: Buffer of event data.
> + * @length: Length of event data buffer.
> + *
> + * Return: Number of events in queue or negative error code on failure.
> + *
> + * This function will handle EC extended events, sending hotkey events
> + * to the input subsystem and queueing others to be read by userspace.
> + */
> +static int wilco_ec_handle_events(struct wilco_ec_device *ec,
> +                                 u8 *buf, u32 length)
> +{
> +       struct wilco_ec_event *queue = &ec->event;
> +       struct ec_event *event;
> +       struct ec_event_entry *entry;
> +       size_t entry_size, num_words;
> +       u32 offset = 0;
> +
> +       while (offset < length) {
> +               event = (struct ec_event *)(buf + offset);
> +               if (!event)
> +                       return -EINVAL;
> +
> +               dev_dbg(ec->dev, "EC event type 0x%02x size %d\n", event->type,
> +                       event->size);
> +
> +               /* Number of 16bit event data words is size - 1 */
> +               num_words = event->size - 1;
> +               entry_size = sizeof(*event) + (num_words * sizeof(u16));
> +
> +               if (num_words > EC_ACPI_MAX_EVENT_DATA) {
> +                       dev_err(ec->dev, "Invalid event word count: %d > %d\n",
> +                               num_words, EC_ACPI_MAX_EVENT_DATA);
> +                       return -EOVERFLOW;
> +               };
> +
> +               /* Ensure event does not overflow the available buffer */
> +               if ((offset + entry_size) > length) {
> +                       dev_err(ec->dev, "Event exceeds buffer: %d > %d\n",
> +                               offset + entry_size, length);
> +                       return -EOVERFLOW;
> +               }
> +
> +               /* Point to the next event in the buffer */
> +               offset += entry_size;
> +
> +               /* Hotkeys are sent to the input subsystem */
> +               if (event->type == EC_EVENT_TYPE_HOTKEY) {
> +                       if (sparse_keymap_report_event(queue->input,
> +                                                      event->event[0],
> +                                                      1, true))
> +                               continue;
> +
> +                       /* Unknown hotkeys are put into the event queue */
> +                       dev_dbg(ec->dev, "Unknown hotkey 0x%04x\n",
> +                               event->event[0]);
> +               }
> +
> +               /* Prepare event for the queue */
> +               entry = kzalloc(entry_size, GFP_KERNEL);
> +               if (!entry)
> +                       return -ENOMEM;
> +
> +               /* Copy event data */
> +               entry->size = entry_size;
> +               memcpy(&entry->event, event, entry->size);
> +
> +               /* Add this event to the queue */
> +               mutex_lock(&queue->lock);
> +               list_add_tail(&entry->list, &queue->list);
> +               queue->count++;
> +               mutex_unlock(&queue->lock);
> +       }
> +
> +       return queue->count;
> +}
> +
> +/**
> + * wilco_ec_acpi_notify() - Handler called by ACPI subsystem for Notify.
> + * @device: EC ACPI device.
> + * @value: Value passed to Notify() in ACPI.
> + * @data: Private data, pointer to EC device.
> + */
> +static void wilco_ec_acpi_notify(acpi_handle device, u32 value, void *data)
> +{
> +       struct wilco_ec_device *ec = data;
> +       struct acpi_buffer event_buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +       union acpi_object *obj;
> +       acpi_status status;
> +       int count;
> +
> +       /* Currently only handle event notifications */
> +       if (value != EC_ACPI_NOTIFY_EVENT) {
> +               dev_err(ec->dev, "Invalid event: 0x%08x\n", value);
> +               return;
> +       }
> +
> +       /* Execute ACPI method to get event data buffer */
> +       status = acpi_evaluate_object(device, EC_ACPI_GET_EVENT,
> +                                     NULL, &event_buffer);
> +       if (ACPI_FAILURE(status)) {
> +               dev_err(ec->dev, "Error executing ACPI method %s()\n",
> +                        EC_ACPI_GET_EVENT);
> +               return;
> +       }
> +
> +       obj = (union acpi_object *)event_buffer.pointer;
> +       if (!obj) {
> +               dev_err(ec->dev, "Nothing returned from %s()\n",
> +                       EC_ACPI_GET_EVENT);
> +               return;
> +       }
> +       if (obj->type != ACPI_TYPE_BUFFER) {
> +               dev_err(ec->dev, "Invalid object returned from %s()\n",
> +                       EC_ACPI_GET_EVENT);
> +               kfree(obj);
> +               return;
> +       }
> +       if (obj->buffer.length < sizeof(struct ec_event)) {
> +               dev_err(ec->dev, "Invalid buffer length %d from %s()\n",
> +                       obj->buffer.length, EC_ACPI_GET_EVENT);
> +               kfree(obj);
> +               return;
> +       }
> +
> +       /* Handle events and notify sysfs if any queued for userspace */
> +       count = wilco_ec_handle_events(ec, obj->buffer.pointer,
> +                                      obj->buffer.length);
> +
> +       if (count > 0) {
> +               dev_dbg(ec->dev, "EC event queue has %d entries\n", count);
> +               sysfs_notify(&ec->dev->kobj, NULL, "event");
> +       }
> +
> +       kfree(obj);
> +}
> +
> +static ssize_t wilco_ec_event_read(struct file *filp, struct kobject *kobj,
> +                                  struct bin_attribute *attr,
> +                                  char *buf, loff_t off, size_t count)
> +{
> +       struct wilco_ec_device *ec = attr->private;
> +       struct ec_event_entry *entry;
> +
> +       /* Only supports reading full events */
> +       if (off != 0)
> +               return -EINVAL;
> +
> +       /* Remove the first event and provide it to userspace */
> +       mutex_lock(&ec->event.lock);
> +       entry = list_first_entry_or_null(&ec->event.list,
> +                                        struct ec_event_entry, list);
> +       if (entry) {
> +               if (entry->size < count)
> +                       count = entry->size;
> +               memcpy(buf, &entry->event, count);
> +               list_del(&entry->list);
> +               kfree(entry);
> +               ec->event.count--;
> +       } else {
> +               count = 0;
> +       }
> +       mutex_unlock(&ec->event.lock);
> +
> +       return count;
> +}
> +
> +static void wilco_ec_event_clear(struct wilco_ec_device *ec)
> +{
> +       struct ec_event_entry *entry, *next;
> +
> +       mutex_lock(&ec->event.lock);
> +
> +       /* Clear the event queue */
> +       list_for_each_entry_safe(entry, next, &ec->event.list, list) {
> +               list_del(&entry->list);
> +               kfree(entry);
> +               ec->event.count--;
> +       }
> +
> +       mutex_unlock(&ec->event.lock);
> +}
> +
> +int wilco_ec_event_init(struct wilco_ec_device *ec)
> +{
> +       struct wilco_ec_event *event = &ec->event;
> +       struct device *dev = ec->dev;
> +       struct acpi_device *adev = ACPI_COMPANION(dev);
> +       acpi_status status;
> +       int ret;
> +
> +       if (!adev) {
> +               dev_err(dev, "Unable to find Wilco ACPI Device\n");
> +               return -ENODEV;
> +       }
> +
> +       INIT_LIST_HEAD(&event->list);
> +       mutex_init(&event->lock);
> +
> +       /* Allocate input device for hotkeys */
> +       event->input = input_allocate_device();
> +       if (!event->input)
> +               return -ENOMEM;
> +       event->input->name = "Wilco EC hotkeys";
> +       event->input->phys = "ec/input0";
> +       event->input->id.bustype = BUS_HOST;
> +       ret = sparse_keymap_setup(event->input, wilco_ec_keymap, NULL);
> +       if (ret) {
> +               dev_err(dev, "Unable to setup input device keymap\n");
> +               input_free_device(event->input);
> +               return ret;
> +       }
> +       ret = input_register_device(event->input);
> +       if (ret) {
> +               dev_err(dev, "Unable to register input device\n");
> +               input_free_device(event->input);
> +               return ret;
> +       }
> +
> +       /* Create sysfs attribute for userspace event handling */
> +       sysfs_bin_attr_init(&event->attr);
> +       event->attr.attr.name = "event";
> +       event->attr.attr.mode = 0400;
> +       event->attr.read = wilco_ec_event_read;
> +       event->attr.private = ec;
> +       ret = device_create_bin_file(dev, &event->attr);
> +       if (ret) {
> +               dev_err(dev, "Failed to create event attribute in sysfs\n");
> +               input_unregister_device(event->input);
> +               return ret;
> +       }
> +
> +       /* Install ACPI handler for Notify events */
> +       status = acpi_install_notify_handler(adev->handle, ACPI_ALL_NOTIFY,
> +                                            wilco_ec_acpi_notify, ec);
> +       if (ACPI_FAILURE(status)) {
> +               dev_err(dev, "Failed to register notifier %08x\n", status);
> +               device_remove_bin_file(dev, &event->attr);
> +               input_unregister_device(event->input);
> +               return -ENODEV;
> +       }
> +
> +       return 0;
> +}
> +
> +void wilco_ec_event_remove(struct wilco_ec_device *ec)
> +{
> +       struct acpi_device *adev = ACPI_COMPANION(ec->dev);
> +
> +       /* Stop new events */
> +       if (adev)
> +               acpi_remove_notify_handler(adev->handle, ACPI_ALL_NOTIFY,
> +                                          wilco_ec_acpi_notify);
> +
> +       /* Remove event interfaces */
> +       device_remove_bin_file(ec->dev, &ec->event.attr);
> +       input_unregister_device(ec->event.input);
> +
> +       /* Clear the event queue */
> +       wilco_ec_event_clear(ec);
> +}
> diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
> index b1487a259a12..2728fad90a3a 100644
> --- a/include/linux/platform_data/wilco-ec.h
> +++ b/include/linux/platform_data/wilco-ec.h
> @@ -9,7 +9,9 @@
>  #define WILCO_EC_H
>
>  #include <linux/device.h>
> +#include <linux/input.h>
>  #include <linux/kernel.h>
> +#include <linux/list.h>
>
>  #define WILCO_EC_FLAG_NO_RESPONSE      BIT(0) /* EC does not respond */
>  #define WILCO_EC_FLAG_EXTENDED_DATA    BIT(1) /* EC returns 256 data bytes */
> @@ -24,6 +26,22 @@
>  /* Extended commands have 256 bytes of response data */
>  #define EC_MAILBOX_DATA_SIZE_EXTENDED  256
>
> +/**
> + * struct wilco_ec_event - EC extended events.
> + * @lock: Mutex to guard the list of events.
> + * @list: Queue of EC events to be provided to userspace.
> + * @attr: Sysfs attribute for userspace to read events.
> + * @count: Count of events in the queue.
> + * @input: Input device for hotkey events.
> + */
> +struct wilco_ec_event {
> +       struct mutex lock;
> +       struct list_head list;
> +       struct bin_attribute attr;
> +       size_t count;
> +       struct input_dev *input;
> +};
> +
>  /**
>   * struct wilco_ec_device - Wilco Embedded Controller handle.
>   * @dev: Device handle.
> @@ -34,6 +52,7 @@
>   * @data_buffer: Buffer used for EC communication.  The same buffer
>   *               is used to hold the request and the response.
>   * @data_size: Size of the data buffer used for EC communication.
> + * @event: EC extended event handler.
>   */
>  struct wilco_ec_device {
>         struct device *dev;
> @@ -43,6 +62,7 @@ struct wilco_ec_device {
>         struct resource *io_packet;
>         void *data_buffer;
>         size_t data_size;
> +       struct wilco_ec_event event;
>  };
>
>  /**
> @@ -152,4 +172,18 @@ int wilco_ec_sysfs_init(struct wilco_ec_device *ec);
>   */
>  void wilco_ec_sysfs_remove(struct wilco_ec_device *ec);
>
> +/**
> + * wilco_ec_event_init() - Prepare to handle EC events.
> + * @ec: EC device.
> + *
> + * Return: 0 for success or negative error code on failure.
> + */
> +int wilco_ec_event_init(struct wilco_ec_device *ec);
> +
> +/**
> + * wilco_ec_event_remove() - Remove EC event handler.
> + * @ec: EC device.
> + */
> +void wilco_ec_event_remove(struct wilco_ec_device *ec);
> +
>  #endif /* WILCO_EC_H */
> --
> 2.20.1.321.g9e740568ce-goog
>

Powered by blists - more mailing lists