Angel Pons submitted this change.
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(-)
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;
To view, visit change 51967. To unsubscribe, or for help writing mail filters, visit settings.