[U-Boot] [PATCH V4] I2C: mxc_i2c rework
Rewrite the mxc_i2c driver. * This version is much closer to Linux implementation. * Fixes IPG_PERCLK being incorrectly used as clock source * Fixes behaviour of the driver on iMX51 * Clean up coding style a bit ;-)
Based on commit: e39428d53d080ad2615b772d7f99d2a70c2aaab2 Date: Mon Jun 21 09:27:05 2010 +0200 i2c-imx: do not allow interruptions when waiting for I2C to complete
Signed-off-by: Marek Vasut marek.vasut@gmail.com --- drivers/i2c/mxc_i2c.c | 425 +++++++++++++++++++++++++++++++++---------------- 1 files changed, 290 insertions(+), 135 deletions(-)
V2: Convert register access to struct mxc_i2c_regs. V3: Update licensing info V3+: Add commit ID into commit message V4: Fix coding style error and fix ifdef issue with clock
diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c index 89d1973..8049c93 100644 --- a/drivers/i2c/mxc_i2c.c +++ b/drivers/i2c/mxc_i2c.c @@ -1,7 +1,15 @@ /* - * i2c driver for Freescale mx31 + * i2c driver for Freescale i.MX series * * (c) 2007 Pengutronix, Sascha Hauer s.hauer@pengutronix.de + * (c) 2011 Marek Vasut marek.vasut@gmail.com + * + * Based on i2c-imx.c from linux kernel: + * Copyright (C) 2005 Torsten Koschorrek <koschorrek at synertronixx.de> + * Copyright (C) 2005 Matthias Blaschke <blaschke at synertronixx.de> + * Copyright (C) 2007 RightHand Technologies, Inc. + * Copyright (C) 2008 Darius Augulis <darius.augulis at teltonika.lt> + * * * See file CREDITS for list of people who contributed to this * project. @@ -30,11 +38,13 @@ #include <asm/arch/clock.h> #include <asm/arch/imx-regs.h>
-#define IADR 0x00 -#define IFDR 0x04 -#define I2CR 0x08 -#define I2SR 0x0c -#define I2DR 0x10 +struct mxc_i2c_regs { + uint32_t iadr; + uint32_t ifdr; + uint32_t i2cr; + uint32_t i2sr; + uint32_t i2dr; +};
#define I2CR_IEN (1 << 7) #define I2CR_IIEN (1 << 6) @@ -68,218 +78,363 @@ #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, - 1152, 1280, 1536, 1920, 2304, 2560, 3072, 3840}; +static u16 i2c_clk_div[50][2] = { + { 22, 0x20 }, { 24, 0x21 }, { 26, 0x22 }, { 28, 0x23 }, + { 30, 0x00 }, { 32, 0x24 }, { 36, 0x25 }, { 40, 0x26 }, + { 42, 0x03 }, { 44, 0x27 }, { 48, 0x28 }, { 52, 0x05 }, + { 56, 0x29 }, { 60, 0x06 }, { 64, 0x2A }, { 72, 0x2B }, + { 80, 0x2C }, { 88, 0x09 }, { 96, 0x2D }, { 104, 0x0A }, + { 112, 0x2E }, { 128, 0x2F }, { 144, 0x0C }, { 160, 0x30 }, + { 192, 0x31 }, { 224, 0x32 }, { 240, 0x0F }, { 256, 0x33 }, + { 288, 0x10 }, { 320, 0x34 }, { 384, 0x35 }, { 448, 0x36 }, + { 480, 0x13 }, { 512, 0x37 }, { 576, 0x14 }, { 640, 0x38 }, + { 768, 0x39 }, { 896, 0x3A }, { 960, 0x17 }, { 1024, 0x3B }, + { 1152, 0x18 }, { 1280, 0x3C }, { 1536, 0x3D }, { 1792, 0x3E }, + { 1920, 0x1B }, { 2048, 0x3F }, { 2304, 0x1C }, { 2560, 0x1D }, + { 3072, 0x1E }, { 3840, 0x1F } +}; + +static u8 clk_idx;
-static inline void i2c_reset(void) -{ - writew(0, I2C_BASE + I2CR); /* Reset module */ - writew(0, I2C_BASE + I2SR); - writew(I2CR_IEN, I2C_BASE + I2CR); -} - -void i2c_init(int speed, int unused) +/* + * Calculate and set proper clock divider + */ +static void i2c_imx_set_clk(unsigned int rate) { - int freq; + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; + unsigned int i2c_clk_rate; + unsigned int div; int i;
#if defined(CONFIG_MX31) struct clock_control_regs *sc_regs = (struct clock_control_regs *)CCM_BASE;
- freq = mx31_get_ipg_clk(); /* start the required I2C clock */ writel(readl(&sc_regs->cgr0) | (3 << I2C_CLK_OFFSET), &sc_regs->cgr0); -#else - freq = mxc_get_clock(MXC_IPG_PERCLK); #endif
- for (i = 0; i < 0x1f; i++) - if (freq / div[i] <= speed) - break; + /* Divider value calculation */ + i2c_clk_rate = mxc_get_clock(MXC_IPG_CLK); + + div = (i2c_clk_rate + rate - 1) / rate; + if (div < i2c_clk_div[0][0]) + i = 0; + else if (div > i2c_clk_div[ARRAY_SIZE(i2c_clk_div) - 1][0]) + i = ARRAY_SIZE(i2c_clk_div) - 1; + else + for (i = 0; i2c_clk_div[i][0] < div; i++) + ; + + /* Store divider value */ + writeb(div, &i2c_regs->ifdr); + clk_idx = div; +}
- debug("%s: speed: %d\n", __func__, speed); +/* + * Reset I2C Controller + */ +void i2c_reset(void) +{ + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; + + writeb(0, &i2c_regs->i2cr); /* Reset module */ + writeb(0, &i2c_regs->i2sr); +}
- writew(i, I2C_BASE + IFDR); +/* + * Init I2C Bus + */ +void i2c_init(int speed, int unused) +{ + i2c_imx_set_clk(speed); i2c_reset(); }
-static int wait_idle(void) +/* + * Wait for bus to be busy (or free if for_busy = 0) + * + * for_busy = 1: Wait for IBB to be asserted + * for_busy = 0: Wait for IBB to be de-asserted + */ +int i2c_imx_bus_busy(int for_busy) { + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; + unsigned int temp; + int timeout = I2C_MAX_TIMEOUT;
- while ((readw(I2C_BASE + I2SR) & I2SR_IBB) && --timeout) { - writew(0, I2C_BASE + I2SR); + while (timeout--) { + temp = readb(&i2c_regs->i2sr); + + if (for_busy && (temp & I2SR_IBB)) + return 0; + if (!for_busy && !(temp & I2SR_IBB)) + return 0; + udelay(1); } - return timeout ? timeout : (!(readw(I2C_BASE + I2SR) & I2SR_IBB)); + + return 1; }
-static int wait_busy(void) +/* + * Wait for transaction to complete + */ +int i2c_imx_trx_complete(void) { + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; int timeout = I2C_MAX_TIMEOUT;
- while (!(readw(I2C_BASE + I2SR) & I2SR_IBB) && --timeout) + while (timeout--) { + if (readb(&i2c_regs->i2sr) & I2SR_IIF) { + writeb(0, &i2c_regs->i2sr); + return 0; + } + udelay(1); - writew(0, I2C_BASE + I2SR); /* clear interrupt */ + }
- return timeout; + return 1; }
-static int wait_complete(void) +/* + * Check if the transaction was ACKed + */ +int i2c_imx_acked(void) { - int timeout = I2C_MAX_TIMEOUT; + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
- while ((!(readw(I2C_BASE + I2SR) & I2SR_ICF)) && (--timeout)) { - writew(0, I2C_BASE + I2SR); - udelay(1); - } - udelay(200); + return readb(&i2c_regs->i2sr) & I2SR_RX_NO_AK; +}
- writew(0, I2C_BASE + I2SR); /* clear interrupt */ +/* + * Start the controller + */ +int i2c_imx_start(void) +{ + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; + unsigned int temp = 0; + int result;
- return timeout; -} + writeb(clk_idx, &i2c_regs->ifdr);
+ /* Enable I2C controller */ + writeb(0, &i2c_regs->i2sr); + writeb(I2CR_IEN, &i2c_regs->i2cr);
-static int tx_byte(u8 byte) -{ - writew(byte, I2C_BASE + I2DR); + /* Wait controller to be stable */ + udelay(50); + + /* Start I2C transaction */ + temp = readb(&i2c_regs->i2cr); + temp |= I2CR_MSTA; + writeb(temp, &i2c_regs->i2cr); + + result = i2c_imx_bus_busy(1); + if (result) + return result; + + temp |= I2CR_MTX | I2CR_TX_NO_AK; + writeb(temp, &i2c_regs->i2cr);
- if (!wait_complete() || readw(I2C_BASE + I2SR) & I2SR_RX_NO_AK) - return -1; return 0; }
-static int rx_byte(int last) +/* + * Stop the controller + */ +void i2c_imx_stop() { - if (!wait_complete()) - return -1; + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; + unsigned int temp = 0; + + /* Stop I2C transaction */ + temp = readb(&i2c_regs->i2cr); + temp |= ~(I2CR_MSTA | I2CR_MTX); + writeb(temp, &i2c_regs->i2cr);
- if (last) - writew(I2CR_IEN, I2C_BASE + I2CR); + i2c_imx_bus_busy(0);
- return readw(I2C_BASE + I2DR); + /* Disable I2C controller */ + writeb(0, &i2c_regs->i2cr); }
-int i2c_probe(uchar chip) +/* + * Set chip address and access mode + * + * read = 1: READ access + * read = 0: WRITE access + */ +int i2c_imx_set_chip_addr(uchar chip, int read) { + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; int ret;
- writew(0, I2C_BASE + I2CR); /* Reset module */ - writew(I2CR_IEN, I2C_BASE + I2CR); + writeb((chip << 1) | read, &i2c_regs->i2dr); + + ret = i2c_imx_trx_complete(); + if (ret) + return ret;
- writew(I2CR_IEN | I2CR_MSTA | I2CR_MTX, I2C_BASE + I2CR); - ret = tx_byte(chip << 1); - writew(I2CR_IEN | I2CR_MTX, I2C_BASE + I2CR); + ret = i2c_imx_acked(); + if (ret) + return ret;
return ret; }
-static int i2c_addr(uchar chip, uint addr, int alen) +/* + * Write register address + */ +int i2c_imx_set_reg_addr(uint addr, int alen) { - int i, retry = 0; - for (retry = 0; retry < 3; retry++) { - if (wait_idle()) + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; + int ret; + int i; + + for (i = 0; i < (8 * alen); i += 8) { + writeb((addr >> i) & 0xff, &i2c_regs->i2dr); + + ret = i2c_imx_trx_complete(); + if (ret) 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 (!wait_busy()) { - debug("%s:trigger start fail(%x)\n", - __func__, readw(I2C_BASE + I2SR)); - return -1; + ret = i2c_imx_acked(); + if (ret) + break; }
- 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) || - (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; + return ret; }
-int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len) +/* + * Try if a chip add given address responds (probe the chip) + */ +int i2c_probe(uchar chip) { - int timeout = I2C_MAX_TIMEOUT; int ret;
- debug("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n", - __func__, chip, addr, alen, len); + ret = i2c_imx_start(); + if (ret) + return ret;
- if (i2c_addr(chip, addr, alen)) { - printf("i2c_addr failed\n"); - return -1; - } + ret = i2c_imx_set_chip_addr(chip, 0); + if (ret) + return ret;
- writew(I2CR_IEN | I2CR_MSTA | I2CR_MTX | I2CR_RSTA, I2C_BASE + I2CR); + i2c_imx_stop();
- if (tx_byte(chip << 1 | 1)) - return -1; + return ret; +}
- writew(I2CR_IEN | I2CR_MSTA | - ((len == 1) ? I2CR_TX_NO_AK : 0), - I2C_BASE + I2CR); +/* + * Read data from I2C device + */ +int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len) +{ + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; + int ret; + unsigned int temp; + int i;
- ret = readw(I2C_BASE + I2DR); + ret = i2c_imx_start(); + if (ret) + return ret; + + /* write slave address */ + ret = i2c_imx_set_chip_addr(chip, 0); + if (ret) + return ret; + + ret = i2c_imx_set_reg_addr(addr, alen); + if (ret) + return ret; + + temp = readb(&i2c_regs->i2cr); + temp |= I2CR_RSTA; + writeb(temp, &i2c_regs->i2cr); + + ret = i2c_imx_set_chip_addr(chip, 1); + if (ret) + return ret; + + /* setup bus to read data */ + temp = readb(&i2c_regs->i2cr); + temp &= ~I2CR_MTX; + if (len == 1) + temp &= ~I2CR_TX_NO_AK; + writeb(temp, &i2c_regs->i2cr); + readb(&i2c_regs->i2dr); + + /* read data */ + for (i = 0; i < len; i++) { + ret = i2c_imx_trx_complete(); + if (ret) + return ret; + + /* + * It must generate STOP before read I2DR to prevent + * controller from generating another clock cycle + */ + if (i == (len - 1)) { + temp = readb(&i2c_regs->i2cr); + temp &= ~(I2CR_MSTA | I2CR_MTX); + writeb(temp, &i2c_regs->i2cr); + i2c_imx_bus_busy(0); + } else if (i == (len - 2)) { + temp = readb(&i2c_regs->i2cr); + temp |= I2CR_TX_NO_AK; + writeb(temp, &i2c_regs->i2cr); + }
- while (len--) { - ret = rx_byte(len == 0); - if (ret < 0) - return -1; - *buf++ = ret; - if (len <= 1) - writew(I2CR_IEN | I2CR_MSTA | - I2CR_TX_NO_AK, - I2C_BASE + I2CR); + buf[i] = readb(&i2c_regs->i2dr); }
- writew(I2CR_IEN, I2C_BASE + I2CR); - - while (readw(I2C_BASE + I2SR) & I2SR_IBB && --timeout) - udelay(1); + i2c_imx_stop();
- return 0; + return ret; }
+/* + * Write data to I2C device + */ int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len) { - int timeout = I2C_MAX_TIMEOUT; - debug("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n", - __func__, chip, addr, alen, len); + struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; + int ret; + unsigned int temp; + int i;
- if (i2c_addr(chip, addr, alen)) - return -1; + ret = i2c_imx_start(); + if (ret) + return ret;
- while (len--) - if (tx_byte(*buf++)) - return -1; + /* write slave address */ + ret = i2c_imx_set_chip_addr(chip, 0); + if (ret) + return ret;
- writew(I2CR_IEN, I2C_BASE + I2CR); + ret = i2c_imx_set_reg_addr(addr, alen); + if (ret) + return ret;
- while (readw(I2C_BASE + I2SR) & I2SR_IBB && --timeout) - udelay(1); + for (i = 0; i < len; i++) { + writeb(buf[i], &i2c_regs->i2dr);
- return 0; -} + ret = i2c_imx_trx_complete(); + if (ret) + return ret; + + ret = i2c_imx_acked(); + if (ret) + return ret; + }
+ i2c_imx_stop(); + + return ret; +} #endif /* CONFIG_HARD_I2C */
Hi, Marek,
On Fri, Jul 29, 2011 at 5:32 PM, Marek Vasut marek.vasut@gmail.com wrote:
Rewrite the mxc_i2c driver. * This version is much closer to Linux implementation. * Fixes IPG_PERCLK being incorrectly used as clock source * Fixes behaviour of the driver on iMX51 * Clean up coding style a bit ;-)
I don't think you did the right thing by chaning IPG_PERCLK to IPG_CLK As I said, the IPG_CLK is for IP register clock, while IPG_PERCLK is for i2c function clock.
if you run clock command from mx51evk, you will get: ... ipg clock : 66500000Hz ipg per clock : 665000000Hz MX51EVK U-Boot >
It will give you that ipg per clock is 665M, which seems too big. It is due to we configure the pre-divider/pos-devider for perclk to zero, which leads to ipg_per clock to be same as pll2 clock. But I don't think this will have some issue.
BTW, I have applied your patch and test on mx53evk board, it seems the i2c does not work correctly. After apply your patch: MX53EVK U-Boot > pmic dump 30 PMIC ID: 0x0000ffff [Rev: unknown]
0x00: 0001ffff 00ffffff 0039ffff 0000ffff 00ffffff 0000ffff 0000ffff 0000ffff 0x08: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0040ffff 0x10: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0001ffff 0000ffff 0000ffff 0x18: 0045ffff 0045ffff 0000ffff 0080ffff 0021ffff 0000ffff 0002ffff 0000ffff 0x20: 0004ffff 0000ffff 0021ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0x28: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0004ffff 0000ffff 00aeffff
The old: MX53EVK U-Boot > pmic dump 30 PMIC ID: 0x000045d0 [Rev: 2.0]
0x00: 00015088 00ffffff 00395208 00000081 00fff7ff 0000401c 00000418 000045d0 0x08: 00000000 00000000 00000001 00000000 00000000 00000040 00000000 00400000 0x10: 00000000 00000000 00000000 00000000 00000011 0001ffff 00000000 00007fff 0x18: 00454a52 00456739 0000631a 0080739c 0021284a 00000a0a 00024fd0 000001d8 0x20: 00049208 00000000 00218000 00000000 00000000 00000000 00000000 00000000 0x28: 00000000 00000000 00000000 00008000 00000000 00046046 000001c0 00aeeaee
Jason
On Friday, July 29, 2011 01:24:49 PM Jason Hui wrote:
Hi, Marek,
On Fri, Jul 29, 2011 at 5:32 PM, Marek Vasut marek.vasut@gmail.com wrote:
Rewrite the mxc_i2c driver.
- This version is much closer to Linux implementation.
- Fixes IPG_PERCLK being incorrectly used as clock source
- Fixes behaviour of the driver on iMX51
- Clean up coding style a bit ;-)
I don't think you did the right thing by chaning IPG_PERCLK to IPG_CLK As I said, the IPG_CLK is for IP register clock, while IPG_PERCLK is for i2c function clock.
if you run clock command from mx51evk, you will get: ... ipg clock : 66500000Hz ipg per clock : 665000000Hz MX51EVK U-Boot >
It will give you that ipg per clock is 665M, which seems too big. It is due to we configure the pre-divider/pos-devider for perclk to zero, which leads to ipg_per clock to be same as pll2 clock. But I don't think this will have some issue.
Yes it will, the divider will be computed to be maximum in all cases ... you won't be able it divide 665MHz by anything from the table 40-7 in MX51RM to achieve any reasonable frequency.
On the contrary, 66.5MHz does give fine results.
BTW, I have applied your patch and test on mx53evk board, it seems the i2c does not work correctly.
Great. The clock used by the I2C module for this task are likely the module_clock, which are -- like on MX51 -- 66.5MHz. What do you get when you run the "clock" command on MX53EVK ?
Thanks
After apply your patch: MX53EVK U-Boot > pmic dump 30 PMIC ID: 0x0000ffff [Rev: unknown]
0x00: 0001ffff 00ffffff 0039ffff 0000ffff 00ffffff 0000ffff 0000ffff 0000ffff 0x08: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0040ffff 0x10: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0001ffff 0000ffff 0000ffff 0x18: 0045ffff 0045ffff 0000ffff 0080ffff 0021ffff 0000ffff 0002ffff 0000ffff 0x20: 0004ffff 0000ffff 0021ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0x28: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0004ffff 0000ffff 00aeffff
The old: MX53EVK U-Boot > pmic dump 30 PMIC ID: 0x000045d0 [Rev: 2.0]
0x00: 00015088 00ffffff 00395208 00000081 00fff7ff 0000401c 00000418 000045d0 0x08: 00000000 00000000 00000001 00000000 00000000 00000040 00000000 00400000 0x10: 00000000 00000000 00000000 00000000 00000011 0001ffff 00000000 00007fff 0x18: 00454a52 00456739 0000631a 0080739c 0021284a 00000a0a 00024fd0 000001d8 0x20: 00049208 00000000 00218000 00000000 00000000 00000000 00000000 00000000 0x28: 00000000 00000000 00000000 00008000 00000000 00046046 000001c0 00aeeaee
Jason
Hi, Marek,
On Sat, Jul 30, 2011 at 1:09 AM, Marek Vasut marek.vasut@gmail.com wrote:
On Friday, July 29, 2011 01:24:49 PM Jason Hui wrote:
Hi, Marek,
On Fri, Jul 29, 2011 at 5:32 PM, Marek Vasut marek.vasut@gmail.com wrote:
Rewrite the mxc_i2c driver. * This version is much closer to Linux implementation. * Fixes IPG_PERCLK being incorrectly used as clock source * Fixes behaviour of the driver on iMX51 * Clean up coding style a bit ;-)
I don't think you did the right thing by chaning IPG_PERCLK to IPG_CLK As I said, the IPG_CLK is for IP register clock, while IPG_PERCLK is for i2c function clock.
if you run clock command from mx51evk, you will get: ... ipg clock : 66500000Hz ipg per clock : 665000000Hz MX51EVK U-Boot >
It will give you that ipg per clock is 665M, which seems too big. It is due to we configure the pre-divider/pos-devider for perclk to zero, which leads to ipg_per clock to be same as pll2 clock. But I don't think this will have some issue.
Yes it will, the divider will be computed to be maximum in all cases ... you won't be able it divide 665MHz by anything from the table 40-7 in MX51RM to achieve any reasonable frequency.
On the contrary, 66.5MHz does give fine results.
it that, we can change the ipg_perclk to low freq, but we should not change the i2c clock to ipg_clk, this is not correct.
BTW, I have applied your patch and test on mx53evk board, it seems the i2c does not work correctly.
Great. The clock used by the I2C module for this task are likely the module_clock, which are -- like on MX51 -- 66.5MHz. What do you get when you run the "clock" command on MX53EVK ?
I'm not in the office for the whole next week, thus, I can't give you the clock output for mx53evk, but I'm sure the ipg_perclk on mx53evk is not set at 66.5Mhz.
Jason
Thanks
After apply your patch: MX53EVK U-Boot > pmic dump 30 PMIC ID: 0x0000ffff [Rev: unknown]
0x00: 0001ffff 00ffffff 0039ffff 0000ffff 00ffffff 0000ffff 0000ffff 0000ffff 0x08: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0040ffff 0x10: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0001ffff 0000ffff 0000ffff 0x18: 0045ffff 0045ffff 0000ffff 0080ffff 0021ffff 0000ffff 0002ffff 0000ffff 0x20: 0004ffff 0000ffff 0021ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0x28: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0004ffff 0000ffff 00aeffff
The old: MX53EVK U-Boot > pmic dump 30 PMIC ID: 0x000045d0 [Rev: 2.0]
0x00: 00015088 00ffffff 00395208 00000081 00fff7ff 0000401c 00000418 000045d0 0x08: 00000000 00000000 00000001 00000000 00000000 00000040 00000000 00400000 0x10: 00000000 00000000 00000000 00000000 00000011 0001ffff 00000000 00007fff 0x18: 00454a52 00456739 0000631a 0080739c 0021284a 00000a0a 00024fd0 000001d8 0x20: 00049208 00000000 00218000 00000000 00000000 00000000 00000000 00000000 0x28: 00000000 00000000 00000000 00008000 00000000 00046046 000001c0 00aeeaee
Jason
On Saturday, July 30, 2011 08:42:19 AM Jason Hui wrote:
Hi, Marek,
On Sat, Jul 30, 2011 at 1:09 AM, Marek Vasut marek.vasut@gmail.com wrote:
On Friday, July 29, 2011 01:24:49 PM Jason Hui wrote:
Hi, Marek,
On Fri, Jul 29, 2011 at 5:32 PM, Marek Vasut marek.vasut@gmail.com wrote:
Rewrite the mxc_i2c driver.
- This version is much closer to Linux implementation.
- Fixes IPG_PERCLK being incorrectly used as clock source
- Fixes behaviour of the driver on iMX51
- Clean up coding style a bit ;-)
I don't think you did the right thing by chaning IPG_PERCLK to IPG_CLK As I said, the IPG_CLK is for IP register clock, while IPG_PERCLK is for i2c function clock.
if you run clock command from mx51evk, you will get: ... ipg clock : 66500000Hz ipg per clock : 665000000Hz MX51EVK U-Boot >
It will give you that ipg per clock is 665M, which seems too big. It is due to we configure the pre-divider/pos-devider for perclk to zero, which leads to ipg_per clock to be same as pll2 clock. But I don't think this will have some issue.
Yes it will, the divider will be computed to be maximum in all cases ... you won't be able it divide 665MHz by anything from the table 40-7 in MX51RM to achieve any reasonable frequency.
On the contrary, 66.5MHz does give fine results.
it that, we can change the ipg_perclk to low freq, but we should not change the i2c clock to ipg_clk, this is not correct.
BTW, I have applied your patch and test on mx53evk board, it seems the i2c does not work correctly.
Great. The clock used by the I2C module for this task are likely the module_clock, which are -- like on MX51 -- 66.5MHz. What do you get when you run the "clock" command on MX53EVK ?
I'm not in the office for the whole next week, thus, I can't give you the clock output for mx53evk, but I'm sure the ipg_perclk on mx53evk is not set at 66.5Mhz.
Yes, it's 33.3MHz on iMX53. I got a (remote) hand of a MX53 board. The IPG clock, on the other hand, are 66.5MHz on both MX51 and MX53. Maybe the issue with PMIC you're seeing is something else?
btw. how did you obtain these results ? Can you check the FDR divider value with and without this patch?
Thanks
Jason
Thanks
After apply your patch: MX53EVK U-Boot > pmic dump 30 PMIC ID: 0x0000ffff [Rev: unknown]
0x00: 0001ffff 00ffffff 0039ffff 0000ffff 00ffffff 0000ffff 0000ffff 0000ffff 0x08: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0040ffff 0x10: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0001ffff 0000ffff 0000ffff 0x18: 0045ffff 0045ffff 0000ffff 0080ffff 0021ffff 0000ffff 0002ffff 0000ffff 0x20: 0004ffff 0000ffff 0021ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0x28: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0004ffff 0000ffff 00aeffff
The old: MX53EVK U-Boot > pmic dump 30 PMIC ID: 0x000045d0 [Rev: 2.0]
0x00: 00015088 00ffffff 00395208 00000081 00fff7ff 0000401c 00000418 000045d0 0x08: 00000000 00000000 00000001 00000000 00000000 00000040 00000000 00400000 0x10: 00000000 00000000 00000000 00000000 00000011 0001ffff 00000000 00007fff 0x18: 00454a52 00456739 0000631a 0080739c 0021284a 00000a0a 00024fd0 000001d8 0x20: 00049208 00000000 00218000 00000000 00000000 00000000 00000000 00000000 0x28: 00000000 00000000 00000000 00008000 00000000 00046046 000001c0 00aeeaee
Jason
On Saturday, July 30, 2011 12:52:53 PM Marek Vasut wrote:
On Saturday, July 30, 2011 08:42:19 AM Jason Hui wrote:
Hi, Marek,
On Sat, Jul 30, 2011 at 1:09 AM, Marek Vasut marek.vasut@gmail.com wrote:
On Friday, July 29, 2011 01:24:49 PM Jason Hui wrote:
Hi, Marek,
On Fri, Jul 29, 2011 at 5:32 PM, Marek Vasut marek.vasut@gmail.com
wrote:
Rewrite the mxc_i2c driver.
- This version is much closer to Linux implementation.
- Fixes IPG_PERCLK being incorrectly used as clock source
- Fixes behaviour of the driver on iMX51
- Clean up coding style a bit ;-)
I don't think you did the right thing by chaning IPG_PERCLK to IPG_CLK As I said, the IPG_CLK is for IP register clock, while IPG_PERCLK is for i2c function clock.
if you run clock command from mx51evk, you will get: ... ipg clock : 66500000Hz ipg per clock : 665000000Hz MX51EVK U-Boot >
It will give you that ipg per clock is 665M, which seems too big. It is due to we configure the pre-divider/pos-devider for perclk to zero, which leads to ipg_per clock to be same as pll2 clock. But I don't think this will have some issue.
Yes it will, the divider will be computed to be maximum in all cases ... you won't be able it divide 665MHz by anything from the table 40-7 in MX51RM to achieve any reasonable frequency.
On the contrary, 66.5MHz does give fine results.
it that, we can change the ipg_perclk to low freq, but we should not change the i2c clock to ipg_clk, this is not correct.
BTW, I have applied your patch and test on mx53evk board, it seems the i2c does not work correctly.
Great. The clock used by the I2C module for this task are likely the module_clock, which are -- like on MX51 -- 66.5MHz. What do you get when you run the "clock" command on MX53EVK ?
I'm not in the office for the whole next week, thus, I can't give you the clock output for mx53evk, but I'm sure the ipg_perclk on mx53evk is not set at 66.5Mhz.
Yes, it's 33.3MHz on iMX53. I got a (remote) hand of a MX53 board. The IPG clock, on the other hand, are 66.5MHz on both MX51 and MX53. Maybe the issue with PMIC you're seeing is something else?
btw. how did you obtain these results ? Can you check the FDR divider value with and without this patch?
Thanks
BUMP
Jason
Thanks
After apply your patch: MX53EVK U-Boot > pmic dump 30 PMIC ID: 0x0000ffff [Rev: unknown]
0x00: 0001ffff 00ffffff 0039ffff 0000ffff 00ffffff 0000ffff 0000ffff 0000ffff 0x08: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0040ffff 0x10: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0001ffff 0000ffff 0000ffff 0x18: 0045ffff 0045ffff 0000ffff 0080ffff 0021ffff 0000ffff 0002ffff 0000ffff 0x20: 0004ffff 0000ffff 0021ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0x28: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0004ffff 0000ffff 00aeffff
The old: MX53EVK U-Boot > pmic dump 30 PMIC ID: 0x000045d0 [Rev: 2.0]
0x00: 00015088 00ffffff 00395208 00000081 00fff7ff 0000401c 00000418 000045d0 0x08: 00000000 00000000 00000001 00000000 00000000 00000040 00000000 00400000 0x10: 00000000 00000000 00000000 00000000 00000011 0001ffff 00000000 00007fff 0x18: 00454a52 00456739 0000631a 0080739c 0021284a 00000a0a 00024fd0 000001d8 0x20: 00049208 00000000 00218000 00000000 00000000 00000000 00000000 00000000 0x28: 00000000 00000000 00000000 00008000 00000000 00046046 000001c0 00aeeaee
Jason
On Wed, Aug 10, 2011 at 1:59 PM, Marek Vasut marek.vasut@gmail.com wrote:
On Saturday, July 30, 2011 12:52:53 PM Marek Vasut wrote:
On Saturday, July 30, 2011 08:42:19 AM Jason Hui wrote:
Hi, Marek,
On Sat, Jul 30, 2011 at 1:09 AM, Marek Vasut marek.vasut@gmail.com wrote:
On Friday, July 29, 2011 01:24:49 PM Jason Hui wrote:
Hi, Marek,
On Fri, Jul 29, 2011 at 5:32 PM, Marek Vasut marek.vasut@gmail.com
wrote:
Rewrite the mxc_i2c driver.
* This version is much closer to Linux implementation. * Fixes IPG_PERCLK being incorrectly used as clock source * Fixes behaviour of the driver on iMX51 * Clean up coding style a bit ;-)
I don't think you did the right thing by chaning IPG_PERCLK to IPG_CLK As I said, the IPG_CLK is for IP register clock, while IPG_PERCLK is for i2c function clock.
if you run clock command from mx51evk, you will get: ... ipg clock : 66500000Hz ipg per clock : 665000000Hz MX51EVK U-Boot >
It will give you that ipg per clock is 665M, which seems too big. It is due to we configure the pre-divider/pos-devider for perclk to zero, which leads to ipg_per clock to be same as pll2 clock. But I don't think this will have some issue.
Yes it will, the divider will be computed to be maximum in all cases ... you won't be able it divide 665MHz by anything from the table 40-7 in MX51RM to achieve any reasonable frequency.
On the contrary, 66.5MHz does give fine results.
it that, we can change the ipg_perclk to low freq, but we should not change the i2c clock to ipg_clk, this is not correct.
BTW, I have applied your patch and test on mx53evk board, it seems the i2c does not work correctly.
Great. The clock used by the I2C module for this task are likely the module_clock, which are -- like on MX51 -- 66.5MHz. What do you get when you run the "clock" command on MX53EVK ?
I'm not in the office for the whole next week, thus, I can't give you the clock output for mx53evk, but I'm sure the ipg_perclk on mx53evk is not set at 66.5Mhz.
Yes, it's 33.3MHz on iMX53. I got a (remote) hand of a MX53 board. The IPG clock, on the other hand, are 66.5MHz on both MX51 and MX53. Maybe the issue with PMIC you're seeing is something else?
btw. how did you obtain these results ? Can you check the FDR divider value with and without this patch?
You can use the clock command to get it. As for the FDR, you want me to check it on i.mx53evk board right?
Jason
Thanks
Jason
Thanks
After apply your patch: MX53EVK U-Boot > pmic dump 30 PMIC ID: 0x0000ffff [Rev: unknown]
0x00: 0001ffff 00ffffff 0039ffff 0000ffff 00ffffff 0000ffff 0000ffff 0000ffff 0x08: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0040ffff 0x10: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0001ffff 0000ffff 0000ffff 0x18: 0045ffff 0045ffff 0000ffff 0080ffff 0021ffff 0000ffff 0002ffff 0000ffff 0x20: 0004ffff 0000ffff 0021ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0x28: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0004ffff 0000ffff 00aeffff
The old: MX53EVK U-Boot > pmic dump 30 PMIC ID: 0x000045d0 [Rev: 2.0]
0x00: 00015088 00ffffff 00395208 00000081 00fff7ff 0000401c 00000418 000045d0 0x08: 00000000 00000000 00000001 00000000 00000000 00000040 00000000 00400000 0x10: 00000000 00000000 00000000 00000000 00000011 0001ffff 00000000 00007fff 0x18: 00454a52 00456739 0000631a 0080739c 0021284a 00000a0a 00024fd0 000001d8 0x20: 00049208 00000000 00218000 00000000 00000000 00000000 00000000 00000000 0x28: 00000000 00000000 00000000 00008000 00000000 00046046 000001c0 00aeeaee
Jason
On Wednesday, August 10, 2011 08:26:05 AM Jason Hui wrote:
On Wed, Aug 10, 2011 at 1:59 PM, Marek Vasut marek.vasut@gmail.com wrote:
On Saturday, July 30, 2011 12:52:53 PM Marek Vasut wrote:
On Saturday, July 30, 2011 08:42:19 AM Jason Hui wrote:
Hi, Marek,
On Sat, Jul 30, 2011 at 1:09 AM, Marek Vasut marek.vasut@gmail.com
wrote:
On Friday, July 29, 2011 01:24:49 PM Jason Hui wrote:
Hi, Marek,
On Fri, Jul 29, 2011 at 5:32 PM, Marek Vasut marek.vasut@gmail.com
wrote:
> Rewrite the mxc_i2c driver. > > * This version is much closer to Linux implementation. > * Fixes IPG_PERCLK being incorrectly used as clock source > * Fixes behaviour of the driver on iMX51 > * Clean up coding style a bit ;-)
I don't think you did the right thing by chaning IPG_PERCLK to IPG_CLK As I said, the IPG_CLK is for IP register clock, while IPG_PERCLK is for i2c function clock.
if you run clock command from mx51evk, you will get: ... ipg clock : 66500000Hz ipg per clock : 665000000Hz MX51EVK U-Boot >
It will give you that ipg per clock is 665M, which seems too big. It is due to we configure the pre-divider/pos-devider for perclk to zero, which leads to ipg_per clock to be same as pll2 clock. But I don't think this will have some issue.
Yes it will, the divider will be computed to be maximum in all cases ... you won't be able it divide 665MHz by anything from the table 40-7 in MX51RM to achieve any reasonable frequency.
On the contrary, 66.5MHz does give fine results.
it that, we can change the ipg_perclk to low freq, but we should not change the i2c clock to ipg_clk, this is not correct.
BTW, I have applied your patch and test on mx53evk board, it seems the i2c does not work correctly.
Great. The clock used by the I2C module for this task are likely the module_clock, which are -- like on MX51 -- 66.5MHz. What do you get when you run the "clock" command on MX53EVK ?
I'm not in the office for the whole next week, thus, I can't give you the clock output for mx53evk, but I'm sure the ipg_perclk on mx53evk is not set at 66.5Mhz.
Yes, it's 33.3MHz on iMX53. I got a (remote) hand of a MX53 board. The IPG clock, on the other hand, are 66.5MHz on both MX51 and MX53. Maybe the issue with PMIC you're seeing is something else?
btw. how did you obtain these results ? Can you check the FDR divider value with and without this patch?
You can use the clock command to get it. As for the FDR, you want me to check it on i.mx53evk board right?
Yes please. I hope to get my hands on mx53 system in the evening probably, but I'd still be grateful if you could test it.
Jason
Thanks
Jason
Thanks
After apply your patch: MX53EVK U-Boot > pmic dump 30 PMIC ID: 0x0000ffff [Rev: unknown]
0x00: 0001ffff 00ffffff 0039ffff 0000ffff 00ffffff 0000ffff 0000ffff 0000ffff 0x08: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0040ffff 0x10: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0001ffff 0000ffff 0000ffff 0x18: 0045ffff 0045ffff 0000ffff 0080ffff 0021ffff 0000ffff 0002ffff 0000ffff 0x20: 0004ffff 0000ffff 0021ffff 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0x28: 0000ffff 0000ffff 0000ffff 0000ffff 0000ffff 0004ffff 0000ffff 00aeffff
The old: MX53EVK U-Boot > pmic dump 30 PMIC ID: 0x000045d0 [Rev: 2.0]
0x00: 00015088 00ffffff 00395208 00000081 00fff7ff 0000401c 00000418 000045d0 0x08: 00000000 00000000 00000001 00000000 00000000 00000040 00000000 00400000 0x10: 00000000 00000000 00000000 00000000 00000011 0001ffff 00000000 00007fff 0x18: 00454a52 00456739 0000631a 0080739c 0021284a 00000a0a 00024fd0 000001d8 0x20: 00049208 00000000 00218000 00000000 00000000 00000000 00000000 00000000 0x28: 00000000 00000000 00000000 00008000 00000000 00046046 000001c0 00aeeaee
Jason
Hi, Marek,
On Wed, Aug 10, 2011 at 3:11 PM, Marek Vasut marek.vasut@gmail.com wrote:
On Wednesday, August 10, 2011 08:26:05 AM Jason Hui wrote:
On Wed, Aug 10, 2011 at 1:59 PM, Marek Vasut marek.vasut@gmail.com wrote:
On Saturday, July 30, 2011 12:52:53 PM Marek Vasut wrote:
On Saturday, July 30, 2011 08:42:19 AM Jason Hui wrote:
Hi, Marek,
On Sat, Jul 30, 2011 at 1:09 AM, Marek Vasut marek.vasut@gmail.com
wrote:
On Friday, July 29, 2011 01:24:49 PM Jason Hui wrote: > Hi, Marek, > > On Fri, Jul 29, 2011 at 5:32 PM, Marek Vasut > marek.vasut@gmail.com
wrote:
> > Rewrite the mxc_i2c driver. > > > > * This version is much closer to Linux implementation. > > * Fixes IPG_PERCLK being incorrectly used as clock source > > * Fixes behaviour of the driver on iMX51 > > * Clean up coding style a bit ;-) > > I don't think you did the right thing by chaning IPG_PERCLK to > IPG_CLK As I said, the IPG_CLK is for IP register clock, while > IPG_PERCLK is for i2c function clock. > > if you run clock command from mx51evk, you will get: > ... > ipg clock : 66500000Hz > ipg per clock : 665000000Hz > MX51EVK U-Boot > > > It will give you that ipg per clock is 665M, which seems too big. > It is due to we configure > the pre-divider/pos-devider for perclk to zero, which leads to > ipg_per clock to be same as pll2 clock. > But I don't think this will have some issue.
Yes it will, the divider will be computed to be maximum in all cases ... you won't be able it divide 665MHz by anything from the table 40-7 in MX51RM to achieve any reasonable frequency.
On the contrary, 66.5MHz does give fine results.
it that, we can change the ipg_perclk to low freq, but we should not change the i2c clock to ipg_clk, this is not correct.
> BTW, I have applied your patch and test on mx53evk board, it seems > the i2c does not work correctly.
Great. The clock used by the I2C module for this task are likely the module_clock, which are -- like on MX51 -- 66.5MHz. What do you get when you run the "clock" command on MX53EVK ?
I'm not in the office for the whole next week, thus, I can't give you the clock output for mx53evk, but I'm sure the ipg_perclk on mx53evk is not set at 66.5Mhz.
Yes, it's 33.3MHz on iMX53. I got a (remote) hand of a MX53 board. The IPG clock, on the other hand, are 66.5MHz on both MX51 and MX53. Maybe the issue with PMIC you're seeing is something else?
btw. how did you obtain these results ? Can you check the FDR divider value with and without this patch?
You can use the clock command to get it. As for the FDR, you want me to check it on i.mx53evk board right?
Yes please. I hope to get my hands on mx53 system in the evening probably, but I'd still be grateful if you could test it.
The clock for i2c should be ipg_perclk, the linux kernel code has been fixed by the following commit:
commit 9d73242458d9a2fe26e2e240488063d414eacb1c Author: Lothar Waßmann LW@KARO-electronics.de Date: Mon Jul 4 15:52:17 2011 +0200
mach-mx5: fix the I2C clock parents
The clock from which the I2C timing is derived is the ipg_perclk not ipg_clk.
I2C bus frequency was lower by a factor of ~8 due to the clock divider calculation being based on 66.5MHz IPG clock while the bus actually uses 8MHz ipg_perclk.
Kernel version: 3.0.0-rc2 branch 'imx-for-next' of git://git.pengutronix.de/git/imx/linux-2.6
Signed-off-by: Lothar Waßmann LW@KARO-electronics.de Signed-off-by: Sascha Hauer s.hauer@pengutronix.de
Jason
Jason
Thanks
Jason
Thanks
> After apply your patch: > MX53EVK U-Boot > pmic dump 30 > PMIC ID: 0x0000ffff [Rev: unknown] > > 0x00: 0001ffff 00ffffff 0039ffff 0000ffff 00ffffff 0000ffff > 0000ffff 0000ffff 0x08: 0000ffff 0000ffff 0000ffff 0000ffff > 0000ffff 0000ffff 0000ffff 0040ffff 0x10: 0000ffff 0000ffff > 0000ffff 0000ffff 0000ffff 0001ffff 0000ffff 0000ffff 0x18: > 0045ffff 0045ffff 0000ffff 0080ffff 0021ffff 0000ffff 0002ffff > 0000ffff 0x20: 0004ffff 0000ffff 0021ffff 0000ffff 0000ffff > 0000ffff 0000ffff 0000ffff 0x28: 0000ffff 0000ffff 0000ffff > 0000ffff 0000ffff 0004ffff 0000ffff 00aeffff > > The old: > MX53EVK U-Boot > pmic dump 30 > PMIC ID: 0x000045d0 [Rev: 2.0] > > 0x00: 00015088 00ffffff 00395208 00000081 00fff7ff 0000401c > 00000418 000045d0 0x08: 00000000 00000000 00000001 00000000 > 00000000 00000040 00000000 00400000 0x10: 00000000 00000000 > 00000000 00000000 00000011 0001ffff 00000000 00007fff 0x18: > 00454a52 00456739 0000631a 0080739c 0021284a 00000a0a 00024fd0 > 000001d8 0x20: 00049208 00000000 00218000 00000000 00000000 > 00000000 00000000 00000000 0x28: 00000000 00000000 00000000 > 00008000 00000000 00046046 000001c0 00aeeaee > > Jason
participants (2)
-
Jason Hui -
Marek Vasut