linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Media Device Allocator API
@ 2016-05-24 23:39 Shuah Khan
  2016-05-24 23:39 ` [PATCH v2 1/2] media: " Shuah Khan
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Shuah Khan @ 2016-05-24 23:39 UTC (permalink / raw)
  To: mchehab, laurent.pinchart, sakari.ailus, hans.verkuil,
	chehabrafael, javier, inki.dae, g.liakhovetski, jh1009.sung
  Cc: Shuah Khan, linux-media, linux-kernel

Media Device Allocator API to allows multiple drivers share a media device.
Using this API, drivers can allocate a media device with the shared struct
device as the key. Once the media device is allocated by a driver, other
drivers can get a reference to it. The media device is released when all
the references are released.

This patch series has been tested with au0828 and snd-usb-audio drivers.
snd-us-audio patch isn't included in this series. Once this patch series
is reviews and gets a stable state, I will send out the snd-usb-audio
patch.

Changes since Patch v1 series: (based on Hans Virkuil's review)
- Removed media_device_get() and media_device_allocate(). These are
  unnecessary.
- media_device_usb_allocate() holds media_device_lock to do allocate
  and initialize the media device.
- Changed media_device_put() to media_device_delete() for symmetry with
  media_device_*_allocate().
- Dropped media_device_unregister_put(). au0828 calls media_device_delete()
  instead.

Shuah Khan (2):
  media: Media Device Allocator API
  media: change au0828 to use Media Device Allocator API

 drivers/media/Makefile                 |   3 +-
 drivers/media/media-dev-allocator.c    | 120 +++++++++++++++++++++++++++++++++
 drivers/media/usb/au0828/au0828-core.c |  12 ++--
 drivers/media/usb/au0828/au0828.h      |   1 +
 include/media/media-dev-allocator.h    |  85 +++++++++++++++++++++++
 5 files changed, 212 insertions(+), 9 deletions(-)
 create mode 100644 drivers/media/media-dev-allocator.c
 create mode 100644 include/media/media-dev-allocator.h

-- 
2.7.4

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] media: Media Device Allocator API
  2016-05-24 23:39 [PATCH v2 0/2] Media Device Allocator API Shuah Khan
@ 2016-05-24 23:39 ` Shuah Khan
  2016-05-27 13:26   ` Hans Verkuil
  2016-05-24 23:39 ` [PATCH v2 2/2] media: change au0828 to use " Shuah Khan
  2016-06-28 19:56 ` [PATCH v2 0/2] " Shuah Khan
  2 siblings, 1 reply; 6+ messages in thread
From: Shuah Khan @ 2016-05-24 23:39 UTC (permalink / raw)
  To: mchehab, laurent.pinchart, sakari.ailus, hans.verkuil,
	chehabrafael, javier, inki.dae, g.liakhovetski, jh1009.sung
  Cc: Shuah Khan, linux-media, linux-kernel

Media Device Allocator API to allows multiple drivers share a media device.
Using this API, drivers can allocate a media device with the shared struct
device as the key. Once the media device is allocated by a driver, other
drivers can get a reference to it. The media device is released when all
the references are released.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 drivers/media/Makefile              |   3 +-
 drivers/media/media-dev-allocator.c | 120 ++++++++++++++++++++++++++++++++++++
 include/media/media-dev-allocator.h |  85 +++++++++++++++++++++++++
 3 files changed, 207 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..b8c9811
--- /dev/null
+++ b/drivers/media/media-dev-allocator.c
@@ -0,0 +1,120 @@
+/*
+ * media-dev-allocator.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 a global refcounted Media Controller Device Instance API.
+ * A system wide global media device list is managed and each media device
+ * includes a kref count. The last put on the media device releases the media
+ * device instance.
+ *
+*/
+
+#include <linux/slab.h>
+#include <linux/kref.h>
+#include <linux/usb.h>
+#include <media/media-device.h>
+
+static LIST_HEAD(media_device_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;
+};
+
+static inline struct media_device_instance *
+to_media_device_instance(struct media_device *mdev)
+{
+	return container_of(mdev, struct media_device_instance, mdev);
+}
+
+static void media_device_instance_release(struct kref *kref)
+{
+	struct media_device_instance *mdi =
+		container_of(kref, struct media_device_instance, refcount);
+
+	dev_dbg(mdi->mdev.dev, "%s: mdev=%p\n", __func__, &mdi->mdev);
+
+	mutex_lock(&media_device_lock);
+
+	media_device_unregister(&mdi->mdev);
+	media_device_cleanup(&mdi->mdev);
+
+	list_del(&mdi->list);
+	mutex_unlock(&media_device_lock);
+
+	kfree(mdi);
+}
+
+static struct media_device *__media_device_get(struct device *dev,
+					       bool allocate)
+{
+	struct media_device_instance *mdi;
+
+	list_for_each_entry(mdi, &media_device_list, list) {
+		if (mdi->dev == dev) {
+			kref_get(&mdi->refcount);
+			dev_dbg(dev, "%s: get 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);
+
+	dev_dbg(dev, "%s: alloc mdev=%p\n", __func__, &mdi->mdev);
+done:
+	return mdi ? &mdi->mdev : NULL;
+}
+
+struct media_device *media_device_usb_allocate(struct usb_device *udev,
+					       char *driver_name)
+{
+	struct media_device *mdev;
+
+	mutex_lock(&media_device_lock);
+	mdev = __media_device_get(&udev->dev, true);
+	if (!mdev) {
+		mutex_unlock(&media_device_lock);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	/* check if media device is already initialized */
+	if (!mdev->dev)
+		__media_device_usb_init(mdev, udev, udev->product,
+					driver_name);
+	mutex_unlock(&media_device_lock);
+
+	dev_dbg(&udev->dev, "%s\n", __func__);
+	return mdev;
+}
+EXPORT_SYMBOL_GPL(media_device_usb_allocate);
+
+void media_device_delete(struct media_device *mdev)
+{
+	struct media_device_instance *mdi = to_media_device_instance(mdev);
+
+	dev_dbg(mdi->mdev.dev, "%s: mdev=%p\n", __func__, &mdi->mdev);
+	kref_put(&mdi->refcount, media_device_instance_release);
+}
+EXPORT_SYMBOL_GPL(media_device_delete);
diff --git a/include/media/media-dev-allocator.h b/include/media/media-dev-allocator.h
new file mode 100644
index 0000000..fda032b
--- /dev/null
+++ b/include/media/media-dev-allocator.h
@@ -0,0 +1,85 @@
+/*
+ * media-dev-allocator.h - 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 a global ref-counted Media Controller Device Instance API.
+ * A system wide global media device list is managed and each media device
+ * includes a kref count. The last put on the media device releases the media
+ * device instance.
+*/
+
+#ifndef _MEDIA_DEV_ALLOCTOR_H
+#define _MEDIA_DEV_ALLOCTOR_H
+
+struct usb_device;
+
+#ifdef CONFIG_MEDIA_CONTROLLER
+/**
+ * DOC: Media Controller Device Allocator API
+ *
+ * When media device belongs to more than one driver, the shared media device
+ * is allocated with the shared struct device as the key for look ups.
+ *
+ * Shared media device should stay in registered state until the last driver
+ * unregisters it. In addition, media device should be released when all the
+ * references are released. Each driver gets a reference to the media device
+ * during probe, when it allocates the media device. If media device is already
+ * allocated, allocate API bumps up the refcount and return the existing media
+ * device. Driver puts the reference back from its disconnect routine when it
+ * calls media_device_delete().
+ *
+ * Media device is unregistered and cleaned up from the kref put handler to
+ * ensure that the media device stays in registered state until the last driver
+ * unregisters the media device.
+ *
+ * Driver Usage:
+ *
+ * Drivers should use the media-core routines to get register reference and
+ * call media_device_delete() routine to make sure the shared media device
+ * delete is handled correctly.
+ *
+ * driver probe:
+ *	Call media_device_usb_allocate() to allocate or get a reference
+ *	Call media_device_register(), if media devnode isn't registered
+ *
+ * driver disconnect:
+ *	Call media_device_delete() to free the media_device. Free'ing is
+ *	handled by the put handler.
+ *
+ */
+
+/**
+ * media_device_usb_allocate() - Allocate and return media device
+ *
+ * @udev - struct usb_device pointer
+ * @driver_name
+ *
+ * This interface should be called to allocate a media device when multiple
+ * drivers share usb_device and the media device. This interface allocates
+ * media device and calls media_device_usb_init() to initialize it.
+ *
+ */
+struct media_device *media_device_usb_allocate(struct usb_device *udev,
+					       char *driver_name);
+/**
+ * media_device_delete() - Release media device. Calls kref_put().
+ *
+ * @mdev - struct media_device pointer
+ *
+ * This interface should be called to put Media Device Instance kref.
+ */
+void media_device_delete(struct media_device *mdev);
+#else
+static inline struct media_device *media_device_usb_allocate(
+			struct usb_device *udev, char *driver_name)
+			{ return NULL; }
+static inline void media_device_delete(struct media_device *mdev) { }
+#endif /* CONFIG_MEDIA_CONTROLLER */
+#endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] media: change au0828 to use Media Device Allocator API
  2016-05-24 23:39 [PATCH v2 0/2] Media Device Allocator API Shuah Khan
  2016-05-24 23:39 ` [PATCH v2 1/2] media: " Shuah Khan
@ 2016-05-24 23:39 ` Shuah Khan
  2016-06-28 19:56 ` [PATCH v2 0/2] " Shuah Khan
  2 siblings, 0 replies; 6+ messages in thread
From: Shuah Khan @ 2016-05-24 23:39 UTC (permalink / raw)
  To: mchehab, laurent.pinchart, sakari.ailus, hans.verkuil,
	chehabrafael, javier, inki.dae, g.liakhovetski, jh1009.sung
  Cc: Shuah Khan, linux-media, linux-kernel

Change au0828 to use Media Device Allocator API to allocate media device
with the parent usb struct device as the key, so it can be shared with the
snd usb audio driver.

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

diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c
index bf53553..fc5f122 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -157,9 +157,7 @@ static void au0828_unregister_media_device(struct au0828_dev *dev)
 	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);
+	media_device_delete(dev->media_dev);
 	dev->media_dev = NULL;
 #endif
 }
@@ -212,14 +210,10 @@ 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_usb_allocate(udev, KBUILD_MODNAME);
 	if (!mdev)
 		return -ENOMEM;
 
-	/* check if media device is already initialized */
-	if (!mdev->dev)
-		media_device_usb_init(mdev, udev, udev->product);
-
 	dev->media_dev = mdev;
 #endif
 	return 0;
@@ -487,6 +481,8 @@ static int au0828_media_device_register(struct au0828_dev *dev,
 		/* register media device */
 		ret = media_device_register(dev->media_dev);
 		if (ret) {
+			media_device_delete(dev->media_dev);
+			dev->media_dev = NULL;
 			dev_err(&udev->dev,
 				"Media Device Register Error: %d\n", ret);
 			return ret;
diff --git a/drivers/media/usb/au0828/au0828.h b/drivers/media/usb/au0828/au0828.h
index dd7b378..4bf1b0c 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.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] media: Media Device Allocator API
  2016-05-24 23:39 ` [PATCH v2 1/2] media: " Shuah Khan
@ 2016-05-27 13:26   ` Hans Verkuil
  2016-05-27 20:44     ` Shuah Khan
  0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2016-05-27 13:26 UTC (permalink / raw)
  To: Shuah Khan, mchehab, laurent.pinchart, sakari.ailus,
	hans.verkuil, chehabrafael, javier, inki.dae, g.liakhovetski,
	jh1009.sung
  Cc: linux-media, linux-kernel

On 05/25/2016 01:39 AM, Shuah Khan wrote:
> Media Device Allocator API to allows multiple drivers share a media device.
> Using this API, drivers can allocate a media device with the shared struct
> device as the key. Once the media device is allocated by a driver, other
> drivers can get a reference to it. The media device is released when all
> the references are released.
> 
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> ---
>  drivers/media/Makefile              |   3 +-
>  drivers/media/media-dev-allocator.c | 120 ++++++++++++++++++++++++++++++++++++
>  include/media/media-dev-allocator.h |  85 +++++++++++++++++++++++++
>  3 files changed, 207 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..b8c9811
> --- /dev/null
> +++ b/drivers/media/media-dev-allocator.c
> @@ -0,0 +1,120 @@
> +/*
> + * media-dev-allocator.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 a global refcounted Media Controller Device Instance API.
> + * A system wide global media device list is managed and each media device
> + * includes a kref count. The last put on the media device releases the media
> + * device instance.
> + *
> +*/
> +
> +#include <linux/slab.h>
> +#include <linux/kref.h>
> +#include <linux/usb.h>
> +#include <media/media-device.h>
> +
> +static LIST_HEAD(media_device_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;
> +};
> +
> +static inline struct media_device_instance *
> +to_media_device_instance(struct media_device *mdev)
> +{
> +	return container_of(mdev, struct media_device_instance, mdev);
> +}
> +
> +static void media_device_instance_release(struct kref *kref)
> +{
> +	struct media_device_instance *mdi =
> +		container_of(kref, struct media_device_instance, refcount);
> +
> +	dev_dbg(mdi->mdev.dev, "%s: mdev=%p\n", __func__, &mdi->mdev);
> +
> +	mutex_lock(&media_device_lock);
> +
> +	media_device_unregister(&mdi->mdev);
> +	media_device_cleanup(&mdi->mdev);
> +
> +	list_del(&mdi->list);
> +	mutex_unlock(&media_device_lock);
> +
> +	kfree(mdi);
> +}
> +

Add a comment here saying that media_device_lock has to be locked when
calling this get function.

> +static struct media_device *__media_device_get(struct device *dev,
> +					       bool allocate)

I think you can just drop the allocate argument: you're always allocating if
the media_device wasn't found.

> +{
> +	struct media_device_instance *mdi;
> +
> +	list_for_each_entry(mdi, &media_device_list, list) {
> +		if (mdi->dev == dev) {
> +			kref_get(&mdi->refcount);
> +			dev_dbg(dev, "%s: get mdev=%p\n",
> +				 __func__, &mdi->mdev);
> +			goto done;

Just say 'return &mdi->mdev;', no goto needed.

> +		}
> +	}
> +
> +	if (!allocate) {
> +		mdi = NULL;
> +		goto done;
> +	}
> +
> +	mdi = kzalloc(sizeof(*mdi), GFP_KERNEL);
> +	if (!mdi)
> +		goto done;

Same here, just do 'return NULL;'

> +
> +	mdi->dev = dev;
> +	kref_init(&mdi->refcount);
> +	list_add_tail(&mdi->list, &media_device_list);
> +
> +	dev_dbg(dev, "%s: alloc mdev=%p\n", __func__, &mdi->mdev);
> +done:
> +	return mdi ? &mdi->mdev : NULL;
> +}
> +
> +struct media_device *media_device_usb_allocate(struct usb_device *udev,
> +					       char *driver_name)
> +{
> +	struct media_device *mdev;
> +
> +	mutex_lock(&media_device_lock);
> +	mdev = __media_device_get(&udev->dev, true);
> +	if (!mdev) {
> +		mutex_unlock(&media_device_lock);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	/* check if media device is already initialized */
> +	if (!mdev->dev)
> +		__media_device_usb_init(mdev, udev, udev->product,
> +					driver_name);
> +	mutex_unlock(&media_device_lock);
> +
> +	dev_dbg(&udev->dev, "%s\n", __func__);
> +	return mdev;
> +}
> +EXPORT_SYMBOL_GPL(media_device_usb_allocate);
> +
> +void media_device_delete(struct media_device *mdev)
> +{
> +	struct media_device_instance *mdi = to_media_device_instance(mdev);
> +
> +	dev_dbg(mdi->mdev.dev, "%s: mdev=%p\n", __func__, &mdi->mdev);
> +	kref_put(&mdi->refcount, media_device_instance_release);
> +}
> +EXPORT_SYMBOL_GPL(media_device_delete);
> diff --git a/include/media/media-dev-allocator.h b/include/media/media-dev-allocator.h
> new file mode 100644
> index 0000000..fda032b
> --- /dev/null
> +++ b/include/media/media-dev-allocator.h
> @@ -0,0 +1,85 @@
> +/*
> + * media-dev-allocator.h - 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 a global ref-counted Media Controller Device Instance API.
> + * A system wide global media device list is managed and each media device
> + * includes a kref count. The last put on the media device releases the media
> + * device instance.
> +*/
> +
> +#ifndef _MEDIA_DEV_ALLOCTOR_H
> +#define _MEDIA_DEV_ALLOCTOR_H
> +
> +struct usb_device;
> +
> +#ifdef CONFIG_MEDIA_CONTROLLER
> +/**
> + * DOC: Media Controller Device Allocator API
> + *
> + * When media device belongs to more than one driver, the shared media device
> + * is allocated with the shared struct device as the key for look ups.
> + *
> + * Shared media device should stay in registered state until the last driver
> + * unregisters it. In addition, media device should be released when all the
> + * references are released. Each driver gets a reference to the media device
> + * during probe, when it allocates the media device. If media device is already
> + * allocated, allocate API bumps up the refcount and return the existing media
> + * device. Driver puts the reference back from its disconnect routine when it
> + * calls media_device_delete().
> + *
> + * Media device is unregistered and cleaned up from the kref put handler to
> + * ensure that the media device stays in registered state until the last driver
> + * unregisters the media device.
> + *
> + * Driver Usage:
> + *
> + * Drivers should use the media-core routines to get register reference and
> + * call media_device_delete() routine to make sure the shared media device
> + * delete is handled correctly.
> + *
> + * driver probe:
> + *	Call media_device_usb_allocate() to allocate or get a reference
> + *	Call media_device_register(), if media devnode isn't registered
> + *
> + * driver disconnect:
> + *	Call media_device_delete() to free the media_device. Free'ing is
> + *	handled by the put handler.
> + *
> + */
> +
> +/**
> + * media_device_usb_allocate() - Allocate and return media device
> + *
> + * @udev - struct usb_device pointer
> + * @driver_name
> + *
> + * This interface should be called to allocate a media device when multiple
> + * drivers share usb_device and the media device. This interface allocates
> + * media device and calls media_device_usb_init() to initialize it.
> + *
> + */
> +struct media_device *media_device_usb_allocate(struct usb_device *udev,
> +					       char *driver_name);
> +/**
> + * media_device_delete() - Release media device. Calls kref_put().
> + *
> + * @mdev - struct media_device pointer
> + *
> + * This interface should be called to put Media Device Instance kref.
> + */
> +void media_device_delete(struct media_device *mdev);
> +#else
> +static inline struct media_device *media_device_usb_allocate(
> +			struct usb_device *udev, char *driver_name)
> +			{ return NULL; }
> +static inline void media_device_delete(struct media_device *mdev) { }
> +#endif /* CONFIG_MEDIA_CONTROLLER */
> +#endif
> 

Regards,

	Hans

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] media: Media Device Allocator API
  2016-05-27 13:26   ` Hans Verkuil
@ 2016-05-27 20:44     ` Shuah Khan
  0 siblings, 0 replies; 6+ messages in thread
From: Shuah Khan @ 2016-05-27 20:44 UTC (permalink / raw)
  To: Hans Verkuil, mchehab, laurent.pinchart, sakari.ailus,
	hans.verkuil, chehabrafael, javier, inki.dae, g.liakhovetski,
	jh1009.sung
  Cc: linux-media, linux-kernel, Shuah Khan

On 05/27/2016 07:26 AM, Hans Verkuil wrote:
> On 05/25/2016 01:39 AM, Shuah Khan wrote:
>> Media Device Allocator API to allows multiple drivers share a media device.
>> Using this API, drivers can allocate a media device with the shared struct
>> device as the key. Once the media device is allocated by a driver, other
>> drivers can get a reference to it. The media device is released when all
>> the references are released.
>>
>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>> ---
>>  drivers/media/Makefile              |   3 +-
>>  drivers/media/media-dev-allocator.c | 120 ++++++++++++++++++++++++++++++++++++
>>  include/media/media-dev-allocator.h |  85 +++++++++++++++++++++++++
>>  3 files changed, 207 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..b8c9811
>> --- /dev/null
>> +++ b/drivers/media/media-dev-allocator.c
>> @@ -0,0 +1,120 @@
>> +/*
>> + * media-dev-allocator.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 a global refcounted Media Controller Device Instance API.
>> + * A system wide global media device list is managed and each media device
>> + * includes a kref count. The last put on the media device releases the media
>> + * device instance.
>> + *
>> +*/
>> +
>> +#include <linux/slab.h>
>> +#include <linux/kref.h>
>> +#include <linux/usb.h>
>> +#include <media/media-device.h>
>> +
>> +static LIST_HEAD(media_device_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;
>> +};
>> +
>> +static inline struct media_device_instance *
>> +to_media_device_instance(struct media_device *mdev)
>> +{
>> +	return container_of(mdev, struct media_device_instance, mdev);
>> +}
>> +
>> +static void media_device_instance_release(struct kref *kref)
>> +{
>> +	struct media_device_instance *mdi =
>> +		container_of(kref, struct media_device_instance, refcount);
>> +
>> +	dev_dbg(mdi->mdev.dev, "%s: mdev=%p\n", __func__, &mdi->mdev);
>> +
>> +	mutex_lock(&media_device_lock);
>> +
>> +	media_device_unregister(&mdi->mdev);
>> +	media_device_cleanup(&mdi->mdev);
>> +
>> +	list_del(&mdi->list);
>> +	mutex_unlock(&media_device_lock);
>> +
>> +	kfree(mdi);
>> +}
>> +
> 
> Add a comment here saying that media_device_lock has to be locked when
> calling this get function.
> 

Fixed all the comments and sent patch v3. Thanks again for the review.

-- Shuah

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 0/2] Media Device Allocator API
  2016-05-24 23:39 [PATCH v2 0/2] Media Device Allocator API Shuah Khan
  2016-05-24 23:39 ` [PATCH v2 1/2] media: " Shuah Khan
  2016-05-24 23:39 ` [PATCH v2 2/2] media: change au0828 to use " Shuah Khan
@ 2016-06-28 19:56 ` Shuah Khan
  2 siblings, 0 replies; 6+ messages in thread
From: Shuah Khan @ 2016-06-28 19:56 UTC (permalink / raw)
  To: mchehab, hans.verkuil, inki.dae, g.liakhovetski
  Cc: laurent.pinchart, sakari.ailus, chehabrafael, javier,
	jh1009.sung, linux-media, linux-kernel, Shuah Khan

On 05/24/2016 05:39 PM, Shuah Khan wrote:
> Media Device Allocator API to allows multiple drivers share a media device.
> Using this API, drivers can allocate a media device with the shared struct
> device as the key. Once the media device is allocated by a driver, other
> drivers can get a reference to it. The media device is released when all
> the references are released.
> 
> This patch series has been tested with au0828 and snd-usb-audio drivers.
> snd-us-audio patch isn't included in this series. Once this patch series
> is reviews and gets a stable state, I will send out the snd-usb-audio
> patch.
> 
> Changes since Patch v1 series: (based on Hans Virkuil's review)
> - Removed media_device_get() and media_device_allocate(). These are
>   unnecessary.
> - media_device_usb_allocate() holds media_device_lock to do allocate
>   and initialize the media device.
> - Changed media_device_put() to media_device_delete() for symmetry with
>   media_device_*_allocate().
> - Dropped media_device_unregister_put(). au0828 calls media_device_delete()
>   instead.
> 
> Shuah Khan (2):
>   media: Media Device Allocator API
>   media: change au0828 to use Media Device Allocator API
> 
>  drivers/media/Makefile                 |   3 +-
>  drivers/media/media-dev-allocator.c    | 120 +++++++++++++++++++++++++++++++++
>  drivers/media/usb/au0828/au0828-core.c |  12 ++--
>  drivers/media/usb/au0828/au0828.h      |   1 +
>  include/media/media-dev-allocator.h    |  85 +++++++++++++++++++++++
>  5 files changed, 212 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/media/media-dev-allocator.c
>  create mode 100644 include/media/media-dev-allocator.h
> 


Hi Mauro,

Are you planning to get this inot 4,8-rc1? 

The first patch is now at [PATCH v3] media: Media Device Allocator API
that has been reviewed by Hans.

https://lkml.org/lkml/2016/5/27/530

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-06-28 19:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-24 23:39 [PATCH v2 0/2] Media Device Allocator API Shuah Khan
2016-05-24 23:39 ` [PATCH v2 1/2] media: " Shuah Khan
2016-05-27 13:26   ` Hans Verkuil
2016-05-27 20:44     ` Shuah Khan
2016-05-24 23:39 ` [PATCH v2 2/2] media: change au0828 to use " Shuah Khan
2016-06-28 19:56 ` [PATCH v2 0/2] " Shuah Khan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).