Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33055 )
Change subject: soc/sifive/fu540: add code for spi and map flash to memory spaces ......................................................................
Patch Set 2:
(5 comments)
Thanks for working on it. It would be nice if you could add the fu540 SPI interface in one commit and the ISSI flash in a seperate one, to make it easier to review.
https://review.coreboot.org/#/c/33055/2/src/drivers/spi/issi.c File src/drivers/spi/issi.c:
https://review.coreboot.org/#/c/33055/2/src/drivers/spi/issi.c@3 PS2, Line 3: * missing copyright
https://review.coreboot.org/#/c/33055/1/src/soc/sifive/fu540/spi.c File src/soc/sifive/fu540/spi.c:
https://review.coreboot.org/#/c/33055/1/src/soc/sifive/fu540/spi.c@68 PS1, Line 68: if (bytesout) { don't you need to transmit something in order to receive something?
https://review.coreboot.org/#/c/33055/1/src/soc/sifive/fu540/spi.c@103 PS1, Line 103: .xfer = spi_xfer_, spi_xfer_vector is commonly used for non duplex transfers
https://review.coreboot.org/#/c/33055/1/src/soc/sifive/fu540/spi.c@110 PS1, Line 110: .bus_end = 0, there are 3 SPI controllers, thus bus_end = 2
https://review.coreboot.org/#/c/33055/1/src/soc/sifive/fu540/spi.c@137 PS1, Line 137: spictrl->sckdiv = spi_min_clk_divisor(clock_get_tlclk_khz(), use read64()/write64() for volatile memory access