RE: [i2c] i2c-mpc.c driver issues

["Joakim Tjernlund" <joakim.tjernlund@transmode.se>]

> -----Original Message-----
> From: 
> linuxppc-dev-bounces+joakim.tjernlund=transmode.se@ozlabs.org 
> [mailto:linuxppc-dev-bounces+joakim.tjernlund=transmode.se@ozl
abs.org] On Behalf Of Jean Delvare
> Sent: den 26 oktober 2007 11:53
> To: Tjernlund
> Cc: linuxppc-dev@ozlabs.org; i2c@lm-sensors.org
> Subject: Re: [i2c] i2c-mpc.c driver issues
> 
> Hi Jocke,
> 
> On Wed, 24 Oct 2007 23:06:13 +0200, Tjernlund wrote:
> > While browsing the i2c-mpc.c driver I noticed some things 
> that look odd
> > to me so I figured I report them. Could not find a 
> maintainer in the MAINTANERS file
> > so I sent here, cc:ed linuxppc-dev as well.
> > 
> > 1) There are a lot of return -1 error code that is 
> propagated back to
> >    userspace. Should be changed to proper -Exxx codes.
> 
> This is true of many Linux i2c bus drivers, unfortunately. 
> While nothing
> actually prevents drivers from returning -1 to userspace on error,
> meaningful error codes would of course be preferred.
> 
> > 2) mpc_read(), according to the comment below it sends a 
> STOP condition here but
> >    this function does not known if this is the last read or 
> not. mpc_xfer is
> >    the one that knows when the transaction is over and 
> should send the stop, which it already
> >    does.
> > 
> >  /* Generate stop on last byte */
> >   if (i == length - 1)
> >        writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_TXAK);
> 
> Probably correct, although I am not familiar with this specific
> hardware. I guess that the same is true of mpc_write as well, which is
> even worse because write + read combined transactions are very common
> (while read + write are not.)
> 
> I'm not completely sure that mpc_xfer sends the stop. mpc_i2c_stop
> doesn't seem to do much.

Don't have the manual handy, but there is something bothering me with
the read function. After reading the last char there is nothing that
wait for the STOP to complete, instead one just exits and call
mpc_i2c_stop(). It might be so that the i2c_wait() won't complete until
the STOP has been sent, but I would not bet on it.

 Jocke