mxs: i2c: Implement algorithm to set up arbitrary i2c speed
Subsystems (3)
PATCH 1/1 mxs: i2c: Implement algorithm to set up arbitrary i2c speed
This algorithm computes the values of TIMING{0,1,2} registers for the MX28 I2C block. This algorithm was derived by using a scope, but the result seems correct.
The resulting values programmed into the registers do not correlate with the contents of the datasheet. When using the values from the datasheet, the I2C clock were completely wrong.
--- drivers/i2c/mxs_i2c.c | 73 ++++++++++++++----------------------------------- 1 file changed, 21 insertions(+), 52 deletions(-)
NOTE: Heiko, please apply it onto your u-boot-i2c tree, it's dependent on the patches that are already there.
drivers/i2c/mxs_i2c.c +21 -52
@@ -28,6 +28,7 @@#include <common.h> #include <malloc.h> +#include <i2c.h> #include <asm/errno.h> #include <asm/io.h> #include <asm/arch/clock.h>
@@ -40,6 +41,7 @@ void mxs_i2c_reset(void) {struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE ;int ret; +int speed =i2c_get_bus_speed (); ret =mxs_reset_block (&i2c_regs->hw_i2c_ctrl0_reg);if (ret) {
@@ -53,6 +55,8 @@ void mxs_i2c_reset(void) &i2c_regs->hw_i2c_ctrl1_clr);writel (I2C_QUEUECTRL_PIO_QUEUE_MODE , &i2c_regs->hw_i2c_queuectrl_set); + +i2c_set_bus_speed (speed); }void mxs_i2c_setup_read (uint8_t chip,int len)
@@ -210,62 +214,28 @@ int i2c_probe(uchar chip)return ret; } -static struct mxs_i2c_speed_table { -uint32_t speed; -uint32_t timing0; -uint32_t timing1; -} mxs_i2c_tbl[] = { - { -100000 , - (0x0078 <<I2C_TIMING0_HIGH_COUNT_OFFSET ) | - (0x0030 <<I2C_TIMING0_RCV_COUNT_OFFSET ), - (0x0080 <<I2C_TIMING1_LOW_COUNT_OFFSET ) | - (0x0030 <<I2C_TIMING1_XMIT_COUNT_OFFSET ) - }, - { -400000 , - (0x000f <<I2C_TIMING0_HIGH_COUNT_OFFSET ) | - (0x0007 <<I2C_TIMING0_RCV_COUNT_OFFSET ), - (0x001f <<I2C_TIMING1_LOW_COUNT_OFFSET ) | - (0x000f <<I2C_TIMING1_XMIT_COUNT_OFFSET ), - } -}; - -static struct mxs_i2c_speed_table *mxs_i2c_speed_to_cfg (uint32_t speed) -{ -int i; -for (i =0 ; i <ARRAY_SIZE (mxs_i2c_tbl); i++) -if (mxs_i2c_tbl[i].speed == speed) -return &mxs_i2c_tbl[i]; -return NULL ; -} - -static uint32_t mxs_i2c_cfg_to_speed (uint32_t timing0,uint32_t timing1) -{ -int i; -for (i =0 ; i <ARRAY_SIZE (mxs_i2c_tbl); i++) { -if (mxs_i2c_tbl[i].timing0 != timing0) -continue ; -if (mxs_i2c_tbl[i].timing1 != timing1) -continue ; -return mxs_i2c_tbl[i].speed; - } - -return 0 ; -} -int i2c_set_bus_speed (unsigned int speed) {struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE ; -struct mxs_i2c_speed_table *spd =mxs_i2c_speed_to_cfg (speed); -if (!spd) { -printf ("MXS I2C: Invalid speed selected (%d Hz)\n" , speed); +uint32_t base = ((24000000 / speed) -38 ) /2 ; +uint16_t high_count = base +3 ; +uint16_t low_count = base -3 ; +uint16_t rcv_count = (high_count *3 ) /4 ; +uint16_t xmit_count = low_count /4 ; + +if (speed >540000 ) { +printf ("MXS I2C: Speed too high (%d Hz)\n" , speed); +return -EINVAL ; + } + +if (speed <12000 ) { +printf ("MXS I2C: Speed too low (%d Hz)\n" , speed);return -EINVAL ; } -writel (spd->timing0, &i2c_regs->hw_i2c_timing0); -writel (spd->timing1, &i2c_regs->hw_i2c_timing1); +writel ((high_count <<16 ) | rcv_count, &i2c_regs->hw_i2c_timing0); +writel ((low_count <<16 ) | xmit_count, &i2c_regs->hw_i2c_timing1); writel((0x0030 << I2C_TIMING2_BUS_FREE_OFFSET) | (0x0030 << I2C_TIMING2_LEADIN_COUNT_OFFSET),
@@ -277,12 +247,11 @@ int i2c_set_bus_speed(unsigned int speed)unsigned int i2c_get_bus_speed (void ) {struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE ; -uint32_t timing0, timing1; +uint32_t timing0; timing0 =readl (&i2c_regs->hw_i2c_timing0); - timing1 =readl (&i2c_regs->hw_i2c_timing1); -return mxs_i2c_cfg_to_speed (timing0, timing1); +return 24000000 / ((((timing0 >>16 ) -3 ) *2 ) +38 ); }void i2c_init (int speed,int slaveadd)
Signature · 1 line
1.7.10.4
PATCH 1/1 mxs: i2c: Implement algorithm to set up arbitrary i2c speed
Dear Marek Vasut,
Quoted 3 lines · In message <[email protected]> you wrote:
This algorithm computes the values of TIMING{0,1,2} registers for the MX28 I2C block. This algorithm was derived by using a scope, but the result seems correct.
Thanks! I like that!
...
Quoted 1 line
drivers/i2c/mxs_i2c.c +1
-0+uint32_t base = ((24000000 / speed) -38 ) /2 ;
...
drivers/i2c/mxs_i2c.c int i2c_set_bus_speed(unsigned int speed) · line 254
Quoted 1 line
drivers/i2c/mxs_i2c.c +1
-0+return 24000000 / ((((timing0 >>16 ) -3 ) *2 ) +38 );
But we should get rid of this magic constant. On other i.MX systems that would probably be MXC_HCLK ?
For iMX28, we have CONFIG_PL011_CLOCK in the board config files instead.
Hm... grepping the source tree for 24000000 I get the feeling that this really needs some cleanup....
Best regards,
Wolfgang Denk
Signature · 5 lines
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 Vulcans worship peace above all. -- McCoy, "Return to Tomorrow", stardate 4768.3
PATCH 1/1 mxs: i2c: Implement algorithm to set up arbitrary i2c speed
Dear Wolfgang Denk,
Quoted 11 lines
Dear Marek Vasut,
Quoted 3 lines · In message <[email protected]> you wrote:
This algorithm computes the values of TIMING{0,1,2} registers for the MX28 I2C block. This algorithm was derived by using a scope, but the result seems correct.
Thanks! I like that!
...
Quoted 8 lines
Quoted 1 line
drivers/i2c/mxs_i2c.c +1
-0+uint32_t base = ((24000000 / speed) -38 ) /2 ;...
drivers/i2c/mxs_i2c.c int i2c_set_bus_speed(unsigned int speed) · line 254
Quoted 1 line
drivers/i2c/mxs_i2c.c +1
-0+return 24000000 / ((((timing0 >>16 ) -3 ) *2 ) +38 );But we should get rid of this magic constant. On other i.MX systems that would probably be
MXC_HCLK?
It's APBX clock ... you are so right ... I rolled the patch out too fast. [...]
Best regards,
Marek Vasut
PATCH v2 1/1 mxs: i2c: Implement algorithm to set up arbitrary i2c speed
In reply to Marek VasutThis algorithm computes the values of TIMING{0,1,2} registers for the MX28 I2C block. This algorithm was derived by using a scope, but the result seems correct.
The resulting values programmed into the registers do not correlate with the contents in datasheet. When using the values from the datasheet, the I2C clock were completely wrong.
--- arch/arm/cpu/arm926ejs/mxs/clock.c | 2 + arch/arm/include/asm/arch-mxs/clock.h | 1 + drivers/i2c/mxs_i2c.c | 75 ++++++++++----------------------- 3 files changed, 26 insertions(+), 52 deletions(-)
v2: Properly implement XTAL clock retrieval. The i2c clock are derived from the
24MHz XTAL clock.
arch/arm/cpu/arm926ejs/mxs/clock.c +2 -0
@@ -333,6 +333,8 @@ uint32_t mxc_get_clock(enum mxc_clock clk)return mx28_get_sspclk (MXC_SSPCLK2 );case MXC_SSP3_CLK :return mx28_get_sspclk (MXC_SSPCLK3 ); +case MXC_XTAL_CLK : +return XTAL_FREQ_KHZ *1000 ; }return 0 ;
arch/arm/include/asm/arch-mxs/clock.h +1 -0
@@ -35,6 +35,7 @@ enum mxc_clock {MXC_SSP1_CLK ,MXC_SSP2_CLK ,MXC_SSP3_CLK , + MXC_XTAL_CLK, }; enum mxs_ioclock {
drivers/i2c/mxs_i2c.c +23 -52
115 lines not on this page · raw message
Signature · 1 line
1.7.10.4
PATCH v2 1/1 mxs: i2c: Implement algorithm to set up arbitrary i2c speed
Hello Marek,
Quoted 21 lines · Marek Vasut
This algorithm computes the values of TIMING{0,1,2} registers for the MX28 I2C block. This algorithm was derived by using a scope, but the result seems correct.
The resulting values programmed into the registers do not correlate with the contents in datasheet. When using the values from the datasheet, the I2C clock were completely wrong.
Signed-off-by Marek Vasut<[email protected]>Cc Stefano Babic<[email protected]>Cc Fabio Estevam<[email protected]>Cc Wolfgang Denk<[email protected]>--- arch/arm/cpu/arm926ejs/mxs/clock.c | 2 + arch/arm/include/asm/arch-mxs/clock.h | 1 + drivers/i2c/mxs_i2c.c | 75 ++++++++++----------------------- 3 files changed, 26 insertions(+), 52 deletions(-)v2: Properly implement XTAL clock retrieval. The i2c clock are derived from the
24MHz XTAL clock.
[...]
Quoted 21 lines
drivers/i2c/mxs_i2c.c +2
-0@@ -28,6 +28,7 @@#include <common.h> #include <malloc.h> +#include <i2c.h> #include <asm/errno.h> #include <asm/io.h> #include <asm/arch/clock.h> @@ -40,6 +41,7 @@ void mxs_i2c_reset(void) {struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE ;int ret; +int speed =i2c_get_bus_speed (); ret =mxs_reset_block (&i2c_regs->hw_i2c_ctrl0_reg);if (ret) {
drivers/i2c/mxs_i2c.c void mxs_i2c_reset(void) · line 60
Quoted 6 lines
drivers/i2c/mxs_i2c.c +2
-0@@ -53,6 +55,8 @@ void mxs_i2c_reset(void) &i2c_regs->hw_i2c_ctrl1_clr);writel (I2C_QUEUECTRL_PIO_QUEUE_MODE ,&i2c_regs->hw_i2c_queuectrl_set); + +i2c_set_bus_speed (speed); }
This change is not described in the patch description, please fix.
Quoted 6 lines
drivers/i2c/mxs_i2c.c +0
-1void mxs_i2c_setup_read (uint8_t chip,int len)@@ -210,62 +214,29 @@ int i2c_probe(uchar chip)return ret; } -static struct mxs_i2c_speed_table {
[...]
drivers/i2c/mxs_i2c.c int i2c_probe(uchar chip) · line 226
Quoted 13 lines
int i2c_set_bus_speed(unsigned int speed) { struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE;drivers/i2c/mxs_i2c.c +6
-3-struct mxs_i2c_speed_table *spd =mxs_i2c_speed_to_cfg (speed); -if (!spd) { -printf ("MXS I2C: Invalid speed selected (%d Hz)\n" , speed); +uint32_t clk =mxc_get_clock (MXC_XTAL_CLK ); +uint32_t base = ((clk / speed) -38 ) /2 ; +uint16_t high_count = base +3 ; +uint16_t low_count = base -3 ; +uint16_t rcv_count = (high_count *3 ) /4 ; +uint16_t xmit_count = low_count /4 ;
a lot of magic constants ...
Quoted 5 lines
drivers/i2c/mxs_i2c.c +5
-0+ +if (speed>540000 ) { +printf ("MXS I2C: Speed too high (%d Hz)\n" , speed); +return -EINVAL ; + }
drivers/i2c/mxs_i2c.c int i2c_probe(uchar chip)
Quoted 10 lines
drivers/i2c/mxs_i2c.c +5
-2+ +if (speed<12000 ) { +printf ("MXS I2C: Speed too low (%d Hz)\n" , speed);return -EINVAL ; } -writel (spd->timing0,&i2c_regs->hw_i2c_timing0); -writel (spd->timing1,&i2c_regs->hw_i2c_timing1); +writel ((high_count<<16 ) | rcv_count,&i2c_regs->hw_i2c_timing0); +writel ((low_count<<16 ) | xmit_count,&i2c_regs->hw_i2c_timing1);
^ ^
spaces
Quoted 6 lines
writel((0x0030<<
I2C_TIMING2_BUS_FREE_OFFSET) |
(0x0030<<I2C_TIMING2_LEADIN_COUNT_OFFSET),drivers/i2c/mxs_i2c.c +0
-0@@ -277,12 +248,12 @@ int i2c_set_bus_speed(unsigned int speed)unsigned int i2c_get_bus_speed (void ) {
drivers/i2c/mxs_i2c.c int i2c_set_bus_speed(unsigned int speed) · line 256
Quoted 10 lines
drivers/i2c/mxs_i2c.c +3
-3struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE ; -uint32_t timing0, timing1; +uint32_t clk =mxc_get_clock (MXC_XTAL_CLK ); +uint32_t timing0; timing0 =readl (&i2c_regs->hw_i2c_timing0); - timing1 =readl (&i2c_regs->hw_i2c_timing1); -return mxs_i2c_cfg_to_speed (timing0, timing1); +return clk / ((((timing0>>16 ) -3 ) *2 ) +38 );
^
spaces... and here a lot of magic constants too ... as this is a result of measuring ... we should at least note here, that we have this values from a scope session and the values in the manual seem to be not correct ...
Hmm... I am a little confused ... you write the i2c_regs->hw_i2c_timing{0,1,2} registers in i2c_set_bus_speed() but you return in i2c_get_bus_speed() just results from reading the i2c_regs->hw_i2c_timing0 register only and do some funny calculations ... is this correct?
drivers/i2c/mxs_i2c.c int i2c_set_bus_speed(unsigned int speed) · line 259
Quoted 3 lines
drivers/i2c/mxs_i2c.c +0
-0}void i2c_init (int speed,int slaveadd)
Thanks!
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH v2 1/1 mxs: i2c: Implement algorithm to set up arbitrary i2c speed
Dear Heiko Schocher,
Quoted 1 line
Hello Marek,
[...]
Quoted 31 lines
Quoted 31 lines
drivers/i2c/mxs_i2c.c +1
-0@@ -28,6 +28,7 @@#include <common.h> #include <malloc.h> +#include <i2c.h> #include <asm/errno.h> #include<asm/io.h> #include<asm/arch/clock.h>drivers/i2c/mxs_i2c.c +1
-0@@ -40,6 +41,7 @@ void mxs_i2c_reset(void) {struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE ;int ret; +int speed =i2c_get_bus_speed ();ret = mxs_reset_block(&i2c_regs->hw_i2c_ctrl0_reg); if (ret) {drivers/i2c/mxs_i2c.c +0
-0@@ -53,6 +55,8 @@ void mxs_i2c_reset(void) &i2c_regs->hw_i2c_ctrl1_clr);
Quoted 8 lines
Quoted 6 lines
drivers/i2c/mxs_i2c.c +2
-0writel (I2C_QUEUECTRL_PIO_QUEUE_MODE ,&i2c_regs->hw_i2c_queuectrl_set); + +i2c_set_bus_speed (speed);}This change is not described in the patch description, please fix.
I suspect I'll move this to a separate patch.
Quoted 19 lines
Quoted 9 lines
void
mxs_i2c_setup_read(uint8_t chip, int len)drivers/i2c/mxs_i2c.c +0
-1@@ -210,62 +214,29 @@ int i2c_probe(uchar chip)return ret; } -static struct mxs_i2c_speed_table {[...]
Quoted 7 lines
int i2c_set_bus_speed(unsigned int speed) { struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE;drivers/i2c/mxs_i2c.c +0
-1-struct mxs_i2c_speed_table *spd =mxs_i2c_speed_to_cfg (speed);
Quoted 10 lines
drivers/i2c/mxs_i2c.c int i2c_probe(uchar chip) · line 226
Quoted 8 lines
drivers/i2c/mxs_i2c.c +6
-2-if (!spd) { -printf ("MXS I2C: Invalid speed selected (%d Hz)\n" , speed); +uint32_t clk =mxc_get_clock (MXC_XTAL_CLK ); +uint32_t base = ((clk / speed) -38 ) /2 ; +uint16_t high_count = base +3 ; +uint16_t low_count = base -3 ; +uint16_t rcv_count = (high_count *3 ) /4 ; +uint16_t xmit_count = low_count /4 ;a lot of magic constants ...
True ... and they have no other meaning than to align stuff on the scope ;-)
Quoted 13 lines
Quoted 13 lines
drivers/i2c/mxs_i2c.c +8
-0+ +if (speed>540000 ) { +printf ("MXS I2C: Speed too high (%d Hz)\n" , speed); +return -EINVAL ; + } + +if (speed<12000 ) { +printf ("MXS I2C: Speed too low (%d Hz)\n" , speed);return -EINVAL ; }
Quoted 7 lines
drivers/i2c/mxs_i2c.c
Quoted 4 lines
drivers/i2c/mxs_i2c.c +2
-2-writel (spd->timing0,&i2c_regs->hw_i2c_timing0); -writel (spd->timing1,&i2c_regs->hw_i2c_timing1); +writel ((high_count<<16 ) | rcv_count,&i2c_regs->hw_i2c_timing0); +writel ((low_count<<16 ) | xmit_count,&i2c_regs->hw_i2c_timing1);^ ^
spaces
Could this be your mailer's issue?
[...]
Quoted 4 lines
... and here a lot of magic constants too ... as this is a result of measuring ... we should at least note here, that we have this values from a scope session and the values in the manual seem to be not correct ...
True.
Quoted 5 lines
Hmm... I am a little confused ... you write the i2c_regs->hw_i2c_timing{0,1,2} registers in
i2c_set_bus_speed()but you return ini2c_get_bus_speed()just results from reading thei2c_regs->hw_i2c_timing0register only and do some funny calculations ... is this correct?
Yes, the speed is configured upon boot anyway, so the timing0 register contains result of previous call to i2c_set_bus_speed(). And if it doesn't, we can't properly determine the speed anyway :(
Quoted 8 lines
Quoted 3 lines
}void
i2c_init(int speed, int slaveadd)Thanks!
bye,
Heiko
PATCH 1/2 mxs: i2c: Restore speed setting after block reset
In reply to Marek VasutThe I2C block reset configures the I2C bus speed to strange value. Read the I2C speed from the block before reseting the block and restore it afterwards, so the I2C operates correctly. This issue can be replicated by doing unsuccessful I2C transfer, after such transfer finishes, the I2C block clock speed is misconfigured.
--- drivers/i2c/mxs_i2c.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/i2c/mxs_i2c.c +3 -0
@@ -40,6 +40,7 @@ void mxs_i2c_reset(void) {struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE ;int ret; +int speed =i2c_get_bus_speed (); ret =mxs_reset_block (&i2c_regs->hw_i2c_ctrl0_reg);if (ret) {
@@ -53,6 +54,8 @@ void mxs_i2c_reset(void) &i2c_regs->hw_i2c_ctrl1_clr);writel (I2C_QUEUECTRL_PIO_QUEUE_MODE , &i2c_regs->hw_i2c_queuectrl_set); + +i2c_set_bus_speed (speed); }void mxs_i2c_setup_read (uint8_t chip,int len)
Signature · 1 line
1.7.10.4
PATCH v3 2/2 mxs: i2c: Implement algorithm to set up arbitrary i2c speed
This algorithm computes the values of TIMING{0,1,2} registers for the MX28 I2C block. This algorithm was derived by using a scope, but the result seems correct.
The resulting values programmed into the registers do not correlate with the contents in datasheet. When using the values from the datasheet, the I2C clock were completely wrong.
--- arch/arm/cpu/arm926ejs/mxs/clock.c | 2 + arch/arm/include/asm/arch-mxs/clock.h | 1 + drivers/i2c/mxs_i2c.c | 87 +++++++++++++-------------------- 3 files changed, 37 insertions(+), 53 deletions(-)
V2: Properly implement XTAL clock retrieval. The i2c clock are derived from the
24MHz XTAL clock.
V3: Split away reset speed fix
Add comment about the funny algorithm
arch/arm/cpu/arm926ejs/mxs/clock.c +2 -0
@@ -333,6 +333,8 @@ uint32_t mxc_get_clock(enum mxc_clock clk)return mx28_get_sspclk (MXC_SSPCLK2 );case MXC_SSP3_CLK :return mx28_get_sspclk (MXC_SSPCLK3 ); +case MXC_XTAL_CLK : +return XTAL_FREQ_KHZ *1000 ; }return 0 ;
arch/arm/include/asm/arch-mxs/clock.h +1 -0
@@ -35,6 +35,7 @@ enum mxc_clock {MXC_SSP1_CLK ,MXC_SSP2_CLK ,MXC_SSP3_CLK , + MXC_XTAL_CLK, }; enum mxs_ioclock {
drivers/i2c/mxs_i2c.c +34 -53
114 lines not on this page · raw message
Signature · 1 line
1.7.10.4