On Tue, Jul 30, 2013 at 11:20:36PM +0200, Marek Vasut wrote: > Analyze and rework the PIO mode operation. The PIO mode operation > was unreliable on MX28, by analyzing the bus with LA, the checks > for when data were available or were to be sent were wrong. > > The PIO WRITE has to be completely reworked as it multiple problems. > The MX23 datasheet helped here, see comments in the code for details. > The problems boil down to: > - RUN bit in CTRL0 must be set after DATA register was written > - The PIO transfer must be 4 bytes long tops, otherwise use > clock stretching. > Both of these fixes are implemented. > > The PIO READ operation can only be done for up to four bytes as > we are unable to read out the data from the DATA register fast > enough. > > This patch also tries to document the investigation within the > code. > > Signed-off-by: Marek Vasut <marex@xxxxxxx> Thanks. I'd love to see Acked, Reviewed or Tested tags cause I know ppl have used these patches. Don't be shy :) > Cc: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxxxxxxxxx> > Cc: Christoph Baumann <cb@xxxxxxx> > Cc: Fabio Estevam <r49496@xxxxxxxxxxxxx> > Cc: Shawn Guo <shawn.guo@xxxxxxxxxx> > Cc: Torsten Fleischer <to-fleischer@xxxxxxxxxxx> > Cc: Wolfram Sang <wsa@xxxxxxxxxxxxx> > --- > drivers/i2c/busses/i2c-mxs.c | 263 ++++++++++++++++++++++++++---------------- > 1 file changed, 166 insertions(+), 97 deletions(-) > > V2: - Fix the data register shift computation algorithm and document that > some more. The original algo failed for short 1-byte writes as seen > in http://www.mail-archive.com/linux-i2c@xxxxxxxxxxxxxxx/msg12812.html > - Add me a copyright/authorship line, since according to git blame, I > have quite a lot of authored lines in the driver. Wolfram, I brought > this up some time ago already, but finally got to it. You're OK with > this, right? Sure. One question: Wouldn't it be more logical to have this patch first (fix pio) and then squash patches 1 and 3 as one on the top (add mx23 to now working pio)? I am not pushing too hard if this means a lot of work, but it sounds a bit more logical to me. > if (msg->flags & I2C_M_RD) { > + /* > + * PIO READ transfer: > + * > + * This transfer MUST be limited to 4 bytes maximum. It is not > + * possible to transfer more than four bytes via PIO, since we > + * can not in any way make sure we can read the data from the > + * DATA register fast enough. Besides, the RX FIFO is only four > + * bytes deep, thus we can only really read up to four bytes at > + * time. Finally, there is no bit indicating us that new data > + * arrived at the FIFO and can thus be fetched from the DATA > + * register. > + */ > + BUG_ON(msg->len > 4); How could this happen? There is a check in mxs_i2c_xfer_msg.
Attachment:
signature.asc
Description: Digital signature
