Peter Marheine has uploaded this change for review.

View Change

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);

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

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ieea57b2445f3aca16c87d8f839dff131009531a0
Gerrit-Change-Number: 52405
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Marheine <pmarheine@chromium.org>
Gerrit-MessageType: newchange