Hi Philipp,
On Tuesday 27 August 2013 11:30:51 Philipp Zabel wrote:
> Hi Laurent,
>
> I have another small issue with the graph helpers below:
>
> Am Samstag, den 10.08.2013, 01:03 +0200 schrieb Laurent Pinchart:
> [...]
>
> > +/*
> > -------------------------------------------------------------------------
> > ---->
> > * Graph Helpers
> > */
> >
> > @@ -420,6 +599,161 @@ int display_entity_link_graph(struct device *dev,
> > struct list_head *entities)>
> > }
> > EXPORT_SYMBOL_GPL(display_entity_link_graph);
> >
> > +#ifdef CONFIG_OF
> > +
> > +static int display_of_entity_link_entity(struct device *dev,
> > + struct display_entity *entity,
> > + struct list_head *entities,
> > + struct display_entity *root)
> > +{
> > + u32 link_flags = MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED;
> > + const struct device_node *node = entity->dev->of_node;
> > + struct media_entity *local = &entity->entity;
> > + struct device_node *ep = NULL;
> > + int ret = 0;
> > +
> > + dev_dbg(dev, "creating links for entity %s\n", local->name);
> > +
> > + while (1) {
> > + struct media_entity *remote = NULL;
> > + struct media_pad *remote_pad;
> > + struct media_pad *local_pad;
> > + struct display_of_link link;
> > + struct display_entity *ent;
> > + struct device_node *next;
> > +
> > + /* Get the next endpoint and parse its link. */
> > + next = display_of_get_next_endpoint(node, ep);
> > + if (next == NULL)
> > + break;
> > +
> > + of_node_put(ep);
> > + ep = next;
> > +
> > + dev_dbg(dev, "processing endpoint %s\n", ep->full_name);
> > +
> > + ret = display_of_parse_link(ep, &link);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to parse link for %s\n",
> > + ep->full_name);
> > + continue;
> > + }
> > +
> > + /* Skip source pads, they will be processed from the other end
> > of
> > + * the link.
> > + */
> > + if (link.local_port >= local->num_pads) {
> > + dev_err(dev, "invalid port number %u on %s\n",
> > + link.local_port, link.local_node->full_name);
> > + display_of_put_link(&link);
> > + ret = -EINVAL;
> > + break;
> > + }
> > +
> > + local_pad = &local->pads[link.local_port];
> > +
> > + if (local_pad->flags & MEDIA_PAD_FL_SOURCE) {
> > + dev_dbg(dev, "skipping source port %s:%u\n",
> > + link.local_node->full_name, link.local_port);
> > + display_of_put_link(&link);
> > + continue;
> > + }
> > +
> > + /* Find the remote entity. If not found, just skip the link as
> > + * it goes out of scope of the entities handled by the notifier.
> > + */
> > + list_for_each_entry(ent, entities, list) {
> > + if (ent->dev->of_node == link.remote_node) {
> > + remote = &ent->entity;
> > + break;
> > + }
> > + }
> > +
> > + if (root->dev->of_node == link.remote_node)
> > + remote = &root->entity;
> > +
> > + if (remote == NULL) {
> > + dev_dbg(dev, "no entity found for %s\n",
> > + link.remote_node->full_name);
> > + display_of_put_link(&link);
> > + continue;
> > + }
> > +
> > + if (link.remote_port >= remote->num_pads) {
> > + dev_err(dev, "invalid port number %u on %s\n",
> > + link.remote_port, link.remote_node->full_name);
> > + display_of_put_link(&link);
> > + ret = -EINVAL;
> > + break;
> > + }
> > +
> > + remote_pad = &remote->pads[link.remote_port];
> > +
> > + display_of_put_link(&link);
> > +
> > + /* Create the media link. */
> > + dev_dbg(dev, "creating %s:%u -> %s:%u link\n",
> > + remote->name, remote_pad->index,
> > + local->name, local_pad->index);
> > +
> > + ret = media_entity_create_link(remote, remote_pad->index,
> > + local, local_pad->index,
> > + link_flags);
> > + if (ret < 0) {
> > + dev_err(dev,
> > + "failed to create %s:%u -> %s:%u link\n",
> > + remote->name, remote_pad->index,
> > + local->name, local_pad->index);
> > + break;
> > + }
> > + }
> > +
> > + of_node_put(ep);
> > + return ret;
> > +}
> > +
> > +/**
> > + * display_of_entity_link_graph - Link all entities in a graph
> > + * @dev: device used to print debugging and error messages
> > + * @root: optional root display entity
> > + * @entities: list of display entities in the graph
> > + *
> > + * This function creates media controller links for all entities in a
> > graph + * based on the device tree graph representation. It relies on all
> > entities + * having been instantiated from the device tree.
> > + *
> > + * The list of entities is typically taken directly from a display
> > notifier + * done list. It will thus not include any display entity not
> > handled by the + * notifier, such as entities directly accessible by the
> > caller without going + * through the notification process. The optional
> > root entity parameter can be + * used to pass such a display entity and
> > include it in the graph. For all + * practical purpose the root entity is
> > handled is if it was part of the + * entities list.
> > + *
> > + * Return 0 on success or a negative error code otherwise.
> > + */
> > +int display_of_entity_link_graph(struct device *dev, struct list_head
> > *entities, + struct display_entity *root)
> > +{
> > + struct display_entity *entity;
> > + int ret;
> > +
> > + list_for_each_entry(entity, entities, list) {
> > + if (WARN_ON(entity->match->type != DISPLAY_ENTITY_BUS_DT))
> > + return -EINVAL;
> > +
> > + ret = display_of_entity_link_entity(dev, entity, entities,
> > + root);
> > + if (ret < 0)
> > + return ret;
> > + }
> > +
> > + return display_of_entity_link_entity(dev, root, entities, root);
> > +}
> > +EXPORT_SYMBOL_GPL(display_of_entity_link_graph);
>
> The root display entity given to display_of_entity_link_graph is documented
> to be optional. Therefore, do not try to dereference root if it is NULL:
Good catch ! I'll fix it in v4, thank you.
> diff --git a/drivers/video/display/display-core.c
> b/drivers/video/display/display-core.c index 328ead7..6c8094f 100644
> --- a/drivers/video/display/display-core.c
> +++ b/drivers/video/display/display-core.c
> @@ -669,7 +669,7 @@ static int display_of_entity_link_entity(struct device
> *dev, }
> }
>
> - if (root->dev->of_node == link.remote_node)
> + if (root && root->dev->of_node == link.remote_node)
> remote = &root->entity;
>
> if (remote == NULL) {
> @@ -748,6 +748,9 @@ int display_of_entity_link_graph(struct device *dev,
> struct list_head *entities, return ret;
> }
>
> + if (!root)
> + return 0;
> +
> return display_of_entity_link_entity(dev, root, entities, root);
> }
> EXPORT_SYMBOL_GPL(display_of_entity_link_graph);
--
Regards,
Laurent Pinchart
