Dear Matthias,
Matthias Weisser wrote:
This patch adds support for the display controller in the MB86R01 'Jade' SoC.
Looks better now, but still some small issues. Please see comments below, fix and resubmit. Thanks!
...
--- a/drivers/video/Makefile +++ b/drivers/video/Makefile @@ -36,6 +36,7 @@ COBJS-$(CONFIG_VIDEO_SED13806) += sed13806.o COBJS-$(CONFIG_SED156X) += sed156x.o COBJS-$(CONFIG_VIDEO_SM501) += sm501.o COBJS-$(CONFIG_VIDEO_SMI_LYNXEM) += smiLynxEM.o +COBJS-$(CONFIG_VIDEO_JADEGDC) += jadegdc.o
please keep list sorted in alphabetical order.
...
--- a/drivers/video/cfb_console.c +++ b/drivers/video/cfb_console.c
...
+#if defined(VIDEO_FB_16BPP_PIXEL_SWAP) || defined (CONFIG_VIDEO_JADEGDC)
---------------------------------------------------^^^ please remove space here.
...
diff --git a/drivers/video/jadegdc.c b/drivers/video/jadegdc.c new file mode 100755 index 0000000..55d0a96 --- /dev/null +++ b/drivers/video/jadegdc.c
...
+GraphicDevice jadegdc;
+void *video_hw_init(void) +{
- GraphicDevice *pGD = (GraphicDevice *) & jadegdc;
-----------------------------------------------^^^ you added a space here, please remove it. Also we should drop the cast, it is not needed.
...
if (NULL == (penv = getenv(varName)))if((i == 1) || (NULL == (penv = getenv("videomode"))))
please add a space between if and open parenthesis in the above line. Also please use ((penv = getenv("videomode") == NULL) in both lines above, i didn't catch it in previous comment, sorry. Thanks!
htp = var_mode[i].left_margin + var_mode[i].xres +var_mode[i].hsync_len + var_mode[i].right_margin;
please remove one tab in the above line, this line is over 80 chars.
...
vsw = var_mode[i].vsync_len;vtr = var_mode[i].upper_margin + var_mode[i].yres +var_mode[i].vsync_len + var_mode[i].lower_margin;
please remove one tab in the above line, this line is over 80 chars, too.
Thanks, Anatolij