[U-Boot] [PATCH V2 2/9] video: add amba-clcd prime-cell

[Anatolij Gustschin <agust@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