Attention is currently required from: Thomas Heijligen, qianfan, Anastasia Klimchuk, Nicholas Chin.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70573 )
Change subject: ch347_spi.c: Add initial support for the WCH CH347 ......................................................................
Patch Set 3: Code-Review+1
(4 comments)
File ch347_spi.c:
https://review.coreboot.org/c/flashrom/+/70573/comment/d5b9cafa_2aa6e65b PS3, Line 149: command_buf[0] = CH347_CMD_SPI_IN; : command_buf[1] = 4; : command_buf[2] = 0; : command_buf[3] = readcnt & 0xFF; : command_buf[4] = (readcnt & 0xFF00) >> 8; : command_buf[5] = (readcnt & 0xFF0000) >> 16; : command_buf[6] = (readcnt & 0xFF000000) >> 24; Use an initialiser as done in `ch347_set_cs()`?
https://review.coreboot.org/c/flashrom/+/70573/comment/a002aee5_c626bbdf PS3, Line 223: -3 nit: spacing here and in the line below
https://review.coreboot.org/c/flashrom/+/70573/comment/d7f0def5_20be6950 PS3, Line 222: buff[0] = CH347_CMD_SPI_CFG; : buff[1] = (sizeof(buff)-3) & 0xFF; : buff[2] = ((sizeof(buff)-3) & 0xFF00) >> 8; : /* Not sure what these two bytes do, but the vendor : * drivers seem to unconditionally set these values : */ : buff[5] = 4; : buff[6] = 1; : /* Clock polarity: bit 1 */ : buff[9] = 0; : /* Clock phase: bit 0 */ : buff[11] = 0; : /* Another mystery byte */ : buff[14] = 2; : /* Clock divisor: bits 5:3 */ : buff[15] = (divisor & 0x7) << 3; : /* Bit order: bit 7, 0=MSB */ : buff[17] = 0; : /* Yet another mystery byte */ : buff[19] = 7; : /* CS polarity: bit 7 CS2, bit 6 CS1. 0 = active low */ : buff[24] = 0; Use an initialiser as done in `ch347_set_cs()`?
https://review.coreboot.org/c/flashrom/+/70573/comment/634e2f29_91d75e48 PS3, Line 261: 64 * 1024 Where does this come from?
-
Angel Pons (Code Review)