From 581738642fbeacdf97fc737a41b3128d72cf1a1c Mon Sep 17 00:00:00 2001 From: Nico Huber Date: Tue, 1 Aug 2017 17:09:35 +0200 Subject: [PATCH] Reinvent I2C ops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Do not use the global platform_i2c_transfer() function that can only be implemented by a single driver. Instead, make a `struct device` aware transfer() function the only interface function for I2C controller dri- vers to implement. To not force the slave device drivers to be implemented either above generic I2C or specialized SMBus operations, we support SMBus control- lers in the slave device interface too. We start with four simple slave functions: i2c_readb(), i2c_writeb(), i2c_readb_at() and i2c_writeb_at(). They are all compatible to respec- tive SMBus functions. But we keep aliases because it would be weird to force e.g. an I2C EEPROM driver to call smbus_read_byte(). Change-Id: I98386f91bf4799ba3df84ec8bc0f64edd4142818 Signed-off-by: Nico Huber Reviewed-on: https://review.coreboot.org/20846 Tested-by: build bot (Jenkins) Reviewed-by: Aaron Durbin Reviewed-by: Kyösti Mälkki --- src/device/Makefile.inc | 1 + src/device/i2c.c | 83 ----------- src/device/i2c_bus.c | 161 ++++++++++++++++++++++ src/include/device/i2c_bus.h | 82 ++++++++--- src/include/device/smbus_def.h | 10 +- src/include/types.h | 9 +- src/soc/intel/common/block/i2c/i2c.c | 10 +- src/soc/intel/common/block/i2c/lpss_i2c.c | 15 +- src/soc/intel/common/block/i2c/lpss_i2c.h | 8 ++ 9 files changed, 267 insertions(+), 112 deletions(-) create mode 100644 src/device/i2c_bus.c diff --git a/src/device/Makefile.inc b/src/device/Makefile.inc index 8e6bf5be60..a1d592051b 100644 --- a/src/device/Makefile.inc +++ b/src/device/Makefile.inc @@ -37,3 +37,4 @@ bootblock-y += i2c.c verstage-y += i2c.c romstage-y += i2c.c ramstage-y += i2c.c +ramstage-y += i2c_bus.c diff --git a/src/device/i2c.c b/src/device/i2c.c index 6fdd5cff26..aa695caf03 100644 --- a/src/device/i2c.c +++ b/src/device/i2c.c @@ -13,92 +13,9 @@ * GNU General Public License for more details. */ -#include -#include #include #include -#if ENV_RAMSTAGE -/* Return I2C operations for a bus */ -static inline const struct i2c_bus_operations *ops_i2c_bus(struct bus *bus) -{ - if (bus && bus->dev && bus->dev->ops) - return bus->dev->ops->ops_i2c_bus; - return NULL; -} - -int i2c_dev_find_bus(struct device *dev) -{ - const struct i2c_bus_operations *ops; - struct bus *pbus; - - if (!dev) - return -1; - - /* Locate parent bus with I2C controller ops */ - pbus = dev->bus; - while (pbus && pbus->dev && !ops_i2c_bus(pbus)) - if (pbus->dev->bus != pbus) - pbus = pbus->dev->bus; - - /* Check if this I2C controller ops implements dev_to_bus() */ - ops = ops_i2c_bus(pbus); - if (!ops || !ops->dev_to_bus) - return -1; - - /* Use controller ops to determine the bus number */ - return ops->dev_to_bus(pbus->dev); -} - -int i2c_dev_transfer(struct device *dev, struct i2c_msg *segments, int count) -{ - int bus = i2c_dev_find_bus(dev); - if (bus < 0) - return -1; - return i2c_transfer(bus, segments, count); -} - -int i2c_dev_readb(struct device *dev, uint8_t reg, uint8_t *data) -{ - int bus = i2c_dev_find_bus(dev); - if (bus < 0) - return -1; - return i2c_readb(bus, dev->path.i2c.device, reg, data); -} - -int i2c_dev_writeb(struct device *dev, uint8_t reg, uint8_t data) -{ - int bus = i2c_dev_find_bus(dev); - if (bus < 0) - return -1; - return i2c_writeb(bus, dev->path.i2c.device, reg, data); -} - -int i2c_dev_read_bytes(struct device *dev, uint8_t reg, uint8_t *data, int len) -{ - int bus = i2c_dev_find_bus(dev); - if (bus < 0) - return -1; - return i2c_read_bytes(bus, dev->path.i2c.device, reg, data, len); -} - -int i2c_dev_read_raw(struct device *dev, uint8_t *data, int len) -{ - int bus = i2c_dev_find_bus(dev); - if (bus < 0) - return -1; - return i2c_read_raw(bus, dev->path.i2c.device, data, len); -} - -int i2c_dev_write_raw(struct device *dev, uint8_t *data, int len) -{ - int bus = i2c_dev_find_bus(dev); - if (bus < 0) - return -1; - return i2c_write_raw(bus, dev->path.i2c.device, data, len); -} -#endif - int i2c_read_field(unsigned bus, uint8_t chip, uint8_t reg, uint8_t *data, uint8_t mask, uint8_t shift) { diff --git a/src/device/i2c_bus.c b/src/device/i2c_bus.c new file mode 100644 index 0000000000..1c543efaba --- /dev/null +++ b/src/device/i2c_bus.c @@ -0,0 +1,161 @@ +/* + * This file is part of the coreboot project. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include + +struct bus *i2c_link(struct device *const dev) +{ + if (!dev || !dev->bus) + return NULL; + + struct bus *link = dev->bus; + while (link) { + struct device *const parent = link->dev; + + if (parent && parent->ops && + (parent->ops->ops_i2c_bus || parent->ops->ops_smbus_bus)) + break; + + if (parent && parent->bus) + link = parent->bus; + else + link = NULL; + } + + if (!link) { + printk(BIOS_ALERT, "%s Cannot find I2C or SMBus bus operations", + dev_path(dev)); + } + + return link; +} + +int i2c_readb(struct device *const dev) +{ + struct device *const busdev = i2c_busdev(dev); + if (!busdev) + return -1; + + if (busdev->ops->ops_i2c_bus) { + uint8_t val; + const struct i2c_msg msg = { + .flags = I2C_M_RD, + .slave = dev->path.i2c.device, + .buf = &val, + .len = sizeof(val), + }; + + const int ret = busdev->ops->ops_i2c_bus-> + transfer(busdev, &msg, 1); + if (ret) + return ret; + else + return val; + } else if (busdev->ops->ops_smbus_bus->recv_byte) { + return busdev->ops->ops_smbus_bus->recv_byte(dev); + } else { + printk(BIOS_ERR, "%s Missing ops_smbus_bus->recv_byte", + dev_path(busdev)); + return -1; + } +} + +int i2c_writeb(struct device *const dev, uint8_t val) +{ + struct device *const busdev = i2c_busdev(dev); + if (!busdev) + return -1; + + if (busdev->ops->ops_i2c_bus) { + const struct i2c_msg msg = { + .flags = 0, + .slave = dev->path.i2c.device, + .buf = &val, + .len = sizeof(val), + }; + return busdev->ops->ops_i2c_bus->transfer(busdev, &msg, 1); + } else if (busdev->ops->ops_smbus_bus->send_byte) { + return busdev->ops->ops_smbus_bus->send_byte(dev, val); + } else { + printk(BIOS_ERR, "%s Missing ops_smbus_bus->send_byte", + dev_path(busdev)); + return -1; + } +} + +int i2c_readb_at(struct device *const dev, uint8_t off) +{ + struct device *const busdev = i2c_busdev(dev); + if (!busdev) + return -1; + + if (busdev->ops->ops_i2c_bus) { + uint8_t val; + const struct i2c_msg msg[] = { + { + .flags = 0, + .slave = dev->path.i2c.device, + .buf = &off, + .len = sizeof(off), + }, + { + .flags = I2C_M_RD, + .slave = dev->path.i2c.device, + .buf = &val, + .len = sizeof(val), + }, + }; + + const int ret = busdev->ops->ops_i2c_bus-> + transfer(busdev, msg, ARRAY_SIZE(msg)); + if (ret) + return ret; + else + return val; + } else if (busdev->ops->ops_smbus_bus->read_byte) { + return busdev->ops->ops_smbus_bus->read_byte(dev, off); + } else { + printk(BIOS_ERR, "%s Missing ops_smbus_bus->read_byte", + dev_path(busdev)); + return -1; + } +} + +int i2c_writeb_at(struct device *const dev, + const uint8_t off, const uint8_t val) +{ + struct device *const busdev = i2c_busdev(dev); + if (!busdev) + return -1; + + if (busdev->ops->ops_i2c_bus) { + uint8_t buf[] = { off, val }; + const struct i2c_msg msg = { + .flags = 0, + .slave = dev->path.i2c.device, + .buf = buf, + .len = sizeof(buf), + }; + return busdev->ops->ops_i2c_bus->transfer(busdev, &msg, 1); + } else if (busdev->ops->ops_smbus_bus->write_byte) { + return busdev->ops->ops_smbus_bus->write_byte(dev, off, val); + } else { + printk(BIOS_ERR, "%s Missing ops_smbus_bus->write_byte", + dev_path(busdev)); + return -1; + } +} diff --git a/src/include/device/i2c_bus.h b/src/include/device/i2c_bus.h index 474d9e5b2c..f1416a7411 100644 --- a/src/include/device/i2c_bus.h +++ b/src/include/device/i2c_bus.h @@ -1,8 +1,6 @@ /* * This file is part of the coreboot project. * - * Copyright (C) 2014 Google, Inc. - * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; version 2 of the License. @@ -16,29 +14,79 @@ #ifndef _DEVICE_I2C_BUS_H_ #define _DEVICE_I2C_BUS_H_ +#include #include #include #include /* I2C bus operation for ramstage drivers */ struct i2c_bus_operations { - /* - * This is an SOC specific method that can be provided to translate the - * 'struct device' for an I2C controller into a unique I2C bus number. - * Returns -1 if the bus number for this device cannot be determined. - */ - int (*dev_to_bus)(struct device *dev); + int (*transfer)(struct device *, const struct i2c_msg *, size_t count); }; -/* Return I2C bus number for provided device, -1 if not found */ -int i2c_dev_find_bus(struct device *dev); +/* + * Returns the first upstream facing link whose bus implements either + * `i2c_bus_operations` *or* `smbus_bus_operations`. + * + * If not NULL, guarantees that `->dev`, `->dev->ops` and either + * `->dev->ops->ops_i2c_bus` or `->dev->ops->ops_smbus_bus` are + * not NULL. + */ +struct bus *i2c_link(struct device *); -/* Variants of I2C helper functions that take a device instead of bus number */ -int i2c_dev_transfer(struct device *dev, struct i2c_msg *segments, int count); -int i2c_dev_readb(struct device *dev, uint8_t reg, uint8_t *data); -int i2c_dev_writeb(struct device *dev, uint8_t reg, uint8_t data); -int i2c_dev_read_bytes(struct device *dev, uint8_t reg, uint8_t *data, int len); -int i2c_dev_read_raw(struct device *dev, uint8_t *data, int len); -int i2c_dev_write_raw(struct device *dev, uint8_t *data, int len); +/* + * Shorthand for `i2c_link(dev)->dev`. + * + * Returns NULL if i2c_link(dev) returns NULL. + */ +static inline struct device *i2c_busdev(struct device *dev) +{ + struct bus *const link = i2c_link(dev); + return link ? link->dev : NULL; +} + +/* + * Slave driver interface functions. These will look for the next + * `i2c_bus_operations` *or* `smbus_bus_operations` and perform the + * respective transfers. + * + * The interface is limited to what current slave drivers demand. + * Extend as required. + * + * All functions return a negative `enum cb_err` value on error. + * Either CB_ERR, CB_ERR_ARG or any CB_I2C_* (cf. include/types.h). + */ + +/* + * Reads one byte. + * Compatible to smbus_recv_byte(). + * + * Returns the read byte on success, negative `enum cb_err` value on error. + */ +int i2c_readb(struct device *); + +/* + * Writes the byte `val`. + * Compatible to smbus_send_byte(). + * + * Returns 0 on success, negative `enum cb_err` value on error. + */ +int i2c_writeb(struct device *, uint8_t val); + +/* + * Sends the register offset `off` and reads one byte. + * Compatible to smbus_read_byte(). + * + * Returns the read byte on success, negative `enum cb_err` value on error. + */ +int i2c_readb_at(struct device *, uint8_t off); + +/* + * Sends the register offset `off` followed by the byte `val`. + * Compatible to smbus_write_byte(). + * + * Returns 0 on success, negative `enum cb_err` value on error. + */ +int i2c_writeb_at(struct device *, uint8_t off, uint8_t val); #endif /* _DEVICE_I2C_BUS_H_ */ diff --git a/src/include/device/smbus_def.h b/src/include/device/smbus_def.h index 0b07400b37..61d786107b 100644 --- a/src/include/device/smbus_def.h +++ b/src/include/device/smbus_def.h @@ -1,10 +1,12 @@ #ifndef DEVICE_SMBUS_DEF_H #define DEVICE_SMBUS_DEF_H +#include + /* Error results are negative success is >= 0 */ -#define SMBUS_ERROR -1 -#define SMBUS_WAIT_UNTIL_READY_TIMEOUT -2 -#define SMBUS_WAIT_UNTIL_DONE_TIMEOUT -3 -#define SMBUS_WAIT_UNTIL_ACTIVE_TIMEOUT -4 +#define SMBUS_ERROR CB_ERR +#define SMBUS_WAIT_UNTIL_READY_TIMEOUT CB_I2C_BUSY +#define SMBUS_WAIT_UNTIL_DONE_TIMEOUT CB_I2C_TIMEOUT +#define SMBUS_WAIT_UNTIL_ACTIVE_TIMEOUT CB_I2C_NO_DEVICE #endif /* DEVICE_SMBUS_DEF_H */ diff --git a/src/include/types.h b/src/include/types.h index df0947370f..7307cd5072 100644 --- a/src/include/types.h +++ b/src/include/types.h @@ -47,7 +47,14 @@ enum cb_err { /* Keyboard test failures */ CB_KBD_CONTROLLER_FAILURE = -200, - CB_KBD_INTERFACE_FAILURE = -201 + CB_KBD_INTERFACE_FAILURE = -201, + + /* I2C controller failures */ + CB_I2C_NO_DEVICE = -300, /**< Device is not responding */ + CB_I2C_BUSY = -301, /**< Device tells it's busy */ + CB_I2C_PROTOCOL_ERROR = -302, /**< Data lost or spurious slave + device response, try again? */ + CB_I2C_TIMEOUT = -303, /**< Transmission timed out */ }; #endif /* __TYPES_H */ diff --git a/src/soc/intel/common/block/i2c/i2c.c b/src/soc/intel/common/block/i2c/i2c.c index aeb6d8f2ed..625bb991d4 100644 --- a/src/soc/intel/common/block/i2c/i2c.c +++ b/src/soc/intel/common/block/i2c/i2c.c @@ -150,8 +150,14 @@ static void lpss_i2c_acpi_fill_ssdt(struct device *dev) acpigen_pop_len(); } -static struct i2c_bus_operations i2c_bus_ops = { - .dev_to_bus = &lpss_i2c_dev_to_bus, +static int lpss_i2c_dev_transfer(struct device *dev, + const struct i2c_msg *msg, size_t count) +{ + return lpss_i2c_transfer(lpss_i2c_dev_to_bus(dev), msg, count); +} + +static const struct i2c_bus_operations i2c_bus_ops = { + .transfer = lpss_i2c_dev_transfer, }; static struct device_operations i2c_dev_ops = { diff --git a/src/soc/intel/common/block/i2c/lpss_i2c.c b/src/soc/intel/common/block/i2c/lpss_i2c.c index 868a6ad300..7db3fe9668 100644 --- a/src/soc/intel/common/block/i2c/lpss_i2c.c +++ b/src/soc/intel/common/block/i2c/lpss_i2c.c @@ -258,7 +258,7 @@ static int lpss_i2c_wait_for_bus_idle(struct lpss_i2c_regs *regs) /* Transfer one byte of one segment, sending stop bit if requested */ static int lpss_i2c_transfer_byte(struct lpss_i2c_regs *regs, - struct i2c_msg *segment, + const struct i2c_msg *segment, size_t byte, int send_stop) { struct stopwatch sw; @@ -297,16 +297,15 @@ static int lpss_i2c_transfer_byte(struct lpss_i2c_regs *regs, return 0; } -/* Global I2C bus handler, defined in include/i2c.h */ -int platform_i2c_transfer(unsigned int bus, struct i2c_msg *segments, - int count) +int lpss_i2c_transfer(unsigned int bus, + const struct i2c_msg *segments, size_t count) { struct stopwatch sw; struct lpss_i2c_regs *regs; size_t byte; int ret = -1; - if (count <= 0 || !segments) + if (count == 0 || !segments) return -1; regs = (struct lpss_i2c_regs *)lpss_i2c_base_address(bus); @@ -396,6 +395,12 @@ out: return ret; } +/* Global I2C bus handler, defined in include/device/i2c_simple.h */ +int platform_i2c_transfer(unsigned int bus, struct i2c_msg *msg, int count) +{ + return lpss_i2c_transfer(bus, msg, count < 0 ? 0 : count); +} + static int lpss_i2c_set_speed_config(unsigned int bus, const struct lpss_i2c_speed_config *config) { diff --git a/src/soc/intel/common/block/i2c/lpss_i2c.h b/src/soc/intel/common/block/i2c/lpss_i2c.h index 2cb3d5ed52..72341621c0 100644 --- a/src/soc/intel/common/block/i2c/lpss_i2c.h +++ b/src/soc/intel/common/block/i2c/lpss_i2c.h @@ -89,3 +89,11 @@ int lpss_i2c_gen_speed_config(struct lpss_i2c_regs *regs, enum i2c_speed speed, const struct lpss_i2c_bus_config *bcfg, struct lpss_i2c_speed_config *config); + +/* + * Process given I2C segments in a single transfer + * Return value: + * -1 = failure + * 0 = success + */ +int lpss_i2c_transfer(unsigned int bus, const struct i2c_msg *, size_t count);