All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] Update ALSA, and au0828 drivers to use Managed Media Controller API
@ 2015-07-15  0:33 Shuah Khan
  2015-07-15  0:33 ` [PATCH] media-ctl: Update to add support for ALSA devices Shuah Khan
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Shuah Khan @ 2015-07-15  0:33 UTC (permalink / raw)
  To: mchehab, hans.verkuil, laurent.pinchart, tiwai, perex, crope,
	sakari.ailus, arnd, stefanr, ruchandani.tina, chehabrafael,
	dan.carpenter, prabhakar.csengg, chris.j.arges, agoode,
	pierre-louis.bossart, gtmkramer, clemens, daniel, vladcatoi,
	misterpib, damien, pmatilai, takamichiho, normalperson,
	bugzilla.frnkcg, joe, calcprogrammer1, jussi
  Cc: Shuah Khan, linux-media, alsa-devel

s patch series updates ALSA driver, and au0828 core driver to
use Managed Media controller API to share tuner. Please note that
Managed Media Controller API and DVB and V4L2 drivers updates to
use Media Controller API have been added in a prior patch series.

Media Controller API is enhanced with two new interfaces to
register and unregister entity_notify hooks to allow drivers
to take appropriate actions when as new entities get added to
the shared media device.

Tested exclusion between digital, analog, and audio to ensure
when tuner has an active link to DVB FE, analog, and audio will
detect and honor the tuner busy conditions and vice versa.

This patch series has been updated to address comments from
3 previous versions of this series. Link to v3 below for
reference.

https://www.mail-archive.com/linux-media%40vger.kernel.org/msg89491.html
https://www.mail-archive.com/linux-media@vger.kernel.org/msg89492.html
https://www.mail-archive.com/linux-media%40vger.kernel.org/msg89493.html

Open Issues:

ALSA has makes media_entity_pipeline_start() call in irq
path. I am seeing warnings that the graph_mutex is unsafe
irq lock. Media Controller API updates to start/stop pipeline
to be irq safe might be necessary. Maybe there are other MC
interfaces that need to be irq safe, but I haven't seen any
problems with my testing.

So as per options, graph_mutex could be changed to a spinlock.
It looks like drivers hold this lock and it isn't abstracted to
MC API. Unfortunate, this would require changes to drivers that
directly hold the lock for graph walks if this mutex is changed
to spinlock. This issue needs to be resolved.

Note: This series includes a revert to a patch that added media
controller entity framework enhancements that implemented entity
ops for entity_notify functionality. Entity ops for entity_notify
doesn't handle and cover entity create ordering variations that
could occur during boot. entity_notify list has been moved to media
device level which makes the entity_notify calls to work correctly.

Shuah Khan (7):
  Revert "[media] media: media controller entity framework enhancements
    for ALSA"
  media: Media Controller register/unregister entity_notify API
  media: Add ALSA Media Controller devnodes
  media: change dvb-frontend to honor MC tuner enable error
  media: au8522 change to create MC pad for ALSA Audio Out
  media: au0828 change to use Managed Media Controller API
  sound/usb: Update ALSA driver to use Managed Media Controller API

 drivers/media/dvb-core/dvb_frontend.c        |   5 +-
 drivers/media/dvb-frontends/au8522.h         |   8 +
 drivers/media/dvb-frontends/au8522_decoder.c |   1 +
 drivers/media/dvb-frontends/au8522_priv.h    |   8 -
 drivers/media/media-device.c                 |  46 ++++-
 drivers/media/usb/au0828/au0828-core.c       | 132 ++++++++------
 drivers/media/usb/au0828/au0828.h            |   5 +
 include/media/media-device.h                 |  23 +++
 include/media/media-entity.h                 |   4 -
 include/uapi/linux/media.h                   |   5 +
 sound/usb/Makefile                           |  15 +-
 sound/usb/card.c                             |   5 +
 sound/usb/card.h                             |   1 +
 sound/usb/media.c                            | 260 +++++++++++++++++++++++++++
 sound/usb/media.h                            |  52 ++++++
 sound/usb/pcm.c                              |  15 +-
 sound/usb/quirks-table.h                     |   1 +
 sound/usb/quirks.c                           |   9 +-
 sound/usb/stream.c                           |   2 +
 sound/usb/usbaudio.h                         |   1 +
 20 files changed, 523 insertions(+), 75 deletions(-)
 create mode 100644 sound/usb/media.c
 create mode 100644 sound/usb/media.h

-- 
2.1.4


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

* [PATCH] media-ctl: Update to add support for ALSA devices
  2015-07-15  0:33 [PATCH 0/7] Update ALSA, and au0828 drivers to use Managed Media Controller API Shuah Khan
@ 2015-07-15  0:33 ` Shuah Khan
  2015-07-15  0:34 ` [PATCH 1/7] Revert "[media] media: media controller entity framework enhancements for ALSA" Shuah Khan
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Shuah Khan @ 2015-07-15  0:33 UTC (permalink / raw)
  To: mchehab, hans.verkuil, laurent.pinchart, tiwai, sakari.ailus
  Cc: Shuah Khan, linux-media

Add support to recognize ALSA media nodes and generate
media graph that includes ALSA pcm and mixer devices.
This patch depends on kernel Media Controller changes
to support ALSA.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 utils/media-ctl/libmediactl.c | 18 ++++++++++++------
 utils/media-ctl/media-ctl.c   |  3 +++
 utils/media-ctl/mediactl.h    |  4 +++-
 3 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
index dce8eeb..329a41a 100644
--- a/utils/media-ctl/libmediactl.c
+++ b/utils/media-ctl/libmediactl.c
@@ -153,7 +153,9 @@ struct media_entity *media_get_default_entity(struct media_device *media,
 		return media->def.v4l;
 	case MEDIA_ENT_T_DEVNODE_FB:
 		return media->def.fb;
-	case MEDIA_ENT_T_DEVNODE_ALSA:
+	case MEDIA_ENT_T_DEVNODE_ALSA_CAPTURE:
+	case MEDIA_ENT_T_DEVNODE_ALSA_PLAYBACK:
+	case MEDIA_ENT_T_DEVNODE_ALSA_MIXER:
 		return media->def.alsa;
 	case MEDIA_ENT_T_DEVNODE_DVB_FE:
 	case MEDIA_ENT_T_DEVNODE_DVB_DEMUX:
@@ -481,7 +483,6 @@ static int media_get_devname_sysfs(struct media_entity *entity)
 	if (p == NULL)
 		return -EINVAL;
 
-	sprintf(devname, "/dev/%s", p + 1);
 	if (strstr(p + 1, "dvb")) {
 		char *s = p + 1;
 
@@ -493,6 +494,8 @@ static int media_get_devname_sysfs(struct media_entity *entity)
 			return -EINVAL;
 		*p = '/';
 		sprintf(devname, "/dev/dvb/adapter%s", s);
+	} else if (strstr(p + 1, "pcm") || strstr(p + 1, "control")) {
+		sprintf(devname, "/dev/snd/%s", p + 1);
 	} else {
 		sprintf(devname, "/dev/%s", p + 1);
 	}
@@ -562,7 +565,9 @@ static int media_enum_entities(struct media_device *media)
 			case MEDIA_ENT_T_DEVNODE_FB:
 				media->def.fb = entity;
 				break;
-			case MEDIA_ENT_T_DEVNODE_ALSA:
+			case MEDIA_ENT_T_DEVNODE_ALSA_CAPTURE:
+			case MEDIA_ENT_T_DEVNODE_ALSA_PLAYBACK:
+			case MEDIA_ENT_T_DEVNODE_ALSA_MIXER:
 				media->def.alsa = entity;
 				break;
 			case MEDIA_ENT_T_DEVNODE_DVB_FE:
@@ -577,8 +582,7 @@ static int media_enum_entities(struct media_device *media)
 
 		/* Find the corresponding device name. */
 		if (media_entity_type(entity) != MEDIA_ENT_T_DEVNODE &&
-		    media_entity_type(entity) != MEDIA_ENT_T_V4L2_SUBDEV &&
-		    entity->info.type == MEDIA_ENT_T_DEVNODE_ALSA)
+		    media_entity_type(entity) != MEDIA_ENT_T_V4L2_SUBDEV)
 			continue;
 
 		/* Try to get the device name via udev */
@@ -774,7 +778,9 @@ int media_device_add_entity(struct media_device *media,
 		defent = &media->def.fb;
 		entity->info.fb = desc->fb;
 		break;
-	case MEDIA_ENT_T_DEVNODE_ALSA:
+	case MEDIA_ENT_T_DEVNODE_ALSA_CAPTURE:
+	case MEDIA_ENT_T_DEVNODE_ALSA_PLAYBACK:
+	case MEDIA_ENT_T_DEVNODE_ALSA_MIXER:
 		defent = &media->def.alsa;
 		entity->info.alsa = desc->alsa;
 		break;
diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
index 602486f..85151b5 100644
--- a/utils/media-ctl/media-ctl.c
+++ b/utils/media-ctl/media-ctl.c
@@ -290,6 +290,9 @@ static const char *media_entity_subtype_to_string(unsigned type)
 		"DVB DVR",
 		"DVB CA",
 		"DVB NET",
+		"ALSA CAPTURE",
+		"ALSA PLAYBACK",
+		"ALSA MIXER",
 	};
 	static const char *subdev_types[] = {
 		"Unknown",
diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h
index 03d9f70..3ac91eb 100644
--- a/utils/media-ctl/mediactl.h
+++ b/utils/media-ctl/mediactl.h
@@ -305,7 +305,9 @@ struct media_entity *media_get_entity(struct media_device *media, unsigned int i
  *
  *	MEDIA_ENT_T_DEVNODE_V4L
  *	MEDIA_ENT_T_DEVNODE_FB
- *	MEDIA_ENT_T_DEVNODE_ALSA
+ *	MEDIA_ENT_T_DEVNODE_ALSA_CAPTURE
+ *	MEDIA_ENT_T_DEVNODE_ALSA_PLAYBACK
+ *	MEDIA_ENT_T_DEVNODE_ALSA_MIXER
  *	MEDIA_ENT_T_DEVNODE_DVB_FE
  *	MEDIA_ENT_T_DEVNODE_DVB_DEMUX
  *	MEDIA_ENT_T_DEVNODE_DVB_DVR
-- 
2.1.4


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

* [PATCH 1/7] Revert "[media] media: media controller entity framework enhancements for ALSA"
  2015-07-15  0:33 [PATCH 0/7] Update ALSA, and au0828 drivers to use Managed Media Controller API Shuah Khan
  2015-07-15  0:33 ` [PATCH] media-ctl: Update to add support for ALSA devices Shuah Khan
@ 2015-07-15  0:34 ` Shuah Khan
  2015-07-15  0:34 ` [PATCH 2/7] media: Media Controller register/unregister entity_notify API Shuah Khan
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Shuah Khan @ 2015-07-15  0:34 UTC (permalink / raw)
  To: mchehab, hans.verkuil, laurent.pinchart, tiwai, perex, crope,
	sakari.ailus, arnd, stefanr, ruchandani.tina, chehabrafael,
	dan.carpenter, prabhakar.csengg, chris.j.arges, agoode,
	pierre-louis.bossart, gtmkramer, clemens, daniel, vladcatoi,
	misterpib, damien, pmatilai, takamichiho, normalperson,
	bugzilla.frnkcg, joe, calcprogrammer1, jussi
  Cc: Shuah Khan, linux-media, alsa-devel

This reverts commit ed64cf1e182fb30fe67652386c0880fcf3302f97.
This patch is no longer necessary as the entity register
callback is implemented at media_device level.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 drivers/media/media-device.c | 7 -------
 include/media/media-entity.h | 4 ----
 2 files changed, 11 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 76590ba..c55ab50 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -428,8 +428,6 @@ EXPORT_SYMBOL_GPL(media_device_unregister);
 int __must_check media_device_register_entity(struct media_device *mdev,
 					      struct media_entity *entity)
 {
-	struct media_entity *eptr;
-
 	/* Warn if we apparently re-register an entity */
 	WARN_ON(entity->parent != NULL);
 	entity->parent = mdev;
@@ -442,11 +440,6 @@ int __must_check media_device_register_entity(struct media_device *mdev,
 	list_add_tail(&entity->list, &mdev->entities);
 	spin_unlock(&mdev->lock);
 
-	media_device_for_each_entity(eptr, mdev) {
-		if (eptr != entity)
-			media_entity_call(eptr, register_notify);
-	}
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(media_device_register_entity);
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 0bc4c2f..0c003d8 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -46,7 +46,6 @@ struct media_pad {
 
 /**
  * struct media_entity_operations - Media entity operations
- * @register_notify	Notify entity of newly registered entity
  * @link_setup:		Notify the entity of link changes. The operation can
  *			return an error, in which case link setup will be
  *			cancelled. Optional.
@@ -55,7 +54,6 @@ struct media_pad {
  *			validates all links by calling this operation. Optional.
  */
 struct media_entity_operations {
-	int (*register_notify)(struct media_entity *entity);
 	int (*link_setup)(struct media_entity *entity,
 			  const struct media_pad *local,
 			  const struct media_pad *remote, u32 flags);
@@ -103,8 +101,6 @@ struct media_entity {
 		/* Sub-device specifications */
 		/* Nothing needed yet */
 	} info;
-
-	void *private;			/* private data for the entity */
 };
 
 static inline u32 media_entity_type(struct media_entity *entity)
-- 
2.1.4


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

* [PATCH 2/7] media: Media Controller register/unregister entity_notify API
  2015-07-15  0:33 [PATCH 0/7] Update ALSA, and au0828 drivers to use Managed Media Controller API Shuah Khan
  2015-07-15  0:33 ` [PATCH] media-ctl: Update to add support for ALSA devices Shuah Khan
  2015-07-15  0:34 ` [PATCH 1/7] Revert "[media] media: media controller entity framework enhancements for ALSA" Shuah Khan
@ 2015-07-15  0:34 ` Shuah Khan
  2015-07-15  0:34 ` [PATCH 3/7] media: Add ALSA Media Controller devnodes Shuah Khan
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Shuah Khan @ 2015-07-15  0:34 UTC (permalink / raw)
  To: mchehab, hans.verkuil, laurent.pinchart, tiwai, perex, crope,
	sakari.ailus, arnd, stefanr, ruchandani.tina, chehabrafael,
	dan.carpenter, prabhakar.csengg, chris.j.arges, agoode,
	pierre-louis.bossart, gtmkramer, clemens, daniel, vladcatoi,
	misterpib, damien, pmatilai, takamichiho, normalperson,
	bugzilla.frnkcg, joe, calcprogrammer1, jussi
  Cc: Shuah Khan, linux-media, alsa-devel

Add new interfaces to register and unregister entity_notify
hook to media device to allow drivers to take appropriate
actions when as new entities get added to the shared media
device.When a new entity is registered, all registered
entity_notify hooks are invoked to allow drivers or modules
that registered hook to take appropriate action. For example,
ALSA driver registers an entity_notify hook to parse the list
of registered entities to determine if decoder has been linked
to ALSA entity. au0828 bridge driver registers an entity_notify
hook to create media graph for the device.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 drivers/media/media-device.c | 45 ++++++++++++++++++++++++++++++++++++++++++++
 include/media/media-device.h | 23 ++++++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index c55ab50..22565a8 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -381,6 +381,7 @@ int __must_check __media_device_register(struct media_device *mdev,
 
 	mdev->entity_id = 1;
 	INIT_LIST_HEAD(&mdev->entities);
+	INIT_LIST_HEAD(&mdev->entity_notify);
 	spin_lock_init(&mdev->lock);
 	mutex_init(&mdev->graph_mutex);
 
@@ -411,9 +412,12 @@ void media_device_unregister(struct media_device *mdev)
 {
 	struct media_entity *entity;
 	struct media_entity *next;
+	struct media_entity_notify *notify, *nextp;
 
 	list_for_each_entry_safe(entity, next, &mdev->entities, list)
 		media_device_unregister_entity(entity);
+	list_for_each_entry_safe(notify, nextp, &mdev->entity_notify, list)
+		media_device_unregister_entity_notify(mdev, notify);
 
 	device_remove_file(&mdev->devnode.dev, &dev_attr_model);
 	media_devnode_unregister(&mdev->devnode);
@@ -421,6 +425,39 @@ void media_device_unregister(struct media_device *mdev)
 EXPORT_SYMBOL_GPL(media_device_unregister);
 
 /**
+ * media_device_register_entity_notify - Register a media entity notify
+ * callback with a media device. When a new entity is registered, all
+ * the registered media_entity_notify callbacks are invoked.
+ * @mdev:	The media device
+ * @nptr:	The media_entity_notify
+ */
+int __must_check media_device_register_entity_notify(struct media_device *mdev,
+					struct media_entity_notify *nptr)
+{
+	spin_lock(&mdev->lock);
+	list_add_tail(&nptr->list, &mdev->entity_notify);
+	spin_unlock(&mdev->lock);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(media_device_register_entity_notify);
+
+/**
+ * media_device_unregister_entity_notify - Unregister a media entity notify
+ * callback with a media device. When a new entity is registered, all
+ * the registered media_entity_notify callbacks are invoked.
+ * @mdev:	The media device
+ * @nptr:	The media_entity_notify
+ */
+void media_device_unregister_entity_notify(struct media_device *mdev,
+					struct media_entity_notify *nptr)
+{
+	spin_lock(&mdev->lock);
+	list_del(&nptr->list);
+	spin_unlock(&mdev->lock);
+}
+EXPORT_SYMBOL_GPL(media_device_unregister_entity_notify);
+
+/**
  * media_device_register_entity - Register an entity with a media device
  * @mdev:	The media device
  * @entity:	The entity
@@ -428,6 +465,8 @@ EXPORT_SYMBOL_GPL(media_device_unregister);
 int __must_check media_device_register_entity(struct media_device *mdev,
 					      struct media_entity *entity)
 {
+	struct media_entity_notify *notify, *next;
+
 	/* Warn if we apparently re-register an entity */
 	WARN_ON(entity->parent != NULL);
 	entity->parent = mdev;
@@ -440,6 +479,11 @@ int __must_check media_device_register_entity(struct media_device *mdev,
 	list_add_tail(&entity->list, &mdev->entities);
 	spin_unlock(&mdev->lock);
 
+	/* invoke entity_notify callbacks */
+	list_for_each_entry_safe(notify, next, &mdev->entity_notify, list) {
+		(notify)->notify(entity, notify->notify_data);
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(media_device_register_entity);
@@ -462,6 +506,7 @@ void media_device_unregister_entity(struct media_entity *entity)
 	list_del(&entity->list);
 	spin_unlock(&mdev->lock);
 	entity->parent = NULL;
+	/* invoke entity_notify callbacks to handle entity removal?? */
 }
 EXPORT_SYMBOL_GPL(media_device_unregister_entity);
 
diff --git a/include/media/media-device.h b/include/media/media-device.h
index a44f18f..a3854f6 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -32,6 +32,12 @@
 
 struct device;
 
+struct media_entity_notify {
+	struct list_head list;
+	void *notify_data;
+	void (*notify)(struct media_entity *entity, void *notify_data);
+};
+
 /**
  * struct media_device - Media device
  * @dev:	Parent device
@@ -70,6 +76,8 @@ struct media_device {
 
 	u32 entity_id;
 	struct list_head entities;
+	/* notify callback list invoked when a new entity is registered */
+	struct list_head entity_notify;
 
 	/* Protects the entities list */
 	spinlock_t lock;
@@ -94,6 +102,10 @@ int __must_check __media_device_register(struct media_device *mdev,
 #define media_device_register(mdev) __media_device_register(mdev, THIS_MODULE)
 void media_device_unregister(struct media_device *mdev);
 
+int __must_check media_device_register_entity_notify(struct media_device *mdev,
+					struct media_entity_notify *nptr);
+void media_device_unregister_entity_notify(struct media_device *mdev,
+					struct media_entity_notify *nptr);
 int __must_check media_device_register_entity(struct media_device *mdev,
 					      struct media_entity *entity);
 void media_device_unregister_entity(struct media_entity *entity);
@@ -112,6 +124,17 @@ static inline int media_device_register(struct media_device *mdev)
 static inline void media_device_unregister(struct media_device *mdev)
 {
 }
+static inline int media_device_register_entity_notify(
+					struct media_device *mdev,
+					struct media_entity_notify *nptr)
+{
+	return 0;
+}
+static inline void media_device_unregister_entity_notify(
+					struct media_device *mdev,
+					struct media_entity_notify *nptr)
+{
+}
 static inline int media_device_register_entity(struct media_device *mdev,
 						struct media_entity *entity)
 {
-- 
2.1.4


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

* [PATCH 3/7] media: Add ALSA Media Controller devnodes
  2015-07-15  0:33 [PATCH 0/7] Update ALSA, and au0828 drivers to use Managed Media Controller API Shuah Khan
                   ` (2 preceding siblings ...)
  2015-07-15  0:34 ` [PATCH 2/7] media: Media Controller register/unregister entity_notify API Shuah Khan
@ 2015-07-15  0:34 ` Shuah Khan
  2015-07-15  0:34 ` [PATCH 4/7] media: change dvb-frontend to honor MC tuner enable error Shuah Khan
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Shuah Khan @ 2015-07-15  0:34 UTC (permalink / raw)
  To: mchehab, hans.verkuil, laurent.pinchart, tiwai, perex, crope,
	sakari.ailus, arnd, stefanr, ruchandani.tina, chehabrafael,
	dan.carpenter, prabhakar.csengg, chris.j.arges, agoode,
	pierre-louis.bossart, gtmkramer, clemens, daniel, vladcatoi,
	misterpib, damien, pmatilai, takamichiho, normalperson,
	bugzilla.frnkcg, joe, calcprogrammer1, jussi
  Cc: Shuah Khan, linux-media, alsa-devel

Add ALSA Media Controller capture, playback, and mixer
devnode defines.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 include/uapi/linux/media.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 4e816be..4a30ea3 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -49,12 +49,17 @@ struct media_device_info {
 #define MEDIA_ENT_T_DEVNODE		(1 << MEDIA_ENT_TYPE_SHIFT)
 #define MEDIA_ENT_T_DEVNODE_V4L		(MEDIA_ENT_T_DEVNODE + 1)
 #define MEDIA_ENT_T_DEVNODE_FB		(MEDIA_ENT_T_DEVNODE + 2)
+/* Legacy ALSA symbol. Keep it to avoid userspace compilation breakages */
 #define MEDIA_ENT_T_DEVNODE_ALSA	(MEDIA_ENT_T_DEVNODE + 3)
 #define MEDIA_ENT_T_DEVNODE_DVB_FE	(MEDIA_ENT_T_DEVNODE + 4)
 #define MEDIA_ENT_T_DEVNODE_DVB_DEMUX	(MEDIA_ENT_T_DEVNODE + 5)
 #define MEDIA_ENT_T_DEVNODE_DVB_DVR	(MEDIA_ENT_T_DEVNODE + 6)
 #define MEDIA_ENT_T_DEVNODE_DVB_CA	(MEDIA_ENT_T_DEVNODE + 7)
 #define MEDIA_ENT_T_DEVNODE_DVB_NET	(MEDIA_ENT_T_DEVNODE + 8)
+/* ALSA devnodes */
+#define MEDIA_ENT_T_DEVNODE_ALSA_CAPTURE	(MEDIA_ENT_T_DEVNODE + 9)
+#define MEDIA_ENT_T_DEVNODE_ALSA_PLAYBACK	(MEDIA_ENT_T_DEVNODE + 10)
+#define MEDIA_ENT_T_DEVNODE_ALSA_MIXER		(MEDIA_ENT_T_DEVNODE + 11)
 
 /* Legacy symbol. Use it to avoid userspace compilation breakages */
 #define MEDIA_ENT_T_DEVNODE_DVB		MEDIA_ENT_T_DEVNODE_DVB_FE
-- 
2.1.4


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

* [PATCH 4/7] media: change dvb-frontend to honor MC tuner enable error
  2015-07-15  0:33 [PATCH 0/7] Update ALSA, and au0828 drivers to use Managed Media Controller API Shuah Khan
                   ` (3 preceding siblings ...)
  2015-07-15  0:34 ` [PATCH 3/7] media: Add ALSA Media Controller devnodes Shuah Khan
@ 2015-07-15  0:34 ` Shuah Khan
  2015-07-15  0:34 ` [PATCH 5/7] media: au8522 change to create MC pad for ALSA Audio Out Shuah Khan
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Shuah Khan @ 2015-07-15  0:34 UTC (permalink / raw)
  To: mchehab, hans.verkuil, laurent.pinchart, tiwai, perex, crope,
	sakari.ailus, arnd, stefanr, ruchandani.tina, chehabrafael,
	dan.carpenter, prabhakar.csengg, chris.j.arges, agoode,
	pierre-louis.bossart, gtmkramer, clemens, daniel, vladcatoi,
	misterpib, damien, pmatilai, takamichiho, normalperson,
	bugzilla.frnkcg, joe, calcprogrammer1, jussi
  Cc: Shuah Khan, linux-media, alsa-devel

Change dvb_frontend_thread() to honor MC tuner enable error and
return as opposed to ignoring the error and continuing to use it.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 drivers/media/dvb-core/dvb_frontend.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 842b9c8..5a86211 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -713,9 +713,8 @@ static int dvb_frontend_thread(void *data)
 #ifdef CONFIG_MEDIA_CONTROLLER_DVB
 	ret = dvb_enable_media_tuner(fe);
 	if (ret) {
-		/* FIXME: return an error if it fails */
-		dev_info(fe->dvb->device,
-			"proceeding with FE task\n");
+		dev_err(fe->dvb->device, "Tuner is busy. Error %d\n", ret);
+		return ret;
 	} else if (fepriv->pipe_start_entity) {
 		ret = media_entity_pipeline_start(fepriv->pipe_start_entity,
 						  &fepriv->pipe);
-- 
2.1.4


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

* [PATCH 5/7] media: au8522 change to create MC pad for ALSA Audio Out
  2015-07-15  0:33 [PATCH 0/7] Update ALSA, and au0828 drivers to use Managed Media Controller API Shuah Khan
                   ` (4 preceding siblings ...)
  2015-07-15  0:34 ` [PATCH 4/7] media: change dvb-frontend to honor MC tuner enable error Shuah Khan
@ 2015-07-15  0:34 ` Shuah Khan
  2015-07-15  0:34 ` [PATCH 6/7] media: au0828 change to use Managed Media Controller API Shuah Khan
  2015-07-15  0:34 ` [PATCH 7/7] sound/usb: Update ALSA driver " Shuah Khan
  7 siblings, 0 replies; 16+ messages in thread
From: Shuah Khan @ 2015-07-15  0:34 UTC (permalink / raw)
  To: mchehab, hans.verkuil, laurent.pinchart, tiwai, perex, crope,
	sakari.ailus, arnd, stefanr, ruchandani.tina, chehabrafael,
	dan.carpenter, prabhakar.csengg, chris.j.arges, agoode,
	pierre-louis.bossart, gtmkramer, clemens, daniel, vladcatoi,
	misterpib, damien, pmatilai, takamichiho, normalperson,
	bugzilla.frnkcg, joe, calcprogrammer1, jussi
  Cc: Shuah Khan, linux-media, alsa-devel

Add new pad for ALSA Audio Out to au8522_media_pads. Move the
au8522_media_pads enum to au8522.h from au8522_priv.h. This will
allow au0828-core to use these defines instead of hard-coding the
pad values when it creates media graph linking decode pads to other
entities.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 drivers/media/dvb-frontends/au8522.h         | 8 ++++++++
 drivers/media/dvb-frontends/au8522_decoder.c | 1 +
 drivers/media/dvb-frontends/au8522_priv.h    | 8 --------
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/dvb-frontends/au8522.h b/drivers/media/dvb-frontends/au8522.h
index dde6158..d7a997f 100644
--- a/drivers/media/dvb-frontends/au8522.h
+++ b/drivers/media/dvb-frontends/au8522.h
@@ -90,4 +90,12 @@ enum au8522_audio_input {
 	AU8522_AUDIO_SIF,
 };
 
+enum au8522_media_pads {
+	AU8522_PAD_INPUT,
+	AU8522_PAD_VID_OUT,
+	AU8522_PAD_VBI_OUT,
+	AU8522_PAD_AUDIO_OUT,
+
+	AU8522_NUM_PADS
+};
 #endif /* __AU8522_H__ */
diff --git a/drivers/media/dvb-frontends/au8522_decoder.c b/drivers/media/dvb-frontends/au8522_decoder.c
index 24990db..01d8fe7 100644
--- a/drivers/media/dvb-frontends/au8522_decoder.c
+++ b/drivers/media/dvb-frontends/au8522_decoder.c
@@ -775,6 +775,7 @@ static int au8522_probe(struct i2c_client *client,
 	state->pads[AU8522_PAD_INPUT].flags = MEDIA_PAD_FL_SINK;
 	state->pads[AU8522_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
 	state->pads[AU8522_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE;
+	state->pads[AU8522_PAD_AUDIO_OUT].flags = MEDIA_PAD_FL_SINK;
 	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
 
 	ret = media_entity_init(&sd->entity, ARRAY_SIZE(state->pads),
diff --git a/drivers/media/dvb-frontends/au8522_priv.h b/drivers/media/dvb-frontends/au8522_priv.h
index d6209d9..4c2a6ed 100644
--- a/drivers/media/dvb-frontends/au8522_priv.h
+++ b/drivers/media/dvb-frontends/au8522_priv.h
@@ -39,14 +39,6 @@
 #define AU8522_DIGITAL_MODE 1
 #define AU8522_SUSPEND_MODE 2
 
-enum au8522_media_pads {
-	AU8522_PAD_INPUT,
-	AU8522_PAD_VID_OUT,
-	AU8522_PAD_VBI_OUT,
-
-	AU8522_NUM_PADS
-};
-
 struct au8522_state {
 	struct i2c_client *c;
 	struct i2c_adapter *i2c;
-- 
2.1.4


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

* [PATCH 6/7] media: au0828 change to use Managed Media Controller API
  2015-07-15  0:33 [PATCH 0/7] Update ALSA, and au0828 drivers to use Managed Media Controller API Shuah Khan
                   ` (5 preceding siblings ...)
  2015-07-15  0:34 ` [PATCH 5/7] media: au8522 change to create MC pad for ALSA Audio Out Shuah Khan
@ 2015-07-15  0:34 ` Shuah Khan
  2015-07-20  8:42   ` Dan Carpenter
  2015-07-15  0:34 ` [PATCH 7/7] sound/usb: Update ALSA driver " Shuah Khan
  7 siblings, 1 reply; 16+ messages in thread
From: Shuah Khan @ 2015-07-15  0:34 UTC (permalink / raw)
  To: mchehab, hans.verkuil, laurent.pinchart, tiwai, perex, crope,
	sakari.ailus, arnd, stefanr, ruchandani.tina, chehabrafael,
	dan.carpenter, prabhakar.csengg, chris.j.arges, agoode,
	pierre-louis.bossart, gtmkramer, clemens, daniel, vladcatoi,
	misterpib, damien, pmatilai, takamichiho, normalperson,
	bugzilla.frnkcg, joe, calcprogrammer1, jussi
  Cc: Shuah Khan, linux-media, alsa-devel

Change au0828 to use Managed Media Controller API to coordinate
creating/deleting media device on parent usb device it shares
with the snd-usb-audio driver. With this change, au0828 uses
media_device_get_devres() to allocate a new media device devres
or return an existing one, if it finds one.

In addition, au0828 registers entity_notify hook to create media
graph for the device. It creates necessary links from video, vbi,
and ALSA entities to decoder and links tuner and decoder entities.

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

diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c
index 0378a2c..492e910 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -20,6 +20,7 @@
  */
 
 #include "au0828.h"
+#include "au8522.h"
 
 #include <linux/module.h>
 #include <linux/slab.h>
@@ -131,10 +132,12 @@ static void au0828_unregister_media_device(struct au0828_dev *dev)
 {
 
 #ifdef CONFIG_MEDIA_CONTROLLER
-	if (dev->media_dev) {
-		media_device_unregister(dev->media_dev);
-		kfree(dev->media_dev);
-		dev->media_dev = NULL;
+	if (dev->media_dev &&
+		media_devnode_is_registered(&dev->media_dev->devnode)) {
+			media_device_unregister_entity_notify(dev->media_dev,
+							&dev->entity_notify);
+			media_device_unregister(dev->media_dev);
+			dev->media_dev = NULL;
 	}
 #endif
 }
@@ -196,53 +199,23 @@ static void au0828_usb_disconnect(struct usb_interface *interface)
 	au0828_usb_release(dev);
 }
 
-static void au0828_media_device_register(struct au0828_dev *dev,
-					  struct usb_device *udev)
-{
-#ifdef CONFIG_MEDIA_CONTROLLER
-	struct media_device *mdev;
-	int ret;
-
-	mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
-	if (!mdev)
-		return;
-
-	mdev->dev = &udev->dev;
-
-	if (!dev->board.name)
-		strlcpy(mdev->model, "unknown au0828", sizeof(mdev->model));
-	else
-		strlcpy(mdev->model, dev->board.name, sizeof(mdev->model));
-	if (udev->serial)
-		strlcpy(mdev->serial, udev->serial, sizeof(mdev->serial));
-	strcpy(mdev->bus_info, udev->devpath);
-	mdev->hw_revision = le16_to_cpu(udev->descriptor.bcdDevice);
-	mdev->driver_version = LINUX_VERSION_CODE;
-
-	ret = media_device_register(mdev);
-	if (ret) {
-		pr_err(
-			"Couldn't create a media device. Error: %d\n",
-			ret);
-		kfree(mdev);
-		return;
-	}
-
-	dev->media_dev = mdev;
-#endif
-}
-
-
-static void au0828_create_media_graph(struct au0828_dev *dev)
+void au0828_create_media_graph(struct media_entity *new, void *notify_data)
 {
 #ifdef CONFIG_MEDIA_CONTROLLER
+	struct au0828_dev *dev = (struct au0828_dev *) notify_data;
 	struct media_device *mdev = dev->media_dev;
 	struct media_entity *entity;
 	struct media_entity *tuner = NULL, *decoder = NULL;
+	struct media_entity *alsa_capture = NULL;
+	int ret = 0;
 
 	if (!mdev)
 		return;
 
+	if (dev->tuner_linked && dev->vdev_linked && dev->vbi_linked &&
+		dev->alsa_capture_linked)
+		return;
+
 	media_device_for_each_entity(entity, mdev) {
 		switch (entity->type) {
 		case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
@@ -251,6 +224,9 @@ static void au0828_create_media_graph(struct au0828_dev *dev)
 		case MEDIA_ENT_T_V4L2_SUBDEV_DECODER:
 			decoder = entity;
 			break;
+		case MEDIA_ENT_T_DEVNODE_ALSA_CAPTURE:
+			alsa_capture = entity;
+			break;
 		}
 	}
 
@@ -259,15 +235,69 @@ static void au0828_create_media_graph(struct au0828_dev *dev)
 	if (!decoder)
 		return;
 
-	if (tuner)
-		media_entity_create_link(tuner, 0, decoder, 0,
+	if (tuner && !dev->tuner_linked) {
+		ret = media_entity_create_link(tuner, 0, decoder, 0,
 					 MEDIA_LNK_FL_ENABLED);
-	if (dev->vdev.entity.links)
-		media_entity_create_link(decoder, 1, &dev->vdev.entity, 0,
-				 MEDIA_LNK_FL_ENABLED);
-	if (dev->vbi_dev.entity.links)
-		media_entity_create_link(decoder, 2, &dev->vbi_dev.entity, 0,
-				 MEDIA_LNK_FL_ENABLED);
+		if (ret == 0)
+			dev->tuner_linked = 1;
+	}
+	if (dev->vdev.entity.links && !dev->vdev_linked) {
+		ret = media_entity_create_link(decoder, AU8522_PAD_VID_OUT,
+				&dev->vdev.entity, 0, MEDIA_LNK_FL_ENABLED);
+		if (ret == 0)
+			dev->vdev_linked = 1;
+	}
+	if (dev->vbi_dev.entity.links && !dev->vbi_linked) {
+		ret = media_entity_create_link(decoder, AU8522_PAD_VBI_OUT,
+				&dev->vbi_dev.entity, 0, MEDIA_LNK_FL_ENABLED);
+		if (ret == 0)
+			dev->vbi_linked = 1;
+	}
+	if (alsa_capture && !dev->alsa_capture_linked) {
+		ret = media_entity_create_link(decoder, AU8522_PAD_AUDIO_OUT,
+						alsa_capture, 0,
+						MEDIA_LNK_FL_ENABLED);
+		if (ret == 0)
+			dev->alsa_capture_linked = 1;
+	}
+#endif
+}
+
+static void au0828_media_device_register(struct au0828_dev *dev,
+					  struct usb_device *udev)
+{
+#ifdef CONFIG_MEDIA_CONTROLLER
+	struct media_device *mdev;
+	int ret;
+
+	mdev = media_device_get_devres(&udev->dev);
+	if (!mdev)
+		return;
+
+	if (!media_devnode_is_registered(&mdev->devnode)) {
+		/* register media device */
+		mdev->dev = &udev->dev;
+		if (udev->product)
+			strlcpy(mdev->model, udev->product,
+				sizeof(mdev->model));
+		if (udev->serial)
+			strlcpy(mdev->serial, udev->serial,
+				sizeof(mdev->serial));
+		strcpy(mdev->bus_info, udev->devpath);
+		mdev->hw_revision = le16_to_cpu(udev->descriptor.bcdDevice);
+		ret = media_device_register(mdev);
+		if (ret) {
+			dev_err(&udev->dev,
+				"Couldn't create a media device. Error: %d\n",
+				ret);
+			return;
+		}
+		/* register entity_notify callback */
+		dev->entity_notify.notify_data = (void *) dev;
+		dev->entity_notify.notify = au0828_create_media_graph;
+		media_device_register_entity_notify(mdev, &dev->entity_notify);
+	}
+	dev->media_dev = mdev;
 #endif
 }
 
@@ -383,8 +413,6 @@ static int au0828_usb_probe(struct usb_interface *interface,
 
 	mutex_unlock(&dev->lock);
 
-	au0828_create_media_graph(dev);
-
 	return retval;
 }
 
diff --git a/drivers/media/usb/au0828/au0828.h b/drivers/media/usb/au0828/au0828.h
index d3644b3..a59ba08 100644
--- a/drivers/media/usb/au0828/au0828.h
+++ b/drivers/media/usb/au0828/au0828.h
@@ -281,6 +281,11 @@ struct au0828_dev {
 	struct media_device *media_dev;
 	struct media_pad video_pad, vbi_pad;
 	struct media_entity *decoder;
+	struct media_entity_notify entity_notify;
+	bool tuner_linked;
+	bool vdev_linked;
+	bool vbi_linked;
+	bool alsa_capture_linked;
 #endif
 };
 
-- 
2.1.4


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

* [PATCH 7/7] sound/usb: Update ALSA driver to use Managed Media Controller API
  2015-07-15  0:33 [PATCH 0/7] Update ALSA, and au0828 drivers to use Managed Media Controller API Shuah Khan
                   ` (6 preceding siblings ...)
  2015-07-15  0:34 ` [PATCH 6/7] media: au0828 change to use Managed Media Controller API Shuah Khan
@ 2015-07-15  0:34 ` Shuah Khan
  2015-07-20  8:47   ` Dan Carpenter
  7 siblings, 1 reply; 16+ messages in thread
From: Shuah Khan @ 2015-07-15  0:34 UTC (permalink / raw)
  To: mchehab, hans.verkuil, laurent.pinchart, tiwai, perex, crope,
	sakari.ailus, arnd, stefanr, ruchandani.tina, chehabrafael,
	dan.carpenter, prabhakar.csengg, chris.j.arges, agoode,
	pierre-louis.bossart, gtmkramer, clemens, daniel, vladcatoi,
	misterpib, damien, pmatilai, takamichiho, normalperson,
	bugzilla.frnkcg, joe, calcprogrammer1, jussi
  Cc: Shuah Khan, linux-media, alsa-devel

Change ALSA driver to use Managed ~media Managed Controller
API to share tuner with DVB and V4L2 drivers that control
AU0828 media device.  Media device is created based on a
newly added field value in the struct snd_usb_audio_quirk.
Using this approach, the media controller API usage can be
added for a specific device. In this patch, Media Controller
API is enabled for AU0828 hw. snd_usb_create_quirk() will
check this new field, if set will create a media device using
media_device_get_devres() interface.

media_device_get_devres() will allocate a new media device
devres or return an existing one, if it finds one.

During probe, media usb driver could have created the media
device devres. It will then register the media device if it
isn't already registered. Media device unregister is done from
usb_audio_disconnect().

During probe, media usb driver could have created the
media device devres. It will then register the media
device if it isn't already registered. Media device
unregister is done from usb_audio_disconnect().

New structure media_ctl is added to group the new
fields to support media entity and links. This new
structure is added to struct snd_usb_substream.

A new entity_notify hook and a new ALSA capture media
entity are registered from snd_usb_pcm_open() after
setting up hardware information for the PCM device.

When a new entity is registered, Media Controller API
interface media_device_register_entity() invokes all
registered entity_notify hooks for the media device.
ALSA entity_notify hook parses all the entity list to
find a link from decoder it ALSA entity. This indicates
that the bridge driver created a link from decoder to
ALSA capture entity. At this time, ALSA will attempt to
enable the tuner to link the tuner to the decoder calling
media_entity_setup_link(). media_entity_setup_link() will
either return success activating tuner to decoder link or
error if it tuner is busy.

Media pipeline gets started to mark the tuner busy from
snd_usb_substream_capture_trigger() in response to
SNDRV_PCM_TRIGGER_START and pipeline is stopped in
response to SNDRV_PCM_TRIGGER_STOP. snd_usb_pcm_close()
stops pipeline to cover the case when SNDRV_PCM_TRIGGER_STOP
isn't issued. Pipeline start and stop are done only when
decoder_linked is set. Tuner enable step could be done
from snd_usb_substream_capture_trigger(). The reason it
is done from snd_usb_pcm_open() is that it reduces the
work that is needed from snd_usb_substream_capture_trigger().

There is an open issue with graph_mutex hold in Media Controller
media_entity_pipeline_start() and media_entity_pipeline_stop()
API. snd-us-audio calls these from snd_usb_substream_capture_trigger()
which could be called in irq path. This issue needs to be resolved
before this patch can go in.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 sound/usb/Makefile       |  15 ++-
 sound/usb/card.c         |   5 +
 sound/usb/card.h         |   1 +
 sound/usb/media.c        | 260 +++++++++++++++++++++++++++++++++++++++++++++++
 sound/usb/media.h        |  52 ++++++++++
 sound/usb/pcm.c          |  15 ++-
 sound/usb/quirks-table.h |   1 +
 sound/usb/quirks.c       |   9 +-
 sound/usb/stream.c       |   2 +
 sound/usb/usbaudio.h     |   1 +
 10 files changed, 357 insertions(+), 4 deletions(-)
 create mode 100644 sound/usb/media.c
 create mode 100644 sound/usb/media.h

diff --git a/sound/usb/Makefile b/sound/usb/Makefile
index 2d2d122..665fdd9 100644
--- a/sound/usb/Makefile
+++ b/sound/usb/Makefile
@@ -2,6 +2,18 @@
 # Makefile for ALSA
 #
 
+# Media Controller
+ifeq ($(CONFIG_MEDIA_CONTROLLER),y)
+  ifeq ($(CONFIG_MEDIA_SUPPORT),y)
+        KBUILD_CFLAGS += -DUSE_MEDIA_CONTROLLER
+  endif
+  ifeq ($(CONFIG_MEDIA_SUPPORT_MODULE),y)
+    ifeq ($(MODULE),y)
+          KBUILD_CFLAGS += -DUSE_MEDIA_CONTROLLER
+    endif
+  endif
+endif
+
 snd-usb-audio-objs := 	card.o \
 			clock.o \
 			endpoint.o \
@@ -13,7 +25,8 @@ snd-usb-audio-objs := 	card.o \
 			pcm.o \
 			proc.o \
 			quirks.o \
-			stream.o
+			stream.o \
+			media.o
 
 snd-usbmidi-lib-objs := midi.o
 
diff --git a/sound/usb/card.c b/sound/usb/card.c
index 1fab977..469d2bf 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");
@@ -619,6 +620,10 @@ static void usb_audio_disconnect(struct usb_interface *intf)
 		list_for_each_entry(mixer, &chip->mixer_list, list) {
 			snd_usb_mixer_disconnect(mixer);
 		}
+		/* Nice to check quirk && quirk->media_device
+		 * need some special handlings. Doesn't look like
+		 * we have access to quirk here */
+		media_device_delete(intf);
 	}
 
 	chip->num_interfaces--;
diff --git a/sound/usb/card.h b/sound/usb/card.h
index ef580b4..235a85f 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -155,6 +155,7 @@ struct snd_usb_substream {
 	} dsd_dop;
 
 	bool trigger_tstamp_pending_update; /* trigger timestamp being updated from initial estimate */
+	void *media_ctl;
 };
 
 struct snd_usb_stream {
diff --git a/sound/usb/media.c b/sound/usb/media.c
new file mode 100644
index 0000000..cabbb60
--- /dev/null
+++ b/sound/usb/media.c
@@ -0,0 +1,260 @@
+/*
+ * media.c - Media Controller specific ALSA driver code
+ *
+ * Copyright (c) 2015 Shuah Khan <shuahkh@osg.samsung.com>
+ * Copyright (c) 2015 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/slab.h>
+#include <linux/string.h>
+#include <linux/ctype.h>
+#include <linux/usb.h>
+#include <linux/moduleparam.h>
+#include <linux/mutex.h>
+#include <linux/usb/audio.h>
+#include <linux/usb/audio-v2.h>
+#include <linux/module.h>
+
+#include <sound/control.h>
+#include <sound/core.h>
+#include <sound/info.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/initval.h>
+
+#include "usbaudio.h"
+#include "card.h"
+#include "midi.h"
+#include "mixer.h"
+#include "proc.h"
+#include "quirks.h"
+#include "endpoint.h"
+#include "helper.h"
+#include "debug.h"
+#include "pcm.h"
+#include "format.h"
+#include "power.h"
+#include "stream.h"
+#include "media.h"
+
+#ifdef USE_MEDIA_CONTROLLER
+int media_device_init(struct usb_interface *iface)
+{
+	struct media_device *mdev;
+	struct usb_device *usbdev = interface_to_usbdev(iface);
+	int ret;
+
+	mdev = media_device_get_devres(&usbdev->dev);
+	if (!mdev)
+		return -ENOMEM;
+	if (!media_devnode_is_registered(&mdev->devnode)) {
+		/* register media device */
+		mdev->dev = &usbdev->dev;
+		if (usbdev->product)
+			strlcpy(mdev->model, usbdev->product,
+				sizeof(mdev->model));
+		if (usbdev->serial)
+			strlcpy(mdev->serial, usbdev->serial,
+				sizeof(mdev->serial));
+		strcpy(mdev->bus_info, usbdev->devpath);
+		mdev->hw_revision = le16_to_cpu(usbdev->descriptor.bcdDevice);
+		ret = media_device_register(mdev);
+		if (ret) {
+			dev_err(&usbdev->dev,
+				"Couldn't create a media device. Error: %d\n",
+				ret);
+			return ret;
+		}
+	}
+	return 0;
+}
+
+void media_device_delete(struct usb_interface *iface)
+{
+	struct media_device *mdev;
+	struct usb_device *usbdev = interface_to_usbdev(iface);
+
+	mdev = media_device_find_devres(&usbdev->dev);
+	if (mdev && media_devnode_is_registered(&mdev->devnode))
+		media_device_unregister(mdev);
+}
+
+static void media_entity_alsa_create_link(struct media_entity *entity,
+					void *notify_data)
+{
+	struct snd_usb_substream *subs;
+	struct media_ctl *mctl;
+	struct media_entity *eptr;
+
+	/* private and mctl should be valid for ALSA DEVNODEs */
+	subs = (struct snd_usb_substream *) notify_data;
+	mctl = (struct media_ctl *) subs->media_ctl;
+
+	if (mctl->decoder_linked)
+		return;
+
+	media_device_for_each_entity(eptr, mctl->media_dev) {
+		if (eptr->type == MEDIA_ENT_T_V4L2_SUBDEV_DECODER) {
+			int i;
+
+			for (i = 0; i < eptr->num_links; i++) {
+				struct media_link *link = &eptr->links[i];
+				struct media_entity *sink = link->sink->entity;
+
+				if (!(link->flags & MEDIA_LNK_FL_ENABLED))
+					continue;
+				if (sink == &mctl->media_entity) {
+					mctl->decoder_linked = 1;
+					mctl->decoder = eptr;
+				}
+			}
+		}
+	}
+}
+
+static void media_enable_tuner(struct media_ctl *mctl)
+{
+	struct media_entity  *source;
+	struct media_link *link, *found_link = NULL;
+	int i, ret, active_links = 0;
+
+	if (!mctl || !mctl->decoder)
+		return;
+
+	for (i = 0; i < mctl->decoder->num_links; i++) {
+		link = &mctl->decoder->links[i];
+		if (link->sink->entity == mctl->decoder) {
+			found_link = link;
+			if (link->flags & MEDIA_LNK_FL_ENABLED)
+				active_links++;
+			break;
+		}
+	}
+
+	if (active_links == 1 || !found_link)
+		return;
+
+	source = found_link->source->entity;
+	for (i = 0; i < source->num_links; i++) {
+		struct media_entity *sink;
+		int flags = 0;
+
+		link = &source->links[i];
+		sink = link->sink->entity;
+
+		if (sink == mctl->decoder)
+			flags = MEDIA_LNK_FL_ENABLED;
+
+		ret = media_entity_setup_link(link, flags);
+		if (ret) {
+			dev_err(mctl->media_dev->dev,
+				"Couldn't change tuner link",
+				"%s->%s to %s. Error %d\n",
+				source->name, sink->name,
+				flags ? "enabled" : "disabled",
+				ret);
+			return;
+		}
+		mctl->tuner_enabled = 1;
+	}
+	dev_info(mctl->media_dev->dev, "Tuner is enabled\n");
+}
+
+void media_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;
+
+	mdev = media_device_find_devres(&subs->dev->dev);
+	if (!mdev)
+		return;
+
+	if (subs->media_ctl) {
+		/* enable tuner */
+		media_enable_tuner(subs->media_ctl);
+		return;
+	}
+
+	/* allocate media_ctl */
+	mctl = kzalloc(sizeof(struct media_ctl), GFP_KERNEL);
+	if (!mctl)
+		return;
+
+	subs->media_ctl = (void *) mctl;
+	mctl->media_dev = mdev;
+	mctl->entity_notify.notify_data = (void *) subs;
+	mctl->entity_notify.notify = media_entity_alsa_create_link;
+	media_device_register_entity_notify(mctl->media_dev,
+							&mctl->entity_notify);
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
+		mctl->media_entity.type = MEDIA_ENT_T_DEVNODE_ALSA_PLAYBACK;
+	else
+		mctl->media_entity.type = MEDIA_ENT_T_DEVNODE_ALSA_CAPTURE;
+	mctl->media_entity.name = pcm->name;
+	mctl->media_entity.info.dev.major = MAJOR(pcm_dev->devt);
+	mctl->media_entity.info.dev.minor = MINOR(pcm_dev->devt);
+	mctl->media_pad.flags = MEDIA_PAD_FL_SINK;
+	media_entity_init(&mctl->media_entity, 1, &mctl->media_pad, 0);
+	media_device_register_entity(mctl->media_dev, &mctl->media_entity);
+	/* enable tuner */
+	media_enable_tuner(mctl);
+}
+
+void media_stream_delete(struct snd_usb_substream *subs)
+{
+	struct media_ctl *mctl = (struct media_ctl *) subs->media_ctl;
+
+	if (mctl && mctl->media_dev) {
+		struct media_device *mdev;
+
+		mdev = media_device_find_devres(&subs->dev->dev);
+		if (mdev) {
+			media_entity_remove_links(&mctl->media_entity);
+			media_device_unregister_entity(&mctl->media_entity);
+			media_entity_cleanup(&mctl->media_entity);
+			media_device_unregister_entity_notify(mctl->media_dev,
+							&mctl->entity_notify);
+		}
+		mctl->media_dev = NULL;
+		mctl->decoder_linked = 0;
+		mctl->decoder = NULL;
+		mctl->tuner_enabled = 0;
+		kfree(mctl);
+		subs->media_ctl = NULL;
+	}
+}
+
+int media_start_pipeline(struct snd_usb_substream *subs)
+{
+	struct media_ctl *mctl = (struct media_ctl *) subs->media_ctl;
+	int ret = 0;
+
+	if (mctl && mctl->decoder_linked)
+		ret = media_entity_pipeline_start(&mctl->media_entity,
+							&mctl->media_pipe);
+	return ret;
+}
+
+void media_stop_pipeline(struct snd_usb_substream *subs)
+{
+	struct media_ctl *mctl = (struct media_ctl *) subs->media_ctl;
+
+	if (mctl && mctl->decoder_linked)
+		media_entity_pipeline_stop(&mctl->media_entity);
+}
+#endif
diff --git a/sound/usb/media.h b/sound/usb/media.h
new file mode 100644
index 0000000..07799b4
--- /dev/null
+++ b/sound/usb/media.h
@@ -0,0 +1,52 @@
+/*
+ * media.h - Media Controller specific ALSA driver code
+ *
+ * Copyright (c) 2015 Shuah Khan <shuahkh@osg.samsung.com>
+ * Copyright (c) 2015 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 USE_MEDIA_CONTROLLER
+#include <media/media-device.h>
+#include <media/media-entity.h>
+
+struct media_ctl {
+	struct media_device *media_dev;
+	struct media_entity_notify entity_notify;
+	struct media_entity media_entity;
+	struct media_pad media_pad;
+	struct media_pipeline media_pipe;
+	struct media_entity *decoder;
+	bool   decoder_linked;
+	bool   tuner_enabled;
+};
+
+int media_device_init(struct usb_interface *iface);
+void media_device_delete(struct usb_interface *iface);
+void media_stream_init(struct snd_usb_substream *subs, struct snd_pcm *pcm,
+			int stream);
+void media_stream_delete(struct snd_usb_substream *subs);
+int media_start_pipeline(struct snd_usb_substream *subs);
+void media_stop_pipeline(struct snd_usb_substream *subs);
+#else
+static inline int media_device_init(struct usb_interface *iface) { return 0; }
+static inline void media_device_delete(struct usb_interface *iface) { }
+static inline void media_stream_init(struct snd_usb_substream *subs,
+					struct snd_pcm *pcm, int stream) { }
+static inline void media_stream_delete(struct snd_usb_substream *subs) { }
+static inline int media_start_pipeline(struct snd_usb_substream *subs)
+					{ return 0; }
+static inline void media_stop_pipeline(struct snd_usb_substream *subs) { }
+#endif
+#endif /* __MEDIA_H */
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index b4ef410..0e73a3a 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
@@ -1195,6 +1196,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;
@@ -1208,7 +1210,10 @@ 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)
+		media_stream_init(subs, as->pcm, direction);
+	return ret;
 }
 
 static int snd_usb_pcm_close(struct snd_pcm_substream *substream, int direction)
@@ -1587,9 +1592,14 @@ static int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
+		err = media_start_pipeline(subs);
+		if (err)
+			return err;
 		err = start_endpoints(subs, false);
-		if (err < 0)
+		if (err < 0) {
+			media_stop_pipeline(subs);
 			return err;
+		}
 
 		subs->data_endpoint->retire_data_urb = retire_capture_urb;
 		subs->running = 1;
@@ -1597,6 +1607,7 @@ static int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream
 	case SNDRV_PCM_TRIGGER_STOP:
 		stop_endpoints(subs, false);
 		subs->running = 0;
+		media_stop_pipeline(subs);
 		return 0;
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 		subs->data_endpoint->retire_data_urb = NULL;
diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h
index 2f6d3e9..51c544f 100644
--- a/sound/usb/quirks-table.h
+++ b/sound/usb/quirks-table.h
@@ -2798,6 +2798,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/quirks.c b/sound/usb/quirks.c
index 7c5a701..e9a2009 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -36,6 +36,7 @@
 #include "pcm.h"
 #include "clock.h"
 #include "stream.h"
+#include "media.h"
 
 /*
  * handle the quirks for the contained interfaces
@@ -541,13 +542,19 @@ int snd_usb_create_quirk(struct snd_usb_audio *chip,
 		[QUIRK_AUDIO_ALIGN_TRANSFER] = create_align_transfer_quirk,
 		[QUIRK_AUDIO_STANDARD_MIXER] = create_standard_mixer_quirk,
 	};
+	int ret;
 
+	if (quirk->media_device) {
+		/* don't want to fail when media_device_init() doesn't work */
+		media_device_init(iface);
+	}
 	if (quirk->type < QUIRK_TYPE_COUNT) {
-		return quirk_funcs[quirk->type](chip, iface, driver, quirk);
+		ret = quirk_funcs[quirk->type](chip, iface, driver, quirk);
 	} else {
 		usb_audio_err(chip, "invalid quirk type %d\n", quirk->type);
 		return -ENXIO;
 	}
+	return ret;
 }
 
 /*
diff --git a/sound/usb/stream.c b/sound/usb/stream.c
index 310a382..6d01c47 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_stream_delete(subs);
 }
 
 
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
index 91d0380..c2dbf1d 100644
--- a/sound/usb/usbaudio.h
+++ b/sound/usb/usbaudio.h
@@ -108,6 +108,7 @@ struct snd_usb_audio_quirk {
 	const char *product_name;
 	int16_t ifnum;
 	uint16_t type;
+	bool media_device;
 	const void *data;
 };
 
-- 
2.1.4


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

* Re: [PATCH 6/7] media: au0828 change to use Managed Media Controller API
  2015-07-15  0:34 ` [PATCH 6/7] media: au0828 change to use Managed Media Controller API Shuah Khan
@ 2015-07-20  8:42   ` Dan Carpenter
  2015-07-20 15:55     ` Shuah Khan
  0 siblings, 1 reply; 16+ messages in thread
From: Dan Carpenter @ 2015-07-20  8:42 UTC (permalink / raw)
  To: Shuah Khan
  Cc: mchehab, hans.verkuil, laurent.pinchart, tiwai, perex, crope,
	sakari.ailus, arnd, stefanr, ruchandani.tina, chehabrafael,
	prabhakar.csengg, chris.j.arges, agoode, pierre-louis.bossart,
	gtmkramer, clemens, daniel, vladcatoi, misterpib, damien,
	pmatilai, takamichiho, normalperson, bugzilla.frnkcg, joe,
	calcprogrammer1, jussi, linux-media, alsa-devel

Sorry for this nit-pick and some time after you sent this patch.  It's
not a redo the patch" complaint, it's something that could be fixed
later.

On Tue, Jul 14, 2015 at 06:34:05PM -0600, Shuah Khan wrote:
> @@ -131,10 +132,12 @@ static void au0828_unregister_media_device(struct au0828_dev *dev)
>  {
>  
>  #ifdef CONFIG_MEDIA_CONTROLLER
> -	if (dev->media_dev) {
> -		media_device_unregister(dev->media_dev);
> -		kfree(dev->media_dev);
> -		dev->media_dev = NULL;
> +	if (dev->media_dev &&
> +		media_devnode_is_registered(&dev->media_dev->devnode)) {
> +			media_device_unregister_entity_notify(dev->media_dev,
> +							&dev->entity_notify);
> +			media_device_unregister(dev->media_dev);
> +			dev->media_dev = NULL;


The indenting is slightly off here.  It should be:

	if (dev->media_dev &&
	    media_devnode_is_registered(&dev->media_dev->devnode)) {
		media_device_unregister_entity_notify(dev->media_dev,
						      &dev->entity_notify);
		media_device_unregister(dev->media_dev);
		dev->media_dev = NULL;
	}

Aligning if statements using spaces like that is nicer and checkpatch.pl
won't complain.

regards,
dan carpenter


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

* Re: [PATCH 7/7] sound/usb: Update ALSA driver to use Managed Media Controller API
  2015-07-15  0:34 ` [PATCH 7/7] sound/usb: Update ALSA driver " Shuah Khan
@ 2015-07-20  8:47   ` Dan Carpenter
  2015-07-20  9:00     ` Takashi Iwai
  0 siblings, 1 reply; 16+ messages in thread
From: Dan Carpenter @ 2015-07-20  8:47 UTC (permalink / raw)
  To: Shuah Khan
  Cc: mchehab, hans.verkuil, laurent.pinchart, tiwai, perex, crope,
	sakari.ailus, arnd, stefanr, ruchandani.tina, chehabrafael,
	prabhakar.csengg, chris.j.arges, agoode, pierre-louis.bossart,
	gtmkramer, clemens, daniel, vladcatoi, misterpib, damien,
	pmatilai, takamichiho, normalperson, bugzilla.frnkcg, joe,
	calcprogrammer1, jussi, linux-media, alsa-devel

On Tue, Jul 14, 2015 at 06:34:06PM -0600, Shuah Khan wrote:
> +		ret = media_entity_setup_link(link, flags);
> +		if (ret) {
> +			dev_err(mctl->media_dev->dev,
> +				"Couldn't change tuner link",
> +				"%s->%s to %s. Error %d\n",

Add a space after "link".

				"Couldn't change tuner link ",
				"%s->%s to %s. Error %d\n",

regards,
dan carpenter

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

* Re: [PATCH 7/7] sound/usb: Update ALSA driver to use Managed Media Controller API
  2015-07-20  8:47   ` Dan Carpenter
@ 2015-07-20  9:00     ` Takashi Iwai
  2015-07-20 14:22       ` Shuah Khan
  0 siblings, 1 reply; 16+ messages in thread
From: Takashi Iwai @ 2015-07-20  9:00 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Shuah Khan, mchehab, hans.verkuil, laurent.pinchart, perex, crope,
	sakari.ailus, arnd, stefanr, ruchandani.tina, chehabrafael,
	prabhakar.csengg, chris.j.arges, agoode, pierre-louis.bossart,
	gtmkramer, clemens, daniel, vladcatoi, misterpib, damien,
	pmatilai, takamichiho, normalperson, bugzilla.frnkcg, joe,
	calcprogrammer1, jussi, linux-media, alsa-devel

On Mon, 20 Jul 2015 10:47:46 +0200,
Dan Carpenter wrote:
> 
> On Tue, Jul 14, 2015 at 06:34:06PM -0600, Shuah Khan wrote:
> > +		ret = media_entity_setup_link(link, flags);
> > +		if (ret) {
> > +			dev_err(mctl->media_dev->dev,
> > +				"Couldn't change tuner link",
> > +				"%s->%s to %s. Error %d\n",
> 
> Add a space after "link".
> 
> 				"Couldn't change tuner link ",
> 				"%s->%s to %s. Error %d\n",

Such a message string would be better to be in a single line even if
it's over 80 chars.  Otherwise it becomes hard to grep.


thanks,

Takashi

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

* Re: [PATCH 7/7] sound/usb: Update ALSA driver to use Managed Media Controller API
  2015-07-20  9:00     ` Takashi Iwai
@ 2015-07-20 14:22       ` Shuah Khan
  0 siblings, 0 replies; 16+ messages in thread
From: Shuah Khan @ 2015-07-20 14:22 UTC (permalink / raw)
  To: Takashi Iwai, Dan Carpenter
  Cc: mchehab, hans.verkuil, laurent.pinchart, perex, crope,
	sakari.ailus, arnd, stefanr, ruchandani.tina, chehabrafael,
	prabhakar.csengg, chris.j.arges, agoode, pierre-louis.bossart,
	gtmkramer, clemens, daniel, vladcatoi, misterpib, damien,
	pmatilai, takamichiho, normalperson, bugzilla.frnkcg, joe,
	calcprogrammer1, jussi, linux-media, alsa-devel, Shuah Khan

On 07/20/2015 03:00 AM, Takashi Iwai wrote:
> On Mon, 20 Jul 2015 10:47:46 +0200,
> Dan Carpenter wrote:
>>
>> On Tue, Jul 14, 2015 at 06:34:06PM -0600, Shuah Khan wrote:
>>> +		ret = media_entity_setup_link(link, flags);
>>> +		if (ret) {
>>> +			dev_err(mctl->media_dev->dev,
>>> +				"Couldn't change tuner link",
>>> +				"%s->%s to %s. Error %d\n",
>>
>> Add a space after "link".
>>
>> 				"Couldn't change tuner link ",
>> 				"%s->%s to %s. Error %d\n",
> 
> Such a message string would be better to be in a single line even if
> it's over 80 chars.  Otherwise it becomes hard to grep.
> 

Right. The above generates a warning and I found it after I sent
the patch. I will have to re-do this patch to fix this. I prefer
not splitting the message, but checkpatch compains if I don't.

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

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

* Re: [PATCH 6/7] media: au0828 change to use Managed Media Controller API
  2015-07-20  8:42   ` Dan Carpenter
@ 2015-07-20 15:55     ` Shuah Khan
  2015-07-20 19:01       ` Dan Carpenter
  0 siblings, 1 reply; 16+ messages in thread
From: Shuah Khan @ 2015-07-20 15:55 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: mchehab, hans.verkuil, laurent.pinchart, tiwai, perex, crope,
	sakari.ailus, arnd, stefanr, ruchandani.tina, chehabrafael,
	prabhakar.csengg, chris.j.arges, agoode, pierre-louis.bossart,
	gtmkramer, clemens, daniel, vladcatoi, misterpib, damien,
	pmatilai, takamichiho, normalperson, bugzilla.frnkcg, joe,
	calcprogrammer1, jussi, linux-media, alsa-devel, Shuah Khan

On 07/20/2015 02:42 AM, Dan Carpenter wrote:
> Sorry for this nit-pick and some time after you sent this patch.  It's
> not a redo the patch" complaint, it's something that could be fixed
> later.
> 
> On Tue, Jul 14, 2015 at 06:34:05PM -0600, Shuah Khan wrote:
>> @@ -131,10 +132,12 @@ static void au0828_unregister_media_device(struct au0828_dev *dev)
>>  {
>>  
>>  #ifdef CONFIG_MEDIA_CONTROLLER
>> -	if (dev->media_dev) {
>> -		media_device_unregister(dev->media_dev);
>> -		kfree(dev->media_dev);
>> -		dev->media_dev = NULL;
>> +	if (dev->media_dev &&
>> +		media_devnode_is_registered(&dev->media_dev->devnode)) {
>> +			media_device_unregister_entity_notify(dev->media_dev,
>> +							&dev->entity_notify);
>> +			media_device_unregister(dev->media_dev);
>> +			dev->media_dev = NULL;
> 
> 
> The indenting is slightly off here.  It should be:
> 
> 	if (dev->media_dev &&
> 	    media_devnode_is_registered(&dev->media_dev->devnode)) {
> 		media_device_unregister_entity_notify(dev->media_dev,
> 						      &dev->entity_notify);
> 		media_device_unregister(dev->media_dev);
> 		dev->media_dev = NULL;
> 	}
> 
> Aligning if statements using spaces like that is nicer and checkpatch.pl
> won't complain.
> 

Yeah. I try to do that whenever. In this case, if I align, the line
becomes longer than 80 chars adding more things to worry about. In
such cases, I tend to not worry about aligning.

thanks,
-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

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

* Re: [PATCH 6/7] media: au0828 change to use Managed Media Controller API
  2015-07-20 15:55     ` Shuah Khan
@ 2015-07-20 19:01       ` Dan Carpenter
  2015-07-20 19:10         ` Shuah Khan
  0 siblings, 1 reply; 16+ messages in thread
From: Dan Carpenter @ 2015-07-20 19:01 UTC (permalink / raw)
  To: Shuah Khan
  Cc: mchehab, hans.verkuil, laurent.pinchart, tiwai, perex, crope,
	sakari.ailus, arnd, stefanr, ruchandani.tina, chehabrafael,
	prabhakar.csengg, chris.j.arges, agoode, pierre-louis.bossart,
	gtmkramer, clemens, daniel, vladcatoi, misterpib, damien,
	pmatilai, takamichiho, normalperson, bugzilla.frnkcg, joe,
	calcprogrammer1, jussi, linux-media, alsa-devel

On Mon, Jul 20, 2015 at 09:55:50AM -0600, Shuah Khan wrote:
> >>  #ifdef CONFIG_MEDIA_CONTROLLER
> >> -	if (dev->media_dev) {
> >> -		media_device_unregister(dev->media_dev);
> >> -		kfree(dev->media_dev);
> >> -		dev->media_dev = NULL;
> >> +	if (dev->media_dev &&
> >> +		media_devnode_is_registered(&dev->media_dev->devnode)) {
> >> +			media_device_unregister_entity_notify(dev->media_dev,
> >> +							&dev->entity_notify);
> >> +			media_device_unregister(dev->media_dev);
> >> +			dev->media_dev = NULL;
> > 
> > 
> > The indenting is slightly off here.  It should be:
> > 
> > 	if (dev->media_dev &&
> > 	    media_devnode_is_registered(&dev->media_dev->devnode)) {
> > 		media_device_unregister_entity_notify(dev->media_dev,
> > 						      &dev->entity_notify);
> > 		media_device_unregister(dev->media_dev);
> > 		dev->media_dev = NULL;
> > 	}
> > 
> > Aligning if statements using spaces like that is nicer and checkpatch.pl
> > won't complain.
> > 
> 
> Yeah. I try to do that whenever. In this case, if I align, the line
> becomes longer than 80 chars adding more things to worry about. In
> such cases, I tend to not worry about aligning.

The main thing is that the body of the if statement is intended 16
spaces instead of 8.  Otherwise I wouldn't have commented.

regards,
dan carpenter

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

* Re: [PATCH 6/7] media: au0828 change to use Managed Media Controller API
  2015-07-20 19:01       ` Dan Carpenter
@ 2015-07-20 19:10         ` Shuah Khan
  0 siblings, 0 replies; 16+ messages in thread
From: Shuah Khan @ 2015-07-20 19:10 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: mchehab, hans.verkuil, laurent.pinchart, tiwai, perex, crope,
	sakari.ailus, arnd, stefanr, ruchandani.tina, chehabrafael,
	prabhakar.csengg, chris.j.arges, agoode, pierre-louis.bossart,
	gtmkramer, clemens, daniel, vladcatoi, misterpib, damien,
	pmatilai, takamichiho, normalperson, bugzilla.frnkcg, joe,
	calcprogrammer1, jussi, linux-media, alsa-devel, Shuah Khan

On 07/20/2015 01:01 PM, Dan Carpenter wrote:
> On Mon, Jul 20, 2015 at 09:55:50AM -0600, Shuah Khan wrote:
>>>>  #ifdef CONFIG_MEDIA_CONTROLLER
>>>> -	if (dev->media_dev) {
>>>> -		media_device_unregister(dev->media_dev);
>>>> -		kfree(dev->media_dev);
>>>> -		dev->media_dev = NULL;
>>>> +	if (dev->media_dev &&
>>>> +		media_devnode_is_registered(&dev->media_dev->devnode)) {
>>>> +			media_device_unregister_entity_notify(dev->media_dev,
>>>> +							&dev->entity_notify);
>>>> +			media_device_unregister(dev->media_dev);
>>>> +			dev->media_dev = NULL;
>>>
>>>
>>> The indenting is slightly off here.  It should be:
>>>
>>> 	if (dev->media_dev &&
>>> 	    media_devnode_is_registered(&dev->media_dev->devnode)) {
>>> 		media_device_unregister_entity_notify(dev->media_dev,
>>> 						      &dev->entity_notify);
>>> 		media_device_unregister(dev->media_dev);
>>> 		dev->media_dev = NULL;
>>> 	}
>>>
>>> Aligning if statements using spaces like that is nicer and checkpatch.pl
>>> won't complain.
>>>
>>
>> Yeah. I try to do that whenever. In this case, if I align, the line
>> becomes longer than 80 chars adding more things to worry about. In
>> such cases, I tend to not worry about aligning.
> 
> The main thing is that the body of the if statement is intended 16
> spaces instead of 8.  Otherwise I wouldn't have commented.
> 

Yes. I see what you are saying. I will fix it in v2 series.

thanks,
-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

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

end of thread, other threads:[~2015-07-20 19:10 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-15  0:33 [PATCH 0/7] Update ALSA, and au0828 drivers to use Managed Media Controller API Shuah Khan
2015-07-15  0:33 ` [PATCH] media-ctl: Update to add support for ALSA devices Shuah Khan
2015-07-15  0:34 ` [PATCH 1/7] Revert "[media] media: media controller entity framework enhancements for ALSA" Shuah Khan
2015-07-15  0:34 ` [PATCH 2/7] media: Media Controller register/unregister entity_notify API Shuah Khan
2015-07-15  0:34 ` [PATCH 3/7] media: Add ALSA Media Controller devnodes Shuah Khan
2015-07-15  0:34 ` [PATCH 4/7] media: change dvb-frontend to honor MC tuner enable error Shuah Khan
2015-07-15  0:34 ` [PATCH 5/7] media: au8522 change to create MC pad for ALSA Audio Out Shuah Khan
2015-07-15  0:34 ` [PATCH 6/7] media: au0828 change to use Managed Media Controller API Shuah Khan
2015-07-20  8:42   ` Dan Carpenter
2015-07-20 15:55     ` Shuah Khan
2015-07-20 19:01       ` Dan Carpenter
2015-07-20 19:10         ` Shuah Khan
2015-07-15  0:34 ` [PATCH 7/7] sound/usb: Update ALSA driver " Shuah Khan
2015-07-20  8:47   ` Dan Carpenter
2015-07-20  9:00     ` Takashi Iwai
2015-07-20 14:22       ` Shuah Khan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.