Shiyu Sun has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46089 )
Change subject: realtek_mst_i2c_spi.c: Update wp disable function and add param ......................................................................
Patch Set 1:
(8 comments)
https://review.coreboot.org/c/flashrom/+/46089/1/realtek_mst_i2c_spi.c File realtek_mst_i2c_spi.c:
https://review.coreboot.org/c/flashrom/+/46089/1/realtek_mst_i2c_spi.c@a336 PS1, Line 336: ret = realtek_mst_i2c_spi_disable_protection(fd); : if (ret) : return ret;
This change should just generalise this functions implementation and not too much else. […]
Done
https://review.coreboot.org/c/flashrom/+/46089/1/realtek_mst_i2c_spi.c@149 PS1, Line 149: disable
this function shouldn't be just _disable_, it should toggle and be parametric with a bool to toggle […]
Done
https://review.coreboot.org/c/flashrom/+/46089/1/realtek_mst_i2c_spi.c@154 PS1, Line 154: // Configure Pin to Push-Pull GPIO
/* read 0x4F into val. […]
Done
https://review.coreboot.org/c/flashrom/+/46089/1/realtek_mst_i2c_spi.c@155 PS1, Line 155: ret |= realtek_mst_i2c_spi_write_register(fd, 0xF4, 0x9F); : ret |= realtek_mst_i2c_spi_write_register(fd, 0xF5, 0x10); : ret |= realtek_mst_i2c_spi_write_register(fd, 0xF4, 0x4F); : : ret |= realtek_mst_i2c_spi_read_register(fd, 0xF5, &val); :
/* write 0x4F[3:0] = b0001 */
Done
https://review.coreboot.org/c/flashrom/+/46089/1/realtek_mst_i2c_spi.c@167 PS1, Line 167: // Set Pin Value to High
/* read 0x3F into val. […]
Done
https://review.coreboot.org/c/flashrom/+/46089/1/realtek_mst_i2c_spi.c@174 PS1, Line 174:
/* write 0x3F[1:0] = b|toggle| */
Done
https://review.coreboot.org/c/flashrom/+/46089/1/realtek_mst_i2c_spi.c@176 PS1, Line 176: 0x3F
Actually in retrospect it looks like #define GPIO_CONFIG 0x4F and #define GPIO_DATA 0x3F
Done
https://review.coreboot.org/c/flashrom/+/46089/1/realtek_mst_i2c_spi.c@495 PS1, Line 495: if (wp_disable) { : ret |= realtek_mst_i2c_spi_disable_hw_protection(fd); : if (ret != 0) { : msg_perr("%s: failed to disable the write protection.\n", __func__); : return ret; : } : } :
I don't think wp should be toggled via this mechanism by using a bespoke spi master parameter. […]
As discussed we will revisit this on the following cls. Just removed this for now.