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