[PATCH 0/4] media: tuner large grain locking

[PATCH 0/4] media: tuner large grain locking

[Shuah Khan]

Add a managed token resource that can be created at device level
which can be used as a large grain lock by diverse group of drivers
such as the media drivers that share a resource.

Token resource manages a unique named string resource which is
derived from common bus_name, and hardware address fields from
the struct device.

au0828 is changed to use token devres as a large locking
for exclusive access to tuner function. A new tuner_tkn
field is added to struct au0828_dev. Tuner token is created
from au0828 probe() and destroyed from disconnect().

Two new routines au0828_create_token_resources() and
au0828_destroy_token_resources() create and destroy the
tuner token.

au0828-dvb exports the tuner token to dvb-frontend when
it registers the digital frontend using the tuner_tkn
field in struct dvb_frontend.

au0828-video exports the tuner token to v4l2-core when
it registers the analog function using tuner_tkn field
in struct video_device.

Before this change:

- digital tv app disrupts an active analog app when it
  tries to use the tuner
  e.g:  tvtime analog video stream stops when kaffeine starts
- analog tv app disrupts another analog app when it tries to
  use the tuner
  e.g: tvtime audio glitches when xawtv starts and vice versa.
- analog tv app disrupts an active digital app when it tries
  to use the tuner
  e.g: kaffeine digital stream stops when tvtime starts
- digital tv app disrupts another digital tv app when it tries
  to use the tuner
  e.g: kaffeine digital stream stops when vlc starts and vice
  versa

After this change:
- digital tv app detects tuner is busy without disrupting
  the active app.
- analog tv app detects tuner is busy without disrupting
  the active analog app.
- analog tv app detects tuner is busy without disrupting
  the active digital app.
- digital tv app detects tuner is busy without disrupting
  the active digital app.

Requesting feedback on any use-cases I missed and overall
approach to solving the contention between media functions.

Shuah Khan (4):
  drivers/base: add managed token dev resource
  media: dvb-fe changes to use tuner token
  media: v4l2-core changes to use tuner token
  media: au0828 changes to use token devres for tuner access

 drivers/base/Makefile                   |    2 +-
 drivers/base/token_devres.c             |  134 +++++++++++++++++++++++++++++++
 drivers/media/dvb-core/dvb_frontend.c   |   21 +++++
 drivers/media/dvb-core/dvb_frontend.h   |    1 +
 drivers/media/usb/au0828/au0828-core.c  |   42 ++++++++++
 drivers/media/usb/au0828/au0828-dvb.c   |    1 +
 drivers/media/usb/au0828/au0828-video.c |    4 +
 drivers/media/usb/au0828/au0828.h       |    4 +
 drivers/media/v4l2-core/v4l2-dev.c      |   23 +++++-
 include/linux/token_devres.h            |   19 +++++
 include/media/v4l2-dev.h                |    1 +
 11 files changed, 250 insertions(+), 2 deletions(-)
 create mode 100644 drivers/base/token_devres.c
 create mode 100644 include/linux/token_devres.h

-- 
1.7.10.4

[PATCH 1/4] drivers/base: add managed token dev resource

[Shuah Khan]

Add a managed token resource that can be created at device level
which can be used as a large grain lock by diverse group of drivers
such as the media drivers that share a resource.

Media devices often have hardware resources that are shared
across several functions. These devices appear as a group of
independent devices. Each device implements a function which
could be shared by one or more functions supported by the same
device. For example, tuner is shared by analog and digital TV
functions.

Media drivers that control a single media TV stick are a
diversified group. Analog and digital TV function drivers have
to coordinate access to their shared functions.

Some media devices provide multiple almost-independent functions.
USB and PCI core aids in allowing multiple drivers to handle these
almost-independent functions. In this model, a media device could
have snd-usb-audio driving the audio function.

As a result, snd-usb-audio driver has to coordinate with the media
driver analog and digital function drivers.

A shared managed resource framework at drivers/base level will
allow a media device to be controlled by drivers that don't
fall under drivers/media and share functions with other media
drivers.

Token resource manages a unique named string resource which is
derived from common bus_name, and hardware address fields from
the struct device.

Interfaces:
    devm_token_create()
    devm_token_destroy()
    devm_token_lock()
    devm_token_unlock()
Usage:
    Create token: Call devm_token_create() with a token id string.
    Lock token: Call devm_token_lock() to lock or try lock a token.
    Unlock token: Call devm_token_unlock().
    Destroy token: Call devm_token_destroy() to delete the token.

Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
---
 drivers/base/Makefile        |    2 +-
 drivers/base/token_devres.c  |  134 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/token_devres.h |   19 ++++++
 3 files changed, 154 insertions(+), 1 deletion(-)
 create mode 100644 drivers/base/token_devres.c
 create mode 100644 include/linux/token_devres.h

diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 04b314e..924665b 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -4,7 +4,7 @@ obj-y			:= component.o core.o bus.o dd.o syscore.o \
 			   driver.o class.o platform.o \
 			   cpu.o firmware.o init.o map.o devres.o \
 			   attribute_container.o transport_class.o \
-			   topology.o container.o
+			   topology.o container.o token_devres.o
 obj-$(CONFIG_DEVTMPFS)	+= devtmpfs.o
 obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
 obj-y			+= power/
diff --git a/drivers/base/token_devres.c b/drivers/base/token_devres.c
new file mode 100644
index 0000000..86bcd25
--- /dev/null
+++ b/drivers/base/token_devres.c
@@ -0,0 +1,134 @@
+/*
+ * drivers/base/token_devres.c - managed token resource
+ *
+ * Copyright (c) 2014 Shuah Khan <shuah.kh@samsung.com>
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ *
+ * This file is released under the GPLv2.
+ */
+/*
+ * Media devices often have hardware resources that are shared
+ * across several functions. For instance, TV tuner cards often
+ * have MUXes, converters, radios, tuners, etc. that are shared
+ * across various functions. However, v4l2, alsa, DVB, usbfs, and
+ * all other drivers have no knowledge of what resources are
+ * shared. For example, users can't access DVB and alsa at the same
+ * time, or the DVB and V4L analog API at the same time, since many
+ * only have one converter that can be in either analog or digital
+ * mode. Accessing and/or changing mode of a converter while it is
+ * in use by another function results in video stream error.
+ *
+ * A shared devres that can be locked and unlocked by various drivers
+ * that control media functions on a single media device is needed to
+ * address the above problems.
+ *
+ * A token devres that can be looked up by a token for locking, try
+ * locking, unlocking will help avoid adding data structure
+ * dependencies between various media drivers. This token is a unique
+ * string that can be constructed from a common data structure such as
+ * struct device, bus_name, and hardware address.
+ *
+ * The devm_token_* interfaces manage access to token resource.
+ *
+ * Interfaces:
+ *	devm_token_create()
+ *	devm_token_destroy()
+ *	devm_token_lock()
+ *	devm_token_unlock()
+ * Usage:
+ *	Create token:
+ *		Call devm_token_create() with a token id which is
+ *		a unique string.
+ *	Lock token:
+ *		Call devm_token_lock() to lock or try lock a token.
+ *	Unlock token:
+ *		Call devm_token_unlock().
+ *	Destroy token:
+ *		Call devm_token_destroy() to delete the token.
+ *
+*/
+#include <linux/device.h>
+#include <linux/token_devres.h>
+
+struct token_devres {
+	struct mutex	lock;
+	char		id[];
+};
+
+static int devm_token_match(struct device *dev, void *res, void *data)
+{
+	struct token_devres *tkn = res;
+
+	/* compare the token data and return 1 if it matches */
+	return !strcmp(tkn->id, data);
+}
+
+static void devm_token_release(struct device *dev, void *res)
+{
+	struct token_devres *tkn = res;
+
+	mutex_destroy(&tkn->lock);
+}
+
+/* creates a token devres and marks it available */
+int devm_token_create(struct device *dev, const char *id)
+{
+	struct token_devres *tkn;
+	size_t tkn_size;
+
+	tkn_size = sizeof(struct token_devres) + strlen(id) + 1;
+	tkn = devres_alloc(devm_token_release, tkn_size, GFP_KERNEL);
+	if (!tkn)
+		return -ENOMEM;
+
+	strcpy(tkn->id, id);
+	mutex_init(&tkn->lock);
+
+	devres_add(dev, tkn);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devm_token_create);
+
+/* If token is available, lock it for the caller, If not return -EBUSY */
+int devm_token_lock(struct device *dev, const char *id)
+{
+	struct token_devres *tkn_ptr;
+
+	tkn_ptr = devres_find(dev, devm_token_release, devm_token_match,
+				(void *)id);
+	if (tkn_ptr == NULL)
+		return -ENODEV;
+
+	if (!mutex_trylock(&tkn_ptr->lock))
+		return -EBUSY;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devm_token_lock);
+
+/* If token is locked, unlock */
+int devm_token_unlock(struct device *dev, const char *id)
+{
+	struct token_devres *tkn_ptr;
+
+	tkn_ptr = devres_find(dev, devm_token_release, devm_token_match,
+				(void *) id);
+	if (tkn_ptr == NULL)
+		return -ENODEV;
+
+	mutex_unlock(&tkn_ptr->lock);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devm_token_unlock);
+
+/* destroy an existing token */
+int devm_token_destroy(struct device *dev, const char *id)
+{
+	int rc;
+
+	rc = devres_release(dev, devm_token_release, devm_token_match,
+				(void *) id);
+	WARN_ON(rc);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devm_token_destroy);
diff --git a/include/linux/token_devres.h b/include/linux/token_devres.h
new file mode 100644
index 0000000..e411fd5
--- /dev/null
+++ b/include/linux/token_devres.h
@@ -0,0 +1,19 @@
+/*
+ * token_devres.h - managed token resource
+ *
+ * Copyright (c) 2014 Shuah Khan <shuah.kh@samsung.com>
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ *
+ * This file is released under the GPLv2.
+ */
+#ifndef __LINUX_TOKEN_DEVRES_H
+#define __LINUX_TOKEN_DEVRES_H
+
+struct device;
+
+extern int devm_token_create(struct device *dev, const char *id);
+extern int devm_token_lock(struct device *dev, const char *id);
+extern int devm_token_unlock(struct device *dev, const char *id);
+extern int devm_token_destroy(struct device *dev, const char *id);
+
+#endif	/* __LINUX_TOKEN_DEVRES_H */
-- 
1.7.10.4

[PATCH 2/4] media: dvb-fe changes to use tuner token

[Shuah Khan]

Add a new field tuner_tkn to struct dvb_frontend. Drivers can
create tuner token using devm_token_create() and initialize
the tuner_tkn when frontend is registered with the dvb-core.
This change enables drivers to provide a token devres for tuner
access control.

Change dvb_frontend to lock tuner token for exclusive access to
tuner function for digital TV function use. When Tuner token is
present, dvb_frontend_start() calls devm_token_lock() to lock
the token. If token is busy, -EBUSY is returned to the user-space.
Tuner token is unlocked if kdvb adapter fe kthread fails to start.
This token is held in use as long as the kdvb adapter fe kthread
is running. Tuner token is unlocked in dvb_frontend_thread() when
kdvb adapter fe thread exits.

Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
---
 drivers/media/dvb-core/dvb_frontend.c |   21 +++++++++++++++++++++
 drivers/media/dvb-core/dvb_frontend.h |    1 +
 2 files changed, 22 insertions(+)

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 6cc2631..37d3a4e 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -41,6 +41,7 @@
 #include <linux/jiffies.h>
 #include <linux/kthread.h>
 #include <asm/processor.h>
+#include <linux/token_devres.h>
 
 #include "dvb_frontend.h"
 #include "dvbdev.h"
@@ -747,6 +748,11 @@ restart:
 	if (semheld)
 		up(&fepriv->sem);
 	dvb_frontend_wakeup(fe);
+
+	if (fe->tuner_tkn) {
+		devm_token_unlock(fe->dvb->device, fe->tuner_tkn);
+		dev_info(fe->dvb->device, "dvb: Tuner is unlocked\n");
+	}
 	return 0;
 }
 
@@ -840,6 +846,17 @@ static int dvb_frontend_start(struct dvb_frontend *fe)
 	fepriv->state = FESTATE_IDLE;
 	fepriv->exit = DVB_FE_NO_EXIT;
 	fepriv->thread = NULL;
+
+	if (fe->tuner_tkn) {
+		ret = devm_token_lock(fe->dvb->device, fe->tuner_tkn);
+		if (ret) {
+			dev_info(fe->dvb->device,
+				"dvb: Tuner is busy %d\n", ret);
+			return ret;
+		}
+		dev_info(fe->dvb->device, "dvb: Tuner is locked\n");
+	}
+
 	mb();
 
 	fe_thread = kthread_run(dvb_frontend_thread, fe,
@@ -850,6 +867,10 @@ static int dvb_frontend_start(struct dvb_frontend *fe)
 				"dvb_frontend_start: failed to start kthread (%d)\n",
 				ret);
 		up(&fepriv->sem);
+		if (fe->tuner_tkn) {
+			devm_token_unlock(fe->dvb->device, fe->tuner_tkn);
+			dev_info(fe->dvb->device, "dvb: Tuner is unlocked\n");
+		}
 		return ret;
 	}
 	fepriv->thread = fe_thread;
diff --git a/drivers/media/dvb-core/dvb_frontend.h b/drivers/media/dvb-core/dvb_frontend.h
index 371b6ca..c9ba5fd 100644
--- a/drivers/media/dvb-core/dvb_frontend.h
+++ b/drivers/media/dvb-core/dvb_frontend.h
@@ -418,6 +418,7 @@ struct dvb_frontend {
 #define DVB_FRONTEND_COMPONENT_DEMOD 1
 	int (*callback)(void *adapter_priv, int component, int cmd, int arg);
 	int id;
+	char *tuner_tkn;
 };
 
 extern int dvb_register_frontend(struct dvb_adapter *dvb,
-- 
1.7.10.4

[PATCH 3/4] media: v4l2-core changes to use tuner token

[Shuah Khan]

Add a new field tuner_tkn to struct video_device. Drivers can
create tuner token using devm_token_create() and initialize
the tuner_tkn when frontend is registered with the dvb-core.
This change enables drivers to provide a token devres for tuner
access control.

Change v4l2-core to lock tuner token for exclusive access to
tuner function for analog TV function use. When Tuner token is
present, v4l2_open() calls devm_token_lock() to lock the token.
If token is busy, -EBUSY is returned to the user-space.

Tuner token is unlocked in error paths in v4l2_open(). This token
is held as long as the v4l2 device is open and unlocked from
v4l2_release().

Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
---
 drivers/media/v4l2-core/v4l2-dev.c |   23 ++++++++++++++++++++++-
 include/media/v4l2-dev.h           |    1 +
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 634d863..8dff809 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -26,6 +26,7 @@
 #include <linux/kmod.h>
 #include <linux/slab.h>
 #include <asm/uaccess.h>
+#include <linux/token_devres.h>
 
 #include <media/v4l2-common.h>
 #include <media/v4l2-device.h>
@@ -445,6 +446,17 @@ static int v4l2_open(struct inode *inode, struct file *filp)
 		mutex_unlock(&videodev_lock);
 		return -ENODEV;
 	}
+	/* check if tuner is busy first */
+	if (vdev->tuner_tkn && vdev->dev_parent) {
+		ret = devm_token_lock(vdev->dev_parent, vdev->tuner_tkn);
+		if (ret) {
+			mutex_unlock(&videodev_lock);
+			dev_info(vdev->dev_parent, "v4l2: Tuner is busy\n");
+			return ret;
+		}
+		dev_info(vdev->dev_parent, "v4l2: Tuner is locked\n");
+	}
+
 	/* and increase the device refcount */
 	video_get(vdev);
 	mutex_unlock(&videodev_lock);
@@ -459,8 +471,13 @@ static int v4l2_open(struct inode *inode, struct file *filp)
 		printk(KERN_DEBUG "%s: open (%d)\n",
 			video_device_node_name(vdev), ret);
 	/* decrease the refcount in case of an error */
-	if (ret)
+	if (ret) {
 		video_put(vdev);
+		if (vdev->tuner_tkn && vdev->dev_parent) {
+			devm_token_unlock(vdev->dev_parent, vdev->tuner_tkn);
+			dev_info(vdev->dev_parent, "v4l2: Tuner is unlocked\n");
+		}
+	}
 	return ret;
 }
 
@@ -479,6 +496,10 @@ static int v4l2_release(struct inode *inode, struct file *filp)
 	/* decrease the refcount unconditionally since the release()
 	   return value is ignored. */
 	video_put(vdev);
+	if (vdev->tuner_tkn && vdev->dev_parent) {
+		devm_token_unlock(vdev->dev_parent, vdev->tuner_tkn);
+		dev_info(vdev->dev_parent, "v4l2: Tuner is unlocked\n");
+	}
 	return ret;
 }
 
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index eec6e46..1676349 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -141,6 +141,7 @@ struct video_device
 	/* serialization lock */
 	DECLARE_BITMAP(disable_locking, BASE_VIDIOC_PRIVATE);
 	struct mutex *lock;
+	char *tuner_tkn;
 };
 
 #define media_entity_to_video_device(__e) \
-- 
1.7.10.4

[PATCH 4/4] media: au0828 changes to use token devres for tuner access

[Shuah Khan]

au0828 is changed to use token devres as a large locking
for exclusive access to tuner function. A new tuner_tkn
field is added to struct au0828_dev. Tuner token is created
from au0828 probe() and destroyed from disconnect().

Two new routines au0828_create_token_resources() and
au0828_destroy_token_resources() create and destroy the
tuner token.

au0828-dvb exports the tuner token to dvb-frontend when
it registers the digital frontend using the tuner_tkn
field in struct dvb_frontend.

au0828-video exports the tuner token to v4l2-core when
it registers the analog function using tuner_tkn field
in struct video_device.

Before this change:

- digital tv app disrupts an active analog app when it
  tries to use the tuner
  e.g:  tvtime analog video stream stops when kaffeine starts
- analog tv app disrupts another analog app when it tries to
  use the tuner
  e.g: tvtime audio glitches when xawtv starts and vice versa.
- analog tv app disrupts an active digital app when it tries
  to use the tuner
  e.g: kaffeine digital stream stops when tvtime starts
- digital tv app disrupts another digital tv app when it tries
  to use the tuner
  e.g: kaffeine digital stream stops when vlc starts and vice
  versa

After this change:
- digital tv app detects tuner is busy without disrupting
  the active app.
- analog tv app detects tuner is busy without disrupting
  the active analog app.
- analog tv app detects tuner is busy without disrupting
  the active digital app.
- digital tv app detects tuner is busy without disrupting
  the active digital app.

Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
---
 drivers/media/usb/au0828/au0828-core.c  |   42 +++++++++++++++++++++++++++++++
 drivers/media/usb/au0828/au0828-dvb.c   |    1 +
 drivers/media/usb/au0828/au0828-video.c |    4 +++
 drivers/media/usb/au0828/au0828.h       |    4 +++
 4 files changed, 51 insertions(+)

diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c
index ab45a6f..df04a99 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -125,6 +125,37 @@ static int recv_control_msg(struct au0828_dev *dev, u16 request, u32 value,
 	return status;
 }
 
+/* interfaces to create and destroy token resources */
+static int au0828_create_token_resources(struct au0828_dev *dev)
+{
+	int rc = 0, len;
+	char buf[64];
+
+	/* create token devres for tuner */
+	len = snprintf(buf, sizeof(buf),
+		"tuner:%s-%s-%d",
+		dev_name(&dev->usbdev->dev),
+		dev->usbdev->bus->bus_name,
+		dev->board.tuner_addr);
+
+	dev->tuner_tkn = devm_kzalloc(&dev->usbdev->dev, len + 1, GFP_KERNEL);
+	if (!dev->tuner_tkn)
+		return -ENOMEM;
+
+	strcpy(dev->tuner_tkn, buf);
+	rc = devm_token_create(&dev->usbdev->dev, dev->tuner_tkn);
+	if (rc)
+		return rc;
+
+	pr_info("au0828_create_token_resources(): Tuner token created\n");
+	return rc;
+}
+
+static void au0828_destroy_token_resources(struct au0828_dev *dev)
+{
+	devm_token_destroy(&dev->usbdev->dev, dev->tuner_tkn);
+}
+
 static void au0828_usb_release(struct au0828_dev *dev)
 {
 	/* I2C */
@@ -154,6 +185,8 @@ static void au0828_usb_disconnect(struct usb_interface *interface)
 	/* Digital TV */
 	au0828_dvb_unregister(dev);
 
+	au0828_destroy_token_resources(dev);
+
 	usb_set_intfdata(interface, NULL);
 	mutex_lock(&dev->mutex);
 	dev->usbdev = NULL;
@@ -213,6 +246,13 @@ static int au0828_usb_probe(struct usb_interface *interface,
 	dev->usbdev = usbdev;
 	dev->boardnr = id->driver_info;
 
+	/* create token resources */
+	if (au0828_create_token_resources(dev)) {
+		mutex_unlock(&dev->lock);
+		kfree(dev);
+		return -ENOMEM;
+	}
+
 #ifdef CONFIG_VIDEO_AU0828_V4L2
 	dev->v4l2_dev.release = au0828_usb_v4l2_release;
 
@@ -221,6 +261,7 @@ static int au0828_usb_probe(struct usb_interface *interface,
 	if (retval) {
 		pr_err("%s() v4l2_device_register failed\n",
 		       __func__);
+		au0828_destroy_token_resources(dev);
 		mutex_unlock(&dev->lock);
 		kfree(dev);
 		return retval;
@@ -230,6 +271,7 @@ static int au0828_usb_probe(struct usb_interface *interface,
 	if (retval) {
 		pr_err("%s() v4l2_ctrl_handler_init failed\n",
 		       __func__);
+		au0828_destroy_token_resources(dev);
 		mutex_unlock(&dev->lock);
 		kfree(dev);
 		return retval;
diff --git a/drivers/media/usb/au0828/au0828-dvb.c b/drivers/media/usb/au0828/au0828-dvb.c
index d8b5d94..1195d29 100755
--- a/drivers/media/usb/au0828/au0828-dvb.c
+++ b/drivers/media/usb/au0828/au0828-dvb.c
@@ -412,6 +412,7 @@ static int dvb_register(struct au0828_dev *dev)
 		goto fail_adapter;
 	}
 	dvb->adapter.priv = dev;
+	dvb->frontend->tuner_tkn = dev->tuner_tkn;
 
 	/* register frontend */
 	result = dvb_register_frontend(&dvb->adapter, dvb->frontend);
diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index 385894a..0b50bda 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -2018,6 +2018,9 @@ int au0828_analog_register(struct au0828_dev *dev,
 	dev->vdev->lock = &dev->lock;
 	set_bit(V4L2_FL_USE_FH_PRIO, &dev->vdev->flags);
 	strcpy(dev->vdev->name, "au0828a video");
+	/* is there another way v4l2 core can get struct device ?? */
+	dev->vdev->dev_parent = &dev->usbdev->dev;
+	dev->vdev->tuner_tkn = dev->tuner_tkn;
 
 	/* Setup the VBI device */
 	*dev->vbi_dev = au0828_video_template;
@@ -2025,6 +2028,7 @@ int au0828_analog_register(struct au0828_dev *dev,
 	dev->vbi_dev->lock = &dev->lock;
 	set_bit(V4L2_FL_USE_FH_PRIO, &dev->vbi_dev->flags);
 	strcpy(dev->vbi_dev->name, "au0828a vbi");
+	dev->vbi_dev->tuner_tkn = dev->tuner_tkn;
 
 	/* Register the v4l2 device */
 	video_set_drvdata(dev->vdev, dev);
diff --git a/drivers/media/usb/au0828/au0828.h b/drivers/media/usb/au0828/au0828.h
index 7112b9d..11bc933 100644
--- a/drivers/media/usb/au0828/au0828.h
+++ b/drivers/media/usb/au0828/au0828.h
@@ -23,6 +23,7 @@
 #include <linux/i2c.h>
 #include <linux/i2c-algo-bit.h>
 #include <media/tveeprom.h>
+#include <linux/token_devres.h>
 
 /* Analog */
 #include <linux/videodev2.h>
@@ -198,6 +199,9 @@ struct au0828_dev {
 	struct au0828_board	board;
 	u8			ctrlmsg[64];
 
+	/* token resources */
+	char *tuner_tkn; /* tuner token id */
+
 	/* I2C */
 	struct i2c_adapter		i2c_adap;
 	struct i2c_algorithm		i2c_algo;
-- 
1.7.10.4

Re: [PATCH 3/4] media: v4l2-core changes to use tuner token

[Hans Verkuil]

Hi Shuah,

On 06/25/2014 01:57 AM, Shuah Khan wrote:
> Add a new field tuner_tkn to struct video_device. Drivers can
> create tuner token using devm_token_create() and initialize
> the tuner_tkn when frontend is registered with the dvb-core.
> This change enables drivers to provide a token devres for tuner
> access control.
>
> Change v4l2-core to lock tuner token for exclusive access to
> tuner function for analog TV function use. When Tuner token is
> present, v4l2_open() calls devm_token_lock() to lock the token.
> If token is busy, -EBUSY is returned to the user-space.
>
> Tuner token is unlocked in error paths in v4l2_open(). This token
> is held as long as the v4l2 device is open and unlocked from
> v4l2_release().
>
> Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
> ---
>   drivers/media/v4l2-core/v4l2-dev.c |   23 ++++++++++++++++++++++-
>   include/media/v4l2-dev.h           |    1 +
>   2 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index 634d863..8dff809 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -26,6 +26,7 @@
>   #include <linux/kmod.h>
>   #include <linux/slab.h>
>   #include <asm/uaccess.h>
> +#include <linux/token_devres.h>
>
>   #include <media/v4l2-common.h>
>   #include <media/v4l2-device.h>
> @@ -445,6 +446,17 @@ static int v4l2_open(struct inode *inode, struct file *filp)
>   		mutex_unlock(&videodev_lock);
>   		return -ENODEV;
>   	}
> +	/* check if tuner is busy first */
> +	if (vdev->tuner_tkn && vdev->dev_parent) {
> +		ret = devm_token_lock(vdev->dev_parent, vdev->tuner_tkn);
> +		if (ret) {
> +			mutex_unlock(&videodev_lock);
> +			dev_info(vdev->dev_parent, "v4l2: Tuner is busy\n");
> +			return ret;
> +		}

If I understand this correctly, then this will make it impossible to open
the same video node twice since devm_token_lock will return EBUSY the
second time. Ditto for DVB as far as I could tell. That is certainly not
what you want, at least not for V4L.

In V4L2 the rules for tuner ownership are complicated, especially if the
tuner has a radio mode as well, see this presentation (the last two slides):

http://linuxtv.org/downloads/presentations/media_ws_2012_EU/ambiguities2.odp

Regards,

	Hans

> +		dev_info(vdev->dev_parent, "v4l2: Tuner is locked\n");
> +	}
> +
>   	/* and increase the device refcount */
>   	video_get(vdev);
>   	mutex_unlock(&videodev_lock);
> @@ -459,8 +471,13 @@ static int v4l2_open(struct inode *inode, struct file *filp)
>   		printk(KERN_DEBUG "%s: open (%d)\n",
>   			video_device_node_name(vdev), ret);
>   	/* decrease the refcount in case of an error */
> -	if (ret)
> +	if (ret) {
>   		video_put(vdev);
> +		if (vdev->tuner_tkn && vdev->dev_parent) {
> +			devm_token_unlock(vdev->dev_parent, vdev->tuner_tkn);
> +			dev_info(vdev->dev_parent, "v4l2: Tuner is unlocked\n");
> +		}
> +	}
>   	return ret;
>   }
>
> @@ -479,6 +496,10 @@ static int v4l2_release(struct inode *inode, struct file *filp)
>   	/* decrease the refcount unconditionally since the release()
>   	   return value is ignored. */
>   	video_put(vdev);
> +	if (vdev->tuner_tkn && vdev->dev_parent) {
> +		devm_token_unlock(vdev->dev_parent, vdev->tuner_tkn);
> +		dev_info(vdev->dev_parent, "v4l2: Tuner is unlocked\n");
> +	}
>   	return ret;
>   }
>
> diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
> index eec6e46..1676349 100644
> --- a/include/media/v4l2-dev.h
> +++ b/include/media/v4l2-dev.h
> @@ -141,6 +141,7 @@ struct video_device
>   	/* serialization lock */
>   	DECLARE_BITMAP(disable_locking, BASE_VIDIOC_PRIVATE);
>   	struct mutex *lock;
> +	char *tuner_tkn;
>   };
>
>   #define media_entity_to_video_device(__e) \
>

Re: [PATCH 4/4] media: au0828 changes to use token devres for tuner access

[Hans Verkuil]

Hi Shuah,

On 06/25/2014 01:57 AM, Shuah Khan wrote:
> au0828 is changed to use token devres as a large locking
> for exclusive access to tuner function. A new tuner_tkn
> field is added to struct au0828_dev. Tuner token is created
> from au0828 probe() and destroyed from disconnect().
>
> Two new routines au0828_create_token_resources() and
> au0828_destroy_token_resources() create and destroy the
> tuner token.
>
> au0828-dvb exports the tuner token to dvb-frontend when
> it registers the digital frontend using the tuner_tkn
> field in struct dvb_frontend.
>
> au0828-video exports the tuner token to v4l2-core when
> it registers the analog function using tuner_tkn field
> in struct video_device.
>
> Before this change:
>
> - digital tv app disrupts an active analog app when it
>    tries to use the tuner
>    e.g:  tvtime analog video stream stops when kaffeine starts
> - analog tv app disrupts another analog app when it tries to
>    use the tuner
>    e.g: tvtime audio glitches when xawtv starts and vice versa.
> - analog tv app disrupts an active digital app when it tries
>    to use the tuner
>    e.g: kaffeine digital stream stops when tvtime starts
> - digital tv app disrupts another digital tv app when it tries
>    to use the tuner
>    e.g: kaffeine digital stream stops when vlc starts and vice
>    versa
>
> After this change:
> - digital tv app detects tuner is busy without disrupting
>    the active app.
> - analog tv app detects tuner is busy without disrupting
>    the active analog app.
> - analog tv app detects tuner is busy without disrupting
>    the active digital app.
> - digital tv app detects tuner is busy without disrupting
>    the active digital app.
>
> Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
> ---
>   drivers/media/usb/au0828/au0828-core.c  |   42 +++++++++++++++++++++++++++++++
>   drivers/media/usb/au0828/au0828-dvb.c   |    1 +
>   drivers/media/usb/au0828/au0828-video.c |    4 +++
>   drivers/media/usb/au0828/au0828.h       |    4 +++
>   4 files changed, 51 insertions(+)
>
> diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c
> index ab45a6f..df04a99 100644
> --- a/drivers/media/usb/au0828/au0828-core.c
> +++ b/drivers/media/usb/au0828/au0828-core.c
> @@ -125,6 +125,37 @@ static int recv_control_msg(struct au0828_dev *dev, u16 request, u32 value,
>   	return status;
>   }
>
> +/* interfaces to create and destroy token resources */
> +static int au0828_create_token_resources(struct au0828_dev *dev)
> +{
> +	int rc = 0, len;
> +	char buf[64];
> +
> +	/* create token devres for tuner */
> +	len = snprintf(buf, sizeof(buf),
> +		"tuner:%s-%s-%d",
> +		dev_name(&dev->usbdev->dev),
> +		dev->usbdev->bus->bus_name,
> +		dev->board.tuner_addr);
> +
> +	dev->tuner_tkn = devm_kzalloc(&dev->usbdev->dev, len + 1, GFP_KERNEL);
> +	if (!dev->tuner_tkn)
> +		return -ENOMEM;
> +
> +	strcpy(dev->tuner_tkn, buf);
> +	rc = devm_token_create(&dev->usbdev->dev, dev->tuner_tkn);
> +	if (rc)
> +		return rc;
> +
> +	pr_info("au0828_create_token_resources(): Tuner token created\n");
> +	return rc;
> +}
> +
> +static void au0828_destroy_token_resources(struct au0828_dev *dev)
> +{
> +	devm_token_destroy(&dev->usbdev->dev, dev->tuner_tkn);

I don't quite understand this. Isn't this a managed resource that will
be released automatically? So why release it explicitly?

> +}
> +
>   static void au0828_usb_release(struct au0828_dev *dev)
>   {
>   	/* I2C */
> @@ -154,6 +185,8 @@ static void au0828_usb_disconnect(struct usb_interface *interface)
>   	/* Digital TV */
>   	au0828_dvb_unregister(dev);
>
> +	au0828_destroy_token_resources(dev);
> +
>   	usb_set_intfdata(interface, NULL);
>   	mutex_lock(&dev->mutex);
>   	dev->usbdev = NULL;
> @@ -213,6 +246,13 @@ static int au0828_usb_probe(struct usb_interface *interface,
>   	dev->usbdev = usbdev;
>   	dev->boardnr = id->driver_info;
>
> +	/* create token resources */
> +	if (au0828_create_token_resources(dev)) {
> +		mutex_unlock(&dev->lock);
> +		kfree(dev);
> +		return -ENOMEM;
> +	}
> +
>   #ifdef CONFIG_VIDEO_AU0828_V4L2
>   	dev->v4l2_dev.release = au0828_usb_v4l2_release;
>
> @@ -221,6 +261,7 @@ static int au0828_usb_probe(struct usb_interface *interface,
>   	if (retval) {
>   		pr_err("%s() v4l2_device_register failed\n",
>   		       __func__);
> +		au0828_destroy_token_resources(dev);
>   		mutex_unlock(&dev->lock);
>   		kfree(dev);
>   		return retval;
> @@ -230,6 +271,7 @@ static int au0828_usb_probe(struct usb_interface *interface,
>   	if (retval) {
>   		pr_err("%s() v4l2_ctrl_handler_init failed\n",
>   		       __func__);
> +		au0828_destroy_token_resources(dev);
>   		mutex_unlock(&dev->lock);
>   		kfree(dev);
>   		return retval;
> diff --git a/drivers/media/usb/au0828/au0828-dvb.c b/drivers/media/usb/au0828/au0828-dvb.c
> index d8b5d94..1195d29 100755
> --- a/drivers/media/usb/au0828/au0828-dvb.c
> +++ b/drivers/media/usb/au0828/au0828-dvb.c
> @@ -412,6 +412,7 @@ static int dvb_register(struct au0828_dev *dev)
>   		goto fail_adapter;
>   	}
>   	dvb->adapter.priv = dev;
> +	dvb->frontend->tuner_tkn = dev->tuner_tkn;
>
>   	/* register frontend */
>   	result = dvb_register_frontend(&dvb->adapter, dvb->frontend);
> diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
> index 385894a..0b50bda 100644
> --- a/drivers/media/usb/au0828/au0828-video.c
> +++ b/drivers/media/usb/au0828/au0828-video.c
> @@ -2018,6 +2018,9 @@ int au0828_analog_register(struct au0828_dev *dev,
>   	dev->vdev->lock = &dev->lock;
>   	set_bit(V4L2_FL_USE_FH_PRIO, &dev->vdev->flags);
>   	strcpy(dev->vdev->name, "au0828a video");
> +	/* is there another way v4l2 core can get struct device ?? */
> +	dev->vdev->dev_parent = &dev->usbdev->dev;
> +	dev->vdev->tuner_tkn = dev->tuner_tkn;
>
>   	/* Setup the VBI device */
>   	*dev->vbi_dev = au0828_video_template;
> @@ -2025,6 +2028,7 @@ int au0828_analog_register(struct au0828_dev *dev,
>   	dev->vbi_dev->lock = &dev->lock;
>   	set_bit(V4L2_FL_USE_FH_PRIO, &dev->vbi_dev->flags);
>   	strcpy(dev->vbi_dev->name, "au0828a vbi");
> +	dev->vbi_dev->tuner_tkn = dev->tuner_tkn;
>
>   	/* Register the v4l2 device */
>   	video_set_drvdata(dev->vdev, dev);
> diff --git a/drivers/media/usb/au0828/au0828.h b/drivers/media/usb/au0828/au0828.h
> index 7112b9d..11bc933 100644
> --- a/drivers/media/usb/au0828/au0828.h
> +++ b/drivers/media/usb/au0828/au0828.h
> @@ -23,6 +23,7 @@
>   #include <linux/i2c.h>
>   #include <linux/i2c-algo-bit.h>
>   #include <media/tveeprom.h>
> +#include <linux/token_devres.h>
>
>   /* Analog */
>   #include <linux/videodev2.h>
> @@ -198,6 +199,9 @@ struct au0828_dev {
>   	struct au0828_board	board;
>   	u8			ctrlmsg[64];
>
> +	/* token resources */
> +	char *tuner_tkn; /* tuner token id */
> +
>   	/* I2C */
>   	struct i2c_adapter		i2c_adap;
>   	struct i2c_algorithm		i2c_algo;
>

DVB and V4L are in the same driver, so it is easy to share the same token, but how
should this work if snd-usb-audio needs this to coordinate with the media driver?

A more fundamental problem is that you take a mutex and just return and only unlock
it much, much later. I'm not sure what lockdep will make of that... However, why
would you do it like that? What you really need is struct kref (or something like
it) to keep track of the number of references to the token, and the token has to
have some sort of a mode as well. I.e., you can only switch mode if the refcount
is 0. This would allow multiple opens of video/vbi/radio nodes as long as that
will not change the tuner mode.

Not all that different really from this attempt of mine that I never finished:

http://git.linuxtv.org/cgit.cgi/hverkuil/media_tree.git/log/?h=tuner

Regards,

	Hans

Re: [PATCH 1/4] drivers/base: add managed token dev resource

[Hans Verkuil]

On 06/25/2014 01:57 AM, Shuah Khan wrote:
> Add a managed token resource that can be created at device level
> which can be used as a large grain lock by diverse group of drivers
> such as the media drivers that share a resource.
>
> Media devices often have hardware resources that are shared
> across several functions. These devices appear as a group of
> independent devices. Each device implements a function which
> could be shared by one or more functions supported by the same
> device. For example, tuner is shared by analog and digital TV
> functions.
>
> Media drivers that control a single media TV stick are a
> diversified group. Analog and digital TV function drivers have
> to coordinate access to their shared functions.
>
> Some media devices provide multiple almost-independent functions.
> USB and PCI core aids in allowing multiple drivers to handle these
> almost-independent functions. In this model, a media device could
> have snd-usb-audio driving the audio function.
>
> As a result, snd-usb-audio driver has to coordinate with the media
> driver analog and digital function drivers.
>
> A shared managed resource framework at drivers/base level will
> allow a media device to be controlled by drivers that don't
> fall under drivers/media and share functions with other media
> drivers.
>
> Token resource manages a unique named string resource which is
> derived from common bus_name, and hardware address fields from
> the struct device.
>
> Interfaces:
>      devm_token_create()
>      devm_token_destroy()
>      devm_token_lock()
>      devm_token_unlock()
> Usage:
>      Create token: Call devm_token_create() with a token id string.
>      Lock token: Call devm_token_lock() to lock or try lock a token.
>      Unlock token: Call devm_token_unlock().
>      Destroy token: Call devm_token_destroy() to delete the token.
>
> Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
> ---
>   drivers/base/Makefile        |    2 +-
>   drivers/base/token_devres.c  |  134 ++++++++++++++++++++++++++++++++++++++++++
>   include/linux/token_devres.h |   19 ++++++
>   3 files changed, 154 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/base/token_devres.c
>   create mode 100644 include/linux/token_devres.h
>
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 04b314e..924665b 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -4,7 +4,7 @@ obj-y			:= component.o core.o bus.o dd.o syscore.o \
>   			   driver.o class.o platform.o \
>   			   cpu.o firmware.o init.o map.o devres.o \
>   			   attribute_container.o transport_class.o \
> -			   topology.o container.o
> +			   topology.o container.o token_devres.o
>   obj-$(CONFIG_DEVTMPFS)	+= devtmpfs.o
>   obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
>   obj-y			+= power/
> diff --git a/drivers/base/token_devres.c b/drivers/base/token_devres.c
> new file mode 100644
> index 0000000..86bcd25
> --- /dev/null
> +++ b/drivers/base/token_devres.c
> @@ -0,0 +1,134 @@
> +/*
> + * drivers/base/token_devres.c - managed token resource
> + *
> + * Copyright (c) 2014 Shuah Khan <shuah.kh@samsung.com>
> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.

Just a small one:

I don't know the Samsung guidelines, but I would expect that the copyright is
with Samsung, not with you as well. You are the author, but you most likely
do not have the copyright, that's with Samsung.

Regards,

	Hans