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;

> +
> +     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?

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?

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to