On 01/07/2016 08:44 AM, Takashi Iwai wrote:
On Wed, 06 Jan 2016 22:05:35 +0100, Shuah Khan wrote:
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
Can't we define this rather via Kconfig? Doing this in Makefile is way too tricky, and it's unclear to users whether MC is actually enabled or not.
Yeah doing this in Makefile is a bit tricky and can lead to confusion.
I can't think of any specific reason why I added this check to the Makefile instead of Kconfig. Looks like I added this in my second version of the patch series several months ago and didn't revisit. I will add this to Kconfig.
diff --git a/sound/usb/media.c b/sound/usb/media.c new file mode 100644 index 0000000..747a66a --- /dev/null +++ b/sound/usb/media.c @@ -0,0 +1,214 @@ +/*
- 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"
I believe we can get rid of many include files just for MC support...
+#ifdef USE_MEDIA_CONTROLLER
This ifdef can be removed once if we build this object file conditionally in Makefile.
Right.
@@ -1232,7 +1244,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)
ret = media_stream_init(subs, as->pcm, direction);Need to call snd_usb_autosuspend() in the error path.
I will add it.
--- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -544,13 +545,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_create() fails */media_device_create(chip, iface);}
So far, so good...
if (quirk->type < QUIRK_TYPE_COUNT) {
return quirk_funcs[quirk->type](chip, iface, driver, quirk);
ret = quirk_funcs[quirk->type](chip, iface, driver, quirk);- return ret;
Any reason to change this?
Thanks for catching this. I think I might have added some debug code to print ret value and missed it when I cleaned up the debug code. I will fix it.
thanks, -- Shuah