From mboxrd@z Thu Jan  1 00:00:00 1970
From: Anatolij Gustschin <agust@denx.de>
Date: Sun, 29 Nov 2009 22:24:44 +0100
Subject: [U-Boot] [PATCH V2 2/9] video: add amba-clcd prime-cell
In-Reply-To: <84aa128b71cbe6595dd5859c64a592b122cef723.1259185128.git.rubini@unipv.it>
References: <cover.1259185128.git.rubini@unipv.it>
	<84aa128b71cbe6595dd5859c64a592b122cef723.1259185128.git.rubini@unipv.it>
Message-ID: <20091129222444.2e454083@wker>
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

Hello Alessandro,

Alessandro Rubini <rubini-list@gnudd.com> wrote:

<snip>
> diff --git a/include/amba_clcd.h b/include/amba_clcd.h
> new file mode 100644
> index 0000000..09c9aa2
> --- /dev/null
> +++ b/include/amba_clcd.h
> @@ -0,0 +1,85 @@
...
> +/*
> + * CLCD Controller Internal Register addresses
> + */
> +#define CLCD_TIM0		0x00000000
> +#define CLCD_TIM1		0x00000004
> +#define CLCD_TIM2		0x00000008
> +#define CLCD_TIM3		0x0000000c
> +#define CLCD_UBAS		0x00000010
> +#define CLCD_LBAS		0x00000014

CLCD_LBAS isn't used in the code, please remove it.

...
> +#define CLCD_STAT		0x00000020
> +#define CLCD_INTR		0x00000024
> +#define CLCD_UCUR		0x00000028
> +#define CLCD_LCUR		0x0000002C
> +#define CLCD_PALL		0x00000200
> +#define CLCD_PALETTE		0x00000200
> +
> +#define TIM2_CLKSEL		(1 << 5)
> +#define TIM2_IVS		(1 << 11)
> +#define TIM2_IHS		(1 << 12)
> +#define TIM2_IPC		(1 << 13)
> +#define TIM2_IOE		(1 << 14)
> +#define TIM2_BCD		(1 << 26)

all these defines above are not used in the code,
please remove them.

> +#define CNTL_LCDEN		(1 << 0)
> +#define CNTL_LCDBPP1		(0 << 1)
> +#define CNTL_LCDBPP2		(1 << 1)
> +#define CNTL_LCDBPP4		(2 << 1)
> +#define CNTL_LCDBPP8		(3 << 1)
> +#define CNTL_LCDBPP16		(4 << 1)
> +#define CNTL_LCDBPP16_565	(6 << 1)
> +#define CNTL_LCDBPP24		(5 << 1)
> +#define CNTL_LCDBW		(1 << 4)
> +#define CNTL_LCDTFT		(1 << 5)
> +#define CNTL_LCDMONO8		(1 << 6)
> +#define CNTL_LCDDUAL		(1 << 7)
> +#define CNTL_BGR		(1 << 8)
> +#define CNTL_BEBO		(1 << 9)
> +#define CNTL_BEPO		(1 << 10)
> +#define CNTL_LCDPWR		(1 << 11)
> +#define CNTL_LCDVCOMP(x)	((x) << 12)
> +#define CNTL_LDMAFIFOTIME	(1 << 15)
> +#define CNTL_WATERMARK		(1 << 16)

It seems that only CNTL_LCDEN is uses in the code, please
remove other unused defines, too.

...
> +/* u-boot specific: cooked information (not used at this point) */
> +struct clcd_params {
> +	int left_margin, right_margin, upper_margin, lower_margin;
> +	int hsync_len, vsync_len;
> +	int sync, vmode;
> +};

this is also not used, please do not add unused code.
I couldn't comment on this in the first patch version, sorry.

Best regards,
Anatolij