Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35118 )
Change subject: drivers/spi: add drivers for sdcard mounted on the spi bus ......................................................................
Patch Set 4: Code-Review+1
(5 comments)
https://review.coreboot.org/c/coreboot/+/35118/4/src/drivers/spi/spi_sdcard.... File src/drivers/spi/spi_sdcard.c:
https://review.coreboot.org/c/coreboot/+/35118/4/src/drivers/spi/spi_sdcard.... PS4, Line 23: #ifdef SPI_SDCARD_DEBUG was it copied from somewhere?
https://review.coreboot.org/c/coreboot/+/35118/4/src/drivers/spi/spi_sdcard.... PS4, Line 179: static uint16_t crc16_byte(uint16_t prev, uint8_t in) that should propably go to a common place.
https://review.coreboot.org/c/coreboot/+/35118/4/src/drivers/spi/spi_sdcard.... PS4, Line 225: case 0: why are those magic values? Can't you use the defines from above?
https://review.coreboot.org/c/coreboot/+/35118/4/src/include/spi_sdcard.h File src/include/spi_sdcard.h:
https://review.coreboot.org/c/coreboot/+/35118/4/src/include/spi_sdcard.h@25 PS4, Line 25: unsigned int bus, can those arguments be made const?
https://review.coreboot.org/c/coreboot/+/35118/4/src/include/spi_sdcard.h@52 PS4, Line 52: size_t spi_sdcard_size(const struct spi_sdcard *card); what unit does it return? bytes?