Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46089 )
Change subject: realtek_mst_i2c_spi.c: Update write protection toggle function ......................................................................
Patch Set 6:
(5 comments)
https://review.coreboot.org/c/flashrom/+/46089/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/flashrom/+/46089/6//COMMIT_MSG@11 PS6, Line 11: protectoin protection
https://review.coreboot.org/c/flashrom/+/46089/6//COMMIT_MSG@11 PS6, Line 11: The process would be Format this as a list?
The process would be: 1. write to GPIO ... 2. write to GPIO ...
https://review.coreboot.org/c/flashrom/+/46089/6/realtek_mst_i2c_spi.c File realtek_mst_i2c_spi.c:
https://review.coreboot.org/c/flashrom/+/46089/6/realtek_mst_i2c_spi.c@151 PS6, Line 151: realtek_mst_i2c_spi_toggle_gpio_88_strap realtek_mst_i2c_spi_toggle_write_protect_gpio
https://review.coreboot.org/c/flashrom/+/46089/6/realtek_mst_i2c_spi.c@157 PS6, Line 157: ret |= realtek_mst_i2c_spi_write_register(fd, 0xF4, 0x9F); : ret |= realtek_mst_i2c_spi_write_register(fd, 0xF5, GPIO_CONFIG_ADDRESS >> 8); : ret |= realtek_mst_i2c_spi_write_register(fd, 0xF4, GPIO_CONFIG_ADDRESS & 0xFF); Maybe turn these groups of three register writes into an auxiliary function?
static int realtek_mst_i2c_spi_select_indexed_register(uint16_t address) { int ret = 0;
ret |= realtek_mst_i2c_spi_write_register(fd, 0xF4, 0x9F); ret |= realtek_mst_i2c_spi_write_register(fd, 0xF5, address >> 8); ret |= realtek_mst_i2c_spi_write_register(fd, 0xF4, address & 0xFF);
return ret; }
Feel free to rename it for accuracy. It's not specific to GPIO, since `realtek_mst_i2c_spi_enter_isp_mode` also does the same thing with another register. Feel free to do it in a follow-up.
https://review.coreboot.org/c/flashrom/+/46089/6/realtek_mst_i2c_spi.c@182 PS6, Line 182: (toggle ? 0x01 : 0x00) This can be simplified as follows:
!!toggle