Re: [PATCH 4/7] platform/surface: aggregator_cdev: Add support for forwarding events to user-space

Hans de Goede
2021-06-04T11:32:55Z

Hi,

I've one review remark inline below.

On 6/4/21 1:45 AM, Maximilian Luz wrote:
> Currently, debugging unknown events requires writing a custom driver.
> This is somewhat difficult, slow to adapt, and not entirely
> user-friendly for quickly trying to figure out things on devices of some
> third-party user. We can do better. We already have a user-space
> interface intended for debugging SAM EC requests, so let's add support
> for receiving events to that.
>
> This commit provides support for receiving events by reading from the
> controller file. It additionally introduces two new IOCTLs to control
> which event categories will be forwarded. Specifically, a user-space
> client can specify which target categories it wants to receive events
> from by registering the corresponding notifier(s) via the IOCTLs and
> after that, read the received events by reading from the controller
> device.
>
> Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
> ---
> .../userspace-api/ioctl/ioctl-number.rst | 2 +-
> .../surface/surface_aggregator_cdev.c | 457 +++++++++++++++++-
> include/uapi/linux/surface_aggregator/cdev.h | 41 +-
> 3 files changed, 474 insertions(+), 26 deletions(-)
>
> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
> index 9bfc2b510c64..1409e40e6345 100644
> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> @@ -325,7 +325,7 @@ Code Seq# Include File Comments
> 0xA3 90-9F linux/dtlk.h
> 0xA4 00-1F uapi/linux/tee.h Generic TEE subsystem
> 0xA4 00-1F uapi/asm/sgx.h <mailto:linux-sgx@vger.kernel.org>
> -0xA5 01 linux/surface_aggregator/cdev.h Microsoft Surface Platform System Aggregator
> +0xA5 01-05 linux/surface_aggregator/cdev.h Microsoft Surface Platform System Aggregator
> <mailto:luzmaximilian@gmail.com>
> 0xA5 20-2F linux/surface_aggregator/dtx.h Microsoft Surface DTX driver
> <mailto:luzmaximilian@gmail.com>
> diff --git a/drivers/platform/surface/surface_aggregator_cdev.c b/drivers/platform/surface/surface_aggregator_cdev.c
> index 79e28fab7e40..807930144039 100644
> --- a/drivers/platform/surface/surface_aggregator_cdev.c
> +++ b/drivers/platform/surface/surface_aggregator_cdev.c
> @@ -3,29 +3,69 @@
> * Provides user-space access to the SSAM EC via the /dev/surface/aggregator
> * misc device. Intended for debugging and development.
> *
> - * Copyright (C) 2020 Maximilian Luz <luzmaximilian@gmail.com>
> + * Copyright (C) 2020-2021 Maximilian Luz <luzmaximilian@gmail.com>
> */
>
> #include <linux/fs.h>
> +#include <linux/ioctl.h>
> #include <linux/kernel.h>
> +#include <linux/kfifo.h>
> #include <linux/kref.h>
> #include <linux/miscdevice.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> +#include <linux/poll.h>
> #include <linux/rwsem.h>
> #include <linux/slab.h>
> #include <linux/uaccess.h>
> +#include <linux/vmalloc.h>
>
> #include <linux/surface_aggregator/cdev.h>
> #include <linux/surface_aggregator/controller.h>
> +#include <linux/surface_aggregator/serial_hub.h>
>
> #define SSAM_CDEV_DEVICE_NAME "surface_aggregator_cdev"
>
> +
> +/* -- Main structures. ------------------------------------------------------ */
> +
> +enum ssam_cdev_device_state {
> + SSAM_CDEV_DEVICE_SHUTDOWN_BIT = BIT(0),
> +};
> +
> struct ssam_cdev {
> struct kref kref;
> struct rw_semaphore lock;
> +
> + struct device *dev;
> struct ssam_controller *ctrl;
> struct miscdevice mdev;
> + unsigned long flags;
> +
> + struct rw_semaphore client_lock; /* Guards client list. */
> + struct list_head client_list;
> +};
> +
> +struct ssam_cdev_client;
> +
> +struct ssam_cdev_notifier {
> + struct ssam_cdev_client *client;
> + struct ssam_event_notifier nf;
> +};
> +
> +struct ssam_cdev_client {
> + struct ssam_cdev *cdev;
> + struct list_head node;
> +
> + struct mutex notifier_lock; /* Guards notifier access for registration */
> + struct ssam_cdev_notifier *notifier[SSH_NUM_EVENTS];
> +
> + struct mutex read_lock; /* Guards FIFO buffer read access */
> + struct mutex write_lock; /* Guards FIFO buffer write access */
> + DECLARE_KFIFO(buffer, u8, 4096);
> +
> + wait_queue_head_t waitq;
> + struct fasync_struct *fasync;
> };
>
> static void __ssam_cdev_release(struct kref *kref)
> @@ -47,24 +87,169 @@ static void ssam_cdev_put(struct ssam_cdev *cdev)
> kref_put(&cdev->kref, __ssam_cdev_release);
> }
>
> -static int ssam_cdev_device_open(struct inode *inode, struct file *filp)
> +
> +/* -- Notifier handling. ---------------------------------------------------- */
> +
> +static u32 ssam_cdev_notifier(struct ssam_event_notifier *nf, const struct ssam_event *in)
> {
> - struct miscdevice *mdev = filp->private_data;
> - struct ssam_cdev *cdev = container_of(mdev, struct ssam_cdev, mdev);
> + struct ssam_cdev_notifier *cdev_nf = container_of(nf, struct ssam_cdev_notifier, nf);
> + struct ssam_cdev_client *client = cdev_nf->client;
> + struct ssam_cdev_event event;
> + size_t n = struct_size(&event, data, in->length);
> +
> + /* Translate event. */
> + event.target_category = in->target_category;
> + event.target_id = in->target_id;
> + event.command_id = in->command_id;
> + event.instance_id = in->instance_id;
> + event.length = in->length;
> +
> + mutex_lock(&client->write_lock);
> +
> + /* Make sure we have enough space. */
> + if (kfifo_avail(&client->buffer) < n) {
> + dev_warn(client->cdev->dev,
> + "buffer full, dropping event (tc: %#04x, tid: %#04x, cid: %#04x, iid: %#04x)\n",
> + in->target_category, in->target_id, in->command_id, in->instance_id);
> + mutex_unlock(&client->write_lock);
> + return 0;
> + }
>
> - filp->private_data = ssam_cdev_get(cdev);
> - return stream_open(inode, filp);
> + /* Copy event header and payload. */
> + kfifo_in(&client->buffer, (const u8 *)&event, struct_size(&event, data, 0));
> + kfifo_in(&client->buffer, &in->data[0], in->length);
> +
> + mutex_unlock(&client->write_lock);
> +
> + /* Notify waiting readers. */
> + kill_fasync(&client->fasync, SIGIO, POLL_IN);
> + wake_up_interruptible(&client->waitq);
> +
> + /*
> + * Don't mark events as handled, this is the job of a proper driver and
> + * not the debugging interface.
> + */
> + return 0;
> }
>
> -static int ssam_cdev_device_release(struct inode *inode, struct file *filp)
> +static int ssam_cdev_notifier_register(struct ssam_cdev_client *client, u8 tc, int priority)
> {
> - ssam_cdev_put(filp->private_data);
> - return 0;
> + const u16 rqid = ssh_tc_to_rqid(tc);
> + const u16 event = ssh_rqid_to_event(rqid);
> + struct ssam_cdev_notifier *nf;
> + int status;
> +
> + /* Validate notifier target category. */
> + if (!ssh_rqid_is_event(rqid))
> + return -EINVAL;
> +
> + mutex_lock(&client->notifier_lock);
> +
> + /* Check if the notifier has already been registered. */
> + if (client->notifier[event]) {
> + mutex_unlock(&client->notifier_lock);
> + return -EEXIST;
> + }
> +
> + /* Allocate new notifier. */
> + nf = kzalloc(sizeof(*nf), GFP_KERNEL);
> + if (!nf) {
> + mutex_unlock(&client->notifier_lock);
> + return -ENOMEM;
> + }
> +
> + /*
> + * Create a dummy notifier with the minimal required fields for
> + * observer registration. Note that we can skip fully specifying event
> + * and registry here as we do not need any matching and use silent
> + * registration, which does not enable the corresponding event.
> + */
> + nf->client = client;
> + nf->nf.base.fn = ssam_cdev_notifier;
> + nf->nf.base.priority = priority;
> + nf->nf.event.id.target_category = tc;
> + nf->nf.event.mask = 0; /* Do not do any matching. */
> + nf->nf.flags = SSAM_EVENT_NOTIFIER_OBSERVER;
> +
> + /* Register notifier. */
> + status = ssam_notifier_register(client->cdev->ctrl, &nf->nf);
> + if (status)
> + kfree(nf);
> + else
> + client->notifier[event] = nf;
> +
> + mutex_unlock(&client->notifier_lock);
> + return status;
> }
>
> -static long ssam_cdev_request(struct ssam_cdev *cdev, unsigned long arg)
> +static int ssam_cdev_notifier_unregister(struct ssam_cdev_client *client, u8 tc)
> +{
> + const u16 rqid = ssh_tc_to_rqid(tc);
> + const u16 event = ssh_rqid_to_event(rqid);
> + int status;
> +
> + /* Validate notifier target category. */
> + if (!ssh_rqid_is_event(rqid))
> + return -EINVAL;
> +
> + mutex_lock(&client->notifier_lock);
> +
> + /* Check if the notifier is currently registered. */
> + if (!client->notifier[event]) {
> + mutex_unlock(&client->notifier_lock);
> + return -ENOENT;
> + }
> +
> + /* Unregister and free notifier. */
> + status = ssam_notifier_unregister(client->cdev->ctrl, &client->notifier[event]->nf);
> + kfree(client->notifier[event]);
> + client->notifier[event] = NULL;
> +
> + mutex_unlock(&client->notifier_lock);
> + return status;
> +}
> +
> +static void ssam_cdev_notifier_unregister_all(struct ssam_cdev_client *client)
> +{
> + int i;
> +
> + down_read(&client->cdev->lock);
> +
> + /*
> + * This function may be used during shutdown, thus we need to test for
> + * cdev->ctrl instead of the SSAM_CDEV_DEVICE_SHUTDOWN_BIT bit.
> + */
> + if (client->cdev->ctrl) {
> + for (i = 0; i < SSH_NUM_EVENTS; i++)
> + ssam_cdev_notifier_unregister(client, i + 1);
> +
> + } else {
> + int count = 0;
> +
> + /*
> + * Device has been shut down. Any notifier remaining is a bug,
> + * so warn about that as this would otherwise hardly be
> + * noticeable. Nevertheless, free them as well.
> + */
> + mutex_lock(&client->notifier_lock);
> + for (i = 0; i < SSH_NUM_EVENTS; i++) {
> + count += !!(client->notifier[i]);
> + kfree(client->notifier[i]);
> + client->notifier[i] = NULL;
> + }
> + mutex_unlock(&client->notifier_lock);
> +
> + WARN_ON(count > 0);
> + }
> +
> + up_read(&client->cdev->lock);
> +}
> +
> +
> +/* -- IOCTL functions. ------------------------------------------------------ */
> +
> +static long ssam_cdev_request(struct ssam_cdev_client *client, struct ssam_cdev_request __user *r)
> {
> - struct ssam_cdev_request __user *r;
> struct ssam_cdev_request rqst;
> struct ssam_request spec = {};
> struct ssam_response rsp = {};
> @@ -72,7 +257,6 @@ static long ssam_cdev_request(struct ssam_cdev *cdev, unsigned long arg)
> void __user *rspdata;
> int status = 0, ret = 0, tmp;
>
> - r = (struct ssam_cdev_request __user *)arg;
> ret = copy_struct_from_user(&rqst, sizeof(rqst), r, sizeof(*r));
> if (ret)
> goto out;
> @@ -152,7 +336,7 @@ static long ssam_cdev_request(struct ssam_cdev *cdev, unsigned long arg)
> }
>
> /* Perform request. */
> - status = ssam_request_sync(cdev->ctrl, &spec, &rsp);
> + status = ssam_request_sync(client->cdev->ctrl, &spec, &rsp);
> if (status)
> goto out;
>
> @@ -177,48 +361,244 @@ static long ssam_cdev_request(struct ssam_cdev *cdev, unsigned long arg)
> return ret;
> }
>
> -static long __ssam_cdev_device_ioctl(struct ssam_cdev *cdev, unsigned int cmd,
> +static long ssam_cdev_notif_register(struct ssam_cdev_client *client,
> + const struct ssam_cdev_notifier_desc __user *d)
> +{
> + struct ssam_cdev_notifier_desc desc;
> + long ret;
> +
> + ret = copy_struct_from_user(&desc, sizeof(desc), d, sizeof(*d));
> + if (ret)
> + return ret;
> +
> + return ssam_cdev_notifier_register(client, desc.target_category, desc.priority);
> +}
> +
> +static long ssam_cdev_notif_unregister(struct ssam_cdev_client *client,
> + const struct ssam_cdev_notifier_desc __user *d)
> +{
> + struct ssam_cdev_notifier_desc desc;
> + long ret;
> +
> + ret = copy_struct_from_user(&desc, sizeof(desc), d, sizeof(*d));
> + if (ret)
> + return ret;
> +
> + return ssam_cdev_notifier_unregister(client, desc.target_category);
> +}
> +
> +
> +/* -- File operations. ------------------------------------------------------ */
> +
> +static int ssam_cdev_device_open(struct inode *inode, struct file *filp)
> +{
> + struct miscdevice *mdev = filp->private_data;
> + struct ssam_cdev_client *client;
> + struct ssam_cdev *cdev = container_of(mdev, struct ssam_cdev, mdev);
> +
> + /* Initialize client */
> + client = vzalloc(sizeof(*client));
> + if (!client)
> + return -ENOMEM;
> +
> + client->cdev = ssam_cdev_get(cdev);
> +
> + INIT_LIST_HEAD(&client->node);
> +
> + mutex_init(&client->notifier_lock);
> +
> + mutex_init(&client->read_lock);
> + mutex_init(&client->write_lock);
> + INIT_KFIFO(client->buffer);
> + init_waitqueue_head(&client->waitq);
> +
> + filp->private_data = client;
> +
> + /* Attach client. */
> + down_write(&cdev->client_lock);
> +
> + if (test_bit(SSAM_CDEV_DEVICE_SHUTDOWN_BIT, &cdev->flags)) {
> + up_write(&cdev->client_lock);
> + ssam_cdev_put(client->cdev);

You are missing the mutex_destroy() calls here which you are
doing in ssam_cdev_device_release().

Or maybe move the mutex_init calls below this check
(before the up_write()) since I don't think the client can
be accessed by any code until the up_write is done?

Regards,

Hans

> + vfree(client);
> + return -ENODEV;
> + }
> + list_add_tail(&client->node, &cdev->client_list);
> +
> + up_write(&cdev->client_lock);
> +
> + stream_open(inode, filp);
> + return 0;
> +}
> +
> +static int ssam_cdev_device_release(struct inode *inode, struct file *filp)
> +{
> + struct ssam_cdev_client *client = filp->private_data;
> +
> + /* Force-unregister all remaining notifiers of this client. */
> + ssam_cdev_notifier_unregister_all(client);
> +
> + /* Detach client. */
> + down_write(&client->cdev->client_lock);
> + list_del(&client->node);
> + up_write(&client->cdev->client_lock);
> +
> + /* Free client. */
> + mutex_destroy(&client->write_lock);
> + mutex_destroy(&client->read_lock);
> +
> + mutex_destroy(&client->notifier_lock);
> +
> + ssam_cdev_put(client->cdev);
> + vfree(client);
> +
> + return 0;
> +}
> +
> +static long __ssam_cdev_device_ioctl(struct ssam_cdev_client *client, unsigned int cmd,
> unsigned long arg)
> {
> switch (cmd) {
> case SSAM_CDEV_REQUEST:
> - return ssam_cdev_request(cdev, arg);
> + return ssam_cdev_request(client, (struct ssam_cdev_request __user *)arg);
> +
> + case SSAM_CDEV_NOTIF_REGISTER:
> + return ssam_cdev_notif_register(client,
> + (struct ssam_cdev_notifier_desc __user *)arg);
> +
> + case SSAM_CDEV_NOTIF_UNREGISTER:
> + return ssam_cdev_notif_unregister(client,
> + (struct ssam_cdev_notifier_desc __user *)arg);
>
> default:
> return -ENOTTY;
> }
> }
>
> -static long ssam_cdev_device_ioctl(struct file *file, unsigned int cmd,
> - unsigned long arg)
> +static long ssam_cdev_device_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> {
> - struct ssam_cdev *cdev = file->private_data;
> + struct ssam_cdev_client *client = file->private_data;
> long status;
>
> /* Ensure that controller is valid for as long as we need it. */
> + if (down_read_killable(&client->cdev->lock))
> + return -ERESTARTSYS;
> +
> + if (test_bit(SSAM_CDEV_DEVICE_SHUTDOWN_BIT, &client->cdev->flags)) {
> + up_read(&client->cdev->lock);
> + return -ENODEV;
> + }
> +
> + status = __ssam_cdev_device_ioctl(client, cmd, arg);
> +
> + up_read(&client->cdev->lock);
> + return status;
> +}
> +
> +static ssize_t ssam_cdev_read(struct file *file, char __user *buf, size_t count, loff_t *offs)
> +{
> + struct ssam_cdev_client *client = file->private_data;
> + struct ssam_cdev *cdev = client->cdev;
> + unsigned int copied;
> + int status = 0;
> +
> if (down_read_killable(&cdev->lock))
> return -ERESTARTSYS;
>
> - if (!cdev->ctrl) {
> + /* Make sure we're not shut down. */
> + if (test_bit(SSAM_CDEV_DEVICE_SHUTDOWN_BIT, &cdev->flags)) {
> up_read(&cdev->lock);
> return -ENODEV;
> }
>
> - status = __ssam_cdev_device_ioctl(cdev, cmd, arg);
> + do {
> + /* Check availability, wait if necessary. */
> + if (kfifo_is_empty(&client->buffer)) {
> + up_read(&cdev->lock);
> +
> + if (file->f_flags & O_NONBLOCK)
> + return -EAGAIN;
> +
> + status = wait_event_interruptible(client->waitq,
> + !kfifo_is_empty(&client->buffer) ||
> + test_bit(SSAM_CDEV_DEVICE_SHUTDOWN_BIT,
> + &cdev->flags));
> + if (status < 0)
> + return status;
> +
> + if (down_read_killable(&cdev->lock))
> + return -ERESTARTSYS;
> +
> + /* Need to check that we're not shut down again. */
> + if (test_bit(SSAM_CDEV_DEVICE_SHUTDOWN_BIT, &cdev->flags)) {
> + up_read(&cdev->lock);
> + return -ENODEV;
> + }
> + }
> +
> + /* Try to read from FIFO. */
> + if (mutex_lock_interruptible(&client->read_lock)) {
> + up_read(&cdev->lock);
> + return -ERESTARTSYS;
> + }
> +
> + status = kfifo_to_user(&client->buffer, buf, count, &copied);
> + mutex_unlock(&client->read_lock);
> +
> + if (status < 0) {
> + up_read(&cdev->lock);
> + return status;
> + }
> +
> + /* We might not have gotten anything, check this here. */
> + if (copied == 0 && (file->f_flags & O_NONBLOCK)) {
> + up_read(&cdev->lock);
> + return -EAGAIN;
> + }
> + } while (copied == 0);
>
> up_read(&cdev->lock);
> - return status;
> + return copied;
> +}
> +
> +static __poll_t ssam_cdev_poll(struct file *file, struct poll_table_struct *pt)
> +{
> + struct ssam_cdev_client *client = file->private_data;
> + __poll_t events = 0;
> +
> + if (test_bit(SSAM_CDEV_DEVICE_SHUTDOWN_BIT, &client->cdev->flags))
> + return EPOLLHUP | EPOLLERR;
> +
> + poll_wait(file, &client->waitq, pt);
> +
> + if (!kfifo_is_empty(&client->buffer))
> + events |= EPOLLIN | EPOLLRDNORM;
> +
> + return events;
> +}
> +
> +static int ssam_cdev_fasync(int fd, struct file *file, int on)
> +{
> + struct ssam_cdev_client *client = file->private_data;
> +
> + return fasync_helper(fd, file, on, &client->fasync);
> }
>
> static const struct file_operations ssam_controller_fops = {
> .owner = THIS_MODULE,
> .open = ssam_cdev_device_open,
> .release = ssam_cdev_device_release,
> + .read = ssam_cdev_read,
> + .poll = ssam_cdev_poll,
> + .fasync = ssam_cdev_fasync,
> .unlocked_ioctl = ssam_cdev_device_ioctl,
> .compat_ioctl = ssam_cdev_device_ioctl,
> - .llseek = noop_llseek,
> + .llseek = no_llseek,
> };
>
> +
> +/* -- Device and driver setup ----------------------------------------------- */
> +
> static int ssam_dbg_device_probe(struct platform_device *pdev)
> {
> struct ssam_controller *ctrl;
> @@ -236,6 +616,7 @@ static int ssam_dbg_device_probe(struct platform_device *pdev)
> kref_init(&cdev->kref);
> init_rwsem(&cdev->lock);
> cdev->ctrl = ctrl;
> + cdev->dev = &pdev->dev;
>
> cdev->mdev.parent = &pdev->dev;
> cdev->mdev.minor = MISC_DYNAMIC_MINOR;
> @@ -243,6 +624,9 @@ static int ssam_dbg_device_probe(struct platform_device *pdev)
> cdev->mdev.nodename = "surface/aggregator";
> cdev->mdev.fops = &ssam_controller_fops;
>
> + init_rwsem(&cdev->client_lock);
> + INIT_LIST_HEAD(&cdev->client_list);
> +
> status = misc_register(&cdev->mdev);
> if (status) {
> kfree(cdev);
> @@ -256,8 +640,32 @@ static int ssam_dbg_device_probe(struct platform_device *pdev)
> static int ssam_dbg_device_remove(struct platform_device *pdev)
> {
> struct ssam_cdev *cdev = platform_get_drvdata(pdev);
> + struct ssam_cdev_client *client;
>
> - misc_deregister(&cdev->mdev);
> + /*
> + * Mark device as shut-down. Prevent new clients from being added and
> + * new operations from being executed.
> + */
> + set_bit(SSAM_CDEV_DEVICE_SHUTDOWN_BIT, &cdev->flags);
> +
> + down_write(&cdev->client_lock);
> +
> + /* Remove all notifiers registered by us. */
> + list_for_each_entry(client, &cdev->client_list, node) {
> + ssam_cdev_notifier_unregister_all(client);
> + }
> +
> + /* Wake up async clients. */
> + list_for_each_entry(client, &cdev->client_list, node) {
> + kill_fasync(&client->fasync, SIGIO, POLL_HUP);
> + }
> +
> + /* Wake up blocking clients. */
> + list_for_each_entry(client, &cdev->client_list, node) {
> + wake_up_interruptible(&client->waitq);
> + }
> +
> + up_write(&cdev->client_lock);
>
> /*
> * The controller is only guaranteed to be valid for as long as the
> @@ -266,8 +674,11 @@ static int ssam_dbg_device_remove(struct platform_device *pdev)
> */
> down_write(&cdev->lock);
> cdev->ctrl = NULL;
> + cdev->dev = NULL;
> up_write(&cdev->lock);
>
> + misc_deregister(&cdev->mdev);
> +
> ssam_cdev_put(cdev);
> return 0;
> }
> diff --git a/include/uapi/linux/surface_aggregator/cdev.h b/include/uapi/linux/surface_aggregator/cdev.h
> index fbcce04abfe9..4f393fafc235 100644
> --- a/include/uapi/linux/surface_aggregator/cdev.h
> +++ b/include/uapi/linux/surface_aggregator/cdev.h
> @@ -6,7 +6,7 @@
> * device. This device provides direct user-space access to the SSAM EC.
> * Intended for debugging and development.
> *
> - * Copyright (C) 2020 Maximilian Luz <luzmaximilian@gmail.com>
> + * Copyright (C) 2020-2021 Maximilian Luz <luzmaximilian@gmail.com>
> */
>
> #ifndef _UAPI_LINUX_SURFACE_AGGREGATOR_CDEV_H
> @@ -73,6 +73,43 @@ struct ssam_cdev_request {
> } response;
> } __attribute__((__packed__));
>
> -#define SSAM_CDEV_REQUEST _IOWR(0xA5, 1, struct ssam_cdev_request)
> +/**
> + * struct ssam_cdev_notifier_desc - Notifier descriptor.
> + * @priority: Priority value determining the order in which notifier
> + * callbacks will be called. A higher value means higher
> + * priority, i.e. the associated callback will be executed
> + * earlier than other (lower priority) callbacks.
> + * @target_category: The event target category for which this notifier should
> + * receive events.
> + *
> + * Specifies the notifier that should be registered or unregistered,
> + * specifically with which priority and for which target category of events.
> + */
> +struct ssam_cdev_notifier_desc {
> + __s32 priority;
> + __u8 target_category;
> +} __attribute__((__packed__));
> +
> +/**
> + * struct ssam_cdev_event - SSAM event sent by the EC.
> + * @target_category: Target category of the event source. See &enum ssam_ssh_tc.
> + * @target_id: Target ID of the event source.
> + * @command_id: Command ID of the event.
> + * @instance_id: Instance ID of the event source.
> + * @length: Length of the event payload in bytes.
> + * @data: Event payload data.
> + */
> +struct ssam_cdev_event {
> + __u8 target_category;
> + __u8 target_id;
> + __u8 command_id;
> + __u8 instance_id;
> + __u16 length;
> + __u8 data[];
> +} __attribute__((__packed__));
> +
> +#define SSAM_CDEV_REQUEST _IOWR(0xA5, 1, struct ssam_cdev_request)
> +#define SSAM_CDEV_NOTIF_REGISTER _IOW(0xA5, 2, struct ssam_cdev_notifier_desc)
> +#define SSAM_CDEV_NOTIF_UNREGISTER _IOW(0xA5, 3, struct ssam_cdev_notifier_desc)
>
> #endif /* _UAPI_LINUX_SURFACE_AGGREGATOR_CDEV_H */
>