[i2c] [PATCH] s3c24xx: fix writes to certain devices

[Jean Delvare]

Hi Ben,

Sorry for the delay.

On Wed, 27 Sep 2006 07:08:21 +0100, Ben Dooks wrote:
> On Tue, Sep 19, 2006 at 11:01:26PM +0200, Jean Delvare wrote:
> > > +			/* sometimes write operations fail if the write is
> > > +			 * let go too quickly, slow down the proceedings
> > > +			 * slightly.
> > > +			 */
> > > +
> > 
> > No blank line here please,
> > 
> > > +			for (i = 0; i < 6; i++) 
> > > +				tmp += readl(i2c->regs + S3C2410_IICSTAT);
> > > +
> > 
> > nor here.
> > 
> > >  		} else if (!is_lastmsg(i2c)) {
> > >  			/* we need to go to the next i2c message */
> > >  
> > > 
> > 
> > What a ugly hack :( Is there really no cleaner way to fix it? No way to
> > find out when exactly this "sometimes" occurs? Performance will suffer
> > with these unconditional 6 more I/O reads.
> 
> No, without it, on some devices, the I2C simply stalls on every
> write. The bus the i2c controller is on is running at 66MHz, so
> is not exactly slow, especially as we've executed several 100
> instructions per-i2c byte moved just going in and out of the ISR.
>  
> > Where does this arbitrary 6 come from?
> 
> expermimentation
> 
> > Why do you add the read value to tmp?
> 
> An attempt to stop the compiler trying to optimise
> this out of the code.

Reading from hardware registers could have any kind of side effect the
compiler doesn't know about, so I would be very surprised if the
compiler would attempt to "optimize" such instructions. Please try
without the tmp trick, and see if it works. If it really doesn't, I
would consider it a compiler bug, which should be reported.

Thanks,
-- 
Jean Delvare