On Thu, Oct 25, 2012 at 12:00:48PM +0300, Felipe Balbi wrote: > if we allow compiler reorder our writes, we could > fall into a situation where dev->buf_len is reset > for no apparent reason. > > This bug was found with a simple script which would > transfer data to an i2c client from 1 to 1024 bytes > (a simple for loop), when we got to transfer sizes > bigger than the fifo size, dev->buf_len was reset > to zero before we had an oportunity to handle XDR > Interrupt. Because dev->buf_len was zero, we entered > omap_i2c_transmit_data() to transfer zero bytes, > which would mean we would just silently exit > omap_i2c_transmit_data() without actually writing > anything to DATA register. That would cause XDR > IRQ to trigger forever and we would never transfer > the remaining bytes. > > After adding the memory barrier, we also drop resetting > dev->buf_len to zero in omap_i2c_xfer_msg() because > both omap_i2c_transmit_data() and omap_i2c_receive_data() > will act until dev->buf_len reaches zero, rendering the > other write in omap_i2c_xfer_msg() redundant. > > This patch has been tested with pandaboard for a few > iterations of the script mentioned above. > > Signed-off-by: Felipe Balbi <balbi@xxxxxx> > --- > > This bug has been there forever, but it's quite annoying. > I think it deserves being pushed upstream during this -rc > cycle, but if Wolfram decides to wait until v3.8, I don't > mind. I would add this into 3.7, but what about the comments suggesting to use barrier()? -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
Attachment:
signature.asc
Description: Digital signature
