[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <56D76FBF.9050209@osg.samsung.com>
Date: Wed, 2 Mar 2016 15:57:03 -0700
From: Shuah Khan <shuahkh@....samsung.com>
To: Dan Carpenter <dan.carpenter@...cle.com>
Cc: mchehab@....samsung.com, tiwai@...e.com, clemens@...isch.de,
hans.verkuil@...co.com, laurent.pinchart@...asonboard.com,
sakari.ailus@...ux.intel.com, javier@....samsung.com,
pawel@...iak.com, m.szyprowski@...sung.com,
kyungmin.park@...sung.com, perex@...ex.cz, arnd@...db.de,
tvboxspy@...il.com, crope@....fi, ruchandani.tina@...il.com,
corbet@....net, chehabrafael@...il.com, k.kozlowski@...sung.com,
stefanr@...6.in-berlin.de, inki.dae@...sung.com,
jh1009.sung@...sung.com, elfring@...rs.sourceforge.net,
prabhakar.csengg@...il.com, sw0312.kim@...sung.com,
p.zabel@...gutronix.de, ricardo.ribalda@...il.com,
labbott@...oraproject.org, pierre-louis.bossart@...ux.intel.com,
ricard.wanderlof@...s.com, julian@...st.de, takamichiho@...il.com,
dominic.sacre@....de, misterpib@...il.com, daniel@...que.org,
gtmkramer@...all.nl, normalperson@...t.net, joe@...po.co.uk,
linuxbugs@...tgam.net, johan@...ud.se, klock.android@...il.com,
nenggun.kim@...sung.com, j.anaszewski@...sung.com,
geliangtang@....com, albert@...tsing.nl,
linux-kernel@...r.kernel.org, linux-media@...r.kernel.org,
alsa-devel@...a-project.org, Shuah Khan <shuahkh@....samsung.com>
Subject: Re: [PATCH v5 22/22] sound/usb: Use Media Controller API to share
media resources
On 03/02/2016 01:41 PM, Dan Carpenter wrote:
> On Wed, Mar 02, 2016 at 09:50:31AM -0700, Shuah Khan wrote:
>> + mctl = kzalloc(sizeof(*mctl), GFP_KERNEL);
>> + if (!mctl)
>> + return -ENOMEM;
>> +
>> + mctl->media_dev = mdev;
>> + if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
>> + intf_type = MEDIA_INTF_T_ALSA_PCM_PLAYBACK;
>> + mctl->media_entity.function = MEDIA_ENT_F_AUDIO_PLAYBACK;
>> + mctl->media_pad.flags = MEDIA_PAD_FL_SOURCE;
>> + mixer_pad = 1;
>> + } else {
>> + intf_type = MEDIA_INTF_T_ALSA_PCM_CAPTURE;
>> + mctl->media_entity.function = MEDIA_ENT_F_AUDIO_CAPTURE;
>> + mctl->media_pad.flags = MEDIA_PAD_FL_SINK;
>> + mixer_pad = 2;
>> + }
>> + mctl->media_entity.name = pcm->name;
>> + media_entity_pads_init(&mctl->media_entity, 1, &mctl->media_pad);
>> + ret = media_device_register_entity(mctl->media_dev,
>> + &mctl->media_entity);
>> + if (ret)
>> + goto err1;
>
> Could we give this label a meaningful name instead of a number?
> goto free_mctl;
I do see other places where numbered labels are used.
Names might help with code readability.
register_entity_fail probably makes more sense as a
label than free_mctl. In any case, I can address the
labels in a follow-on patch.
>
>> +
>> + mctl->intf_devnode = media_devnode_create(mdev, intf_type, 0,
>> + MAJOR(pcm_dev->devt),
>> + MINOR(pcm_dev->devt));
>> + if (!mctl->intf_devnode) {
>> + ret = -ENOMEM;
>> + goto err2;
>
> goto unregister_device;
>
>> + }
>> + mctl->intf_link = media_create_intf_link(&mctl->media_entity,
>> + &mctl->intf_devnode->intf,
>> + MEDIA_LNK_FL_ENABLED);
>> + if (!mctl->intf_link) {
>> + ret = -ENOMEM;
>> + goto err3;
>
> goto delete_devnode;
>
>> + }
>> +
>> + /* create link between mixer and audio */
>> + media_device_for_each_entity(entity, mdev) {
>> + switch (entity->function) {
>> + case MEDIA_ENT_F_AUDIO_MIXER:
>> + ret = media_create_pad_link(entity, mixer_pad,
>> + &mctl->media_entity, 0,
>> + MEDIA_LNK_FL_ENABLED);
>> + if (ret)
>> + goto err4;
>
> This is a bit weird because we're inside a loop. Shouldn't we call
> media_entity_remove_links() or something if this is the second time
> through the loop?
Links are removed from media_device_unregister_entity()
which is called in the error path.
>
> I don't understand this. The kernel has the media_entity_cleanup() stub
> function which is supposed to do this but it hasn't been implemented
> yet?
>
Please see above. Links are removed when entity is
unregistered. media_entity_cleanup() is a stub. It
isn't intended for removing links.
thanks,
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@....samsung.com | (970) 217-8978
Powered by blists - more mailing lists