From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751334AbcAPJHG (ORCPT ); Sat, 16 Jan 2016 04:07:06 -0500 Received: from mx2.suse.de ([195.135.220.15]:37113 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750993AbcAPJGz (ORCPT ); Sat, 16 Jan 2016 04:06:55 -0500 Date: Sat, 16 Jan 2016 10:06:50 +0100 Message-ID: From: Takashi Iwai To: Shuah Khan Cc: hans.verkuil@cisco.com, laurent.pinchart@ideasonboard.com, clemens@ladisch.de, sakari.ailus@linux.intel.com, javier@osg.samsung.com, mchehab@osg.samsung.com, alsa-devel@alsa-project.org, arnd@arndb.de, ricard.wanderlof@axis.com, labbott@fedoraproject.org, chehabrafael@gmail.com, misterpib@gmail.com, prabhakar.csengg@gmail.com, ricardo.ribalda@gmail.com, ruchandani.tina@gmail.com, takamichiho@gmail.com, tvboxspy@gmail.com, dominic.sacre@gmx.de, crope@iki.fi, julian@jusst.de, pierre-louis.bossart@linux.intel.com, corbet@lwn.net, joe@oampo.co.uk, johan@oljud.se, dan.carpenter@oracle.com, pawel@osciak.com, p.zabel@pengutronix.de, perex@perex.cz, stefanr@s5r6.in-berlin.de, inki.dae@samsung.com, jh1009.sung@samsung.com, k.kozlowski@samsung.com, kyungmin.park@samsung.com, m.szyprowski@samsung.com, sw0312.kim@samsung.com, elfring@users.sourceforge.net, linux-api@vger.kernel.org, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org, linuxbugs@vittgam.net, gtmkramer@xs4all.nl, normalperson@yhbt.net, daniel@zonque.org Subject: Re: [PATCH v2 26/31] sound/usb: Update ALSA driver to use Managed Media Controller API In-Reply-To: <56999103.6000005@osg.samsung.com> References: <56999103.6000005@osg.samsung.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.5 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, 16 Jan 2016 01:38:27 +0100, Shuah Khan wrote: > > On 01/15/2016 06:38 AM, Takashi Iwai wrote: > > On Thu, 14 Jan 2016 20:52:28 +0100, > > Shuah Khan wrote: > >> > >> 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 initialze (if necessary) and > >> register the media device if it isn't already initialized > >> and 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. > >> > >> ALSA will attempt to enable the tuner to link the tuner > >> to the decoder calling enable_source handler if one is > >> provided by the bridge driver prior to starting Media > >> pipeline from snd_usb_hw_params(). If enable_source returns > >> with tuner busy condition, then snd_usb_hw_params() will fail > >> with -EBUSY. Media pipeline is stopped from snd_usb_hw_free(). > >> > >> Signed-off-by: Shuah Khan > >> --- > >> Changes since v1: Addressed Takasi Iwai's comments on v1 > >> - Move config defines to Kconfig and add logic > >> to Makefile to conditionally compile media.c > >> - Removed extra includes from media.c > >> - Added snd_usb_autosuspend() in error leg > >> - Removed debug related code that was missed in v1 > >> > >> sound/usb/Kconfig | 7 ++ > >> sound/usb/Makefile | 4 + > >> sound/usb/card.c | 7 ++ > >> sound/usb/card.h | 1 + > >> sound/usb/media.c | 190 +++++++++++++++++++++++++++++++++++++++++++++++ > >> sound/usb/media.h | 56 ++++++++++++++ > >> sound/usb/pcm.c | 28 +++++-- > >> sound/usb/quirks-table.h | 1 + > >> sound/usb/quirks.c | 5 ++ > >> sound/usb/stream.c | 2 + > >> sound/usb/usbaudio.h | 1 + > >> 11 files changed, 297 insertions(+), 5 deletions(-) > >> create mode 100644 sound/usb/media.c > >> create mode 100644 sound/usb/media.h > >> > >> diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig > >> index a452ad7..8d5cdab 100644 > >> --- a/sound/usb/Kconfig > >> +++ b/sound/usb/Kconfig > >> @@ -15,6 +15,7 @@ config SND_USB_AUDIO > >> select SND_RAWMIDI > >> select SND_PCM > >> select BITREVERSE > >> + select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER && (MEDIA_SUPPORT || MEDIA_SUPPORT_MODULE) > > > > This looks wrong as a Kconfig syntax. "... if MEDIA_CONTROLLER" > > should work, I suppose? > > The conditional select syntax is used in several Kconfig > files. You are right about (MEDIA_SUPPORT || MEDIA_SUPPORT_MODULE) > Adding checks for that is unnecessary. Well, my point was that check should be like: select SND_USB_AUDIO_XXX if MEDIA_CONTROLLER && MEDIA_SUPPORT as there is no MEDIA_SUPPORT_MODULE in Kconfig. It's MEDIA_SUPPORT=m in Kconfig syntax. In C code, it's CONFIG_MEDIA_SUPPORT_MODULE, though. > > Above all, can MEDIA_CONTROLLER be tristate? It's currently a bool. > > If it's a tristate, it causes a problem wrt dependency. Imagine > > USB-audio is built-in while MC is a module. > > I don't think MEDIA_CONTROLLER will evebr tristate. I have this > logic to the following and it works with and without MEDIA_CONTROLLER > > diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig > index a452ad7..8ccbb35 100644 > --- a/sound/usb/Kconfig > +++ b/sound/usb/Kconfig > @@ -15,6 +15,7 @@ config SND_USB_AUDIO > select SND_RAWMIDI > select SND_PCM > select BITREVERSE > + select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER > help > Say Y here to include support for USB audio and USB MIDI > devices. > @@ -22,6 +23,11 @@ config SND_USB_AUDIO > To compile this driver as a module, choose M here: the module > will be called snd-usb-audio. > > +config SND_USB_AUDIO_USE_MEDIA_CONTROLLER > + bool "USB Audio/MIDI driver with Media Controller Support" > + depends on MEDIA_CONTROLLER > + default y > + > > > > >> help > >> Say Y here to include support for USB audio and USB MIDI > >> devices. > >> @@ -22,6 +23,12 @@ config SND_USB_AUDIO > >> To compile this driver as a module, choose M here: the module > >> will be called snd-usb-audio. > >> > >> +config SND_USB_AUDIO_USE_MEDIA_CONTROLLER > >> + bool "USB Audio/MIDI driver with Media Controller Support" > >> + depends on SND_USB_AUDIO && MEDIA_CONTROLLER > >> + depends on MEDIA_SUPPORT || MEDIA_SUPPORT_MODULE > > See above. I got rid of depends on for SND_USB_AUDIO There is a revere-select, so this menu itself doesn't play any role. If you want this item selectable, drop select from SND_USB_AUDIO but just have proper dependencies for this item: config SND_USB_AUDIO_USE_MEDIA_CONTROLLER bool "USB Audio/MIDI driver with Media Controller Support" depends on SND_USB_AUDIO depends on MEDIA_CONTROLLER default y (Whether default y or not is a remaining question: usually we keep it empty (i.e. no), but I put it here since you had it.) If you want this auto-selected unconditionally, drop the prompt and superfluous dependency (and don't set default y): config SND_USB_AUDIO .... select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER .... config SND_USB_AUDIO_USE_MEDIA_CONTROLLER bool Takashi