[U-Boot] [PATCH 1/3] fsl_i2c: Wait for STOP condition to propagate
After issuing a STOP one must wait until the STOP has completed on the bus before doing something new to the controller.
Also add an extra read of SR as the manual mentions doing that is a good idea.
Remove surplus write of CR just before a write, isn't required and could potentially disturb the I2C bus.
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se --- drivers/i2c/fsl_i2c.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c index 47bbf79..56f9680 100644 --- a/drivers/i2c/fsl_i2c.c +++ b/drivers/i2c/fsl_i2c.c @@ -223,7 +223,7 @@ i2c_init(int speed, int slaveadd) #endif }
-static __inline__ int +static int i2c_wait4bus(void) { unsigned long long timeval = get_ticks(); @@ -248,6 +248,8 @@ i2c_wait(int write) csr = readb(&i2c_dev[i2c_bus_num]->sr); if (!(csr & I2C_SR_MIF)) continue; + /* Read again to allow register to stabilise */ + csr = readb(&i2c_dev[i2c_bus_num]->sr);
writeb(0x0, &i2c_dev[i2c_bus_num]->sr);
@@ -293,9 +295,6 @@ __i2c_write(u8 *data, int length) { int i;
- writeb(I2C_CR_MEN | I2C_CR_MSTA | I2C_CR_MTX, - &i2c_dev[i2c_bus_num]->cr); - for (i = 0; i < length; i++) { writeb(data[i], &i2c_dev[i2c_bus_num]->dr);
@@ -351,6 +350,9 @@ i2c_read(u8 dev, uint addr, int alen, u8 *data, int length) && i2c_write_addr(dev, I2C_READ_BIT, 1) != 0) i = __i2c_read(data, length);
+ if (length && i2c_wait4bus()) /* Wait until STOP */ + debug("i2c_read: wait4bus timed out\n"); + writeb(I2C_CR_MEN, &i2c_dev[i2c_bus_num]->cr);
if (i == length) @@ -372,6 +374,8 @@ i2c_write(u8 dev, uint addr, int alen, u8 *data, int length) }
writeb(I2C_CR_MEN, &i2c_dev[i2c_bus_num]->cr); + if (i2c_wait4bus()) /* Wait until STOP */ + debug("i2c_write: wait4bus timed out\n");
if (i == length) return 0;
Some boards need a higher DFSR value than the spec currently recommends so give these boards the means to define there own.
For completeness, add CONFIG_FSL_I2C_CUSTOM_FDR too.
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se --- drivers/i2c/fsl_i2c.c | 14 +++++++++++--- 1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c index 56f9680..0c5f6be 100644 --- a/drivers/i2c/fsl_i2c.c +++ b/drivers/i2c/fsl_i2c.c @@ -172,14 +172,22 @@ static unsigned int set_i2c_bus_speed(const struct fsl_i2c *dev, u8 fdr; #ifdef __PPC__ u8 dfsr; +#ifdef CONFIG_FSL_I2C_CUSTOM_DFSR + dfsr = CONFIG_FSL_I2C_CUSTOM_DFSR; +#else dfsr = fsl_i2c_speed_map[i].dfsr; #endif + writeb(dfsr, &dev->dfsrr); /* set default filter */ +#endif +#ifdef CONFIG_FSL_I2C_CUSTOM_FDR + fdr = CONFIG_FSL_I2C_CUSTOM_FDR; + speed = i2c_clk / divider; /* Fake something */ +#else fdr = fsl_i2c_speed_map[i].fdr; speed = i2c_clk / fsl_i2c_speed_map[i].divider; - writeb(fdr, &dev->fdr); /* set bus speed */ -#ifdef __PPC__ - writeb(dfsr, &dev->dfsrr); /* set default filter */ #endif + writeb(fdr, &dev->fdr); /* set bus speed */ + break; }
The latest AN2919 has changed the way FDR/DFSR should be calculated. Update the driver according to spec. However, Condition 2 is not accounted for as it is not clear how to do so.
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se --- drivers/i2c/fsl_i2c.c | 90 ++++++++++++++++++++++++++++++------------------- 1 files changed, 55 insertions(+), 35 deletions(-)
diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c index 0c5f6be..78f21c7 100644 --- a/drivers/i2c/fsl_i2c.c +++ b/drivers/i2c/fsl_i2c.c @@ -100,29 +100,9 @@ static const struct fsl_i2c *i2c_dev[2] = { */ static const struct { unsigned short divider; -#ifdef __PPC__ - u8 dfsr; -#endif u8 fdr; } fsl_i2c_speed_map[] = { -#ifdef __PPC__ - {160, 1, 32}, {192, 1, 33}, {224, 1, 34}, {256, 1, 35}, - {288, 1, 0}, {320, 1, 1}, {352, 6, 1}, {384, 1, 2}, {416, 6, 2}, - {448, 1, 38}, {480, 1, 3}, {512, 1, 39}, {544, 11, 3}, {576, 1, 4}, - {608, 22, 3}, {640, 1, 5}, {672, 32, 3}, {704, 11, 5}, {736, 43, 3}, - {768, 1, 6}, {800, 54, 3}, {832, 11, 6}, {896, 1, 42}, {960, 1, 7}, - {1024, 1, 43}, {1088, 22, 7}, {1152, 1, 8}, {1216, 43, 7}, {1280, 1, 9}, - {1408, 22, 9}, {1536, 1, 10}, {1664, 22, 10}, {1792, 1, 46}, - {1920, 1, 11}, {2048, 1, 47}, {2176, 43, 11}, {2304, 1, 12}, - {2560, 1, 13}, {2816, 43, 13}, {3072, 1, 14}, {3328, 43, 14}, - {3584, 1, 50}, {3840, 1, 15}, {4096, 1, 51}, {4608, 1, 16}, - {5120, 1, 17}, {6144, 1, 18}, {7168, 1, 54}, {7680, 1, 19}, - {8192, 1, 55}, {9216, 1, 20}, {10240, 1, 21}, {12288, 1, 22}, - {14336, 1, 58}, {15360, 1, 23}, {16384, 1, 59}, {18432, 1, 24}, - {20480, 1, 25}, {24576, 1, 26}, {28672, 1, 62}, {30720, 1, 27}, - {32768, 1, 63}, {36864, 1, 28}, {40960, 1, 29}, {49152, 1, 30}, - {61440, 1, 31}, {-1, 1, 31} -#elif defined(__M68K__) +#ifdef __M68K__ {20, 32}, {22, 33}, {24, 34}, {26, 35}, {28, 0}, {28, 36}, {30, 1}, {32, 37}, {34, 2}, {36, 38}, {40, 3}, {40, 39}, @@ -158,7 +138,6 @@ static unsigned int set_i2c_bus_speed(const struct fsl_i2c *dev, unsigned int i2c_clk, unsigned int speed) { unsigned short divider = min(i2c_clk / speed, (unsigned short) -1); - unsigned int i;
/* * We want to choose an FDR/DFSR that generates an I2C bus speed that @@ -166,31 +145,72 @@ static unsigned int set_i2c_bus_speed(const struct fsl_i2c *dev, * want the first divider that is equal to or greater than the * calculated divider. */ - - for (i = 0; i < ARRAY_SIZE(fsl_i2c_speed_map); i++) - if (fsl_i2c_speed_map[i].divider >= divider) { - u8 fdr; #ifdef __PPC__ - u8 dfsr; + u8 dfsr, fdr = 0x31; /* Default if no FDR found */ + /* a, b and dfsr matches identifiers A,B and C respectively in AN2919 */ + unsigned short a, b, ga, gb; + unsigned long c_div, est_div; + #ifdef CONFIG_FSL_I2C_CUSTOM_DFSR - dfsr = CONFIG_FSL_I2C_CUSTOM_DFSR; + dfsr = CONFIG_FSL_I2C_CUSTOM_DFSR; #else - dfsr = fsl_i2c_speed_map[i].dfsr; -#endif - writeb(dfsr, &dev->dfsrr); /* set default filter */ + /* Condition 1: dfsr <= 50/T */ + dfsr = (5 * (i2c_clk / 1000)) / 100000; #endif #ifdef CONFIG_FSL_I2C_CUSTOM_FDR - fdr = CONFIG_FSL_I2C_CUSTOM_FDR; - speed = i2c_clk / divider; /* Fake something */ + fdr = CONFIG_FSL_I2C_CUSTOM_FDR; + speed = i2c_clk / divider; /* Fake something */ #else + debug("Requested speed:%d, i2c_clk:%d\n", speed, i2c_clk); + if (!dfsr) + dfsr = 1; + + est_div = ~0; + for (ga = 0x4, a = 10; a <= 30; ga++, a += 2) { + for (gb = 0; gb < 8; gb++) { + b = 16 << gb; + c_div = b * (a + ((3*dfsr)/b)*2); + if ((c_div > divider) && (c_div < est_div)) { + unsigned short bin_gb, bin_ga; + + est_div = c_div; + bin_gb = gb << 2; + bin_ga = (ga & 0x3) | ((ga & 0x4) << 3); + fdr = bin_gb | bin_ga; + speed = i2c_clk / est_div; + debug("FDR:0x%.2x, div:%ld, ga:0x%x, gb:0x%x, " + "a:%d, b:%d, speed:%d\n", + fdr, est_div, ga, gb, a, b, speed); + /* Condition 2 not accounted for */ + debug("Tr <= %d ns\n", + (b - 3 * dfsr) * 1000000 / + (i2c_clk / 1000)); + } + } + if (a == 20) + a += 2; + if (a == 24) + a += 4; + } + debug("divider:%d, est_div:%ld, DFSR:%d\n", divider, est_div, dfsr); + debug("FDR:0x%.2x, speed:%d\n", fdr, speed); +#endif + writeb(dfsr, &dev->dfsrr); /* set default filter */ + writeb(fdr, &dev->fdr); /* set bus speed */ +#else + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(fsl_i2c_speed_map); i++) + if (fsl_i2c_speed_map[i].divider >= divider) { + u8 fdr; + fdr = fsl_i2c_speed_map[i].fdr; speed = i2c_clk / fsl_i2c_speed_map[i].divider; -#endif writeb(fdr, &dev->fdr); /* set bus speed */
break; } - +#endif return speed; }
Hello Joakim,
Joakim Tjernlund wrote:
The latest AN2919 has changed the way FDR/DFSR should be calculated. Update the driver according to spec. However, Condition 2 is not accounted for as it is not clear how to do so.
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se
drivers/i2c/fsl_i2c.c | 90 ++++++++++++++++++++++++++++++------------------- 1 files changed, 55 insertions(+), 35 deletions(-)
Applied to u-boot-i2c testing with one fix:
Applying: fsl_i2c: Impl. AN2919, rev 5 to calculate FDR/DFSR /home/hs/i2c/u-boot-i2c/.git/rebase-apply/patch:100: trailing whitespace. debug("Tr <= %d ns\n", warning: 1 line applied after fixing whitespace errors.
Thanks.
bye Heiko
Hi Joakim,
Joakim Tjernlund wrote:
The latest AN2919 has changed the way FDR/DFSR should be calculated. Update the driver according to spec. However, Condition 2 is not accounted for as it is not clear how to do so.
I compared rev. 5 of AN2919 with rev. 3 and, as you pointed out, it puts additional constraints on how to select dfsr and fdr. Especially dfsr should not exceed a certain, frequency dependent value: dfsr <= 50 / period-in-ns. Therefore, I expected problems with divider values from the table which high dfsr values. I did your "=> date;date;date;date" test on a MPC8548 board using dfsr=43 and fdr=7 but it did not fail. According to the rev. 5, dfsr is not allowed to be greater than 8. Your patch works fine on this board as well. I have no time for a more thorough testing with different CPUs and frequencies. Anyhow...
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se
Acked-by: Wolfgang Grandegger wg@grandegger.com
I realized some minor coding style issues, though. See below.
drivers/i2c/fsl_i2c.c | 90 ++++++++++++++++++++++++++++++------------------- 1 files changed, 55 insertions(+), 35 deletions(-)
diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c index 0c5f6be..78f21c7 100644 --- a/drivers/i2c/fsl_i2c.c +++ b/drivers/i2c/fsl_i2c.c @@ -100,29 +100,9 @@ static const struct fsl_i2c *i2c_dev[2] = { */ static const struct { unsigned short divider; -#ifdef __PPC__
- u8 dfsr;
-#endif u8 fdr; } fsl_i2c_speed_map[] = { -#ifdef __PPC__
- {160, 1, 32}, {192, 1, 33}, {224, 1, 34}, {256, 1, 35},
- {288, 1, 0}, {320, 1, 1}, {352, 6, 1}, {384, 1, 2}, {416, 6, 2},
- {448, 1, 38}, {480, 1, 3}, {512, 1, 39}, {544, 11, 3}, {576, 1, 4},
- {608, 22, 3}, {640, 1, 5}, {672, 32, 3}, {704, 11, 5}, {736, 43, 3},
- {768, 1, 6}, {800, 54, 3}, {832, 11, 6}, {896, 1, 42}, {960, 1, 7},
- {1024, 1, 43}, {1088, 22, 7}, {1152, 1, 8}, {1216, 43, 7}, {1280, 1, 9},
- {1408, 22, 9}, {1536, 1, 10}, {1664, 22, 10}, {1792, 1, 46},
- {1920, 1, 11}, {2048, 1, 47}, {2176, 43, 11}, {2304, 1, 12},
- {2560, 1, 13}, {2816, 43, 13}, {3072, 1, 14}, {3328, 43, 14},
- {3584, 1, 50}, {3840, 1, 15}, {4096, 1, 51}, {4608, 1, 16},
- {5120, 1, 17}, {6144, 1, 18}, {7168, 1, 54}, {7680, 1, 19},
- {8192, 1, 55}, {9216, 1, 20}, {10240, 1, 21}, {12288, 1, 22},
- {14336, 1, 58}, {15360, 1, 23}, {16384, 1, 59}, {18432, 1, 24},
- {20480, 1, 25}, {24576, 1, 26}, {28672, 1, 62}, {30720, 1, 27},
- {32768, 1, 63}, {36864, 1, 28}, {40960, 1, 29}, {49152, 1, 30},
- {61440, 1, 31}, {-1, 1, 31}
-#elif defined(__M68K__) +#ifdef __M68K__ {20, 32}, {22, 33}, {24, 34}, {26, 35}, {28, 0}, {28, 36}, {30, 1}, {32, 37}, {34, 2}, {36, 38}, {40, 3}, {40, 39}, @@ -158,7 +138,6 @@ static unsigned int set_i2c_bus_speed(const struct fsl_i2c *dev, unsigned int i2c_clk, unsigned int speed) { unsigned short divider = min(i2c_clk / speed, (unsigned short) -1);
unsigned int i;
/*
- We want to choose an FDR/DFSR that generates an I2C bus speed that
@@ -166,31 +145,72 @@ static unsigned int set_i2c_bus_speed(const struct fsl_i2c *dev, * want the first divider that is equal to or greater than the * calculated divider. */
- for (i = 0; i < ARRAY_SIZE(fsl_i2c_speed_map); i++)
if (fsl_i2c_speed_map[i].divider >= divider) {u8 fdr;#ifdef __PPC__
u8 dfsr;
- u8 dfsr, fdr = 0x31; /* Default if no FDR found */
- /* a, b and dfsr matches identifiers A,B and C respectively in AN2919 */
- unsigned short a, b, ga, gb;
- unsigned long c_div, est_div;
#ifdef CONFIG_FSL_I2C_CUSTOM_DFSR
dfsr = CONFIG_FSL_I2C_CUSTOM_DFSR;
- dfsr = CONFIG_FSL_I2C_CUSTOM_DFSR;
#else
dfsr = fsl_i2c_speed_map[i].dfsr;-#endif
writeb(dfsr, &dev->dfsrr); /* set default filter */
- /* Condition 1: dfsr <= 50/T */
- dfsr = (5 * (i2c_clk / 1000)) / 100000;
#endif #ifdef CONFIG_FSL_I2C_CUSTOM_FDR
fdr = CONFIG_FSL_I2C_CUSTOM_FDR;speed = i2c_clk / divider; /* Fake something */
- fdr = CONFIG_FSL_I2C_CUSTOM_FDR;
- speed = i2c_clk / divider; /* Fake something */
#else
- debug("Requested speed:%d, i2c_clk:%d\n", speed, i2c_clk);
- if (!dfsr)
dfsr = 1;- est_div = ~0;
- for (ga = 0x4, a = 10; a <= 30; ga++, a += 2) {
for (gb = 0; gb < 8; gb++) {b = 16 << gb;c_div = b * (a + ((3*dfsr)/b)*2);
Please use spaces around "/" and "*".
if ((c_div > divider) && (c_div < est_div)) {unsigned short bin_gb, bin_ga;est_div = c_div;bin_gb = gb << 2;bin_ga = (ga & 0x3) | ((ga & 0x4) << 3);fdr = bin_gb | bin_ga;speed = i2c_clk / est_div;debug("FDR:0x%.2x, div:%ld, ga:0x%x, gb:0x%x, ""a:%d, b:%d, speed:%d\n",fdr, est_div, ga, gb, a, b, speed);/* Condition 2 not accounted for */debug("Tr <= %d ns\n",
Please remove trailing white space.
Wolfgang.
Wolfgang Grandegger wg@denx.de wrote on 21/09/2009 12:53:36:
Hi Joakim,
Joakim Tjernlund wrote:
The latest AN2919 has changed the way FDR/DFSR should be calculated. Update the driver according to spec. However, Condition 2 is not accounted for as it is not clear how to do so.
I compared rev. 5 of AN2919 with rev. 3 and, as you pointed out, it puts additional constraints on how to select dfsr and fdr. Especially dfsr should not exceed a certain, frequency dependent value: dfsr <= 50 / period-in-ns. Therefore, I expected problems with divider values from the table which high dfsr values. I did your "=> date;date;date;date" test on a MPC8548 board using dfsr=43 and fdr=7 but it did not fail. According to the rev. 5, dfsr is not allowed to be greater than 8. Your patch works fine on this board as well. I have no time for a more thorough testing with different CPUs and frequencies. Anyhow...
Yes, I too notice that higher dfsr values than the spec says works, in fact my board needs at least dfsr 8 to be 100% stable but the spec says no more that 6 for my board. I suspect that we should enforce a minimum value of 8 to be on the safe side, especially as the HW default is 0x10. Anyhow it is better now than before.
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se
Acked-by: Wolfgang Grandegger wg@grandegger.com
Thanks.
I realized some minor coding style issues, though. See below.
Noted.
Joakim Tjernlund wrote:
Wolfgang Grandegger wg@denx.de wrote on 21/09/2009 12:53:36:
Hi Joakim,
Joakim Tjernlund wrote:
The latest AN2919 has changed the way FDR/DFSR should be calculated. Update the driver according to spec. However, Condition 2 is not accounted for as it is not clear how to do so.
I compared rev. 5 of AN2919 with rev. 3 and, as you pointed out, it puts additional constraints on how to select dfsr and fdr. Especially dfsr should not exceed a certain, frequency dependent value: dfsr <= 50 / period-in-ns. Therefore, I expected problems with divider values from the table which high dfsr values. I did your "=> date;date;date;date" test on a MPC8548 board using dfsr=43 and fdr=7 but it did not fail. According to the rev. 5, dfsr is not allowed to be greater than 8. Your patch works fine on this board as well. I have no time for a more thorough testing with different CPUs and frequencies. Anyhow...
Yes, I too notice that higher dfsr values than the spec says works, in fact my board needs at least dfsr 8 to be 100% stable but the spec says no more that 6 for my board. I suspect that we should enforce a minimum value of 8 to be on the safe side, especially as the HW default is 0x10.
You say that the new constraints introduced by rev. 5 are not even correct!? Well, these modifications are obscure anyhow.
Anyhow it is better now than before.
At least for your board. Let's keep an eye on people reporting I2C problems for these processors on the U-Boot and linuxppc-dev ML.
Wolfgang.
Wolfgang Grandegger wg@denx.de wrote on 21/09/2009 13:59:04:
Joakim Tjernlund wrote:
Wolfgang Grandegger wg@denx.de wrote on 21/09/2009 12:53:36:
Hi Joakim,
Joakim Tjernlund wrote:
The latest AN2919 has changed the way FDR/DFSR should be calculated. Update the driver according to spec. However, Condition 2 is not accounted for as it is not clear how to do so.
I compared rev. 5 of AN2919 with rev. 3 and, as you pointed out, it puts additional constraints on how to select dfsr and fdr. Especially dfsr should not exceed a certain, frequency dependent value: dfsr <= 50 / period-in-ns. Therefore, I expected problems with divider values from the table which high dfsr values. I did your "=> date;date;date;date" test on a MPC8548 board using dfsr=43 and fdr=7 but it did not fail. According to the rev. 5, dfsr is not allowed to be greater than 8. Your patch works fine on this board as well. I have no time for a more thorough testing with different CPUs and frequencies. Anyhow...
Yes, I too notice that higher dfsr values than the spec says works, in fact my board needs at least dfsr 8 to be 100% stable but the spec says no more that 6 for my board. I suspect that we should enforce a minimum value of 8 to be on the safe side, especially as the HW default is 0x10.
You say that the new constraints introduced by rev. 5 are not even correct!? Well, these modifications are obscure anyhow.
My board has a higher rise time than the I2C spec(1 us) so it may be the reason for me. It *looks* like a higher dfsr makes the controller tolerate more "noise". Where does the dfsr=1 come from in the first place? It seems random and is very far from HW default.
Hello Joakim,
Joakim Tjernlund wrote:
Some boards need a higher DFSR value than the spec currently recommends so give these boards the means to define there own.
For completeness, add CONFIG_FSL_I2C_CUSTOM_FDR too.
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se
drivers/i2c/fsl_i2c.c | 14 +++++++++++--- 1 files changed, 11 insertions(+), 3 deletions(-)
Applied to u-boot-i2c testing
Thanks.
bye Heiko
Joakim Tjernlund wrote:
+#ifdef CONFIG_FSL_I2C_CUSTOM_FDR
fdr = CONFIG_FSL_I2C_CUSTOM_FDR;speed = i2c_clk / divider; /* Fake something */
How about setting 'speed' to CONFIG_SYS_I2C_SPEED?
Joakim Tjernlund wrote:
+#ifdef CONFIG_FSL_I2C_CUSTOM_FDR
fdr = CONFIG_FSL_I2C_CUSTOM_FDR;speed = i2c_clk / divider; /* Fake something */How about setting 'speed' to CONFIG_SYS_I2C_SPEED?
Naa, if you want that you just pass CONFIG_SYS_I2C_SPEED to i2c_init(), which is the default.
Hello Joakim,
Joakim Tjernlund wrote:
After issuing a STOP one must wait until the STOP has completed on the bus before doing something new to the controller.
Also add an extra read of SR as the manual mentions doing that is a good idea.
Remove surplus write of CR just before a write, isn't required and could potentially disturb the I2C bus.
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se
drivers/i2c/fsl_i2c.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)
Applied to u-boot-i2c testing
I applied your 3 patches at the moment to the u-boot-i2c testing branch, in the hope to see some "Tested-by" ;-)
Thanks.
bye Heiko
Hello Joakim,
Joakim Tjernlund wrote:
After issuing a STOP one must wait until the STOP has completed on the bus before doing something new to the controller.
Also add an extra read of SR as the manual mentions doing that is a good idea.
Remove surplus write of CR just before a write, isn't required and could potentially disturb the I2C bus.
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se
drivers/i2c/fsl_i2c.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)
Applied to u-boot-i2c testing
I applied your 3 patches at the moment to the u-boot-i2c testing branch, in the hope to see some "Tested-by" ;-)
Great, hopefully you will get some Tested-by from Wolfgang G. and Timur
Dear Joakim Tjernlund,
In message 1253178437-32398-1-git-send-email-Joakim.Tjernlund@transmode.se you wrote:
After issuing a STOP one must wait until the STOP has completed on the bus before doing something new to the controller.
Also add an extra read of SR as the manual mentions doing that is a good idea.
Remove surplus write of CR just before a write, isn't required and could potentially disturb the I2C bus.
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se
drivers/i2c/fsl_i2c.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)
Could you _please_ start adding some version information to your patches, and a list of changes between the versions?
Thanks.
Best regards,
Wolfgang Denk
Dear Joakim Tjernlund,
In message 1253178437-32398-1-git-send-email-Joakim.Tjernlund@transmode.se you wrote:
After issuing a STOP one must wait until the STOP has completed on the bus before doing something new to the controller.
Also add an extra read of SR as the manual mentions doing that is a good idea.
Remove surplus write of CR just before a write, isn't required and could potentially disturb the I2C bus.
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se
drivers/i2c/fsl_i2c.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)
Could you _please_ start adding some version information to your patches, and a list of changes between the versions?
Thanks.
Yes, that was a bit sloppy on my part, sorry. What do you want me to do now? Resend the whole series or do I get away with it this time? All but the last patch is in u-boot-i2c testing(by Heiko) repo already.
Hello Joakim,
Joakim Tjernlund wrote:
Dear Joakim Tjernlund,
In message 1253178437-32398-1-git-send-email-Joakim.Tjernlund@transmode.se you wrote:
After issuing a STOP one must wait until the STOP has completed on the bus before doing something new to the controller.
Also add an extra read of SR as the manual mentions doing that is a good idea.
Remove surplus write of CR just before a write, isn't required and could potentially disturb the I2C bus.
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se
drivers/i2c/fsl_i2c.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)
Could you _please_ start adding some version information to your patches, and a list of changes between the versions?
Thanks.
Yes, that was a bit sloppy on my part, sorry. What do you want me to do now? Resend the whole series or do I get away with it this time? All but the last patch is in u-boot-i2c testing(by Heiko) repo already.
You mean with last patch, this patch:?
http://lists.denx.de/pipermail/u-boot/2009-September/061187.html
If, so, I think, I add it also to the u-boot-i2c testing branch, and it is OK for now, but please for further patches, add some information as Wolfgang suggested.
thanks
bye Heiko
Heiko Schocher heiko.schocher@invitel.hu wrote on 23/09/2009 11:02:09:
Hello Joakim,
Joakim Tjernlund wrote:
Dear Joakim Tjernlund,
In message <1253178437-32398-1-git-send-email-
Joakim.Tjernlund@transmode.se> you wrote:
After issuing a STOP one must wait until the STOP has completed on the bus before doing something new to the controller.
Also add an extra read of SR as the manual mentions doing that is a good idea.
Remove surplus write of CR just before a write, isn't required and could potentially disturb the I2C bus.
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se
drivers/i2c/fsl_i2c.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)
Could you _please_ start adding some version information to your patches, and a list of changes between the versions?
Thanks.
Yes, that was a bit sloppy on my part, sorry. What do you want me to do now? Resend the whole series or do I get away with it this time? All but the last patch is in u-boot-i2c testing(by Heiko) repo already.
You mean with last patch, this patch:?
http://lists.denx.de/pipermail/u-boot/2009-September/061187.html
Yes, that is the last one. I don't see it in testing though. I also think that fsl_i2c: Wait for STOP condition to propagate, fsl_i2c: Add CONFIG_FSL_I2C_CUSTOM_{DFSR/FDR} and fsl_i2c: Wait for STOP condition to propagate can go to WD
The fsl_i2c: Impl. AN2919, rev 5 to calculate FDR/DFSR can wait a bit longer to get some more testing if you think it is needed.
Jocke
Hello Joakim,
Joakim Tjernlund wrote:
Heiko Schocher heiko.schocher@invitel.hu wrote on 23/09/2009 11:02:09:
Hello Joakim,
Joakim Tjernlund wrote:
Dear Joakim Tjernlund,
In message <1253178437-32398-1-git-send-email-
Joakim.Tjernlund@transmode.se> you wrote:
After issuing a STOP one must wait until the STOP has completed on the bus before doing something new to the controller.
Also add an extra read of SR as the manual mentions doing that is a good idea.
Remove surplus write of CR just before a write, isn't required and could potentially disturb the I2C bus.
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se
drivers/i2c/fsl_i2c.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)
Could you _please_ start adding some version information to your patches, and a list of changes between the versions?
Thanks.
Yes, that was a bit sloppy on my part, sorry. What do you want me to do now? Resend the whole series or do I get away with it this time? All but the last patch is in u-boot-i2c testing(by Heiko) repo already.
You mean with last patch, this patch:?
http://lists.denx.de/pipermail/u-boot/2009-September/061187.html
Yes, that is the last one. I don't see it in testing though.
applied to u-boot-i2c testing
I also think that fsl_i2c: Wait for STOP condition to propagate, fsl_i2c: Add CONFIG_FSL_I2C_CUSTOM_{DFSR/FDR} and fsl_i2c: Wait for STOP condition to propagate can go to WD
The fsl_i2c: Impl. AN2919, rev 5 to calculate FDR/DFSR can wait a bit longer to get some more testing if you think it is needed.
I wait for the end of the week (I hope, I find time to make some tests too), and send a pull request on monday to Wolfgang.
bye Heiko
participants (7)
-
Heiko Schocher -
Heiko Schocher -
Joakim Tjernlund -
Joakim Tjernlund
-
Timur Tabi -
Wolfgang Denk -
Wolfgang Grandegger