[U-Boot] ColdFire I2C implementing I2C idle [PATCH]
Signed-off-by: David Wu davidwu@arcturusnetworks.com Signed-off-by: Michael Durrant mdurrant@arcturusnetworks.com
Patch created against u-boot-2009.11 release
drivers_i2c_fsl_i2c.patch - need to set I2C to be idle acoording to the MCF5282 user's manual
If I2SR[IBB] is set when the I2C bus module is enabled, execute the following code sequence before proceeding with normal initialization code. This issues a STOP command to the slave device, placing it in idle state as if it were just power-cycled on.
I2CR = 0x0 I2CR = 0xA dummy read of I2DR I2SR = 0x0 I2CR = 0x0
-- Michael Durrant mdurrant@arcturusnetworks.com
Hello Michael,
Michael Durrant wrote:
Signed-off-by: David Wu davidwu@arcturusnetworks.com Signed-off-by: Michael Durrant mdurrant@arcturusnetworks.com
Can you please use git-format-patch for creating this patch, and post it here again not attached, just as plain text?
(Same for your other 3 patches)
Thanks.
bye Heiko
drivers_i2c_fsl_i2c.patch - need to set I2C to be idle according to the MCF5282 user's manual
If I2SR[IBB] is set when the I2C bus module is enabled, execute the following code sequence before proceeding with normal initialization code. This issues a STOP command to the slave device, placing it in idle state as if it were just power-cycled on.
I2CR = 0x0 I2CR = 0xA dummy read of I2DR I2SR = 0x0 I2CR = 0x0
Signed-off-by: David Wu davidwu@arcturusnetworks.com Signed-off-by: Michael Durrant mdurrant@arcturusnetworks.com --- drivers/i2c/fsl_i2c.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c index 2241990..bce750c 100644 --- a/drivers/i2c/fsl_i2c.c +++ b/drivers/i2c/fsl_i2c.c @@ -251,12 +251,25 @@ i2c_init(int speed, int slaveadd) #endif }
+static __inline__ int i2c_set_idle(void) +{ + writeb(0, &i2c_dev[i2c_bus_num]->cr); + writeb(I2C_CR_MEN | I2C_CR_MSTA, &i2c_dev[i2c_bus_num]->cr); + readb(&i2c_dev[i2c_bus_num]->dr); + writeb(0, &i2c_dev[i2c_bus_num]->sr); + writeb(0, &i2c_dev[i2c_bus_num]->cr); + writeb(I2C_CR_MEN, &i2c_dev[i2c_bus_num]->cr); +} + static int i2c_wait4bus(void) { unsigned long long timeval = get_ticks(); const unsigned long long timeout = usec2ticks(CONFIG_I2C_MBB_TIMEOUT);
+ if (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB) + i2c_set_idle(); + while (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB) { if ((get_ticks() - timeval) > timeout) return -1; -- 1.4.3.4
Hello Michael,
Thanks for posting your patches in plain text.
Michael Durrant wrote:
drivers_i2c_fsl_i2c.patch
- need to set I2C to be idle according to the MCF5282 user's manual
If I2SR[IBB] is set when the I2C bus module is enabled, execute the following code sequence before proceeding with normal initialization code. This issues a STOP command to the slave device, placing it in idle state as if it were just power-cycled on.
I2CR = 0x0 I2CR = 0xA dummy read of I2DR I2SR = 0x0 I2CR = 0x0
Signed-off-by: David Wu davidwu@arcturusnetworks.com Signed-off-by: Michael Durrant mdurrant@arcturusnetworks.com
drivers/i2c/fsl_i2c.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c index 2241990..bce750c 100644 --- a/drivers/i2c/fsl_i2c.c +++ b/drivers/i2c/fsl_i2c.c @@ -251,12 +251,25 @@ i2c_init(int speed, int slaveadd) #endif }
+static __inline__ int i2c_set_idle(void) +{
- writeb(0, &i2c_dev[i2c_bus_num]->cr);
- writeb(I2C_CR_MEN | I2C_CR_MSTA, &i2c_dev[i2c_bus_num]->cr);
- readb(&i2c_dev[i2c_bus_num]->dr);
- writeb(0, &i2c_dev[i2c_bus_num]->sr);
- writeb(0, &i2c_dev[i2c_bus_num]->cr);
- writeb(I2C_CR_MEN, &i2c_dev[i2c_bus_num]->cr);
+}
static int i2c_wait4bus(void) { unsigned long long timeval = get_ticks(); const unsigned long long timeout = usec2ticks(CONFIG_I2C_MBB_TIMEOUT);
- if (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB)
i2c_set_idle();- while (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB) { if ((get_ticks() - timeval) > timeout) return -1;
Hmm.. I can;t find this for example in the MPC8360ERM.pdf, which uses also this driver ... So I vote for activating this only, if this driver is used for __M68K__ ...
Also, you write, that this sequence is necessary before normal initialization code, but i2c_wait4bus() is called from i2c_read() and i2c_write(), so the I2C bus is long ago initialized ... or what do you mean with "normal initialization code" ?
Also, whats with multimaster systems? Your code maybe cuts a running data transmission. The MPC8360ERM.pdf manual says "check the MBB bit, if the bus is free, before switching to master mode", thats what actual code did ... so, I want this only activated, if __M68K__ is defined ...
bye Heiko
Hello Michael,
Thanks for posting your patches in plain text.
Michael Durrant wrote:
drivers_i2c_fsl_i2c.patch
- need to set I2C to be idle according to the MCF5282 user's manual
If I2SR[IBB] is set when the I2C bus module is enabled, execute the following code sequence before proceeding with normal initialization code. This issues a STOP command to the slave device, placing it in idle state as if it were just power-cycled on.
I2CR = 0x0 I2CR = 0xA dummy read of I2DR I2SR = 0x0 I2CR = 0x0
Signed-off-by: David Wu davidwu@arcturusnetworks.com Signed-off-by: Michael Durrant mdurrant@arcturusnetworks.com
drivers/i2c/fsl_i2c.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c index 2241990..bce750c 100644 --- a/drivers/i2c/fsl_i2c.c +++ b/drivers/i2c/fsl_i2c.c @@ -251,12 +251,25 @@ i2c_init(int speed, int slaveadd) #endif }
+static __inline__ int i2c_set_idle(void) +{
- writeb(0, &i2c_dev[i2c_bus_num]->cr);
- writeb(I2C_CR_MEN | I2C_CR_MSTA, &i2c_dev[i2c_bus_num]->cr);
- readb(&i2c_dev[i2c_bus_num]->dr);
- writeb(0, &i2c_dev[i2c_bus_num]->sr);
- writeb(0, &i2c_dev[i2c_bus_num]->cr);
- writeb(I2C_CR_MEN, &i2c_dev[i2c_bus_num]->cr);
+}
static int i2c_wait4bus(void) { unsigned long long timeval = get_ticks(); const unsigned long long timeout = usec2ticks(CONFIG_I2C_MBB_TIMEOUT);
- if (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB)
i2c_set_idle();- while (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB) { if ((get_ticks() - timeval) > timeout) return -1;
Hmm.. I can;t find this for example in the MPC8360ERM.pdf, which uses also this driver ... So I vote for activating this only, if this driver is used for __M68K__ ...
Also, you write, that this sequence is necessary before normal initialization code, but i2c_wait4bus() is called from i2c_read() and i2c_write(), so the I2C bus is long ago initialized ... or what do you mean with "normal initialization code" ?
Also, whats with multimaster systems? Your code maybe cuts a running data transmission. The MPC8360ERM.pdf manual says "check the MBB bit, if the bus is free, before switching to master mode", thats what actual code did ... so, I want this only activated, if __M68K__ is defined ...
All true. This cannot go in as is. What you need is a I2C reset sequence as the above isn't enough in the general case. Michael, have a look at the kernel driver, it has some fixup code you could borrow.
Jocke
Hello Joakim,
Joakim Tjernlund wrote:
Hello Michael,
Thanks for posting your patches in plain text.
Michael Durrant wrote:
drivers_i2c_fsl_i2c.patch
- need to set I2C to be idle according to the MCF5282 user's manual
If I2SR[IBB] is set when the I2C bus module is enabled, execute the following code sequence before proceeding with normal initialization code. This issues a STOP command to the slave device, placing it in idle state as if it were just power-cycled on.
I2CR = 0x0 I2CR = 0xA dummy read of I2DR I2SR = 0x0 I2CR = 0x0
Signed-off-by: David Wu davidwu@arcturusnetworks.com Signed-off-by: Michael Durrant mdurrant@arcturusnetworks.com
drivers/i2c/fsl_i2c.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c index 2241990..bce750c 100644 --- a/drivers/i2c/fsl_i2c.c +++ b/drivers/i2c/fsl_i2c.c @@ -251,12 +251,25 @@ i2c_init(int speed, int slaveadd) #endif }
+static __inline__ int i2c_set_idle(void) +{
- writeb(0, &i2c_dev[i2c_bus_num]->cr);
- writeb(I2C_CR_MEN | I2C_CR_MSTA, &i2c_dev[i2c_bus_num]->cr);
- readb(&i2c_dev[i2c_bus_num]->dr);
- writeb(0, &i2c_dev[i2c_bus_num]->sr);
- writeb(0, &i2c_dev[i2c_bus_num]->cr);
- writeb(I2C_CR_MEN, &i2c_dev[i2c_bus_num]->cr);
+}
static int i2c_wait4bus(void) { unsigned long long timeval = get_ticks(); const unsigned long long timeout = usec2ticks(CONFIG_I2C_MBB_TIMEOUT);
- if (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB)
i2c_set_idle();- while (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB) { if ((get_ticks() - timeval) > timeout) return -1;
Hmm.. I can;t find this for example in the MPC8360ERM.pdf, which uses also this driver ... So I vote for activating this only, if this driver is used for __M68K__ ...
Also, you write, that this sequence is necessary before normal initialization code, but i2c_wait4bus() is called from i2c_read() and i2c_write(), so the I2C bus is long ago initialized ... or what do you mean with "normal initialization code" ?
Also, whats with multimaster systems? Your code maybe cuts a running data transmission. The MPC8360ERM.pdf manual says "check the MBB bit, if the bus is free, before switching to master mode", thats what actual code did ... so, I want this only activated, if __M68K__ is defined ...
All true. This cannot go in as is. What you need is a I2C reset sequence as the above isn't enough in the general case. Michael, have a look at the kernel driver, it has some fixup code you could borrow.
Michael: Maybe you take a look in u-boot:board/keymile/common/common.c i2c_init_board(), there is a I2C reset sequence for 83xx based boards from keymile, which use this i2c driver.
Maybe its time to move this code in the i2c driver?
bye Heiko
Hello Heiko,
On Thu, Jan 21, 2010 at 09:04:29AM +0100, Heiko Schocher wrote:
Hello Joakim,
Joakim Tjernlund wrote:
Hello Michael,
Thanks for posting your patches in plain text.
Michael Durrant wrote:
drivers_i2c_fsl_i2c.patch
- need to set I2C to be idle according to the MCF5282 user's manual
If I2SR[IBB] is set when the I2C bus module is enabled, execute the following code sequence before proceeding with normal initialization code. This issues a STOP command to the slave device, placing it in idle state as if it were just power-cycled on.
I2CR = 0x0 I2CR = 0xA dummy read of I2DR I2SR = 0x0 I2CR = 0x0
Signed-off-by: David Wu davidwu@arcturusnetworks.com Signed-off-by: Michael Durrant mdurrant@arcturusnetworks.com
drivers/i2c/fsl_i2c.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c index 2241990..bce750c 100644 --- a/drivers/i2c/fsl_i2c.c +++ b/drivers/i2c/fsl_i2c.c @@ -251,12 +251,25 @@ i2c_init(int speed, int slaveadd) #endif }
+static __inline__ int i2c_set_idle(void) +{
- writeb(0, &i2c_dev[i2c_bus_num]->cr);
- writeb(I2C_CR_MEN | I2C_CR_MSTA, &i2c_dev[i2c_bus_num]->cr);
- readb(&i2c_dev[i2c_bus_num]->dr);
- writeb(0, &i2c_dev[i2c_bus_num]->sr);
- writeb(0, &i2c_dev[i2c_bus_num]->cr);
- writeb(I2C_CR_MEN, &i2c_dev[i2c_bus_num]->cr);
+}
static int i2c_wait4bus(void) { unsigned long long timeval = get_ticks(); const unsigned long long timeout = usec2ticks(CONFIG_I2C_MBB_TIMEOUT);
- if (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB)
i2c_set_idle();- while (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB) { if ((get_ticks() - timeval) > timeout) return -1;
Hmm.. I can;t find this for example in the MPC8360ERM.pdf, which uses also this driver ... So I vote for activating this only, if this driver is used for __M68K__ ...
Also, you write, that this sequence is necessary before normal initialization code, but i2c_wait4bus() is called from i2c_read() and i2c_write(), so the I2C bus is long ago initialized ... or what do you mean with "normal initialization code" ?
Also, whats with multimaster systems? Your code maybe cuts a running data transmission. The MPC8360ERM.pdf manual says "check the MBB bit, if the bus is free, before switching to master mode", thats what actual code did ... so, I want this only activated, if __M68K__ is defined ...
All true. This cannot go in as is. What you need is a I2C reset sequence as the above isn't enough in the general case. Michael, have a look at the kernel driver, it has some fixup code you could borrow.
Michael: Maybe you take a look in u-boot:board/keymile/common/common.c i2c_init_board(), there is a I2C reset sequence for 83xx based boards from keymile, which use this i2c driver.
Maybe its time to move this code in the i2c driver?
this code looks good except I do not see how the special case of multi-master systems you mentioned is handled. I think for multi-master a timeout would have to be implemented to wait for a possible other master transfer to finish before initiating the abort sequence, or is this too much time spent during init?
Or am I missing something obvious?
(Sorry, no PPC experience here, we just had the exact same problem with the old uclinux I2C driver on coldfire, where I currently use a quickly hacked own driver in favor of the completely broken original driver which did not even return an error on NACK...)
Regards, Wolfgang
Hello Wolfgang,
Wolfgang Wegner wrote:
On Thu, Jan 21, 2010 at 09:04:29AM +0100, Heiko Schocher wrote:
Hello Joakim,
Joakim Tjernlund wrote:
Hello Michael,
Thanks for posting your patches in plain text.
Michael Durrant wrote:
drivers_i2c_fsl_i2c.patch
- need to set I2C to be idle according to the MCF5282 user's manual
If I2SR[IBB] is set when the I2C bus module is enabled, execute the following code sequence before proceeding with normal initialization code. This issues a STOP command to the slave device, placing it in idle state as if it were just power-cycled on.
I2CR = 0x0 I2CR = 0xA dummy read of I2DR I2SR = 0x0 I2CR = 0x0
Signed-off-by: David Wu davidwu@arcturusnetworks.com Signed-off-by: Michael Durrant mdurrant@arcturusnetworks.com
drivers/i2c/fsl_i2c.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c index 2241990..bce750c 100644 --- a/drivers/i2c/fsl_i2c.c +++ b/drivers/i2c/fsl_i2c.c @@ -251,12 +251,25 @@ i2c_init(int speed, int slaveadd) #endif }
+static __inline__ int i2c_set_idle(void) +{
- writeb(0, &i2c_dev[i2c_bus_num]->cr);
- writeb(I2C_CR_MEN | I2C_CR_MSTA, &i2c_dev[i2c_bus_num]->cr);
- readb(&i2c_dev[i2c_bus_num]->dr);
- writeb(0, &i2c_dev[i2c_bus_num]->sr);
- writeb(0, &i2c_dev[i2c_bus_num]->cr);
- writeb(I2C_CR_MEN, &i2c_dev[i2c_bus_num]->cr);
+}
static int i2c_wait4bus(void) { unsigned long long timeval = get_ticks(); const unsigned long long timeout = usec2ticks(CONFIG_I2C_MBB_TIMEOUT);
- if (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB)
i2c_set_idle();- while (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB) { if ((get_ticks() - timeval) > timeout) return -1;
Hmm.. I can;t find this for example in the MPC8360ERM.pdf, which uses also this driver ... So I vote for activating this only, if this driver is used for __M68K__ ...
Also, you write, that this sequence is necessary before normal initialization code, but i2c_wait4bus() is called from i2c_read() and i2c_write(), so the I2C bus is long ago initialized ... or what do you mean with "normal initialization code" ?
Also, whats with multimaster systems? Your code maybe cuts a running data transmission. The MPC8360ERM.pdf manual says "check the MBB bit, if the bus is free, before switching to master mode", thats what actual code did ... so, I want this only activated, if __M68K__ is defined ...
All true. This cannot go in as is. What you need is a I2C reset sequence as the above isn't enough in the general case. Michael, have a look at the kernel driver, it has some fixup code you could borrow.
Michael: Maybe you take a look in u-boot:board/keymile/common/common.c i2c_init_board(), there is a I2C reset sequence for 83xx based boards from keymile, which use this i2c driver.
Maybe its time to move this code in the i2c driver?
this code looks good except I do not see how the special case of multi-master systems you mentioned is handled.
I have no experience with multimaster systems, and I think, this special case is not yet factored in code.
I think for multi-master a timeout would have to be implemented to wait for a possible other master transfer to finish before initiating the abort sequence, or is this too much time spent during init?
Or, it could be detected? If BB Bit is set and the SCL pin doesn;t alter, the I2C bus must be blocked ... just an idea.
bye Heiko
Heiko Schocher wrote:
Hello Wolfgang,
Wolfgang Wegner wrote:
On Thu, Jan 21, 2010 at 09:04:29AM +0100, Heiko Schocher wrote:
Hello Joakim,
Joakim Tjernlund wrote:
Hello Michael,
Thanks for posting your patches in plain text.
Michael Durrant wrote:
drivers_i2c_fsl_i2c.patch
- need to set I2C to be idle according to the MCF5282 user's manual
If I2SR[IBB] is set when the I2C bus module is enabled, execute the following code sequence before proceeding with normal initialization code. This issues a STOP command to the slave device, placing it in idle state as if it were just power-cycled on.
I2CR = 0x0 I2CR = 0xA dummy read of I2DR I2SR = 0x0 I2CR = 0x0
Signed-off-by: David Wu davidwu@arcturusnetworks.com Signed-off-by: Michael Durrant mdurrant@arcturusnetworks.com
drivers/i2c/fsl_i2c.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c index 2241990..bce750c 100644 --- a/drivers/i2c/fsl_i2c.c +++ b/drivers/i2c/fsl_i2c.c @@ -251,12 +251,25 @@ i2c_init(int speed, int slaveadd) #endif }
+static __inline__ int i2c_set_idle(void) +{
- writeb(0, &i2c_dev[i2c_bus_num]->cr);
- writeb(I2C_CR_MEN | I2C_CR_MSTA, &i2c_dev[i2c_bus_num]->cr);
- readb(&i2c_dev[i2c_bus_num]->dr);
- writeb(0, &i2c_dev[i2c_bus_num]->sr);
- writeb(0, &i2c_dev[i2c_bus_num]->cr);
- writeb(I2C_CR_MEN, &i2c_dev[i2c_bus_num]->cr);
+}
static int i2c_wait4bus(void) { unsigned long long timeval = get_ticks(); const unsigned long long timeout = usec2ticks(CONFIG_I2C_MBB_TIMEOUT);
- if (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB)
i2c_set_idle();- while (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB) { if ((get_ticks() - timeval) > timeout) return -1;
Hmm.. I can;t find this for example in the MPC8360ERM.pdf, which uses also this driver ... So I vote for activating this only, if this driver is used for __M68K__ ...
Also, you write, that this sequence is necessary before normal initialization code, but i2c_wait4bus() is called from i2c_read() and i2c_write(), so the I2C bus is long ago initialized ... or what do you mean with "normal initialization code" ?
Also, whats with multimaster systems? Your code maybe cuts a running data transmission. The MPC8360ERM.pdf manual says "check the MBB bit, if the bus is free, before switching to master mode", thats what actual code did ... so, I want this only activated, if __M68K__ is defined ...
All true. This cannot go in as is. What you need is a I2C reset sequence as the above isn't enough in the general case. Michael, have a look at the kernel driver, it has some fixup code you could borrow.
Michael: Maybe you take a look in u-boot:board/keymile/common/common.c i2c_init_board(), there is a I2C reset sequence for 83xx based boards from keymile, which use this i2c driver.
Maybe its time to move this code in the i2c driver?
Will do ... David Wu emailed me a few comments as well that I include at the end. For now I agree that we should protect non ColdFire V2/V3 users with the __M68K__ definition
this code looks good except I do not see how the special case of multi-master systems you mentioned is handled.
I have no experience with multimaster systems, and I think, this special case is not yet factored in code.
I think for multi-master a timeout would have to be implemented to wait for a possible other master transfer to finish before initiating the abort sequence, or is this too much time spent during init?
Or, it could be detected? If BB Bit is set and the SCL pin doesn;t alter, the I2C bus must be blocked ... just an idea.
bye Heiko
David Wu's thoughts follow:
1. agree to add a protection using #if defined(__M68K__) at some point this i2c_set_idle() has to be called when this I2C master is enabled. In one master environment it can be in - i2c_set_bus_speed() where the I2C is enabled for that bus, or - i2c_init() or, - board specific routine - please suggest the right location.
2. since only the master can do a read and write call, when the bus is busy there is no need to wait until it is free which may never be free if we don't force a STOP.
3. In a multi-master environment, there should be an additional condition as to when one master can read/write, still i2c_wait4bus() may timeout.
-- Regards, David Wu
Thanks for the great feedback.
Regards, Michael Durrant
Hello Michael,
Michael Durrant wrote:
Heiko Schocher wrote:
Hello Wolfgang,
Wolfgang Wegner wrote:
On Thu, Jan 21, 2010 at 09:04:29AM +0100, Heiko Schocher wrote:
Hello Joakim,
Joakim Tjernlund wrote:
Hello Michael,
Thanks for posting your patches in plain text.
Michael Durrant wrote:
> drivers_i2c_fsl_i2c.patch > - need to set I2C to be idle according to the MCF5282 user's manual
[...]
> Hmm.. I can;t find this for example in the MPC8360ERM.pdf, which uses also this driver ... So I vote for activating this only, if this driver is used for __M68K__ ...
Also, you write, that this sequence is necessary before normal initialization code, but i2c_wait4bus() is called from i2c_read() and i2c_write(), so the I2C bus is long ago initialized ... or what do you mean with "normal initialization code" ?
Also, whats with multimaster systems? Your code maybe cuts a running data transmission. The MPC8360ERM.pdf manual says "check the MBB bit, if the bus is free, before switching to master mode", thats what actual code did ... so, I want this only activated, if __M68K__ is defined ...
All true. This cannot go in as is. What you need is a I2C reset sequence as the above isn't enough in the general case. Michael, have a look at the kernel driver, it has some fixup code you could borrow.
Michael: Maybe you take a look in u-boot:board/keymile/common/common.c i2c_init_board(), there is a I2C reset sequence for 83xx based boards from keymile, which use this i2c driver.
Maybe its time to move this code in the i2c driver?
Will do ... David Wu emailed me a few comments as well that I include at the end. For now I agree that we should protect non ColdFire V2/V3 users with the __M68K__ definition
this code looks good except I do not see how the special case of multi-master systems you mentioned is handled.
I have no experience with multimaster systems, and I think, this special case is not yet factored in code.
I think for multi-master a timeout would have to be implemented to wait for a possible other master transfer to finish before initiating the abort sequence, or is this too much time spent during init?
Or, it could be detected? If BB Bit is set and the SCL pin doesn;t alter, the I2C bus must be blocked ... just an idea.
bye Heiko
David Wu's thoughts follow:
- agree to add a protection using #if defined(__M68K__) at some point this i2c_set_idle() has to be called when this I2C
master is enabled. In one master environment it can be in - i2c_set_bus_speed() where the I2C is enabled for that bus, or - i2c_init() or, - board specific routine - please suggest the right location.
You can use the CONFIG_SYS_I2C_INIT_BOARD define, and then define i2c_init_board() in board specific code. i2c_init_board() is called from i2c_init(), so there is no __M68K__ specific code necessary in this driver.
- since only the master can do a read and write call, when the bus is busy there is no need to wait until it is free which may never be free if we don't force a STOP.
Ok.
- In a multi-master environment, there should be an additional condition as to when one master can read/write, still i2c_wait4bus() may timeout.
Ok.
bye Heiko
participants (4)
-
Heiko Schocher -
Joakim Tjernlund -
Michael Durrant -
wolfgang＠leila.ping.de