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

[Ben Dooks]

On Tue, Sep 19, 2006 at 11:01:26PM +0200, Jean Delvare wrote:
> > [ Patch is Repost to correct list ]
> > 
> > Fix the write to certain types of device connected
> > to the S3C2410 or compatible i2c controller by
> > adding a small delay (by accessing i2c registers)
> > to the write path.
> > 
> > Signed-off-by: Ben Dooks <ben-linux at fluff.org>
> 
> > diff -urpN -X ../dontdiff linux-2.6.17-git9-bjd1/drivers/i2c/busses/i2c-s3c2410.c linux-2.6.17-git9/drivers/i2c/busses/i2c-s3c2410.c
> > --- linux-2.6.17-git9-bjd1/drivers/i2c/busses/i2c-s3c2410.c	2006-06-18 02:49:35.000000000 +0100
> > +++ linux-2.6.17-git9/drivers/i2c/busses/i2c-s3c2410.c	2006-06-26 23:05:56.000000000 +0100
> > @@ -269,6 +269,7 @@ static int i2s_s3c_irq_nextbyte(struct s
> >  	unsigned long tmp;
> >  	unsigned char byte;
> >  	int ret = 0;
> > +	int i;
> >  
> >  	switch (i2c->state) {
> >  
> > @@ -325,6 +326,14 @@ static int i2s_s3c_irq_nextbyte(struct s
> >  			byte = i2c->msg->buf[i2c->msg_ptr++];
> >  			writeb(byte, i2c->regs + S3C2410_IICDS);
> >  			
> > +			/* 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.

-- 
Ben (ben at fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'