[U-Boot] [PATCH 1/3] mxc_i2c: specify i2c base address in config file
The following platforms had their config files changed flea3, imx31_phycore, mx35pdk, mx53ard, mx53evk, mx53smd
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com --- arch/arm/include/asm/arch-mx31/imx-regs.h | 7 +++++++ arch/arm/include/asm/arch-mx35/imx-regs.h | 2 +- drivers/i2c/mxc_i2c.c | 25 ++++--------------------- include/configs/flea3.h | 2 +- include/configs/imx31_phycore.h | 3 ++- include/configs/mx35pdk.h | 2 +- include/configs/mx53ard.h | 2 +- include/configs/mx53evk.h | 2 +- include/configs/mx53smd.h | 2 +- 9 files changed, 19 insertions(+), 28 deletions(-)
diff --git a/arch/arm/include/asm/arch-mx31/imx-regs.h b/arch/arm/include/asm/arch-mx31/imx-regs.h index 6454acb..7ddbbd6 100644 --- a/arch/arm/include/asm/arch-mx31/imx-regs.h +++ b/arch/arm/include/asm/arch-mx31/imx-regs.h @@ -606,6 +606,13 @@ struct esdc_regs { #define UART4_BASE 0x43FB0000 #define UART5_BASE 0x43FB4000
+#define I2C1_BASE_ADDR 0x43f80000 +#define I2C1_CLK_OFFSET 26 +#define I2C2_BASE_ADDR 0x43F98000 +#define I2C2_CLK_OFFSET 28 +#define I2C3_BASE_ADDR 0x43f84000 +#define I2C3_CLK_OFFSET 30 + #define ESDCTL_SDE (1 << 31) #define ESDCTL_CMD_RW (0 << 28) #define ESDCTL_CMD_PRECHARGE (1 << 28) diff --git a/arch/arm/include/asm/arch-mx35/imx-regs.h b/arch/arm/include/asm/arch-mx35/imx-regs.h index e570ad1..3146006 100644 --- a/arch/arm/include/asm/arch-mx35/imx-regs.h +++ b/arch/arm/include/asm/arch-mx35/imx-regs.h @@ -39,7 +39,7 @@ #define MAX_BASE_ADDR 0x43F04000 #define EVTMON_BASE_ADDR 0x43F08000 #define CLKCTL_BASE_ADDR 0x43F0C000 -#define I2C_BASE_ADDR 0x43F80000 +#define I2C1_BASE_ADDR 0x43F80000 #define I2C3_BASE_ADDR 0x43F84000 #define ATA_BASE_ADDR 0x43F8C000 #define UART1_BASE 0x43F90000 diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c index c88ac7c..416ffee 100644 --- a/drivers/i2c/mxc_i2c.c +++ b/drivers/i2c/mxc_i2c.c @@ -59,27 +59,10 @@ struct mxc_i2c_regs { #define I2SR_IIF (1 << 1) #define I2SR_RX_NO_AK (1 << 0)
-#if defined(CONFIG_SYS_I2C_MX31_PORT1) -#define I2C_BASE 0x43f80000 -#define I2C_CLK_OFFSET 26 -#elif defined (CONFIG_SYS_I2C_MX31_PORT2) -#define I2C_BASE 0x43f98000 -#define I2C_CLK_OFFSET 28 -#elif defined (CONFIG_SYS_I2C_MX31_PORT3) -#define I2C_BASE 0x43f84000 -#define I2C_CLK_OFFSET 30 -#elif defined(CONFIG_SYS_I2C_MX53_PORT1) -#define I2C_BASE I2C1_BASE_ADDR -#elif defined(CONFIG_SYS_I2C_MX53_PORT2) -#define I2C_BASE I2C2_BASE_ADDR -#elif defined(CONFIG_SYS_I2C_MX35_PORT1) -#define I2C_BASE I2C_BASE_ADDR -#elif defined(CONFIG_SYS_I2C_MX35_PORT2) -#define I2C_BASE I2C2_BASE_ADDR -#elif defined(CONFIG_SYS_I2C_MX35_PORT3) -#define I2C_BASE I2C3_BASE_ADDR +#ifdef CONFIG_SYS_I2C_BASE +#define I2C_BASE CONFIG_SYS_I2C_BASE #else -#error "define CONFIG_SYS_I2C_MX<Processor>_PORTx to use the mx I2C driver" +#error "define CONFIG_SYS_I2C_BASE to use the mxc_i2c driver" #endif
#define I2C_MAX_TIMEOUT 10000 @@ -114,7 +97,7 @@ static uint8_t i2c_imx_get_clk(unsigned int rate) (struct clock_control_regs *)CCM_BASE;
/* start the required I2C clock */ - writel(readl(&sc_regs->cgr0) | (3 << I2C_CLK_OFFSET), + writel(readl(&sc_regs->cgr0) | (3 << CONFIG_SYS_I2C_CLK_OFFSET), &sc_regs->cgr0); #endif
diff --git a/include/configs/flea3.h b/include/configs/flea3.h index f046a58..75330c4 100644 --- a/include/configs/flea3.h +++ b/include/configs/flea3.h @@ -68,7 +68,7 @@ */ #define CONFIG_HARD_I2C #define CONFIG_I2C_MXC -#define CONFIG_SYS_I2C_MX35_PORT3 +#define CONFIG_SYS_I2C_BASE I2C3_BASE_ADDR #define CONFIG_SYS_I2C_SPEED 100000 #define CONFIG_SYS_I2C_SLAVE 0xfe #define CONFIG_MXC_SPI diff --git a/include/configs/imx31_phycore.h b/include/configs/imx31_phycore.h index 3153eb5..197cefa 100644 --- a/include/configs/imx31_phycore.h +++ b/include/configs/imx31_phycore.h @@ -54,7 +54,8 @@
#define CONFIG_HARD_I2C #define CONFIG_I2C_MXC -#define CONFIG_SYS_I2C_MX31_PORT2 +#define CONFIG_SYS_I2C_BASE I2C2_BASE_ADDR +#define CONFIG_SYS_I2C_CLK_OFFSET I2C2_CLK_OFFSET #define CONFIG_SYS_I2C_SPEED 100000 #define CONFIG_SYS_I2C_SLAVE 0xfe
diff --git a/include/configs/mx35pdk.h b/include/configs/mx35pdk.h index de4b954..dfe39b8 100644 --- a/include/configs/mx35pdk.h +++ b/include/configs/mx35pdk.h @@ -59,7 +59,7 @@ */ #define CONFIG_HARD_I2C #define CONFIG_I2C_MXC -#define CONFIG_SYS_I2C_MX35_PORT1 +#define CONFIG_SYS_I2C_BASE I2C1_BASE_ADDR #define CONFIG_SYS_I2C_SPEED 100000 #define CONFIG_SYS_I2C_SLAVE 0xfe #define CONFIG_MXC_SPI diff --git a/include/configs/mx53ard.h b/include/configs/mx53ard.h index f48a41e..0bdf6a3 100644 --- a/include/configs/mx53ard.h +++ b/include/configs/mx53ard.h @@ -50,7 +50,7 @@ #define CONFIG_CMD_I2C #define CONFIG_HARD_I2C #define CONFIG_I2C_MXC -#define CONFIG_SYS_I2C_MX53_PORT2 +#define CONFIG_SYS_I2C_BASE I2C2_BASE_ADDR #define CONFIG_SYS_I2C_SPEED 100000 #define CONFIG_SYS_I2C_SLAVE 0xfe
diff --git a/include/configs/mx53evk.h b/include/configs/mx53evk.h index a77e5b2..3c7c329 100644 --- a/include/configs/mx53evk.h +++ b/include/configs/mx53evk.h @@ -53,7 +53,7 @@ #define CONFIG_CMD_I2C #define CONFIG_HARD_I2C #define CONFIG_I2C_MXC -#define CONFIG_SYS_I2C_MX53_PORT2 1 +#define CONFIG_SYS_I2C_BASE I2C2_BASE_ADDR #define CONFIG_SYS_I2C_SPEED 100000 #define CONFIG_SYS_I2C_SLAVE 0xfe
diff --git a/include/configs/mx53smd.h b/include/configs/mx53smd.h index a04db3f..a904130 100644 --- a/include/configs/mx53smd.h +++ b/include/configs/mx53smd.h @@ -50,7 +50,7 @@ #define CONFIG_CMD_I2C #define CONFIG_HARD_I2C #define CONFIG_I2C_MXC -#define CONFIG_SYS_I2C_MX53_PORT2 +#define CONFIG_SYS_I2C_BASE I2C2_BASE_ADDR #define CONFIG_SYS_I2C_SPEED 100000 #define CONFIG_SYS_I2C_SLAVE 0xfe
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com --- board/freescale/mx6qsabrelite/mx6qsabrelite.c | 11 +++++++++++ include/configs/mx6qsabrelite.h | 8 ++++++++ 2 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index 1d09a72..8690f22 100644 --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c @@ -50,6 +50,11 @@ DECLARE_GLOBAL_DATA_PTR; PAD_CTL_PUS_100K_DOWN | PAD_CTL_SPEED_MED | \ PAD_CTL_DSE_40ohm | PAD_CTL_SRE_FAST)
+#define I2C_PAD_CTRL (PAD_CTL_PKE | PAD_CTL_PUE | \ + PAD_CTL_PUS_100K_UP | PAD_CTL_SPEED_MED | \ + PAD_CTL_DSE_40ohm | PAD_CTL_HYS | \ + PAD_CTL_ODE | PAD_CTL_SRE_FAST) + int dram_init(void) { gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE); @@ -67,6 +72,11 @@ iomux_v3_cfg_t uart2_pads[] = { MX6Q_PAD_EIM_D27__UART2_RXD | MUX_PAD_CTRL(UART_PAD_CTRL), };
+iomux_v3_cfg_t i2c3_pads[] = { + MX6Q_PAD_GPIO_5__I2C3_SCL | MUX_PAD_CTRL(I2C_PAD_CTRL), + MX6Q_PAD_GPIO_16__I2C3_SDA | MUX_PAD_CTRL(I2C_PAD_CTRL), +}; + iomux_v3_cfg_t usdhc3_pads[] = { MX6Q_PAD_SD3_CLK__USDHC3_CLK | MUX_PAD_CTRL(USDHC_PAD_CTRL), MX6Q_PAD_SD3_CMD__USDHC3_CMD | MUX_PAD_CTRL(USDHC_PAD_CTRL), @@ -282,6 +292,7 @@ int board_init(void) #ifdef CONFIG_MXC_SPI setup_spi(); #endif + imx_iomux_v3_setup_multiple_pads(i2c3_pads, ARRAY_SIZE(i2c3_pads));
return 0; } diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h index f52c3c7..311494b 100644 --- a/include/configs/mx6qsabrelite.h +++ b/include/configs/mx6qsabrelite.h @@ -58,6 +58,14 @@ #define CONFIG_SF_DEFAULT_MODE (SPI_MODE_0) #endif
+/* I2C Configs */ +#define CONFIG_CMD_I2C +#define CONFIG_HARD_I2C +#define CONFIG_I2C_MXC +#define CONFIG_SYS_I2C_BASE I2C3_BASE_ADDR +#define CONFIG_SYS_I2C_SPEED 100000 +#define CONFIG_SYS_I2C_SLAVE 0xfe + /* MMC Configs */ #define CONFIG_FSL_ESDHC #define CONFIG_FSL_USDHC
On 25/04/2012 05:33, Troy Kisky wrote:
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
board/freescale/mx6qsabrelite/mx6qsabrelite.c | 11 +++++++++++ include/configs/mx6qsabrelite.h | 8 ++++++++ 2 files changed, 19 insertions(+), 0 deletions(-)
Acked-by: Stefano Babic sbabic@denx.de
Best regards, Stefano Babic
Other then being very weird, this code was also wrong. For example, say I set speed to 100K. I'll read back the speed as 85937. But the speed is really 85937.5, so we I reset the speed to 85937, I'll get 73660.7. After a couple of transactions my speed is now exactly 68750 so it will remain there.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com --- drivers/i2c/mxc_i2c.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c index 416ffee..fc68062 100644 --- a/drivers/i2c/mxc_i2c.c +++ b/drivers/i2c/mxc_i2c.c @@ -231,12 +231,6 @@ int i2c_imx_start(void) struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; unsigned int temp = 0; int result; - int speed = i2c_get_bus_speed(); - u8 clk_idx = i2c_imx_get_clk(speed); - u8 idx = i2c_clk_div[clk_idx][1]; - - /* Store divider value */ - writeb(idx, &i2c_regs->ifdr);
/* Enable I2C controller */ writeb(0, &i2c_regs->i2sr);
On 4/24/2012 8:33 PM, Troy Kisky wrote:
Other then being very weird, this code was also wrong. For example, say I set speed to 100K. I'll read back the speed as 85937. But the speed is really 85937.5, so we I reset the speed to 85937, I'll get 73660.7. After a couple of transactions my speed is now exactly 68750 so it will remain there.
Signed-off-by: Troy Kiskytroy.kisky@boundarydevices.com
drivers/i2c/mxc_i2c.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c index 416ffee..fc68062 100644 --- a/drivers/i2c/mxc_i2c.c +++ b/drivers/i2c/mxc_i2c.c @@ -231,12 +231,6 @@ int i2c_imx_start(void) struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; unsigned int temp = 0; int result;
int speed = i2c_get_bus_speed();
u8 clk_idx = i2c_imx_get_clk(speed);
u8 idx = i2c_clk_div[clk_idx][1];
/* Store divider value */
writeb(idx,&i2c_regs->ifdr);
/* Enable I2C controller */ writeb(0,&i2c_regs->i2sr);
Marek would you care to ack/nak this? It is deleting code that you added.
Thanks Troy
Dear Troy Kisky,
On 4/24/2012 8:33 PM, Troy Kisky wrote:
Other then being very weird, this code was also wrong. For example, say I set speed to 100K. I'll read back the speed as 85937. But the speed is really 85937.5, so we I reset the speed to 85937, I'll get 73660.7. After a couple of transactions my speed is now exactly 68750 so it will remain there.
Signed-off-by: Troy Kiskytroy.kisky@boundarydevices.com
drivers/i2c/mxc_i2c.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c index 416ffee..fc68062 100644 --- a/drivers/i2c/mxc_i2c.c +++ b/drivers/i2c/mxc_i2c.c @@ -231,12 +231,6 @@ int i2c_imx_start(void)
struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; unsigned int temp = 0; int result;
int speed = i2c_get_bus_speed();
u8 clk_idx = i2c_imx_get_clk(speed);
u8 idx = i2c_clk_div[clk_idx][1];
/* Store divider value */
writeb(idx,&i2c_regs->ifdr);
/* Enable I2C controller */ writeb(0,&i2c_regs->i2sr);
Marek would you care to ack/nak this? It is deleting code that you added.
Lemme check
Thanks Troy
Best regards, Marek Vasut
Dear Troy Kisky,
On 4/24/2012 8:33 PM, Troy Kisky wrote:
Other then being very weird, this code was also wrong. For example, say I set speed to 100K. I'll read back the speed as 85937. But the speed is really 85937.5, so we I reset the speed to 85937, I'll get 73660.7. After a couple of transactions my speed is now exactly 68750 so it will remain there.
Signed-off-by: Troy Kiskytroy.kisky@boundarydevices.com
drivers/i2c/mxc_i2c.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c index 416ffee..fc68062 100644 --- a/drivers/i2c/mxc_i2c.c +++ b/drivers/i2c/mxc_i2c.c @@ -231,12 +231,6 @@ int i2c_imx_start(void)
struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; unsigned int temp = 0; int result;
int speed = i2c_get_bus_speed();
u8 clk_idx = i2c_imx_get_clk(speed);
u8 idx = i2c_clk_div[clk_idx][1];
/* Store divider value */
writeb(idx,&i2c_regs->ifdr);
/* Enable I2C controller */ writeb(0,&i2c_regs->i2sr);
Marek would you care to ack/nak this? It is deleting code that you added.
Ok, who will set the controller speed if you remove this?
Thanks Troy
Best regards, Marek Vasut
On 5/5/2012 6:08 AM, Marek Vasut wrote:
Dear Troy Kisky,
On 4/24/2012 8:33 PM, Troy Kisky wrote:
Other then being very weird, this code was also wrong. For example, say I set speed to 100K. I'll read back the speed as 85937. But the speed is really 85937.5, so we I reset the speed to 85937, I'll get 73660.7. After a couple of transactions my speed is now exactly 68750 so it will remain there.
Signed-off-by: Troy Kiskytroy.kisky@boundarydevices.com
drivers/i2c/mxc_i2c.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c index 416ffee..fc68062 100644 --- a/drivers/i2c/mxc_i2c.c +++ b/drivers/i2c/mxc_i2c.c @@ -231,12 +231,6 @@ int i2c_imx_start(void)
struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; unsigned int temp = 0; int result;
int speed = i2c_get_bus_speed();
u8 clk_idx = i2c_imx_get_clk(speed);
u8 idx = i2c_clk_div[clk_idx][1];
/* Store divider value */
writeb(idx,&i2c_regs->ifdr);
/* Enable I2C controller */ writeb(0,&i2c_regs->i2sr);
Marek would you care to ack/nak this? It is deleting code that you added.
Ok, who will set the controller speed if you remove this?
i2c_init is the only function that writes the ifdr register after this patch. This is fine because this register is not affected by a software reset. If this register were affected by a software reset, then the current code would not work either, as i2c_imx_start is reading from this register before trying(and often failing) to set it to the same value.
Thanks Troy
Best regards, Marek Vasut
Dear Troy Kisky,
On 5/5/2012 6:08 AM, Marek Vasut wrote:
Dear Troy Kisky,
On 4/24/2012 8:33 PM, Troy Kisky wrote:
Other then being very weird, this code was also wrong. For example, say I set speed to 100K. I'll read back the speed as 85937. But the speed is really 85937.5, so we I reset the speed to 85937, I'll get 73660.7. After a couple of transactions my speed is now exactly 68750 so it will remain there.
Signed-off-by: Troy Kiskytroy.kisky@boundarydevices.com
drivers/i2c/mxc_i2c.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c index 416ffee..fc68062 100644 --- a/drivers/i2c/mxc_i2c.c +++ b/drivers/i2c/mxc_i2c.c @@ -231,12 +231,6 @@ int i2c_imx_start(void)
struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; unsigned int temp = 0; int result;
int speed = i2c_get_bus_speed();
u8 clk_idx = i2c_imx_get_clk(speed);
u8 idx = i2c_clk_div[clk_idx][1];
/* Store divider value */
writeb(idx,&i2c_regs->ifdr);
/* Enable I2C controller */ writeb(0,&i2c_regs->i2sr);
Marek would you care to ack/nak this? It is deleting code that you added.
Ok, who will set the controller speed if you remove this?
i2c_init is the only function that writes the ifdr register after this patch.
And i2c_init() is called on every boot. Correct?
This is fine because this register is not affected by a software reset.
I take it you verified this or that you're sure here :)
If this register were affected by a software reset, then the current code would not work either, as i2c_imx_start is reading from this register before trying(and often failing) to set it to the same value.
Reading from ifdr? That seems indeed wrong :-(
Well ... I have no objection then
Acked-by: Marek Vasut marex@denx.de
Thanks Troy
Best regards, Marek Vasut
Best regards, Marek Vasut
On 5/5/2012 2:36 PM, Marek Vasut wrote:
Dear Troy Kisky,
On 5/5/2012 6:08 AM, Marek Vasut wrote:
Dear Troy Kisky,
On 4/24/2012 8:33 PM, Troy Kisky wrote:
Other then being very weird, this code was also wrong. For example, say I set speed to 100K. I'll read back the speed as 85937. But the speed is really 85937.5, so we I reset the speed to 85937, I'll get 73660.7. After a couple of transactions my speed is now exactly 68750 so it will remain there.
Signed-off-by: Troy Kiskytroy.kisky@boundarydevices.com
drivers/i2c/mxc_i2c.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-)diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c index 416ffee..fc68062 100644 --- a/drivers/i2c/mxc_i2c.c +++ b/drivers/i2c/mxc_i2c.c @@ -231,12 +231,6 @@ int i2c_imx_start(void)
struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; unsigned int temp = 0; int result;
int speed = i2c_get_bus_speed();
u8 clk_idx = i2c_imx_get_clk(speed);
u8 idx = i2c_clk_div[clk_idx][1];
/* Store divider value */
writeb(idx,&i2c_regs->ifdr);
/* Enable I2C controller */ writeb(0,&i2c_regs->i2sr);Marek would you care to ack/nak this? It is deleting code that you added.
Ok, who will set the controller speed if you remove this?
i2c_init is the only function that writes the ifdr register after this patch.
And i2c_init() is called on every boot. Correct?
This is fine because this register is not affected by a software reset.
I take it you verified this or that you're sure here :)
I haven't looked at every reference manual, but if some oddball chip needs it reinitialized after a software reset, then the fix should go into the i2c_reset function, not i2c_imx_start. And this patch would not be introducing a regression for the oddball chip anyway.
Troy
Dear Troy Kisky,
On 5/5/2012 2:36 PM, Marek Vasut wrote:
Dear Troy Kisky,
On 5/5/2012 6:08 AM, Marek Vasut wrote:
Dear Troy Kisky,
On 4/24/2012 8:33 PM, Troy Kisky wrote:
Other then being very weird, this code was also wrong. For example, say I set speed to 100K. I'll read back the speed as 85937. But the speed is really 85937.5, so we I reset the speed to 85937, I'll get 73660.7. After a couple of transactions my speed is now exactly 68750 so it will remain there.
Signed-off-by: Troy Kiskytroy.kisky@boundarydevices.com
drivers/i2c/mxc_i2c.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-)diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c index 416ffee..fc68062 100644 --- a/drivers/i2c/mxc_i2c.c +++ b/drivers/i2c/mxc_i2c.c @@ -231,12 +231,6 @@ int i2c_imx_start(void)
struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; unsigned int temp = 0; int result;
int speed = i2c_get_bus_speed();
u8 clk_idx = i2c_imx_get_clk(speed);
u8 idx = i2c_clk_div[clk_idx][1];
/* Store divider value */
writeb(idx,&i2c_regs->ifdr);
/* Enable I2C controller */ writeb(0,&i2c_regs->i2sr);Marek would you care to ack/nak this? It is deleting code that you added.
Ok, who will set the controller speed if you remove this?
i2c_init is the only function that writes the ifdr register after this patch.
And i2c_init() is called on every boot. Correct?
This is fine because this register is not affected by a software reset.
I take it you verified this or that you're sure here :)
I haven't looked at every reference manual, but if some oddball chip needs it reinitialized after a software reset, then the fix should go into the i2c_reset function, not i2c_imx_start. And this patch would not be introducing a regression for the oddball chip anyway.
Agreed, I think I already acked this, sorry for the delay :)
Troy
Best regards, Marek Vasut
On 06/05/2012 01:06, Marek Vasut wrote:
Dear Troy Kisky,
On 5/5/2012 2:36 PM, Marek Vasut wrote:
Dear Troy Kisky,
On 5/5/2012 6:08 AM, Marek Vasut wrote:
Dear Troy Kisky,
On 4/24/2012 8:33 PM, Troy Kisky wrote: > Other then being very weird, this code was also wrong. > For example, say I set speed to 100K. I'll read back the speed > as 85937. But the speed is really 85937.5, so we I reset > the speed to 85937, I'll get 73660.7. After a couple of transactions > my speed is now exactly 68750 so it will remain there. > > Signed-off-by: Troy Kiskytroy.kisky@boundarydevices.com > --- > > drivers/i2c/mxc_i2c.c | 6 ------ > 1 files changed, 0 insertions(+), 6 deletions(-) > > diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c > index 416ffee..fc68062 100644 > --- a/drivers/i2c/mxc_i2c.c > +++ b/drivers/i2c/mxc_i2c.c > @@ -231,12 +231,6 @@ int i2c_imx_start(void) > > struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; > unsigned int temp = 0; > int result; > > - int speed = i2c_get_bus_speed(); > - u8 clk_idx = i2c_imx_get_clk(speed); > - u8 idx = i2c_clk_div[clk_idx][1]; > - > - /* Store divider value */ > - writeb(idx,&i2c_regs->ifdr); > > /* Enable I2C controller */ > writeb(0,&i2c_regs->i2sr);
Marek would you care to ack/nak this? It is deleting code that you added.
Ok, who will set the controller speed if you remove this?
i2c_init is the only function that writes the ifdr register after this patch.
And i2c_init() is called on every boot. Correct?
This is fine because this register is not affected by a software reset.
I take it you verified this or that you're sure here :)
I haven't looked at every reference manual, but if some oddball chip needs it reinitialized after a software reset, then the fix should go into the i2c_reset function, not i2c_imx_start. And this patch would not be introducing a regression for the oddball chip anyway.
Ok, everything was already clear, and I can also add my:
Acked-by: Stefano Babic sbabic@denx.de
Best regards, Stefano Babic
On 06.05.2012 17:27, Stefano Babic wrote:
On 06/05/2012 01:06, Marek Vasut wrote:
Dear Troy Kisky,
On 5/5/2012 2:36 PM, Marek Vasut wrote:
Dear Troy Kisky,
On 5/5/2012 6:08 AM, Marek Vasut wrote:
Dear Troy Kisky,
> On 4/24/2012 8:33 PM, Troy Kisky wrote: >> Other then being very weird, this code was also wrong. >> For example, say I set speed to 100K. I'll read back the speed >> as 85937. But the speed is really 85937.5, so we I reset >> the speed to 85937, I'll get 73660.7. After a couple of transactions >> my speed is now exactly 68750 so it will remain there. >> >> Signed-off-by: Troy Kiskytroy.kisky@boundarydevices.com >> --- >> >> drivers/i2c/mxc_i2c.c | 6 ------ >> 1 files changed, 0 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c >> index 416ffee..fc68062 100644 >> --- a/drivers/i2c/mxc_i2c.c >> +++ b/drivers/i2c/mxc_i2c.c >> @@ -231,12 +231,6 @@ int i2c_imx_start(void) >> >> struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; >> unsigned int temp = 0; >> int result; >> >> - int speed = i2c_get_bus_speed(); >> - u8 clk_idx = i2c_imx_get_clk(speed); >> - u8 idx = i2c_clk_div[clk_idx][1]; >> - >> - /* Store divider value */ >> - writeb(idx,&i2c_regs->ifdr); >> >> /* Enable I2C controller */ >> writeb(0,&i2c_regs->i2sr); > Marek would you care to ack/nak this? It is deleting code that you > added. Ok, who will set the controller speed if you remove this?
i2c_init is the only function that writes the ifdr register after this patch.
And i2c_init() is called on every boot. Correct?
This is fine because this register is not affected by a software reset.
I take it you verified this or that you're sure here :)
I haven't looked at every reference manual, but if some oddball chip needs it reinitialized after a software reset, then the fix should go into the i2c_reset function, not i2c_imx_start. And this patch would not be introducing a regression for the oddball chip anyway.
Ok, everything was already clear, and I can also add my:
Acked-by: Stefano Babic sbabic@denx.de
Same for this one: Is this applied anywhere? It doesn't seem to be part of v2012.07-rc1?
Best regards
Dirk
Dear Dirk Behme,
[...]
Ok, everything was already clear, and I can also add my:
Acked-by: Stefano Babic sbabic@denx.de
Same for this one: Is this applied anywhere? It doesn't seem to be part of v2012.07-rc1?
Best regards
Stefano, once you're out of mont blanc, can you apply this please?
Best regards, Marek Vasut
Dear Marek Vasut,
In message 201207131238.11964.marex@denx.de you wrote:
Ok, everything was already clear, and I can also add my:
Acked-by: Stefano Babic sbabic@denx.de
Same for this one: Is this applied anywhere? It doesn't seem to be part of v2012.07-rc1?
Best regards
Stefano, once you're out of mont blanc, can you apply this please?
This is in mainline:
9ca37d7 2012-07-11 10:54:52 +0200 mxc_i2c: remove setting speed at each start
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
Dear Marek Vasut,
In message 201207131238.11964.marex@denx.de you wrote:
Ok, everything was already clear, and I can also add my:
Acked-by: Stefano Babic sbabic@denx.de
Same for this one: Is this applied anywhere? It doesn't seem to be part of v2012.07-rc1?
Best regards
Stefano, once you're out of mont blanc, can you apply this please?
This is in mainline:
9ca37d7 2012-07-11 10:54:52 +0200 mxc_i2c: remove setting speed at each start
Ok, sorry for the noise then.
Best regards,
Wolfgang Denk
Best regards, Marek Vasut
On 13/07/2012 12:38, Marek Vasut wrote:
Dear Dirk Behme,
[...]
Ok, everything was already clear, and I can also add my:
Acked-by: Stefano Babic sbabic@denx.de
Same for this one: Is this applied anywhere? It doesn't seem to be part of v2012.07-rc1?
Best regards
Stefano, once you're out of mont blanc, can you apply this please?
It is applied into u-boot-i2c, it will be merged directly into mainline ny next Heiko's pull request.
Best regards, Stefano
On 25/04/2012 05:33, Troy Kisky wrote:
The following platforms had their config files changed flea3, imx31_phycore, mx35pdk, mx53ard, mx53evk, mx53smd
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
Hi Troy,
arch/arm/include/asm/arch-mx31/imx-regs.h | 7 +++++++ arch/arm/include/asm/arch-mx35/imx-regs.h | 2 +- drivers/i2c/mxc_i2c.c | 25 ++++--------------------- include/configs/flea3.h | 2 +- include/configs/imx31_phycore.h | 3 ++- include/configs/mx35pdk.h | 2 +- include/configs/mx53ard.h | 2 +- include/configs/mx53evk.h | 2 +- include/configs/mx53smd.h | 2 +- 9 files changed, 19 insertions(+), 28 deletions(-)
Acked-by: Stefano Babic sbabic@denx.de
Best regards, Stefano Babic
On 06.05.2012 17:27, Stefano Babic wrote:
On 25/04/2012 05:33, Troy Kisky wrote:
The following platforms had their config files changed flea3, imx31_phycore, mx35pdk, mx53ard, mx53evk, mx53smd
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
Hi Troy,
arch/arm/include/asm/arch-mx31/imx-regs.h | 7 +++++++ arch/arm/include/asm/arch-mx35/imx-regs.h | 2 +- drivers/i2c/mxc_i2c.c | 25 ++++--------------------- include/configs/flea3.h | 2 +- include/configs/imx31_phycore.h | 3 ++- include/configs/mx35pdk.h | 2 +- include/configs/mx53ard.h | 2 +- include/configs/mx53evk.h | 2 +- include/configs/mx53smd.h | 2 +- 9 files changed, 19 insertions(+), 28 deletions(-)
Acked-by: Stefano Babic sbabic@denx.de
Hmm, is this already applied anywhere? It doesn't seem to be included in v2012.07-rc1?
Best regards
Dirk
Hello Dirk,
On 11.07.2012 08:25, Dirk Behme wrote:
On 06.05.2012 17:27, Stefano Babic wrote:
On 25/04/2012 05:33, Troy Kisky wrote:
The following platforms had their config files changed flea3, imx31_phycore, mx35pdk, mx53ard, mx53evk, mx53smd
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
Hi Troy,
arch/arm/include/asm/arch-mx31/imx-regs.h | 7 +++++++ arch/arm/include/asm/arch-mx35/imx-regs.h | 2 +- drivers/i2c/mxc_i2c.c | 25 ++++--------------------- include/configs/flea3.h | 2 +- include/configs/imx31_phycore.h | 3 ++- include/configs/mx35pdk.h | 2 +- include/configs/mx53ard.h | 2 +- include/configs/mx53evk.h | 2 +- include/configs/mx53smd.h | 2 +- 9 files changed, 19 insertions(+), 28 deletions(-)
Acked-by: Stefano Babic sbabic@denx.de
Hmm, is this already applied anywhere? It doesn't seem to be included in v2012.07-rc1?
Yes, it is applied in:
http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=summary
But I could not found my pull request to Wolfgang :-(
Currently rebased to current master, send pull reqeust soon.
Thanks for detecting this!
bye, Heiko
participants (7)
-
Dirk Behme -
Heiko Schocher -
Marek Vasut -
Marek Vasut -
Stefano Babic -
Troy Kisky -
Wolfgang Denk