Patrick Rudolph 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 27: Code-Review+1
(5 comments)
https://review.coreboot.org/#/c/30466/27/src/mainboard/sifive/hifive-unleash... File src/mainboard/sifive/hifive-unleashed/Makefile.inc:
https://review.coreboot.org/#/c/30466/27/src/mainboard/sifive/hifive-unleash... PS27, Line 24: ramstage-y += flash.c isn't it sufficient to compile it in the stage that calls boot_device_init()? I guess that is bootblock?
https://review.coreboot.org/#/c/30466/27/src/soc/sifive/fu540/include/soc/sp... File src/soc/sifive/fu540/include/soc/spi.h:
https://review.coreboot.org/#/c/30466/27/src/soc/sifive/fu540/include/soc/sp... PS27, Line 172: typedef volatile struct { I'd prefer not to have a typedef
https://review.coreboot.org/#/c/30466/27/src/soc/sifive/fu540/spi_flash.c File src/soc/sifive/fu540/spi_flash.c:
https://review.coreboot.org/#/c/30466/27/src/soc/sifive/fu540/spi_flash.c@39 PS27, Line 39: static int _initialize_spi_flash_mmap( move everything in here to initialize_spi_flash_mmap, there's no need for a separate function.
https://review.coreboot.org/#/c/30466/27/src/soc/sifive/fu540/spi_flash.c@46 PS27, Line 46: // Max desired SPI clock is 10MHz same as above
https://review.coreboot.org/#/c/30466/27/src/soc/sifive/fu540/spi_flash.c@61 PS27, Line 61: add comment and explain what this function does, as done on initialize_spi_flash_direct