From patchwork Thu Jan 20 17:51:31 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefano Babic X-Patchwork-Id: 79739 X-Patchwork-Delegate: sbabic@denx.de Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id 0E56FB7105 for ; Fri, 21 Jan 2011 04:51:51 +1100 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id AD1C628167; Thu, 20 Jan 2011 18:51:49 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id c+AMAW9DlgFi; Thu, 20 Jan 2011 18:51:49 +0100 (CET) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id CEA8F2815D; Thu, 20 Jan 2011 18:51:46 +0100 (CET) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 7113D2815D for ; Thu, 20 Jan 2011 18:51:45 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 4Mmy9SEHQcrl for ; Thu, 20 Jan 2011 18:51:45 +0100 (CET) X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from smtpi3.ngi.it (smtpi3.ngi.it [88.149.128.33]) by theia.denx.de (Postfix) with ESMTP id 0469428151 for ; Thu, 20 Jan 2011 18:51:43 +0100 (CET) Received: from paperina.lan (unknown [88.149.182.160]) by smtpi3.ngi.it (Postfix) with ESMTP id B300B31813A; Thu, 20 Jan 2011 18:51:42 +0100 (CET) Received: from papero.lan (papero.lan [192.168.2.245]) by paperina.lan (Postfix) with ESMTP id 67C6C140A224; Thu, 20 Jan 2011 18:51:42 +0100 (CET) From: Stefano Babic To: u-boot@lists.denx.de Date: Thu, 20 Jan 2011 18:51:31 +0100 Message-Id: <1295545891-12574-1-git-send-email-sbabic@denx.de> X-Mailer: git-send-email 1.7.1 In-Reply-To: <1295513194-16158-6-git-send-email-sbabic@denx.de> References: <1295513194-16158-6-git-send-email-sbabic@denx.de> Cc: Heiko Schocher Subject: [U-Boot] [PATCH V3 05/11] I2C: mxc_i2c: address failure with mx35 processor X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.9 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: u-boot-bounces@lists.denx.de Errors-To: u-boot-bounces@lists.denx.de There is sporadic failures when more as one I2C slave is on the bus and the processor tries to communicate with more as one slave. The problem was seen on a mx35pdk (two I2C slaves, PMIC controller and CAN/RTC chip). The current driver uses the IIF bit in the status register to check if the bus is busy or not. According to the manual, this is not correct, because the IIB bit should be checked. Not only, to check if a transfer is finished must be checked the ICF bit, and this is not tested at all. This patch comes from analyse with a corresponding driver provided by Freescale as part of the LTIB tool. Comparing the two drivers, it appears that the current u-boot driver checks the wrong bits, and depending on race condition, the transfer can be successful or not. The patch gets rid also of own debug function (DPRINTF), replaced with the general debug(). Tested on Freescale mx35pdk. Signed-off-by: Stefano Babic CC: Heiko Schocher Acked-by: Heiko Schocher --- Changes: Wolfgang Denk: - change commit message explaining the problem and the changes - describe in commit message the drop of DPRINTF drivers/i2c/mxc_i2c.c | 86 ++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 68 insertions(+), 18 deletions(-) diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c index fd6db18..c5ec486 100755 --- a/drivers/i2c/mxc_i2c.c +++ b/drivers/i2c/mxc_i2c.c @@ -72,11 +72,8 @@ #error "define CONFIG_SYS_I2C_MX_PORTx to use the mx I2C driver" #endif -#ifdef DEBUG -#define DPRINTF(args...) printf(args) -#else -#define DPRINTF(args...) -#endif +#define I2C_MAX_TIMEOUT 10000 +#define I2C_MAX_RETRIES 3 static u16 div[] = { 30, 32, 36, 42, 48, 52, 60, 72, 80, 88, 104, 128, 144, 160, 192, 240, 288, 320, 384, 480, 576, 640, 768, 960, @@ -116,31 +113,61 @@ void i2c_init(int speed, int unused) i2c_reset(); } +static int wait_idle(void) +{ + int timeout = I2C_MAX_TIMEOUT; + + while ((readw(I2C_BASE + I2SR) & I2SR_IBB) && --timeout) { + writew(0, I2C_BASE + I2SR); + udelay(1); + } + return timeout ? timeout : (!(readw(I2C_BASE + I2SR) & I2SR_IBB)); +} + static int wait_busy(void) { - int timeout = 10000; + int timeout = I2C_MAX_TIMEOUT; - while (!(readw(I2C_BASE + I2SR) & I2SR_IIF) && --timeout) + while (!(readw(I2C_BASE + I2SR) & I2SR_IBB) && --timeout) udelay(1); writew(0, I2C_BASE + I2SR); /* clear interrupt */ return timeout; } +static int wait_complete(void) +{ + int timeout = I2C_MAX_TIMEOUT; + + while ((!(readw(I2C_BASE + I2SR) & I2SR_ICF)) && (--timeout)) { + writew(0, I2C_BASE + I2SR); + udelay(1); + } + udelay(200); + + writew(0, I2C_BASE + I2SR); /* clear interrupt */ + + return timeout; +} + + static int tx_byte(u8 byte) { writew(byte, I2C_BASE + I2DR); - if (!wait_busy() || readw(I2C_BASE + I2SR) & I2SR_RX_NO_AK) + if (!wait_complete() || readw(I2C_BASE + I2SR) & I2SR_RX_NO_AK) return -1; return 0; } -static int rx_byte(void) +static int rx_byte(int last) { - if (!wait_busy()) + if (!wait_complete()) return -1; + if (last) + writew(I2CR_IEN, I2C_BASE + I2CR); + return readw(I2C_BASE + I2DR); } @@ -160,21 +187,45 @@ int i2c_probe(uchar chip) static int i2c_addr(uchar chip, uint addr, int alen) { - writew(0, I2C_BASE + I2SR); - writew(I2CR_IEN | I2CR_MSTA | I2CR_MTX, I2C_BASE + I2CR); + int i, retry = 0; + for (retry = 0; retry < 3; retry++) { + if (wait_idle()) + break; + i2c_reset(); + for (i = 0; i < I2C_MAX_TIMEOUT; i++) + udelay(1); + } + if (retry >= I2C_MAX_RETRIES) { + debug("%s:bus is busy(%x)\n", + __func__, readw(I2C_BASE + I2SR)); + return -1; + } + writew(I2CR_IEN | I2CR_MSTA | I2CR_MTX, I2C_BASE + I2CR); - if (tx_byte(chip << 1)) + if (!wait_busy()) { + debug("%s:trigger start fail(%x)\n", + __func__, readw(I2C_BASE + I2SR)); return -1; + } + if (tx_byte(chip << 1) || (readw(I2C_BASE + I2SR) & I2SR_RX_NO_AK)) { + debug("%s:chip address cycle fail(%x)\n", + __func__, readw(I2C_BASE + I2SR)); + return -1; + } while (alen--) - if (tx_byte((addr >> (alen * 8)) & 0xff)) + if (tx_byte((addr >> (alen * 8)) & 0xff) || + (readw(I2C_BASE + I2SR) & I2SR_RX_NO_AK)) { + debug("%s:device address cycle fail(%x)\n", + __func__, readw(I2C_BASE + I2SR)); return -1; + } return 0; } int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len) { - int timeout = 10000; + int timeout = I2C_MAX_TIMEOUT; int ret; debug("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n", @@ -197,7 +248,8 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len) ret = readw(I2C_BASE + I2DR); while (len--) { - if ((ret = rx_byte()) < 0) + ret = rx_byte(len == 0); + if (ret < 0) return -1; *buf++ = ret; if (len <= 1) @@ -206,8 +258,6 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len) I2C_BASE + I2CR); } - wait_busy(); - writew(I2CR_IEN, I2C_BASE + I2CR); while (readw(I2C_BASE + I2SR) & I2SR_IBB && --timeout)