Re: [PATCH v2 2/2] drm/tegra: Obtain head number from DT

[Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]

On 01/14/2014 07:45 AM, Thierry Reding wrote:
> The head number of a given display controller is fixed in hardware and
> required to program outputs appropriately. Relying on the driver probe
> order to determine this number will not work, since that could yield a
> situation where the second head was probed first and would be assigned
> head number 0 instead of 1.
> 
> By explicitly specifying the head number in the device tree, it is no
> longer necessary to rely on these assumptions. As a fallback, if the
> property isn't available, derive the head number from the display
> controller node's position in the device tree. That's somewhat more
> reliable than the previous default but not a proper solution.

The series,
Tested-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

This patch should really have been sent to the DT maintainers and list
since it changes a DT binding...

> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c

> +static int tegra_dc_parse_dt(struct tegra_dc *dc)
> +{
> +	struct device_node *np;
> +	u32 value = 0;
> +	int err;
> +
> +	err = of_property_read_u32(dc->dev->of_node, "nvidia,head", &value);

If of_property_read_u32() returns an error, does it guarantee that value
is left unchanged? I suspect it'd be safer to add ...

> +	if (err < 0) {
> +		dev_err(dc->dev, "missing \"nvidia,head\" property\n");
> +
> +		/*
> +		 * If the nvidia,head property isn't present, try to find the
> +		 * correct head number by looking up the position of this
> +		 * display controller's node within the device tree. Assuming
> +		 * that the nodes are ordered properly in the DTS file and
> +		 * that the translation into a flattened device tree blob
> +		 * preserves that ordering this will actually yield the right
> +		 * head number.
> +		 *
> +		 * If those assumptions don't hold, this will still work for
> +		 * cases where only a single display controller is used.
> +		 */

... "value = 0;" here?

> +		for_each_matching_node(np, tegra_dc_of_match) {
> +			if (np == dc->dev->of_node)
> +				break;
> +
> +			value++;
> +		}
> +	}
> +
> +	dc->pipe = value;