[U-Boot] [PATCH 05/11] ARM: mxs: Add Creative ZEN XFi3 board

Wolfgang Denk
2013-07-31T19:30:32Z

Dear Marek,

In message <1375220281-11132-6-git-send-email-marex@denx.de> you wrote:
>
...
> + /* Program the SmartLCD controller */
> + writel(LCDIF_CTRL1_RECOVER_ON_UNDERFLOW, &regs->hw_lcdif_ctrl1_set);
> +
> + writel(0x03030202, &regs->hw_lcdif_timing);

Please don't use such magic numbers.

> + mxsfb_write_register(1, 0x01c);
> +
> + mxsfb_write_register(2, 0x100);
> + /* 0x30 flips the LCD */
> + mxsfb_write_register(3, 0x1038);
> + mxsfb_write_register(8, 0x808);
> + /* This can possibly contain 0x111 */
> + mxsfb_write_register(0xc, 0x0);
> + mxsfb_write_register(0xf, 0xc01);
> + mxsfb_write_register(0x20, 0);
> + mxsfb_write_register(0x21, 0);
> + mdelay(30);
> + mxsfb_write_register(0x10, 0xa00);
> + mxsfb_write_register(0x11, 0x1038);
> + mdelay(30);
> + mxsfb_write_register(0x12, 0x1010);
> + mxsfb_write_register(0x13, 0x50);
> + mxsfb_write_register(0x14, 0x4f58);
> + mxsfb_write_register(0x30, 0);
> + mxsfb_write_register(0x31, 0xdb);
> + mxsfb_write_register(0x32, 0);
> + mxsfb_write_register(0x33, 0);
> + mxsfb_write_register(0x34, 0xdb);
> + mxsfb_write_register(0x35, 0);
> + mxsfb_write_register(0x36, 0xaf);
> + mxsfb_write_register(0x37, 0);
> + mxsfb_write_register(0x38, 0xdb);
> + mxsfb_write_register(0x39, 0);
> + mxsfb_write_register(0x50, 0);
> + mxsfb_write_register(0x51, 0x705);
> + mxsfb_write_register(0x52, 0xe0a);
> + mxsfb_write_register(0x53, 0x300);
> + mxsfb_write_register(0x54, 0xa0e);
> + mxsfb_write_register(0x55, 0x507);
> + mxsfb_write_register(0x56, 0);
> + mxsfb_write_register(0x57, 3);
> + mxsfb_write_register(0x58, 0x90a);
> + mxsfb_write_register(0x59, 0xa09);
> + mdelay(30);
> + mxsfb_write_register(7, 0x1017);
> + mdelay(40);
> + mxsfb_write_register(0x36, 0xaf);
> + mxsfb_write_register(0x37, 0);
> + mxsfb_write_register(0x38, 0xdb);
> + mxsfb_write_register(0x39, 0);
> + mxsfb_write_register(0x20, 0);
> + mxsfb_write_register(0x21, 0);

This is terrible. WHy don't you use an array for these values, and
then a simple loop to program the (register,value) pairs? That would
not only be much easier to read and to maintain but also result in
smaller code...

> +#define CONFIG_MACH_TYPE 0xffffffff

NAK. Please use a proper value.

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
God made the integers; all else is the work of Man. - Kronecker