[flashrom-gerrit] Change in flashrom[master]: realtek_mst_i2c_spi.c: Update write protection toggle function

[Angel Pons (Code Review)]

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:

(2 comments)

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
> I suggested to Shiyu to use this name instead Angel as gpio88 need not be connected to the wp pin of […]

I see. Maybe add a comment for clarification?

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);
> I forgot to add, the func is almost correct but would be: […]

Adding the last write inside the function makes it a write-function. For reads, the second operation on 0xF5 is a read. So there would be two functions:

static int realtek_mst_i2c_spi_read_indexed_register(uint16_t address, uint8_t *val)
 {
    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);
    ret |= realtek_mst_i2c_spi_read_register(fd, 0xF5, &val);

return ret;
 }

static int realtek_mst_i2c_spi_write_indexed_register(uint16_t address, uint8_t val)
 {
    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);
    ret |= realtek_mst_i2c_spi_write_register(fd, 0xF5, val);

return ret;
 }

I'm fine with either having a single `select indexed register` function (my suggestion) or having separate `read indexed register` and `write indexed register` functions (Edward's suggestion).