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 <davi...@arcturusnetworks.com> > >>> Signed-off-by: Michael Durrant <mdurr...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot