[PATCH 0/7] staging: most: fix issues of v4l2-aim

[PATCH 0/7] staging: most: fix issues of v4l2-aim

[Christian Gromm]

This patch set is needed to fix issues inside the video-4-linux AIM of
MOST driver.

Andrey Shvetsov (7):
  v4l2-aim: fix interrupt unsafe spinlocks
  v4l2-aim: optimize list_for_each_entry_safe
  v4l2-aim: assign unique names to devices
  v4l2-aim: replace pr_xx calls by v4l2_xx calls
  v4l2-aim: remove unnecessary retval
  v4l2-aim: remove unnecessary label err_vbi_dev
  v4l2-aim: remove unnecessary spaces

 drivers/staging/most/aim-v4l2/video.c | 117 ++++++++++++++++++----------------
 1 file changed, 62 insertions(+), 55 deletions(-)

-- 
1.9.1

[PATCH 1/7] staging: most: v4l2-aim: fix interrupt unsafe spinlocks

[Christian Gromm]

The functions get_aim_dev() and aim_rx_data() are using interrupt unsafe
spinlocks even though they may be called from an interrupt context.

This patch fixes the described problem.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-v4l2/video.c | 42 ++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/most/aim-v4l2/video.c b/drivers/staging/most/aim-v4l2/video.c
index 13abf7c..1fea839 100644
--- a/drivers/staging/most/aim-v4l2/video.c
+++ b/drivers/staging/most/aim-v4l2/video.c
@@ -139,15 +139,15 @@ static int aim_vdev_close(struct file *filp)
 	 * This must be implemented in core.
 	 */
 
-	spin_lock(&mdev->list_lock);
+	spin_lock_irq(&mdev->list_lock);
 	mdev->mute = true;
 	list_for_each_entry_safe(mbo, tmp, &mdev->pending_mbos, list) {
 		list_del(&mbo->list);
-		spin_unlock(&mdev->list_lock);
+		spin_unlock_irq(&mdev->list_lock);
 		most_put_mbo(mbo);
-		spin_lock(&mdev->list_lock);
+		spin_lock_irq(&mdev->list_lock);
 	}
-	spin_unlock(&mdev->list_lock);
+	spin_unlock_irq(&mdev->list_lock);
 	most_stop_channel(mdev->iface, mdev->ch_idx, &aim_info);
 	mdev->mute = false;
 
@@ -200,9 +200,9 @@ static ssize_t aim_vdev_read(struct file *filp, char __user *buf,
 
 		if (cnt >= rem) {
 			fh->offs = 0;
-			spin_lock(&mdev->list_lock);
+			spin_lock_irq(&mdev->list_lock);
 			list_del(&mbo->list);
-			spin_unlock(&mdev->list_lock);
+			spin_unlock_irq(&mdev->list_lock);
 			most_put_mbo(mbo);
 		}
 	}
@@ -394,34 +394,36 @@ static struct most_video_dev *get_aim_dev(
 	struct most_interface *iface, int channel_idx)
 {
 	struct most_video_dev *mdev, *tmp;
+	unsigned long flags;
 
-	spin_lock(&list_lock);
+	spin_lock_irqsave(&list_lock, flags);
 	list_for_each_entry_safe(mdev, tmp, &video_devices, list) {
 		if (mdev->iface == iface && mdev->ch_idx == channel_idx) {
-			spin_unlock(&list_lock);
+			spin_unlock_irqrestore(&list_lock, flags);
 			return mdev;
 		}
 	}
-	spin_unlock(&list_lock);
+	spin_unlock_irqrestore(&list_lock, flags);
 	return NULL;
 }
 
 static int aim_rx_data(struct mbo *mbo)
 {
+	unsigned long flags;
 	struct most_video_dev *mdev =
 		get_aim_dev(mbo->ifp, mbo->hdm_channel_id);
 
 	if (!mdev)
 		return -EIO;
 
-	spin_lock(&mdev->list_lock);
+	spin_lock_irqsave(&mdev->list_lock, flags);
 	if (unlikely(mdev->mute)) {
-		spin_unlock(&mdev->list_lock);
+		spin_unlock_irqrestore(&mdev->list_lock, flags);
 		return -EIO;
 	}
 
 	list_add_tail(&mbo->list, &mdev->pending_mbos);
-	spin_unlock(&mdev->list_lock);
+	spin_unlock_irqrestore(&mdev->list_lock, flags);
 	wake_up_interruptible(&mdev->wait_data);
 	return 0;
 }
@@ -529,9 +531,9 @@ static int aim_probe_channel(struct most_interface *iface, int channel_idx,
 	if (ret)
 		goto err_unreg;
 
-	spin_lock(&list_lock);
+	spin_lock_irq(&list_lock);
 	list_add(&mdev->list, &video_devices);
-	spin_unlock(&list_lock);
+	spin_unlock_irq(&list_lock);
 	return 0;
 
 err_unreg:
@@ -552,9 +554,9 @@ static int aim_disconnect_channel(struct most_interface *iface,
 		return -ENOENT;
 	}
 
-	spin_lock(&list_lock);
+	spin_lock_irq(&list_lock);
 	list_del(&mdev->list);
-	spin_unlock(&list_lock);
+	spin_unlock_irq(&list_lock);
 
 	aim_unregister_videodev(mdev);
 	v4l2_device_disconnect(&mdev->v4l2_dev);
@@ -585,17 +587,17 @@ static void __exit aim_exit(void)
 	 * we simulate this call here.
 	 * This must be fixed in core.
 	 */
-	spin_lock(&list_lock);
+	spin_lock_irq(&list_lock);
 	list_for_each_entry_safe(mdev, tmp, &video_devices, list) {
 		list_del(&mdev->list);
-		spin_unlock(&list_lock);
+		spin_unlock_irq(&list_lock);
 
 		aim_unregister_videodev(mdev);
 		v4l2_device_disconnect(&mdev->v4l2_dev);
 		v4l2_device_put(&mdev->v4l2_dev);
-		spin_lock(&list_lock);
+		spin_lock_irq(&list_lock);
 	}
-	spin_unlock(&list_lock);
+	spin_unlock_irq(&list_lock);
 
 	most_deregister_aim(&aim_info);
 	BUG_ON(!list_empty(&video_devices));
-- 
1.9.1

[PATCH 2/7] staging: most: v4l2-aim: optimize list_for_each_entry_safe

[Christian Gromm]

As the function get_aim_dev() does not delete elements of the list, the
use of macro list_for_each_entry_safe is not necessary.

This patch replaces the macro list_for_each_entry_safe with the macro
list_for_each_entry.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-v4l2/video.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/most/aim-v4l2/video.c b/drivers/staging/most/aim-v4l2/video.c
index 1fea839..6b7e220 100644
--- a/drivers/staging/most/aim-v4l2/video.c
+++ b/drivers/staging/most/aim-v4l2/video.c
@@ -393,11 +393,11 @@ static const struct video_device aim_videodev_template = {
 static struct most_video_dev *get_aim_dev(
 	struct most_interface *iface, int channel_idx)
 {
-	struct most_video_dev *mdev, *tmp;
+	struct most_video_dev *mdev;
 	unsigned long flags;
 
 	spin_lock_irqsave(&list_lock, flags);
-	list_for_each_entry_safe(mdev, tmp, &video_devices, list) {
+	list_for_each_entry(mdev, &video_devices, list) {
 		if (mdev->iface == iface && mdev->ch_idx == channel_idx) {
 			spin_unlock_irqrestore(&list_lock, flags);
 			return mdev;
-- 
1.9.1

[PATCH 3/7] staging: most: v4l2-aim: assign unique names to devices

[Christian Gromm]

The current V4L2 AIM implementation assigns to all video devices and to
all V4L devices the same names.

This patch makes use of hardware dependent names for the video devices and
allows the user to choose the names for the V4L devices.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-v4l2/video.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/most/aim-v4l2/video.c b/drivers/staging/most/aim-v4l2/video.c
index 6b7e220..9a383e0 100644
--- a/drivers/staging/most/aim-v4l2/video.c
+++ b/drivers/staging/most/aim-v4l2/video.c
@@ -259,7 +259,7 @@ static int vidioc_querycap(struct file *file, void  *priv,
 	pr_info("vidioc_querycap()\n");
 
 	strlcpy(cap->driver, "v4l2_most_aim", sizeof(cap->driver));
-	strlcpy(cap->card, "my_card", sizeof(cap->card));
+	strlcpy(cap->card, "MOST", sizeof(cap->card));
 	snprintf(cap->bus_info, sizeof(cap->bus_info),
 		 "%s", mdev->iface->description);
 
@@ -446,7 +446,8 @@ static int aim_register_videodev(struct most_video_dev *mdev)
 	*mdev->vdev = aim_videodev_template;
 	mdev->vdev->v4l2_dev = &mdev->v4l2_dev;
 	mdev->vdev->lock = &mdev->lock;
-	strcpy(mdev->vdev->name, "most v4l2 aim video");
+	snprintf(mdev->vdev->name, sizeof(mdev->vdev->name), "MOST: %s",
+		 mdev->v4l2_dev.name);
 
 	/* Register the v4l2 device */
 	video_set_drvdata(mdev->vdev, mdev);
@@ -518,8 +519,7 @@ static int aim_probe_channel(struct most_interface *iface, int channel_idx,
 	mdev->v4l2_dev.release = aim_v4l2_dev_release;
 
 	/* Create the v4l2_device */
-	strlcpy(mdev->v4l2_dev.name, "most_video_device",
-		sizeof(mdev->v4l2_dev.name));
+	strlcpy(mdev->v4l2_dev.name, name, sizeof(mdev->v4l2_dev.name));
 	ret = v4l2_device_register(NULL, &mdev->v4l2_dev);
 	if (ret) {
 		pr_err("v4l2_device_register() failed\n");
-- 
1.9.1

[PATCH 4/7] staging: most: v4l2-aim: replace pr_xx calls by v4l2_xx calls

[Christian Gromm]

This patch replaces pr_info, pr_err, etc. function calls by the
v4l2_... ones to get the device dependent logs.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-v4l2/video.c | 43 ++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/most/aim-v4l2/video.c b/drivers/staging/most/aim-v4l2/video.c
index 9a383e0..b55ab62 100644
--- a/drivers/staging/most/aim-v4l2/video.c
+++ b/drivers/staging/most/aim-v4l2/video.c
@@ -79,7 +79,7 @@ static int aim_vdev_open(struct file *filp)
 	struct most_video_dev *mdev = video_drvdata(filp);
 	struct aim_fh *fh;
 
-	pr_info("aim_vdev_open()\n");
+	v4l2_info(&mdev->v4l2_dev, "aim_vdev_open()\n");
 
 	switch (vdev->vfl_type) {
 	case VFL_TYPE_GRABBER:
@@ -93,7 +93,7 @@ static int aim_vdev_open(struct file *filp)
 		return -ENOMEM;
 
 	if (!atomic_inc_and_test(&mdev->access_ref)) {
-		pr_err("too many clients\n");
+		v4l2_err(&mdev->v4l2_dev, "too many clients\n");
 		ret = -EBUSY;
 		goto err_dec;
 	}
@@ -106,7 +106,7 @@ static int aim_vdev_open(struct file *filp)
 
 	ret = most_start_channel(mdev->iface, mdev->ch_idx, &aim_info);
 	if (ret) {
-		pr_err("most_start_channel() failed\n");
+		v4l2_err(&mdev->v4l2_dev, "most_start_channel() failed\n");
 		goto err_rm;
 	}
 
@@ -128,7 +128,7 @@ static int aim_vdev_close(struct file *filp)
 	struct most_video_dev *mdev = fh->mdev;
 	struct mbo *mbo, *tmp;
 
-	pr_info("aim_vdev_close()\n");
+	v4l2_info(&mdev->v4l2_dev, "aim_vdev_close()\n");
 
 	/*
 	 * We need to put MBOs back before we call most_stop_channel()
@@ -187,7 +187,7 @@ static ssize_t aim_vdev_read(struct file *filp, char __user *buf,
 		int const cnt = rem < count ? rem : count;
 
 		if (copy_to_user(buf, mbo->virt_address + fh->offs, cnt)) {
-			pr_err("read: copy_to_user failed\n");
+			v4l2_err(&mdev->v4l2_dev, "read: copy_to_user failed\n");
 			if (!ret)
 				ret = -EFAULT;
 			return ret;
@@ -256,7 +256,7 @@ static int vidioc_querycap(struct file *file, void  *priv,
 	struct aim_fh *fh = priv;
 	struct most_video_dev *mdev = fh->mdev;
 
-	pr_info("vidioc_querycap()\n");
+	v4l2_info(&mdev->v4l2_dev, "vidioc_querycap()\n");
 
 	strlcpy(cap->driver, "v4l2_most_aim", sizeof(cap->driver));
 	strlcpy(cap->card, "MOST", sizeof(cap->card));
@@ -273,7 +273,10 @@ static int vidioc_querycap(struct file *file, void  *priv,
 static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
 				   struct v4l2_fmtdesc *f)
 {
-	pr_info("vidioc_enum_fmt_vid_cap() %d\n", f->index);
+	struct aim_fh *fh = priv;
+	struct most_video_dev *mdev = fh->mdev;
+
+	v4l2_info(&mdev->v4l2_dev, "vidioc_enum_fmt_vid_cap() %d\n", f->index);
 
 	if (f->index)
 		return -EINVAL;
@@ -289,7 +292,10 @@ static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
 static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
 				struct v4l2_format *f)
 {
-	pr_info("vidioc_g_fmt_vid_cap()\n");
+	struct aim_fh *fh = priv;
+	struct most_video_dev *mdev = fh->mdev;
+
+	v4l2_info(&mdev->v4l2_dev, "vidioc_g_fmt_vid_cap()\n");
 
 	aim_set_format_struct(f);
 	return 0;
@@ -315,7 +321,10 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
 
 static int vidioc_g_std(struct file *file, void *priv, v4l2_std_id *norm)
 {
-	pr_info("vidioc_g_std()\n");
+	struct aim_fh *fh = priv;
+	struct most_video_dev *mdev = fh->mdev;
+
+	v4l2_info(&mdev->v4l2_dev, "vidioc_g_std()\n");
 
 	*norm = V4L2_STD_UNKNOWN;
 	return 0;
@@ -352,7 +361,7 @@ static int vidioc_s_input(struct file *file, void *priv, unsigned int index)
 	struct aim_fh *fh = priv;
 	struct most_video_dev *mdev = fh->mdev;
 
-	pr_info("vidioc_s_input(%d)\n", index);
+	v4l2_info(&mdev->v4l2_dev, "vidioc_s_input(%d)\n", index);
 
 	if (index >= V4L2_AIM_MAX_INPUT)
 		return -EINVAL;
@@ -433,7 +442,7 @@ static int aim_register_videodev(struct most_video_dev *mdev)
 	int retval = -ENOMEM;
 	int ret;
 
-	pr_info("aim_register_videodev()\n");
+	v4l2_info(&mdev->v4l2_dev, "aim_register_videodev()\n");
 
 	init_waitqueue_head(&mdev->wait_data);
 
@@ -453,7 +462,8 @@ static int aim_register_videodev(struct most_video_dev *mdev)
 	video_set_drvdata(mdev->vdev, mdev);
 	retval = video_register_device(mdev->vdev, VFL_TYPE_GRABBER, -1);
 	if (retval != 0) {
-		pr_err("video_register_device failed (%d)\n", retval);
+		v4l2_err(&mdev->v4l2_dev, "video_register_device failed (%d)\n",
+			 retval);
 		ret = -ENODEV;
 		goto err_vbi_dev;
 	}
@@ -467,7 +477,7 @@ err_vbi_dev:
 
 static void aim_unregister_videodev(struct most_video_dev *mdev)
 {
-	pr_info("aim_unregister_videodev()\n");
+	v4l2_info(&mdev->v4l2_dev, "aim_unregister_videodev()\n");
 
 	video_unregister_device(mdev->vdev);
 }
@@ -488,7 +498,7 @@ static int aim_probe_channel(struct most_interface *iface, int channel_idx,
 	int ret;
 	struct most_video_dev *mdev = get_aim_dev(iface, channel_idx);
 
-	pr_info("aim_probe_channel()\n");
+	pr_info("aim_probe_channel(%s)\n", name);
 
 	if (mdev) {
 		pr_err("channel already linked\n");
@@ -534,6 +544,7 @@ static int aim_probe_channel(struct most_interface *iface, int channel_idx,
 	spin_lock_irq(&list_lock);
 	list_add(&mdev->list, &video_devices);
 	spin_unlock_irq(&list_lock);
+	v4l2_info(&mdev->v4l2_dev, "aim_probe_channel() done\n");
 	return 0;
 
 err_unreg:
@@ -547,13 +558,13 @@ static int aim_disconnect_channel(struct most_interface *iface,
 {
 	struct most_video_dev *mdev = get_aim_dev(iface, channel_idx);
 
-	pr_info("aim_disconnect_channel()\n");
-
 	if (!mdev) {
 		pr_err("no such channel is linked\n");
 		return -ENOENT;
 	}
 
+	v4l2_info(&mdev->v4l2_dev, "aim_disconnect_channel()\n");
+
 	spin_lock_irq(&list_lock);
 	list_del(&mdev->list);
 	spin_unlock_irq(&list_lock);
-- 
1.9.1

[PATCH 5/7] staging: most: v4l2-aim: remove unnecessary retval

[Christian Gromm]

The function aim_register_videodev() uses the variables 'ret' and
'retval' to represent the same value.

This patch removes 'retval' and replaces it with 'ret'. Further, it
replaces the constant return value '-ENODEV' with the result returned by
function video_register_device() in the event something went wrong.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-v4l2/video.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/most/aim-v4l2/video.c b/drivers/staging/most/aim-v4l2/video.c
index b55ab62..e0fee88 100644
--- a/drivers/staging/most/aim-v4l2/video.c
+++ b/drivers/staging/most/aim-v4l2/video.c
@@ -439,7 +439,6 @@ static int aim_rx_data(struct mbo *mbo)
 
 static int aim_register_videodev(struct most_video_dev *mdev)
 {
-	int retval = -ENOMEM;
 	int ret;
 
 	v4l2_info(&mdev->v4l2_dev, "aim_register_videodev()\n");
@@ -460,11 +459,10 @@ static int aim_register_videodev(struct most_video_dev *mdev)
 
 	/* Register the v4l2 device */
 	video_set_drvdata(mdev->vdev, mdev);
-	retval = video_register_device(mdev->vdev, VFL_TYPE_GRABBER, -1);
-	if (retval != 0) {
+	ret = video_register_device(mdev->vdev, VFL_TYPE_GRABBER, -1);
+	if (ret) {
 		v4l2_err(&mdev->v4l2_dev, "video_register_device failed (%d)\n",
-			 retval);
-		ret = -ENODEV;
+			 ret);
 		goto err_vbi_dev;
 	}
 
-- 
1.9.1

[PATCH 6/7] staging: most: v4l2-aim: remove unnecessary label err_vbi_dev

[Christian Gromm]

For optimization purposes this patch removes the label err_vbi_dev.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-v4l2/video.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/staging/most/aim-v4l2/video.c b/drivers/staging/most/aim-v4l2/video.c
index e0fee88..0cca9ce 100644
--- a/drivers/staging/most/aim-v4l2/video.c
+++ b/drivers/staging/most/aim-v4l2/video.c
@@ -463,13 +463,9 @@ static int aim_register_videodev(struct most_video_dev *mdev)
 	if (ret) {
 		v4l2_err(&mdev->v4l2_dev, "video_register_device failed (%d)\n",
 			 ret);
-		goto err_vbi_dev;
+		video_device_release(mdev->vdev);
 	}
 
-	return 0;
-
-err_vbi_dev:
-	video_device_release(mdev->vdev);
 	return ret;
 }
 
-- 
1.9.1

[PATCH 7/7] staging: most: v4l2-aim: remove unnecessary spaces

[Christian Gromm]

This patch removes the unnecessary spaces.

Signed-off-by: Andrey Shvetsov <andrey.shvetsov@k2l.de>
Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-v4l2/video.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/most/aim-v4l2/video.c b/drivers/staging/most/aim-v4l2/video.c
index 0cca9ce..150dc89 100644
--- a/drivers/staging/most/aim-v4l2/video.c
+++ b/drivers/staging/most/aim-v4l2/video.c
@@ -250,7 +250,7 @@ static int aim_set_format(struct most_video_dev *mdev, unsigned int cmd,
 	return 0;
 }
 
-static int vidioc_querycap(struct file *file, void  *priv,
+static int vidioc_querycap(struct file *file, void *priv,
 			   struct v4l2_capability *cap)
 {
 	struct aim_fh *fh = priv;
@@ -270,7 +270,7 @@ static int vidioc_querycap(struct file *file, void  *priv,
 	return 0;
 }
 
-static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
+static int vidioc_enum_fmt_vid_cap(struct file *file, void *priv,
 				   struct v4l2_fmtdesc *f)
 {
 	struct aim_fh *fh = priv;
@@ -304,7 +304,7 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
 static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 				  struct v4l2_format *f)
 {
-	struct aim_fh *fh  = priv;
+	struct aim_fh *fh = priv;
 	struct most_video_dev *mdev = fh->mdev;
 
 	return aim_set_format(mdev, VIDIOC_TRY_FMT, f);
@@ -313,7 +313,7 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
 				struct v4l2_format *f)
 {
-	struct aim_fh *fh  = priv;
+	struct aim_fh *fh = priv;
 	struct most_video_dev *mdev = fh->mdev;
 
 	return aim_set_format(mdev, VIDIOC_S_FMT, f);
-- 
1.9.1