Jonathan Neuschäfer 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 13:
(3 comments)
https://review.coreboot.org/#/c/30466/13/src/soc/sifive/fu540/include/soc/sp... File src/soc/sifive/fu540/include/soc/spi.h:
https://review.coreboot.org/#/c/30466/13/src/soc/sifive/fu540/include/soc/sp... PS13, Line 20: #define _ASSERT_SIZEOF(type, size) _Static_assert( \ : sizeof(type) == (size), \ : #type " must be " #size " bytes wide") This macro should ideally be in a non RISC-V-specific directory
https://review.coreboot.org/#/c/30466/13/src/soc/sifive/fu540/include/soc/sp... PS13, Line 217: // Inlining header functions in C : // https://stackoverflow.com/a/23699777/7433423 This stackoverflow link seems unnecessary, because inlining is a common C feature.
https://review.coreboot.org/#/c/30466/13/src/soc/sifive/fu540/include/soc/sp... File src/soc/sifive/fu540/include/soc/spi_flash.h:
https://review.coreboot.org/#/c/30466/13/src/soc/sifive/fu540/include/soc/sp... PS13, Line 29: */ That's a lot of functions for a header. Do they have to be in a header? Ideally, I'd prefer to have them in a .c file