On 10/06/2024 12:05, Sakari Ailus wrote:
> The list of file handles is needed to deliver media events as well as for
> other purposes in the future.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> ---
> drivers/media/mc/mc-device.c | 19 ++++++++++++++++++-
> drivers/media/mc/mc-devnode.c | 2 +-
> include/media/media-devnode.h | 4 +++-
> 3 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/mc/mc-device.c b/drivers/media/mc/mc-device.c
> index a9505ab4412d..46d1b0c9d8be 100644
> --- a/drivers/media/mc/mc-device.c
> +++ b/drivers/media/mc/mc-device.c
> @@ -45,8 +45,9 @@ static inline void __user *media_get_uptr(__u64 arg)
> return (void __user *)(uintptr_t)arg;
> }
>
> -static int media_device_open(struct file *filp)
> +static int media_device_open(struct media_devnode *devnode, struct file *filp)
> {
> + struct media_device *mdev = to_media_device(devnode);
> struct media_device_fh *fh;
>
> fh = kzalloc(sizeof(*fh), GFP_KERNEL);
> @@ -55,13 +56,23 @@ static int media_device_open(struct file *filp)
>
> filp->private_data = &fh->fh;
>
> + spin_lock_irq(&mdev->fh_list_lock);
> + list_add(&fh->mdev_list, &mdev->fh_list);
> + spin_unlock_irq(&mdev->fh_list_lock);
> +
> return 0;
> }
>
> static int media_device_close(struct file *filp)
> {
> + struct media_devnode *devnode = media_devnode_data(filp);
> + struct media_device *mdev = to_media_device(devnode);
> struct media_device_fh *fh = media_device_fh(filp);
>
> + spin_lock_irq(&mdev->fh_list_lock);
> + list_del(&fh->mdev_list);
> + spin_unlock_irq(&mdev->fh_list_lock);
> +
> kfree(fh);
>
> return 0;
> @@ -769,11 +780,13 @@ void media_device_init(struct media_device *mdev)
> INIT_LIST_HEAD(&mdev->pads);
> INIT_LIST_HEAD(&mdev->links);
> INIT_LIST_HEAD(&mdev->entity_notify);
> + INIT_LIST_HEAD(&mdev->fh_list);
>
> mutex_init(&mdev->req_queue_mutex);
> mutex_init(&mdev->graph_mutex);
> ida_init(&mdev->entity_internal_idx);
> atomic_set(&mdev->request_id, 0);
> + spin_lock_init(&mdev->fh_list_lock);
>
> mdev->devnode.release = media_device_release;
> media_devnode_init(&mdev->devnode);
> @@ -830,6 +843,10 @@ void media_device_unregister(struct media_device *mdev)
> if (!media_devnode_is_registered(&mdev->devnode))
> return;
>
> + spin_lock_irq(&mdev->fh_list_lock);
> + list_del_init(&mdev->fh_list);
> + spin_unlock_irq(&mdev->fh_list_lock);
Huh? This doesn't make sense to me. Unregistering the media device
makes no difference to the list of open filehandles.
> +
> device_remove_file(&mdev->devnode.dev, &dev_attr_model);
> dev_dbg(mdev->dev, "Media device unregistering\n");
> media_devnode_unregister(&mdev->devnode);
> diff --git a/drivers/media/mc/mc-devnode.c b/drivers/media/mc/mc-devnode.c
> index 26491daaba96..617156963911 100644
> --- a/drivers/media/mc/mc-devnode.c
> +++ b/drivers/media/mc/mc-devnode.c
> @@ -154,7 +154,7 @@ static int media_open(struct inode *inode, struct file *filp)
> get_device(&devnode->dev);
> mutex_unlock(&media_devnode_lock);
>
> - ret = devnode->fops->open(filp);
> + ret = devnode->fops->open(devnode, filp);
> if (ret) {
> put_device(&devnode->dev);
> return ret;
> diff --git a/include/media/media-devnode.h b/include/media/media-devnode.h
> index e4e8552598eb..898fa67ca090 100644
> --- a/include/media/media-devnode.h
> +++ b/include/media/media-devnode.h
> @@ -21,6 +21,8 @@
> #include <linux/device.h>
> #include <linux/cdev.h>
>
> +struct media_devnode;
> +
> /*
> * Flag to mark the media_devnode struct as registered. Drivers must not touch
> * this flag directly, it will be set and cleared by media_devnode_register and
> @@ -49,7 +51,7 @@ struct media_file_operations {
> __poll_t (*poll) (struct file *, struct poll_table_struct *);
> long (*ioctl) (struct file *, unsigned int, unsigned long);
> long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
> - int (*open) (struct file *);
> + int (*open) (struct media_devnode *, struct file *);
> int (*release) (struct file *);
> };
>
Regards,
Hans
