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 9:
(6 comments)
Tested on the unleashed board with CONFIG_COMMON_CBFS_SPI_WRAPPER=y
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?
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().
https://review.coreboot.org/#/c/33055/9/src/mainboard/sifive/hifive-unleashe... PS9, Line 50: spi_xfer(&slave, &reset_en, 0, NULL, 0); 1
https://review.coreboot.org/#/c/33055/9/src/mainboard/sifive/hifive-unleashe... PS9, Line 51: spi_xfer(&slave, &reset, 0, NULL, 0); 1
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. Shouldn't you transmit receive at the same time?
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?