On 08/14/2015 05:18 PM, Hans Verkuil wrote: > On 08/14/2015 04:56 PM, Mauro Carvalho Chehab wrote: >> Just like entities and pads, links also need to have unique >> Object IDs along a given media controller. >> >> So, let's add a media_graph_obj inside it and initialize >> the object then a new link is created. >> >> Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx> >> >> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c >> index 3ac5803b327e..9f02939c2864 100644 >> --- a/drivers/media/media-device.c >> +++ b/drivers/media/media-device.c >> @@ -466,6 +466,8 @@ void media_device_unregister_entity(struct media_entity *entity) >> graph_obj_remove(&entity->graph_obj); >> for (i = 0; i < entity->num_pads; i++) >> graph_obj_remove(&entity->pads[i].graph_obj); >> + for (i = 0; entity->num_links; i++) >> + graph_obj_remove(&entity->links[i].graph_obj); >> list_del(&entity->list); >> spin_unlock(&mdev->lock); >> entity->parent = NULL; >> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c >> index d3dee6fc79d7..4f18bd10b162 100644 >> --- a/drivers/media/media-entity.c >> +++ b/drivers/media/media-entity.c >> @@ -50,6 +50,8 @@ void graph_obj_init(struct media_device *mdev, >> gobj->id |= ++mdev->entity_id; >> case MEDIA_GRAPH_PAD: >> gobj->id |= ++mdev->pad_id; >> + case MEDIA_GRAPH_LINK: >> + gobj->id |= ++mdev->pad_id; > > Same issue with missing breaks. Also for links you want to use link_id, not > pad_id. Clearly a copy-and-paste mistake. > > A bigger question is whether you actually need graph_obj for a link? Links are > *between* graph objects, but are they really graph objects in their own right? > > Currently a link is identified by the source/sink objects and I think that is > all you need. I didn't realize that when reviewing the v3 patch series, but I'm > now wondering about it. > > If you *don't* make links a graph_obj, will anything break or become difficult > to use? I don't think so, but let me know if I am overlooking something. Hmm, you probably want the list part of a graph_obj since you need to link the links (so to speak) in a list. I'm not sure if you need unique IDs for a link object, but it doesn't hurt either. Just ignore my comments about using the graph_obj in media_link. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html