Peter Marheine has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/52405 )
Change subject: realtek_mst: add support for a devpath option ......................................................................
realtek_mst: add support for a devpath option
As with the lspcon_i2c_spi programmer, it may be more convenient for callers to specify the I2C bus by device path rather than number. Add support for a 'devpath' option that is mutually exclusive with the 'bus' option and specifies how to communicate with the target.
TEST=-p realtek_mst_i2c_spi:devpath=/dev/i2c-8 --flash-size works, bus=8 continues to work. If both or neither are given, fails gracefully.
Change-Id: Ieea57b2445f3aca16c87d8f839dff131009531a0 --- M realtek_mst_i2c_spi.c 1 file changed, 49 insertions(+), 26 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/52405/1
diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index ae79fdd..1c35495 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -442,36 +442,51 @@ return ret; }
-static int get_params(int *i2c_bus, int *reset, int *enter_isp) +static int get_params(int *i2c_bus, char **i2c_devpath, int *reset, int *enter_isp) { - char *bus_str = NULL, *reset_str = NULL, *isp_str = NULL; - int ret = SPI_GENERIC_ERROR; + char *bus_str = NULL, *bus_path = NULL, *reset_str = NULL, *isp_str = NULL; + int ret = 0;
bus_str = extract_programmer_param("bus"); - if (bus_str) { - char *bus_suffix; - errno = 0; - int bus = (int)strtol(bus_str, &bus_suffix, 10); - if (errno != 0 || bus_str == bus_suffix) { - msg_perr("%s: Could not convert 'bus'.\n", __func__); - goto _get_params_failed; - } + bus_path = extract_programmer_param("devpath"); + if (bus_str == NULL && bus_path == NULL) { + msg_perr("%s: one of 'bus' or 'devpath' must be specified\n", __func__); + return SPI_GENERIC_ERROR; + } + if (bus_str != NULL && bus_path != NULL) { + msg_perr("%s: only one of 'bus' or 'devpath' may be specified\n", __func__); + free(bus_str); + free(bus_path); + return SPI_GENERIC_ERROR; + }
+ if (bus_str) { + // Up to three characters for '255', plus one byte to detect trailing garbage + // and a null terminator. Use a local buffer to avoid needing to remember to + // free bus_str later. + char bus_str_buf[5] = {0}; + strncpy(bus_str_buf, bus_str, sizeof(bus_str_buf) - 1); + free(bus_str); + + char *bus_suffix; + int bus = (int)strtol(bus_str_buf, &bus_suffix, 10); + if (bus_str_buf == bus_suffix) { + msg_perr("%s: Could not convert 'bus'.\n", __func__); + return SPI_GENERIC_ERROR; + } if (bus < 0 || bus > 255) { msg_perr("%s: Value for 'bus' is out of range(0-255).\n", __func__); - goto _get_params_failed; + return SPI_GENERIC_ERROR; }
if (strlen(bus_suffix) > 0) { msg_perr("%s: Garbage following 'bus' value.\n", __func__); - goto _get_params_failed; + return SPI_GENERIC_ERROR; }
- msg_pinfo("Using i2c bus %i.\n", bus); *i2c_bus = bus; - ret = 0; } else { - msg_perr("%s: Bus number not specified.\n", __func__); + *i2c_devpath = bus_path; }
reset_str = extract_programmer_param("reset-mcu"); @@ -502,32 +517,40 @@ *enter_isp = 1; /* Default behaviour is enter ISP on setup. */ free(isp_str);
-_get_params_failed: - if (bus_str) - free(bus_str); - + // If failing, free a devpath that was being returned + if (ret && *i2c_devpath != NULL) + free(*i2c_devpath); return ret; }
int realtek_mst_i2c_spi_init(void) { int ret = 0; - int i2c_bus = 0, reset = 0, enter_isp = 0; + int i2c_bus = -1, reset = 0, enter_isp = 0; + char *i2c_devpath = NULL;
- if (get_params(&i2c_bus, &reset, &enter_isp)) + if (get_params(&i2c_bus, &i2c_devpath, &reset, &enter_isp)) return SPI_GENERIC_ERROR;
- int fd = i2c_open(i2c_bus, REGISTER_ADDRESS, 0); + int fd; + // bus= and devpath= should be mutually exclusive, but defensively handle both here + if (i2c_bus != -1) { + fd = i2c_open(i2c_bus, REGISTER_ADDRESS, 0); + } + if (i2c_devpath != NULL) { + fd = i2c_open_path(i2c_devpath, REGISTER_ADDRESS, 0); + free(i2c_devpath); + } if (fd < 0) return fd;
if (enter_isp) { - ret |= realtek_mst_i2c_spi_enter_isp_mode(fd); + ret = realtek_mst_i2c_spi_enter_isp_mode(fd); if (ret) return ret; }
- ret |= realtek_mst_i2c_spi_toggle_gpio_88_strap(fd, true); + ret = realtek_mst_i2c_spi_toggle_gpio_88_strap(fd, true); if (ret) { msg_perr("Unable to toggle gpio 88 strap to True.\n"); return ret; @@ -541,7 +564,7 @@
data->fd = fd; data->reset = reset; - ret |= register_shutdown(realtek_mst_i2c_spi_shutdown, data); + ret = register_shutdown(realtek_mst_i2c_spi_shutdown, data);
spi_master_i2c_realtek_mst.data = data; ret |= register_spi_master(&spi_master_i2c_realtek_mst);
-
Peter Marheine (Code Review)