[U-Boot] [PATCH v2 10/19] tegra: Add LCD driver

[Stephen Warren]

On 06/13/2012 10:19 AM, Simon Glass wrote:
> This driver supports driving a single LCD and providing a U-Boot console
> on it.

> +int fdt_decode_lcd(const void *blob, struct fdt_lcd *config)

> +	fdtdec_decode_gpio(blob, display_node, "nvidia,backlight-vdd-gpios",
> +			   &config->backlight_vdd);

Given that's a power supply control, I expect this to be represented as
a regulator in the DT, not as a simple GPIO (it could be controlled by a
register bit in a PMU on some boards).

> +void lcd_enable(void)
> +{
> +	/*
> +	 * Backlight and power init will be done separately in
> +	 * tegra_lcd_check_next_stage(), which should be called in
> +	 * board_late_init().
> +	 *
> +	 * U-Boot code supports only colour depth, selected at compile time.
> +	 * The device tree setting should match this. Otherwise the display
> +	 * will not look right, and U-Boot may crash.
> +	 */
> +	if (config.log2_bpp != LCD_BPP) {
> +		printf("%s: Error: LCD depth configured in FDT (%d = %dbpp)"
> +			" must match setting of LCD_BPP (%d)\n", __func__,
> +		       config.log2_bpp, config.bpp, LCD_BPP);
> +	}
> +}

I wonder why even read the configuration from DT if the value can't be used?

> diff --git a/lib/fdtdec.c b/lib/fdtdec.c

> @@ -43,6 +43,7 @@ static const char * const compat_names[COMPAT_COUNT] = {

> +	COMPAT(NVIDIA_TEGRA20_DISPLAY, "nvidia,tegra20-display"),

Surely all the DT plumbing for the display controller itself should be
in the previous patch that adds the display controller driver.

The LCD stuff that sits on top of that seems like it should be a
separate node in DT, and the two patches kept much more separate.