[U-Boot] Antw: Re: [PATCH] Add suport for the graphic controller included in the JADE SOC

[Matthias Weisser]

>>> Anatolij Gustschin <agust at denx.de> schrieb am 01.07.2009 um 15:40:
> Dear Matthias,
> 
> Thanks for this patch! Please see some comments/issues below which
we
> should resolve before committing the driver.

First of all thanks for your detailed comments. I fix them all before 
resending the patch. Please see some additional comments at some
points.

>> +		if(0 == bpp){
> 
> if (bpp == 0) {

The spaces are OK but why turn around the comparison? I always use 
this style to prevent from unwanted assignments. Well, GCC warns 
about assignments in ifs and as u-boot is GCC only I can do that.

> We also should use macros for register offsets, I think. Can we
> coordinate efforts for fixing this in mb862xx driver also?

As the graphic controller in the jade soc is only a slight evolution
of
the coral graphic chip (additional display output/video input) I think

we may even merge the two drivers into one with some #ifdefs to deal
with the differences.

When I started to implement the driver I decided against that idea
because I don't have any hardware here to test the "non-jade" case.

But a common include file with the register offsets seems to be a
good idea to me.

Thanks for your time,
Matthias

------------------------------------ 
Amtsgericht Freiburg HRA 602707
Ust. ID-Nr.: DE232464428

Geschäftsführer: 
Dipl. Ing. (FH) Martin Graf 
Dipl. Ing. (FH) David Graf 
Dipl. Inf. Fabian Graf
 
Komplementärin:
GRAF-SYTECO Verwaltungs-GmbH
Amtsgericht Freiburg HRB 602868 
------------------------------------