[U-Boot] [PATCH 0/8] drivers: i2c: davinci_i2c: Convert driver to DM
This patch series converts the davinci i2c driver to use device model. This updated driver has been verified on both Keystone K2G and Keystone K2L evms by performing several i2c operations in U-boot prompt.
Some additional work was required to get things working on K2G due to the code that reads the on board EEPROM. DM I2C sets the default address length to a default value of 1 when the on EEPROM requires an address length of 2. Therefore, an additional function and minor changes were required to get things working properly.
Franklin S Cooper Jr (8): i2c: davinci: Split functions into two parts for future DM support drivers: i2c: davinci_i2c: Update davinci i2c driver to driver model ti: common: board_detect: Introduce function to set the address length. ti: common: board_detect: Set alen to expected value before i2c read ARM: dts: k2g: Add I2C nodes to 66AK2Gx ARM: dts: keystone2: add I2C aliases for davinci I2C nodes ARM: dts: keystone-k2g-evm: Enable I2C0 and I2C1 defconfig: keystone: Enable DM I2C
arch/arm/dts/keystone-k2g-evm.dts | 8 ++ arch/arm/dts/keystone-k2g.dtsi | 32 +++++ arch/arm/dts/keystone.dtsi | 3 + board/ti/common/board_detect.c | 61 ++++++++- configs/k2e_evm_defconfig | 1 + configs/k2g_evm_defconfig | 1 + configs/k2hk_evm_defconfig | 1 + configs/k2l_evm_defconfig | 1 + drivers/i2c/davinci_i2c.c | 277 +++++++++++++++++++++++++++----------- 9 files changed, 306 insertions(+), 79 deletions(-)
The i2c driver will be converted to support device model. In preparation for that change split the various functions into two parts. This will allow device model specific driver to reuse the majority of the code from the non device model implementation.
Also rename the probe function to probe_chip to better reflect its purpose.
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com --- drivers/i2c/davinci_i2c.c | 185 +++++++++++++++++++++++++++------------------- 1 file changed, 110 insertions(+), 75 deletions(-)
diff --git a/drivers/i2c/davinci_i2c.c b/drivers/i2c/davinci_i2c.c index c5bd38c..a6bce26 100644 --- a/drivers/i2c/davinci_i2c.c +++ b/drivers/i2c/davinci_i2c.c @@ -29,9 +29,8 @@
static struct i2c_regs *davinci_get_base(struct i2c_adapter *adap);
-static int wait_for_bus(struct i2c_adapter *adap) +static int _wait_for_bus(struct i2c_regs *i2c_base) { - struct i2c_regs *i2c_base = davinci_get_base(adap); int stat, timeout;
REG(&(i2c_base->i2c_stat)) = 0xffff; @@ -51,10 +50,8 @@ static int wait_for_bus(struct i2c_adapter *adap) return 1; }
- -static int poll_i2c_irq(struct i2c_adapter *adap, int mask) +static int _poll_i2c_irq(struct i2c_regs *i2c_base, int mask) { - struct i2c_regs *i2c_base = davinci_get_base(adap); int stat, timeout;
for (timeout = 0; timeout < 10; timeout++) { @@ -68,10 +65,8 @@ static int poll_i2c_irq(struct i2c_adapter *adap, int mask) return stat | I2C_TIMEOUT; }
-static void flush_rx(struct i2c_adapter *adap) +static void _flush_rx(struct i2c_regs *i2c_base) { - struct i2c_regs *i2c_base = davinci_get_base(adap); - while (1) { if (!(REG(&(i2c_base->i2c_stat)) & I2C_STAT_RRDY)) break; @@ -82,9 +77,9 @@ static void flush_rx(struct i2c_adapter *adap) } }
-static uint davinci_i2c_setspeed(struct i2c_adapter *adap, uint speed) +static uint _davinci_i2c_setspeed(struct i2c_regs *i2c_base, + uint speed) { - struct i2c_regs *i2c_base = davinci_get_base(adap); uint32_t div, psc;
psc = 2; @@ -94,20 +89,18 @@ static uint davinci_i2c_setspeed(struct i2c_adapter *adap, uint speed) REG(&(i2c_base->i2c_scll)) = (div * 50) / 100; /* 50% Duty */ REG(&(i2c_base->i2c_sclh)) = div - REG(&(i2c_base->i2c_scll));
- adap->speed = speed; return 0; }
-static void davinci_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd) +static void _davinci_i2c_init(struct i2c_regs *i2c_base, + uint speed, int slaveadd) { - struct i2c_regs *i2c_base = davinci_get_base(adap); - if (REG(&(i2c_base->i2c_con)) & I2C_CON_EN) { REG(&(i2c_base->i2c_con)) = 0; udelay(50000); }
- davinci_i2c_setspeed(adap, speed); + _davinci_i2c_setspeed(i2c_base, speed);
REG(&(i2c_base->i2c_oa)) = slaveadd; REG(&(i2c_base->i2c_cnt)) = 0; @@ -122,47 +115,9 @@ static void davinci_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd) udelay(1000); }
-static int davinci_i2c_probe(struct i2c_adapter *adap, uint8_t chip) +static int _davinci_i2c_read(struct i2c_regs *i2c_base, uint8_t chip, + uint32_t addr, int alen, uint8_t *buf, int len) { - struct i2c_regs *i2c_base = davinci_get_base(adap); - int rc = 1; - - if (chip == REG(&(i2c_base->i2c_oa))) - return rc; - - REG(&(i2c_base->i2c_con)) = 0; - if (wait_for_bus(adap)) - return 1; - - /* try to read one byte from current (or only) address */ - REG(&(i2c_base->i2c_cnt)) = 1; - REG(&(i2c_base->i2c_sa)) = chip; - REG(&(i2c_base->i2c_con)) = (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | - I2C_CON_STP); - udelay(50000); - - if (!(REG(&(i2c_base->i2c_stat)) & I2C_STAT_NACK)) { - rc = 0; - flush_rx(adap); - REG(&(i2c_base->i2c_stat)) = 0xffff; - } else { - REG(&(i2c_base->i2c_stat)) = 0xffff; - REG(&(i2c_base->i2c_con)) |= I2C_CON_STP; - udelay(20000); - if (wait_for_bus(adap)) - return 1; - } - - flush_rx(adap); - REG(&(i2c_base->i2c_stat)) = 0xffff; - REG(&(i2c_base->i2c_cnt)) = 0; - return rc; -} - -static int davinci_i2c_read(struct i2c_adapter *adap, uint8_t chip, - uint32_t addr, int alen, uint8_t *buf, int len) -{ - struct i2c_regs *i2c_base = davinci_get_base(adap); uint32_t tmp; int i;
@@ -171,7 +126,7 @@ static int davinci_i2c_read(struct i2c_adapter *adap, uint8_t chip, return 1; }
- if (wait_for_bus(adap)) + if (_wait_for_bus(i2c_base)) return 1;
if (alen != 0) { @@ -181,7 +136,7 @@ static int davinci_i2c_read(struct i2c_adapter *adap, uint8_t chip, REG(&(i2c_base->i2c_sa)) = chip; REG(&(i2c_base->i2c_con)) = tmp;
- tmp = poll_i2c_irq(adap, I2C_STAT_XRDY | I2C_STAT_NACK); + tmp = _poll_i2c_irq(i2c_base, I2C_STAT_XRDY | I2C_STAT_NACK);
CHECK_NACK();
@@ -195,7 +150,8 @@ static int davinci_i2c_read(struct i2c_adapter *adap, uint8_t chip, return 1; }
- tmp = poll_i2c_irq(adap, I2C_STAT_XRDY | I2C_STAT_NACK); + tmp = _poll_i2c_irq(i2c_base, + I2C_STAT_XRDY | I2C_STAT_NACK);
CHECK_NACK(); /* No break, fall through */ @@ -208,8 +164,8 @@ static int davinci_i2c_read(struct i2c_adapter *adap, uint8_t chip, return 1; }
- tmp = poll_i2c_irq(adap, I2C_STAT_XRDY | - I2C_STAT_NACK | I2C_STAT_ARDY); + tmp = _poll_i2c_irq(i2c_base, I2C_STAT_XRDY | + I2C_STAT_NACK | I2C_STAT_ARDY);
CHECK_NACK();
@@ -227,7 +183,7 @@ static int davinci_i2c_read(struct i2c_adapter *adap, uint8_t chip, REG(&(i2c_base->i2c_con)) = tmp;
for (i = 0; i < len; i++) { - tmp = poll_i2c_irq(adap, I2C_STAT_RRDY | I2C_STAT_NACK | + tmp = _poll_i2c_irq(i2c_base, I2C_STAT_RRDY | I2C_STAT_NACK | I2C_STAT_ROVR);
CHECK_NACK(); @@ -240,7 +196,7 @@ static int davinci_i2c_read(struct i2c_adapter *adap, uint8_t chip, } }
- tmp = poll_i2c_irq(adap, I2C_STAT_SCD | I2C_STAT_NACK); + tmp = _poll_i2c_irq(i2c_base, I2C_STAT_SCD | I2C_STAT_NACK);
CHECK_NACK();
@@ -249,7 +205,7 @@ static int davinci_i2c_read(struct i2c_adapter *adap, uint8_t chip, return 1; }
- flush_rx(adap); + _flush_rx(i2c_base); REG(&(i2c_base->i2c_stat)) = 0xffff; REG(&(i2c_base->i2c_cnt)) = 0; REG(&(i2c_base->i2c_con)) = 0; @@ -257,10 +213,9 @@ static int davinci_i2c_read(struct i2c_adapter *adap, uint8_t chip, return 0; }
-static int davinci_i2c_write(struct i2c_adapter *adap, uint8_t chip, - uint32_t addr, int alen, uint8_t *buf, int len) +static int _davinci_i2c_write(struct i2c_regs *i2c_base, uint8_t chip, + uint32_t addr, int alen, uint8_t *buf, int len) { - struct i2c_regs *i2c_base = davinci_get_base(adap); uint32_t tmp; int i;
@@ -273,7 +228,7 @@ static int davinci_i2c_write(struct i2c_adapter *adap, uint8_t chip, return 1; }
- if (wait_for_bus(adap)) + if (_wait_for_bus(i2c_base)) return 1;
/* Start address phase */ @@ -287,7 +242,7 @@ static int davinci_i2c_write(struct i2c_adapter *adap, uint8_t chip, switch (alen) { case 2: /* Send address MSByte */ - tmp = poll_i2c_irq(adap, I2C_STAT_XRDY | I2C_STAT_NACK); + tmp = _poll_i2c_irq(i2c_base, I2C_STAT_XRDY | I2C_STAT_NACK);
CHECK_NACK();
@@ -300,7 +255,7 @@ static int davinci_i2c_write(struct i2c_adapter *adap, uint8_t chip, /* No break, fall through */ case 1: /* Send address LSByte */ - tmp = poll_i2c_irq(adap, I2C_STAT_XRDY | I2C_STAT_NACK); + tmp = _poll_i2c_irq(i2c_base, I2C_STAT_XRDY | I2C_STAT_NACK);
CHECK_NACK();
@@ -313,7 +268,7 @@ static int davinci_i2c_write(struct i2c_adapter *adap, uint8_t chip, }
for (i = 0; i < len; i++) { - tmp = poll_i2c_irq(adap, I2C_STAT_XRDY | I2C_STAT_NACK); + tmp = _poll_i2c_irq(i2c_base, I2C_STAT_XRDY | I2C_STAT_NACK);
CHECK_NACK();
@@ -323,7 +278,7 @@ static int davinci_i2c_write(struct i2c_adapter *adap, uint8_t chip, return 1; }
- tmp = poll_i2c_irq(adap, I2C_STAT_SCD | I2C_STAT_NACK); + tmp = _poll_i2c_irq(i2c_base, I2C_STAT_SCD | I2C_STAT_NACK);
CHECK_NACK();
@@ -332,7 +287,7 @@ static int davinci_i2c_write(struct i2c_adapter *adap, uint8_t chip, return 1; }
- flush_rx(adap); + _flush_rx(i2c_base); REG(&(i2c_base->i2c_stat)) = 0xffff; REG(&(i2c_base->i2c_cnt)) = 0; REG(&(i2c_base->i2c_con)) = 0; @@ -340,6 +295,42 @@ static int davinci_i2c_write(struct i2c_adapter *adap, uint8_t chip, return 0; }
+static int _davinci_i2c_probe_chip(struct i2c_regs *i2c_base, uint8_t chip) +{ + int rc = 1; + + if (chip == REG(&(i2c_base->i2c_oa))) + return rc; + + REG(&(i2c_base->i2c_con)) = 0; + if (_wait_for_bus(i2c_base)) + return 1; + + /* try to read one byte from current (or only) address */ + REG(&(i2c_base->i2c_cnt)) = 1; + REG(&(i2c_base->i2c_sa)) = chip; + REG(&(i2c_base->i2c_con)) = (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | + I2C_CON_STP); + udelay(50000); + + if (!(REG(&(i2c_base->i2c_stat)) & I2C_STAT_NACK)) { + rc = 0; + _flush_rx(i2c_base); + REG(&(i2c_base->i2c_stat)) = 0xffff; + } else { + REG(&(i2c_base->i2c_stat)) = 0xffff; + REG(&(i2c_base->i2c_con)) |= I2C_CON_STP; + udelay(20000); + if (_wait_for_bus(i2c_base)) + return 1; + } + + _flush_rx(i2c_base); + REG(&(i2c_base->i2c_stat)) = 0xffff; + REG(&(i2c_base->i2c_cnt)) = 0; + return rc; +} + static struct i2c_regs *davinci_get_base(struct i2c_adapter *adap) { switch (adap->hwadapnr) { @@ -361,7 +352,51 @@ static struct i2c_regs *davinci_get_base(struct i2c_adapter *adap) return NULL; }
-U_BOOT_I2C_ADAP_COMPLETE(davinci_0, davinci_i2c_init, davinci_i2c_probe, +static uint davinci_i2c_setspeed(struct i2c_adapter *adap, uint speed) +{ + struct i2c_regs *i2c_base = davinci_get_base(adap); + uint ret; + + adap->speed = speed; + ret = _davinci_i2c_setspeed(i2c_base, speed); + + return ret; +} + +static void davinci_i2c_init(struct i2c_adapter *adap, int speed, + int slaveadd) +{ + struct i2c_regs *i2c_base = davinci_get_base(adap); + + adap->speed = speed; + _davinci_i2c_init(i2c_base, speed, slaveadd); + + return; +} + +static int davinci_i2c_read(struct i2c_adapter *adap, uint8_t chip, + uint32_t addr, int alen, uint8_t *buf, int len) +{ + struct i2c_regs *i2c_base = davinci_get_base(adap); + return _davinci_i2c_read(i2c_base, chip, addr, alen, buf, len); +} + +static int davinci_i2c_write(struct i2c_adapter *adap, uint8_t chip, + uint32_t addr, int alen, uint8_t *buf, int len) +{ + struct i2c_regs *i2c_base = davinci_get_base(adap); + + return _davinci_i2c_write(i2c_base, chip, addr, alen, buf, len); +} + +static int davinci_i2c_probe_chip(struct i2c_adapter *adap, uint8_t chip) +{ + struct i2c_regs *i2c_base = davinci_get_base(adap); + + return _davinci_i2c_probe_chip(i2c_base, chip); +} + +U_BOOT_I2C_ADAP_COMPLETE(davinci_0, davinci_i2c_init, davinci_i2c_probe_chip, davinci_i2c_read, davinci_i2c_write, davinci_i2c_setspeed, CONFIG_SYS_DAVINCI_I2C_SPEED, @@ -369,7 +404,7 @@ U_BOOT_I2C_ADAP_COMPLETE(davinci_0, davinci_i2c_init, davinci_i2c_probe, 0)
#if I2C_BUS_MAX >= 2 -U_BOOT_I2C_ADAP_COMPLETE(davinci_1, davinci_i2c_init, davinci_i2c_probe, +U_BOOT_I2C_ADAP_COMPLETE(davinci_1, davinci_i2c_init, davinci_i2c_probe_chip, davinci_i2c_read, davinci_i2c_write, davinci_i2c_setspeed, CONFIG_SYS_DAVINCI_I2C_SPEED1, @@ -378,7 +413,7 @@ U_BOOT_I2C_ADAP_COMPLETE(davinci_1, davinci_i2c_init, davinci_i2c_probe, #endif
#if I2C_BUS_MAX >= 3 -U_BOOT_I2C_ADAP_COMPLETE(davinci_2, davinci_i2c_init, davinci_i2c_probe, +U_BOOT_I2C_ADAP_COMPLETE(davinci_2, davinci_i2c_init, davinci_i2c_probe_chip, davinci_i2c_read, davinci_i2c_write, davinci_i2c_setspeed, CONFIG_SYS_DAVINCI_I2C_SPEED2,
Convert davinci i2c driver to driver model.
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com --- drivers/i2c/davinci_i2c.c | 92 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 90 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/davinci_i2c.c b/drivers/i2c/davinci_i2c.c index a6bce26..4471193 100644 --- a/drivers/i2c/davinci_i2c.c +++ b/drivers/i2c/davinci_i2c.c @@ -14,11 +14,21 @@
#include <common.h> #include <i2c.h> +#include <dm.h> #include <asm/arch/hardware.h> #include <asm/arch/i2c_defs.h> #include <asm/io.h> #include "davinci_i2c.h"
+#ifdef CONFIG_DM_I2C +/* Information about i2c controller */ +struct i2c_bus { + int id; + uint speed; + struct i2c_regs *regs; +}; +#endif + #define CHECK_NACK() \ do {\ if (tmp & (I2C_TIMEOUT | I2C_STAT_NACK)) {\ @@ -27,8 +37,6 @@ } \ } while (0)
-static struct i2c_regs *davinci_get_base(struct i2c_adapter *adap); - static int _wait_for_bus(struct i2c_regs *i2c_base) { int stat, timeout; @@ -331,6 +339,7 @@ static int _davinci_i2c_probe_chip(struct i2c_regs *i2c_base, uint8_t chip) return rc; }
+#ifndef CONFIG_DM_I2C static struct i2c_regs *davinci_get_base(struct i2c_adapter *adap) { switch (adap->hwadapnr) { @@ -420,3 +429,82 @@ U_BOOT_I2C_ADAP_COMPLETE(davinci_2, davinci_i2c_init, davinci_i2c_probe_chip, CONFIG_SYS_DAVINCI_I2C_SLAVE2, 2) #endif + +#else /* CONFIG_DM_I2C */ + +static int davinci_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, + int nmsgs) +{ + struct i2c_bus *i2c_bus = dev_get_priv(bus); + int ret; + + debug("i2c_xfer: %d messages\n", nmsgs); + for (; nmsgs > 0; nmsgs--, msg++) { + debug("i2c_xfer: chip=0x%x, len=0x%x\n", msg->addr, msg->len); + if (msg->flags & I2C_M_RD) { + ret = _davinci_i2c_read(i2c_bus->regs, msg->addr, + 0, 0, msg->buf, msg->len); + } else { + ret = _davinci_i2c_write(i2c_bus->regs, msg->addr, + 0, 0, msg->buf, msg->len); + } + if (ret) { + debug("i2c_write: error sending\n"); + return -EREMOTEIO; + } + } + + return ret; +} + +static int davinci_i2c_set_speed(struct udevice *dev, uint speed) +{ + struct i2c_bus *i2c_bus = dev_get_priv(dev); + + i2c_bus->speed = speed; + return _davinci_i2c_setspeed(i2c_bus->regs, speed); +} + +static int davinci_i2c_probe(struct udevice *dev) +{ + struct i2c_bus *i2c_bus = dev_get_priv(dev); + + i2c_bus->id = dev->seq; + i2c_bus->regs = (struct i2c_regs *)dev_get_addr(dev); + + i2c_bus->speed = 100000; + _davinci_i2c_init(i2c_bus->regs, i2c_bus->speed, 0); + + return 0; +} + +static int davinci_i2c_probe_chip(struct udevice *bus, uint chip_addr, + uint chip_flags) +{ + struct i2c_bus *i2c_bus = dev_get_priv(bus); + + return _davinci_i2c_probe_chip(i2c_bus->regs, chip_addr); +} + +static const struct dm_i2c_ops davinci_i2c_ops = { + .xfer = davinci_i2c_xfer, + .probe_chip = davinci_i2c_probe_chip, + .set_bus_speed = davinci_i2c_set_speed, +}; + +static const struct udevice_id davinci_i2c_ids[] = { + { .compatible = "ti,davinci-i2c"}, + { .compatible = "ti,keystone-i2c"}, + { } +}; + +U_BOOT_DRIVER(i2c_davinci) = { + .name = "i2c_davinci", + .id = UCLASS_I2C, + .of_match = davinci_i2c_ids, + .probe = davinci_i2c_probe, + .priv_auto_alloc_size = sizeof(struct i2c_bus), + .ops = &davinci_i2c_ops, +}; + +#endif /* CONFIG_DM_I2C */
Reading from the I2C EEPROM used typically requires using an address length of 2. However, when using DM for I2C the default address length used is 1. To fix this introduce a new function that allows the address length to be changed. The logic to do so was copied from cmd/i2c.c.
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com --- board/ti/common/board_detect.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)
diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c index c55e24e..1e695f4 100644 --- a/board/ti/common/board_detect.c +++ b/board/ti/common/board_detect.c @@ -10,10 +10,47 @@
#include <common.h> #include <asm/omap_common.h> +#include <dm/uclass.h> #include <i2c.h>
#include "board_detect.h"
+#if defined(CONFIG_DM_I2C_COMPAT) +/** + * ti_i2c_set_alen - Set chip's i2c address length + * @bus_addr - I2C bus number + * @dev_addr - I2C eeprom id + * @alen - I2C address length in bytes + * + * DM_I2C by default sets the address length to be used to 1. This + * function allows this address length to be changed to match the + * eeprom used for board detection. + */ +int __maybe_unused ti_i2c_set_alen(int bus_addr, int dev_addr, int alen) +{ + struct udevice *dev; + struct udevice *bus; + int rc; + + rc = uclass_get_device_by_seq(UCLASS_I2C, bus_addr, &bus); + if (rc) + return rc; + rc = i2c_get_chip(bus, dev_addr, 1, &dev); + if (rc) + return rc; + rc = i2c_set_chip_offset_len(dev, alen); + if (rc) + return rc; + + return 0; +} +#else +int __maybe_unused ti_i2c_set_alen(int bus_addr, int dev_addr, int alen) +{ + return 0; +} +#endif + /** * ti_i2c_eeprom_init - Initialize an i2c bus and probe for a device * @i2c_bus: i2c bus number to initialize
On Wednesday 12 April 2017 12:07 AM, Franklin S Cooper Jr wrote:
Reading from the I2C EEPROM used typically requires using an address length of 2. However, when using DM for I2C the default address length used is 1. To fix this introduce a new function that allows the address length to be changed. The logic to do so was copied from cmd/i2c.c.
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com
I maybe wrong, but doesn't adding DT property u-boot,i2c-offset-len = <2>; to the i2c slave node help to achieve this?
Regards Vignesh
board/ti/common/board_detect.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)
diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c index c55e24e..1e695f4 100644 --- a/board/ti/common/board_detect.c +++ b/board/ti/common/board_detect.c @@ -10,10 +10,47 @@
#include <common.h> #include <asm/omap_common.h> +#include <dm/uclass.h> #include <i2c.h>
#include "board_detect.h"
+#if defined(CONFIG_DM_I2C_COMPAT) +/**
- ti_i2c_set_alen - Set chip's i2c address length
- @bus_addr - I2C bus number
- @dev_addr - I2C eeprom id
- @alen - I2C address length in bytes
- DM_I2C by default sets the address length to be used to 1. This
- function allows this address length to be changed to match the
- eeprom used for board detection.
- */
+int __maybe_unused ti_i2c_set_alen(int bus_addr, int dev_addr, int alen) +{
- struct udevice *dev;
- struct udevice *bus;
- int rc;
- rc = uclass_get_device_by_seq(UCLASS_I2C, bus_addr, &bus);
- if (rc)
return rc;- rc = i2c_get_chip(bus, dev_addr, 1, &dev);
- if (rc)
return rc;- rc = i2c_set_chip_offset_len(dev, alen);
- if (rc)
return rc;- return 0;
+} +#else +int __maybe_unused ti_i2c_set_alen(int bus_addr, int dev_addr, int alen) +{
- return 0;
+} +#endif
/**
- ti_i2c_eeprom_init - Initialize an i2c bus and probe for a device
- @i2c_bus: i2c bus number to initialize
On 04/12/2017 03:40 AM, Vignesh R wrote:
On Wednesday 12 April 2017 12:07 AM, Franklin S Cooper Jr wrote:
Reading from the I2C EEPROM used typically requires using an address length of 2. However, when using DM for I2C the default address length used is 1. To fix this introduce a new function that allows the address length to be changed. The logic to do so was copied from cmd/i2c.c.
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com
I maybe wrong, but doesn't adding DT property u-boot,i2c-offset-len = <2>; to the i2c slave node help to achieve this?
There is no i2c slave node for this. This is just a function making raw i2c read operations. In the future especially when OMAP SPL uses DM it would be useful to see if this can be converted to a actual driver. Then u-boot,i2c-offset-len should work.
Regards Vignesh
board/ti/common/board_detect.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)
diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c index c55e24e..1e695f4 100644 --- a/board/ti/common/board_detect.c +++ b/board/ti/common/board_detect.c @@ -10,10 +10,47 @@
#include <common.h> #include <asm/omap_common.h> +#include <dm/uclass.h> #include <i2c.h>
#include "board_detect.h"
+#if defined(CONFIG_DM_I2C_COMPAT) +/**
- ti_i2c_set_alen - Set chip's i2c address length
- @bus_addr - I2C bus number
- @dev_addr - I2C eeprom id
- @alen - I2C address length in bytes
- DM_I2C by default sets the address length to be used to 1. This
- function allows this address length to be changed to match the
- eeprom used for board detection.
- */
+int __maybe_unused ti_i2c_set_alen(int bus_addr, int dev_addr, int alen) +{
- struct udevice *dev;
- struct udevice *bus;
- int rc;
- rc = uclass_get_device_by_seq(UCLASS_I2C, bus_addr, &bus);
- if (rc)
return rc;- rc = i2c_get_chip(bus, dev_addr, 1, &dev);
- if (rc)
return rc;- rc = i2c_set_chip_offset_len(dev, alen);
- if (rc)
return rc;- return 0;
+} +#else +int __maybe_unused ti_i2c_set_alen(int bus_addr, int dev_addr, int alen) +{
- return 0;
+} +#endif
/**
- ti_i2c_eeprom_init - Initialize an i2c bus and probe for a device
- @i2c_bus: i2c bus number to initialize
In non DM I2C read operations the address length passed in during a read operation will be used automatically. However, in DM I2C the address length is set to a default value of one which causes problems when trying to perform a read with a differing alen. Therefore, before the first read in a series of read operations set the alen to the correct value.
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com --- board/ti/common/board_detect.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c index 1e695f4..6fdcb61 100644 --- a/board/ti/common/board_detect.c +++ b/board/ti/common/board_detect.c @@ -83,7 +83,17 @@ static int __maybe_unused ti_i2c_eeprom_init(int i2c_bus, int dev_addr) static int __maybe_unused ti_i2c_eeprom_read(int dev_addr, int offset, uchar *ep, int epsize) { - return i2c_read(dev_addr, offset, 2, ep, epsize); + int bus_num, rc, alen; + + bus_num = i2c_get_bus_num(); + + alen = 2; + + rc = ti_i2c_set_alen(bus_num, dev_addr, alen); + if (rc) + return rc; + + return i2c_read(dev_addr, offset, alen, ep, epsize); }
/** @@ -125,6 +135,11 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr, * Read the header first then only read the other contents. */ byte = 2; + + rc = ti_i2c_set_alen(bus_addr, dev_addr, byte); + if (rc) + return rc; + rc = i2c_read(dev_addr, 0x0, byte, (uint8_t *)&hdr_read, 4); if (rc) return rc; @@ -137,9 +152,14 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr, * 1 byte address (some legacy boards need this..) */ byte = 1; - if (rc) + if (rc) { + rc = ti_i2c_set_alen(bus_addr, dev_addr, byte); + if (rc) + return rc; + rc = i2c_read(dev_addr, 0x0, byte, (uint8_t *)&hdr_read, 4); + } if (rc) return rc; }
On Tue, Apr 11, 2017 at 01:37:21PM -0500, Franklin S Cooper Jr wrote:
In non DM I2C read operations the address length passed in during a read operation will be used automatically. However, in DM I2C the address length is set to a default value of one which causes problems when trying to perform a read with a differing alen. Therefore, before the first read in a series of read operations set the alen to the correct value.
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com
Reviewed-by: Tom Rini trini@konsulko.com
Add I2C nodes to the 66AK2Gx dtsi.
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com --- arch/arm/dts/keystone-k2g.dtsi | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
diff --git a/arch/arm/dts/keystone-k2g.dtsi b/arch/arm/dts/keystone-k2g.dtsi index 2193f9f..191e3f1 100644 --- a/arch/arm/dts/keystone-k2g.dtsi +++ b/arch/arm/dts/keystone-k2g.dtsi @@ -24,6 +24,9 @@ spi2 = &spi2; spi3 = &spi3; spi4 = &qspi; + i2c0 = &i2c0; + i2c1 = &i2c1; + i2c2 = &i2c2; };
memory { @@ -149,6 +152,35 @@ #size-cells = <0>; status = "disabled"; }; + i2c0: i2c@2530000 { + compatible = "ti,keystone-i2c"; + reg = <0x02530000 0x400>; + clock-frequency = <100000>; + interrupts = <GIC_SPI 88 IRQ_TYPE_EDGE_RISING>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + }; + + i2c1: i2c@2530400 { + compatible = "ti,keystone-i2c"; + reg = <0x02530400 0x400>; + clock-frequency = <100000>; + interrupts = <GIC_SPI 89 IRQ_TYPE_EDGE_RISING>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + }; + + i2c2: i2c@2530800 { + compatible = "ti,keystone-i2c"; + reg = <0x02530800 0x400>; + clock-frequency = <100000>; + interrupts = <GIC_SPI 90 IRQ_TYPE_EDGE_RISING>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + };
mmc0: mmc@23000000 { compatible = "ti,omap4-hsmmc";
On Tue, Apr 11, 2017 at 01:37:22PM -0500, Franklin S Cooper Jr wrote:
Add I2C nodes to the 66AK2Gx dtsi.
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com
arch/arm/dts/keystone-k2g.dtsi | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
OK, are these upstream as well? If so, should we perhaps sync with the kernel for a bunch of updates?
On 04/12/2017 02:11 PM, Tom Rini wrote:
On Tue, Apr 11, 2017 at 01:37:22PM -0500, Franklin S Cooper Jr wrote:
Add I2C nodes to the 66AK2Gx dtsi.
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com
arch/arm/dts/keystone-k2g.dtsi | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
OK, are these upstream as well? If so, should we perhaps sync with the kernel for a bunch of updates?
No this isn't upstreamed yet. There is quite a bit of clock related work that needs to be upstreamed before we can add various peripherals to the DT. It will be several months before this happens. Once it happens I can sync U-boot's K2G DT with the one that has been upstreamed.
On Wed, Apr 12, 2017 at 02:17:40PM -0500, Franklin S Cooper Jr wrote:
On 04/12/2017 02:11 PM, Tom Rini wrote:
On Tue, Apr 11, 2017 at 01:37:22PM -0500, Franklin S Cooper Jr wrote:
Add I2C nodes to the 66AK2Gx dtsi.
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com
arch/arm/dts/keystone-k2g.dtsi | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
OK, are these upstream as well? If so, should we perhaps sync with the kernel for a bunch of updates?
No this isn't upstreamed yet. There is quite a bit of clock related work that needs to be upstreamed before we can add various peripherals to the DT. It will be several months before this happens. Once it happens I can sync U-boot's K2G DT with the one that has been upstreamed.
OK. Just please make sure that any u-boot specific properties end up in a -u-boot.dtsi file, thanks!
Reviewed-by: Tom Rini trini@konsulko.com
Add aliases for I2C nodes required for the DM framework to probe the davinci-i2c driver.
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com --- arch/arm/dts/keystone.dtsi | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/arch/arm/dts/keystone.dtsi b/arch/arm/dts/keystone.dtsi index be97f3f..9a2e1f6 100644 --- a/arch/arm/dts/keystone.dtsi +++ b/arch/arm/dts/keystone.dtsi @@ -22,6 +22,9 @@ spi0 = &spi0; spi1 = &spi1; spi2 = &spi2; + i2c0 = &i2c0; + i2c1 = &i2c1; + i2c2 = &i2c2; };
chosen {
Enable I2C0 and I2C1 which is needed to enable usage of DM I2C.
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com --- arch/arm/dts/keystone-k2g-evm.dts | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/arch/arm/dts/keystone-k2g-evm.dts b/arch/arm/dts/keystone-k2g-evm.dts index 696a0d7..2c99df4 100644 --- a/arch/arm/dts/keystone-k2g-evm.dts +++ b/arch/arm/dts/keystone-k2g-evm.dts @@ -108,3 +108,11 @@ &mmc1 { status = "okay"; }; + +&i2c0 { + status = "okay"; +}; + +&i2c1 { + status = "okay"; +};
Now that the davinci I2C driver is converted to driver model enable it in 66AK2Gx defconfig
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com --- configs/k2e_evm_defconfig | 1 + configs/k2g_evm_defconfig | 1 + configs/k2hk_evm_defconfig | 1 + configs/k2l_evm_defconfig | 1 + 4 files changed, 4 insertions(+)
diff --git a/configs/k2e_evm_defconfig b/configs/k2e_evm_defconfig index d319705..52e5bd4 100644 --- a/configs/k2e_evm_defconfig +++ b/configs/k2e_evm_defconfig @@ -54,3 +54,4 @@ CONFIG_USB=y CONFIG_USB_XHCI_HCD=y CONFIG_USB_XHCI_DWC3=y CONFIG_USB_STORAGE=y +CONFIG_DM_I2C=y diff --git a/configs/k2g_evm_defconfig b/configs/k2g_evm_defconfig index bbdcc2f..d4cdf61 100644 --- a/configs/k2g_evm_defconfig +++ b/configs/k2g_evm_defconfig @@ -58,3 +58,4 @@ CONFIG_USB=y CONFIG_USB_XHCI_HCD=y CONFIG_USB_XHCI_DWC3=y CONFIG_USB_STORAGE=y +CONFIG_DM_I2C=y diff --git a/configs/k2hk_evm_defconfig b/configs/k2hk_evm_defconfig index b22086d..7d22cd6 100644 --- a/configs/k2hk_evm_defconfig +++ b/configs/k2hk_evm_defconfig @@ -54,3 +54,4 @@ CONFIG_USB=y CONFIG_USB_XHCI_HCD=y CONFIG_USB_XHCI_DWC3=y CONFIG_USB_STORAGE=y +CONFIG_DM_I2C=y diff --git a/configs/k2l_evm_defconfig b/configs/k2l_evm_defconfig index 5a28112..ec2fcbf 100644 --- a/configs/k2l_evm_defconfig +++ b/configs/k2l_evm_defconfig @@ -54,3 +54,4 @@ CONFIG_USB=y CONFIG_USB_XHCI_HCD=y CONFIG_USB_XHCI_DWC3=y CONFIG_USB_STORAGE=y +CONFIG_DM_I2C=y
On Tue, Apr 11, 2017 at 01:37:25PM -0500, Franklin S Cooper Jr wrote:
Now that the davinci I2C driver is converted to driver model enable it in 66AK2Gx defconfig
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com
configs/k2e_evm_defconfig | 1 + configs/k2g_evm_defconfig | 1 + configs/k2hk_evm_defconfig | 1 + configs/k2l_evm_defconfig | 1 + 4 files changed, 4 insertions(+)
Should we be imply/select'ing DM_I2C from the Kconfig file now?
On 04/12/2017 02:13 PM, Tom Rini wrote:
On Tue, Apr 11, 2017 at 01:37:25PM -0500, Franklin S Cooper Jr wrote:
Now that the davinci I2C driver is converted to driver model enable it in 66AK2Gx defconfig
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com
configs/k2e_evm_defconfig | 1 + configs/k2g_evm_defconfig | 1 + configs/k2hk_evm_defconfig | 1 + configs/k2l_evm_defconfig | 1 + 4 files changed, 4 insertions(+)
Should we be imply/select'ing DM_I2C from the Kconfig file now?
Is the goal to reduce the size of the defconfig? Or is it to essentially force all K2 boards (including possible future non TI boards) to use the DM so we can deprecate/remove non DM code in the future?
If its for the latter then I don't have an issue doing so.
On Wed, Apr 12, 2017 at 02:26:21PM -0500, Franklin S Cooper Jr wrote:
On 04/12/2017 02:13 PM, Tom Rini wrote:
On Tue, Apr 11, 2017 at 01:37:25PM -0500, Franklin S Cooper Jr wrote:
Now that the davinci I2C driver is converted to driver model enable it in 66AK2Gx defconfig
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com
configs/k2e_evm_defconfig | 1 + configs/k2g_evm_defconfig | 1 + configs/k2hk_evm_defconfig | 1 + configs/k2l_evm_defconfig | 1 + 4 files changed, 4 insertions(+)
Should we be imply/select'ing DM_I2C from the Kconfig file now?
Is the goal to reduce the size of the defconfig? Or is it to essentially force all K2 boards (including possible future non TI boards) to use the DM so we can deprecate/remove non DM code in the future?
If its for the latter then I don't have an issue doing so.
Well, both. I think 'imply' is a great way to specify default values in such a way that making new boards/defconfigs is easy and likely to be correct. But also, yes, non-DM will go away at some point.
On 04/12/2017 02:33 PM, Tom Rini wrote:
On Wed, Apr 12, 2017 at 02:26:21PM -0500, Franklin S Cooper Jr wrote:
On 04/12/2017 02:13 PM, Tom Rini wrote:
On Tue, Apr 11, 2017 at 01:37:25PM -0500, Franklin S Cooper Jr wrote:
Now that the davinci I2C driver is converted to driver model enable it in 66AK2Gx defconfig
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com
configs/k2e_evm_defconfig | 1 + configs/k2g_evm_defconfig | 1 + configs/k2hk_evm_defconfig | 1 + configs/k2l_evm_defconfig | 1 + 4 files changed, 4 insertions(+)
Should we be imply/select'ing DM_I2C from the Kconfig file now?
Is the goal to reduce the size of the defconfig? Or is it to essentially force all K2 boards (including possible future non TI boards) to use the DM so we can deprecate/remove non DM code in the future?
If its for the latter then I don't have an issue doing so.
Well, both. I think 'imply' is a great way to specify default values in such a way that making new boards/defconfigs is easy and likely to be correct. But also, yes, non-DM will go away at some point.
I'm ok with that. Do you want me to resend this patchset and switch this patch from enabling DM_I2C via defconfig to KConfig? Or you can just drop this patch from the patchset and then I can just send a separate patch enabling DM_I2C via KConfig.
I'm ok with what ever you prefer.
On Wed, Apr 12, 2017 at 02:41:50PM -0500, Franklin S Cooper Jr wrote:
On 04/12/2017 02:33 PM, Tom Rini wrote:
On Wed, Apr 12, 2017 at 02:26:21PM -0500, Franklin S Cooper Jr wrote:
On 04/12/2017 02:13 PM, Tom Rini wrote:
On Tue, Apr 11, 2017 at 01:37:25PM -0500, Franklin S Cooper Jr wrote:
Now that the davinci I2C driver is converted to driver model enable it in 66AK2Gx defconfig
Signed-off-by: Franklin S Cooper Jr fcooper@ti.com
configs/k2e_evm_defconfig | 1 + configs/k2g_evm_defconfig | 1 + configs/k2hk_evm_defconfig | 1 + configs/k2l_evm_defconfig | 1 + 4 files changed, 4 insertions(+)
Should we be imply/select'ing DM_I2C from the Kconfig file now?
Is the goal to reduce the size of the defconfig? Or is it to essentially force all K2 boards (including possible future non TI boards) to use the DM so we can deprecate/remove non DM code in the future?
If its for the latter then I don't have an issue doing so.
Well, both. I think 'imply' is a great way to specify default values in such a way that making new boards/defconfigs is easy and likely to be correct. But also, yes, non-DM will go away at some point.
I'm ok with that. Do you want me to resend this patchset and switch this patch from enabling DM_I2C via defconfig to KConfig? Or you can just drop this patch from the patchset and then I can just send a separate patch enabling DM_I2C via KConfig.
I'm ok with what ever you prefer.
Do a new 8/8 that does imply instead please, thanks!
participants (3)
-
Franklin S Cooper Jr -
Tom Rini -
Vignesh R