ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 29:
(3 comments)
https://review.coreboot.org/#/c/30466/29/src/mainboard/sifive/hifive-unleash... File src/mainboard/sifive/hifive-unleashed/flash.h:
https://review.coreboot.org/#/c/30466/29/src/mainboard/sifive/hifive-unleash... PS29, Line 4: * Copyright (C) 2018 HardenedLinux Per today's announcement about the change in how we do copyright notices, you can remove this copyright line.
https://review.coreboot.org/#/c/30466/29/src/mainboard/sifive/hifive-unleash... PS29, Line 16: #ifndef __HIFIVE_UNLEASHED_FLASH_H__ we've stopped guarding prototypes with #ifdef it seems so you can delete the preprocessor guards here. Once this is done, you will have a file with one prototype. Can it be put in some other file?
https://review.coreboot.org/#/c/30466/29/src/soc/sifive/fu540/spi_flash.c File src/soc/sifive/fu540/spi_flash.c:
https://review.coreboot.org/#/c/30466/29/src/soc/sifive/fu540/spi_flash.c@52 PS29, Line 52: spi_txrx(spictrl, command_enable);
There's no reason to increase code fragmentation. […]
Please follow Rudolph's suggestion. He is right. We've worked hard to avoid this kind of code fragmentation in coreboot, even at the cost of using "heavy" code. The cost is worth it.