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 11:
(4 comments)
https://review.coreboot.org/c/coreboot/+/33055/9/src/mainboard/sifive/hifive... File src/mainboard/sifive/hifive-unleashed/media.c:
https://review.coreboot.org/c/coreboot/+/33055/9/src/mainboard/sifive/hifive... PS9, Line 40: void boot_device_init(void)
but in the end only MSEL decides which boot media to use. […]
MSEL is only used to determine where ZSBL and FSBL (coreboot) are. How to use MSEL is then determined by coreboot, which is related to the motherboard.
https://review.coreboot.org/c/coreboot/+/33055/9/src/mainboard/sifive/hifive... PS9, Line 42: const static uint8_t reset_en = 0x66;
Any reason not to call the generic code, i.e. […]
This is not to initialize the spi flash, here is to tell the fu540 spi controller how to read the flash. spi_flash_read/spi_flash_write will not be called after
https://review.coreboot.org/c/coreboot/+/33055/9/src/soc/sifive/fu540/spi.c File src/soc/sifive/fu540/spi.c:
https://review.coreboot.org/c/coreboot/+/33055/9/src/soc/sifive/fu540/spi.c@... PS9, Line 71: if (fmt.proto == FU540_SPI_PROTO_S) {
Please add this information as inline comment, as it's not clear on the first sight.
Done
https://review.coreboot.org/c/coreboot/+/33055/9/src/soc/sifive/fu540/spi.c@... PS9, Line 78: spi_tx(spictrl, out);
- you can't use half duplex function here, as hardware operates in full duplex mode. […]
I tried to look at the spi-generic.h file, both xfer() and xfer_vector() work in full duplex. We can determine whether can perform half-duplex operation by check dout&&din