Hi Philipp, On Tuesday 13 August 2013 16:37:07 Philipp Zabel wrote: > Hi Laurent, > > thanks for this update. I'm very happy about the move to the display entity > model, given that the i.MX6 IPU has both drm/display and v4l2/capture ports > in a single device - this will allow to use a unified device tree binding > scheme. Thanks for the support :-) > I'm still trying to see how this all fits together, so far I have only one > comment, below. > > Am Freitag, den 09.08.2013, 19:14 +0200 schrieb Laurent Pinchart: > [...] > > > +static int display_of_parse_dt(struct display_entity_notifier *notifier, > > + struct list_head *entities, > > + struct device_node *node) > > +{ > > + struct display_entity_of *entity; > > + struct device_node *remote; > > + struct device_node *ep = NULL; > > + struct device_node *next; > > + unsigned int num_entities = 0; > > + int ret = 0; > > + > > + /* Walk the device tree and build a list of nodes. */ > > + dev_dbg(notifier->dev, "parsing node %s\n", node->full_name); > > + > > + while (1) { > > + next = display_of_get_next_endpoint(node, ep); > > + if (next == NULL) > > + break; > > + > > + of_node_put(ep); > > + ep = next; > > + > > + dev_dbg(notifier->dev, "handling endpoint %s\n", ep->full_name); > > + > > + remote = display_of_get_remote_port_parent(ep); > > + if (remote == NULL) > > + continue; > > + > > + /* Skip entities that we have already processed. */ > > + if (display_of_find_entity(entities, remote) || remote == node) { > > + dev_dbg(notifier->dev, > > + "entity %s already in list, skipping\n", > > + remote->full_name); > > + continue; > > + } > > device tree nodes with status = "disabled" should be skipped here: > > if (!of_device_is_available(remote)) { > dev_dbg(notifier->dev, > "entity %s is disabled, skipping\n", > remote->full_name); > continue; > } > > Otherwise the completion notification will never be delivered if there > are any disabled entities in the graph. That's a good point, but if a device is disabled, why would it be in the DT graph in the first place ? Do you have a use case for this ? > > + entity = kzalloc(sizeof(*entity), GFP_KERNEL); > > + if (entity == NULL) { > > + of_node_put(remote); > > + ret = -ENOMEM; > > + break; > > + } > > + > > + dev_dbg(notifier->dev, "adding remote entity %s to list\n", > > + remote->full_name); > > + > > + entity->node = remote; > > + list_add_tail(&entity->list, entities); > > + num_entities++; > > + } > > + > > + of_node_put(ep); > > + > > + if (ret < 0) > > + return ret; > > + > > + return num_entities; > > +} > > [...] -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel