Xiang Wang 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 10:
(6 comments)
https://review.coreboot.org/#/c/33055/9/src/mainboard/sifive/hifive-unleashe... File src/mainboard/sifive/hifive-unleashed/media.c:
https://review.coreboot.org/#/c/33055/9/src/mainboard/sifive/hifive-unleashe... PS9, Line 40: void boot_device_init(void)
why is it board specific?
SPI flash can be linked to QSPI0/QSPI1/QSPI2, which is related to the connection of the specific motherboard.
https://review.coreboot.org/#/c/33055/9/src/mainboard/sifive/hifive-unleashe... PS9, Line 42: const static uint8_t reset_en = 0x66;
why? it's already done for ISSI if you call spi_flash_generic_probe().
There is no spi_flash_probe used here, so needs to add code to reset spi flash
https://review.coreboot.org/#/c/33055/9/src/mainboard/sifive/hifive-unleashe... PS9, Line 50: spi_xfer(&slave, &reset_en, 0, NULL, 0);
1
Done
https://review.coreboot.org/#/c/33055/9/src/mainboard/sifive/hifive-unleashe... PS9, Line 51: spi_xfer(&slave, &reset, 0, NULL, 0);
1
Done
https://review.coreboot.org/#/c/33055/9/src/soc/sifive/fu540/spi.c File src/soc/sifive/fu540/spi.c:
https://review.coreboot.org/#/c/33055/9/src/soc/sifive/fu540/spi.c@56 PS9, Line 56: static uint8_t spi_rx(volatile struct spi_ctrl *spictrl)
that doesn't work for me. The method never returns. […]
It separate call requires SPI work in DPI / QPI mode
https://review.coreboot.org/#/c/33055/9/src/soc/sifive/fu540/spi.c@71 PS9, Line 71: if (fmt.proto == FU540_SPI_PROTO_S) {
why do you check for active the bit mode?
The SPI is full-duplex and needs to send data to trigger receive. DPI/QPI is half-duplex and does not need to send data to trigger receive