topic Re: mx28 I2C support for RESTART operation in i.MX Processors

mx28 I2C support for RESTART operation

Matt_ — Tue, 11 Dec 2012 15:21:08 GMT

Some I2C peripherals require a RESTART sequence to read registers, e.g.

START, I2C address+WR, write register number, RESTART, I2C address+RD, read register value, STOP

Previous Freescale I2C drivers handled this OK but the mx28 driver linux-2.6.35.3/drivers/i2c/busses/i2c-mxs.c has a couple of bugs that prevent this from working.

In DMA mode, the function hw_i2c_dma_setup_write has line desc[2]->cmd.pio_words[0] |= BM_I2C_CTRL0_POST_SEND_STOP; which forces a STOP operation on the bus instead of allowing a possible RESTART.

This line can be safely deleted as the very next line ORs in "flags" which will assert STOP only when necessary.

In PIOQUEUE mode as it stands the I2C operation times out as the isr never sees the bus go idle after the I2C write sequence - we are correctly holding the bus for a RESTART. One way to make it work is to queue up both the I2C write and I2C read commands before looking for command completion. This way both commands run and the isr correctly completes after the read portion.
i.e. in mxs_i2c_xfer_msg

hw_i2c_pioq_setup_write(dev,

msg->addr,

msg->buf, msg->len, flags);

hw_i2c_pioq_run(dev);

---becomes---

hw_i2c_pioq_setup_write(dev,

msg->addr,

msg->buf, msg->len, flags);

if (!stop)

return dev->cmd_err;

hw_i2c_pioq_run(dev);

In keeping with the rest of the file there is no error checking on FIFO space available.

It would also be nice to be able to specify the I2C clock speed in the platform data and have the module calculate values for the TIMING registers. At present I've just hard coded these into the i2c-mxs.c file and apply them anytime the I2C module is reset.

Hope this helps some-one.

Matt.

Re: mx28 I2C support for RESTART operation

harryflink — Tue, 11 Dec 2012 21:17:37 GMT

Matthew, thank you for your tip!

I have searched for fixes to "i2c-mxs.c" bugs for days by now and hope that your suggestion helps.

We have problems that when using the I2C in DMA-mode it causes the kernel crash because DMA-code assumes only one scatter-gather DMA-task can be completed between the DMA-interrupts. Sometimes two tasks are completed and the driver crashes. That happens with touch screen controller at the moment.

In PIO-mode some I2C devices works OK (e.g.touch screen and audio-chip) but when user space application tries to access real-time clock chip using the I2C user space interface, the I2C driver (i2c-mxc.c) returns ETIMEDOUT error which means that the "Connection Timed Out".

I hope your suggestion fixes the timeouts in the I2C driver and we get the devices working.

Re: mx28 I2C support for RESTART operation

harryflink — Tue, 11 Dec 2012 21:41:15 GMT

I tried the Matthew's patch and the I2C driver works better but eventually it will timeout after a while. I get kernel error message "mxs_i2c_xfer_msg: Timeout!" and after this reading the I2C touch screen doesn't work anymore.

Anyone else have any idea how to get "i2c-mxs.c" working in PIOQUEUE mode without timeouts? At this time i.MX28 CPU's I2C bus is totally unusable with Kernel 2.6.35.

Re: mx28 I2C support for RESTART operation

Matt_ — Tue, 11 Dec 2012 22:24:49 GMT

Sorry it didn't work for you.

We have a very simple periperal on the I2C and never send/receive more than a byte (after the address) and I guess for a touch screen you're probably streaming quite a few bytes.

I wonder whether you are getting FIFO issues?

The code for PIOQUEUE really needs to be rewritten using the QUEUE_IRQs to properly unload the FIFOs as they fill.

Good luck

Re: mx28 I2C support for RESTART operation

samuelsalas — Fri, 14 Dec 2012 17:29:13 GMT

Have you tried this :

Re: i.MX28 Linux BSP I2C source codes bug

I have a problem connecting 2 seperate i2c on imx283, so I'd like to have some tips if any... Here my 2 issues :

I2C frequency on 2 buses

Conflict on 2 I2C buses

Sam.

Re: mx28 I2C support for RESTART operation

Matt_ — Mon, 17 Dec 2012 10:59:17 GMT

Hi Sam,

I'm not sure the ERESTARTSYS return value is relevent to my RESTART issue.

On the idea of seperate frequencies for each bus I would suggest setting flags in device.c along similar lines to the pioqueue_mode flag and making i2c-mxs.c change the timing accordingly after any i2c resets.

Now I'd like to point out that I'm not a s/w engineer, I'm just a h/w engineer you has to fix things in s/w now and again so the following might be rubbish.....

At the top of i2c-mxs.c some static variables are declared which might mean that two instances of I2C would share the same data. I think this is dangerous and that the variables should be contained in within each instance of I2c, e.g. in the mxs_i2c_dev structure.

I think some-one who knows what they're doing needs to fix this driver!

Matt.