From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Wed, 31 Jul 2013 22:16:47 +0200
Subject: [U-Boot] [PATCH 05/11] ARM: mxs: Add Creative ZEN XFi3 board
In-Reply-To: <20130731193032.B15FF380490@gemini.denx.de>
References: <1375220281-11132-1-git-send-email-marex@denx.de>
	<1375220281-11132-6-git-send-email-marex@denx.de>
	<20130731193032.B15FF380490@gemini.denx.de>
Message-ID: <201307312216.47374.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Dear Wolfgang Denk,

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

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