[U-Boot] [PATCH] VGA text console support

Wolfgang Denk wd at denx.de
Sun Apr 7 19:07:31 CEST 2013


Dear Vladimir 'φ-coder/phcoder' Serbinenko,

I wrote:

> I cannot comment on the actual technical content of your patches, but
> here are a few formal comments; I highly recommend to study
> http://www.denx.de/wiki/U-Boot/Patches
...

Oops.  Sent to quickly.

Please ALWAYS make sure to run your patches through checkpatch - ther
are a number of issues with your coding style:

ERROR: that open brace { should be on the previous line
#147: FILE: drivers/video/cfb_console.c:453:
+	if (vga_text)
+	{

ERROR: that open brace { should be on the previous line
#151: FILE: drivers/video/cfb_console.c:457:
+		while (count--)
+		{

WARNING: line over 80 characters
#153: FILE: drivers/video/cfb_console.c:459:
+			((uint16_t *)video_fb_address) [yy * 80 + xx] = 0x700 | *s++;

ERROR: space prohibited before open square bracket '['
#153: FILE: drivers/video/cfb_console.c:459:
+			((uint16_t *)video_fb_address) [yy * 80 + xx] = 0x700 | *s++;

WARNING: space prohibited between function name and open parenthesis '('
#167: FILE: drivers/video/cfb_console.c:660:
+cr_write (uint8_t val, uint8_t addr)

WARNING: please, no spaces at the start of a line
#169: FILE: drivers/video/cfb_console.c:662:
+  outb (addr, 0x3d4);$

WARNING: space prohibited between function name and open parenthesis '('
#169: FILE: drivers/video/cfb_console.c:662:
+  outb (addr, 0x3d4);

WARNING: please, no spaces at the start of a line
#170: FILE: drivers/video/cfb_console.c:663:
+  outb (val, 0x3d5);$

WARNING: space prohibited between function name and open parenthesis '('
#170: FILE: drivers/video/cfb_console.c:663:
+  outb (val, 0x3d5);

WARNING: space prohibited between function name and open parenthesis '('
#174: FILE: drivers/video/cfb_console.c:667:
+cr_read (uint8_t addr)

WARNING: please, no spaces at the start of a line
#176: FILE: drivers/video/cfb_console.c:669:
+  outb (addr, 0x3d4);$

WARNING: space prohibited between function name and open parenthesis '('
#176: FILE: drivers/video/cfb_console.c:669:
+  outb (addr, 0x3d4);

WARNING: please, no spaces at the start of a line
#177: FILE: drivers/video/cfb_console.c:670:
+  return inb (0x3d5);$

WARNING: space prohibited between function name and open parenthesis '('
#177: FILE: drivers/video/cfb_console.c:670:
+  return inb (0x3d5);

ERROR: that open brace { should be on the previous line
#187: FILE: drivers/video/cfb_console.c:696:
+	if (vga_text)
+	{

WARNING: space prohibited between function name and open parenthesis '('
#191: FILE: drivers/video/cfb_console.c:700:
+		old = cr_read (0x0a);

WARNING: space prohibited between function name and open parenthesis '('
#193: FILE: drivers/video/cfb_console.c:702:
+			cr_write (old & ~0x20, 0x0a);

WARNING: space prohibited between function name and open parenthesis '('
#195: FILE: drivers/video/cfb_console.c:704:
+			cr_write (old | 0x20, 0x0a);

WARNING: space prohibited between function name and open parenthesis '('
#197: FILE: drivers/video/cfb_console.c:706:
+		cr_write (pos >> 8, 0x0e);

WARNING: space prohibited between function name and open parenthesis '('
#198: FILE: drivers/video/cfb_console.c:707:
+		cr_write (pos & 0xFF, 0x0f);

ERROR: that open brace { should be on the previous line
#210: FILE: drivers/video/cfb_console.c:749:
+	if (vga_text)
+	{

ERROR: that open brace { should be on the previous line
#215: FILE: drivers/video/cfb_console.c:754:
+		for (; begin < end; begin++)
+		{

WARNING: line over 80 characters
#217: FILE: drivers/video/cfb_console.c:756:
+			((uint16_t *)video_fb_address) [line * 80 + begin] = 0x700 | ' ';

ERROR: space prohibited before open square bracket '['
#217: FILE: drivers/video/cfb_console.c:756:
+			((uint16_t *)video_fb_address) [line * 80 + begin] = 0x700 | ' ';

ERROR: that open brace { should be on the previous line
#229: FILE: drivers/video/cfb_console.c:799:
+	if (vga_text)
+	{

WARNING: line over 80 characters
#232: FILE: drivers/video/cfb_console.c:802:
+		memcpyl(video_fb_address, ((int32_t *)video_fb_address) + 80 / 2,

WARNING: line over 80 characters
#235: FILE: drivers/video/cfb_console.c:805:
+			((uint16_t *)video_fb_address) [24 * 80 + i] = 0x700 | ' ';

ERROR: space prohibited before open square bracket '['
#235: FILE: drivers/video/cfb_console.c:805:
+			((uint16_t *)video_fb_address) [24 * 80 + i] = 0x700 | ' ';

ERROR: that open brace { should be on the previous line
#245: FILE: drivers/video/cfb_console.c:845:
+	if (vga_text)
+	{

ERROR: space prohibited before open square bracket '['
#249: FILE: drivers/video/cfb_console.c:849:
+			((uint16_t *)video_fb_address) [i] = 0x700 | ' ';

ERROR: that open brace { should be on the previous line
#280: FILE: drivers/video/cfb_console.c:2193:
 	if (pGD == NULL)
+	{

WARNING: Avoid CamelCase: <pGD->frameAdrs>
#295: FILE: drivers/video/cfb_console.c:2207:
+	video_fb_address = (void *) pGD->frameAdrs;

WARNING: Avoid CamelCase: <pGD->winSizeY>
#296: FILE: drivers/video/cfb_console.c:2208:
+	VIDEO_VISIBLE_ROWS = pGD->winSizeY;

WARNING: Avoid CamelCase: <pGD->winSizeX>
#297: FILE: drivers/video/cfb_console.c:2209:
+	VIDEO_VISIBLE_COLS = pGD->winSizeX;

WARNING: Avoid CamelCase: <pGD->gdfBytesPP>
#298: FILE: drivers/video/cfb_console.c:2210:
+	VIDEO_PIXEL_SIZE = (pGD->gdfBytesPP);

ERROR: that open brace { should be on the previous line
#309: FILE: drivers/video/cfb_console.c:2392:
+	if (vga_text)
+	{

ERROR: space prohibited before open square bracket '['
#313: FILE: drivers/video/cfb_console.c:2396:
+			((uint16_t *)video_fb_address) [i] = 0x700 | ' ';

ERROR: Missing Signed-off-by: line(s)

total: 15 errors, 23 warnings, 188 lines checked

/tmp/patch has style problems, please review.


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
It took him several minutes to understand any new idea  put  to  him,
and  this is a very valuable trait in a leader, because anything any-
one is still trying to explain to you after two minutes  is  probably
important  and anything they give up after a mere minute or so is al-
most certainly something they shouldn't have been bothering you  with
in the first place.                   - Terry Pratchett, _Reaper Man_


More information about the U-Boot mailing list