[M] Change in coreboot[master]: drivers/pc80/vga: Add API to write multi-line video message
Attention is currently required from: Tarun Tuli, Jeremy Compostella, Kapil Porwal, Angel Pons.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71265 )
Change subject: drivers/pc80/vga: Add API to write multi-line video message ......................................................................
Patch Set 6:
(5 comments)
File src/drivers/pc80/vga/vga.c:
https://review.coreboot.org/c/coreboot/+/71265/comment/470de0be_ff561be7 PS3, Line 303: memcpy(str, string, strlen(string));
I dont think we should attempt to handle line wrapping and leave it to the creation of the string having the tokens instead.
@Kapil, marking this resolve now, we shall revive it later if required. Hope you are good with this. (feel free to open this comment if you think otherwise)
File src/drivers/pc80/vga/vga.c:
https://review.coreboot.org/c/coreboot/+/71265/comment/9ef60493_3f3e557e PS6, Line 268: VGA_COLUMNS
`VGA_COLUMNS - offset`?
Ack
File src/include/pc80/vga.h:
https://review.coreboot.org/c/coreboot/+/71265/comment/0615fab3_7b0b4f01 PS6, Line 11: #define VGA_TEXT_HORIZONTAL_TOP 0
are these used anywhere?
kind of helper macro where caller of vga_write_text() can pass this macro as argument 1 (for specifying the line as VGA_LINES / 2) to specify the starting offset.
https://review.coreboot.org/c/coreboot/+/71265/comment/949b825f_004fbb86 PS6, Line 12: #define VGA_TEXT_HORIZONTAL_MIDDLE (VGA_LINES / 2)
are these used anywhere?
same as above. Are you suggesting to drop this and leave it up to the caller of vga_write_text()?
https://review.coreboot.org/c/coreboot/+/71265/comment/c76ec754_ce435a61 PS6, Line 33: * vga_write_text() function can print the video message as per give alignment
nit- suggest something like: […]
Done
-
Subrata Banik (Code Review)