Dear Wolfram Sang, > 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.htm > > l > > > > - 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. I would prefer to keep Juergens' patch separate from mine if you don't mind to be clear on the authorship. > > 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. It cannot happen, I added the check here to make sure when someone becomes adventurous enough to start messing with these constants, it will stop him early enough. Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
