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

Marek Vasut marex at denx.de
Wed Jul 31 22:16:47 CEST 2013


Dear Wolfgang Denk,

> Dear Marek,
> 
> In message <1375220281-11132-6-git-send-email-marex at 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.

Fixed

[...]

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

True, fixed.

> > +#define CONFIG_MACH_TYPE	0xffffffff
> 
> NAK.  Please use a proper value.

Removed altogether, we use only DT here.

Best regards,
Marek Vasut


More information about the U-Boot mailing list