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 2:
(3 comments)
https://review.coreboot.org/#/c/19849/2/src/console/Kconfig File src/console/Kconfig:
Line 258: region in the CBFS.
What do you mean with "custom fmd"? Just integrate this in the default FMAP
This was discussed on IRC with Aaron. Adding it to the default FMAP would make everyone have a CONSOLE area (with size 0 if disabled), which is not practical for such a pre-bootable-board-porting debugging feature. I think anyone wants to use that feature, they can set a custom fmap, or use the cbfs feature instead if they don't want to bother.
IRC log for posterity :
May 24 14:17:47 <KaKaRoTo> adurbin, well, I figured if I added it, it would have a size of zero if flashconsole is disabled, but the area would still be present May 24 14:19:01 <KaKaRoTo> unless I replace '##FMAP_SIZE##" by "0x100\n\tCONSOLE ...." so the default-x86.fmd file isn't modified, but that's hacky. other possibility is having one default fmd with and one without the console, but I don't like the redundancy May 24 14:26:03 <adurbin> KaKaRoTo: i would just leave it alone and just test for it dynamically first in the driver. it gets complicated pretty quickly since you need to allocate your area with proper alignment. our tooling for doing that sorta thing automatically in fmap is non-existent May 24 14:26:42 <KaKaRoTo> adurbin, right, didn't think about alignment (although not needed anyways since I don't do any erase operations anymore) May 24 14:27:05 <adurbin> ahh. well, if there are no erase paths then it doesn't matter as much
https://review.coreboot.org/#/c/19849/9/src/drivers/spi/flashconsole.c File src/drivers/spi/flashconsole.c:
Line 93: car_set_var(g_rdev, rdev);
Here as well (since due to the way the console fills up and has to be manua
The FMAP and CBFS APIs already do prints, such as : "FMAP: Found "FLASH" version 1.1 at e00000." and "FMAP: area %s found @ %x (%d bytes)" same with CBFS APIs. Also, those lines will not appear because the console is not yet initialized. They would only appear in cbmem, and flashconsole should not be used if cbmem is available (in which case, they are helpful to tell us we forgot to disable the option).
Line 167
nit: just pull this into _tx_flush() so you don't need to have it twice?
Good idea. I hadn't noticed that all the 'risky' code was in tx_flush now.
-
Youness Alaoui (Code Review)