Matthias Weisser wrote:
>>>> Anatolij Gustschin <ag...@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.
it is a matter of personal preference. "if (0 == bpp) {" is OK for
me, too.
>> 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.
if you want to use the existing driver, it shouldn't be too complicated.
To use the mb862xx driver you can implement "board_video_init()" in your
board code in which you initialize both display controllers, set
"GraphicDevice mb862xx" structure fields winSizeX, etc. and return
the video RAM base. Then additionally implement "board_get_regs()" in
your board code, e.g.
#include <mb862xx.h>
static const gdc_regs init_regs [] =
{
{0, 0}
}
const gdc_regs *board_get_regs (void)
{
return init_regs;
}
and define CONFIG_VIDEO_MB862xx in the board config file.
see e.g. the appropriate code in "board/lwmon5/lwmon5.c".
It does the display controller initialization differently and
therefore my suggestion to do the display controller init in
custom "board_video_init()".
We have to fix some register access macros in the mb862xx driver
for JADE and it should work. I can try to provide a patch for
this fix in mb862xx.c.
> But a common include file with the register offsets seems to be a
> good idea to me.
I'will prepare a list of register offset defines for "include/mb862xx.h"
and send a patch so that you can use them in your code.
Thanks,
Anatolij
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
