[U-Boot] [PATCH 1/3] mxc_i2c: specify i2c base address in config file

[U-Boot] [PATCH 1/3] mxc_i2c: specify i2c base address in config file

[Troy Kisky]

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
 
-- 
1.7.5.4

[U-Boot] [PATCH 2/3] mx6qsabrelite: add i2c support

[Troy Kisky]

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
-- 
1.7.5.4

[U-Boot] [PATCH 2/3] mx6qsabrelite: add i2c support

[Stefano Babic]

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


-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH 3/3] mxc_i2c: remove setting speed at each start

[Troy Kisky]

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);
-- 
1.7.5.4

[U-Boot] [PATCH 3/3] mxc_i2c: remove setting speed at each start

[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 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);
Marek would you care to ack/nak this? It is deleting code that you added.

Thanks
Troy

[U-Boot] [PATCH 3/3] mxc_i2c: remove setting speed at each start

[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 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);
> 
> Marek would you care to ack/nak this? It is deleting code that you added.

Lemme check

> 
> Thanks
> Troy

Best regards,
Marek Vasut

[U-Boot] [PATCH 3/3] mxc_i2c: remove setting speed at each start

[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 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);
> 
> 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

[U-Boot] [PATCH 3/3] mxc_i2c: remove setting speed at each start

[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 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);
>> 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
>

[U-Boot] [PATCH 3/3] mxc_i2c: remove setting speed at each start

[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 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);
> >> 
> >> 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

[U-Boot] [PATCH 3/3] mxc_i2c: remove setting speed at each start

[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 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);
>>>> 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

[U-Boot] [PATCH 3/3] mxc_i2c: remove setting speed at each start

[Marek Vasut]

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 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);
> >>>> 
> >>>> 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

[U-Boot] [PATCH 3/3] mxc_i2c: remove setting speed at each start

[Stefano Babic]

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 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);
>>>>>>
>>>>>> 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


-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH 3/3] mxc_i2c: remove setting speed at each start

[Dirk Behme]

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 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);
>>>>>>> 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

[U-Boot] [PATCH 3/3] mxc_i2c: remove setting speed at each start

[Marek Vasut]

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

[U-Boot] [PATCH 3/3] mxc_i2c: remove setting speed at each start

[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


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The rule on staying alive as a program manager is to give 'em a  num-
ber or give 'em a date, but never give 'em both at once.

[U-Boot] [PATCH 3/3] mxc_i2c: remove setting speed at each start

[Marek Vasut]

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

[U-Boot] [PATCH 3/3] mxc_i2c: remove setting speed at each start

[Stefano Babic]

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


-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH 1/3] mxc_i2c: specify i2c base address in config file

[Stefano Babic]

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

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH 1/3] mxc_i2c: specify i2c base address in config file

[Dirk Behme]

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

[U-Boot] [PATCH 1/3] mxc_i2c: specify i2c base address in config file

[Heiko Schocher]

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
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany