* [RFC PATCH 0/7] Simplify first open/last close checks
@ 2015-07-24 10:21 Hans Verkuil
2015-07-24 10:21 ` [RFC PATCH 1/7] v4l2-fh: change int to bool for v4l2_fh_is_singular(_file) Hans Verkuil
` (6 more replies)
0 siblings, 7 replies; 12+ messages in thread
From: Hans Verkuil @ 2015-07-24 10:21 UTC (permalink / raw)
To: linux-media; +Cc: sakari.ailus
From: Hans Verkuil <hans.verkuil@cisco.com>
This patch series makes a number of API improvements to simplify checking
if an open or close of a filehandle was the first or last.
Regards,
Hans
Hans Verkuil (7):
v4l2-fh: change int to bool for v4l2_fh_is_singular(_file)
v4l2-fh: v4l2_fh_add/del now return whether it was the first or last
fh.
v4l2-fh: add v4l2_fh_open_is_first and v4l2_fh_release_is_last
am437x-vpfe/fimc-capture: always return 0 on close
vb2: _vb2_fop_release returns if this was the last fh.
am437x/exynos4-is/marvell-ccic/sh_vou: simplify release()
cpia2/si470x: simplify open
drivers/media/platform/am437x/am437x-vpfe.c | 16 ++------
drivers/media/platform/exynos4-is/fimc-capture.c | 8 +---
drivers/media/platform/marvell-ccic/mcam-core.c | 5 +--
drivers/media/platform/sh_vou.c | 5 +--
drivers/media/radio/si470x/radio-si470x-i2c.c | 21 ++++-------
drivers/media/usb/cpia2/cpia2_v4l.c | 5 +--
drivers/media/v4l2-core/v4l2-fh.c | 41 +++++++++++++--------
drivers/media/v4l2-core/videobuf2-core.c | 7 ++--
include/media/v4l2-fh.h | 47 ++++++++++++++++++------
include/media/videobuf2-core.h | 2 +-
10 files changed, 82 insertions(+), 75 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC PATCH 1/7] v4l2-fh: change int to bool for v4l2_fh_is_singular(_file)
2015-07-24 10:21 [RFC PATCH 0/7] Simplify first open/last close checks Hans Verkuil
@ 2015-07-24 10:21 ` Hans Verkuil
2015-07-25 22:13 ` Sakari Ailus
2015-07-24 10:21 ` [RFC PATCH 2/7] v4l2-fh: v4l2_fh_add/del now return whether it was the first or last fh Hans Verkuil
` (5 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Hans Verkuil @ 2015-07-24 10:21 UTC (permalink / raw)
To: linux-media; +Cc: sakari.ailus, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
This function really returns a bool, so use that as the return type
instead of int.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/v4l2-core/v4l2-fh.c | 6 +++---
include/media/v4l2-fh.h | 10 +++++-----
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-fh.c b/drivers/media/v4l2-core/v4l2-fh.c
index c97067a..20c0a0c 100644
--- a/drivers/media/v4l2-core/v4l2-fh.c
+++ b/drivers/media/v4l2-core/v4l2-fh.c
@@ -110,13 +110,13 @@ int v4l2_fh_release(struct file *filp)
}
EXPORT_SYMBOL_GPL(v4l2_fh_release);
-int v4l2_fh_is_singular(struct v4l2_fh *fh)
+bool v4l2_fh_is_singular(struct v4l2_fh *fh)
{
unsigned long flags;
- int is_singular;
+ bool is_singular;
if (fh == NULL || fh->vdev == NULL)
- return 0;
+ return false;
spin_lock_irqsave(&fh->vdev->fh_lock, flags);
is_singular = list_is_singular(&fh->list);
spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
diff --git a/include/media/v4l2-fh.h b/include/media/v4l2-fh.h
index 8035167..13dcaae 100644
--- a/include/media/v4l2-fh.h
+++ b/include/media/v4l2-fh.h
@@ -91,15 +91,15 @@ void v4l2_fh_exit(struct v4l2_fh *fh);
*/
int v4l2_fh_release(struct file *filp);
/*
- * Returns 1 if this filehandle is the only filehandle opened for the
- * associated video_device. If fh is NULL, then it returns 0.
+ * Returns true if this filehandle is the only filehandle opened for the
+ * associated video_device. If fh is NULL, then it returns false.
*/
-int v4l2_fh_is_singular(struct v4l2_fh *fh);
+bool v4l2_fh_is_singular(struct v4l2_fh *fh);
/*
* Helper function with struct file as argument. If filp->private_data is
- * NULL, then it will return 0.
+ * NULL, then it will return false.
*/
-static inline int v4l2_fh_is_singular_file(struct file *filp)
+static inline bool v4l2_fh_is_singular_file(struct file *filp)
{
return v4l2_fh_is_singular(filp->private_data);
}
--
2.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RFC PATCH 2/7] v4l2-fh: v4l2_fh_add/del now return whether it was the first or last fh.
2015-07-24 10:21 [RFC PATCH 0/7] Simplify first open/last close checks Hans Verkuil
2015-07-24 10:21 ` [RFC PATCH 1/7] v4l2-fh: change int to bool for v4l2_fh_is_singular(_file) Hans Verkuil
@ 2015-07-24 10:21 ` Hans Verkuil
2015-07-25 22:13 ` Sakari Ailus
2015-07-24 10:21 ` [RFC PATCH 3/7] v4l2-fh: add v4l2_fh_open_is_first and v4l2_fh_release_is_last Hans Verkuil
` (4 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Hans Verkuil @ 2015-07-24 10:21 UTC (permalink / raw)
To: linux-media; +Cc: sakari.ailus, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
Simplify driver code by letting v4l2_fh_add/del return true if the
filehandle was the first open or the last close.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/v4l2-core/v4l2-fh.c | 10 ++++++++--
include/media/v4l2-fh.h | 10 ++++++----
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-fh.c b/drivers/media/v4l2-core/v4l2-fh.c
index 20c0a0c..325e378 100644
--- a/drivers/media/v4l2-core/v4l2-fh.c
+++ b/drivers/media/v4l2-core/v4l2-fh.c
@@ -52,14 +52,17 @@ void v4l2_fh_init(struct v4l2_fh *fh, struct video_device *vdev)
}
EXPORT_SYMBOL_GPL(v4l2_fh_init);
-void v4l2_fh_add(struct v4l2_fh *fh)
+bool v4l2_fh_add(struct v4l2_fh *fh)
{
unsigned long flags;
+ bool is_first;
v4l2_prio_open(fh->vdev->prio, &fh->prio);
spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+ is_first = list_empty(&fh->vdev->fh_list);
list_add(&fh->list, &fh->vdev->fh_list);
spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
+ return is_first;
}
EXPORT_SYMBOL_GPL(v4l2_fh_add);
@@ -77,14 +80,17 @@ int v4l2_fh_open(struct file *filp)
}
EXPORT_SYMBOL_GPL(v4l2_fh_open);
-void v4l2_fh_del(struct v4l2_fh *fh)
+bool v4l2_fh_del(struct v4l2_fh *fh)
{
unsigned long flags;
+ bool is_last;
spin_lock_irqsave(&fh->vdev->fh_lock, flags);
list_del_init(&fh->list);
+ is_last = list_empty(&fh->vdev->fh_list);
spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
v4l2_prio_close(fh->vdev->prio, fh->prio);
+ return is_last;
}
EXPORT_SYMBOL_GPL(v4l2_fh_del);
diff --git a/include/media/v4l2-fh.h b/include/media/v4l2-fh.h
index 13dcaae..17215fc 100644
--- a/include/media/v4l2-fh.h
+++ b/include/media/v4l2-fh.h
@@ -60,9 +60,10 @@ struct v4l2_fh {
void v4l2_fh_init(struct v4l2_fh *fh, struct video_device *vdev);
/*
* Add the fh to the list of file handles on a video_device. The file
- * handle must be initialised first.
+ * handle must be initialised first. This function will return true
+ * if the new filehandle was the first filehandle added and false otherwise.
*/
-void v4l2_fh_add(struct v4l2_fh *fh);
+bool v4l2_fh_add(struct v4l2_fh *fh);
/*
* Can be used as the open() op of v4l2_file_operations.
* It allocates a v4l2_fh and inits and adds it to the video_device associated
@@ -73,9 +74,10 @@ int v4l2_fh_open(struct file *filp);
* Remove file handle from the list of file handles. Must be called in
* v4l2_file_operations->release() handler if the driver uses v4l2_fh.
* On error filp->private_data will be NULL, otherwise it will point to
- * the v4l2_fh struct.
+ * the v4l2_fh struct. This function will return true if this was the
+ * last open filehandle and false otherwise.
*/
-void v4l2_fh_del(struct v4l2_fh *fh);
+bool v4l2_fh_del(struct v4l2_fh *fh);
/*
* Release resources related to a file handle. Parts of the V4L2
* framework using the v4l2_fh must release their resources here, too.
--
2.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RFC PATCH 3/7] v4l2-fh: add v4l2_fh_open_is_first and v4l2_fh_release_is_last
2015-07-24 10:21 [RFC PATCH 0/7] Simplify first open/last close checks Hans Verkuil
2015-07-24 10:21 ` [RFC PATCH 1/7] v4l2-fh: change int to bool for v4l2_fh_is_singular(_file) Hans Verkuil
2015-07-24 10:21 ` [RFC PATCH 2/7] v4l2-fh: v4l2_fh_add/del now return whether it was the first or last fh Hans Verkuil
@ 2015-07-24 10:21 ` Hans Verkuil
2015-07-25 22:42 ` Sakari Ailus
2015-07-24 10:21 ` [RFC PATCH 4/7] am437x-vpfe/fimc-capture: always return 0 on close Hans Verkuil
` (3 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Hans Verkuil @ 2015-07-24 10:21 UTC (permalink / raw)
To: linux-media; +Cc: sakari.ailus, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
Add new helper functions that report back if this was the first open
or last close.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/v4l2-core/v4l2-fh.c | 25 ++++++++++++++-----------
include/media/v4l2-fh.h | 27 +++++++++++++++++++++++++--
2 files changed, 39 insertions(+), 13 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-fh.c b/drivers/media/v4l2-core/v4l2-fh.c
index 325e378..ac9c028 100644
--- a/drivers/media/v4l2-core/v4l2-fh.c
+++ b/drivers/media/v4l2-core/v4l2-fh.c
@@ -66,19 +66,21 @@ bool v4l2_fh_add(struct v4l2_fh *fh)
}
EXPORT_SYMBOL_GPL(v4l2_fh_add);
-int v4l2_fh_open(struct file *filp)
+bool v4l2_fh_open_is_first(struct file *filp, int *err)
{
struct video_device *vdev = video_devdata(filp);
struct v4l2_fh *fh = kzalloc(sizeof(*fh), GFP_KERNEL);
+ *err = fh ? -ENOMEM : 0;
+
filp->private_data = fh;
- if (fh == NULL)
- return -ENOMEM;
- v4l2_fh_init(fh, vdev);
- v4l2_fh_add(fh);
- return 0;
+ if (fh) {
+ v4l2_fh_init(fh, vdev);
+ return v4l2_fh_add(fh);
+ }
+ return false;
}
-EXPORT_SYMBOL_GPL(v4l2_fh_open);
+EXPORT_SYMBOL_GPL(v4l2_fh_open_is_first);
bool v4l2_fh_del(struct v4l2_fh *fh)
{
@@ -103,18 +105,19 @@ void v4l2_fh_exit(struct v4l2_fh *fh)
}
EXPORT_SYMBOL_GPL(v4l2_fh_exit);
-int v4l2_fh_release(struct file *filp)
+bool v4l2_fh_release_is_last(struct file *filp)
{
struct v4l2_fh *fh = filp->private_data;
+ bool is_last = false;
if (fh) {
- v4l2_fh_del(fh);
+ is_last = v4l2_fh_del(fh);
v4l2_fh_exit(fh);
kfree(fh);
}
- return 0;
+ return is_last;
}
-EXPORT_SYMBOL_GPL(v4l2_fh_release);
+EXPORT_SYMBOL_GPL(v4l2_fh_release_is_last);
bool v4l2_fh_is_singular(struct v4l2_fh *fh)
{
diff --git a/include/media/v4l2-fh.h b/include/media/v4l2-fh.h
index 17215fc..e937866 100644
--- a/include/media/v4l2-fh.h
+++ b/include/media/v4l2-fh.h
@@ -65,11 +65,23 @@ void v4l2_fh_init(struct v4l2_fh *fh, struct video_device *vdev);
*/
bool v4l2_fh_add(struct v4l2_fh *fh);
/*
+ * It allocates a v4l2_fh and inits and adds it to the video_device associated
+ * with the file pointer. In addition it returns true if this was the first
+ * open and false otherwise. The error code is returned in *err.
+ */
+bool v4l2_fh_open_is_first(struct file *filp, int *err);
+/*
* Can be used as the open() op of v4l2_file_operations.
* It allocates a v4l2_fh and inits and adds it to the video_device associated
* with the file pointer.
*/
-int v4l2_fh_open(struct file *filp);
+static inline int v4l2_fh_open(struct file *filp)
+{
+ int err;
+
+ v4l2_fh_open_is_first(filp, &err);
+ return err;
+}
/*
* Remove file handle from the list of file handles. Must be called in
* v4l2_file_operations->release() handler if the driver uses v4l2_fh.
@@ -86,12 +98,23 @@ bool v4l2_fh_del(struct v4l2_fh *fh);
*/
void v4l2_fh_exit(struct v4l2_fh *fh);
/*
+ * It deletes and exits the v4l2_fh associated with the file pointer and
+ * frees it. It will do nothing if filp->private_data (the pointer to the
+ * v4l2_fh struct) is NULL. This function returns true if this was the
+ * last open file handle and false otherwise.
+ */
+bool v4l2_fh_release_is_last(struct file *filp);
+/*
* Can be used as the release() op of v4l2_file_operations.
* It deletes and exits the v4l2_fh associated with the file pointer and
* frees it. It will do nothing if filp->private_data (the pointer to the
* v4l2_fh struct) is NULL. This function always returns 0.
*/
-int v4l2_fh_release(struct file *filp);
+static inline int v4l2_fh_release(struct file *filp)
+{
+ v4l2_fh_release_is_last(filp);
+ return 0;
+}
/*
* Returns true if this filehandle is the only filehandle opened for the
* associated video_device. If fh is NULL, then it returns false.
--
2.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RFC PATCH 4/7] am437x-vpfe/fimc-capture: always return 0 on close
2015-07-24 10:21 [RFC PATCH 0/7] Simplify first open/last close checks Hans Verkuil
` (2 preceding siblings ...)
2015-07-24 10:21 ` [RFC PATCH 3/7] v4l2-fh: add v4l2_fh_open_is_first and v4l2_fh_release_is_last Hans Verkuil
@ 2015-07-24 10:21 ` Hans Verkuil
2015-07-24 10:21 ` [RFC PATCH 5/7] vb2: _vb2_fop_release returns if this was the last fh Hans Verkuil
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Hans Verkuil @ 2015-07-24 10:21 UTC (permalink / raw)
To: linux-media; +Cc: sakari.ailus, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
When the filehandle is closed always return 0 and ignore the return
code from _vb2_fop_release().
Currently _vb2_fop_release() always returns 0, but this will change in
the next patch where _vb2_fop_release() will return a boolean telling the
caller if this was the last open filehandle that is closed.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/platform/am437x/am437x-vpfe.c | 5 ++---
drivers/media/platform/exynos4-is/fimc-capture.c | 5 ++---
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c
index c8447fa..210c779 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -1187,7 +1187,6 @@ static int vpfe_release(struct file *file)
{
struct vpfe_device *vpfe = video_drvdata(file);
bool fh_singular;
- int ret;
mutex_lock(&vpfe->lock);
@@ -1195,7 +1194,7 @@ static int vpfe_release(struct file *file)
fh_singular = v4l2_fh_is_singular_file(file);
/* the release helper will cleanup any on-going streaming */
- ret = _vb2_fop_release(file, NULL);
+ _vb2_fop_release(file, NULL);
/*
* If this was the last open file.
@@ -1206,7 +1205,7 @@ static int vpfe_release(struct file *file)
mutex_unlock(&vpfe->lock);
- return ret;
+ return 0;
}
/*
diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
index cfebf29..f4458b0 100644
--- a/drivers/media/platform/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/exynos4-is/fimc-capture.c
@@ -538,7 +538,6 @@ static int fimc_capture_release(struct file *file)
struct fimc_dev *fimc = video_drvdata(file);
struct fimc_vid_cap *vc = &fimc->vid_cap;
bool close = v4l2_fh_is_singular_file(file);
- int ret;
dbg("pid: %d, state: 0x%lx", task_pid_nr(current), fimc->state);
@@ -549,7 +548,7 @@ static int fimc_capture_release(struct file *file)
vc->streaming = false;
}
- ret = _vb2_fop_release(file, NULL);
+ _vb2_fop_release(file, NULL);
if (close) {
clear_bit(ST_CAPT_BUSY, &fimc->state);
@@ -564,7 +563,7 @@ static int fimc_capture_release(struct file *file)
pm_runtime_put_sync(&fimc->pdev->dev);
mutex_unlock(&fimc->lock);
- return ret;
+ return 0;
}
static const struct v4l2_file_operations fimc_capture_fops = {
--
2.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RFC PATCH 5/7] vb2: _vb2_fop_release returns if this was the last fh.
2015-07-24 10:21 [RFC PATCH 0/7] Simplify first open/last close checks Hans Verkuil
` (3 preceding siblings ...)
2015-07-24 10:21 ` [RFC PATCH 4/7] am437x-vpfe/fimc-capture: always return 0 on close Hans Verkuil
@ 2015-07-24 10:21 ` Hans Verkuil
2015-07-24 10:21 ` [RFC PATCH 6/7] am437x/exynos4-is/marvell-ccic/sh_vou: simplify release() Hans Verkuil
2015-07-24 10:21 ` [RFC PATCH 7/7] cpia2/si470x: simplify open Hans Verkuil
6 siblings, 0 replies; 12+ messages in thread
From: Hans Verkuil @ 2015-07-24 10:21 UTC (permalink / raw)
To: linux-media; +Cc: sakari.ailus, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
_vb2_fop_release now returns true if this was the last open fh.
This will simplify drivers that call it and that need to check if it
was the last close.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/v4l2-core/videobuf2-core.c | 7 ++++---
include/media/videobuf2-core.h | 2 +-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index b866a6b..f348b8a 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -3452,7 +3452,7 @@ int vb2_fop_mmap(struct file *file, struct vm_area_struct *vma)
}
EXPORT_SYMBOL_GPL(vb2_fop_mmap);
-int _vb2_fop_release(struct file *file, struct mutex *lock)
+bool _vb2_fop_release(struct file *file, struct mutex *lock)
{
struct video_device *vdev = video_devdata(file);
@@ -3464,7 +3464,7 @@ int _vb2_fop_release(struct file *file, struct mutex *lock)
}
if (lock)
mutex_unlock(lock);
- return v4l2_fh_release(file);
+ return v4l2_fh_release_is_last(file);
}
EXPORT_SYMBOL_GPL(_vb2_fop_release);
@@ -3473,7 +3473,8 @@ int vb2_fop_release(struct file *file)
struct video_device *vdev = video_devdata(file);
struct mutex *lock = vdev->queue->lock ? vdev->queue->lock : vdev->lock;
- return _vb2_fop_release(file, lock);
+ _vb2_fop_release(file, lock);
+ return 0;
}
EXPORT_SYMBOL_GPL(vb2_fop_release);
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 22a44c2..7ed4c37 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -644,7 +644,7 @@ int vb2_ioctl_expbuf(struct file *file, void *priv,
int vb2_fop_mmap(struct file *file, struct vm_area_struct *vma);
int vb2_fop_release(struct file *file);
-int _vb2_fop_release(struct file *file, struct mutex *lock);
+bool _vb2_fop_release(struct file *file, struct mutex *lock);
ssize_t vb2_fop_write(struct file *file, const char __user *buf,
size_t count, loff_t *ppos);
ssize_t vb2_fop_read(struct file *file, char __user *buf,
--
2.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RFC PATCH 6/7] am437x/exynos4-is/marvell-ccic/sh_vou: simplify release()
2015-07-24 10:21 [RFC PATCH 0/7] Simplify first open/last close checks Hans Verkuil
` (4 preceding siblings ...)
2015-07-24 10:21 ` [RFC PATCH 5/7] vb2: _vb2_fop_release returns if this was the last fh Hans Verkuil
@ 2015-07-24 10:21 ` Hans Verkuil
2015-07-24 10:21 ` [RFC PATCH 7/7] cpia2/si470x: simplify open Hans Verkuil
6 siblings, 0 replies; 12+ messages in thread
From: Hans Verkuil @ 2015-07-24 10:21 UTC (permalink / raw)
To: linux-media; +Cc: sakari.ailus, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
Use the _vb2_fop_release() return code to determine if this was
the last close or not.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/platform/am437x/am437x-vpfe.c | 13 +++----------
drivers/media/platform/exynos4-is/fimc-capture.c | 5 +----
drivers/media/platform/marvell-ccic/mcam-core.c | 5 +----
drivers/media/platform/sh_vou.c | 5 +----
4 files changed, 6 insertions(+), 22 deletions(-)
diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c
index 210c779..14b9289 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -1186,21 +1186,14 @@ static int vpfe_initialize_device(struct vpfe_device *vpfe)
static int vpfe_release(struct file *file)
{
struct vpfe_device *vpfe = video_drvdata(file);
- bool fh_singular;
mutex_lock(&vpfe->lock);
- /* Save the singular status before we call the clean-up helper */
- fh_singular = v4l2_fh_is_singular_file(file);
-
- /* the release helper will cleanup any on-going streaming */
- _vb2_fop_release(file, NULL);
-
/*
- * If this was the last open file.
- * Then de-initialize hw module.
+ * The release helper will cleanup any on-going streaming.
+ * If this was the last open file, then de-initialize hw module.
*/
- if (fh_singular)
+ if (_vb2_fop_release(file, NULL))
vpfe_ccdc_close(&vpfe->ccdc, vpfe->pdev);
mutex_unlock(&vpfe->lock);
diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
index f4458b0..f654b1c 100644
--- a/drivers/media/platform/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/exynos4-is/fimc-capture.c
@@ -537,7 +537,6 @@ static int fimc_capture_release(struct file *file)
{
struct fimc_dev *fimc = video_drvdata(file);
struct fimc_vid_cap *vc = &fimc->vid_cap;
- bool close = v4l2_fh_is_singular_file(file);
dbg("pid: %d, state: 0x%lx", task_pid_nr(current), fimc->state);
@@ -548,9 +547,7 @@ static int fimc_capture_release(struct file *file)
vc->streaming = false;
}
- _vb2_fop_release(file, NULL);
-
- if (close) {
+ if (_vb2_fop_release(file, NULL)) {
clear_bit(ST_CAPT_BUSY, &fimc->state);
fimc_pipeline_call(&vc->ve, close);
clear_bit(ST_CAPT_SUSPENDED, &fimc->state);
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
index 5e2b4df..13d4f7f 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1631,12 +1631,9 @@ out:
static int mcam_v4l_release(struct file *filp)
{
struct mcam_camera *cam = video_drvdata(filp);
- bool last_open;
mutex_lock(&cam->s_mutex);
- last_open = v4l2_fh_is_singular_file(filp);
- _vb2_fop_release(filp, NULL);
- if (last_open) {
+ if (_vb2_fop_release(filp, NULL)) {
mcam_disable_mipi(cam);
mcam_ctlr_power_down(cam);
if (cam->buffer_mode == B_vmalloc && alloc_bufs_at_read)
diff --git a/drivers/media/platform/sh_vou.c b/drivers/media/platform/sh_vou.c
index fe5c8ab..da1d78c 100644
--- a/drivers/media/platform/sh_vou.c
+++ b/drivers/media/platform/sh_vou.c
@@ -1157,12 +1157,9 @@ done_open:
static int sh_vou_release(struct file *file)
{
struct sh_vou_device *vou_dev = video_drvdata(file);
- bool is_last;
mutex_lock(&vou_dev->fop_lock);
- is_last = v4l2_fh_is_singular_file(file);
- _vb2_fop_release(file, NULL);
- if (is_last) {
+ if (_vb2_fop_release(file, NULL)) {
/* Last close */
vou_dev->status = SH_VOU_INITIALISING;
sh_vou_reg_a_set(vou_dev, VOUER, 0, 0x101);
--
2.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RFC PATCH 7/7] cpia2/si470x: simplify open
2015-07-24 10:21 [RFC PATCH 0/7] Simplify first open/last close checks Hans Verkuil
` (5 preceding siblings ...)
2015-07-24 10:21 ` [RFC PATCH 6/7] am437x/exynos4-is/marvell-ccic/sh_vou: simplify release() Hans Verkuil
@ 2015-07-24 10:21 ` Hans Verkuil
6 siblings, 0 replies; 12+ messages in thread
From: Hans Verkuil @ 2015-07-24 10:21 UTC (permalink / raw)
To: linux-media; +Cc: sakari.ailus, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
With the new v4l2_fh_open_is_first function the check whether a file
open is the first is now much easier. Implement this in these two
drivers.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/radio/si470x/radio-si470x-i2c.c | 21 +++++++--------------
drivers/media/usb/cpia2/cpia2_v4l.c | 5 +----
2 files changed, 8 insertions(+), 18 deletions(-)
diff --git a/drivers/media/radio/si470x/radio-si470x-i2c.c b/drivers/media/radio/si470x/radio-si470x-i2c.c
index 471d6a8..aa05ffd 100644
--- a/drivers/media/radio/si470x/radio-si470x-i2c.c
+++ b/drivers/media/radio/si470x/radio-si470x-i2c.c
@@ -182,16 +182,15 @@ static int si470x_get_all_registers(struct si470x_device *radio)
int si470x_fops_open(struct file *file)
{
struct si470x_device *radio = video_drvdata(file);
- int retval = v4l2_fh_open(file);
+ int retval;
- if (retval)
- return retval;
-
- if (v4l2_fh_is_singular_file(file)) {
+ if (v4l2_fh_open_is_first(file, &retval)) {
/* start radio */
retval = si470x_start(radio);
- if (retval < 0)
- goto done;
+ if (retval < 0) {
+ v4l2_fh_release(file);
+ return retval;
+ }
/* enable RDS / STC interrupt */
radio->registers[SYSCONFIG1] |= SYSCONFIG1_RDSIEN;
@@ -200,10 +199,6 @@ int si470x_fops_open(struct file *file)
radio->registers[SYSCONFIG1] |= 0x1 << 2;
retval = si470x_set_register(radio, SYSCONFIG1);
}
-
-done:
- if (retval)
- v4l2_fh_release(file);
return retval;
}
@@ -215,11 +210,9 @@ int si470x_fops_release(struct file *file)
{
struct si470x_device *radio = video_drvdata(file);
- if (v4l2_fh_is_singular_file(file))
+ if (v4l2_fh_release_is_last(file))
/* stop radio */
si470x_stop(radio);
-
- return v4l2_fh_release(file);
}
diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c b/drivers/media/usb/cpia2/cpia2_v4l.c
index 9caea83..30f89ba 100644
--- a/drivers/media/usb/cpia2/cpia2_v4l.c
+++ b/drivers/media/usb/cpia2/cpia2_v4l.c
@@ -88,11 +88,8 @@ static int cpia2_open(struct file *file)
if (mutex_lock_interruptible(&cam->v4l2_lock))
return -ERESTARTSYS;
- retval = v4l2_fh_open(file);
- if (retval)
- goto open_unlock;
- if (v4l2_fh_is_singular_file(file)) {
+ if (v4l2_fh_open_is_first(file, &retval)) {
if (cpia2_allocate_buffers(cam)) {
v4l2_fh_release(file);
retval = -ENOMEM;
--
2.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 1/7] v4l2-fh: change int to bool for v4l2_fh_is_singular(_file)
2015-07-24 10:21 ` [RFC PATCH 1/7] v4l2-fh: change int to bool for v4l2_fh_is_singular(_file) Hans Verkuil
@ 2015-07-25 22:13 ` Sakari Ailus
0 siblings, 0 replies; 12+ messages in thread
From: Sakari Ailus @ 2015-07-25 22:13 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, sakari.ailus, Hans Verkuil
On Fri, Jul 24, 2015 at 12:21:30PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> This function really returns a bool, so use that as the return type
> instead of int.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
--
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 2/7] v4l2-fh: v4l2_fh_add/del now return whether it was the first or last fh.
2015-07-24 10:21 ` [RFC PATCH 2/7] v4l2-fh: v4l2_fh_add/del now return whether it was the first or last fh Hans Verkuil
@ 2015-07-25 22:13 ` Sakari Ailus
0 siblings, 0 replies; 12+ messages in thread
From: Sakari Ailus @ 2015-07-25 22:13 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, sakari.ailus, Hans Verkuil
On Fri, Jul 24, 2015 at 12:21:31PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Simplify driver code by letting v4l2_fh_add/del return true if the
> filehandle was the first open or the last close.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
--
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 3/7] v4l2-fh: add v4l2_fh_open_is_first and v4l2_fh_release_is_last
2015-07-24 10:21 ` [RFC PATCH 3/7] v4l2-fh: add v4l2_fh_open_is_first and v4l2_fh_release_is_last Hans Verkuil
@ 2015-07-25 22:42 ` Sakari Ailus
2015-07-28 10:16 ` Hans Verkuil
0 siblings, 1 reply; 12+ messages in thread
From: Sakari Ailus @ 2015-07-25 22:42 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, sakari.ailus, Hans Verkuil
Hi Hans,
On Fri, Jul 24, 2015 at 12:21:32PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Add new helper functions that report back if this was the first open
> or last close.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
...
> @@ -65,11 +65,23 @@ void v4l2_fh_init(struct v4l2_fh *fh, struct video_device *vdev);
> */
> bool v4l2_fh_add(struct v4l2_fh *fh);
> /*
> + * It allocates a v4l2_fh and inits and adds it to the video_device associated
> + * with the file pointer. In addition it returns true if this was the first
> + * open and false otherwise. The error code is returned in *err.
> + */
> +bool v4l2_fh_open_is_first(struct file *filp, int *err);
The new interface functions look a tad clumsy to me.
What would you think of returning the singularity value from v4l2_fh_open()
straight away? Negative integers are errors, so zero and positive values are
free.
A few drivers just check if the value is non-zero and then return that
value, but there are just a handful of those.
> +/*
> * Can be used as the open() op of v4l2_file_operations.
> * It allocates a v4l2_fh and inits and adds it to the video_device associated
> * with the file pointer.
> */
> -int v4l2_fh_open(struct file *filp);
> +static inline int v4l2_fh_open(struct file *filp)
> +{
> + int err;
> +
> + v4l2_fh_open_is_first(filp, &err);
> + return err;
> +}
> /*
> * Remove file handle from the list of file handles. Must be called in
> * v4l2_file_operations->release() handler if the driver uses v4l2_fh.
> @@ -86,12 +98,23 @@ bool v4l2_fh_del(struct v4l2_fh *fh);
> */
> void v4l2_fh_exit(struct v4l2_fh *fh);
> /*
> + * It deletes and exits the v4l2_fh associated with the file pointer and
> + * frees it. It will do nothing if filp->private_data (the pointer to the
> + * v4l2_fh struct) is NULL. This function returns true if this was the
> + * last open file handle and false otherwise.
> + */
> +bool v4l2_fh_release_is_last(struct file *filp);
> +/*
> * Can be used as the release() op of v4l2_file_operations.
> * It deletes and exits the v4l2_fh associated with the file pointer and
> * frees it. It will do nothing if filp->private_data (the pointer to the
> * v4l2_fh struct) is NULL. This function always returns 0.
> */
> -int v4l2_fh_release(struct file *filp);
> +static inline int v4l2_fh_release(struct file *filp)
> +{
> + v4l2_fh_release_is_last(filp);
> + return 0;
> +}
> /*
> * Returns true if this filehandle is the only filehandle opened for the
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 3/7] v4l2-fh: add v4l2_fh_open_is_first and v4l2_fh_release_is_last
2015-07-25 22:42 ` Sakari Ailus
@ 2015-07-28 10:16 ` Hans Verkuil
0 siblings, 0 replies; 12+ messages in thread
From: Hans Verkuil @ 2015-07-28 10:16 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, sakari.ailus, Hans Verkuil
Hi Sakari,
On 07/26/2015 12:42 AM, Sakari Ailus wrote:
> Hi Hans,
>
> On Fri, Jul 24, 2015 at 12:21:32PM +0200, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Add new helper functions that report back if this was the first open
>> or last close.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>
> ...
>
>> @@ -65,11 +65,23 @@ void v4l2_fh_init(struct v4l2_fh *fh, struct video_device *vdev);
>> */
>> bool v4l2_fh_add(struct v4l2_fh *fh);
>> /*
>> + * It allocates a v4l2_fh and inits and adds it to the video_device associated
>> + * with the file pointer. In addition it returns true if this was the first
>> + * open and false otherwise. The error code is returned in *err.
>> + */
>> +bool v4l2_fh_open_is_first(struct file *filp, int *err);
>
> The new interface functions look a tad clumsy to me.
>
> What would you think of returning the singularity value from v4l2_fh_open()
> straight away? Negative integers are errors, so zero and positive values are
> free.
>
> A few drivers just check if the value is non-zero and then return that
> value, but there are just a handful of those.
I don't like messing with that. It can be done because all driver opens go through
either v4l2-dev.c or v4l2-subdev.c and we can convert a return value of >0 to 0. We
have to do that since fs/open.c doesn't check for >0, just != 0.
But that makes our 'open' implementation non-standard, and I feel that that's
both confusing and increases the chances of future bugs (precisely because it is
unexpected).
In pretty much all open() implementations of drivers you will have a 'int err;'
variable or something similar, so being able to do:
if (v4l2_fh_open_is_first(file, &err)) {
}
is actually quite efficient (see patch 7/7).
Regards,
Hans
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-07-28 10:17 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-24 10:21 [RFC PATCH 0/7] Simplify first open/last close checks Hans Verkuil
2015-07-24 10:21 ` [RFC PATCH 1/7] v4l2-fh: change int to bool for v4l2_fh_is_singular(_file) Hans Verkuil
2015-07-25 22:13 ` Sakari Ailus
2015-07-24 10:21 ` [RFC PATCH 2/7] v4l2-fh: v4l2_fh_add/del now return whether it was the first or last fh Hans Verkuil
2015-07-25 22:13 ` Sakari Ailus
2015-07-24 10:21 ` [RFC PATCH 3/7] v4l2-fh: add v4l2_fh_open_is_first and v4l2_fh_release_is_last Hans Verkuil
2015-07-25 22:42 ` Sakari Ailus
2015-07-28 10:16 ` Hans Verkuil
2015-07-24 10:21 ` [RFC PATCH 4/7] am437x-vpfe/fimc-capture: always return 0 on close Hans Verkuil
2015-07-24 10:21 ` [RFC PATCH 5/7] vb2: _vb2_fop_release returns if this was the last fh Hans Verkuil
2015-07-24 10:21 ` [RFC PATCH 6/7] am437x/exynos4-is/marvell-ccic/sh_vou: simplify release() Hans Verkuil
2015-07-24 10:21 ` [RFC PATCH 7/7] cpia2/si470x: simplify open Hans Verkuil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).