Angel Pons has submitted this change. ( https://review.coreboot.org/c/flashrom/+/51967 )
Change subject: lspcon_i2c_spi: support a devpath option ......................................................................
lspcon_i2c_spi: support a devpath option
Some callers may find it easier to provide the path to an I2C device at which to communicate with the device, rather than specify the bus number- doing so might involve trying to parse a path to extract the number only for flashrom to do the reverse, which is error-prone and unnecessary.
This change adds support for a `devpath` option, continuing to allow `bus` and requiring only one of them be specified.
TEST=Verified --flash-size outputs correct values with both devpath=/dev/i2c-7 and bus=7, as well as noting that one is required if neither is specified and only one may be specified if both are given.
Signed-off-by: Peter Marheine pmarheine@chromium.org Change-Id: Id2adf8a307b9205ce5e5804a6c3e22f19d0c34c9 Reviewed-on: https://review.coreboot.org/c/flashrom/+/51967 Reviewed-by: Edward O'Callaghan quasisec@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M i2c_helper.h M i2c_helper_linux.c M lspcon_i2c_spi.c 3 files changed, 78 insertions(+), 50 deletions(-)
Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved
diff --git a/i2c_helper.h b/i2c_helper.h index 599e08b..cab2ce6 100644 --- a/i2c_helper.h +++ b/i2c_helper.h @@ -72,6 +72,14 @@ int i2c_open(int bus, uint16_t addr, int force);
/** + * i2c_open_path: open an I2C device by device path + * + * This function behaves the same as i2c_open, but takes a filesystem + * path (assumed to be an I2C device file) instead of a bus number. + */ +int i2c_open_path(const char *path, uint16_t addr, int force); + +/** * i2c_close - closes the file descriptor returned by i2c_open * * @fd: file descriptor to be closed. diff --git a/i2c_helper_linux.c b/i2c_helper_linux.c index 9383c85..7744505 100644 --- a/i2c_helper_linux.c +++ b/i2c_helper_linux.c @@ -37,14 +37,31 @@ return fd == -1 ? 0 : close(fd); }
+int i2c_open_path(const char *path, uint16_t addr, int force) +{ + int fd = open(path, O_RDWR); + if (fd < 0) { + msg_perr("Unable to open I2C device %s: %s.\n", path, strerror(errno)); + return fd; + } + + int request = force ? I2C_SLAVE_FORCE : I2C_SLAVE; + int ret = ioctl(fd, request, addr); + if (ret < 0) { + msg_perr("Unable to set I2C slave address to 0x%02x: %s.\n", addr, strerror(errno)); + i2c_close(fd); + return ret; + } + + return fd; +} + + int i2c_open(int bus, uint16_t addr, int force) { char dev[sizeof(I2C_DEV_PREFIX)] = {0};
int ret = -1; - int fd = -1; - - int request = force ? I2C_SLAVE_FORCE : I2C_SLAVE;
if (bus < 0 || bus > I2C_MAX_BUS) { msg_perr("Invalid I2C bus %d.\n", bus); @@ -57,20 +74,7 @@ return ret; }
- fd = open(dev, O_RDWR); - if (fd < 0) { - msg_perr("Unable to open I2C device %s: %s.\n", dev, strerror(errno)); - return fd; - } - - ret = ioctl(fd, request, addr); - if (ret < 0) { - msg_perr("Unable to set I2C slave address to 0x%02x: %s.\n", addr, strerror(errno)); - i2c_close(fd); - return ret; - } - - return fd; + return i2c_open_path(dev, addr, force); }
int i2c_read(int fd, uint16_t addr, i2c_buffer_t *buf) diff --git a/lspcon_i2c_spi.c b/lspcon_i2c_spi.c index 7599db6..b79e7f7 100644 --- a/lspcon_i2c_spi.c +++ b/lspcon_i2c_spi.c @@ -434,56 +434,72 @@ }
/* TODO: remove this out of the specific SPI master implementation. */ -static int get_bus(void) +static int get_bus_number(void) { char *bus_str = extract_programmer_param("bus"); - int ret = SPI_GENERIC_ERROR; - 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_bus_done; - } + /* Return INVALID_ADDRESS if bus value was given but invalid, and GENERIC_ERROR + * if no value was provided. */ + int ret = SPI_INVALID_ADDRESS;
- if (bus < 0 || bus > 255) { - msg_perr("%s: Value for 'bus' is out of range(0-255).\n", - __func__); - goto get_bus_done; - } + if (bus_str == NULL) + return SPI_GENERIC_ERROR;
- if (strlen(bus_suffix) > 0) { - msg_perr("%s: Garbage following 'bus' value.\n", - __func__); - goto get_bus_done; - } - - msg_pinfo("Using i2c bus %i.\n", bus); - ret = bus; + 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_bus_done; - } else { - msg_perr("%s: Bus number not specified.\n", __func__); } -get_bus_done: - if (bus_str) - free(bus_str);
+ if (bus < 0 || bus > 255) { + msg_perr("%s: Value for 'bus' is out of range(0-255).\n", __func__); + goto get_bus_done; + } + + if (strlen(bus_suffix) > 0) { + msg_perr("%s: Garbage following 'bus' value.\n", __func__); + goto get_bus_done; + } + + msg_pinfo("Using i2c bus %i.\n", bus); + ret = bus; + +get_bus_done: + free(bus_str); return ret; }
int lspcon_i2c_spi_init(void) { - int lspcon_i2c_spi_bus = get_bus(); - if (lspcon_i2c_spi_bus < 0) + int fd = -1; + int bus_number = get_bus_number(); + if (bus_number == SPI_INVALID_ADDRESS) { + /* Bus was specified but unusable, bail out immediately */ return SPI_GENERIC_ERROR; + } + char *device_path = extract_programmer_param("devpath");
- int ret = 0; - int fd = i2c_open(lspcon_i2c_spi_bus, REGISTER_ADDRESS, 0); + if (device_path != NULL && bus_number >= 0) { + msg_perr("%s: only one of bus and devpath may be specified\n", __func__); + free(device_path); + return SPI_GENERIC_ERROR; + } else if (device_path == NULL && bus_number < 0) { + msg_perr("%s: one of bus and devpath must be specified\n", __func__); + return SPI_GENERIC_ERROR; + } + + if (device_path != NULL) { + fd = i2c_open_path(device_path, REGISTER_ADDRESS, 0); + free(device_path); + } else { + fd = i2c_open(bus_number, REGISTER_ADDRESS, 0); + } + if (fd < 0) return fd;
- ret |= lspcon_i2c_spi_reset_mpu_stop(fd); + int ret = lspcon_i2c_spi_reset_mpu_stop(fd); if (ret) { msg_perr("%s: call to reset_mpu_stop failed.\n", __func__); return ret;