From 0969e43b3fae48c90aaf1df284560f6b76f84efd Mon Sep 17 00:00:00 2001 From: Peter Marheine Date: Wed, 31 Mar 2021 11:28:11 +1100 Subject: 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 Change-Id: Id2adf8a307b9205ce5e5804a6c3e22f19d0c34c9 Reviewed-on: https://review.coreboot.org/c/flashrom/+/51967 Reviewed-by: Edward O'Callaghan Tested-by: build bot (Jenkins) --- i2c_helper.h | 8 +++++ i2c_helper_linux.c | 38 +++++++++++++----------- lspcon_i2c_spi.c | 86 ++++++++++++++++++++++++++++++++---------------------- 3 files changed, 80 insertions(+), 52 deletions(-) diff --git a/i2c_helper.h b/i2c_helper.h index 599e08bb..cab2ce6f 100644 --- a/i2c_helper.h +++ b/i2c_helper.h @@ -71,6 +71,14 @@ static inline int i2c_buffer_t_fill(i2c_buffer_t *i2c_buf, void *buf, uint16_t l */ 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 * diff --git a/i2c_helper_linux.c b/i2c_helper_linux.c index 9383c857..77445052 100644 --- a/i2c_helper_linux.c +++ b/i2c_helper_linux.c @@ -37,14 +37,31 @@ int i2c_close(int fd) 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 @@ int i2c_open(int bus, uint16_t addr, int force) 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 7599db6b..b79e7f7b 100644 --- a/lspcon_i2c_spi.c +++ b/lspcon_i2c_spi.c @@ -434,56 +434,72 @@ static int lspcon_i2c_spi_shutdown(void *data) } /* 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; - } - - 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; + /* 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_str == NULL) + return SPI_GENERIC_ERROR; + + 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; + } + + if (bus < 0 || bus > 255) { + msg_perr("%s: Value for 'bus' is out of range(0-255).\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 (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"); + + 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); + } - int ret = 0; - int fd = i2c_open(lspcon_i2c_spi_bus, 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; -- cgit v1.2.3