Youness Alaoui has posted comments on this change. ( https://review.coreboot.org/19849 )
Change subject: console/flashsconsole: Add spi flash console for debugging ......................................................................
Patch Set 7:
(3 comments)
https://review.coreboot.org/#/c/19849/7/src/drivers/spi/flashconsole.c File src/drivers/spi/flashconsole.c:
PS7, Line 23: <assert.h>
Did we assert anywhere?
no, but I needed it for printk definition. Realizing now it should have been <console/console.h> which assert.h includes. Fixed.
PS7, Line 26: READ_BUFFER_SIZE 0x1000
This is 4KiB of stack. That's probably even more unsafe. Why not just make
Yes, it's to limit the number of I/O. Maybe I can use the boot_device_ro() which I believe has the entire flash in mmap-ed memory ? That way, no need for actual I/O and we can keep a small buffer size.
Line 79: rdev_readat(rdev, buffer, offset, len);
If the read fails you will just keep "reading" all the way through to the e
Nope (would have said it in a comment if it was the intent), it was a lucky oversight. I think I'll keep it as is and add a comment clarifying it. Good catch.
-
Youness Alaoui (Code Review)