[RFC PATCH v2 0/5] Media Device Allocator API

[RFC PATCH v2 0/5] Media Device Allocator API

[Shuah Khan]

There are known problems with media device life time management. When media
device is released while an media ioctl is in progress, ioctls fail with
use-after-free errors and kernel hangs in some cases.

Media Device can be in any the following states:

- Allocated
- Registered (could be tied to more than one driver)
- Unregistered, not in use (media device file is not open)
- Unregistered, in use (media device file is not open)
- Released

When media device belongs to  more than one driver, registrations should be
tracked to avoid unregistering when one of the drivers does unregister. A new
num_drivers field in the struct media_device covers this case. The media device
should be unregistered only when the last unregister occurs with num_drivers
count zero.

When a media device is in use when it is unregistered, it should not be
released until the application exits when it detects the unregistered
status. Media device that is in use when it is unregistered is moved to
to_delete_list. When the last unregister occurs, media device is unregistered
and becomes an unregistered, still allocated device. Unregister marks the
device to be deleted.

When media device belongs to more than one driver, as both drivers could be
unbound/bound, driver should not end up getting stale media device that is
on its way out. Moving the unregistered media device to to_delete_list helps
this case as well.

I ran bind/unbind loop tests on uvcvideo, au0828, and snd-usb-audio while
running application that does ioctls. Didn't see any use-after-free errors
on media device. A couple of known issues seen:

1. When application exits, cdev_put() gets called after media device is
   released. This is a known issue to resolve and Media Device Allocator
   can't solve this one.
2. When au0828 module is removed and then ioctls fail when cdev_get() looks
   for the owning module as au0828 is very often the module that owns the
   media devnode. This is a cdev related issue that needs to be resolved and
   Media Device Allocator can't solve this one.

Shuah Khan (5):
  media: Add Media Device Allocator API
  media: Add driver count to keep track of media device registrations
  media: uvcvideo change to use Media Device Allocator API
  media: au0828 change to use Media Device Allocator API
  sound/usb: Use Media Controller API to share media resources

 drivers/media/Makefile                 |   3 +-
 drivers/media/media-dev-allocator.c    | 154 ++++++++++++++++
 drivers/media/media-device.c           |  49 ++++-
 drivers/media/media-devnode.c          |  10 +-
 drivers/media/usb/au0828/au0828-core.c |  40 +++--
 drivers/media/usb/au0828/au0828.h      |   1 +
 drivers/media/usb/uvc/uvc_driver.c     |  36 ++--
 drivers/media/usb/uvc/uvcvideo.h       |   3 +-
 include/media/media-dev-allocator.h    | 116 ++++++++++++
 include/media/media-device.h           |  31 ++++
 sound/usb/Kconfig                      |   4 +
 sound/usb/Makefile                     |   2 +
 sound/usb/card.c                       |  14 ++
 sound/usb/card.h                       |   3 +
 sound/usb/media.c                      | 320 +++++++++++++++++++++++++++++++++
 sound/usb/media.h                      |  73 ++++++++
 sound/usb/mixer.h                      |   3 +
 sound/usb/pcm.c                        |  28 ++-
 sound/usb/quirks-table.h               |   1 +
 sound/usb/stream.c                     |   2 +
 sound/usb/usbaudio.h                   |   6 +
 21 files changed, 862 insertions(+), 37 deletions(-)
 create mode 100644 drivers/media/media-dev-allocator.c
 create mode 100644 include/media/media-dev-allocator.h
 create mode 100644 sound/usb/media.c
 create mode 100644 sound/usb/media.h

-- 
2.5.0

[RFC PATCH v2 1/5] media: Add Media Device Allocator API

[Shuah Khan]

Add Media Device Allocator API to manage Media Device life time problems.
There are known problems with media device life time management. When media
device is released while an media ioctl is in progress, ioctls fail with
use-after-free errors and kernel hangs in some cases.

Media Allocator API provides interfaces to allocate a refcounted media
device instance from system wide global list and maintains the state until
the last user of the media device releases it.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 drivers/media/Makefile              |   3 +-
 drivers/media/media-dev-allocator.c | 154 ++++++++++++++++++++++++++++++++++++
 include/media/media-dev-allocator.h | 116 +++++++++++++++++++++++++++
 3 files changed, 272 insertions(+), 1 deletion(-)
 create mode 100644 drivers/media/media-dev-allocator.c
 create mode 100644 include/media/media-dev-allocator.h

diff --git a/drivers/media/Makefile b/drivers/media/Makefile
index e608bbc..b08f091 100644
--- a/drivers/media/Makefile
+++ b/drivers/media/Makefile
@@ -2,7 +2,8 @@
 # Makefile for the kernel multimedia device drivers.
 #
 
-media-objs	:= media-device.o media-devnode.o media-entity.o
+media-objs	:= media-device.o media-devnode.o media-entity.o \
+		   media-dev-allocator.o
 
 #
 # I2C drivers should come before other drivers, otherwise they'll fail
diff --git a/drivers/media/media-dev-allocator.c b/drivers/media/media-dev-allocator.c
new file mode 100644
index 0000000..8c3a5f0
--- /dev/null
+++ b/drivers/media/media-dev-allocator.c
@@ -0,0 +1,154 @@
+/*
+ * media-devkref.c - Media Controller Device Allocator API
+ *
+ * Copyright (c) 2016 Shuah Khan <shuahkh@osg.samsung.com>
+ * Copyright (c) 2016 Samsung Electronics Co., Ltd.
+ *
+ * This file is released under the GPLv2.
+ * Credits: Suggested by Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ */
+
+/*
+ * This file adds Media Controller Device Instance with
+ * Kref support. A system wide global media device list
+ * is managed and each  media device is refcounted. The
+ * last put on the media device releases the media device
+ * instance.
+*/
+
+#include <linux/slab.h>
+#include <linux/kref.h>
+#include <media/media-device.h>
+
+static LIST_HEAD(media_device_list);
+static LIST_HEAD(media_device_to_delete_list);
+static DEFINE_MUTEX(media_device_lock);
+
+struct media_device_instance {
+	struct media_device mdev;
+	struct list_head list;
+	struct device *dev;
+	struct kref refcount;
+	bool to_delete; /* should be set when devnode is deleted */
+};
+
+static struct media_device *__media_device_get(struct device *dev,
+					       bool allocate, bool get_kref)
+{
+	struct media_device_instance *mdi;
+
+	mutex_lock(&media_device_lock);
+
+	list_for_each_entry(mdi, &media_device_list, list) {
+		if (mdi->dev == dev) {
+			if (get_kref) {
+				kref_get(&mdi->refcount);
+				pr_debug("%s: mdev=%p\n", __func__, &mdi->mdev);
+			}
+			goto done;
+		}
+	}
+
+	if (!allocate) {
+		mdi = NULL;
+		goto done;
+	}
+
+	mdi = kzalloc(sizeof(*mdi), GFP_KERNEL);
+	if (!mdi)
+		goto done;
+
+	mdi->dev = dev;
+	kref_init(&mdi->refcount);
+	list_add_tail(&mdi->list, &media_device_list);
+	pr_debug("%s: mdev=%p\n", __func__, &mdi->mdev);
+
+done:
+	mutex_unlock(&media_device_lock);
+
+	return mdi ? &mdi->mdev : NULL;
+}
+
+struct media_device *media_device_get(struct device *dev)
+{
+	pr_debug("%s\n", __func__);
+	return __media_device_get(dev, true, true);
+}
+EXPORT_SYMBOL_GPL(media_device_get);
+
+/* Don't increment kref - this is a search and find */
+struct media_device *media_device_find(struct device *dev)
+{
+	pr_debug("%s\n", __func__);
+	return __media_device_get(dev, false, false);
+}
+EXPORT_SYMBOL_GPL(media_device_find);
+
+/* don't allocate - increment kref if one is found */
+struct media_device *media_device_get_ref(struct device *dev)
+{
+	pr_debug("%s\n", __func__);
+	return __media_device_get(dev, false, true);
+}
+EXPORT_SYMBOL_GPL(media_device_get_ref);
+
+static void media_device_instance_release(struct kref *kref)
+{
+	struct media_device_instance *mdi =
+		container_of(kref, struct media_device_instance, refcount);
+
+	pr_debug("%s: mdev=%p\n", __func__, &mdi->mdev);
+
+	list_del(&mdi->list);
+	kfree(mdi);
+}
+
+void media_device_put(struct device *dev)
+{
+	struct media_device_instance *mdi;
+
+	mutex_lock(&media_device_lock);
+	/* search first in the media_device_list */
+	list_for_each_entry(mdi, &media_device_list, list) {
+		if (mdi->dev == dev) {
+			pr_debug("%s: mdev=%p\n", __func__, &mdi->mdev);
+			kref_put(&mdi->refcount, media_device_instance_release);
+			goto done;
+		}
+		pr_debug("%s: media_device_list mdev=%p\n", __func__,
+			&mdi->mdev);
+	}
+	/* search in the media_device_to_delete_list */
+	list_for_each_entry(mdi, &media_device_to_delete_list, list) {
+		if (mdi->dev == dev) {
+			pr_debug("%s: mdev=%p\n", __func__, &mdi->mdev);
+			kref_put(&mdi->refcount,
+				 media_device_instance_release);
+			goto done;
+		}
+		pr_debug("%s: media_device_to_delete_list mdev=%p\n", __func__,
+			&mdi->mdev);
+	}
+done:
+	mutex_unlock(&media_device_lock);
+}
+EXPORT_SYMBOL_GPL(media_device_put);
+
+void media_device_set_to_delete_state(struct device *dev)
+{
+	struct media_device_instance *mdi;
+
+	mutex_lock(&media_device_lock);
+	list_for_each_entry(mdi, &media_device_list, list) {
+		if (mdi->dev == dev) {
+			pr_debug("%s: mdev=%p\n", __func__, &mdi->mdev);
+			mdi->to_delete = true;
+			list_move_tail(&mdi->list,
+				       &media_device_to_delete_list);
+			goto done;
+		}
+	}
+done:
+	mutex_unlock(&media_device_lock);
+}
+EXPORT_SYMBOL_GPL(media_device_set_to_delete_state);
diff --git a/include/media/media-dev-allocator.h b/include/media/media-dev-allocator.h
new file mode 100644
index 0000000..6003bad
--- /dev/null
+++ b/include/media/media-dev-allocator.h
@@ -0,0 +1,116 @@
+/*
+ * media-devkref.c - Media Controller Device Allocator API
+ *
+ * Copyright (c) 2016 Shuah Khan <shuahkh@osg.samsung.com>
+ * Copyright (c) 2016 Samsung Electronics Co., Ltd.
+ *
+ * This file is released under the GPLv2.
+ * Credits: Suggested by Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ */
+
+/*
+ * This file adds Media Controller Device Instance with Kref support.
+ * A system wide global media device list is managed and each media
+ * device is refcounted. The last put on the media device releases
+ * the media device instance.
+*/
+
+#ifndef _MEDIA_DEV_ALLOCTOR_H
+#define _MEDIA_DEV_ALLOCTOR_H
+
+#ifdef CONFIG_MEDIA_CONTROLLER
+/**
+ * DOC: Media Controller Device Allocator API
+ * There are known problems with media device life time management. When media
+ * device is released while an media ioctl is in progress, ioctls fail with
+ * use-after-free errors and kernel hangs in some cases.
+ *
+ * Media Device can be in any the following states:
+ *
+ * - Allocated
+ * - Registered (could be tied to more than one driver)
+ * - Unregistered, not in use (media device file is not open)
+ * - Unregistered, in use (media device file is not open)
+ * - Released
+ *
+ * When media device belongs to  more than one driver, registrations should be
+ * refcounted to avoid unregistering when one of the drivers does unregister.
+ * A refcount field in the struct media_device covers this case. Unregister on
+ * a Media Allocator media device is a kref_put() call. The media device should
+ * be unregistered only when the last unregister occurs.
+ *
+ * When a media device is in use when it is unregistered, it should not be
+ * released until the application exits when it detects the unregistered
+ * status. Media device that is in use when it is unregistered is moved to
+ * to_delete_list. When the last unregister occurs, media device is unregistered
+ * and becomes an unregistered, still allocated device. Unregister marks the
+ * device to be deleted.
+ *
+ * When media device belongs to more than one driver, as both drivers could be
+ * unbound/bound, driver should not end up getting stale media device that is
+ * on its way out. Moving the unregistered media device to to_delete_list helps
+ * this case as well.
+ */
+/**
+ * media_device_get() - Allocate and return global media device
+ *
+ * @mdev
+ *
+ * This interface should be called to allocate media device. A new media
+ * device instance is created and added to the system wide media device
+ * instance list. If media device instance exists, media_device_get()
+ * increments the reference count and returns the media device. When
+ * more than one driver control the media device, the first driver to
+ * probe will allocate and the second driver when it calls media_device_get()
+ * it will get a reference.
+ *
+ */
+struct media_device *media_device_get(struct device *dev);
+/**
+ * media_device_get_ref() - Get reference to an allocated and registered
+ *			    media device.
+ *
+ * @mdev
+ *
+ * This interface should be called to get a reference to an allocated media
+ * device. media_open() ioctl should call this to hold a reference to ensure
+ * the media device will not be released until the media_release() does a put
+ * on it.
+ */
+struct media_device *media_device_get_ref(struct device *dev);
+/**
+ * media_device_find() - Find an allocated and registered media device.
+ *
+ * @mdev
+ *
+ * This interface should be called to find a media device. This will not
+ * incremnet the reference count.
+ */
+struct media_device *media_device_find(struct device *dev);
+/**
+ * media_device_put() - Release refcounted media device. Calls kref_put()
+ *
+ * @mdev
+ *
+ * This interface should be called to decrement refcount.
+ */
+void media_device_put(struct device *dev);
+/**
+ * media_device_set_to_delete_state() - Set the state to be deleted.
+ *
+ * @mdev
+ *
+ * This interface is used to not release the media device under from
+ * an active ioctl if unregister happens.
+ */
+void media_device_set_to_delete_state(struct device *dev);
+#else
+static inline struct media_device *media_device_get(struct device *dev)
+			{ return NULL; }
+static inline struct media_device *media_device_get_ref(struct device *dev);
+static inline struct media_device *media_device_find(struct device *dev)
+			{ return NULL; }
+static inline void media_device_put(struct device *dev) { }
+static inline void media_device_set_to_delete_state(struct device *dev) { }
+#endif /* CONFIG_MEDIA_CONTROLLER */
+#endif
-- 
2.5.0

[RFC PATCH v2 2/5] media: Add driver count to keep track of media device registrations

[Shuah Khan]

Add driver count to keep track of media device registrations to avoid
releasing the media device, when one of the drivers does unregister when
media device belongs to more than one driver. Also add a new interfaces
to unregister a media device allocated using Media Device Allocator and
a increment register count.

Change media_open() to get media device reference and put the reference
in media_release().

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 drivers/media/media-device.c  | 49 ++++++++++++++++++++++++++++++++++++++++++-
 drivers/media/media-devnode.c | 10 ++++++---
 include/media/media-device.h  | 31 +++++++++++++++++++++++++++
 3 files changed, 86 insertions(+), 4 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 6e43c95..f22cf0f 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -36,6 +36,7 @@
 #include <media/media-device.h>
 #include <media/media-devnode.h>
 #include <media/media-entity.h>
+#include <media/media-dev-allocator.h>
 
 #ifdef CONFIG_MEDIA_CONTROLLER
 
@@ -743,12 +744,31 @@ int __must_check __media_device_register(struct media_device *mdev,
 		return ret;
 	}
 
-	dev_dbg(mdev->dev, "Media device registered\n");
+	mdev->num_drivers++;
+	dev_dbg(mdev->dev, "Media device registered num_drivers %d\n",
+		mdev->num_drivers);
 
 	return 0;
 }
 EXPORT_SYMBOL_GPL(__media_device_register);
 
+void media_device_register_ref(struct media_device *mdev)
+{
+	if (!mdev)
+		return;
+
+	mutex_lock(&mdev->graph_mutex);
+
+	/* Check if mdev is registered - bump registered driver count */
+	if (media_devnode_is_registered(&mdev->devnode))
+		mdev->num_drivers++;
+
+	dev_dbg(mdev->dev, "%s: mdev %p num_drivers %d\n", __func__, mdev,
+		mdev->num_drivers);
+	mutex_unlock(&mdev->graph_mutex);
+}
+EXPORT_SYMBOL_GPL(media_device_register_ref);
+
 int __must_check media_device_register_entity_notify(struct media_device *mdev,
 					struct media_entity_notify *nptr)
 {
@@ -820,6 +840,33 @@ void media_device_unregister(struct media_device *mdev)
 }
 EXPORT_SYMBOL_GPL(media_device_unregister);
 
+void media_device_unregister_put(struct media_device *mdev)
+{
+	if (mdev == NULL)
+		return;
+
+	dev_dbg(mdev->dev, "%s: mdev %p num_drivers %d\n", __func__, mdev,
+		mdev->num_drivers);
+
+	mutex_lock(&mdev->graph_mutex);
+	mdev->num_drivers--;
+	if (mdev->num_drivers == 0) {
+		mutex_unlock(&mdev->graph_mutex);
+
+		/* unregister media device and cleanup */
+		media_device_unregister(mdev);
+		media_device_cleanup(mdev);
+
+		/* mark the media device for deletion */
+		media_device_set_to_delete_state(mdev->dev);
+	} else
+		mutex_unlock(&mdev->graph_mutex);
+
+	dev_dbg(mdev->dev, "%s: end mdev %p num_drivers %d\n", __func__, mdev,
+		mdev->num_drivers);
+}
+EXPORT_SYMBOL_GPL(media_device_unregister_put);
+
 static void media_device_release_devres(struct device *dev, void *res)
 {
 }
diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
index 29409f4..ec18815 100644
--- a/drivers/media/media-devnode.c
+++ b/drivers/media/media-devnode.c
@@ -44,6 +44,7 @@
 #include <linux/uaccess.h>
 
 #include <media/media-devnode.h>
+#include <media/media-dev-allocator.h>
 
 #define MEDIA_NUM_DEVICES	256
 #define MEDIA_NAME		"media"
@@ -173,7 +174,6 @@ static int media_open(struct inode *inode, struct file *filp)
 	}
 	/* and increase the device refcount */
 	get_device(&mdev->dev);
-	mutex_unlock(&media_devnode_lock);
 
 	filp->private_data = mdev;
 
@@ -182,11 +182,14 @@ static int media_open(struct inode *inode, struct file *filp)
 		if (ret) {
 			put_device(&mdev->dev);
 			filp->private_data = NULL;
-			return ret;
+			goto done;
 		}
 	}
 
-	return 0;
+	media_device_get_ref(mdev->parent);
+done:
+	mutex_unlock(&media_devnode_lock);
+	return ret;
 }
 
 /* Override for the release function */
@@ -201,6 +204,7 @@ static int media_release(struct inode *inode, struct file *filp)
 	   return value is ignored. */
 	put_device(&mdev->dev);
 	filp->private_data = NULL;
+	media_device_put(mdev->parent);
 	return 0;
 }
 
diff --git a/include/media/media-device.h b/include/media/media-device.h
index df74cfa..aaeac7a 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -284,6 +284,7 @@ struct media_entity_notify {
  * struct media_device - Media device
  * @dev:	Parent device
  * @devnode:	Media device node
+ * @num_drivers: Number of drivers that own the media device and register.
  * @driver_name: Optional device driver name. If not set, calls to
  *		%MEDIA_IOC_DEVICE_INFO will return dev->driver->name.
  *		This is needed for USB drivers for example, as otherwise
@@ -349,6 +350,7 @@ struct media_device {
 	/* dev->driver_data points to this struct. */
 	struct device *dev;
 	struct media_devnode devnode;
+	int num_drivers;
 
 	char model[32];
 	char driver_name[32];
@@ -494,6 +496,17 @@ int __must_check __media_device_register(struct media_device *mdev,
 #define media_device_register(mdev) __media_device_register(mdev, THIS_MODULE)
 
 /**
+ * media_device_register_ref() - Increments media device register driver count
+ *
+ * @mdev:	pointer to struct &media_device
+ *
+ * When more than one driver is associated with the media device, it is
+ * necessary to maintain the number of registrations to avoid unregister
+ * when it is still in use.
+ */
+void media_device_register_ref(struct media_device *mdev);
+
+/**
  * __media_device_unregister() - Unegisters a media device element
  *
  * @mdev:	pointer to struct &media_device
@@ -505,6 +518,18 @@ int __must_check __media_device_register(struct media_device *mdev,
 void media_device_unregister(struct media_device *mdev);
 
 /**
+ * media_device_unregister_put() - Unregisters a media device element
+ *
+ * @mdev:	pointer to struct &media_device
+ *
+ * Should be called to unregister media device allocated with Media Device
+ * Allocator API media_device_get() interface.
+ * It is safe to call this function on an unregistered (but initialised)
+ * media device.
+ */
+void media_device_unregister_put(struct media_device *mdev);
+
+/**
  * media_device_register_entity() - registers a media entity inside a
  *	previously registered media device.
  *
@@ -661,9 +686,15 @@ static inline int media_device_register(struct media_device *mdev)
 {
 	return 0;
 }
+static inline void media_device_register_ref(struct media_device *mdev)
+{
+}
 static inline void media_device_unregister(struct media_device *mdev)
 {
 }
+static inline void media_device_unregister_put(struct media_device *mdev)
+{
+}
 static inline int media_device_register_entity(struct media_device *mdev,
 						struct media_entity *entity)
 {
-- 
2.5.0

[RFC PATCH v2 3/5] media: uvcvideo change to use Media Device Allocator API

[Shuah Khan]

Change uvcvideo to use Media Device Allocator API and new Media Controller
media_device_unregister_put() interface.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 drivers/media/usb/uvc/uvc_driver.c | 36 ++++++++++++++++++++++--------------
 drivers/media/usb/uvc/uvcvideo.h   |  3 ++-
 2 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 451e84e9..95e30c4 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1674,9 +1674,10 @@ static void uvc_delete(struct uvc_device *dev)
 	if (dev->vdev.dev)
 		v4l2_device_unregister(&dev->vdev);
 #ifdef CONFIG_MEDIA_CONTROLLER
-	if (media_devnode_is_registered(&dev->mdev.devnode))
-		media_device_unregister(&dev->mdev);
-	media_device_cleanup(&dev->mdev);
+	if (media_devnode_is_registered(&dev->mdev->devnode)) {
+		media_device_unregister_put(dev->mdev);
+		media_device_put(dev->mdev->dev);
+	}
 #endif
 
 	list_for_each_safe(p, n, &dev->chains) {
@@ -1929,17 +1930,20 @@ static int uvc_probe(struct usb_interface *intf,
 
 	/* Initialize the media device and register the V4L2 device. */
 #ifdef CONFIG_MEDIA_CONTROLLER
-	dev->mdev.dev = &intf->dev;
-	strlcpy(dev->mdev.model, dev->name, sizeof(dev->mdev.model));
+	dev->mdev = media_device_get(&intf->dev);
+	if (!dev->mdev)
+		goto media_device_get_error;
+	dev->mdev->dev = &intf->dev;
+	strlcpy(dev->mdev->model, dev->name, sizeof(dev->mdev->model));
 	if (udev->serial)
-		strlcpy(dev->mdev.serial, udev->serial,
-			sizeof(dev->mdev.serial));
-	strcpy(dev->mdev.bus_info, udev->devpath);
-	dev->mdev.hw_revision = le16_to_cpu(udev->descriptor.bcdDevice);
-	dev->mdev.driver_version = LINUX_VERSION_CODE;
-	media_device_init(&dev->mdev);
-
-	dev->vdev.mdev = &dev->mdev;
+		strlcpy(dev->mdev->serial, udev->serial,
+			sizeof(dev->mdev->serial));
+	strcpy(dev->mdev->bus_info, udev->devpath);
+	dev->mdev->hw_revision = le16_to_cpu(udev->descriptor.bcdDevice);
+	dev->mdev->driver_version = LINUX_VERSION_CODE;
+	media_device_init(dev->mdev);
+
+	dev->vdev.mdev = dev->mdev;
 #endif
 	if (v4l2_device_register(&intf->dev, &dev->vdev) < 0)
 		goto error;
@@ -1958,7 +1962,7 @@ static int uvc_probe(struct usb_interface *intf,
 
 #ifdef CONFIG_MEDIA_CONTROLLER
 	/* Register the media device node */
-	if (media_device_register(&dev->mdev) < 0)
+	if (media_device_register(dev->mdev) < 0)
 		goto error;
 #endif
 	/* Save our data pointer in the interface data. */
@@ -1976,6 +1980,10 @@ static int uvc_probe(struct usb_interface *intf,
 	return 0;
 
 error:
+#ifdef CONFIG_MEDIA_CONTROLLER
+	media_device_put(&intf->dev);
+media_device_get_error:
+#endif
 	uvc_unregister_video(dev);
 	return -ENODEV;
 }
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 7e4d3ee..a5ef719 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -12,6 +12,7 @@
 #include <linux/uvcvideo.h>
 #include <linux/videodev2.h>
 #include <media/media-device.h>
+#include <media/media-dev-allocator.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-event.h>
 #include <media/v4l2-fh.h>
@@ -543,7 +544,7 @@ struct uvc_device {
 
 	/* Video control interface */
 #ifdef CONFIG_MEDIA_CONTROLLER
-	struct media_device mdev;
+	struct media_device *mdev;
 #endif
 	struct v4l2_device vdev;
 	__u16 uvc_version;
-- 
2.5.0

[RFC PATCH v2 4/5] media: au0828 change to use Media Device Allocator API

[Shuah Khan]

Change au0828 to use Media Device Allocator API and new Media Controller
media_device_unregister_put() interface. Fix to unregister entity_notify
hook.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 drivers/media/usb/au0828/au0828-core.c | 40 ++++++++++++++++++++++++----------
 drivers/media/usb/au0828/au0828.h      |  1 +
 2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c
index cc22b32..c34af36 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -131,22 +131,36 @@ static int recv_control_msg(struct au0828_dev *dev, u16 request, u32 value,
 	return status;
 }
 
+#ifdef CONFIG_MEDIA_CONTROLLER
+static void au0828_media_graph_notify(struct media_entity *new,
+				      void *notify_data);
+#endif
+
 static void au0828_unregister_media_device(struct au0828_dev *dev)
 {
 
 #ifdef CONFIG_MEDIA_CONTROLLER
-	if (dev->media_dev &&
-		media_devnode_is_registered(&dev->media_dev->devnode)) {
-		/* clear enable_source, disable_source */
-		dev->media_dev->source_priv = NULL;
-		dev->media_dev->enable_source = NULL;
-		dev->media_dev->disable_source = NULL;
-
-		media_device_unregister(dev->media_dev);
-		media_device_cleanup(dev->media_dev);
-		kfree(dev->media_dev);
-		dev->media_dev = NULL;
+	struct media_device *mdev = dev->media_dev;
+	struct media_entity_notify *notify, *nextp;
+
+	if (!mdev || !media_devnode_is_registered(&mdev->devnode))
+		return;
+
+	/* Remove au0828 entity_notify callbacks */
+	list_for_each_entry_safe(notify, nextp, &mdev->entity_notify, list) {
+		if (notify->notify != au0828_media_graph_notify)
+			continue;
+			media_device_unregister_entity_notify(mdev, notify);
 	}
+
+	/* clear enable_source, disable_source */
+	dev->media_dev->source_priv = NULL;
+	dev->media_dev->enable_source = NULL;
+	dev->media_dev->disable_source = NULL;
+
+	media_device_unregister_put(dev->media_dev);
+	media_device_put(dev->media_dev->dev);
+	dev->media_dev = NULL;
 #endif
 }
 
@@ -198,7 +212,7 @@ static int au0828_media_device_init(struct au0828_dev *dev,
 #ifdef CONFIG_MEDIA_CONTROLLER
 	struct media_device *mdev;
 
-	mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
+	mdev = media_device_get(&udev->dev);
 	if (!mdev)
 		return -ENOMEM;
 
@@ -473,11 +487,13 @@ static int au0828_media_device_register(struct au0828_dev *dev,
 		/* register media device */
 		ret = media_device_register(dev->media_dev);
 		if (ret) {
+			media_device_put(dev->media_dev->dev);
 			dev_err(&udev->dev,
 				"Media Device Register Error: %d\n", ret);
 			return ret;
 		}
 	} else {
+		media_device_register_ref(dev->media_dev);
 		/*
 		 * Call au0828_media_graph_notify() to connect
 		 * audio graph to our graph. In this case, audio
diff --git a/drivers/media/usb/au0828/au0828.h b/drivers/media/usb/au0828/au0828.h
index 87f3284..3edd50f 100644
--- a/drivers/media/usb/au0828/au0828.h
+++ b/drivers/media/usb/au0828/au0828.h
@@ -35,6 +35,7 @@
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-fh.h>
 #include <media/media-device.h>
+#include <media/media-dev-allocator.h>
 
 /* DVB */
 #include "demux.h"
-- 
2.5.0

[RFC PATCH v2 5/5] sound/usb: Use Media Controller API to share media resources

[Shuah Khan]

Change ALSA driver to use Media Controller API to share media resources
with DVB and V4L2 drivers on a AU0828 media device. Media Controller API
specific initialization is done after sound card is registered.

au0828 and snd-usb-audio use Media Device Allocator API to allocate and
share the media device.

ALSA creates Media interface and entity function graph nodes for Control,
Mixer, PCM Playback, and PCM Capture devices.

snd_usb_hw_params() calls Media Controller enable source handler interface
to request the media resource. If resource request is granted, it releases
it from snd_usb_hw_free(). If resource is busy, -EBUSY is returned.

Media Controller API specific cleanup is done in usb_audio_disconnect().

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---

Changes since the revert:
- Consolidated fixes to regressions in this patch.
- Tested on a USB Audio device in addition to au0828 device.

 sound/usb/Kconfig        |   4 +
 sound/usb/Makefile       |   2 +
 sound/usb/card.c         |  14 +++
 sound/usb/card.h         |   3 +
 sound/usb/media.c        | 320 +++++++++++++++++++++++++++++++++++++++++++++++
 sound/usb/media.h        |  73 +++++++++++
 sound/usb/mixer.h        |   3 +
 sound/usb/pcm.c          |  28 ++++-
 sound/usb/quirks-table.h |   1 +
 sound/usb/stream.c       |   2 +
 sound/usb/usbaudio.h     |   6 +
 11 files changed, 451 insertions(+), 5 deletions(-)
 create mode 100644 sound/usb/media.c
 create mode 100644 sound/usb/media.h

diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
index a452ad7..d14bf41 100644
--- a/sound/usb/Kconfig
+++ b/sound/usb/Kconfig
@@ -15,6 +15,7 @@ config SND_USB_AUDIO
 	select SND_RAWMIDI
 	select SND_PCM
 	select BITREVERSE
+	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER && (MEDIA_SUPPORT=y || MEDIA_SUPPORT=SND_USB_AUDIO)
 	help
 	  Say Y here to include support for USB audio and USB MIDI
 	  devices.
@@ -22,6 +23,9 @@ config SND_USB_AUDIO
 	  To compile this driver as a module, choose M here: the module
 	  will be called snd-usb-audio.
 
+config SND_USB_AUDIO_USE_MEDIA_CONTROLLER
+	bool
+
 config SND_USB_UA101
 	tristate "Edirol UA-101/UA-1000 driver"
 	select SND_PCM
diff --git a/sound/usb/Makefile b/sound/usb/Makefile
index 2d2d122..8dca3c4 100644
--- a/sound/usb/Makefile
+++ b/sound/usb/Makefile
@@ -15,6 +15,8 @@ snd-usb-audio-objs := 	card.o \
 			quirks.o \
 			stream.o
 
+snd-usb-audio-$(CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER) += media.o
+
 snd-usbmidi-lib-objs := midi.o
 
 # Toplevel Module Dependency
diff --git a/sound/usb/card.c b/sound/usb/card.c
index 3fc6358..479621e 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -66,6 +66,7 @@
 #include "format.h"
 #include "power.h"
 #include "stream.h"
+#include "media.h"
 
 MODULE_AUTHOR("Takashi Iwai <tiwai@suse.de>");
 MODULE_DESCRIPTION("USB Audio");
@@ -611,6 +612,11 @@ static int usb_audio_probe(struct usb_interface *intf,
 	if (err < 0)
 		goto __error;
 
+	if (quirk && quirk->media_device) {
+		/* don't want to fail when media_snd_device_create() fails */
+		media_snd_device_create(chip, intf);
+	}
+
 	usb_chip[chip->index] = chip;
 	chip->num_interfaces++;
 	usb_set_intfdata(intf, chip);
@@ -667,6 +673,14 @@ static void usb_audio_disconnect(struct usb_interface *intf)
 		list_for_each(p, &chip->midi_list) {
 			snd_usbmidi_disconnect(p);
 		}
+		/*
+		 * Nice to check quirk && quirk->media_device
+		 * need some special handlings. Doesn't look like
+		 * we have access to quirk here
+		 * Acceses mixer_list
+		*/
+		media_snd_device_delete(chip);
+
 		/* release mixer resources */
 		list_for_each_entry(mixer, &chip->mixer_list, list) {
 			snd_usb_mixer_disconnect(mixer);
diff --git a/sound/usb/card.h b/sound/usb/card.h
index 71778ca..34a0898 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -105,6 +105,8 @@ struct snd_usb_endpoint {
 	struct list_head list;
 };
 
+struct media_ctl;
+
 struct snd_usb_substream {
 	struct snd_usb_stream *stream;
 	struct usb_device *dev;
@@ -156,6 +158,7 @@ struct snd_usb_substream {
 	} dsd_dop;
 
 	bool trigger_tstamp_pending_update; /* trigger timestamp being updated from initial estimate */
+	struct media_ctl *media_ctl;
 };
 
 struct snd_usb_stream {
diff --git a/sound/usb/media.c b/sound/usb/media.c
new file mode 100644
index 0000000..e7d1bd3
--- /dev/null
+++ b/sound/usb/media.c
@@ -0,0 +1,320 @@
+/*
+ * media.c - Media Controller specific ALSA driver code
+ *
+ * Copyright (c) 2016 Shuah Khan <shuahkh@osg.samsung.com>
+ * Copyright (c) 2016 Samsung Electronics Co., Ltd.
+ *
+ * This file is released under the GPLv2.
+ */
+
+/*
+ * This file adds Media Controller support to ALSA driver
+ * to use the Media Controller API to share tuner with DVB
+ * and V4L2 drivers that control media device. Media device
+ * is created based on existing quirks framework. Using this
+ * approach, the media controller API usage can be added for
+ * a specific device.
+*/
+
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/usb.h>
+
+#include <sound/pcm.h>
+#include <sound/core.h>
+
+#include "usbaudio.h"
+#include "card.h"
+#include "mixer.h"
+#include "media.h"
+
+static int media_snd_enable_source(struct media_ctl *mctl)
+{
+	if (mctl && mctl->media_dev->enable_source)
+		return mctl->media_dev->enable_source(&mctl->media_entity,
+						      &mctl->media_pipe);
+	return 0;
+}
+
+static void media_snd_disable_source(struct media_ctl *mctl)
+{
+	if (mctl && mctl->media_dev->disable_source)
+		mctl->media_dev->disable_source(&mctl->media_entity);
+}
+
+int media_snd_stream_init(struct snd_usb_substream *subs, struct snd_pcm *pcm,
+			int stream)
+{
+	struct media_device *mdev;
+	struct media_ctl *mctl;
+	struct device *pcm_dev = &pcm->streams[stream].dev;
+	u32 intf_type;
+	int ret = 0;
+	u16 mixer_pad;
+	struct media_entity *entity;
+
+	mdev = subs->stream->chip->media_dev;
+	if (!mdev)
+		return 0;
+
+	if (subs->media_ctl)
+		return 0;
+
+	/* allocate media_ctl */
+	mctl = kzalloc(sizeof(*mctl), GFP_KERNEL);
+	if (!mctl)
+		return -ENOMEM;
+
+	mctl->media_dev = mdev;
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		intf_type = MEDIA_INTF_T_ALSA_PCM_PLAYBACK;
+		mctl->media_entity.function = MEDIA_ENT_F_AUDIO_PLAYBACK;
+		mctl->media_pad.flags = MEDIA_PAD_FL_SOURCE;
+		mixer_pad = 1;
+	} else {
+		intf_type = MEDIA_INTF_T_ALSA_PCM_CAPTURE;
+		mctl->media_entity.function = MEDIA_ENT_F_AUDIO_CAPTURE;
+		mctl->media_pad.flags = MEDIA_PAD_FL_SINK;
+		mixer_pad = 2;
+	}
+	mctl->media_entity.name = pcm->name;
+	media_entity_pads_init(&mctl->media_entity, 1, &mctl->media_pad);
+	ret =  media_device_register_entity(mctl->media_dev,
+					    &mctl->media_entity);
+	if (ret)
+		goto free_mctl;
+
+	mctl->intf_devnode = media_devnode_create(mdev, intf_type, 0,
+						  MAJOR(pcm_dev->devt),
+						  MINOR(pcm_dev->devt));
+	if (!mctl->intf_devnode) {
+		ret = -ENOMEM;
+		goto unregister_entity;
+	}
+	mctl->intf_link = media_create_intf_link(&mctl->media_entity,
+						 &mctl->intf_devnode->intf,
+						 MEDIA_LNK_FL_ENABLED);
+	if (!mctl->intf_link) {
+		ret = -ENOMEM;
+		goto devnode_remove;
+	}
+
+	/* create link between mixer and audio */
+	media_device_for_each_entity(entity, mdev) {
+		switch (entity->function) {
+		case MEDIA_ENT_F_AUDIO_MIXER:
+			ret = media_create_pad_link(entity, mixer_pad,
+						    &mctl->media_entity, 0,
+						    MEDIA_LNK_FL_ENABLED);
+			if (ret)
+				goto remove_intf_link;
+			break;
+		}
+	}
+
+	subs->media_ctl = mctl;
+	return 0;
+
+remove_intf_link:
+	media_remove_intf_link(mctl->intf_link);
+devnode_remove:
+	media_devnode_remove(mctl->intf_devnode);
+unregister_entity:
+	media_device_unregister_entity(&mctl->media_entity);
+free_mctl:
+	kfree(mctl);
+	return ret;
+}
+
+void media_snd_stream_delete(struct snd_usb_substream *subs)
+{
+	struct media_ctl *mctl = subs->media_ctl;
+
+	if (mctl && mctl->media_dev) {
+		struct media_device *mdev;
+
+		mdev = mctl->media_dev;
+		if (mdev && media_devnode_is_registered(&mdev->devnode)) {
+			media_devnode_remove(mctl->intf_devnode);
+			media_device_unregister_entity(&mctl->media_entity);
+			media_entity_cleanup(&mctl->media_entity);
+		}
+		kfree(mctl);
+		subs->media_ctl = NULL;
+	}
+}
+
+int media_snd_start_pipeline(struct snd_usb_substream *subs)
+{
+	struct media_ctl *mctl = subs->media_ctl;
+
+	if (mctl)
+		return media_snd_enable_source(mctl);
+	return 0;
+}
+
+void media_snd_stop_pipeline(struct snd_usb_substream *subs)
+{
+	struct media_ctl *mctl = subs->media_ctl;
+
+	if (mctl)
+		media_snd_disable_source(mctl);
+}
+
+static int media_snd_mixer_init(struct snd_usb_audio *chip)
+{
+	struct device *ctl_dev = &chip->card->ctl_dev;
+	struct media_intf_devnode *ctl_intf;
+	struct usb_mixer_interface *mixer;
+	struct media_device *mdev = chip->media_dev;
+	struct media_mixer_ctl *mctl;
+	u32 intf_type = MEDIA_INTF_T_ALSA_CONTROL;
+	int ret;
+
+	if (!mdev)
+		return -ENODEV;
+
+	ctl_intf = chip->ctl_intf_media_devnode;
+	if (!ctl_intf) {
+		ctl_intf = media_devnode_create(mdev, intf_type, 0,
+						MAJOR(ctl_dev->devt),
+						MINOR(ctl_dev->devt));
+		if (!ctl_intf)
+			return -ENOMEM;
+		chip->ctl_intf_media_devnode = ctl_intf;
+	}
+
+	list_for_each_entry(mixer, &chip->mixer_list, list) {
+
+		if (mixer->media_mixer_ctl)
+			continue;
+
+		/* allocate media_mixer_ctl */
+		mctl = kzalloc(sizeof(*mctl), GFP_KERNEL);
+		if (!mctl)
+			return -ENOMEM;
+
+		mctl->media_dev = mdev;
+		mctl->media_entity.function = MEDIA_ENT_F_AUDIO_MIXER;
+		mctl->media_entity.name = chip->card->mixername;
+		mctl->media_pad[0].flags = MEDIA_PAD_FL_SINK;
+		mctl->media_pad[1].flags = MEDIA_PAD_FL_SOURCE;
+		mctl->media_pad[2].flags = MEDIA_PAD_FL_SOURCE;
+		media_entity_pads_init(&mctl->media_entity, MEDIA_MIXER_PAD_MAX,
+				  mctl->media_pad);
+		ret =  media_device_register_entity(mctl->media_dev,
+						    &mctl->media_entity);
+		if (ret) {
+			kfree(mctl);
+			return ret;
+		}
+
+		mctl->intf_link = media_create_intf_link(&mctl->media_entity,
+							 &ctl_intf->intf,
+							 MEDIA_LNK_FL_ENABLED);
+		if (!mctl->intf_link) {
+			media_device_unregister_entity(&mctl->media_entity);
+			media_entity_cleanup(&mctl->media_entity);
+			kfree(mctl);
+			return -ENOMEM;
+		}
+		mctl->intf_devnode = ctl_intf;
+		mixer->media_mixer_ctl = mctl;
+	}
+	return 0;
+}
+
+static void media_snd_mixer_delete(struct snd_usb_audio *chip)
+{
+	struct usb_mixer_interface *mixer;
+	struct media_device *mdev = chip->media_dev;
+
+	if (!mdev)
+		return;
+
+	list_for_each_entry(mixer, &chip->mixer_list, list) {
+		struct media_mixer_ctl *mctl;
+
+		mctl = mixer->media_mixer_ctl;
+		if (!mixer->media_mixer_ctl)
+			continue;
+
+		if (media_devnode_is_registered(&mdev->devnode)) {
+			media_device_unregister_entity(&mctl->media_entity);
+			media_entity_cleanup(&mctl->media_entity);
+		}
+		kfree(mctl);
+		mixer->media_mixer_ctl = NULL;
+	}
+	if (media_devnode_is_registered(&mdev->devnode))
+		media_devnode_remove(chip->ctl_intf_media_devnode);
+	chip->ctl_intf_media_devnode = NULL;
+}
+
+int media_snd_device_create(struct snd_usb_audio *chip,
+			struct usb_interface *iface)
+{
+	struct media_device *mdev;
+	struct usb_device *usbdev = interface_to_usbdev(iface);
+	int ret;
+
+	mdev = media_device_get(&usbdev->dev);
+	if (!mdev)
+		return -ENOMEM;
+
+	/* Initialize media device */
+	if (!mdev->dev)
+		media_device_usb_init(mdev, usbdev, NULL);
+
+	/* register if needed, otherwise get register reference */
+	if (!media_devnode_is_registered(&mdev->devnode)) {
+		ret = media_device_register(mdev);
+		if (ret) {
+			media_device_put(mdev->dev);
+			dev_err(&usbdev->dev,
+				"Couldn't register media device. Error: %d\n",
+				ret);
+			return ret;
+		}
+	} else
+		media_device_register_ref(mdev);
+
+	/* save media device - avoid lookups */
+	chip->media_dev = mdev;
+
+	/* Create media entities for mixer and control dev */
+	ret = media_snd_mixer_init(chip);
+	if (ret) {
+		dev_err(&usbdev->dev,
+			"Couldn't create media mixer entities. Error: %d\n",
+			ret);
+
+		/* clear saved media_dev */
+		chip->media_dev = NULL;
+
+		return ret;
+	}
+	return 0;
+}
+
+void media_snd_device_delete(struct snd_usb_audio *chip)
+{
+	struct media_device *mdev = chip->media_dev;
+	struct snd_usb_stream *stream;
+
+	/* release resources */
+	list_for_each_entry(stream, &chip->pcm_list, list) {
+		media_snd_stream_delete(&stream->substream[0]);
+		media_snd_stream_delete(&stream->substream[1]);
+	}
+
+	media_snd_mixer_delete(chip);
+
+	if (mdev) {
+		media_device_unregister_put(mdev);
+		media_device_put(mdev->dev);
+		chip->media_dev = NULL;
+	}
+}
diff --git a/sound/usb/media.h b/sound/usb/media.h
new file mode 100644
index 0000000..42ce8eb
--- /dev/null
+++ b/sound/usb/media.h
@@ -0,0 +1,73 @@
+/*
+ * media.h - Media Controller specific ALSA driver code
+ *
+ * Copyright (c) 2016 Shuah Khan <shuahkh@osg.samsung.com>
+ * Copyright (c) 2016 Samsung Electronics Co., Ltd.
+ *
+ * This file is released under the GPLv2.
+ */
+
+/*
+ * This file adds Media Controller support to ALSA driver
+ * to use the Media Controller API to share tuner with DVB
+ * and V4L2 drivers that control media device. Media device
+ * is created based on existing quirks framework. Using this
+ * approach, the media controller API usage can be added for
+ * a specific device.
+*/
+#ifndef __MEDIA_H
+
+#ifdef CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER
+
+#include <media/media-device.h>
+#include <media/media-entity.h>
+#include <media/media-dev-allocator.h>
+#include <sound/asound.h>
+
+struct media_ctl {
+	struct media_device *media_dev;
+	struct media_entity media_entity;
+	struct media_intf_devnode *intf_devnode;
+	struct media_link *intf_link;
+	struct media_pad media_pad;
+	struct media_pipeline media_pipe;
+};
+
+/*
+ * One source pad each for SNDRV_PCM_STREAM_CAPTURE and
+ * SNDRV_PCM_STREAM_PLAYBACK. One for sink pad to link
+ * to AUDIO Source
+*/
+#define MEDIA_MIXER_PAD_MAX    (SNDRV_PCM_STREAM_LAST + 2)
+
+struct media_mixer_ctl {
+	struct media_device *media_dev;
+	struct media_entity media_entity;
+	struct media_intf_devnode *intf_devnode;
+	struct media_link *intf_link;
+	struct media_pad media_pad[MEDIA_MIXER_PAD_MAX];
+	struct media_pipeline media_pipe;
+};
+
+int media_snd_device_create(struct snd_usb_audio *chip,
+			    struct usb_interface *iface);
+void media_snd_device_delete(struct snd_usb_audio *chip);
+int media_snd_stream_init(struct snd_usb_substream *subs, struct snd_pcm *pcm,
+			  int stream);
+void media_snd_stream_delete(struct snd_usb_substream *subs);
+int media_snd_start_pipeline(struct snd_usb_substream *subs);
+void media_snd_stop_pipeline(struct snd_usb_substream *subs);
+#else
+static inline int media_snd_device_create(struct snd_usb_audio *chip,
+					  struct usb_interface *iface)
+						{ return 0; }
+static inline void media_snd_device_delete(struct snd_usb_audio *chip) { }
+static inline int media_snd_stream_init(struct snd_usb_substream *subs,
+					struct snd_pcm *pcm, int stream)
+						{ return 0; }
+static inline void media_snd_stream_delete(struct snd_usb_substream *subs) { }
+static inline int media_snd_start_pipeline(struct snd_usb_substream *subs)
+					{ return 0; }
+static inline void media_snd_stop_pipeline(struct snd_usb_substream *subs) { }
+#endif
+#endif /* __MEDIA_H */
diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h
index 3417ef3..f378944 100644
--- a/sound/usb/mixer.h
+++ b/sound/usb/mixer.h
@@ -3,6 +3,8 @@
 
 #include <sound/info.h>
 
+struct media_mixer_ctl;
+
 struct usb_mixer_interface {
 	struct snd_usb_audio *chip;
 	struct usb_host_interface *hostif;
@@ -22,6 +24,7 @@ struct usb_mixer_interface {
 	struct urb *rc_urb;
 	struct usb_ctrlrequest *rc_setup_packet;
 	u8 rc_buffer[6];
+	struct media_mixer_ctl *media_mixer_ctl;
 };
 
 #define MAX_CHANNELS	16	/* max logical channels */
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 44d178e..0e4e064 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -35,6 +35,7 @@
 #include "pcm.h"
 #include "clock.h"
 #include "power.h"
+#include "media.h"
 
 #define SUBSTREAM_FLAG_DATA_EP_STARTED	0
 #define SUBSTREAM_FLAG_SYNC_EP_STARTED	1
@@ -717,10 +718,14 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream,
 	struct audioformat *fmt;
 	int ret;
 
+	ret = media_snd_start_pipeline(subs);
+	if (ret)
+		return ret;
+
 	ret = snd_pcm_lib_alloc_vmalloc_buffer(substream,
 					       params_buffer_bytes(hw_params));
 	if (ret < 0)
-		return ret;
+		goto err_ret;
 
 	subs->pcm_format = params_format(hw_params);
 	subs->period_bytes = params_period_bytes(hw_params);
@@ -734,22 +739,27 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream,
 		dev_dbg(&subs->dev->dev,
 			"cannot set format: format = %#x, rate = %d, channels = %d\n",
 			   subs->pcm_format, subs->cur_rate, subs->channels);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_ret;
 	}
 
 	ret = snd_usb_lock_shutdown(subs->stream->chip);
 	if (ret < 0)
-		return ret;
+		goto err_ret;
 	ret = set_format(subs, fmt);
 	snd_usb_unlock_shutdown(subs->stream->chip);
 	if (ret < 0)
-		return ret;
+		goto err_ret;
 
 	subs->interface = fmt->iface;
 	subs->altset_idx = fmt->altset_idx;
 	subs->need_setup_ep = true;
 
 	return 0;
+
+err_ret:
+	media_snd_stop_pipeline(subs);
+	return ret;
 }
 
 /*
@@ -761,6 +771,7 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream)
 {
 	struct snd_usb_substream *subs = substream->runtime->private_data;
 
+	media_snd_stop_pipeline(subs);
 	subs->cur_audiofmt = NULL;
 	subs->cur_rate = 0;
 	subs->period_bytes = 0;
@@ -1221,6 +1232,7 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream, int direction)
 	struct snd_usb_stream *as = snd_pcm_substream_chip(substream);
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_usb_substream *subs = &as->substream[direction];
+	int ret;
 
 	subs->interface = -1;
 	subs->altset_idx = 0;
@@ -1234,7 +1246,12 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream, int direction)
 	subs->dsd_dop.channel = 0;
 	subs->dsd_dop.marker = 1;
 
-	return setup_hw_info(runtime, subs);
+	ret = setup_hw_info(runtime, subs);
+	if (ret == 0)
+		ret = media_snd_stream_init(subs, as->pcm, direction);
+	if (ret)
+		snd_usb_autosuspend(subs->stream->chip);
+	return ret;
 }
 
 static int snd_usb_pcm_close(struct snd_pcm_substream *substream, int direction)
@@ -1243,6 +1260,7 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream, int direction)
 	struct snd_usb_substream *subs = &as->substream[direction];
 
 	stop_endpoints(subs, true);
+	media_snd_stop_pipeline(subs);
 
 	if (subs->interface >= 0 &&
 	    !snd_usb_lock_shutdown(subs->stream->chip)) {
diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h
index c60a776..9d087b1 100644
--- a/sound/usb/quirks-table.h
+++ b/sound/usb/quirks-table.h
@@ -2886,6 +2886,7 @@ YAMAHA_DEVICE(0x7010, "UB99"),
 		.product_name = pname, \
 		.ifnum = QUIRK_ANY_INTERFACE, \
 		.type = QUIRK_AUDIO_ALIGN_TRANSFER, \
+		.media_device = 1, \
 	} \
 }
 
diff --git a/sound/usb/stream.c b/sound/usb/stream.c
index c4dc577..51258a1 100644
--- a/sound/usb/stream.c
+++ b/sound/usb/stream.c
@@ -36,6 +36,7 @@
 #include "format.h"
 #include "clock.h"
 #include "stream.h"
+#include "media.h"
 
 /*
  * free a substream
@@ -52,6 +53,7 @@ static void free_substream(struct snd_usb_substream *subs)
 		kfree(fp);
 	}
 	kfree(subs->rate_list.list);
+	media_snd_stream_delete(subs);
 }
 
 
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
index b665d85..a161c7c 100644
--- a/sound/usb/usbaudio.h
+++ b/sound/usb/usbaudio.h
@@ -30,6 +30,9 @@
  *
  */
 
+struct media_device;
+struct media_intf_devnode;
+
 struct snd_usb_audio {
 	int index;
 	struct usb_device *dev;
@@ -60,6 +63,8 @@ struct snd_usb_audio {
 	bool autoclock;			/* from the 'autoclock' module param */
 
 	struct usb_host_interface *ctrl_intf;	/* the audio control interface */
+	struct media_device *media_dev;
+	struct media_intf_devnode *ctl_intf_media_devnode;
 };
 
 #define usb_audio_err(chip, fmt, args...) \
@@ -110,6 +115,7 @@ struct snd_usb_audio_quirk {
 	const char *product_name;
 	int16_t ifnum;
 	uint16_t type;
+	bool media_device;
 	const void *data;
 };
 
-- 
2.5.0

Re: [RFC PATCH v2 0/5] Media Device Allocator API

[Takashi Iwai]

On Tue, 05 Apr 2016 05:35:55 +0200,
Shuah Khan wrote:
> 
> There are known problems with media device life time management. When media
> device is released while an media ioctl is in progress, ioctls fail with
> use-after-free errors and kernel hangs in some cases.
> 
> Media Device can be in any the following states:
> 
> - Allocated
> - Registered (could be tied to more than one driver)
> - Unregistered, not in use (media device file is not open)
> - Unregistered, in use (media device file is not open)
> - Released
> 
> When media device belongs to  more than one driver, registrations should be
> tracked to avoid unregistering when one of the drivers does unregister. A new
> num_drivers field in the struct media_device covers this case. The media device
> should be unregistered only when the last unregister occurs with num_drivers
> count zero.
> 
> When a media device is in use when it is unregistered, it should not be
> released until the application exits when it detects the unregistered
> status. Media device that is in use when it is unregistered is moved to
> to_delete_list. When the last unregister occurs, media device is unregistered
> and becomes an unregistered, still allocated device. Unregister marks the
> device to be deleted.
> 
> When media device belongs to more than one driver, as both drivers could be
> unbound/bound, driver should not end up getting stale media device that is
> on its way out. Moving the unregistered media device to to_delete_list helps
> this case as well.
> 
> I ran bind/unbind loop tests on uvcvideo, au0828, and snd-usb-audio while
> running application that does ioctls. Didn't see any use-after-free errors
> on media device. A couple of known issues seen:
> 
> 1. When application exits, cdev_put() gets called after media device is
>    released. This is a known issue to resolve and Media Device Allocator
>    can't solve this one.
> 2. When au0828 module is removed and then ioctls fail when cdev_get() looks
>    for the owning module as au0828 is very often the module that owns the
>    media devnode. This is a cdev related issue that needs to be resolved and
>    Media Device Allocator can't solve this one.
> 
> Shuah Khan (5):
>   media: Add Media Device Allocator API
>   media: Add driver count to keep track of media device registrations
>   media: uvcvideo change to use Media Device Allocator API
>   media: au0828 change to use Media Device Allocator API
>   sound/usb: Use Media Controller API to share media resources

I don't think we need to include usb-audio patch at this stage yet.
The most important thing for now is to improve / stabilize the API
itself so that other drivers can use it as is.  Once when the API is
really stabilized, we create a solid git branch that may be based for
multiple subsystems, and I'll merge usb-audio stuff through sound git
tree.

Also, the previous usb-audio MC implementation had a few serious bugs,
including quirk NULL dereference.  See the bugzilla below for some fix
patches to 4.6-rc1:
  https://bugzilla.kernel.org/show_bug.cgi?id=115561
Feel free to fold them in, if they are still valid.


thanks,

Takashi

Re: [RFC PATCH v2 0/5] Media Device Allocator API

[Shuah Khan]

On 04/05/2016 12:10 AM, Takashi Iwai wrote:
> On Tue, 05 Apr 2016 05:35:55 +0200,
> Shuah Khan wrote:
>>
>> There are known problems with media device life time management. When media
>> device is released while an media ioctl is in progress, ioctls fail with
>> use-after-free errors and kernel hangs in some cases.
>>
>> Media Device can be in any the following states:
>>
>> - Allocated
>> - Registered (could be tied to more than one driver)
>> - Unregistered, not in use (media device file is not open)
>> - Unregistered, in use (media device file is not open)
>> - Released
>>
>> When media device belongs to  more than one driver, registrations should be
>> tracked to avoid unregistering when one of the drivers does unregister. A new
>> num_drivers field in the struct media_device covers this case. The media device
>> should be unregistered only when the last unregister occurs with num_drivers
>> count zero.
>>
>> When a media device is in use when it is unregistered, it should not be
>> released until the application exits when it detects the unregistered
>> status. Media device that is in use when it is unregistered is moved to
>> to_delete_list. When the last unregister occurs, media device is unregistered
>> and becomes an unregistered, still allocated device. Unregister marks the
>> device to be deleted.
>>
>> When media device belongs to more than one driver, as both drivers could be
>> unbound/bound, driver should not end up getting stale media device that is
>> on its way out. Moving the unregistered media device to to_delete_list helps
>> this case as well.
>>
>> I ran bind/unbind loop tests on uvcvideo, au0828, and snd-usb-audio while
>> running application that does ioctls. Didn't see any use-after-free errors
>> on media device. A couple of known issues seen:
>>
>> 1. When application exits, cdev_put() gets called after media device is
>>    released. This is a known issue to resolve and Media Device Allocator
>>    can't solve this one.
>> 2. When au0828 module is removed and then ioctls fail when cdev_get() looks
>>    for the owning module as au0828 is very often the module that owns the
>>    media devnode. This is a cdev related issue that needs to be resolved and
>>    Media Device Allocator can't solve this one.
>>
>> Shuah Khan (5):
>>   media: Add Media Device Allocator API
>>   media: Add driver count to keep track of media device registrations
>>   media: uvcvideo change to use Media Device Allocator API
>>   media: au0828 change to use Media Device Allocator API
>>   sound/usb: Use Media Controller API to share media resources
> 
> I don't think we need to include usb-audio patch at this stage yet.
> The most important thing for now is to improve / stabilize the API
> itself so that other drivers can use it as is.  Once when the API is
> really stabilized, we create a solid git branch that may be based for
> multiple subsystems, and I'll merge usb-audio stuff through sound git
> tree.

Agreed. I included snd-usb-audio as it provides a good test case for
multiple driver use-case. Yes it is a good idea to have a git branch
for wider testing.

> 
> Also, the previous usb-audio MC implementation had a few serious bugs,
> including quirk NULL dereference.  See the bugzilla below for some fix
> patches to 4.6-rc1:
>   https://bugzilla.kernel.org/show_bug.cgi?id=115561
> Feel free to fold them in, if they are still valid.

I folded them in. It is unfortunate that these bugs were introduced towards
the end when I was making changes to address review comments and I didn't
catch them, especially the quirk NULL dereference.

thanks,
-- Shuah

Re: [RFC PATCH v2 0/5] Media Device Allocator API

[Mauro Carvalho Chehab]

Em Tue, 5 Apr 2016 08:10:11 +0200
Takashi Iwai <tiwai@suse.de> escreveu:

> On Tue, 05 Apr 2016 05:35:55 +0200,
> Shuah Khan wrote:
> > 
> > There are known problems with media device life time management. When media
> > device is released while an media ioctl is in progress, ioctls fail with
> > use-after-free errors and kernel hangs in some cases.
> > 
> > Media Device can be in any the following states:
> > 
> > - Allocated
> > - Registered (could be tied to more than one driver)
> > - Unregistered, not in use (media device file is not open)
> > - Unregistered, in use (media device file is not open)
> > - Released
> > 
> > When media device belongs to  more than one driver, registrations should be
> > tracked to avoid unregistering when one of the drivers does unregister. A new
> > num_drivers field in the struct media_device covers this case. The media device
> > should be unregistered only when the last unregister occurs with num_drivers
> > count zero.
> > 
> > When a media device is in use when it is unregistered, it should not be
> > released until the application exits when it detects the unregistered
> > status. Media device that is in use when it is unregistered is moved to
> > to_delete_list. When the last unregister occurs, media device is unregistered
> > and becomes an unregistered, still allocated device. Unregister marks the
> > device to be deleted.
> > 
> > When media device belongs to more than one driver, as both drivers could be
> > unbound/bound, driver should not end up getting stale media device that is
> > on its way out. Moving the unregistered media device to to_delete_list helps
> > this case as well.
> > 
> > I ran bind/unbind loop tests on uvcvideo, au0828, and snd-usb-audio while
> > running application that does ioctls. Didn't see any use-after-free errors
> > on media device. A couple of known issues seen:
> > 
> > 1. When application exits, cdev_put() gets called after media device is
> >    released. This is a known issue to resolve and Media Device Allocator
> >    can't solve this one.
> > 2. When au0828 module is removed and then ioctls fail when cdev_get() looks
> >    for the owning module as au0828 is very often the module that owns the
> >    media devnode. This is a cdev related issue that needs to be resolved and
> >    Media Device Allocator can't solve this one.
> > 
> > Shuah Khan (5):
> >   media: Add Media Device Allocator API
> >   media: Add driver count to keep track of media device registrations
> >   media: uvcvideo change to use Media Device Allocator API
> >   media: au0828 change to use Media Device Allocator API
> >   sound/usb: Use Media Controller API to share media resources  
> 
> I don't think we need to include usb-audio patch at this stage yet.

I agree. Let's first fix MC races first, then address the multi-driver
issues. Only after having those fixed, we should look at the sound/usb
patch.

Ok, we could keep it on some testing tree, but, IMHO,  it doesn't make any
sense to submit it to review, while the core is not fixed.

> The most important thing for now is to improve / stabilize the API
> itself so that other drivers can use it as is.  Once when the API is
> really stabilized, we create a solid git branch that may be based for
> multiple subsystems, and I'll merge usb-audio stuff through sound git
> tree.

Works for me. After we have this properly fixed and stabilized on media,
I'll pass you a stable topic branch for you. This way, you can test a
new version of the sound/usb patch and apply on your tree when it fits
well for you.

> 
> Also, the previous usb-audio MC implementation had a few serious bugs,
> including quirk NULL dereference.  See the bugzilla below for some fix
> patches to 4.6-rc1:
>   https://bugzilla.kernel.org/show_bug.cgi?id=115561
> Feel free to fold them in, if they are still valid.
> 
> 
> thanks,
> 
> Takashi


-- 

Cheers,
Mauro