Re: [PATCH/RFC v3 06/19] video: display: OF support

[Philipp Zabel <p.zabel@pengutronix.de>]

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. 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.

> +		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
Philipp