Hi Hans,
I've read this patch a large number of times and I think also the details
begin to seem sound. A few comments below.
On Thu, May 03, 2018 at 04:52:53PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>
> Add initial media request support:
>
> 1) Add MEDIA_IOC_REQUEST_ALLOC ioctl support to media-device.c
> 2) Add struct media_request to store request objects.
> 3) Add struct media_request_object to represent a request object.
> 4) Add MEDIA_REQUEST_IOC_QUEUE/REINIT ioctl support.
>
> Basic lifecycle: the application allocates a request, adds
> objects to it, queues the request, polls until it is completed
> and can then read the final values of the objects at the time
> of completion. When it closes the file descriptor the request
> memory will be freed (actually, when the last user of that request
> releases the request).
>
> Drivers will bind an object to a request (the 'adds objects to it'
> phase), when MEDIA_REQUEST_IOC_QUEUE is called the request is
> validated (req_validate op), then queued (the req_queue op).
>
> When done with an object it can either be unbound from the request
> (e.g. when the driver has finished with a vb2 buffer) or marked as
> completed (e.g. for controls associated with a buffer). When all
> objects in the request are completed (or unbound), then the request
> fd will signal an exception (poll).
>
> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> ---
> drivers/media/Makefile | 3 +-
> drivers/media/media-device.c | 14 ++
> drivers/media/media-request.c | 407 ++++++++++++++++++++++++++++++++++
> include/media/media-device.h | 16 ++
> include/media/media-request.h | 244 ++++++++++++++++++++
> 5 files changed, 683 insertions(+), 1 deletion(-)
> create mode 100644 drivers/media/media-request.c
> create mode 100644 include/media/media-request.h
>
> diff --git a/drivers/media/Makefile b/drivers/media/Makefile
> index 594b462ddf0e..985d35ec6b29 100644
> --- a/drivers/media/Makefile
> +++ b/drivers/media/Makefile
> @@ -3,7 +3,8 @@
> # Makefile for the kernel multimedia device drivers.
> #
>
> -media-objs := media-device.o media-devnode.o media-entity.o
> +media-objs := media-device.o media-devnode.o media-entity.o \
> + media-request.o
>
> #
> # I2C drivers should come before other drivers, otherwise they'll fail
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 35e81f7c0d2f..bb6a64acd3f0 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -32,6 +32,7 @@
> #include <media/media-device.h>
> #include <media/media-devnode.h>
> #include <media/media-entity.h>
> +#include <media/media-request.h>
>
> #ifdef CONFIG_MEDIA_CONTROLLER
>
> @@ -366,6 +367,15 @@ static long media_device_get_topology(struct media_device *mdev,
> return ret;
> }
>
> +static long media_device_request_alloc(struct media_device *mdev,
> + struct media_request_alloc *alloc)
> +{
> + if (!mdev->ops || !mdev->ops->req_validate || !mdev->ops->req_queue)
> + return -ENOTTY;
> +
> + return media_request_alloc(mdev, alloc);
> +}
> +
> static long copy_arg_from_user(void *karg, void __user *uarg, unsigned int cmd)
> {
> /* All media IOCTLs are _IOWR() */
> @@ -414,6 +424,7 @@ static const struct media_ioctl_info ioctl_info[] = {
> MEDIA_IOC(ENUM_LINKS, media_device_enum_links, MEDIA_IOC_FL_GRAPH_MUTEX),
> MEDIA_IOC(SETUP_LINK, media_device_setup_link, MEDIA_IOC_FL_GRAPH_MUTEX),
> MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, MEDIA_IOC_FL_GRAPH_MUTEX),
> + MEDIA_IOC(REQUEST_ALLOC, media_device_request_alloc, 0),
> };
>
> static long media_device_ioctl(struct file *filp, unsigned int cmd,
> @@ -686,6 +697,8 @@ void media_device_init(struct media_device *mdev)
> INIT_LIST_HEAD(&mdev->pads);
> INIT_LIST_HEAD(&mdev->links);
> INIT_LIST_HEAD(&mdev->entity_notify);
> +
> + mutex_init(&mdev->req_queue_mutex);
> mutex_init(&mdev->graph_mutex);
> ida_init(&mdev->entity_internal_idx);
>
> @@ -699,6 +712,7 @@ void media_device_cleanup(struct media_device *mdev)
> mdev->entity_internal_idx_max = 0;
> media_graph_walk_cleanup(&mdev->pm_count_walk);
> mutex_destroy(&mdev->graph_mutex);
> + mutex_destroy(&mdev->req_queue_mutex);
> }
> EXPORT_SYMBOL_GPL(media_device_cleanup);
>
> diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c
> new file mode 100644
> index 000000000000..c216c4ab628b
> --- /dev/null
> +++ b/drivers/media/media-request.c
> @@ -0,0 +1,407 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Media device request objects
> + *
> + * Copyright 2018 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
> + * Copyright (C) 2018 Intel Corporation
> + * Copyright (C) 2018 Google, Inc.
> + *
> + * Author: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> + * Author: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> + */
> +
> +#include <linux/anon_inodes.h>
> +#include <linux/file.h>
> +
> +#include <media/media-device.h>
> +#include <media/media-request.h>
> +
> +static const char * const request_state[] = {
> + [MEDIA_REQUEST_STATE_IDLE] = "idle",
> + [MEDIA_REQUEST_STATE_VALIDATING] = "validating",
> + [MEDIA_REQUEST_STATE_QUEUED] = "queued",
> + [MEDIA_REQUEST_STATE_COMPLETE] = "complete",
> + [MEDIA_REQUEST_STATE_CLEANING] = "cleaning",
> +};
> +
> +static const char *
> +media_request_state_str(enum media_request_state state)
> +{
> + if (WARN_ON(state >= ARRAY_SIZE(request_state)))
> + return "invalid";
> + return request_state[state];
> +}
> +
> +static void media_request_clean(struct media_request *req)
> +{
> + struct media_request_object *obj, *obj_safe;
> +
> + WARN_ON(atomic_read(&req->state) != MEDIA_REQUEST_STATE_CLEANING);
> +
> + list_for_each_entry_safe(obj, obj_safe, &req->objects, list) {
> + media_request_object_unbind(obj);
> + media_request_object_put(obj);
> + }
> +
> + req->num_incomplete_objects = 0;
The number of incomplete objects should be already zero here. I'd think
that a different number would suggest that something has gone very wrong
and should be complained about. How about adding
WARN_ON(req->num_incomplete_objects) above this line?
> + wake_up_interruptible_all(&req->poll_wait);
> +}
> +
> +static void media_request_release(struct kref *kref)
> +{
> + struct media_request *req =
> + container_of(kref, struct media_request, kref);
> + struct media_device *mdev = req->mdev;
> +
> + dev_dbg(mdev->dev, "request: release %s\n", req->debug_str);
> +
> + atomic_set(&req->state, MEDIA_REQUEST_STATE_CLEANING);
> +
> + media_request_clean(req);
> +
> + if (mdev->ops->req_free)
> + mdev->ops->req_free(req);
> + else
> + kfree(req);
> +}
> +
> +void media_request_put(struct media_request *req)
> +{
> + kref_put(&req->kref, media_request_release);
> +}
> +EXPORT_SYMBOL_GPL(media_request_put);
> +
> +static int media_request_close(struct inode *inode, struct file *filp)
> +{
> + struct media_request *req = filp->private_data;
> +
> + media_request_put(req);
> + return 0;
> +}
> +
> +static unsigned int media_request_poll(struct file *filp,
> + struct poll_table_struct *wait)
> +{
> + struct media_request *req = filp->private_data;
> + unsigned long flags;
> + unsigned int ret = 0;
> + enum media_request_state state;
> +
> + if (!(poll_requested_events(wait) & POLLPRI))
> + return 0;
> +
> + spin_lock_irqsave(&req->lock, flags);
> + state = atomic_read(&req->state);
> +
> + if (state == MEDIA_REQUEST_STATE_COMPLETE) {
> + ret = POLLPRI;
> + goto unlock;
> + }
> + if (state != MEDIA_REQUEST_STATE_QUEUED) {
> + ret = POLLERR;
> + goto unlock;
> + }
> +
> + poll_wait(filp, &req->poll_wait, wait);
> +
> +unlock:
> + spin_unlock_irqrestore(&req->lock, flags);
> + return ret;
> +}
> +
> +static long media_request_ioctl_queue(struct media_request *req)
> +{
> + struct media_device *mdev = req->mdev;
> + enum media_request_state state;
> + unsigned long flags;
> + int ret = 0;
ret is unconditionally assigned below, no need to initialise here.
> +
> + dev_dbg(mdev->dev, "request: queue %s\n", req->debug_str);
> +
> + /*
> + * Ensure the request that is validated will be the one that gets queued
> + * next by serialising the queueing process. This mutex is also used
> + * to serialize with canceling a vb2 queue and with setting values such
> + * as controls in a request.
> + */
> + mutex_lock(&mdev->req_queue_mutex);
> +
> + spin_lock_irqsave(&req->lock, flags);
> + state = atomic_cmpxchg(&req->state, MEDIA_REQUEST_STATE_IDLE,
> + MEDIA_REQUEST_STATE_VALIDATING);
> + spin_unlock_irqrestore(&req->lock, flags);
> + if (state != MEDIA_REQUEST_STATE_IDLE) {
> + dev_dbg(mdev->dev,
> + "request: unable to queue %s, request in state %s\n",
> + req->debug_str, media_request_state_str(state));
> + mutex_unlock(&mdev->req_queue_mutex);
> + return -EBUSY;
> + }
> +
> + ret = mdev->ops->req_validate(req);
> +
> + /*
> + * If the req_validate was successful, then we mark the state as QUEUED
> + * and call req_queue. The reason we set the state first is that this
> + * allows req_queue to unbind or complete the queued objects in case
> + * they are immediately 'consumed'. State changes from QUEUED to another
> + * state can only happen if either the driver changes the state or if
> + * the user cancels the vb2 queue. The driver can only change the state
> + * after each object is queued through the req_queue op (and note that
> + * that op cannot fail), so setting the state to QUEUED up front is
> + * safe.
> + *
> + * The other reason for changing the state is if the vb2 queue is
> + * canceled, and that uses the req_queue_mutex which is still locked
> + * while req_queue is called, so that's safe as well.
> + */
> + atomic_set(&req->state,
> + ret ? MEDIA_REQUEST_STATE_IDLE : MEDIA_REQUEST_STATE_QUEUED);
> +
> + if (!ret)
> + mdev->ops->req_queue(req);
> +
> + mutex_unlock(&mdev->req_queue_mutex);
> +
> + if (ret)
> + dev_dbg(mdev->dev, "request: can't queue %s (%d)\n",
> + req->debug_str, ret);
> + else
> + media_request_get(req);
> +
> + return ret;
> +}
> +
> +static long media_request_ioctl_reinit(struct media_request *req)
> +{
> + struct media_device *mdev = req->mdev;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&req->lock, flags);
> + if (atomic_read(&req->state) != MEDIA_REQUEST_STATE_IDLE &&
> + atomic_read(&req->state) != MEDIA_REQUEST_STATE_COMPLETE) {
> + dev_dbg(mdev->dev,
> + "request: %s not in idle or complete state, cannot reinit\n",
> + req->debug_str);
> + spin_unlock_irqrestore(&req->lock, flags);
> + return -EBUSY;
> + }
> + atomic_set(&req->state, MEDIA_REQUEST_STATE_CLEANING);
> + spin_unlock_irqrestore(&req->lock, flags);
> +
> + media_request_clean(req);
> +
> + atomic_set(&req->state, MEDIA_REQUEST_STATE_IDLE);
> +
> + return 0;
> +}
> +
> +static long media_request_ioctl(struct file *filp, unsigned int cmd,
> + unsigned long arg)
> +{
> + struct media_request *req = filp->private_data;
> +
> + switch (cmd) {
> + case MEDIA_REQUEST_IOC_QUEUE:
> + return media_request_ioctl_queue(req);
> + case MEDIA_REQUEST_IOC_REINIT:
> + return media_request_ioctl_reinit(req);
> + default:
> + return -ENOIOCTLCMD;
> + }
> +}
> +
> +static const struct file_operations request_fops = {
> + .owner = THIS_MODULE,
> + .poll = media_request_poll,
> + .unlocked_ioctl = media_request_ioctl,
> + .release = media_request_close,
> +};
> +
> +int media_request_alloc(struct media_device *mdev,
> + struct media_request_alloc *alloc)
> +{
> + struct media_request *req;
> + struct file *filp;
> + char comm[TASK_COMM_LEN];
> + int fd;
> + int ret;
> +
> + /* Either both are NULL or both are non-NULL */
> + if (WARN_ON(!mdev->ops->req_alloc ^ !mdev->ops->req_free))
> + return -ENOMEM;
> +
> + fd = get_unused_fd_flags(O_CLOEXEC);
> + if (fd < 0)
> + return fd;
> +
> + filp = anon_inode_getfile("request", &request_fops, NULL, O_CLOEXEC);
> + if (IS_ERR(filp)) {
> + ret = PTR_ERR(filp);
> + goto err_put_fd;
> + }
> +
> + if (mdev->ops->req_alloc)
> + req = mdev->ops->req_alloc(mdev);
> + else
> + req = kzalloc(sizeof(*req), GFP_KERNEL);
> + if (!req) {
> + ret = -ENOMEM;
> + goto err_fput;
> + }
> +
> + filp->private_data = req;
> + req->mdev = mdev;
> + atomic_set(&req->state, MEDIA_REQUEST_STATE_IDLE);
> + req->num_incomplete_objects = 0;
> + kref_init(&req->kref);
> + INIT_LIST_HEAD(&req->objects);
> + spin_lock_init(&req->lock);
> + init_waitqueue_head(&req->poll_wait);
> +
> + alloc->fd = fd;
> +
> + get_task_comm(comm, current);
> + snprintf(req->debug_str, sizeof(req->debug_str), "%s:%d",
> + comm, fd);
> + dev_dbg(mdev->dev, "request: allocated %s\n", req->debug_str);
> +
> + fd_install(fd, filp);
> +
> + return 0;
> +
> +err_fput:
> + fput(filp);
> +
> +err_put_fd:
> + put_unused_fd(fd);
> +
> + return ret;
> +}
> +
> +static void media_request_object_release(struct kref *kref)
> +{
> + struct media_request_object *obj =
> + container_of(kref, struct media_request_object, kref);
> + struct media_request *req = obj->req;
> +
> + if (req)
> + media_request_object_unbind(obj);
> + obj->ops->release(obj);
> +}
> +
> +void media_request_object_put(struct media_request_object *obj)
> +{
> + kref_put(&obj->kref, media_request_object_release);
> +}
> +EXPORT_SYMBOL_GPL(media_request_object_put);
> +
> +void media_request_object_init(struct media_request_object *obj)
> +{
> + obj->ops = NULL;
> + obj->req = NULL;
> + obj->priv = NULL;
> + obj->completed = false;
> + INIT_LIST_HEAD(&obj->list);
> + kref_init(&obj->kref);
> +}
> +EXPORT_SYMBOL_GPL(media_request_object_init);
> +
> +int media_request_object_bind(struct media_request *req,
> + const struct media_request_object_ops *ops,
> + void *priv,
> + struct media_request_object *obj)
> +{
> + unsigned long flags;
> + int ret = -EBUSY;
> +
> + if (WARN_ON(!ops->release))
> + return -EPERM;
> +
> + obj->req = req;
> + obj->ops = ops;
> + obj->priv = priv;
> +
> + spin_lock_irqsave(&req->lock, flags);
> +
> + if (WARN_ON(atomic_read(&req->state) != MEDIA_REQUEST_STATE_IDLE))
> + goto unlock;
Is this worth a kernel warning, or rather how the drivers / other framework
bits (e.g. VB2) prevent user from binding objects to non-idle requests?
Even if you added a similar check to the caller, the request state could
well change in the meantime.
Perhaps add __must_check to the return value?
> +
> + list_add_tail(&obj->list, &req->objects);
> + req->num_incomplete_objects++;
> + ret = 0;
> +
> +unlock:
> + spin_unlock_irqrestore(&req->lock, flags);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(media_request_object_bind);
> +
> +void media_request_object_unbind(struct media_request_object *obj)
> +{
> + struct media_request *req = obj->req;
> + enum media_request_state state;
> + unsigned long flags;
> + bool completed = false;
> +
> + if (WARN_ON(!req))
> + return;
> +
> + spin_lock_irqsave(&req->lock, flags);
> + list_del(&obj->list);
> + obj->req = NULL;
> +
> + state = atomic_read(&req->state);
> +
> + if (state == MEDIA_REQUEST_STATE_COMPLETE ||
> + state == MEDIA_REQUEST_STATE_CLEANING)
> + goto unlock;
> +
> + if (WARN_ON(state == MEDIA_REQUEST_STATE_VALIDATING))
> + goto unlock;
> +
> + if (WARN_ON(!req->num_incomplete_objects))
> + goto unlock;
> +
> + req->num_incomplete_objects--;
> + if (state == MEDIA_REQUEST_STATE_QUEUED &&
> + !req->num_incomplete_objects) {
> + atomic_set(&req->state, MEDIA_REQUEST_STATE_COMPLETE);
> + completed = true;
> + wake_up_interruptible_all(&req->poll_wait);
> + }
> +
> +unlock:
> + spin_unlock_irqrestore(&req->lock, flags);
> + if (obj->ops->unbind)
> + obj->ops->unbind(obj);
> + if (completed)
> + media_request_put(req);
> +}
> +EXPORT_SYMBOL_GPL(media_request_object_unbind);
> +
> +void media_request_object_complete(struct media_request_object *obj)
> +{
> + struct media_request *req = obj->req;
> + unsigned long flags;
> + bool completed = false;
> +
> + spin_lock_irqsave(&req->lock, flags);
> + if (obj->completed)
> + goto unlock;
> + obj->completed = true;
> + if (WARN_ON(!req->num_incomplete_objects) ||
> + WARN_ON(atomic_read(&req->state) != MEDIA_REQUEST_STATE_QUEUED))
> + goto unlock;
> +
> + if (!--req->num_incomplete_objects) {
> + atomic_set(&req->state, MEDIA_REQUEST_STATE_COMPLETE);
> + wake_up_interruptible_all(&req->poll_wait);
> + completed = true;
> + }
> +unlock:
> + spin_unlock_irqrestore(&req->lock, flags);
> + if (completed)
> + media_request_put(req);
> +}
> +EXPORT_SYMBOL_GPL(media_request_object_complete);
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index bcc6ec434f1f..7d855823341c 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -27,6 +27,7 @@
>
> struct ida;
> struct device;
> +struct media_device;
>
> /**
> * struct media_entity_notify - Media Entity Notify
> @@ -50,10 +51,21 @@ struct media_entity_notify {
> * struct media_device_ops - Media device operations
> * @link_notify: Link state change notification callback. This callback is
> * called with the graph_mutex held.
> + * @req_alloc: Allocate a request
> + * @req_free: Free a request
> + * @req_validate: Validate a request, but do not queue yet
> + * @req_queue: Queue a validated request, cannot fail. If something goes
> + * wrong when queueing this request then it should be marked
> + * as such internally in the driver and any related buffers
> + * must eventually return to vb2 with state VB2_BUF_STATE_ERROR.
> */
> struct media_device_ops {
> int (*link_notify)(struct media_link *link, u32 flags,
> unsigned int notification);
> + struct media_request *(*req_alloc)(struct media_device *mdev);
> + void (*req_free)(struct media_request *req);
> + int (*req_validate)(struct media_request *req);
> + void (*req_queue)(struct media_request *req);
> };
>
> /**
> @@ -88,6 +100,8 @@ struct media_device_ops {
> * @disable_source: Disable Source Handler function pointer
> *
> * @ops: Operation handler callbacks
> + * @req_queue_mutex: Serialise the MEDIA_REQUEST_IOC_QUEUE ioctl w.r.t. this
> + * media device.
> *
> * This structure represents an abstract high-level media device. It allows easy
> * access to entities and provides basic media device-level support. The
> @@ -158,6 +172,8 @@ struct media_device {
> void (*disable_source)(struct media_entity *entity);
>
> const struct media_device_ops *ops;
> +
> + struct mutex req_queue_mutex;
> };
>
> /* We don't need to include pci.h or usb.h here */
> diff --git a/include/media/media-request.h b/include/media/media-request.h
> new file mode 100644
> index 000000000000..e39122dfd717
> --- /dev/null
> +++ b/include/media/media-request.h
> @@ -0,0 +1,244 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Media device request objects
> + *
> + * Copyright 2018 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
> + * Copyright (C) 2018 Intel Corporation
> + *
> + * Author: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> + * Author: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> + */
> +
> +#ifndef MEDIA_REQUEST_H
> +#define MEDIA_REQUEST_H
> +
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/atomic.h>
> +
> +#include <media/media-device.h>
> +
> +/**
> + * enum media_request_state - media request state
> + *
> + * @MEDIA_REQUEST_STATE_IDLE: Idle
> + * @MEDIA_REQUEST_STATE_VALIDATING: Validating the request, no state changes
> + * allowed
> + * @MEDIA_REQUEST_STATE_QUEUED: Queued
> + * @MEDIA_REQUEST_STATE_COMPLETE: Completed, the request is done
> + * @MEDIA_REQUEST_STATE_CLEANING: Cleaning, the request is being re-inited
> + */
> +enum media_request_state {
> + MEDIA_REQUEST_STATE_IDLE,
> + MEDIA_REQUEST_STATE_VALIDATING,
> + MEDIA_REQUEST_STATE_QUEUED,
> + MEDIA_REQUEST_STATE_COMPLETE,
> + MEDIA_REQUEST_STATE_CLEANING,
> +};
> +
> +struct media_request_object;
> +
> +/**
> + * struct media_request - Media device request
> + * @mdev: Media device this request belongs to
> + * @kref: Reference count
> + * @debug_str: Prefix for debug messages (process name:fd)
> + * @state: The state of the request
> + * @objects: List of @struct media_request_object request objects
> + * @num_objects: The number of objects in the request
> + * @num_incompleted_objects: The number of incomplete objects in the request
> + * @poll_wait: Wait queue for poll
> + * @lock: Serializes access to this struct
> + */
> +struct media_request {
> + struct media_device *mdev;
> + struct kref kref;
> + char debug_str[TASK_COMM_LEN + 11];
> + atomic_t state;
> + struct list_head objects;
> + unsigned int num_incomplete_objects;
> + struct wait_queue_head poll_wait;
> + spinlock_t lock;
> +};
> +
> +#ifdef CONFIG_MEDIA_CONTROLLER
> +
> +/**
> + * media_request_get - Get the media request
> + *
> + * @req: The request
> + *
> + * Get the media request.
> + */
> +static inline void media_request_get(struct media_request *req)
> +{
> + kref_get(&req->kref);
> +}
> +
> +/**
> + * media_request_put - Put the media request
> + *
> + * @req: The request
> + *
> + * Put the media request. The media request will be released
> + * when the refcount reaches 0.
> + */
> +void media_request_put(struct media_request *req);
> +
> +/**
> + * media_request_alloc - Allocate the media request
> + *
> + * @mdev: Media device this request belongs to
> + * @alloc: Store the request's file descriptor in this struct
> + *
> + * Allocated the media request and put the fd in @alloc->fd.
> + */
> +int media_request_alloc(struct media_device *mdev,
> + struct media_request_alloc *alloc);
> +
> +#else
> +
> +static inline void media_request_get(struct media_request *req)
> +{
> +}
> +
> +static inline void media_request_put(struct media_request *req)
> +{
> +}
> +
> +#endif
> +
> +/**
> + * struct media_request_object_ops - Media request object operations
> + * @prepare: Validate and prepare the request object, optional.
> + * @unprepare: Unprepare the request object, optional.
> + * @queue: Queue the request object, optional.
> + * @unbind: Unbind the request object, optional.
> + * @release: Release the request object, required.
> + */
> +struct media_request_object_ops {
> + int (*prepare)(struct media_request_object *object);
> + void (*unprepare)(struct media_request_object *object);
> + void (*queue)(struct media_request_object *object);
> + void (*unbind)(struct media_request_object *object);
> + void (*release)(struct media_request_object *object);
> +};
> +
> +/**
> + * struct media_request_object - An opaque object that belongs to a media
> + * request
> + *
> + * @ops: object's operations
> + * @priv: object's priv pointer
> + * @req: the request this object belongs to (can be NULL)
> + * @list: List entry of the object for @struct media_request
> + * @kref: Reference count of the object, acquire before releasing req->lock
> + * @completed: If true, then this object was completed.
> + *
> + * An object related to the request. This struct is embedded in the
> + * larger object data.
> + */
> +struct media_request_object {
> + const struct media_request_object_ops *ops;
> + void *priv;
> + struct media_request *req;
> + struct list_head list;
> + struct kref kref;
> + bool completed;
> +};
> +
> +#ifdef CONFIG_MEDIA_CONTROLLER
> +
> +/**
> + * media_request_object_get - Get a media request object
> + *
> + * @obj: The object
> + *
> + * Get a media request object.
> + */
> +static inline void media_request_object_get(struct media_request_object *obj)
> +{
> + kref_get(&obj->kref);
> +}
> +
> +/**
> + * media_request_object_put - Put a media request object
> + *
> + * @obj: The object
> + *
> + * Put a media request object. Once all references are gone, the
> + * object's memory is released.
> + */
> +void media_request_object_put(struct media_request_object *obj);
> +
> +/**
> + * media_request_object_init - Initialise a media request object
> + *
> + * Initialise a media request object. The object will be released using the
> + * release callback of the ops once it has no references (this function
> + * initialises references to one).
> + */
> +void media_request_object_init(struct media_request_object *obj);
> +
> +/**
> + * media_request_object_bind - Bind a media request object to a request
Argument documentation is missing.
I think you should also say that "every bound object must be unbound later
on".
> + */
> +int media_request_object_bind(struct media_request *req,
> + const struct media_request_object_ops *ops,
> + void *priv,
> + struct media_request_object *obj);
> +
> +/**
> + * media_request_object_unbind - Unbind a media request object
> + *
> + * @obj: The object
> + *
> + * Unbind the media request object from the request.
> + */
> +void media_request_object_unbind(struct media_request_object *obj);
> +
> +/**
> + * media_request_object_complete - Mark the media request object as complete
> + *
> + * @obj: The object
> + *
> + * Mark the media request object as complete.
Add:
Only bound request objects may be completed.
> + */
> +void media_request_object_complete(struct media_request_object *obj);
> +
> +#else
> +
> +static inline void media_request_object_get(struct media_request_object *obj)
> +{
> +}
> +
> +static inline void media_request_object_put(struct media_request_object *obj)
> +{
> +}
> +
> +static inline void media_request_object_init(struct media_request_object *obj)
> +{
> + obj->ops = NULL;
> + obj->req = NULL;
> +}
> +
> +static inline int media_request_object_bind(struct media_request *req,
> + const struct media_request_object_ops *ops,
> + void *priv,
> + struct media_request_object *obj)
> +{
> + return 0;
> +}
> +
> +static inline void media_request_object_unbind(struct media_request_object *obj)
> +{
> +}
> +
> +static inline void media_request_object_complete(struct media_request_object *obj)
> +{
> +}
> +
> +#endif
> +
> +#endif
--
Sakari Ailus
e-mail: sakari.ailus@xxxxxx
