[U-Boot] [PATCH v4 0/11] dm: Add I2C support and convert sandbox, tegra
This series adds I2C support to driver model. It has become apparent that this is a high priority as it is widely used. It follows along to some extent from the SPI conversion.
Several changes are made from the original I2C implementations.
Firstly it is not necessary to specify the chip address with every call, since each chip knows its own address - it is stored in struct dm_i2c_chip which is attached to each chip on the I2C bus. However, this information *is* passed to the driver since I presume most drivers need it and it would be cumbersome to look up in every call.
Secondly there is no concept of a 'current' I2C bus so all associated logic is removed. With driver model i2c_set_bus_num() and i2c_get_bus_num() are not available. Since the chip device specifies both the bus and the chip address, there is no need for this concept. It also causes problems when one driver changes the current bus and forgets to change it back.
Thirdly initialisation is handled by driver model's normal probe() method on each device so there should be no need for i2c_init_all(), i2c_init(), i2c_init_board(), i2c_board_late_init() and board_i2c_init().
I2C muxes are not yet supported. To support these we will need to maintain state of the current mux settings to avoid resetting every mux every time. Probably we need to add a sandbox I2C mux driver to permit testing of this. This can probably be done later.
Platform data is not yet supported either, only device tree. The U_BOOT_I2C_MKENT_COMPLETE() and U_BOOT_I2C_ADAP_COMPLETE() macros are not used. Also struct i2c_adapter is not defined anymore. This will need to be addressed, perhaps as part of converting over a board that does not use device tree, assuming that we want to support this.
The following I2C CONFIGs are no-longer needed when driver model is used:
CONFIG_SYS_I2C_INIT_BOARD - each I2C bus is inited in its probe() method CONFIG_I2C_MULTI_BUS - we always support multi-bus with driver model CONFIG_SYS_MAX_I2C_BUS - the device tree aliases define available buses CONFIG_SYS_I2C_SPEED - the device tree specifies the speed for each bus CONFIG_SYS_I2C - this is the 'new old' API, now deprecated
There are a few SPI patches included here due to a dependency on a new device binding function.
v3 changes the uclass to driver interface significantly. It is now a list of messages to be processed by the driver. This matches the Linux user space API which may be a benefit. It does introduce one complication, which is that the protocol does not support separate chip offset and data. I have enhanced it to do so.
This series is available at u-boot-dm/i2c-working2.
Changes in v4: - Add a constant for I2C_MAX_OFFSET_LEN - Add a probe_chip() method for the eeprom - Add a probe_chip() method to the uclass - Add a remove() method for the eeprom - Add an assert for non-null - Add an eeprom test mode - Add chip flags to i2c_probe() - Add comment to i2c_setup_offset() - Add comments to indicate which methods are optional - Add comments to set_bus_speed() method - Add eeprom debugging output - Add new patch to add newline to debug() messages - Add probe_chip() method - Add support for eeprom offset length - Add tests for offset length - Adjust i2c_bind_driver() to avoid calloc()/free() - Adjust tests to match uclass changes - Drop set_offset_len() method - Fix copying of chip flags to message in i2c_setup_offset() - Fix endianness of i2c_setup_offset() - Fix method comment for xfer() - Invert return value of i2c_setup_offset() - Probe with a message length of 0 - Rename i2c_generic_drv to i2c_generic_chip_drv - Rename i2c_read_bytewise() and implement a real one - Update call to i2c_probe()
Changes in v3: - Add a helper to query chip flags - Add new 'i2c flags' command to get/set chip flags - Add support for reading a byte at a time with an address for each byte - Adjust for slightly altered I2C uclass API - Change driver to support new message-based I2C uclass API - Change uclass <=> driver API to use a message list - Correct bogus address len code (was confused with chip address length) - Drop extra call to i2c_bind_driver() in i2c_probe() - Update emulator for new uclass interface - Update for new uclass <=> driver interface - Update for new uclass <=> emulateor interface
Changes in v2: - Add a 'deblock' method to recover an I2C bus stuck in mid-transaction - Add a helper function to find a chip on a particular bus number - Add a new asm/test.h header for tests in sandbox - Add a test for automatic binding of generic I2C devices - Add some debugging for generic I2C device binding - Adjust tegra_i2c_child_pre_probe() to permit generic I2C devices - Call the deblock() method for 'i2c reset' - Change alen to int so that it can be -1 (this was a bug) - Correct the compatible strings for I2C buses - Don't init if the speed is 0, since this breaks the controller - Expand coverage to all Tegra boards - Fix cihp typo - Implement generic I2C devices to allow 'i2c probe' on unknown devices - Return the probed device from i2c_probe() - Set the bus speed after the bus is probed - Update commit message for EEPROM driver
Simon Glass (11): dm: i2c: Add a uclass for I2C dm: i2c: Implement driver model support in the i2c command dm: i2c: Add I2C emulation driver for sandbox dm: i2c: Add a sandbox I2C driver dm: i2c: Add an I2C EEPROM simulator dm: i2c: config: Enable I2C for sandbox using driver model dm: i2c: dts: Add an I2C bus for sandbox dm: Add a simple EEPROM driver dm: i2c: Add tests for I2C dm: device: Add newline to debug() messages dm: i2c: tegra: Convert to driver model
arch/arm/cpu/tegra20-common/pmu.c | 21 +- arch/arm/dts/tegra124-jetson-tk1.dts | 1 - arch/arm/dts/tegra30-tec-ng.dts | 4 + arch/arm/include/asm/arch-tegra/tegra_i2c.h | 2 +- arch/sandbox/dts/sandbox.dts | 17 ++ arch/sandbox/include/asm/test.h | 26 ++ board/avionic-design/common/tamonten-ng.c | 12 +- board/nvidia/cardhu/cardhu.c | 13 +- board/nvidia/common/board.c | 4 - board/nvidia/dalmore/dalmore.c | 21 +- board/nvidia/whistler/whistler.c | 29 +- board/toradex/apalis_t30/apalis_t30.c | 19 +- common/cmd_i2c.c | 376 +++++++++++++++++++---- drivers/core/device.c | 6 +- drivers/i2c/Makefile | 2 + drivers/i2c/i2c-emul-uclass.c | 14 + drivers/i2c/i2c-uclass.c | 457 ++++++++++++++++++++++++++++ drivers/i2c/sandbox_i2c.c | 129 ++++++++ drivers/i2c/tegra_i2c.c | 366 ++++++++-------------- drivers/misc/Makefile | 4 + drivers/misc/i2c_eeprom.c | 51 ++++ drivers/misc/i2c_eeprom_emul.c | 176 +++++++++++ drivers/power/tps6586x.c | 27 +- include/config_fallbacks.h | 6 + include/configs/apalis_t30.h | 3 - include/configs/beaver.h | 3 - include/configs/cardhu.h | 5 - include/configs/colibri_t30.h | 3 - include/configs/dalmore.h | 5 - include/configs/jetson-tk1.h | 5 - include/configs/nyan-big.h | 5 - include/configs/sandbox.h | 6 + include/configs/seaboard.h | 3 - include/configs/tec-ng.h | 5 - include/configs/tegra-common.h | 1 + include/configs/tegra114-common.h | 3 - include/configs/tegra124-common.h | 3 - include/configs/tegra20-common.h | 3 - include/configs/tegra30-common.h | 3 - include/configs/trimslice.h | 3 - include/configs/venice2.h | 5 - include/configs/whistler.h | 3 - include/dm/uclass-id.h | 4 + include/dm/ut.h | 12 + include/i2c.h | 349 +++++++++++++++++++++ include/i2c_eeprom.h | 19 ++ include/tps6586x.h | 4 +- test/dm/Makefile | 1 + test/dm/i2c.c | 210 +++++++++++++ test/dm/test.dts | 17 ++ 50 files changed, 2036 insertions(+), 430 deletions(-) create mode 100644 arch/sandbox/include/asm/test.h create mode 100644 drivers/i2c/i2c-emul-uclass.c create mode 100644 drivers/i2c/i2c-uclass.c create mode 100644 drivers/i2c/sandbox_i2c.c create mode 100644 drivers/misc/i2c_eeprom.c create mode 100644 drivers/misc/i2c_eeprom_emul.c create mode 100644 include/i2c_eeprom.h create mode 100644 test/dm/i2c.c
The uclass implements the same operations as the current I2C framework but makes some changes to make it fit driver model better:
- Remove the chip address from API calls - Remove the address length from API calls - Remove concept of 'current' I2C bus - Drop all existing init functions
Acked-by: Heiko Schocher hs@denx.de Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v4: - Add a constant for I2C_MAX_OFFSET_LEN - Add a probe_chip() method to the uclass - Add chip flags to i2c_probe() - Add comment to i2c_setup_offset() - Add comments to indicate which methods are optional - Add comments to set_bus_speed() method - Adjust i2c_bind_driver() to avoid calloc()/free() - Drop set_offset_len() method - Fix copying of chip flags to message in i2c_setup_offset() - Fix endianness of i2c_setup_offset() - Fix method comment for xfer() - Invert return value of i2c_setup_offset() - Probe with a message length of 0 - Rename i2c_generic_drv to i2c_generic_chip_drv - Rename i2c_read_bytewise() and implement a real one
Changes in v3: - Add a helper to query chip flags - Add support for reading a byte at a time with an address for each byte - Change uclass <=> driver API to use a message list - Correct bogus address len code (was confused with chip address length) - Drop extra call to i2c_bind_driver() in i2c_probe()
Changes in v2: - Add a 'deblock' method to recover an I2C bus stuck in mid-transaction - Add a helper function to find a chip on a particular bus number - Add some debugging for generic I2C device binding - Fix cihp typo - Implement generic I2C devices to allow 'i2c probe' on unknown devices - Return the probed device from i2c_probe() - Set the bus speed after the bus is probed
drivers/i2c/Makefile | 1 + drivers/i2c/i2c-uclass.c | 457 +++++++++++++++++++++++++++++++++++++++++++++ include/config_fallbacks.h | 6 + include/dm/uclass-id.h | 2 + include/i2c.h | 349 ++++++++++++++++++++++++++++++++++ 5 files changed, 815 insertions(+) create mode 100644 drivers/i2c/i2c-uclass.c
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile index dae3d71..063e097 100644 --- a/drivers/i2c/Makefile +++ b/drivers/i2c/Makefile @@ -4,6 +4,7 @@ # # SPDX-License-Identifier: GPL-2.0+ # +obj-$(CONFIG_DM_I2C) += i2c-uclass.o
obj-$(CONFIG_SYS_I2C_ADI) += adi_i2c.o obj-$(CONFIG_I2C_MV) += mv_i2c.o diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c new file mode 100644 index 0000000..e5a50eb --- /dev/null +++ b/drivers/i2c/i2c-uclass.c @@ -0,0 +1,457 @@ +/* + * Copyright (c) 2014 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include <errno.h> +#include <fdtdec.h> +#include <i2c.h> +#include <malloc.h> +#include <dm/device-internal.h> +#include <dm/lists.h> +#include <dm/root.h> + +DECLARE_GLOBAL_DATA_PTR; + +#define I2C_MAX_OFFSET_LEN 4 + +/** + * i2c_setup_offset() - Set up a new message with a chip offset + * + * @chip: Chip to use + * @offset: Byte offset within chip + * @offset_buf: Place to put byte offset + * @msg: Message buffer + * @return 0 if OK, -EADDRNOTAVAIL if the offset length is 0. In that case the + * message is still set up but will not contain an offset. + */ +static int i2c_setup_offset(struct dm_i2c_chip *chip, uint offset, + uint8_t offset_buf[], struct i2c_msg *msg) +{ + int offset_len; + + msg->addr = chip->chip_addr; + msg->flags = chip->flags & DM_I2C_CHIP_10BIT ? I2C_M_TEN : 0; + msg->len = chip->offset_len; + msg->buf = offset_buf; + if (!chip->offset_len) + return -EADDRNOTAVAIL; + offset_len = chip->offset_len; + while (offset_len--) + *offset_buf++ = offset >> (8 * offset_len); + + return 0; +} + +static int i2c_read_bytewise(struct udevice *dev, uint offset, + uint8_t *buffer, int len) +{ + struct dm_i2c_chip *chip = dev_get_parentdata(dev); + struct udevice *bus = dev_get_parent(dev); + struct dm_i2c_ops *ops = i2c_get_ops(bus); + struct i2c_msg msg[2], *ptr; + uint8_t offset_buf[I2C_MAX_OFFSET_LEN]; + int ret; + int i; + + for (i = 0; i < len; i++) { + if (i2c_setup_offset(chip, offset + i, offset_buf, msg)) + return -EINVAL; + ptr = msg + 1; + ptr->addr = chip->chip_addr; + ptr->flags = msg->flags | I2C_M_RD; + ptr->len = 1; + ptr->buf = &buffer[i]; + ptr++; + + ret = ops->xfer(bus, msg, ptr - msg); + if (ret) + return ret; + } + + return 0; +} + +static int i2c_write_bytewise(struct udevice *dev, uint offset, + const uint8_t *buffer, int len) +{ + struct dm_i2c_chip *chip = dev_get_parentdata(dev); + struct udevice *bus = dev_get_parent(dev); + struct dm_i2c_ops *ops = i2c_get_ops(bus); + struct i2c_msg msg[1]; + uint8_t buf[I2C_MAX_OFFSET_LEN + 1]; + int ret; + int i; + + for (i = 0; i < len; i++) { + if (i2c_setup_offset(chip, offset + i, buf, msg)) + return -EINVAL; + buf[msg->len++] = buffer[i]; + + ret = ops->xfer(bus, msg, 1); + if (ret) + return ret; + } + + return 0; +} + +int i2c_read(struct udevice *dev, uint offset, uint8_t *buffer, int len) +{ + struct dm_i2c_chip *chip = dev_get_parentdata(dev); + struct udevice *bus = dev_get_parent(dev); + struct dm_i2c_ops *ops = i2c_get_ops(bus); + struct i2c_msg msg[2], *ptr; + uint8_t offset_buf[I2C_MAX_OFFSET_LEN]; + int msg_count; + + if (!ops->xfer) + return -ENOSYS; + if (chip->flags & DM_I2C_CHIP_RE_ADDRESS) + return i2c_read_bytewise(dev, offset, buffer, len); + ptr = msg; + if (!i2c_setup_offset(chip, offset, offset_buf, ptr)) + ptr++; + + if (len) { + ptr->addr = chip->chip_addr; + ptr->flags = chip->flags & DM_I2C_CHIP_10BIT ? I2C_M_TEN : 0; + ptr->flags |= I2C_M_RD; + ptr->len = len; + ptr->buf = buffer; + ptr++; + } + msg_count = ptr - msg; + + return ops->xfer(bus, msg, msg_count); +} + +int i2c_write(struct udevice *dev, uint offset, const uint8_t *buffer, int len) +{ + struct dm_i2c_chip *chip = dev_get_parentdata(dev); + struct udevice *bus = dev_get_parent(dev); + struct dm_i2c_ops *ops = i2c_get_ops(bus); + struct i2c_msg msg[1]; + + if (!ops->xfer) + return -ENOSYS; + + if (chip->flags & DM_I2C_CHIP_RE_ADDRESS) + return i2c_write_bytewise(dev, offset, buffer, len); + /* + * The simple approach would be to send two messages here: one to + * set the offset and one to write the bytes. However some drivers + * will not be expecting this, and some chips won't like how the + * driver presents this on the I2C bus. + * + * The API does not support separate offset and data. We could extend + * it with a flag indicating that there is data in the next message + * that needs to be processed in the same transaction. We could + * instead add an additional buffer to each message. For now, handle + * this in the uclass since it isn't clear what the impact on drivers + * would be with this extra complication. Unfortunately this means + * copying the message. + * + * Use the stack for small messages, malloc() for larger ones. We + * need to allow space for the offset (up to 4 bytes) and the message + * itself. + */ + if (len < 64) { + uint8_t buf[I2C_MAX_OFFSET_LEN + len]; + + i2c_setup_offset(chip, offset, buf, msg); + msg->len += len; + memcpy(buf + chip->offset_len, buffer, len); + + return ops->xfer(bus, msg, 1); + } else { + uint8_t *buf; + int ret; + + buf = malloc(I2C_MAX_OFFSET_LEN + len); + if (!buf) + return -ENOMEM; + i2c_setup_offset(chip, offset, buf, msg); + msg->len += len; + memcpy(buf + chip->offset_len, buffer, len); + + ret = ops->xfer(bus, msg, 1); + free(buf); + return ret; + } +} + +static int i2c_probe_chip(struct udevice *bus, uint chip_addr, + enum dm_i2c_chip_flags flags) +{ + struct dm_i2c_ops *ops = i2c_get_ops(bus); + struct i2c_msg msg[1]; + uint8_t ch = 0; + int ret; + + if (ops->probe_chip) { + ret = ops->probe_chip(bus, chip_addr, flags); + if (!ret || ret != -ENOSYS) + return ret; + } + + if (!ops->xfer) + return -ENOSYS; + + /* Probe with a zero-length message */ + msg->addr = chip_addr; + msg->flags = 0; + msg->len = 0; + msg->buf = &ch; + + return ops->xfer(bus, msg, 1); +} + +static int i2c_bind_driver(struct udevice *bus, uint chip_addr, + struct udevice **devp) +{ + struct dm_i2c_chip chip; + char name[30], *str; + struct udevice *dev; + int ret; + + snprintf(name, sizeof(name), "generic_%x", chip_addr); + str = strdup(name); + ret = device_bind_driver(bus, "i2c_generic_chip_drv", str, &dev); + debug("%s: device_bind_driver: ret=%d\n", __func__, ret); + if (ret) + goto err_bind; + + /* Tell the device what we know about it */ + memset(&chip, '\0', sizeof(chip)); + chip.chip_addr = chip_addr; + chip.offset_len = 1; /* we assume */ + ret = device_probe_child(dev, &chip); + debug("%s: device_probe_child: ret=%d\n", __func__, ret); + if (ret) + goto err_probe; + + *devp = dev; + return 0; + +err_probe: + device_unbind(dev); +err_bind: + free(str); + return ret; +} + +int i2c_get_chip(struct udevice *bus, uint chip_addr, struct udevice **devp) +{ + struct udevice *dev; + + debug("%s: Searching bus '%s' for address %02x: ", __func__, + bus->name, chip_addr); + for (device_find_first_child(bus, &dev); dev; + device_find_next_child(&dev)) { + struct dm_i2c_chip store; + struct dm_i2c_chip *chip = dev_get_parentdata(dev); + int ret; + + if (!chip) { + chip = &store; + i2c_chip_ofdata_to_platdata(gd->fdt_blob, + dev->of_offset, chip); + } + if (chip->chip_addr == chip_addr) { + ret = device_probe(dev); + debug("found, ret=%d\n", ret); + if (ret) + return ret; + *devp = dev; + return 0; + } + } + debug("not found\n"); + return i2c_bind_driver(bus, chip_addr, devp); +} + +int i2c_get_chip_for_busnum(int busnum, int chip_addr, struct udevice **devp) +{ + struct udevice *bus; + int ret; + + ret = uclass_get_device_by_seq(UCLASS_I2C, busnum, &bus); + if (ret) { + debug("Cannot find I2C bus %d\n", busnum); + return ret; + } + ret = i2c_get_chip(bus, chip_addr, devp); + if (ret) { + debug("Cannot find I2C chip %02x on bus %d\n", chip_addr, + busnum); + return ret; + } + + return 0; +} + +int i2c_probe(struct udevice *bus, uint chip_addr, uint chip_flags, + struct udevice **devp) +{ + int ret; + + *devp = NULL; + + /* First probe that chip */ + ret = i2c_probe_chip(bus, chip_addr, chip_flags); + debug("%s: bus='%s', address %02x, ret=%d\n", __func__, bus->name, + chip_addr, ret); + if (ret) + return ret; + + /* The chip was found, see if we have a driver, and probe it */ + ret = i2c_get_chip(bus, chip_addr, devp); + debug("%s: i2c_get_chip: ret=%d\n", __func__, ret); + + return ret; +} + +int i2c_set_bus_speed(struct udevice *bus, unsigned int speed) +{ + struct dm_i2c_ops *ops = i2c_get_ops(bus); + struct dm_i2c_bus *i2c = bus->uclass_priv; + int ret; + + /* + * If we have a method, call it. If not then the driver probably wants + * to deal with speed changes on the next transfer. It can easily read + * the current speed from this uclass + */ + if (ops->set_bus_speed) { + ret = ops->set_bus_speed(bus, speed); + if (ret) + return ret; + } + i2c->speed_hz = speed; + + return 0; +} + +/* + * i2c_get_bus_speed: + * + * Returns speed of selected I2C bus in Hz + */ +int i2c_get_bus_speed(struct udevice *bus) +{ + struct dm_i2c_ops *ops = i2c_get_ops(bus); + struct dm_i2c_bus *i2c = bus->uclass_priv; + + if (!ops->set_bus_speed) + return i2c->speed_hz; + + return ops->get_bus_speed(bus); +} + +int i2c_set_chip_flags(struct udevice *dev, uint flags) +{ + struct udevice *bus = dev->parent; + struct dm_i2c_chip *chip = dev_get_parentdata(dev); + struct dm_i2c_ops *ops = i2c_get_ops(bus); + int ret; + + if (ops->set_flags) { + ret = ops->set_flags(dev, flags); + if (ret) + return ret; + } + chip->flags = flags; + + return 0; +} + +int i2c_get_chip_flags(struct udevice *dev, uint *flagsp) +{ + struct dm_i2c_chip *chip = dev_get_parentdata(dev); + + *flagsp = chip->flags; + + return 0; +} + +int i2c_set_chip_offset_len(struct udevice *dev, uint offset_len) +{ + struct dm_i2c_chip *chip = dev_get_parentdata(dev); + + if (offset_len > I2C_MAX_OFFSET_LEN) + return -EINVAL; + chip->offset_len = offset_len; + + return 0; +} + +int i2c_deblock(struct udevice *bus) +{ + struct dm_i2c_ops *ops = i2c_get_ops(bus); + + /* + * We could implement a software deblocking here if we could get + * access to the GPIOs used by I2C, and switch them to GPIO mode + * and then back to I2C. This is somewhat beyond our powers in + * driver model at present, so for now just fail. + * + * See https://patchwork.ozlabs.org/patch/399040/ + */ + if (!ops->deblock) + return -ENOSYS; + + return ops->deblock(bus); +} + +int i2c_chip_ofdata_to_platdata(const void *blob, int node, + struct dm_i2c_chip *chip) +{ + chip->offset_len = 1; /* default */ + chip->flags = 0; + chip->chip_addr = fdtdec_get_int(gd->fdt_blob, node, "reg", -1); + if (chip->chip_addr == -1) { + debug("%s: I2C Node '%s' has no 'reg' property\n", __func__, + fdt_get_name(blob, node, NULL)); + return -EINVAL; + } + + return 0; +} + +static int i2c_post_probe(struct udevice *dev) +{ + struct dm_i2c_bus *i2c = dev->uclass_priv; + + i2c->speed_hz = fdtdec_get_int(gd->fdt_blob, dev->of_offset, + "clock-frequency", 100000); + + return i2c_set_bus_speed(dev, i2c->speed_hz); +} + +int i2c_post_bind(struct udevice *dev) +{ + /* Scan the bus for devices */ + return dm_scan_fdt_node(dev, gd->fdt_blob, dev->of_offset, false); +} + +UCLASS_DRIVER(i2c) = { + .id = UCLASS_I2C, + .name = "i2c", + .per_device_auto_alloc_size = sizeof(struct dm_i2c_bus), + .post_bind = i2c_post_bind, + .post_probe = i2c_post_probe, +}; + +UCLASS_DRIVER(i2c_generic) = { + .id = UCLASS_I2C_GENERIC, + .name = "i2c_generic", +}; + +U_BOOT_DRIVER(i2c_generic_chip_drv) = { + .name = "i2c_generic_chip_drv", + .id = UCLASS_I2C_GENERIC, +}; diff --git a/include/config_fallbacks.h b/include/config_fallbacks.h index 7d8daa2..91a0a43 100644 --- a/include/config_fallbacks.h +++ b/include/config_fallbacks.h @@ -87,4 +87,10 @@ #undef CONFIG_IMAGE_FORMAT_LEGACY #endif
+#ifdef CONFIG_DM_I2C +# ifdef CONFIG_SYS_I2C +# error "Cannot define CONFIG_SYS_I2C when CONFIG_DM_I2C is used" +# endif +#endif + #endif /* __CONFIG_FALLBACKS_H */ diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 202f59b..01866c3 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -29,6 +29,8 @@ enum uclass_id { UCLASS_SPI_FLASH, /* SPI flash */ UCLASS_CROS_EC, /* Chrome OS EC */ UCLASS_THERMAL, /* Thermal sensor */ + UCLASS_I2C, /* I2C bus */ + UCLASS_I2C_GENERIC, /* Generic I2C device */
UCLASS_COUNT, UCLASS_INVALID = -1, diff --git a/include/i2c.h b/include/i2c.h index 1b4078e..943490e 100644 --- a/include/i2c.h +++ b/include/i2c.h @@ -18,6 +18,352 @@ #define _I2C_H_
/* + * For now there are essentially two parts to this file - driver model + * here at the top, and the older code below (with CONFIG_SYS_I2C being + * most recent). The plan is to migrate everything to driver model. + * The driver model structures and API are separate as they are different + * enough as to be incompatible for compilation purposes. + */ + +#ifdef CONFIG_DM_I2C + +enum dm_i2c_chip_flags { + DM_I2C_CHIP_10BIT = 1 << 0, /* Use 10-bit addressing */ + DM_I2C_CHIP_RE_ADDRESS = 1 << 1, /* Send address for every byte */ +}; + +/** + * struct dm_i2c_chip - information about an i2c chip + * + * An I2C chip is a device on the I2C bus. It sits at a particular address + * and normally supports 7-bit or 10-bit addressing. + * + * To obtain this structure, use dev_get_parentdata(dev) where dev is the + * chip to examine. + * + * @chip_addr: Chip address on bus + * @offset_len: Length of offset in bytes. A single byte offset can + * represent up to 256 bytes. A value larger than 1 may be + * needed for larger devices. + * @flags: Flags for this chip (dm_i2c_chip_flags) + * @emul: Emulator for this chip address (only used for emulation) + */ +struct dm_i2c_chip { + uint chip_addr; + uint offset_len; + uint flags; +#ifdef CONFIG_SANDBOX + struct udevice *emul; +#endif +}; + +/** + * struct dm_i2c_bus- information about an i2c bus + * + * An I2C bus contains 0 or more chips on it, each at its own address. The + * bus can operate at different speeds (measured in Hz, typically 100KHz + * or 400KHz). + * + * To obtain this structure, use bus->uclass_priv where bus is the I2C + * bus udevice. + * + * @speed_hz: Bus speed in hertz (typically 100000) + */ +struct dm_i2c_bus { + int speed_hz; +}; + +/** + * i2c_read() - read bytes from an I2C chip + * + * To obtain an I2C device (called a 'chip') given the I2C bus address you + * can use i2c_get_chip(). To obtain a bus by bus number use + * uclass_get_device_by_seq(UCLASS_I2C, <bus number>). + * + * To set the address length of a devce use i2c_set_addr_len(). It + * defaults to 1. + * + * @dev: Chip to read from + * @offset: Offset within chip to start reading + * @buffer: Place to put data + * @len: Number of bytes to read + * + * @return 0 on success, -ve on failure + */ +int i2c_read(struct udevice *dev, uint offset, uint8_t *buffer, + int len); + +/** + * i2c_write() - write bytes to an I2C chip + * + * See notes for i2c_read() above. + * + * @dev: Chip to write to + * @offset: Offset within chip to start writing + * @buffer: Buffer containing data to write + * @len: Number of bytes to write + * + * @return 0 on success, -ve on failure + */ +int i2c_write(struct udevice *dev, uint offset, const uint8_t *buffer, + int len); + +/** + * i2c_probe() - probe a particular chip address + * + * This can be useful to check for the existence of a chip on the bus. + * It is typically implemented by writing the chip address to the bus + * and checking that the chip replies with an ACK. + * + * @bus: Bus to probe + * @chip_addr: 7-bit address to probe (10-bit and others are not supported) + * @chip_flags: Flags for the probe (see enum dm_i2c_chip_flags) + * @devp: Returns the device found, or NULL if none + * @return 0 if a chip was found at that address, -ve if not + */ +int i2c_probe(struct udevice *bus, uint chip_addr, uint chip_flags, + struct udevice **devp); + +/** + * i2c_set_bus_speed() - set the speed of a bus + * + * @bus: Bus to adjust + * @speed: Requested speed in Hz + * @return 0 if OK, -EINVAL for invalid values + */ +int i2c_set_bus_speed(struct udevice *bus, unsigned int speed); + +/** + * i2c_get_bus_speed() - get the speed of a bus + * + * @bus: Bus to check + * @return speed of selected I2C bus in Hz, -ve on error + */ +int i2c_get_bus_speed(struct udevice *bus); + +/** + * i2c_set_chip_flags() - set flags for a chip + * + * Typically addresses are 7 bits, but for 10-bit addresses you should set + * flags to DM_I2C_CHIP_10BIT. All accesses will then use 10-bit addressing. + * + * @dev: Chip to adjust + * @flags: New flags + * @return 0 if OK, -EINVAL if value is unsupported, other -ve value on error + */ +int i2c_set_chip_flags(struct udevice *dev, uint flags); + +/** + * i2c_get_chip_flags() - get flags for a chip + * + * @dev: Chip to check + * @flagsp: Place to put flags + * @return 0 if OK, other -ve value on error + */ +int i2c_get_chip_flags(struct udevice *dev, uint *flagsp); + +/** + * i2c_set_offset_len() - set the offset length for a chip + * + * The offset used to access a chip may be up to 4 bytes long. Typically it + * is only 1 byte, which is enough for chips with 256 bytes of memory or + * registers. The default value is 1, but you can call this function to + * change it. + * + * @offset_len: New offset length value (typically 1 or 2) + */ + +int i2c_set_chip_offset_len(struct udevice *dev, uint offset_len); +/** + * i2c_deblock() - recover a bus that is in an unknown state + * + * See the deblock() method in 'struct dm_i2c_ops' for full information + * + * @bus: Bus to recover + * @return 0 if OK, -ve on error + */ +int i2c_deblock(struct udevice *bus); + +/* + * Not all of these flags are implemented in the U-Boot API + */ +enum dm_i2c_msg_flags { + I2C_M_TEN = 0x0010, /* ten-bit chip address */ + I2C_M_RD = 0x0001, /* read data, from slave to master */ + I2C_M_STOP = 0x8000, /* send stop after this message */ + I2C_M_NOSTART = 0x4000, /* no start before this message */ + I2C_M_REV_DIR_ADDR = 0x2000, /* invert polarity of R/W bit */ + I2C_M_IGNORE_NAK = 0x1000, /* continue after NAK */ + I2C_M_NO_RD_ACK = 0x0800, /* skip the Ack bit on reads */ + I2C_M_RECV_LEN = 0x0400, /* length is first received byte */ +}; + +/** + * struct i2c_msg - an I2C message + * + * @addr: Slave address + * @flags: Flags (see enum dm_i2c_msg_flags) + * @len: Length of buffer in bytes + * @buf: Buffer to send/receive + */ +struct i2c_msg { + uint addr; + uint flags; + uint len; + u8 *buf; +}; + +/** + * struct i2c_msg_list - a list of I2C messages + * + * This is called i2c_rdwr_ioctl_data in Linux but the name does not seem + * appropriate in U-Boot. + * + * @msg: Pointer to i2c_msg array + * @nmsgs: Number of elements in the array + */ +struct i2c_msg_list { + struct i2c_msg *msgs; + uint nmsgs; +}; + +/** + * struct dm_i2c_ops - driver operations for I2C uclass + * + * Drivers should support these operations unless otherwise noted. These + * operations are intended to be used by uclass code, not directly from + * other code. + */ +struct dm_i2c_ops { + /** + * xfer() - transfer a list of I2C messages + * + * @bus: Bus to read from + * @msg: List of messages to transfer + * @nmsgs: Number of messages in the list + * @return 0 if OK, -EREMOTEIO if the slave did not ACK a byte, + * other -ve value on some other error + */ + int (*xfer)(struct udevice *bus, struct i2c_msg *msg, int nmsgs); + + /** + * probe_chip() - probe for the presense of a chip address + * + * This function is optional. If omitted, the uclass will send a zero + * length message instead. + * + * @bus: Bus to probe + * @chip_addr: Chip address to probe + * @chip_flags: Probe flags (enum dm_i2c_chip_flags) + * @return 0 if chip was found, -EREMOTEIO if not, -ENOSYS to fall back + * to default probem other -ve value on error + */ + int (*probe_chip)(struct udevice *bus, uint chip_addr, uint chip_flags); + + /** + * set_bus_speed() - set the speed of a bus (optional) + * + * The bus speed value will be updated by the uclass if this function + * does not return an error. This method is optional - if it is not + * provided then the driver can read the speed from + * bus->uclass_priv->speed_hz. + * + * @bus: Bus to adjust + * @speed: Requested speed in Hz + * @return 0 if OK, -INVAL for invalid values + */ + int (*set_bus_speed)(struct udevice *bus, unsigned int speed); + + /** + * get_bus_speed() - get the speed of a bus (optional) + * + * Normally this can be provided by the uclass, but if you want your + * driver to check the bus speed by looking at the hardware, you can + * implement that here. This method is optional. + * + * @bus: Bus to check + * @return speed of selected I2C bus in Hz, -ve on error + */ + int (*get_bus_speed)(struct udevice *bus); + + /** + * set_flags() - set the flags for a chip (optional) + * + * This is generally implemented by the uclass, but drivers can + * check the value to ensure that unsupported options are not used. + * This method is optional. If provided, this method will always be + * called when the flags change. + * + * @dev: Chip to adjust + * @flags: New flags value + * @return 0 if OK, -EINVAL if value is unsupported + */ + int (*set_flags)(struct udevice *dev, uint flags); + + /** + * deblock() - recover a bus that is in an unknown state + * + * I2C is a synchronous protocol and resets of the processor in the + * middle of an access can block the I2C Bus until a powerdown of + * the full unit is done. This is because slaves can be stuck + * waiting for addition bus transitions for a transaction that will + * never complete. Resetting the I2C master does not help. The only + * way is to force the bus through a series of transitions to make + * sure that all slaves are done with the transaction. This method + * performs this 'deblocking' if support by the driver. + * + * This method is optional. + */ + int (*deblock)(struct udevice *bus); +}; + +#define i2c_get_ops(dev) ((struct dm_i2c_ops *)(dev)->driver->ops) + +/** + * i2c_get_chip() - get a device to use to access a chip on a bus + * + * This returns the device for the given chip address. The device can then + * be used with calls to i2c_read(), i2c_write(), i2c_probe(), etc. + * + * @bus: Bus to examine + * @chip_addr: Chip address for the new device + * @devp: Returns pointer to new device if found or -ENODEV if not + * found + */ +int i2c_get_chip(struct udevice *bus, uint chip_addr, struct udevice **devp); + +/** + * i2c_get_chip() - get a device to use to access a chip on a bus number + * + * This returns the device for the given chip address on a particular bus + * number. + * + * @busnum: Bus number to examine + * @chip_addr: Chip address for the new device + * @devp: Returns pointer to new device if found or -ENODEV if not + * found + */ +int i2c_get_chip_for_busnum(int busnum, int chip_addr, struct udevice **devp); + +/** + * i2c_chip_ofdata_to_platdata() - Decode standard I2C platform data + * + * This decodes the chip address from a device tree node and puts it into + * its dm_i2c_chip structure. This should be called in your driver's + * ofdata_to_platdata() method. + * + * @blob: Device tree blob + * @node: Node offset to read from + * @spi: Place to put the decoded information + */ +int i2c_chip_ofdata_to_platdata(const void *blob, int node, + struct dm_i2c_chip *chip); + +#endif + +#ifndef CONFIG_DM_I2C + +/* * WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING * * The implementation MUST NOT use static or global variables if the @@ -451,4 +797,7 @@ int i2c_get_bus_num_fdt(int node); * @return 0 if port was reset, -1 if not found */ int i2c_reset_port_fdt(const void *blob, int node); + +#endif /* !CONFIG_DM_I2C */ + #endif /* _I2C_H_ */
Hi Simon,
Here are some comments on v4.
On Thu, 4 Dec 2014 21:21:20 -0700 Simon Glass sjg@chromium.org wrote:
+#define I2C_MAX_OFFSET_LEN 4
+/**
- i2c_setup_offset() - Set up a new message with a chip offset
- @chip: Chip to use
- @offset: Byte offset within chip
- @offset_buf: Place to put byte offset
- @msg: Message buffer
- @return 0 if OK, -EADDRNOTAVAIL if the offset length is 0. In that case the
- message is still set up but will not contain an offset.
- */
+static int i2c_setup_offset(struct dm_i2c_chip *chip, uint offset,
uint8_t offset_buf[], struct i2c_msg *msg)+{
- int offset_len;
- msg->addr = chip->chip_addr;
- msg->flags = chip->flags & DM_I2C_CHIP_10BIT ? I2C_M_TEN : 0;
- msg->len = chip->offset_len;
- msg->buf = offset_buf;
- if (!chip->offset_len)
return -EADDRNOTAVAIL;
I notice i2c_{read|write}_bytewise checks the return code of this function, but the normal one does not.
I think it seems a little bit strange.
Instead of the code above, can we put this here?
if (chip->offset_len > I2C_MAX_OFFSET_LEN) return -EADDRNOTAVAIL;
And then, the normal i2c_{read|write} should also check the return code of this.
if (!chip->offset_len) return -EADDRNOTAVAIL;
is specific to *_bytewise ones, so it can be moved to there.
- offset_len = chip->offset_len;
- while (offset_len--)
*offset_buf++ = offset >> (8 * offset_len);
We should be very careful about this code because buffer overrun happens if too big offset_len is given.
So, we should make sure that offset_len is no larger than I2C_MAX_OFFSET_LEN. (Or, you can pass the length of offset_buf[] to this function.)
I know you implemented a similar check check in i2c_set_chip_offset_len() function.
But users can skip i2c_set_chip_offset_len() and directly invoke i2c_read() or i2c_write(). This is very dangerous.
+int i2c_write(struct udevice *dev, uint offset, const uint8_t *buffer, int len) +{
- struct dm_i2c_chip *chip = dev_get_parentdata(dev);
- struct udevice *bus = dev_get_parent(dev);
- struct dm_i2c_ops *ops = i2c_get_ops(bus);
- struct i2c_msg msg[1];
- if (!ops->xfer)
return -ENOSYS;- if (chip->flags & DM_I2C_CHIP_RE_ADDRESS)
return i2c_write_bytewise(dev, offset, buffer, len);
Have you noticed that do_i2c_write() function in common/cmd_i2c.c always does the bytewise-equivalent behavior?
(It uses a while() loop and in each loop it calls i2c_write with length=1)
On the other hand, do_i2c_read() does not split it into small chunks.
At first I was wondering why only do_i2c_write() must split into many small transactions, but I got an answer when I was testing it on my board.
My on-board EEPROM chip does not work with long-burst write, but it works with any long-burst read.
It turned out some chips (at least mine) require DM_I2C_CHIP_RE_ADDRESS only for write.
Maybe, do we need to have a _RE_ADDRESS flag for each of write/read?
+static int i2c_probe_chip(struct udevice *bus, uint chip_addr,
enum dm_i2c_chip_flags flags)
I notice you added "flags" argument in v4.
When and how do we use this flag ?
for DM_I2C_CHIP_10BIT ??
+{
- struct dm_i2c_ops *ops = i2c_get_ops(bus);
- struct i2c_msg msg[1];
- uint8_t ch = 0;
- int ret;
- if (ops->probe_chip) {
ret = ops->probe_chip(bus, chip_addr, flags);if (!ret || ret != -ENOSYS)return ret;- }
- if (!ops->xfer)
return -ENOSYS;- /* Probe with a zero-length message */
- msg->addr = chip_addr;
- msg->flags = 0;
If my guess is correct, this line should be like this?
ptr->flags = chip->flags & DM_I2C_CHIP_10BIT ? I2C_M_TEN : 0;
I am not sure..
+/*
- i2c_get_bus_speed:
- Returns speed of selected I2C bus in Hz
- */
+int i2c_get_bus_speed(struct udevice *bus) +{
- struct dm_i2c_ops *ops = i2c_get_ops(bus);
- struct dm_i2c_bus *i2c = bus->uclass_priv;
- if (!ops->set_bus_speed)
return i2c->speed_hz;- return ops->get_bus_speed(bus);
+}
If ops->set_bus_speed is missing, ops->get_bus_speed is never called even if it exists. Isn't it a bit strange?
It is not clear to me why set_bus_speed must be checked.
Why isn't it like this?
int i2c_get_bus_speed(struct udevice *bus) { struct dm_i2c_ops *ops = i2c_get_ops(bus); struct dm_i2c_bus *i2c = bus->uclass_priv;
if (ops->get_bus_speed(bus) return ops->get_bus_speed(bus);
return i2c->speed_hz; }
When opt->set_bus_speed is missing, I think there are two possibilities: [1] Changing the bus speed is not supported [2] Hardware registers are set in .xfer handler
In case [2], somebody still might want to read the bus speed from the hardware register.
- /**
* set_bus_speed() - set the speed of a bus (optional)** The bus speed value will be updated by the uclass if this function* does not return an error. This method is optional - if it is not* provided then the driver can read the speed from* bus->uclass_priv->speed_hz.** @bus: Bus to adjust* @speed: Requested speed in Hz* @return 0 if OK, -INVAL for invalid values*/
I am afraid you missed what I pointed out in v3.
s/-INVAL/-EINVAL/
Best Regards Masahiro Yamada
Hi Masahiro,
On 5 December 2014 at 06:11, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
Hi Simon,
Here are some comments on v4.
On Thu, 4 Dec 2014 21:21:20 -0700 Simon Glass sjg@chromium.org wrote:
+#define I2C_MAX_OFFSET_LEN 4
+/**
- i2c_setup_offset() - Set up a new message with a chip offset
- @chip: Chip to use
- @offset: Byte offset within chip
- @offset_buf: Place to put byte offset
- @msg: Message buffer
- @return 0 if OK, -EADDRNOTAVAIL if the offset length is 0. In that case the
- message is still set up but will not contain an offset.
- */
+static int i2c_setup_offset(struct dm_i2c_chip *chip, uint offset,
uint8_t offset_buf[], struct i2c_msg *msg)+{
int offset_len;msg->addr = chip->chip_addr;msg->flags = chip->flags & DM_I2C_CHIP_10BIT ? I2C_M_TEN : 0;msg->len = chip->offset_len;msg->buf = offset_buf;if (!chip->offset_len)return -EADDRNOTAVAIL;I notice i2c_{read|write}_bytewise checks the return code of this function, but the normal one does not.
I think it seems a little bit strange.
Instead of the code above, can we put this here?
if (chip->offset_len > I2C_MAX_OFFSET_LEN) return -EADDRNOTAVAIL;And then, the normal i2c_{read|write} should also check the return code of this.
if (!chip->offset_len) return -EADDRNOTAVAIL;is specific to *_bytewise ones, so it can be moved to there.
We are not trying to check that the offset length is too large - that has already been done in ic_set_chip_offset_len(). I will add an assert and a comment. The purpose of the return value is to indicate whether an offset was added at all. In the case of a chip with a zero offset length, when doing a read, we want to omit the 'write' cycle entirely since there is no offset to write. The return value makes that possible.
offset_len = chip->offset_len;while (offset_len--)*offset_buf++ = offset >> (8 * offset_len);We should be very careful about this code because buffer overrun happens if too big offset_len is given.
So, we should make sure that offset_len is no larger than I2C_MAX_OFFSET_LEN. (Or, you can pass the length of offset_buf[] to this function.)
I know you implemented a similar check check in i2c_set_chip_offset_len() function.
But users can skip i2c_set_chip_offset_len() and directly invoke i2c_read() or i2c_write(). This is very dangerous.
If they skip it, then the value will be 1, the default. I've added an assert() but I don't really want to bloat the code which duplicate checks. Of course assert() is only useful for debugging.
+int i2c_write(struct udevice *dev, uint offset, const uint8_t *buffer, int len) +{
struct dm_i2c_chip *chip = dev_get_parentdata(dev);struct udevice *bus = dev_get_parent(dev);struct dm_i2c_ops *ops = i2c_get_ops(bus);struct i2c_msg msg[1];if (!ops->xfer)return -ENOSYS;if (chip->flags & DM_I2C_CHIP_RE_ADDRESS)return i2c_write_bytewise(dev, offset, buffer, len);Have you noticed that do_i2c_write() function in common/cmd_i2c.c always does the bytewise-equivalent behavior?
(It uses a while() loop and in each loop it calls i2c_write with length=1)
Yes, in fact someone submitted a patch for this recently.
http://patchwork.ozlabs.org/patch/415117/
I'd like to use the flags to support this at some point.
On the other hand, do_i2c_read() does not split it into small chunks.
At first I was wondering why only do_i2c_write() must split into many small transactions, but I got an answer when I was testing it on my board.
My on-board EEPROM chip does not work with long-burst write, but it works with any long-burst read.
It turned out some chips (at least mine) require DM_I2C_CHIP_RE_ADDRESS only for write.
Maybe, do we need to have a _RE_ADDRESS flag for each of write/read?
I was heading that way originally but then stopped when I wasn't sure of the use case. I think I will do this.
+static int i2c_probe_chip(struct udevice *bus, uint chip_addr,
enum dm_i2c_chip_flags flags)I notice you added "flags" argument in v4.
When and how do we use this flag ?
for DM_I2C_CHIP_10BIT ??
I commented the exported function i2c_probe() but not this, I'll add a comment.
+{
struct dm_i2c_ops *ops = i2c_get_ops(bus);struct i2c_msg msg[1];uint8_t ch = 0;int ret;if (ops->probe_chip) {ret = ops->probe_chip(bus, chip_addr, flags);if (!ret || ret != -ENOSYS)return ret;}if (!ops->xfer)return -ENOSYS;/* Probe with a zero-length message */msg->addr = chip_addr;msg->flags = 0;If my guess is correct, this line should be like this?
ptr->flags = chip->flags & DM_I2C_CHIP_10BIT ? I2C_M_TEN : 0;
I am not sure..
Yes we should do that, I'll add it.
+/*
- i2c_get_bus_speed:
- Returns speed of selected I2C bus in Hz
- */
+int i2c_get_bus_speed(struct udevice *bus) +{
struct dm_i2c_ops *ops = i2c_get_ops(bus);struct dm_i2c_bus *i2c = bus->uclass_priv;if (!ops->set_bus_speed)return i2c->speed_hz;return ops->get_bus_speed(bus);+}
If ops->set_bus_speed is missing, ops->get_bus_speed is never called even if it exists. Isn't it a bit strange?
It is not clear to me why set_bus_speed must be checked.
Why isn't it like this?
int i2c_get_bus_speed(struct udevice *bus) { struct dm_i2c_ops *ops = i2c_get_ops(bus); struct dm_i2c_bus *i2c = bus->uclass_priv;
if (ops->get_bus_speed(bus) return ops->get_bus_speed(bus); return i2c->speed_hz;}
When opt->set_bus_speed is missing, I think there are two possibilities: [1] Changing the bus speed is not supported [2] Hardware registers are set in .xfer handler
In case [2], somebody still might want to read the bus speed from the hardware register.
Well that's a strange case. But I think the simplest thing is to make these two completely independent as you suggest. I'll also document the return value for xfer() when the speed is wrong.
I'm not 100% sure of the use case for get_bus_speed() but I have seen hardware where 100KHz cannot be honoured exactly, or where clock change problems may cause the rate to change under your feet. So I'd like to keep it here.
/*** set_bus_speed() - set the speed of a bus (optional)** The bus speed value will be updated by the uclass if this function* does not return an error. This method is optional - if it is not* provided then the driver can read the speed from* bus->uclass_priv->speed_hz.** @bus: Bus to adjust* @speed: Requested speed in Hz* @return 0 if OK, -INVAL for invalid values*/I am afraid you missed what I pointed out in v3.
s/-INVAL/-EINVAL/
Fixed thanks.
Regards, Simon
Hi Simon, Sorry for my late reply.
On Fri, 5 Dec 2014 07:41:50 -0700 Simon Glass sjg@chromium.org wrote: n -EADDRNOTAVAIL;
I notice i2c_{read|write}_bytewise checks the return code of this function, but the normal one does not.
I think it seems a little bit strange.
Instead of the code above, can we put this here?
if (chip->offset_len > I2C_MAX_OFFSET_LEN) return -EADDRNOTAVAIL;And then, the normal i2c_{read|write} should also check the return code of this.
if (!chip->offset_len) return -EADDRNOTAVAIL;is specific to *_bytewise ones, so it can be moved to there.
We are not trying to check that the offset length is too large - that has already been done in ic_set_chip_offset_len(). I will add an assert and a comment. The purpose of the return value is to indicate whether an offset was added at all. In the case of a chip with a zero offset length, when doing a read, we want to omit the 'write' cycle entirely since there is no offset to write. The return value makes that possible.
I missed this. OK, I agreed that we should keep this as is.
offset_len = chip->offset_len;while (offset_len--)*offset_buf++ = offset >> (8 * offset_len);We should be very careful about this code because buffer overrun happens if too big offset_len is given.
So, we should make sure that offset_len is no larger than I2C_MAX_OFFSET_LEN. (Or, you can pass the length of offset_buf[] to this function.)
I know you implemented a similar check check in i2c_set_chip_offset_len() function.
But users can skip i2c_set_chip_offset_len() and directly invoke i2c_read() or i2c_write(). This is very dangerous.
If they skip it, then the value will be 1, the default. I've added an assert() but I don't really want to bloat the code which duplicate checks. Of course assert() is only useful for debugging.
You are right, thanks.
Best Regards Masahiro Yamada
The concept of a 'current bus' is now implemented in the command line rather than in the uclass. Also the address length does not need to be specified with each command - really we should consider dropping this from most commands but it works OK for now.
Signed-off-by: Simon Glass sjg@chromium.org Acked-by: Heiko Schocher hs@denx.de ---
Changes in v4: - Update call to i2c_probe()
Changes in v3: - Add new 'i2c flags' command to get/set chip flags - Adjust for slightly altered I2C uclass API
Changes in v2: - Call the deblock() method for 'i2c reset' - Change alen to int so that it can be -1 (this was a bug)
common/cmd_i2c.c | 376 +++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 323 insertions(+), 53 deletions(-)
diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c index c266b88..22db1bb 100644 --- a/common/cmd_i2c.c +++ b/common/cmd_i2c.c @@ -69,8 +69,10 @@ #include <bootretry.h> #include <cli.h> #include <command.h> +#include <dm.h> #include <edid.h> #include <environment.h> +#include <errno.h> #include <i2c.h> #include <malloc.h> #include <asm/byteorder.h> @@ -117,6 +119,60 @@ static uchar i2c_no_probes[] = CONFIG_SYS_I2C_NOPROBES;
#define DISP_LINE_LEN 16
+/* + * Default for driver model is to use the chip's existing address length. + * For legacy code, this is not stored, so we need to use a suitable + * default. + */ +#ifdef CONFIG_DM_I2C +#define DEFAULT_ADDR_LEN (-1) +#else +#define DEFAULT_ADDR_LEN 1 +#endif + +#ifdef CONFIG_DM_I2C +static struct udevice *i2c_cur_bus; + +static int i2c_set_bus_num(unsigned int busnum) +{ + struct udevice *bus; + int ret; + + ret = uclass_get_device_by_seq(UCLASS_I2C, busnum, &bus); + if (ret) { + debug("%s: No bus %d\n", __func__, busnum); + return ret; + } + i2c_cur_bus = bus; + + return 0; +} + +static int i2c_get_cur_bus(struct udevice **busp) +{ + if (!i2c_cur_bus) { + puts("No I2C bus selected\n"); + return -ENODEV; + } + *busp = i2c_cur_bus; + + return 0; +} + +static int i2c_get_cur_bus_chip(uint chip_addr, struct udevice **devp) +{ + struct udevice *bus; + int ret; + + ret = i2c_get_cur_bus(&bus); + if (ret) + return ret; + + return i2c_get_chip(bus, chip_addr, devp); +} + +#endif + /** * i2c_init_board() - Board-specific I2C bus init * @@ -143,7 +199,7 @@ void i2c_init_board(void) * * Returns I2C bus speed in Hz. */ -#if !defined(CONFIG_SYS_I2C) +#if !defined(CONFIG_SYS_I2C) && !defined(CONFIG_DM_I2C) /* * TODO: Implement architecture-specific get/set functions * Should go away, if we switched completely to new multibus support @@ -182,12 +238,12 @@ int i2c_set_bus_speed(unsigned int speed) * * Returns the address length. */ -static uint get_alen(char *arg) +static uint get_alen(char *arg, int default_len) { int j; int alen;
- alen = 1; + alen = default_len; for (j = 0; j < 8; j++) { if (arg[j] == '.') { alen = arg[j+1] - '0'; @@ -227,8 +283,13 @@ static int i2c_report_err(int ret, enum i2c_err_op op) static int do_i2c_read ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { u_char chip; - uint devaddr, alen, length; + uint devaddr, length; + int alen; u_char *memaddr; + int ret; +#ifdef CONFIG_DM_I2C + struct udevice *dev; +#endif
if (argc != 5) return CMD_RET_USAGE; @@ -243,7 +304,7 @@ static int do_i2c_read ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv * 2 bytes long. Some day it might be 3 bytes long :-). */ devaddr = simple_strtoul(argv[2], NULL, 16); - alen = get_alen(argv[2]); + alen = get_alen(argv[2], DEFAULT_ADDR_LEN); if (alen > 3) return CMD_RET_USAGE;
@@ -257,18 +318,31 @@ static int do_i2c_read ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv */ memaddr = (u_char *)simple_strtoul(argv[4], NULL, 16);
- if (i2c_read(chip, devaddr, alen, memaddr, length) != 0) { - i2c_report_err(-1, I2C_ERR_READ); - return 1; - } +#ifdef CONFIG_DM_I2C + ret = i2c_get_cur_bus_chip(chip, &dev); + if (!ret && alen != -1) + ret = i2c_set_chip_offset_len(dev, alen); + if (!ret) + ret = i2c_read(dev, devaddr, memaddr, length); +#else + ret = i2c_read(chip, devaddr, alen, memaddr, length); +#endif + if (ret) + return i2c_report_err(ret, I2C_ERR_READ); + return 0; }
static int do_i2c_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { u_char chip; - uint devaddr, alen, length; + uint devaddr, length; + int alen; u_char *memaddr; + int ret; +#ifdef CONFIG_DM_I2C + struct udevice *dev; +#endif
if (argc != 5) return cmd_usage(cmdtp); @@ -288,7 +362,7 @@ static int do_i2c_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[ * 2 bytes long. Some day it might be 3 bytes long :-). */ devaddr = simple_strtoul(argv[3], NULL, 16); - alen = get_alen(argv[3]); + alen = get_alen(argv[3], DEFAULT_ADDR_LEN); if (alen > 3) return cmd_usage(cmdtp);
@@ -297,10 +371,22 @@ static int do_i2c_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[ */ length = simple_strtoul(argv[4], NULL, 16);
+#ifdef CONFIG_DM_I2C + ret = i2c_get_cur_bus_chip(chip, &dev); + if (!ret && alen != -1) + ret = i2c_set_chip_offset_len(dev, alen); + if (ret) + return i2c_report_err(ret, I2C_ERR_WRITE); +#endif + while (length-- > 0) { - if (i2c_write(chip, devaddr++, alen, memaddr++, 1) != 0) { - return i2c_report_err(-1, I2C_ERR_WRITE); - } +#ifdef CONFIG_DM_I2C + ret = i2c_write(dev, devaddr++, memaddr++, 1); +#else + ret = i2c_write(chip, devaddr++, alen, memaddr++, 1); +#endif + if (ret) + return i2c_report_err(ret, I2C_ERR_WRITE); /* * No write delay with FRAM devices. */ @@ -311,6 +397,38 @@ static int do_i2c_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[ return 0; }
+#ifdef CONFIG_DM_I2C +static int do_i2c_flags(cmd_tbl_t *cmdtp, int flag, int argc, + char *const argv[]) +{ + struct udevice *dev; + uint flags; + int chip; + int ret; + + if (argc < 2) + return CMD_RET_USAGE; + + chip = simple_strtoul(argv[1], NULL, 16); + ret = i2c_get_cur_bus_chip(chip, &dev); + if (ret) + return i2c_report_err(ret, I2C_ERR_READ); + + if (argc > 2) { + flags = simple_strtoul(argv[2], NULL, 16); + ret = i2c_set_chip_flags(dev, flags); + } else { + ret = i2c_get_chip_flags(dev, &flags); + if (!ret) + printf("%x\n", flags); + } + if (ret) + return i2c_report_err(ret, I2C_ERR_READ); + + return 0; +} +#endif + /** * do_i2c_md() - Handle the "i2c md" command-line command * @cmdtp: Command data struct pointer @@ -327,8 +445,13 @@ static int do_i2c_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[ static int do_i2c_md ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { u_char chip; - uint addr, alen, length; + uint addr, length; + int alen; int j, nbytes, linebytes; + int ret; +#ifdef CONFIG_DM_I2C + struct udevice *dev; +#endif
/* We use the last specified parameters, unless new ones are * entered. @@ -356,7 +479,7 @@ static int do_i2c_md ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[] * 2 bytes long. Some day it might be 3 bytes long :-). */ addr = simple_strtoul(argv[2], NULL, 16); - alen = get_alen(argv[2]); + alen = get_alen(argv[2], DEFAULT_ADDR_LEN); if (alen > 3) return CMD_RET_USAGE;
@@ -368,6 +491,14 @@ static int do_i2c_md ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[] length = simple_strtoul(argv[3], NULL, 16); }
+#ifdef CONFIG_DM_I2C + ret = i2c_get_cur_bus_chip(chip, &dev); + if (!ret && alen != -1) + ret = i2c_set_chip_offset_len(dev, alen); + if (ret) + return i2c_report_err(ret, I2C_ERR_READ); +#endif + /* * Print the lines. * @@ -381,8 +512,13 @@ static int do_i2c_md ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
linebytes = (nbytes > DISP_LINE_LEN) ? DISP_LINE_LEN : nbytes;
- if (i2c_read(chip, addr, alen, linebuf, linebytes) != 0) - i2c_report_err(-1, I2C_ERR_READ); +#ifdef CONFIG_DM_I2C + ret = i2c_read(dev, addr, linebuf, linebytes); +#else + ret = i2c_read(chip, addr, alen, linebuf, linebytes); +#endif + if (ret) + i2c_report_err(ret, I2C_ERR_READ); else { printf("%04x:", addr); cp = linebuf; @@ -429,9 +565,13 @@ static int do_i2c_mw ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[] { uchar chip; ulong addr; - uint alen; + int alen; uchar byte; int count; + int ret; +#ifdef CONFIG_DM_I2C + struct udevice *dev; +#endif
if ((argc < 4) || (argc > 5)) return CMD_RET_USAGE; @@ -445,10 +585,17 @@ static int do_i2c_mw ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[] * Address is always specified. */ addr = simple_strtoul(argv[2], NULL, 16); - alen = get_alen(argv[2]); + alen = get_alen(argv[2], DEFAULT_ADDR_LEN); if (alen > 3) return CMD_RET_USAGE;
+#ifdef CONFIG_DM_I2C + ret = i2c_get_cur_bus_chip(chip, &dev); + if (!ret && alen != -1) + ret = i2c_set_chip_offset_len(dev, alen); + if (ret) + return i2c_report_err(ret, I2C_ERR_WRITE); +#endif /* * Value to write is always specified. */ @@ -463,8 +610,13 @@ static int do_i2c_mw ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[] count = 1;
while (count-- > 0) { - if (i2c_write(chip, addr++, alen, &byte, 1) != 0) - i2c_report_err(-1, I2C_ERR_WRITE); +#ifdef CONFIG_DM_I2C + ret = i2c_write(dev, addr++, &byte, 1); +#else + ret = i2c_write(chip, addr++, alen, &byte, 1); +#endif + if (ret) + i2c_report_err(ret, I2C_ERR_WRITE); /* * Wait for the write to complete. The write can take * up to 10mSec (we allow a little more time). @@ -499,11 +651,15 @@ static int do_i2c_crc (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[] { uchar chip; ulong addr; - uint alen; + int alen; int count; uchar byte; ulong crc; ulong err; + int ret = 0; +#ifdef CONFIG_DM_I2C + struct udevice *dev; +#endif
if (argc < 4) return CMD_RET_USAGE; @@ -517,10 +673,17 @@ static int do_i2c_crc (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[] * Address is always specified. */ addr = simple_strtoul(argv[2], NULL, 16); - alen = get_alen(argv[2]); + alen = get_alen(argv[2], DEFAULT_ADDR_LEN); if (alen > 3) return CMD_RET_USAGE;
+#ifdef CONFIG_DM_I2C + ret = i2c_get_cur_bus_chip(chip, &dev); + if (!ret && alen != -1) + ret = i2c_set_chip_offset_len(dev, alen); + if (ret) + return i2c_report_err(ret, I2C_ERR_READ); +#endif /* * Count is always specified */ @@ -534,13 +697,18 @@ static int do_i2c_crc (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[] crc = 0; err = 0; while (count-- > 0) { - if (i2c_read(chip, addr, alen, &byte, 1) != 0) +#ifdef CONFIG_DM_I2C + ret = i2c_read(dev, addr, &byte, 1); +#else + ret = i2c_read(chip, addr, alen, &byte, 1); +#endif + if (ret) err++; crc = crc32 (crc, &byte, 1); addr++; } if (err > 0) - i2c_report_err(-1, I2C_ERR_READ); + i2c_report_err(ret, I2C_ERR_READ); else printf ("%08lx\n", crc);
@@ -568,10 +736,14 @@ mod_i2c_mem(cmd_tbl_t *cmdtp, int incrflag, int flag, int argc, char * const arg { uchar chip; ulong addr; - uint alen; + int alen; ulong data; int size = 1; int nbytes; + int ret; +#ifdef CONFIG_DM_I2C + struct udevice *dev; +#endif
if (argc != 3) return CMD_RET_USAGE; @@ -601,19 +773,32 @@ mod_i2c_mem(cmd_tbl_t *cmdtp, int incrflag, int flag, int argc, char * const arg * Address is always specified. */ addr = simple_strtoul(argv[2], NULL, 16); - alen = get_alen(argv[2]); + alen = get_alen(argv[2], DEFAULT_ADDR_LEN); if (alen > 3) return CMD_RET_USAGE; }
+#ifdef CONFIG_DM_I2C + ret = i2c_get_cur_bus_chip(chip, &dev); + if (!ret && alen != -1) + ret = i2c_set_chip_offset_len(dev, alen); + if (ret) + return i2c_report_err(ret, I2C_ERR_WRITE); +#endif + /* * Print the address, followed by value. Then accept input for * the next value. A non-converted value exits. */ do { printf("%08lx:", addr); - if (i2c_read(chip, addr, alen, (uchar *)&data, size) != 0) - i2c_report_err(-1, I2C_ERR_READ); +#ifdef CONFIG_DM_I2C + ret = i2c_read(dev, addr, (uchar *)&data, size); +#else + ret = i2c_read(chip, addr, alen, (uchar *)&data, size); +#endif + if (ret) + i2c_report_err(ret, I2C_ERR_READ); else { data = cpu_to_be32(data); if (size == 1) @@ -655,8 +840,15 @@ mod_i2c_mem(cmd_tbl_t *cmdtp, int incrflag, int flag, int argc, char * const arg * good enough to not time out */ bootretry_reset_cmd_timeout(); - if (i2c_write(chip, addr, alen, (uchar *)&data, size) != 0) - i2c_report_err(-1, I2C_ERR_WRITE); +#ifdef CONFIG_DM_I2C + ret = i2c_write(dev, addr, (uchar *)&data, + size); +#else + ret = i2c_write(chip, addr, alen, + (uchar *)&data, size); +#endif + if (ret) + i2c_report_err(ret, I2C_ERR_WRITE); #ifdef CONFIG_SYS_EEPROM_PAGE_WRITE_DELAY_MS udelay(CONFIG_SYS_EEPROM_PAGE_WRITE_DELAY_MS * 1000); #endif @@ -697,6 +889,13 @@ static int do_i2c_probe (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv int k, skip; unsigned int bus = GET_BUS_NUM; #endif /* NOPROBES */ + int ret; +#ifdef CONFIG_DM_I2C + struct udevice *bus, *dev; + + if (i2c_get_cur_bus(&bus)) + return CMD_RET_FAILURE; +#endif
if (argc == 2) addr = simple_strtol(argv[1], 0, 16); @@ -717,7 +916,12 @@ static int do_i2c_probe (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv if (skip) continue; #endif - if (i2c_probe(j) == 0) { +#ifdef CONFIG_DM_I2C + ret = i2c_probe(bus, j, 0, &dev); +#else + ret = i2c_probe(j); +#endif + if (ret == 0) { printf(" %02X", j); found++; } @@ -754,11 +958,15 @@ static int do_i2c_probe (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv static int do_i2c_loop(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { u_char chip; - ulong alen; + int alen; uint addr; uint length; u_char bytes[16]; int delay; + int ret; +#ifdef CONFIG_DM_I2C + struct udevice *dev; +#endif
if (argc < 3) return CMD_RET_USAGE; @@ -772,9 +980,16 @@ static int do_i2c_loop(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[] * Address is always specified. */ addr = simple_strtoul(argv[2], NULL, 16); - alen = get_alen(argv[2]); + alen = get_alen(argv[2], DEFAULT_ADDR_LEN); if (alen > 3) return CMD_RET_USAGE; +#ifdef CONFIG_DM_I2C + ret = i2c_get_cur_bus_chip(chip, &dev); + if (!ret && alen != -1) + ret = i2c_set_chip_offset_len(dev, alen); + if (ret) + return i2c_report_err(ret, I2C_ERR_WRITE); +#endif
/* * Length is the number of objects, not number of bytes. @@ -794,8 +1009,13 @@ static int do_i2c_loop(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[] * Run the loop... */ while (1) { - if (i2c_read(chip, addr, alen, bytes, length) != 0) - i2c_report_err(-1, I2C_ERR_READ); +#ifdef CONFIG_DM_I2C + ret = i2c_read(dev, addr, bytes, length); +#else + ret = i2c_read(chip, addr, alen, bytes, length); +#endif + if (ret) + i2c_report_err(ret, I2C_ERR_READ); udelay(delay); }
@@ -1345,6 +1565,10 @@ int do_edid(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) { u_char chip; struct edid1_info edid; + int ret; +#ifdef CONFIG_DM_I2C + struct udevice *dev; +#endif
if (argc < 2) { cmd_usage(cmdtp); @@ -1352,10 +1576,15 @@ int do_edid(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) }
chip = simple_strtoul(argv[1], NULL, 16); - if (i2c_read(chip, 0, 1, (uchar *)&edid, sizeof(edid)) != 0) { - i2c_report_err(-1, I2C_ERR_READ); - return 1; - } +#ifdef CONFIG_DM_I2C + ret = i2c_get_cur_bus_chip(chip, &dev); + if (!ret) + ret = i2c_read(dev, 0, (uchar *)&edid, sizeof(edid)); +#else + ret = i2c_read(chip, 0, 1, (uchar *)&edid, sizeof(edid)); +#endif + if (ret) + return i2c_report_err(ret, I2C_ERR_READ);
if (edid_check_info(&edid)) { puts("Content isn't valid EDID.\n"); @@ -1437,17 +1666,28 @@ static int do_i2c_show_bus(cmd_tbl_t *cmdtp, int flag, int argc, * Returns zero on success, CMD_RET_USAGE in case of misuse and negative * on error. */ -#if defined(CONFIG_SYS_I2C) || defined(CONFIG_I2C_MULTI_BUS) +#if defined(CONFIG_SYS_I2C) || defined(CONFIG_I2C_MULTI_BUS) || \ + defined(CONFIG_DM_I2C) static int do_i2c_bus_num(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { int ret = 0; - unsigned int bus_no; + int bus_no;
- if (argc == 1) + if (argc == 1) { /* querying current setting */ - printf("Current bus is %d\n", i2c_get_bus_num()); - else { +#ifdef CONFIG_DM_I2C + struct udevice *bus; + + if (!i2c_get_cur_bus(&bus)) + bus_no = bus->seq; + else + bus_no = -1; +#else + bus_no = i2c_get_bus_num(); +#endif + printf("Current bus is %d\n", bus_no); + } else { bus_no = simple_strtoul(argv[1], NULL, 10); #if defined(CONFIG_SYS_I2C) if (bus_no >= CONFIG_SYS_NUM_I2C_BUSES) { @@ -1478,13 +1718,28 @@ static int do_i2c_bus_speed(cmd_tbl_t * cmdtp, int flag, int argc, char * const { int speed, ret=0;
- if (argc == 1) +#ifdef CONFIG_DM_I2C + struct udevice *bus; + + if (i2c_get_cur_bus(&bus)) + return 1; +#endif + if (argc == 1) { +#ifdef CONFIG_DM_I2C + speed = i2c_get_bus_speed(bus); +#else + speed = i2c_get_bus_speed(); +#endif /* querying current speed */ - printf("Current bus speed=%d\n", i2c_get_bus_speed()); - else { + printf("Current bus speed=%d\n", speed); + } else { speed = simple_strtoul(argv[1], NULL, 10); printf("Setting bus speed to %d Hz\n", speed); +#ifdef CONFIG_DM_I2C + ret = i2c_set_bus_speed(bus, speed); +#else ret = i2c_set_bus_speed(speed); +#endif if (ret) printf("Failure changing bus speed (%d)\n", ret); } @@ -1532,7 +1787,16 @@ static int do_i2c_nm(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) */ static int do_i2c_reset(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) { -#if defined(CONFIG_SYS_I2C) +#if defined(CONFIG_DM_I2C) + struct udevice *bus; + + if (i2c_get_cur_bus(&bus)) + return CMD_RET_FAILURE; + if (i2c_deblock(bus)) { + printf("Error: Not supported by the driver\n"); + return CMD_RET_FAILURE; + } +#elif defined(CONFIG_SYS_I2C) i2c_init(I2C_ADAP->speed, I2C_ADAP->slaveaddr); #else i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE); @@ -1546,7 +1810,7 @@ static cmd_tbl_t cmd_i2c_sub[] = { #endif U_BOOT_CMD_MKENT(crc32, 3, 1, do_i2c_crc, "", ""), #if defined(CONFIG_SYS_I2C) || \ - defined(CONFIG_I2C_MULTI_BUS) + defined(CONFIG_I2C_MULTI_BUS) || defined(CONFIG_DM_I2C) U_BOOT_CMD_MKENT(dev, 1, 1, do_i2c_bus_num, "", ""), #endif /* CONFIG_I2C_MULTI_BUS */ #if defined(CONFIG_I2C_EDID) @@ -1560,6 +1824,9 @@ static cmd_tbl_t cmd_i2c_sub[] = { U_BOOT_CMD_MKENT(probe, 0, 1, do_i2c_probe, "", ""), U_BOOT_CMD_MKENT(read, 5, 1, do_i2c_read, "", ""), U_BOOT_CMD_MKENT(write, 5, 0, do_i2c_write, "", ""), +#ifdef CONFIG_DM_I2C + U_BOOT_CMD_MKENT(flags, 2, 1, do_i2c_flags, "", ""), +#endif U_BOOT_CMD_MKENT(reset, 0, 1, do_i2c_reset, "", ""), #if defined(CONFIG_CMD_SDRAM) U_BOOT_CMD_MKENT(sdram, 1, 1, do_sdram, "", ""), @@ -1610,7 +1877,7 @@ static char i2c_help_text[] = #endif "crc32 chip address[.0, .1, .2] count - compute CRC32 checksum\n" #if defined(CONFIG_SYS_I2C) || \ - defined(CONFIG_I2C_MULTI_BUS) + defined(CONFIG_I2C_MULTI_BUS) || defined(CONFIG_DM_I2C) "i2c dev [dev] - show or set current I2C bus\n" #endif /* CONFIG_I2C_MULTI_BUS */ #if defined(CONFIG_I2C_EDID) @@ -1622,8 +1889,11 @@ static char i2c_help_text[] = "i2c mw chip address[.0, .1, .2] value [count] - write to I2C device (fill)\n" "i2c nm chip address[.0, .1, .2] - write to I2C device (constant address)\n" "i2c probe [address] - test for and show device(s) on the I2C bus\n" - "i2c read chip address[.0, .1, .2] length memaddress - read to memory \n" + "i2c read chip address[.0, .1, .2] length memaddress - read to memory\n" "i2c write memaddress chip address[.0, .1, .2] length - write memory to i2c\n" +#ifdef CONFIG_DM_I2C + "i2c flags chip [flags] - set or get chip flags\n" +#endif "i2c reset - re-init the I2C Controller\n" #if defined(CONFIG_CMD_SDRAM) "i2c sdram chip - print SDRAM configuration information\n"
In order to test I2C we need some sort of emulation interface. Add hooks to allow a driver to emulate an I2C device for sandbox.
Signed-off-by: Simon Glass sjg@chromium.org Acked-by: Heiko Schocher hs@denx.de ---
Changes in v4: None Changes in v3: None Changes in v2: None
drivers/i2c/Makefile | 1 + drivers/i2c/i2c-emul-uclass.c | 14 ++++++++++++++ include/dm/uclass-id.h | 1 + 3 files changed, 16 insertions(+) create mode 100644 drivers/i2c/i2c-emul-uclass.c
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile index 063e097..5a93473 100644 --- a/drivers/i2c/Makefile +++ b/drivers/i2c/Makefile @@ -27,6 +27,7 @@ obj-$(CONFIG_SYS_I2C_OMAP34XX) += omap24xx_i2c.o obj-$(CONFIG_SYS_I2C_PPC4XX) += ppc4xx_i2c.o obj-$(CONFIG_SYS_I2C_RCAR) += rcar_i2c.o obj-$(CONFIG_SYS_I2C_S3C24X0) += s3c24x0_i2c.o +obj-$(CONFIG_SYS_I2C_SANDBOX) += i2c-emul-uclass.o obj-$(CONFIG_SYS_I2C_SH) += sh_i2c.o obj-$(CONFIG_SYS_I2C_SOFT) += soft_i2c.o obj-$(CONFIG_SYS_I2C_TEGRA) += tegra_i2c.o diff --git a/drivers/i2c/i2c-emul-uclass.c b/drivers/i2c/i2c-emul-uclass.c new file mode 100644 index 0000000..aa89f95 --- /dev/null +++ b/drivers/i2c/i2c-emul-uclass.c @@ -0,0 +1,14 @@ +/* + * Copyright (c) 2014 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include <i2c.h> + +UCLASS_DRIVER(i2c_emul) = { + .id = UCLASS_I2C_EMUL, + .name = "i2c_emul", +}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 01866c3..16e4224 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -19,6 +19,7 @@ enum uclass_id { UCLASS_TEST_FDT, UCLASS_TEST_BUS, UCLASS_SPI_EMUL, /* sandbox SPI device emulator */ + UCLASS_I2C_EMUL, /* sandbox I2C device emulator */ UCLASS_SIMPLE_BUS,
/* U-Boot uclasses start here */
This driver includes some test features such as only supporting certain bus speeds. It passes its I2C traffic through to an emulator.
Acked-by: Heiko Schocher hs@denx.de Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v4: - Drop set_offset_len() method
Changes in v3: - Update for new uclass <=> driver interface
Changes in v2: None
drivers/i2c/Makefile | 2 +- drivers/i2c/sandbox_i2c.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 drivers/i2c/sandbox_i2c.c
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile index 5a93473..6f3c86c 100644 --- a/drivers/i2c/Makefile +++ b/drivers/i2c/Makefile @@ -27,7 +27,7 @@ obj-$(CONFIG_SYS_I2C_OMAP34XX) += omap24xx_i2c.o obj-$(CONFIG_SYS_I2C_PPC4XX) += ppc4xx_i2c.o obj-$(CONFIG_SYS_I2C_RCAR) += rcar_i2c.o obj-$(CONFIG_SYS_I2C_S3C24X0) += s3c24x0_i2c.o -obj-$(CONFIG_SYS_I2C_SANDBOX) += i2c-emul-uclass.o +obj-$(CONFIG_SYS_I2C_SANDBOX) += sandbox_i2c.o i2c-emul-uclass.o obj-$(CONFIG_SYS_I2C_SH) += sh_i2c.o obj-$(CONFIG_SYS_I2C_SOFT) += soft_i2c.o obj-$(CONFIG_SYS_I2C_TEGRA) += tegra_i2c.o diff --git a/drivers/i2c/sandbox_i2c.c b/drivers/i2c/sandbox_i2c.c new file mode 100644 index 0000000..c07308e --- /dev/null +++ b/drivers/i2c/sandbox_i2c.c @@ -0,0 +1,129 @@ +/* + * Simulate an I2C port + * + * Copyright (c) 2014 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include <errno.h> +#include <fdtdec.h> +#include <i2c.h> +#include <asm/test.h> +#include <dm/lists.h> +#include <dm/device-internal.h> + +DECLARE_GLOBAL_DATA_PTR; + +struct dm_sandbox_i2c_emul_priv { + struct udevice *emul; +}; + +static int get_emul(struct udevice *bus, uint chip_addr, struct udevice **devp, + struct dm_i2c_ops **opsp) +{ + const void *blob = gd->fdt_blob; + struct dm_i2c_chip *priv; + struct udevice *dev; + int ret; + + *devp = NULL; + *opsp = NULL; + ret = i2c_get_chip(bus, chip_addr, &dev); + if (ret) + return ret; + priv = dev_get_parentdata(dev); + if (!priv->emul) { + int node; + + debug("Scanning i2c bus '%s' for devices\n", dev->name); + for (node = fdt_first_subnode(blob, dev->of_offset); + node >= 0; + node = fdt_next_subnode(blob, node)) { + int ret; + + ret = lists_bind_fdt(dev, blob, node, &priv->emul); + if (ret) + return ret; + debug("Found emul '%s' for i2c device '%s'\n", + priv->emul->name, dev->name); + break; + } + } + + if (!priv->emul) + return -ENODEV; + ret = device_probe(priv->emul); + if (ret) + return ret; + *devp = priv->emul; + *opsp = i2c_get_ops(priv->emul); + + return 0; +} + +static int sandbox_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, + int nmsgs) +{ + struct dm_i2c_bus *i2c = bus->uclass_priv; + struct dm_i2c_ops *ops; + struct udevice *emul, *dev; + bool is_read; + int ret; + + /* Special test code to return success but with no emulation */ + if (msg->addr == SANDBOX_I2C_TEST_ADDR) + return 0; + + ret = get_emul(bus, msg->addr, &emul, &ops); + if (ret) + return ret; + + /* For testing, require an offset length of 1 */ + ret = i2c_get_chip(bus, msg->addr, &dev); + if (ret) + return ret; + + /* + * For testing, don't allow writing above 100KHz for writes and + * 400KHz for reads + */ + is_read = nmsgs > 1; + if (i2c->speed_hz > (is_read ? 400000 : 100000)) + return -EINVAL; + return ops->xfer(emul, msg, nmsgs); +} + +static const struct dm_i2c_ops sandbox_i2c_ops = { + .xfer = sandbox_i2c_xfer, +}; + +static int sandbox_i2c_child_pre_probe(struct udevice *dev) +{ + struct dm_i2c_chip *i2c_chip = dev_get_parentdata(dev); + + /* Ignore our test address */ + if (i2c_chip->chip_addr == SANDBOX_I2C_TEST_ADDR) + return 0; + if (dev->of_offset == -1) + return 0; + + return i2c_chip_ofdata_to_platdata(gd->fdt_blob, dev->of_offset, + i2c_chip); +} + +static const struct udevice_id sandbox_i2c_ids[] = { + { .compatible = "sandbox,i2c" }, + { } +}; + +U_BOOT_DRIVER(i2c_sandbox) = { + .name = "i2c_sandbox", + .id = UCLASS_I2C, + .of_match = sandbox_i2c_ids, + .per_child_auto_alloc_size = sizeof(struct dm_i2c_chip), + .child_pre_probe = sandbox_i2c_child_pre_probe, + .ops = &sandbox_i2c_ops, +};
To enable testing of I2C, add a simple I2C EEPROM simulator for sandbox. It supports reading and writing from a small data store.
Signed-off-by: Simon Glass sjg@chromium.org Acked-by: Heiko Schocher hs@denx.de ---
Changes in v4: - Add a probe_chip() method for the eeprom - Add a remove() method for the eeprom - Add an eeprom test mode - Add eeprom debugging output - Add support for eeprom offset length
Changes in v3: - Update emulator for new uclass interface
Changes in v2: None
arch/sandbox/include/asm/test.h | 26 ++++++ drivers/misc/Makefile | 3 + drivers/misc/i2c_eeprom_emul.c | 176 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 205 insertions(+) create mode 100644 arch/sandbox/include/asm/test.h create mode 100644 drivers/misc/i2c_eeprom_emul.c
diff --git a/arch/sandbox/include/asm/test.h b/arch/sandbox/include/asm/test.h new file mode 100644 index 0000000..25a0c85 --- /dev/null +++ b/arch/sandbox/include/asm/test.h @@ -0,0 +1,26 @@ +/* + * Test-related constants for sandbox + * + * Copyright (c) 2014 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef __ASM_TEST_H +#define __ASM_TEST_H + +/* The sandbox driver always permits an I2C device with this address */ +#define SANDBOX_I2C_TEST_ADDR 0x59 + +enum sandbox_i2c_eeprom_test_mode { + SIE_TEST_MODE_NONE, + /* Permits read/write of only one byte per I2C transaction */ + SIE_TEST_MODE_SINGLE_BYTE, +}; + +void sandbox_i2c_eeprom_set_test_mode(struct udevice *dev, + enum sandbox_i2c_eeprom_test_mode mode); + +void sandbox_i2c_eeprom_set_offset_len(struct udevice *dev, int offset_len); + +#endif diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 2f2e48f..ff02184 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -20,6 +20,9 @@ obj-$(CONFIG_MXC_OCOTP) += mxc_ocotp.o obj-$(CONFIG_MXS_OCOTP) += mxs_ocotp.o obj-$(CONFIG_NS87308) += ns87308.o obj-$(CONFIG_PDSP188x) += pdsp188x.o +ifdef CONFIG_DM_I2C +obj-$(CONFIG_SANDBOX) += i2c_eeprom_emul.o +endif obj-$(CONFIG_STATUS_LED) += status_led.o obj-$(CONFIG_TWL4030_LED) += twl4030_led.o obj-$(CONFIG_FSL_IFC) += fsl_ifc.o diff --git a/drivers/misc/i2c_eeprom_emul.c b/drivers/misc/i2c_eeprom_emul.c new file mode 100644 index 0000000..3e370aa --- /dev/null +++ b/drivers/misc/i2c_eeprom_emul.c @@ -0,0 +1,176 @@ +/* + * Simulate an I2C eeprom + * + * Copyright (c) 2014 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include <fdtdec.h> +#include <i2c.h> +#include <malloc.h> +#include <asm/test.h> + +#ifdef DEBUG +#define debug_buffer print_buffer +#else +#define debug_buffer(x, ...) +#endif + +DECLARE_GLOBAL_DATA_PTR; + +struct sandbox_i2c_flash_plat_data { + enum sandbox_i2c_eeprom_test_mode test_mode; + const char *filename; + int offset_len; /* Length of an offset in bytes */ + int size; /* Size of data buffer */ +}; + +struct sandbox_i2c_flash { + uint8_t *data; +}; + +void sandbox_i2c_eeprom_set_test_mode(struct udevice *dev, + enum sandbox_i2c_eeprom_test_mode mode) +{ + struct sandbox_i2c_flash_plat_data *plat = dev_get_platdata(dev); + + plat->test_mode = mode; +} + +void sandbox_i2c_eeprom_set_offset_len(struct udevice *dev, int offset_len) +{ + struct sandbox_i2c_flash_plat_data *plat = dev_get_platdata(dev); + + plat->offset_len = offset_len; +} + +static int sandbox_i2c_eeprom_xfer(struct udevice *emul, struct i2c_msg *msg, + int nmsgs) +{ + struct sandbox_i2c_flash *priv = dev_get_priv(emul); + uint offset = 0; + + debug("\n%s\n", __func__); + debug_buffer(0, priv->data, 1, 16, 0); + for (; nmsgs > 0; nmsgs--, msg++) { + struct sandbox_i2c_flash_plat_data *plat = + dev_get_platdata(emul); + int len; + u8 *ptr; + + if (!plat->size) + return -ENODEV; + if (msg->addr + msg->len > plat->size) { + debug("%s: Address %x, len %x is outside range 0..%x\n", + __func__, msg->addr, msg->len, plat->size); + return -EINVAL; + } + len = msg->len; + debug(" %s: msg->len=%d", + msg->flags & I2C_M_RD ? "read" : "write", + msg->len); + if (msg->flags & I2C_M_RD) { + if (plat->test_mode == SIE_TEST_MODE_SINGLE_BYTE) + len = 1; + debug(", offset %x, len %x: ", offset, len); + memcpy(msg->buf, priv->data + offset, len); + memset(msg->buf + len, '\xff', msg->len - len); + debug_buffer(0, msg->buf, 1, msg->len, 0); + } else if (len >= plat->offset_len) { + int i; + + ptr = msg->buf; + for (i = 0; i < plat->offset_len; i++, len--) + offset = (offset << 8) | *ptr++; + debug(", set offset %x: ", offset); + debug_buffer(0, msg->buf, 1, msg->len, 0); + if (plat->test_mode == SIE_TEST_MODE_SINGLE_BYTE) + len = min(len, 1); + + /* For testing, map offsets into our limited buffer */ + for (i = 24; i > 0; i -= 8) { + if (offset > (1 << i)) { + offset = (offset >> i) | + (offset & ((1 << i) - 1)); + offset += i; + } + } + memcpy(priv->data + offset, ptr, len); + } + } + debug_buffer(0, priv->data, 1, 16, 0); + + return 0; +} + +int sandbox_i2c_probe_chip(struct udevice *bus, uint chip_addr, uint chip_flags) +{ + /* Always let probe succeed */ + printf("%s: Detected probe\n", __func__); + return 0; +} + +struct dm_i2c_ops sandbox_i2c_emul_ops = { + .xfer = sandbox_i2c_eeprom_xfer, + .probe_chip = sandbox_i2c_probe_chip, +}; + +static int sandbox_i2c_eeprom_ofdata_to_platdata(struct udevice *dev) +{ + struct sandbox_i2c_flash_plat_data *plat = dev_get_platdata(dev); + + plat->size = fdtdec_get_int(gd->fdt_blob, dev->of_offset, + "sandbox,size", 32); + plat->filename = fdt_getprop(gd->fdt_blob, dev->of_offset, + "sandbox,filename", NULL); + if (!plat->filename) { + debug("%s: No filename for device '%s'\n", __func__, + dev->name); + return -EINVAL; + } + plat->test_mode = SIE_TEST_MODE_NONE; + plat->offset_len = 1; + + return 0; +} + +static int sandbox_i2c_eeprom_probe(struct udevice *dev) +{ + struct sandbox_i2c_flash_plat_data *plat = dev_get_platdata(dev); + struct sandbox_i2c_flash *priv = dev_get_priv(dev); + + priv->data = calloc(1, plat->size); + if (!priv->data) + return -ENOMEM; + + return 0; +} + +static int sandbox_i2c_eeprom_remove(struct udevice *dev) +{ + struct sandbox_i2c_flash *priv = dev_get_priv(dev); + + free(priv->data); + + return 0; +} + +static const struct udevice_id sandbox_i2c_ids[] = { + { .compatible = "sandbox,i2c-eeprom" }, + { } +}; + +U_BOOT_DRIVER(sandbox_i2c_emul) = { + .name = "sandbox_i2c_eeprom_emul", + .id = UCLASS_I2C_EMUL, + .of_match = sandbox_i2c_ids, + .ofdata_to_platdata = sandbox_i2c_eeprom_ofdata_to_platdata, + .probe = sandbox_i2c_eeprom_probe, + .remove = sandbox_i2c_eeprom_remove, + .priv_auto_alloc_size = sizeof(struct sandbox_i2c_flash), + .platdata_auto_alloc_size = sizeof(struct sandbox_i2c_flash_plat_data), + .ops = &sandbox_i2c_emul_ops, +};
Enable the options to bring up I2C on sandbox. Also enable all the available I2C commands for testing purposes.
Signed-off-by: Simon Glass sjg@chromium.org Acked-by: Heiko Schocher hs@denx.de ---
Changes in v4: None Changes in v3: None Changes in v2: None
include/configs/sandbox.h | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index 2b03841..657f751 100644 --- a/include/configs/sandbox.h +++ b/include/configs/sandbox.h @@ -112,6 +112,12 @@ #define CONFIG_SPI_FLASH_STMICRO #define CONFIG_SPI_FLASH_WINBOND
+#define CONFIG_DM_I2C +#define CONFIG_CMD_I2C +#define CONFIG_SYS_I2C_SANDBOX +#define CONFIG_I2C_EDID +#define CONFIG_I2C_EEPROM + /* Memory things - we don't really want a memory test */ #define CONFIG_SYS_LOAD_ADDR 0x00000000 #define CONFIG_SYS_MEMTEST_START 0x00100000
Add an I2C bus to the device tree, with an EEPROM emulator attached to one of the addresses.
Signed-off-by: Simon Glass sjg@chromium.org Acked-by: Heiko Schocher hs@denx.de ---
Changes in v4: None Changes in v3: None Changes in v2: None
arch/sandbox/dts/sandbox.dts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts index 7614715..11748ae 100644 --- a/arch/sandbox/dts/sandbox.dts +++ b/arch/sandbox/dts/sandbox.dts @@ -134,6 +134,23 @@ num-gpios = <20>; };
+ i2c@0 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0>; + compatible = "sandbox,i2c"; + clock-frequency = <400000>; + eeprom@2c { + reg = <0x2c>; + compatible = "i2c-eeprom"; + emul { + compatible = "sandbox,i2c-eeprom"; + sandbox,filename = "i2c.bin"; + sandbox,size = <128>; + }; + }; + }; + spi@0 { #address-cells = <1>; #size-cells = <0>;
There seem to be a few EEPROM drivers around - perhaps we should have a single standard one? This simple driver is used for sandbox testing, but could be pressed into more active service.
Signed-off-by: Simon Glass sjg@chromium.org Acked-by: Heiko Schocher hs@denx.de ---
Changes in v4: None Changes in v3: None Changes in v2: - Update commit message for EEPROM driver
drivers/misc/Makefile | 1 + drivers/misc/i2c_eeprom.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + include/i2c_eeprom.h | 19 ++++++++++++++++++ 4 files changed, 72 insertions(+) create mode 100644 drivers/misc/i2c_eeprom.c create mode 100644 include/i2c_eeprom.h
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index ff02184..6fa836f 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -15,6 +15,7 @@ obj-$(CONFIG_CROS_EC_SANDBOX) += cros_ec_sandbox.o obj-$(CONFIG_CROS_EC_SPI) += cros_ec_spi.o obj-$(CONFIG_FSL_IIM) += fsl_iim.o obj-$(CONFIG_GPIO_LED) += gpio_led.o +obj-$(CONFIG_I2C_EEPROM) += i2c_eeprom.o obj-$(CONFIG_FSL_MC9SDZ60) += mc9sdz60.o obj-$(CONFIG_MXC_OCOTP) += mxc_ocotp.o obj-$(CONFIG_MXS_OCOTP) += mxs_ocotp.o diff --git a/drivers/misc/i2c_eeprom.c b/drivers/misc/i2c_eeprom.c new file mode 100644 index 0000000..d0548ec --- /dev/null +++ b/drivers/misc/i2c_eeprom.c @@ -0,0 +1,51 @@ +/* + * Copyright (c) 2014 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include <i2c.h> +#include <i2c_eeprom.h> + +static int i2c_eeprom_read(struct udevice *dev, int offset, uint8_t *buf, + int size) +{ + return -ENODEV; +} + +static int i2c_eeprom_write(struct udevice *dev, int offset, + const uint8_t *buf, int size) +{ + return -ENODEV; +} + +struct i2c_eeprom_ops i2c_eeprom_std_ops = { + .read = i2c_eeprom_read, + .write = i2c_eeprom_write, +}; + +int i2c_eeprom_std_probe(struct udevice *dev) +{ + return 0; +} + +static const struct udevice_id i2c_eeprom_std_ids[] = { + { .compatible = "i2c-eeprom" }, + { } +}; + +U_BOOT_DRIVER(i2c_eeprom_std) = { + .name = "i2c_eeprom", + .id = UCLASS_I2C_EEPROM, + .of_match = i2c_eeprom_std_ids, + .probe = i2c_eeprom_std_probe, + .priv_auto_alloc_size = sizeof(struct i2c_eeprom), + .ops = &i2c_eeprom_std_ops, +}; + +UCLASS_DRIVER(i2c_eeprom) = { + .id = UCLASS_I2C_EEPROM, + .name = "i2c_eeprom", +}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 16e4224..f17c3c2 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -32,6 +32,7 @@ enum uclass_id { UCLASS_THERMAL, /* Thermal sensor */ UCLASS_I2C, /* I2C bus */ UCLASS_I2C_GENERIC, /* Generic I2C device */ + UCLASS_I2C_EEPROM, /* I2C EEPROM device */
UCLASS_COUNT, UCLASS_INVALID = -1, diff --git a/include/i2c_eeprom.h b/include/i2c_eeprom.h new file mode 100644 index 0000000..ea6c962 --- /dev/null +++ b/include/i2c_eeprom.h @@ -0,0 +1,19 @@ +/* + * Copyright (c) 2014 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef __I2C_EEPROM +#define __I2C_EEPROM + +struct i2c_eeprom_ops { + int (*read)(struct udevice *dev, int offset, uint8_t *buf, int size); + int (*write)(struct udevice *dev, int offset, const uint8_t *buf, + int size); +}; + +struct i2c_eeprom { +}; + +#endif
Add some basic tests to check that the system works as expected.
Signed-off-by: Simon Glass sjg@chromium.org Acked-by: Heiko Schocher hs@denx.de ---
Changes in v4: - Add an assert for non-null - Add tests for offset length - Adjust tests to match uclass changes
Changes in v3: - Update for new uclass <=> emulateor interface
Changes in v2: - Add a new asm/test.h header for tests in sandbox - Add a test for automatic binding of generic I2C devices
include/dm/ut.h | 12 ++++ test/dm/Makefile | 1 + test/dm/i2c.c | 210 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ test/dm/test.dts | 17 +++++ 4 files changed, 240 insertions(+) create mode 100644 test/dm/i2c.c
diff --git a/include/dm/ut.h b/include/dm/ut.h index fa9eac0..ec61465 100644 --- a/include/dm/ut.h +++ b/include/dm/ut.h @@ -89,6 +89,18 @@ void ut_failf(struct dm_test_state *dms, const char *fname, int line, } \ }
+/* Assert that a pointer is not NULL */ +#define ut_assertnonnull(expr) { \ + const void *val = (expr); \ + \ + if (val == NULL) { \ + ut_failf(dms, __FILE__, __LINE__, __func__, \ + #expr " = NULL", \ + "Expected non-null, got NULL"); \ + return -1; \ + } \ +} + /* Assert that an operation succeeds (returns 0) */ #define ut_assertok(cond) ut_asserteq(0, cond)
diff --git a/test/dm/Makefile b/test/dm/Makefile index 75d3d41..612aa95 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -20,4 +20,5 @@ ifneq ($(CONFIG_SANDBOX),) obj-$(CONFIG_DM_GPIO) += gpio.o obj-$(CONFIG_DM_SPI) += spi.o obj-$(CONFIG_DM_SPI_FLASH) += sf.o +obj-$(CONFIG_DM_I2C) += i2c.o endif diff --git a/test/dm/i2c.c b/test/dm/i2c.c new file mode 100644 index 0000000..594a9e1 --- /dev/null +++ b/test/dm/i2c.c @@ -0,0 +1,210 @@ +/* + * Copyright (C) 2013 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + * + * Note: Test coverage does not include 10-bit addressing + */ + +#include <common.h> +#include <dm.h> +#include <fdtdec.h> +#include <i2c.h> +#include <dm/device-internal.h> +#include <dm/test.h> +#include <dm/uclass-internal.h> +#include <dm/ut.h> +#include <dm/util.h> +#include <asm/state.h> +#include <asm/test.h> + +static const int busnum; +static const int chip = 0x2c; + +/* Test that we can find buses and chips */ +static int dm_test_i2c_find(struct dm_test_state *dms) +{ + struct udevice *bus, *dev; + const int no_chip = 0x10; + + ut_asserteq(-ENODEV, uclass_find_device_by_seq(UCLASS_I2C, busnum, + false, &bus)); + + /* + * i2c_post_bind() will bind devices to chip selects. Check this then + * remove the emulation and the slave device. + */ + ut_assertok(uclass_get_device_by_seq(UCLASS_I2C, busnum, &bus)); + ut_assertok(i2c_probe(bus, chip, 0, &dev)); + ut_asserteq(-ENODEV, i2c_probe(bus, no_chip, 0, &dev)); + ut_asserteq(-ENODEV, uclass_get_device_by_seq(UCLASS_I2C, 1, &bus)); + + return 0; +} +DM_TEST(dm_test_i2c_find, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); + +static int dm_test_i2c_read_write(struct dm_test_state *dms) +{ + struct udevice *bus, *dev; + uint8_t buf[5]; + + ut_assertok(uclass_get_device_by_seq(UCLASS_I2C, busnum, &bus)); + ut_assertok(i2c_get_chip(bus, chip, &dev)); + ut_assertok(i2c_read(dev, 0, buf, 5)); + ut_assertok(memcmp(buf, "\0\0\0\0\0", sizeof(buf))); + ut_assertok(i2c_write(dev, 2, (uint8_t *)"AB", 2)); + ut_assertok(i2c_read(dev, 0, buf, 5)); + ut_assertok(memcmp(buf, "\0\0AB\0", sizeof(buf))); + + return 0; +} +DM_TEST(dm_test_i2c_read_write, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); + +static int dm_test_i2c_speed(struct dm_test_state *dms) +{ + struct udevice *bus, *dev; + uint8_t buf[5]; + + ut_assertok(uclass_get_device_by_seq(UCLASS_I2C, busnum, &bus)); + ut_assertok(i2c_get_chip(bus, chip, &dev)); + ut_assertok(i2c_set_bus_speed(bus, 100000)); + ut_assertok(i2c_read(dev, 0, buf, 5)); + ut_assertok(i2c_set_bus_speed(bus, 400000)); + ut_asserteq(400000, i2c_get_bus_speed(bus)); + ut_assertok(i2c_read(dev, 0, buf, 5)); + ut_asserteq(-EINVAL, i2c_write(dev, 0, buf, 5)); + + return 0; +} +DM_TEST(dm_test_i2c_speed, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); + +static int dm_test_i2c_offset_len(struct dm_test_state *dms) +{ + struct udevice *bus, *dev; + uint8_t buf[5]; + + ut_assertok(uclass_get_device_by_seq(UCLASS_I2C, busnum, &bus)); + ut_assertok(i2c_get_chip(bus, chip, &dev)); + ut_assertok(i2c_set_chip_offset_len(dev, 1)); + ut_assertok(i2c_read(dev, 0, buf, 5)); + + /* This is not supported by the uclass */ + ut_asserteq(-EINVAL, i2c_set_chip_offset_len(dev, 5)); + + return 0; +} +DM_TEST(dm_test_i2c_offset_len, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); + +static int dm_test_i2c_probe_empty(struct dm_test_state *dms) +{ + struct udevice *bus, *dev; + + ut_assertok(uclass_get_device_by_seq(UCLASS_I2C, busnum, &bus)); + ut_assertok(i2c_probe(bus, SANDBOX_I2C_TEST_ADDR, 0, &dev)); + + return 0; +} +DM_TEST(dm_test_i2c_probe_empty, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); + +static int dm_test_i2c_bytewise(struct dm_test_state *dms) +{ + struct udevice *bus, *dev; + struct udevice *eeprom; + uint8_t buf[5]; + + ut_assertok(uclass_get_device_by_seq(UCLASS_I2C, busnum, &bus)); + ut_assertok(i2c_get_chip(bus, chip, &dev)); + ut_assertok(i2c_read(dev, 0, buf, 5)); + ut_assertok(memcmp(buf, "\0\0\0\0\0", sizeof(buf))); + + /* Tell the EEPROM to only read/write one register at a time */ + ut_assertok(uclass_first_device(UCLASS_I2C_EMUL, &eeprom)); + ut_assertnonnull(eeprom); + sandbox_i2c_eeprom_set_test_mode(eeprom, SIE_TEST_MODE_SINGLE_BYTE); + + /* Now we only get the first byte - the rest will be 0xff */ + ut_assertok(i2c_read(dev, 0, buf, 5)); + ut_assertok(memcmp(buf, "\0\xff\xff\xff\xff", sizeof(buf))); + + /* If we do a separate transaction for each byte, it works */ + ut_assertok(i2c_set_chip_flags(dev, DM_I2C_CHIP_RE_ADDRESS)); + ut_assertok(i2c_read(dev, 0, buf, 5)); + ut_assertok(memcmp(buf, "\0\0\0\0\0", sizeof(buf))); + + /* This will only write A */ + ut_assertok(i2c_set_chip_flags(dev, 0)); + ut_assertok(i2c_write(dev, 2, (uint8_t *)"AB", 2)); + ut_assertok(i2c_read(dev, 0, buf, 5)); + ut_assertok(memcmp(buf, "\0\xff\xff\xff\xff", sizeof(buf))); + + /* Check that the B was ignored */ + ut_assertok(i2c_set_chip_flags(dev, DM_I2C_CHIP_RE_ADDRESS)); + ut_assertok(i2c_read(dev, 0, buf, 5)); + ut_assertok(memcmp(buf, "\0\0A\0\0\0", sizeof(buf))); + + /* Now write it again with the new flags, it should work */ + ut_assertok(i2c_write(dev, 2, (uint8_t *)"AB", 2)); + ut_assertok(i2c_read(dev, 0, buf, 5)); + ut_assertok(memcmp(buf, "\0\0AB\0\0", sizeof(buf))); + + /* Restore defaults */ + sandbox_i2c_eeprom_set_test_mode(eeprom, SIE_TEST_MODE_NONE); + ut_assertok(i2c_set_chip_flags(dev, 0)); + + return 0; +} +DM_TEST(dm_test_i2c_bytewise, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); + +static int dm_test_i2c_offset(struct dm_test_state *dms) +{ + struct udevice *eeprom; + struct udevice *dev; + uint8_t buf[5]; + + ut_assertok(i2c_get_chip_for_busnum(busnum, chip, &dev)); + + /* Do a transfer so we can find the emulator */ + ut_assertok(i2c_read(dev, 0, buf, 5)); + ut_assertok(uclass_first_device(UCLASS_I2C_EMUL, &eeprom)); + + /* Offset length 0 */ + sandbox_i2c_eeprom_set_offset_len(eeprom, 0); + ut_assertok(i2c_set_chip_offset_len(dev, 0)); + ut_assertok(i2c_write(dev, 10 /* ignored */, (uint8_t *)"AB", 2)); + ut_assertok(i2c_read(dev, 0, buf, 5)); + ut_assertok(memcmp(buf, "AB\0\0\0\0", sizeof(buf))); + + /* Offset length 1 */ + sandbox_i2c_eeprom_set_offset_len(eeprom, 1); + ut_assertok(i2c_set_chip_offset_len(dev, 1)); + ut_assertok(i2c_write(dev, 2, (uint8_t *)"AB", 2)); + ut_assertok(i2c_read(dev, 0, buf, 5)); + ut_assertok(memcmp(buf, "ABAB\0", sizeof(buf))); + + /* Offset length 2 */ + sandbox_i2c_eeprom_set_offset_len(eeprom, 2); + ut_assertok(i2c_set_chip_offset_len(dev, 2)); + ut_assertok(i2c_write(dev, 0x210, (uint8_t *)"AB", 2)); + ut_assertok(i2c_read(dev, 0x210, buf, 5)); + ut_assertok(memcmp(buf, "AB\0\0\0", sizeof(buf))); + + /* Offset length 3 */ + sandbox_i2c_eeprom_set_offset_len(eeprom, 2); + ut_assertok(i2c_set_chip_offset_len(dev, 2)); + ut_assertok(i2c_write(dev, 0x410, (uint8_t *)"AB", 2)); + ut_assertok(i2c_read(dev, 0x410, buf, 5)); + ut_assertok(memcmp(buf, "AB\0\0\0", sizeof(buf))); + + /* Offset length 4 */ + sandbox_i2c_eeprom_set_offset_len(eeprom, 2); + ut_assertok(i2c_set_chip_offset_len(dev, 2)); + ut_assertok(i2c_write(dev, 0x420, (uint8_t *)"AB", 2)); + ut_assertok(i2c_read(dev, 0x420, buf, 5)); + ut_assertok(memcmp(buf, "AB\0\0\0", sizeof(buf))); + + /* Restore defaults */ + sandbox_i2c_eeprom_set_offset_len(eeprom, 1); + + return 0; +} +DM_TEST(dm_test_i2c_offset, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); diff --git a/test/dm/test.dts b/test/dm/test.dts index 1fba792..fb0272a 100644 --- a/test/dm/test.dts +++ b/test/dm/test.dts @@ -93,6 +93,23 @@ num-gpios = <10>; };
+ i2c@0 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0>; + compatible = "sandbox,i2c"; + clock-frequency = <100000>; + eeprom@2c { + reg = <0x2c>; + compatible = "i2c-eeprom"; + emul { + compatible = "sandbox,i2c-eeprom"; + sandbox,filename = "i2c.bin"; + sandbox,size = <256>; + }; + }; + }; + spi@0 { #address-cells = <1>; #size-cells = <0>;
Some of these are missing a newline. Add it.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v4: - Add new patch to add newline to debug() messages
Changes in v3: None Changes in v2: None
drivers/core/device.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/core/device.c b/drivers/core/device.c index 6793e1c..963b16f 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -234,7 +234,7 @@ int device_probe(struct udevice *dev) void *dev_get_platdata(struct udevice *dev) { if (!dev) { - dm_warn("%s: null device", __func__); + dm_warn("%s: null device\n", __func__); return NULL; }
@@ -244,7 +244,7 @@ void *dev_get_platdata(struct udevice *dev) void *dev_get_priv(struct udevice *dev) { if (!dev) { - dm_warn("%s: null device", __func__); + dm_warn("%s: null device\n", __func__); return NULL; }
@@ -254,7 +254,7 @@ void *dev_get_priv(struct udevice *dev) void *dev_get_parentdata(struct udevice *dev) { if (!dev) { - dm_warn("%s: null device", __func__); + dm_warn("%s: null device\n", __func__); return NULL; }
This converts all Tegra boards over to use driver model for I2C. The driver is adjusted to use driver model and the following obsolete CONFIGs are removed:
- CONFIG_SYS_I2C_INIT_BOARD - CONFIG_I2C_MULTI_BUS - CONFIG_SYS_MAX_I2C_BUS - CONFIG_SYS_I2C_SPEED - CONFIG_SYS_I2C
This has been tested on: - trimslice (no I2C) - beaver - Jetson-TK1
It has not been tested on Tegra 114 as I don't have that board.
Acked-by: Heiko Schocher hs@denx.de Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v4: - Add probe_chip() method
Changes in v3: - Change driver to support new message-based I2C uclass API
Changes in v2: - Adjust tegra_i2c_child_pre_probe() to permit generic I2C devices - Correct the compatible strings for I2C buses - Don't init if the speed is 0, since this breaks the controller - Expand coverage to all Tegra boards
arch/arm/cpu/tegra20-common/pmu.c | 21 +- arch/arm/dts/tegra124-jetson-tk1.dts | 1 - arch/arm/dts/tegra30-tec-ng.dts | 4 + arch/arm/include/asm/arch-tegra/tegra_i2c.h | 2 +- board/avionic-design/common/tamonten-ng.c | 12 +- board/nvidia/cardhu/cardhu.c | 13 +- board/nvidia/common/board.c | 4 - board/nvidia/dalmore/dalmore.c | 21 +- board/nvidia/whistler/whistler.c | 29 ++- board/toradex/apalis_t30/apalis_t30.c | 19 +- drivers/i2c/tegra_i2c.c | 366 +++++++++------------------- drivers/power/tps6586x.c | 27 +- include/configs/apalis_t30.h | 3 - include/configs/beaver.h | 3 - include/configs/cardhu.h | 5 - include/configs/colibri_t30.h | 3 - include/configs/dalmore.h | 5 - include/configs/jetson-tk1.h | 5 - include/configs/nyan-big.h | 5 - include/configs/seaboard.h | 3 - include/configs/tec-ng.h | 5 - include/configs/tegra-common.h | 1 + include/configs/tegra114-common.h | 3 - include/configs/tegra124-common.h | 3 - include/configs/tegra20-common.h | 3 - include/configs/tegra30-common.h | 3 - include/configs/trimslice.h | 3 - include/configs/venice2.h | 5 - include/configs/whistler.h | 3 - include/tps6586x.h | 4 +- 30 files changed, 210 insertions(+), 374 deletions(-)
diff --git a/arch/arm/cpu/tegra20-common/pmu.c b/arch/arm/cpu/tegra20-common/pmu.c index c595f70..36a76a2 100644 --- a/arch/arm/cpu/tegra20-common/pmu.c +++ b/arch/arm/cpu/tegra20-common/pmu.c @@ -6,6 +6,7 @@ */
#include <common.h> +#include <i2c.h> #include <tps6586x.h> #include <asm/io.h> #include <asm/arch/tegra.h> @@ -23,9 +24,13 @@ #define VDD_TRANSITION_STEP 0x06 /* 150mv */ #define VDD_TRANSITION_RATE 0x06 /* 3.52mv/us */
+#define PMI_I2C_ADDRESS 0x34 /* chip requires this address */ + int pmu_set_nominal(void) { - int core, cpu, bus; + struct udevice *bus, *dev; + int core, cpu; + int ret;
/* by default, the table has been filled with T25 settings */ switch (tegra_get_chip_sku()) { @@ -42,12 +47,18 @@ int pmu_set_nominal(void) return -1; }
- bus = tegra_i2c_get_dvc_bus_num(); - if (bus == -1) { + ret = tegra_i2c_get_dvc_bus(&bus); + if (ret) { debug("%s: Cannot find DVC I2C bus\n", __func__); - return -1; + return ret; } - tps6586x_init(bus); + ret = i2c_get_chip(bus, PMI_I2C_ADDRESS, &dev); + if (ret) { + debug("%s: Cannot find DVC I2C chip\n", __func__); + return ret; + } + + tps6586x_init(dev); tps6586x_set_pwm_mode(TPS6586X_PWM_SM1); return tps6586x_adjust_sm0_sm1(core, cpu, VDD_TRANSITION_STEP, VDD_TRANSITION_RATE, VDD_RELATION); diff --git a/arch/arm/dts/tegra124-jetson-tk1.dts b/arch/arm/dts/tegra124-jetson-tk1.dts index ffad116..f6fe9a0 100644 --- a/arch/arm/dts/tegra124-jetson-tk1.dts +++ b/arch/arm/dts/tegra124-jetson-tk1.dts @@ -16,7 +16,6 @@ i2c2 = "/i2c@7000c400"; i2c3 = "/i2c@7000c500"; i2c4 = "/i2c@7000c700"; - i2c5 = "/i2c@7000d100"; sdhci0 = "/sdhci@700b0600"; sdhci1 = "/sdhci@700b0400"; spi0 = "/spi@7000d400"; diff --git a/arch/arm/dts/tegra30-tec-ng.dts b/arch/arm/dts/tegra30-tec-ng.dts index 8a69e81..e924acc 100644 --- a/arch/arm/dts/tegra30-tec-ng.dts +++ b/arch/arm/dts/tegra30-tec-ng.dts @@ -6,6 +6,10 @@ model = "Avionic Design Tamonten™ NG Evaluation Carrier"; compatible = "ad,tec-ng", "nvidia,tegra30";
+ aliases { + i2c0 = "/i2c@7000c400"; + }; + /* GEN2 */ i2c@7000c400 { status = "okay"; diff --git a/arch/arm/include/asm/arch-tegra/tegra_i2c.h b/arch/arm/include/asm/arch-tegra/tegra_i2c.h index 7ca6907..eeeb247 100644 --- a/arch/arm/include/asm/arch-tegra/tegra_i2c.h +++ b/arch/arm/include/asm/arch-tegra/tegra_i2c.h @@ -167,6 +167,6 @@ struct i2c_ctlr { * * @return number of bus, or -1 if there is no DVC active */ -int tegra_i2c_get_dvc_bus_num(void); +int tegra_i2c_get_dvc_bus(struct udevice **busp);
#endif /* _TEGRA_I2C_H_ */ diff --git a/board/avionic-design/common/tamonten-ng.c b/board/avionic-design/common/tamonten-ng.c index 5870b95..86a0844 100644 --- a/board/avionic-design/common/tamonten-ng.c +++ b/board/avionic-design/common/tamonten-ng.c @@ -6,6 +6,7 @@ */
#include <common.h> +#include <dm.h> #include <asm/arch/pinmux.h> #include <asm/arch/gp_padctrl.h> #include <asm/arch/gpio.h> @@ -51,8 +52,15 @@ void gpio_early_init(void)
void pmu_write(uchar reg, uchar data) { - i2c_set_bus_num(4); /* PMU is on bus 4 */ - i2c_write(PMU_I2C_ADDRESS, reg, 1, &data, 1); + struct udevice *dev; + int ret; + + ret = i2c_get_chip_for_busnum(4, PMU_I2C_ADDRESS, &dev); + if (ret) { + debug("%s: Cannot find PMIC I2C chip\n", __func__); + return; + } + i2c_write(dev, reg, &data, 1); }
/* diff --git a/board/nvidia/cardhu/cardhu.c b/board/nvidia/cardhu/cardhu.c index cc0e5e1..026f45c 100644 --- a/board/nvidia/cardhu/cardhu.c +++ b/board/nvidia/cardhu/cardhu.c @@ -6,6 +6,7 @@ */
#include <common.h> +#include <dm.h> #include <asm/arch/pinmux.h> #include <asm/arch/gp_padctrl.h> #include "pinmux-config-cardhu.h" @@ -37,17 +38,23 @@ void pinmux_init(void) */ void board_sdmmc_voltage_init(void) { + struct udevice *dev; uchar reg, data_buffer[1]; + int ret; int i;
- i2c_set_bus_num(0); /* PMU is on bus 0 */ + ret = i2c_get_chip_for_busnum(0, PMU_I2C_ADDRESS, &dev); + if (ret) { + debug("%s: Cannot find PMIC I2C chip\n", __func__); + return; + }
/* TPS659110: LDO5_REG = 3.3v, ACTIVE to SDMMC1 */ data_buffer[0] = 0x65; reg = 0x32;
for (i = 0; i < MAX_I2C_RETRY; ++i) { - if (i2c_write(PMU_I2C_ADDRESS, reg, 1, data_buffer, 1)) + if (i2c_write(dev, reg, data_buffer, 1)) udelay(100); }
@@ -56,7 +63,7 @@ void board_sdmmc_voltage_init(void) reg = 0x67;
for (i = 0; i < MAX_I2C_RETRY; ++i) { - if (i2c_write(PMU_I2C_ADDRESS, reg, 1, data_buffer, 1)) + if (i2c_write(dev, reg, data_buffer, 1)) udelay(100); } } diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c index 0e4a65a..4bdbf01 100644 --- a/board/nvidia/common/board.c +++ b/board/nvidia/common/board.c @@ -113,10 +113,6 @@ int board_init(void) power_det_init();
#ifdef CONFIG_SYS_I2C_TEGRA -#ifndef CONFIG_SYS_I2C_INIT_BOARD -#error "You must define CONFIG_SYS_I2C_INIT_BOARD to use i2c on Nvidia boards" -#endif - i2c_init_board(); # ifdef CONFIG_TEGRA_PMU if (pmu_set_nominal()) debug("Failed to select nominal voltages\n"); diff --git a/board/nvidia/dalmore/dalmore.c b/board/nvidia/dalmore/dalmore.c index f2d05af..2a73746 100644 --- a/board/nvidia/dalmore/dalmore.c +++ b/board/nvidia/dalmore/dalmore.c @@ -15,6 +15,7 @@ */
#include <common.h> +#include <dm.h> #include <asm/arch/pinmux.h> #include <asm/arch/gp_padctrl.h> #include "pinmux-config-dalmore.h" @@ -50,18 +51,21 @@ void pinmux_init(void) */ void board_sdmmc_voltage_init(void) { + struct udevice *dev; uchar reg, data_buffer[1]; int ret;
- ret = i2c_set_bus_num(0);/* PMU is on bus 0 */ - if (ret) - printf("%s: i2c_set_bus_num returned %d\n", __func__, ret); + ret = i2c_get_chip_for_busnum(0, PMU_I2C_ADDRESS, &dev); + if (ret) { + debug("%s: Cannot find PMIC I2C chip\n", __func__); + return; + }
/* TPS65913: LDO9_VOLTAGE = 3.3V */ data_buffer[0] = 0x31; reg = 0x61;
- ret = i2c_write(PMU_I2C_ADDRESS, reg, 1, data_buffer, 1); + ret = i2c_write(dev, reg, data_buffer, 1); if (ret) printf("%s: PMU i2c_write %02X<-%02X returned %d\n", __func__, reg, data_buffer[0], ret); @@ -70,7 +74,7 @@ void board_sdmmc_voltage_init(void) data_buffer[0] = 0x01; reg = 0x60;
- ret = i2c_write(PMU_I2C_ADDRESS, reg, 1, data_buffer, 1); + ret = i2c_write(dev, reg, data_buffer, 1); if (ret) printf("%s: PMU i2c_write %02X<-%02X returned %d\n", __func__, reg, data_buffer[0], ret); @@ -79,7 +83,12 @@ void board_sdmmc_voltage_init(void) data_buffer[0] = 0x03; reg = 0x14;
- ret = i2c_write(BAT_I2C_ADDRESS, reg, 1, data_buffer, 1); + ret = i2c_get_chip_for_busnum(0, BAT_I2C_ADDRESS, &dev); + if (ret) { + debug("%s: Cannot find charger I2C chip\n", __func__); + return; + } + ret = i2c_write(dev, reg, data_buffer, 1); if (ret) printf("%s: BAT i2c_write %02X<-%02X returned %d\n", __func__, reg, data_buffer[0], ret); diff --git a/board/nvidia/whistler/whistler.c b/board/nvidia/whistler/whistler.c index 3e9d3d9..3114b20 100644 --- a/board/nvidia/whistler/whistler.c +++ b/board/nvidia/whistler/whistler.c @@ -6,6 +6,7 @@ */
#include <common.h> +#include <dm.h> #include <asm/io.h> #include <asm/arch/tegra.h> #include <asm/arch/clock.h> @@ -21,23 +22,26 @@ */ void pin_mux_mmc(void) { + struct udevice *dev; uchar val; int ret;
/* Turn on MAX8907B LDO12 to 2.8V for J40 power */ - ret = i2c_set_bus_num(0); - if (ret) - printf("i2c_set_bus_num failed: %d\n", ret); + ret = i2c_get_chip_for_busnum(0, 0x3c, &dev); + if (ret) { + printf("%s: Cannot find MAX8907B I2C chip\n", __func__); + return; + } val = 0x29; - ret = i2c_write(0x3c, 0x46, 1, &val, 1); + ret = i2c_write(dev, 0x46, &val, 1); if (ret) printf("i2c_write 0 0x3c 0x46 failed: %d\n", ret); val = 0x00; - ret = i2c_write(0x3c, 0x45, 1, &val, 1); + ret = i2c_write(dev, 0x45, &val, 1); if (ret) printf("i2c_write 0 0x3c 0x45 failed: %d\n", ret); val = 0x1f; - ret = i2c_write(0x3c, 0x44, 1, &val, 1); + ret = i2c_write(dev, 0x44, &val, 1); if (ret) printf("i2c_write 0 0x3c 0x44 failed: %d\n", ret);
@@ -49,6 +53,7 @@ void pin_mux_mmc(void) /* this is a weak define that we are overriding */ void pin_mux_usb(void) { + struct udevice *dev; uchar val; int ret;
@@ -59,15 +64,17 @@ void pin_mux_usb(void) */
/* Turn on TAC6416's GPIO 0+1 for USB1/3's VBUS */ - ret = i2c_set_bus_num(0); - if (ret) - printf("i2c_set_bus_num failed: %d\n", ret); + ret = i2c_get_chip_for_busnum(0, 0x20, &dev); + if (ret) { + printf("%s: Cannot find TAC6416 I2C chip\n", __func__); + return; + } val = 0x03; - ret = i2c_write(0x20, 2, 1, &val, 1); + ret = i2c_write(dev, 2, &val, 1); if (ret) printf("i2c_write 0 0x20 2 failed: %d\n", ret); val = 0xfc; - ret = i2c_write(0x20, 6, 1, &val, 1); + ret = i2c_write(dev, 6, &val, 1); if (ret) printf("i2c_write 0 0x20 6 failed: %d\n", ret); } diff --git a/board/toradex/apalis_t30/apalis_t30.c b/board/toradex/apalis_t30/apalis_t30.c index b9d694a..5d2c024 100644 --- a/board/toradex/apalis_t30/apalis_t30.c +++ b/board/toradex/apalis_t30/apalis_t30.c @@ -6,7 +6,7 @@ */
#include <common.h> - +#include <dm.h> #include <asm/arch/gp_padctrl.h> #include <asm/arch/pinmux.h> #include <asm/gpio.h> @@ -38,23 +38,20 @@ void pinmux_init(void) #ifdef CONFIG_PCI_TEGRA int tegra_pcie_board_init(void) { - unsigned int old_bus; + struct udevice *dev; u8 addr, data[1]; int err;
- old_bus = i2c_get_bus_num(); - - err = i2c_set_bus_num(0); + err = i2c_get_chip_for_busnum(0, PMU_I2C_ADDRESS, &dev); if (err) { - debug("failed to set I2C bus\n"); + debug("%s: Cannot find PMIC I2C chip\n", __func__); return err; } - /* TPS659110: VDD2_OP_REG = 1.05V */ data[0] = 0x27; addr = 0x25;
- err = i2c_write(PMU_I2C_ADDRESS, addr, 1, data, 1); + err = i2c_write(dev, addr, data, 1); if (err) { debug("failed to set VDD supply\n"); return err; @@ -64,7 +61,7 @@ int tegra_pcie_board_init(void) data[0] = 0x0D; addr = 0x24;
- err = i2c_write(PMU_I2C_ADDRESS, addr, 1, data, 1); + err = i2c_write(dev, addr, data, 1); if (err) { debug("failed to enable VDD supply\n"); return err; @@ -74,14 +71,12 @@ int tegra_pcie_board_init(void) data[0] = 0x0D; addr = 0x35;
- err = i2c_write(PMU_I2C_ADDRESS, addr, 1, data, 1); + err = i2c_write(dev, addr, data, 1); if (err) { debug("failed to set AVDD supply\n"); return err; }
- i2c_set_bus_num(old_bus); - return 0; }
diff --git a/drivers/i2c/tegra_i2c.c b/drivers/i2c/tegra_i2c.c index 562211e..87290c3 100644 --- a/drivers/i2c/tegra_i2c.c +++ b/drivers/i2c/tegra_i2c.c @@ -7,6 +7,8 @@ */
#include <common.h> +#include <dm.h> +#include <errno.h> #include <fdtdec.h> #include <i2c.h> #include <asm/io.h> @@ -19,6 +21,12 @@
DECLARE_GLOBAL_DATA_PTR;
+enum i2c_type { + TYPE_114, + TYPE_STD, + TYPE_DVC, +}; + /* Information about i2c controller */ struct i2c_bus { int id; @@ -27,20 +35,17 @@ struct i2c_bus { int pinmux_config; struct i2c_control *control; struct i2c_ctlr *regs; - int is_dvc; /* DVC type, rather than I2C */ - int is_scs; /* single clock source (T114+) */ + enum i2c_type type; int inited; /* bus is inited */ };
-static struct i2c_bus i2c_controllers[TEGRA_I2C_NUM_CONTROLLERS]; - static void set_packet_mode(struct i2c_bus *i2c_bus) { u32 config;
config = I2C_CNFG_NEW_MASTER_FSM_MASK | I2C_CNFG_PACKET_MODE_MASK;
- if (i2c_bus->is_dvc) { + if (i2c_bus->type == TYPE_DVC) { struct dvc_ctlr *dvc = (struct dvc_ctlr *)i2c_bus->regs;
writel(config, &dvc->cnfg); @@ -65,6 +70,9 @@ static void i2c_reset_controller(struct i2c_bus *i2c_bus)
static void i2c_init_controller(struct i2c_bus *i2c_bus) { + if (!i2c_bus->speed) + return; + debug("%s: speed=%d\n", __func__, i2c_bus->speed); /* * Use PLLP - DP-04508-001_v06 datasheet indicates a divisor of 8 * here, in section 23.3.1, but in fact we seem to need a factor of @@ -73,7 +81,7 @@ static void i2c_init_controller(struct i2c_bus *i2c_bus) clock_start_periph_pll(i2c_bus->periph_id, CLOCK_ID_PERIPH, i2c_bus->speed * 2 * 8);
- if (i2c_bus->is_scs) { + if (i2c_bus->type == TYPE_114) { /* * T114 I2C went to a single clock source for standard/fast and * HS clock speeds. The new clock rate setting calculation is: @@ -98,7 +106,7 @@ static void i2c_init_controller(struct i2c_bus *i2c_bus) i2c_reset_controller(i2c_bus);
/* Configure I2C controller. */ - if (i2c_bus->is_dvc) { /* only for DVC I2C */ + if (i2c_bus->type == TYPE_DVC) { /* only for DVC I2C */ struct dvc_ctlr *dvc = (struct dvc_ctlr *)i2c_bus->regs;
setbits_le32(&dvc->ctrl3, DVC_CTRL_REG3_I2C_HW_SW_PROG_MASK); @@ -272,7 +280,7 @@ exit: return error; }
-static int tegra_i2c_write_data(struct i2c_bus *bus, u32 addr, u8 *data, +static int tegra_i2c_write_data(struct i2c_bus *i2c_bus, u32 addr, u8 *data, u32 len, bool end_with_repeated_start) { int error; @@ -286,14 +294,14 @@ static int tegra_i2c_write_data(struct i2c_bus *bus, u32 addr, u8 *data, trans_info.num_bytes = len; trans_info.is_10bit_address = 0;
- error = send_recv_packets(bus, &trans_info); + error = send_recv_packets(i2c_bus, &trans_info); if (error) debug("tegra_i2c_write_data: Error (%d) !!!\n", error);
return error; }
-static int tegra_i2c_read_data(struct i2c_bus *bus, u32 addr, u8 *data, +static int tegra_i2c_read_data(struct i2c_bus *i2c_bus, u32 addr, u8 *data, u32 len) { int error; @@ -305,52 +313,32 @@ static int tegra_i2c_read_data(struct i2c_bus *bus, u32 addr, u8 *data, trans_info.num_bytes = len; trans_info.is_10bit_address = 0;
- error = send_recv_packets(bus, &trans_info); + error = send_recv_packets(i2c_bus, &trans_info); if (error) debug("tegra_i2c_read_data: Error (%d) !!!\n", error);
return error; }
-#ifndef CONFIG_OF_CONTROL -#error "Please enable device tree support to use this driver" -#endif - -/** - * Check that a bus number is valid and return a pointer to it - * - * @param bus_num Bus number to check / return - * @return pointer to bus, if valid, else NULL - */ -static struct i2c_bus *tegra_i2c_get_bus(struct i2c_adapter *adap) +static int tegra_i2c_set_bus_speed(struct udevice *dev, unsigned int speed) { - struct i2c_bus *bus; + struct i2c_bus *i2c_bus = dev_get_priv(dev);
- bus = &i2c_controllers[adap->hwadapnr]; - if (!bus->inited) { - debug("%s: Bus %u not available\n", __func__, adap->hwadapnr); - return NULL; - } - - return bus; -} - -static unsigned int tegra_i2c_set_bus_speed(struct i2c_adapter *adap, - unsigned int speed) -{ - struct i2c_bus *bus; - - bus = tegra_i2c_get_bus(adap); - if (!bus) - return 0; - bus->speed = speed; - i2c_init_controller(bus); + i2c_bus->speed = speed; + i2c_init_controller(i2c_bus);
return 0; }
-static int i2c_get_config(const void *blob, int node, struct i2c_bus *i2c_bus) +static int tegra_i2c_probe(struct udevice *dev) { + struct i2c_bus *i2c_bus = dev_get_priv(dev); + const void *blob = gd->fdt_blob; + int node = dev->of_offset; + bool is_dvc; + + i2c_bus->id = dev->seq; + i2c_bus->type = dev_get_of_data(dev); i2c_bus->regs = (struct i2c_ctlr *)fdtdec_get_addr(blob, node, "reg");
/* @@ -358,7 +346,6 @@ static int i2c_get_config(const void *blob, int node, struct i2c_bus *i2c_bus) * far no one needs anything other than the default. */ i2c_bus->pinmux_config = FUNCMUX_DEFAULT; - i2c_bus->speed = fdtdec_get_int(blob, node, "clock-frequency", 0); i2c_bus->periph_id = clock_decode_periph_id(blob, node);
/* @@ -371,107 +358,25 @@ static int i2c_get_config(const void *blob, int node, struct i2c_bus *i2c_bus) * i2c_bus->pinmux_config = FUNCMUX_I2C2_PTA; */ if (i2c_bus->periph_id == -1) - return -FDT_ERR_NOTFOUND; + return -EINVAL;
- return 0; -} - -/* - * Process a list of nodes, adding them to our list of I2C ports. - * - * @param blob fdt blob - * @param node_list list of nodes to process (any <=0 are ignored) - * @param count number of nodes to process - * @param is_dvc 1 if these are DVC ports, 0 if standard I2C - * @param is_scs 1 if this HW uses a single clock source (T114+) - * @return 0 if ok, -1 on error - */ -static int process_nodes(const void *blob, int node_list[], int count, - int is_dvc, int is_scs) -{ - struct i2c_bus *i2c_bus; - int i; - - /* build the i2c_controllers[] for each controller */ - for (i = 0; i < count; i++) { - int node = node_list[i]; - - if (node <= 0) - continue; - - i2c_bus = &i2c_controllers[i]; - i2c_bus->id = i; - - if (i2c_get_config(blob, node, i2c_bus)) { - printf("i2c_init_board: failed to decode bus %d\n", i); - return -1; - } - - i2c_bus->is_scs = is_scs; - - i2c_bus->is_dvc = is_dvc; - if (is_dvc) { - i2c_bus->control = - &((struct dvc_ctlr *)i2c_bus->regs)->control; - } else { - i2c_bus->control = &i2c_bus->regs->control; - } - debug("%s: controller bus %d at %p, periph_id %d, speed %d: ", - is_dvc ? "dvc" : "i2c", i, i2c_bus->regs, - i2c_bus->periph_id, i2c_bus->speed); - i2c_init_controller(i2c_bus); - debug("ok\n"); - i2c_bus->inited = 1; - - /* Mark position as used */ - node_list[i] = -1; + is_dvc = dev_get_of_data(dev) == TYPE_DVC; + if (is_dvc) { + i2c_bus->control = + &((struct dvc_ctlr *)i2c_bus->regs)->control; + } else { + i2c_bus->control = &i2c_bus->regs->control; } + i2c_init_controller(i2c_bus); + debug("%s: controller bus %d at %p, periph_id %d, speed %d: ", + is_dvc ? "dvc" : "i2c", dev->seq, i2c_bus->regs, + i2c_bus->periph_id, i2c_bus->speed);
return 0; }
-/* Sadly there is no error return from this function */ -void i2c_init_board(void) -{ - int node_list[TEGRA_I2C_NUM_CONTROLLERS]; - const void *blob = gd->fdt_blob; - int count; - - /* First check for newer (T114+) I2C ports */ - count = fdtdec_find_aliases_for_id(blob, "i2c", - COMPAT_NVIDIA_TEGRA114_I2C, node_list, - TEGRA_I2C_NUM_CONTROLLERS); - if (process_nodes(blob, node_list, count, 0, 1)) - return; - - /* Now get the older (T20/T30) normal I2C ports */ - count = fdtdec_find_aliases_for_id(blob, "i2c", - COMPAT_NVIDIA_TEGRA20_I2C, node_list, - TEGRA_I2C_NUM_CONTROLLERS); - if (process_nodes(blob, node_list, count, 0, 0)) - return; - - /* Now look for dvc ports */ - count = fdtdec_add_aliases_for_id(blob, "i2c", - COMPAT_NVIDIA_TEGRA20_DVC, node_list, - TEGRA_I2C_NUM_CONTROLLERS); - if (process_nodes(blob, node_list, count, 1, 0)) - return; -} - -static void tegra_i2c_init(struct i2c_adapter *adap, int speed, int slaveaddr) -{ - /* No i2c support prior to relocation */ - if (!(gd->flags & GD_FLG_RELOC)) - return; - - /* This will override the speed selected in the fdt for that port */ - debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr); - i2c_set_bus_speed(speed); -} - /* i2c write version without the register address */ -static int i2c_write_data(struct i2c_bus *bus, uchar chip, uchar *buffer, +static int i2c_write_data(struct i2c_bus *i2c_bus, uchar chip, uchar *buffer, int len, bool end_with_repeated_start) { int rc; @@ -484,7 +389,7 @@ static int i2c_write_data(struct i2c_bus *bus, uchar chip, uchar *buffer, debug("\n");
/* Shift 7-bit address over for lower-level i2c functions */ - rc = tegra_i2c_write_data(bus, chip << 1, buffer, len, + rc = tegra_i2c_write_data(i2c_bus, chip << 1, buffer, len, end_with_repeated_start); if (rc) debug("i2c_write_data(): rc=%d\n", rc); @@ -493,14 +398,14 @@ static int i2c_write_data(struct i2c_bus *bus, uchar chip, uchar *buffer, }
/* i2c read version without the register address */ -static int i2c_read_data(struct i2c_bus *bus, uchar chip, uchar *buffer, - int len) +static int i2c_read_data(struct i2c_bus *i2c_bus, uchar chip, uchar *buffer, + int len) { int rc;
debug("inside i2c_read_data():\n"); /* Shift 7-bit address over for lower-level i2c functions */ - rc = tegra_i2c_read_data(bus, chip << 1, buffer, len); + rc = tegra_i2c_read_data(i2c_bus, chip << 1, buffer, len); if (rc) { debug("i2c_read_data(): rc=%d\n", rc); return rc; @@ -516,132 +421,99 @@ static int i2c_read_data(struct i2c_bus *bus, uchar chip, uchar *buffer, }
/* Probe to see if a chip is present. */ -static int tegra_i2c_probe(struct i2c_adapter *adap, uchar chip) +static int tegra_i2c_probe_chip(struct udevice *bus, uint chip_addr, + uint chip_flags) { - struct i2c_bus *bus; + struct i2c_bus *i2c_bus = dev_get_priv(bus); int rc; - uchar reg; - - debug("i2c_probe: addr=0x%x\n", chip); - bus = tegra_i2c_get_bus(adap); - if (!bus) - return 1; - reg = 0; - rc = i2c_write_data(bus, chip, ®, 1, false); - if (rc) { - debug("Error probing 0x%x.\n", chip); - return 1; - } - return 0; -} + u8 reg;
-static int i2c_addr_ok(const uint addr, const int alen) -{ - /* We support 7 or 10 bit addresses, so one or two bytes each */ - return alen == 1 || alen == 2; + /* Shift 7-bit address over for lower-level i2c functions */ + rc = tegra_i2c_write_data(i2c_bus, chip_addr << 1, ®, sizeof(reg), + false); + + return rc; }
-/* Read bytes */ -static int tegra_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, - int alen, uchar *buffer, int len) +static int tegra_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, + int nmsgs) { - struct i2c_bus *bus; - uint offset; - int i; - - debug("i2c_read: chip=0x%x, addr=0x%x, alen=0x%x len=0x%x\n", - chip, addr, alen, len); - bus = tegra_i2c_get_bus(adap); - if (!bus) - return 1; - if (!i2c_addr_ok(addr, alen)) { - debug("i2c_read: Bad address %x.%d.\n", addr, alen); - return 1; - } - for (offset = 0; offset < len; offset++) { - if (alen) { - uchar data[alen]; - for (i = 0; i < alen; i++) { - data[alen - i - 1] = - (addr + offset) >> (8 * i); - } - if (i2c_write_data(bus, chip, data, alen, true)) { - debug("i2c_read: error sending (0x%x)\n", - addr); - return 1; - } + struct i2c_bus *i2c_bus = dev_get_priv(bus); + int ret; + + debug("i2c_xfer: %d messages\n", nmsgs); + for (; nmsgs > 0; nmsgs--, msg++) { + bool next_is_read = nmsgs > 1 && (msg[1].flags & I2C_M_RD); + + debug("i2c_xfer: chip=0x%x, len=0x%x\n", msg->addr, msg->len); + if (msg->flags & I2C_M_RD) { + ret = i2c_read_data(i2c_bus, msg->addr, msg->buf, + msg->len); + } else { + ret = i2c_write_data(i2c_bus, msg->addr, msg->buf, + msg->len, next_is_read); } - if (i2c_read_data(bus, chip, buffer + offset, 1)) { - debug("i2c_read: error reading (0x%x)\n", addr); - return 1; + if (ret) { + debug("i2c_write: error sending\n"); + return -EREMOTEIO; } }
return 0; }
-/* Write bytes */ -static int tegra_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr, - int alen, uchar *buffer, int len) +int tegra_i2c_get_dvc_bus(struct udevice **busp) { - struct i2c_bus *bus; - uint offset; - int i; - - debug("i2c_write: chip=0x%x, addr=0x%x, alen=0x%x len=0x%x\n", - chip, addr, alen, len); - bus = tegra_i2c_get_bus(adap); - if (!bus) - return 1; - if (!i2c_addr_ok(addr, alen)) { - debug("i2c_write: Bad address %x.%d.\n", addr, alen); - return 1; - } - for (offset = 0; offset < len; offset++) { - uchar data[alen + 1]; - for (i = 0; i < alen; i++) - data[alen - i - 1] = (addr + offset) >> (8 * i); - data[alen] = buffer[offset]; - if (i2c_write_data(bus, chip, data, alen + 1, false)) { - debug("i2c_write: error sending (0x%x)\n", addr); - return 1; + struct udevice *bus; + + for (uclass_first_device(UCLASS_I2C, &bus); + bus; + uclass_next_device(&bus)) { + if (dev_get_of_data(bus) == TYPE_DVC) { + *busp = bus; + return 0; } }
- return 0; + return -ENODEV; }
-int tegra_i2c_get_dvc_bus_num(void) -{ - int i; +static const struct dm_i2c_ops tegra_i2c_ops = { + .xfer = tegra_i2c_xfer, + .probe_chip = tegra_i2c_probe_chip, + .set_bus_speed = tegra_i2c_set_bus_speed, +};
- for (i = 0; i < TEGRA_I2C_NUM_CONTROLLERS; i++) { - struct i2c_bus *bus = &i2c_controllers[i]; +static int tegra_i2c_child_pre_probe(struct udevice *dev) +{ + struct dm_i2c_chip *i2c_chip = dev_get_parentdata(dev);
- if (bus->inited && bus->is_dvc) - return i; - } + if (dev->of_offset == -1) + return 0; + return i2c_chip_ofdata_to_platdata(gd->fdt_blob, dev->of_offset, + i2c_chip); +}
- return -1; +static int tegra_i2c_ofdata_to_platdata(struct udevice *dev) +{ + return 0; }
-/* - * Register soft i2c adapters - */ -U_BOOT_I2C_ADAP_COMPLETE(tegra0, tegra_i2c_init, tegra_i2c_probe, - tegra_i2c_read, tegra_i2c_write, - tegra_i2c_set_bus_speed, 100000, 0, 0) -U_BOOT_I2C_ADAP_COMPLETE(tegra1, tegra_i2c_init, tegra_i2c_probe, - tegra_i2c_read, tegra_i2c_write, - tegra_i2c_set_bus_speed, 100000, 0, 1) -U_BOOT_I2C_ADAP_COMPLETE(tegra2, tegra_i2c_init, tegra_i2c_probe, - tegra_i2c_read, tegra_i2c_write, - tegra_i2c_set_bus_speed, 100000, 0, 2) -U_BOOT_I2C_ADAP_COMPLETE(tegra3, tegra_i2c_init, tegra_i2c_probe, - tegra_i2c_read, tegra_i2c_write, - tegra_i2c_set_bus_speed, 100000, 0, 3) -#if TEGRA_I2C_NUM_CONTROLLERS > 4 -U_BOOT_I2C_ADAP_COMPLETE(tegra4, tegra_i2c_init, tegra_i2c_probe, - tegra_i2c_read, tegra_i2c_write, - tegra_i2c_set_bus_speed, 100000, 0, 4) -#endif +static const struct udevice_id tegra_i2c_ids[] = { + { .compatible = "nvidia,tegra114-i2c", .data = TYPE_114 }, + { .compatible = "nvidia,tegra20-i2c", .data = TYPE_STD }, + { .compatible = "nvidia,tegra20-i2c-dvc", .data = TYPE_DVC }, + { } +}; + +U_BOOT_DRIVER(i2c_tegra) = { + .name = "i2c_tegra", + .id = UCLASS_I2C, + .of_match = tegra_i2c_ids, + .ofdata_to_platdata = tegra_i2c_ofdata_to_platdata, + .probe = tegra_i2c_probe, + .per_child_auto_alloc_size = sizeof(struct dm_i2c_chip), + .child_pre_probe = tegra_i2c_child_pre_probe, + .priv_auto_alloc_size = sizeof(struct i2c_bus), + .ops = &tegra_i2c_ops, +}; diff --git a/drivers/power/tps6586x.c b/drivers/power/tps6586x.c index d29d969..29bab4c 100644 --- a/drivers/power/tps6586x.c +++ b/drivers/power/tps6586x.c @@ -10,9 +10,7 @@ #include <asm/io.h> #include <i2c.h>
-static int bus_num; /* I2C bus we are on */ -#define I2C_ADDRESS 0x34 /* chip requires this address */ -static char inited; /* 1 if we have been inited */ +static struct udevice *tps6586x_dev;
enum { /* Registers that we access */ @@ -37,13 +35,9 @@ static int tps6586x_read(int reg) int i; uchar data; int retval = -1; - int old_bus_num; - - old_bus_num = i2c_get_bus_num(); - i2c_set_bus_num(bus_num);
for (i = 0; i < MAX_I2C_RETRY; ++i) { - if (!i2c_read(I2C_ADDRESS, reg, 1, &data, 1)) { + if (!i2c_read(tps6586x_dev, reg, &data, 1)) { retval = (int)data; goto exit; } @@ -53,7 +47,6 @@ static int tps6586x_read(int reg) }
exit: - i2c_set_bus_num(old_bus_num); debug("pmu_read %x=%x\n", reg, retval); if (retval < 0) debug("%s: failed to read register %#x: %d\n", __func__, reg, @@ -65,13 +58,9 @@ static int tps6586x_write(int reg, uchar *data, uint len) { int i; int retval = -1; - int old_bus_num; - - old_bus_num = i2c_get_bus_num(); - i2c_set_bus_num(bus_num);
for (i = 0; i < MAX_I2C_RETRY; ++i) { - if (!i2c_write(I2C_ADDRESS, reg, 1, data, len)) { + if (!i2c_write(tps6586x_dev, reg, data, len)) { retval = 0; goto exit; } @@ -81,7 +70,6 @@ static int tps6586x_write(int reg, uchar *data, uint len) }
exit: - i2c_set_bus_num(old_bus_num); debug("pmu_write %x=%x: ", reg, retval); for (i = 0; i < len; i++) debug("%x ", data[i]); @@ -163,7 +151,7 @@ int tps6586x_set_pwm_mode(int mask) uchar val; int ret;
- assert(inited); + assert(tps6586x_dev); ret = tps6586x_read(PFM_MODE); if (ret != -1) { val = (uchar)ret; @@ -184,7 +172,7 @@ int tps6586x_adjust_sm0_sm1(int sm0_target, int sm1_target, int step, int rate, int sm0, sm1; int bad;
- assert(inited); + assert(tps6586x_dev);
/* get current voltage settings */ if (read_voltages(&sm0, &sm1)) { @@ -255,10 +243,9 @@ int tps6586x_adjust_sm0_sm1(int sm0_target, int sm1_target, int step, int rate, return bad ? -1 : 0; }
-int tps6586x_init(int bus) +int tps6586x_init(struct udevice *dev) { - bus_num = bus; - inited = 1; + tps6586x_dev = dev;
return 0; } diff --git a/include/configs/apalis_t30.h b/include/configs/apalis_t30.h index 3cde923..61809fc 100644 --- a/include/configs/apalis_t30.h +++ b/include/configs/apalis_t30.h @@ -26,10 +26,7 @@
/* I2C */ #define CONFIG_SYS_I2C_TEGRA -#define CONFIG_SYS_I2C_INIT_BOARD -#define CONFIG_SYS_I2C_SPEED 100000 #define CONFIG_CMD_I2C -#define CONFIG_SYS_I2C
/* SD/MMC */ #define CONFIG_MMC diff --git a/include/configs/beaver.h b/include/configs/beaver.h index 164b2dd..5d765f3 100644 --- a/include/configs/beaver.h +++ b/include/configs/beaver.h @@ -40,10 +40,7 @@
/* I2C */ #define CONFIG_SYS_I2C_TEGRA -#define CONFIG_SYS_I2C_INIT_BOARD -#define CONFIG_SYS_I2C_SPEED 100000 #define CONFIG_CMD_I2C -#define CONFIG_SYS_I2C
/* SD/MMC */ #define CONFIG_MMC diff --git a/include/configs/cardhu.h b/include/configs/cardhu.h index 09129c7..758b7ad 100644 --- a/include/configs/cardhu.h +++ b/include/configs/cardhu.h @@ -43,12 +43,7 @@
/* I2C */ #define CONFIG_SYS_I2C_TEGRA -#define CONFIG_SYS_I2C_INIT_BOARD -#define CONFIG_I2C_MULTI_BUS -#define CONFIG_SYS_MAX_I2C_BUS TEGRA_I2C_NUM_CONTROLLERS -#define CONFIG_SYS_I2C_SPEED 100000 #define CONFIG_CMD_I2C -#define CONFIG_SYS_I2C
/* SD/MMC */ #define CONFIG_MMC diff --git a/include/configs/colibri_t30.h b/include/configs/colibri_t30.h index a582e25..ce6f23b 100644 --- a/include/configs/colibri_t30.h +++ b/include/configs/colibri_t30.h @@ -25,10 +25,7 @@
/* I2C */ #define CONFIG_SYS_I2C_TEGRA -#define CONFIG_SYS_I2C_INIT_BOARD -#define CONFIG_SYS_I2C_SPEED 100000 #define CONFIG_CMD_I2C -#define CONFIG_SYS_I2C
/* SD/MMC */ #define CONFIG_MMC diff --git a/include/configs/dalmore.h b/include/configs/dalmore.h index ff7ec4a..0b04ee6 100644 --- a/include/configs/dalmore.h +++ b/include/configs/dalmore.h @@ -36,12 +36,7 @@
/* I2C */ #define CONFIG_SYS_I2C_TEGRA -#define CONFIG_SYS_I2C_INIT_BOARD -#define CONFIG_I2C_MULTI_BUS -#define CONFIG_SYS_MAX_I2C_BUS TEGRA_I2C_NUM_CONTROLLERS -#define CONFIG_SYS_I2C_SPEED 100000 #define CONFIG_CMD_I2C -#define CONFIG_SYS_I2C
/* SD/MMC */ #define CONFIG_MMC diff --git a/include/configs/jetson-tk1.h b/include/configs/jetson-tk1.h index d67c025..a7d7665 100644 --- a/include/configs/jetson-tk1.h +++ b/include/configs/jetson-tk1.h @@ -25,12 +25,7 @@
/* I2C */ #define CONFIG_SYS_I2C_TEGRA -#define CONFIG_SYS_I2C_INIT_BOARD -#define CONFIG_I2C_MULTI_BUS -#define CONFIG_SYS_MAX_I2C_BUS TEGRA_I2C_NUM_CONTROLLERS -#define CONFIG_SYS_I2C_SPEED 100000 #define CONFIG_CMD_I2C -#define CONFIG_SYS_I2C
/* SD/MMC */ #define CONFIG_MMC diff --git a/include/configs/nyan-big.h b/include/configs/nyan-big.h index a62bc4e..cf331ab 100644 --- a/include/configs/nyan-big.h +++ b/include/configs/nyan-big.h @@ -25,12 +25,7 @@
/* I2C */ #define CONFIG_SYS_I2C_TEGRA -#define CONFIG_SYS_I2C_INIT_BOARD -#define CONFIG_I2C_MULTI_BUS -#define CONFIG_SYS_MAX_I2C_BUS TEGRA_I2C_NUM_CONTROLLERS -#define CONFIG_SYS_I2C_SPEED 100000 #define CONFIG_CMD_I2C -#define CONFIG_SYS_I2C
/* SD/MMC */ #define CONFIG_MMC diff --git a/include/configs/seaboard.h b/include/configs/seaboard.h index 04e4f82..5f77051 100644 --- a/include/configs/seaboard.h +++ b/include/configs/seaboard.h @@ -37,10 +37,7 @@
/* I2C */ #define CONFIG_SYS_I2C_TEGRA -#define CONFIG_SYS_I2C_INIT_BOARD -#define CONFIG_SYS_I2C_SPEED 100000 #define CONFIG_CMD_I2C -#define CONFIG_SYS_I2C
/* SD/MMC */ #define CONFIG_MMC diff --git a/include/configs/tec-ng.h b/include/configs/tec-ng.h index 51f87da..e37b233 100644 --- a/include/configs/tec-ng.h +++ b/include/configs/tec-ng.h @@ -23,12 +23,7 @@
/* I2C */ #define CONFIG_SYS_I2C_TEGRA -#define CONFIG_SYS_I2C_INIT_BOARD -#define CONFIG_I2C_MULTI_BUS -#define CONFIG_SYS_MAX_I2C_BUS TEGRA_I2C_NUM_CONTROLLERS -#define CONFIG_SYS_I2C_SPEED 100000 #define CONFIG_CMD_I2C -#define CONFIG_SYS_I2C
/* SD/MMC */ #define CONFIG_MMC diff --git a/include/configs/tegra-common.h b/include/configs/tegra-common.h index d690045..0685328 100644 --- a/include/configs/tegra-common.h +++ b/include/configs/tegra-common.h @@ -26,6 +26,7 @@ #endif #define CONFIG_DM_SPI #define CONFIG_DM_SPI_FLASH +#define CONFIG_DM_I2C
#define CONFIG_SYS_TIMER_RATE 1000000 #define CONFIG_SYS_TIMER_COUNTER NV_PA_TMRUS_BASE diff --git a/include/configs/tegra114-common.h b/include/configs/tegra114-common.h index 555c237..9eba5d5 100644 --- a/include/configs/tegra114-common.h +++ b/include/configs/tegra114-common.h @@ -76,9 +76,6 @@ #define CONFIG_SYS_SPL_MALLOC_START 0x80090000 #define CONFIG_SPL_STACK 0x800ffffc
-/* Total I2C ports on Tegra114 */ -#define TEGRA_I2C_NUM_CONTROLLERS 5 - /* For USB EHCI controller */ #define CONFIG_EHCI_IS_TDI #define CONFIG_USB_EHCI_TXFIFO_THRESH 0x10 diff --git a/include/configs/tegra124-common.h b/include/configs/tegra124-common.h index 61e5026..f2b3774 100644 --- a/include/configs/tegra124-common.h +++ b/include/configs/tegra124-common.h @@ -68,9 +68,6 @@ #define CONFIG_SYS_SPL_MALLOC_START 0x80090000 #define CONFIG_SPL_STACK 0x800ffffc
-/* Total I2C ports on Tegra124 */ -#define TEGRA_I2C_NUM_CONTROLLERS 5 - /* For USB EHCI controller */ #define CONFIG_EHCI_IS_TDI #define CONFIG_USB_EHCI_TXFIFO_THRESH 0x10 diff --git a/include/configs/tegra20-common.h b/include/configs/tegra20-common.h index 21bf977..6330281 100644 --- a/include/configs/tegra20-common.h +++ b/include/configs/tegra20-common.h @@ -97,9 +97,6 @@ #define CONFIG_EHCI_IS_TDI #define CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS 1
-/* Total I2C ports on Tegra20 */ -#define TEGRA_I2C_NUM_CONTROLLERS 4 - #define CONFIG_SYS_NAND_SELF_INIT #define CONFIG_SYS_NAND_ONFI_DETECTION
diff --git a/include/configs/tegra30-common.h b/include/configs/tegra30-common.h index 443c842..bfdbeb7 100644 --- a/include/configs/tegra30-common.h +++ b/include/configs/tegra30-common.h @@ -73,9 +73,6 @@ #define CONFIG_SYS_SPL_MALLOC_START 0x80090000 #define CONFIG_SPL_STACK 0x800ffffc
-/* Total I2C ports on Tegra30 */ -#define TEGRA_I2C_NUM_CONTROLLERS 5 - /* For USB EHCI controller */ #define CONFIG_EHCI_IS_TDI #define CONFIG_USB_EHCI_TXFIFO_THRESH 0x10 diff --git a/include/configs/trimslice.h b/include/configs/trimslice.h index 7c00642..a254f86 100644 --- a/include/configs/trimslice.h +++ b/include/configs/trimslice.h @@ -34,10 +34,7 @@
/* I2C */ #define CONFIG_SYS_I2C_TEGRA -#define CONFIG_SYS_I2C_INIT_BOARD -#define CONFIG_SYS_I2C_SPEED 100000 #define CONFIG_CMD_I2C -#define CONFIG_SYS_I2C
/* SD/MMC */ #define CONFIG_MMC diff --git a/include/configs/venice2.h b/include/configs/venice2.h index 6897aa8..8880de8 100644 --- a/include/configs/venice2.h +++ b/include/configs/venice2.h @@ -25,12 +25,7 @@
/* I2C */ #define CONFIG_SYS_I2C_TEGRA -#define CONFIG_SYS_I2C_INIT_BOARD -#define CONFIG_I2C_MULTI_BUS -#define CONFIG_SYS_MAX_I2C_BUS TEGRA_I2C_NUM_CONTROLLERS -#define CONFIG_SYS_I2C_SPEED 100000 #define CONFIG_CMD_I2C -#define CONFIG_SYS_I2C
/* SD/MMC */ #define CONFIG_MMC diff --git a/include/configs/whistler.h b/include/configs/whistler.h index 10e70d2..e083cbd 100644 --- a/include/configs/whistler.h +++ b/include/configs/whistler.h @@ -26,10 +26,7 @@
/* I2C */ #define CONFIG_SYS_I2C_TEGRA -#define CONFIG_SYS_I2C_INIT_BOARD -#define CONFIG_SYS_I2C_SPEED 100000 #define CONFIG_CMD_I2C -#define CONFIG_SYS_I2C
/* SD/MMC */ #define CONFIG_MMC diff --git a/include/tps6586x.h b/include/tps6586x.h index 78ce428..eefc95f 100644 --- a/include/tps6586x.h +++ b/include/tps6586x.h @@ -44,9 +44,9 @@ int tps6586x_adjust_sm0_sm1(int sm0_target, int sm1_target, int step, int rate, * Set up the TPS6586X I2C bus number. This will be used for all operations * on the device. This function must be called before using other functions. * - * @param bus I2C bus number containing the TPS6586X chip + * @param bus I2C bus containing the TPS6586X chip * @return 0 (always succeeds) */ -int tps6586x_init(int bus); +int tps6586x_init(struct udevice *bus);
#endif /* _TPS6586X_H_ */
Hi Simon,
On Thu, 4 Dec 2014 21:21:30 -0700 Simon Glass sjg@chromium.org wrote:
+static const struct dm_i2c_ops tegra_i2c_ops = {
- .xfer = tegra_i2c_xfer,
- .probe_chip = tegra_i2c_probe_chip,
- .set_bus_speed = tegra_i2c_set_bus_speed,
+};
- for (i = 0; i < TEGRA_I2C_NUM_CONTROLLERS; i++) {
struct i2c_bus *bus = &i2c_controllers[i];+static int tegra_i2c_child_pre_probe(struct udevice *dev) +{
- struct dm_i2c_chip *i2c_chip = dev_get_parentdata(dev);
if (bus->inited && bus->is_dvc)return i;- }
- if (dev->of_offset == -1)
return 0;- return i2c_chip_ofdata_to_platdata(gd->fdt_blob, dev->of_offset,
i2c_chip);+}
As I said in v3, it is unfortunate that uclass does not support .child_pre_probe.
sandbox_i2c.c and tegra-i2c.c have a similar .child_pre_probe. I copied tegra_i2c_child_pre_probe() to my i2c driver verbatim.
I guess every driver will do so.
Is it a good idea to move .child_pre_probe to uclass_driver?
Best Regards Masahiro Yamada
Hi Masahiro,
On 5 December 2014 at 06:15, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
Hi Simon,
On Thu, 4 Dec 2014 21:21:30 -0700 Simon Glass sjg@chromium.org wrote:
+static const struct dm_i2c_ops tegra_i2c_ops = {
.xfer = tegra_i2c_xfer,.probe_chip = tegra_i2c_probe_chip,.set_bus_speed = tegra_i2c_set_bus_speed,+};
for (i = 0; i < TEGRA_I2C_NUM_CONTROLLERS; i++) {struct i2c_bus *bus = &i2c_controllers[i];+static int tegra_i2c_child_pre_probe(struct udevice *dev) +{
struct dm_i2c_chip *i2c_chip = dev_get_parentdata(dev);
if (bus->inited && bus->is_dvc)return i;}
if (dev->of_offset == -1)return 0;return i2c_chip_ofdata_to_platdata(gd->fdt_blob, dev->of_offset,i2c_chip);+}
As I said in v3, it is unfortunate that uclass does not support .child_pre_probe.
sandbox_i2c.c and tegra-i2c.c have a similar .child_pre_probe. I copied tegra_i2c_child_pre_probe() to my i2c driver verbatim.
I guess every driver will do so.
Is it a good idea to move .child_pre_probe to uclass_driver?
Sorry I didn't reply on this.
I have been thinking about this for a while. For SPI we have the same problem and I think I mentioned it in the code somewhere. For the PCI RFC I have already adjusted the platdata lifecycle a little since I think it will simplify all buses, and have added a another data pointer for a different purpose.
For the case you raise the problem as you say is that each driver is required to provide a certain structure for its children so that its uclass is happy. On the other hand the driver may want to add additional information We already have a lot of flexibility so I was waiting to see how much this would really matter (there is after all a complexity trade-off).
With PCI I've made some changes and I think I'll end up adding another pointer. At that point I will also want to change SPI and I2C.
For now, I'd like to leave this alone. It does work and avoids adding new features at this late stage.
Let's revisit when I come back to PCI.
Regards, Simon
Hi Simon,
Sorry for my late reply.
On Fri, 5 Dec 2014 07:50:33 -0700 Simon Glass sjg@chromium.org wrote:
Sorry I didn't reply on this.
I have been thinking about this for a while. For SPI we have the same problem and I think I mentioned it in the code somewhere. For the PCI RFC I have already adjusted the platdata lifecycle a little since I think it will simplify all buses, and have added a another data pointer for a different purpose.
For the case you raise the problem as you say is that each driver is required to provide a certain structure for its children so that its uclass is happy. On the other hand the driver may want to add additional information We already have a lot of flexibility so I was waiting to see how much this would really matter (there is after all a complexity trade-off).
With PCI I've made some changes and I think I'll end up adding another pointer. At that point I will also want to change SPI and I2C.
For now, I'd like to leave this alone. It does work and avoids adding new features at this late stage.
Let's revisit when I come back to PCI.
I hope you will come back to here before it gets too late.
If we have lots of drivers converted to DM and some of them start to have various deviations, it will be difficult to merge the common parts.
Best Regards Masahiro Yamada
Hi Masahiro,
On 10 December 2014 at 06:18, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
Hi Simon,
Sorry for my late reply.
On Fri, 5 Dec 2014 07:50:33 -0700 Simon Glass sjg@chromium.org wrote:
Sorry I didn't reply on this.
I have been thinking about this for a while. For SPI we have the same problem and I think I mentioned it in the code somewhere. For the PCI RFC I have already adjusted the platdata lifecycle a little since I think it will simplify all buses, and have added a another data pointer for a different purpose.
For the case you raise the problem as you say is that each driver is required to provide a certain structure for its children so that its uclass is happy. On the other hand the driver may want to add additional information We already have a lot of flexibility so I was waiting to see how much this would really matter (there is after all a complexity trade-off).
With PCI I've made some changes and I think I'll end up adding another pointer. At that point I will also want to change SPI and I2C.
For now, I'd like to leave this alone. It does work and avoids adding new features at this late stage.
Let's revisit when I come back to PCI.
I hope you will come back to here before it gets too late.
If we have lots of drivers converted to DM and some of them start to have various deviations, it will be difficult to merge the common parts.
First priority is to move everything to Kconfig. I sent an RFC.
Second I need to figure out the platdata size of things and the discussion above.
I am not worried about the task. So far it only affects I2C and SPI (and PCI RFC). There are only 3-4 drivers I think. It should be fine. The good news is that with this many uclasses we can be a lot more confident that the design is solid and can cover U-Boot's needs.
Regards, Simon
participants (2)
-
Masahiro Yamada -
Simon Glass