From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shuah Khan Subject: Re: [PATCH MC Next Gen v2 2/3] sound/usb: Create media mixer function and control interface entities Date: Tue, 3 Nov 2015 09:43:49 -0700 Message-ID: <5638E445.50900@osg.samsung.com> References: <2f95ce0190c05e994e02bdc4393be21ec7609adf.1445380851.git.shuahkh@osg.samsung.com> <562D4B9E.7010006@osg.samsung.com> <5638DB95.3070007@osg.samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-media-owner@vger.kernel.org To: Takashi Iwai Cc: mchehab@osg.samsung.com, perex@perex.cz, chehabrafael@gmail.com, hans.verkuil@cisco.com, prabhakar.csengg@gmail.com, chris.j.arges@canonical.com, linux-media@vger.kernel.org, alsa-devel@alsa-project.org, Shuah Khan List-Id: alsa-devel@alsa-project.org On 11/03/2015 09:23 AM, Takashi Iwai wrote: > On Tue, 03 Nov 2015 17:06:45 +0100, > Shuah Khan wrote: >> >> On 10/25/2015 03:37 PM, Shuah Khan wrote: >>> On 10/22/2015 01:16 AM, Takashi Iwai wrote: >>>> On Wed, 21 Oct 2015 01:25:15 +0200, >>>> Shuah Khan wrote: >>>>> >>>>> Add support for creating MEDIA_ENT_F_AUDIO_MIXER entity for >>>>> each mixer and a MEDIA_INTF_T_ALSA_CONTROL control interface >>>>> entity that links to mixer entities. MEDIA_INTF_T_ALSA_CONTROL >>>>> entity corresponds to the control device for the card. >>>>> >>>>> Signed-off-by: Shuah Khan >>>>> --- >>>>> sound/usb/card.c | 5 +++ >>>>> sound/usb/media.c | 89 >>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> sound/usb/media.h | 20 ++++++++++++ >>>>> sound/usb/mixer.h | 1 + >>>>> sound/usb/usbaudio.h | 1 + >>>>> 5 files changed, 116 insertions(+) >>>>> >>>>> diff --git a/sound/usb/card.c b/sound/usb/card.c >>>>> index 469d2bf..d004cb4 100644 >>>>> --- a/sound/usb/card.c >>>>> +++ b/sound/usb/card.c >>>>> @@ -560,6 +560,9 @@ static int usb_audio_probe(struct usb_interface >>>>> *intf, >>>>> if (err < 0) >>>>> goto __error; >>>>> >>>>> + /* Create media entities for mixer and control dev */ >>>>> + media_mixer_init(chip); >>>>> + >>>>> usb_chip[chip->index] = chip; >>>>> chip->num_interfaces++; >>>>> chip->probing = 0; >>>>> @@ -616,6 +619,8 @@ static void usb_audio_disconnect(struct >>>>> usb_interface *intf) >>>>> list_for_each(p, &chip->midi_list) { >>>>> snd_usbmidi_disconnect(p); >>>>> } >>>>> + /* delete mixer media resources */ >>>>> + media_mixer_delete(chip); >>>>> /* release mixer resources */ >>>>> list_for_each_entry(mixer, &chip->mixer_list, list) { >>>>> snd_usb_mixer_disconnect(mixer); >>>>> diff --git a/sound/usb/media.c b/sound/usb/media.c >>>>> index 0cbfee6..a26ea8b 100644 >>>>> --- a/sound/usb/media.c >>>>> +++ b/sound/usb/media.c >>>>> @@ -199,4 +199,93 @@ void media_stop_pipeline(struct >>>>> snd_usb_substream *subs) >>>>> if (mctl) >>>>> media_disable_source(mctl); >>>>> } >>>>> + >>>>> +int media_mixer_init(struct snd_usb_audio *chip) >>>>> +{ >>>>> + struct device *ctl_dev = &chip->card->ctl_dev; >>>>> + struct media_intf_devnode *ctl_intf; >>>>> + struct usb_mixer_interface *mixer; >>>>> + struct media_device *mdev; >>>>> + struct media_mixer_ctl *mctl; >>>>> + u32 intf_type = MEDIA_INTF_T_ALSA_CONTROL; >>>>> + int ret; >>>>> + >>>>> + mdev = media_device_find_devres(&chip->dev->dev); >>>>> + if (!mdev) >>>>> + return -ENODEV; >>>>> + >>>>> + ctl_intf = (struct media_intf_devnode *) >>>>> chip->ctl_intf_media_devnode; >>>> >>>> Why do we need cast? Can't chip->ctl_intf_media_devnode itself be >>>> struct media_intf_devndoe pointer? >>> >>> Yeah. There is no need to cast here. I will fix it. >> >> Sorry I misspoke. The reason for this cast is ctl_intf_media_devnode >> is void to avoid including media.h and other media files in usbaudio.h > > You can declare the struct without definition in each header file. > So just declare it and use it in usbaudio.h like: > > struct media_intf_devnode; > > struct snd_usb_audio { > .... > struct media_intf_devnode *ctl_intf_media_devnode; > .... > > And even if you're using a void pointer there instead of the explicit > struct pointer, the cast is superfluous. The implicit cast between a > void pointer and any other pointer is valid in plain C. > Yes. cast is definitely not necessary. I will drop the cast and leave the rest alone. thanks, -- Shuah -- Shuah Khan Sr. Linux Kernel Developer Open Source Innovation Group Samsung Research America (Silicon Valley) shuahkh@osg.samsung.com | (970) 217-8978