[U-Boot] [PATCH] Add 16bpp BMP support

[Wolfgang Denk]

Dear Mark Jackson,

In message <497EDF68.7050801 at mimc.co.uk> you wrote:
> This patch adds 16bpp BMP support to the common lcd code.
> 
> At the moment it's only been tested on the MIMC200 AVR32 board, but the hooks 
> are in place to extend this to other platforms.
> 
> Signed-off-by: Mark Jackson <mpfj at mimc.co.uk>
> ---
...
> +	case 16:
> +		for (i = 0; i < height; ++i) {
> +			WATCHDOG_RESET();
> +			for (j = 0; j < width; j++) {
> +#if defined(CONFIG_PXA250)
> +#error 16 bpp support not added for PXA250
> +#elif defined(CONFIG_ATMEL_LCD)
> +#if defined(CONFIG_ATMEL_LCD_BGR555)

Oops. Right in the middle of a for loop?
This is probably not exactly a good
place for the placement of such a generic compile time check.

> +				*(fb++) = ((bmap[0] & 0x1f) << 2) | (bmap[1] & 0x03);
> +				*(fb++) = (bmap[0] & 0xe0) | ((bmap[1] & 0x7c) >> 2);
> +				bmap += 2;
> +#else
> +				*(fb++) = *(bmap++);
> +				*(fb++) = *(bmap++);
> +#endif
> +#elif defined(CONFIG_MPC823)
> +#error 16 bpp support not added for MPC823
> +#elif defined(CONFIG_MCC200)
> +#error 16 bpp support not added for MCC200
> +#endif

Ditto here.

I don't like this approach. The code is too ugly. If you need dfferent
versions, thatn factor out the different code into functions, or
similar.


NAK.

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
Even historians fail to learn from history -- they repeat the same
mistakes.
	-- John Gill, "Patterns of Force", stardate 2534.7