[U-Boot] [PATCH V2 5/7] cm-t35: add support for dvi displays

[Wolfgang Denk]

Dear Nikita Kiryanov,

In message <1359463349-11649-6-git-send-email-nikita at compulab.co.il> you wrote:
> Add support for dvi displays with user selectable dvi presets.
...
> --- a/board/cm_t35/cm_t35.c
> +++ b/board/cm_t35/cm_t35.c
> @@ -216,6 +216,9 @@ static void cm_t3x_set_common_muxconf(void)
>  	/* SB-T35 Ethernet */
>  	MUX_VAL(CP(GPMC_NCS4),		(IEN  | PTU | EN  | M0)); /*GPMC_nCS4*/
>  
> +	/* DVI enable */
> +	MUX_VAL(CP(GPMC_NCS3),		(IDIS  | PTU | DIS  | M4));/*GPMC_nCS3*/

Is it intentional and correct to always enable DVI, even when this is
not configured by the user?


> +/*
> + * The frame buffer is allocated before we have the chance to parse user input.

This seems broken to me.

Please explain why you think so?

> + * vl_{col | row} to the maximal resolution supported by OMAP3.
> + */
> +vidinfo_t panel_info = {
> +	.vl_col  = 1400,
> +	.vl_row  = 1050,
> +	.vl_bpix = LCD_BPP,
> +	.cmap = (ushort *)0x80100000,

Can we please avoid such hard coded magic numbers?


> +/*
> + * env_parse_displaytype() - parse display type.
> + *
> + * Parses the environment variable "displaytype", which contains the
> + * name of the display type or preset, in which case it applies its
> + * configurations.

So we have yet another custom implementation for setting the display
type?  Can we please agree on using common standard methods?  thanks.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A father doesn't destroy his children.
	-- Lt. Carolyn Palamas, "Who Mourns for Adonais?",
	   stardate 3468.1.