Re: [alsa-devel] [PATCH v5 22/22] sound/usb: Use Media Controller API to share media resources

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