View Change

2 comments:

• File realtek_mst_i2c_spi.c:

  • Patch Set #6, 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?

  • Patch Set #6, 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).

To view, visit change 46089. To unsubscribe, or for help writing mail filters, visit settings.