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