build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30901 )
Change subject: soc/sifive/fu540: add support boot from sdcard ......................................................................
Patch Set 1:
(40 comments)
https://review.coreboot.org/#/c/30901/1/src/mainboard/sifive/hifive-unleashe... File src/mainboard/sifive/hifive-unleashed/media.c:
https://review.coreboot.org/#/c/30901/1/src/mainboard/sifive/hifive-unleashe... PS1, Line 43: if (sd_copy((spi_ctrl*)FU540_QSPI2, tmp, lba, 1) != 0) { "(foo*)" should be "(foo *)"
https://review.coreboot.org/#/c/30901/1/src/mainboard/sifive/hifive-unleashe... PS1, Line 43: if (sd_copy((spi_ctrl*)FU540_QSPI2, tmp, lba, 1) != 0) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/30901/1/src/mainboard/sifive/hifive-unleashe... PS1, Line 57: if (sd_copy((spi_ctrl*)FU540_QSPI2, tmp, lba, 1) != 0) { "(foo*)" should be "(foo *)"
https://review.coreboot.org/#/c/30901/1/src/mainboard/sifive/hifive-unleashe... PS1, Line 57: if (sd_copy((spi_ctrl*)FU540_QSPI2, tmp, lba, 1) != 0) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/30901/1/src/mainboard/sifive/hifive-unleashe... PS1, Line 64: size_t size = (last_lba - has_end) - (first_lba + has_begin) + 1; line over 80 characters
https://review.coreboot.org/#/c/30901/1/src/mainboard/sifive/hifive-unleashe... PS1, Line 66: if (sd_copy((spi_ctrl*)FU540_QSPI2, dest + o, lba, size) != 0) { line over 80 characters
https://review.coreboot.org/#/c/30901/1/src/mainboard/sifive/hifive-unleashe... PS1, Line 66: if (sd_copy((spi_ctrl*)FU540_QSPI2, dest + o, lba, size) != 0) { "(foo*)" should be "(foo *)"
https://review.coreboot.org/#/c/30901/1/src/mainboard/sifive/hifive-unleashe... PS1, Line 66: if (sd_copy((spi_ctrl*)FU540_QSPI2, dest + o, lba, size) != 0) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/30901/1/src/mainboard/sifive/hifive-unleashe... PS1, Line 75: if (sd_copy((spi_ctrl*)FU540_QSPI2, tmp, lba, 1) != 0) { "(foo*)" should be "(foo *)"
https://review.coreboot.org/#/c/30901/1/src/mainboard/sifive/hifive-unleashe... PS1, Line 75: if (sd_copy((spi_ctrl*)FU540_QSPI2, tmp, lba, 1) != 0) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/30901/1/src/mainboard/sifive/hifive-unleashe... PS1, Line 97: switch(read32((uint32_t *)FU540_MSEL)) { space required before the open parenthesis '('
https://review.coreboot.org/#/c/30901/1/src/mainboard/sifive/hifive-unleashe... PS1, Line 108: void boot_device_init() Bad function definition - void boot_device_init() should probably be void boot_device_init(void)
https://review.coreboot.org/#/c/30901/1/src/mainboard/sifive/hifive-unleashe... PS1, Line 110: switch(read32((uint32_t *)FU540_MSEL)) { space required before the open parenthesis '('
https://review.coreboot.org/#/c/30901/1/src/mainboard/sifive/hifive-unleashe... PS1, Line 117: sd_init((spi_ctrl*)FU540_QSPI2, clock_get_tlclk_khz()); "(foo*)" should be "(foo *)"
https://review.coreboot.org/#/c/30901/1/src/mainboard/sifive/hifive-unleashe... PS1, Line 118: mmap_helper_device_init(&sd_mdev, _cbfs_cache, _cbfs_cache_size); line over 80 characters
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/include/soc/sd.... File src/soc/sifive/fu540/include/soc/sd.h:
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/include/soc/sd.... PS1, Line 35: int sd_init(spi_ctrl* spi, unsigned int input_clk_hz); "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/include/soc/sd.... PS1, Line 36: int sd_copy(spi_ctrl* spi, void* dst, uint32_t src_lba, size_t size); "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/include/soc/sd.... PS1, Line 36: int sd_copy(spi_ctrl* spi, void* dst, uint32_t src_lba, size_t size); "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c File src/soc/sifive/fu540/sd.c:
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@66 PS1, Line 66: static inline uint8_t sd_dummy(spi_ctrl* spi) "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@72 PS1, Line 72: static int sd_cmd(spi_ctrl* spi, uint8_t cmd, uint32_t arg, uint8_t crc) "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@89 PS1, Line 89: if (!(r & 0x80)) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@97 PS1, Line 97: static inline void sd_cmd_end(spi_ctrl* spi) "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@104 PS1, Line 104: static void sd_poweron(spi_ctrl* spi, unsigned int input_clk_khz) "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@121 PS1, Line 121: // SD CMD pin is the same pin as SPI DI, so CMD = 0xff means assert DI high line over 80 characters
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@133 PS1, Line 133: static int sd_cmd0(spi_ctrl* spi) "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@136 PS1, Line 136: rc = (sd_cmd(spi, SD_CMD(SD_CMD_GO_IDLE_STATE), 0, 0x95) != SD_RESPONSE_IDLE); line over 80 characters
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@145 PS1, Line 145: static int sd_cmd8(spi_ctrl* spi) "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@150 PS1, Line 150: rc = (sd_cmd(spi, SD_CMD(SD_CMD_SEND_IF_COND), 0x000001AA, 0x87) != SD_RESPONSE_IDLE); line over 80 characters
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@163 PS1, Line 163: static void sd_cmd55(spi_ctrl* spi) "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@173 PS1, Line 173: static int sd_acmd41(spi_ctrl* spi) "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@178 PS1, Line 178: r = sd_cmd(spi, SD_CMD(SD_CMD_APP_SEND_OP_COND), 0x40000000, 0x77); /* HCS = 1 */ line over 80 characters
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@189 PS1, Line 189: static int sd_cmd58(spi_ctrl* spi) "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@192 PS1, Line 192: // to issue this command if we only support SD cards that support SDHC mode. line over 80 characters
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@208 PS1, Line 208: static int sd_cmd16(spi_ctrl* spi) "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@239 PS1, Line 239: //int sd_init(spi_ctrl* spi, unsigned int input_clk_khz, int skip_sd_init_commands) line over 80 characters
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@240 PS1, Line 240: int sd_init(spi_ctrl* spi, unsigned int input_clk_khz) "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@260 PS1, Line 260: int sd_copy(spi_ctrl* spi, void* dst, uint32_t src_lba, size_t size) "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@260 PS1, Line 260: int sd_copy(spi_ctrl* spi, void* dst, uint32_t src_lba, size_t size) "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@273 PS1, Line 273: if (sd_cmd(spi, SD_CMD(SD_CMD_READ_BLOCK_MULTIPLE), src_lba, c) != 0x00) { line over 80 characters
https://review.coreboot.org/#/c/30901/1/src/soc/sifive/fu540/sd.c@283 PS1, Line 283: while (sd_dummy(spi) != SD_DATA_TOKEN); trailing statements should be on next line