Attention is currently required from: Peter Marheine. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52405 )
Change subject: realtek_mst: add support for a devpath option ......................................................................
Patch Set 4:
(3 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/52405/comment/8ab41a93_751e78a2 PS4, Line 9: As with the lspcon_i2c_spi programmer I'd suggest looking into moving the I2C bus handling code to a common place, i2c_helper_linux.c for example?
This would be done in two commits: 1. Move common I2C stuff from lspcon_i2c_spi into i2c_helper_linux 2. Adapt realtek_mst_i2c_spi to use the common I2C stuff
File realtek_mst_i2c_spi.c:
https://review.coreboot.org/c/flashrom/+/52405/comment/3de43b89_e56d669a PS4, Line 466: // free bus_str later. nit: For consistency with existing comments, I'd use C-style
https://review.coreboot.org/c/flashrom/+/52405/comment/e0655a3a_989fdf76 PS4, Line 536: // bus= and devpath= should be mutually exclusive, but defensively handle both here This would already be guaranteed by the `get_params` function. If you're worried about leaking `i2c_devpath`, one could do:
if (i2c_devpath != NULL) { fd = i2c_open_path(i2c_devpath, REGISTER_ADDRESS, 0); free(i2c_devpath); } else if (i2c_bus != -1) { fd = i2c_open(i2c_bus, REGISTER_ADDRESS, 0); }