[i2c] i2c-mpc.c driver issues

Joakim Tjernlund joakim.tjernlund at transmode.se
Fri Oct 26 21:21:33 EST 2007


On Fri, 2007-10-26 at 11:53 +0200, Jean Delvare wrote:
> 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.)

Don't think write is a problem, only read. I would have to look at the
HW spec to make sure though.

> 
> I'm not completely sure that mpc_xfer sends the stop. mpc_i2c_stop
> doesn't seem to do much.
> 
> Now that you've identified these bugs, what about sending patches
> to fix them?

Normally I would do that, but I am too busy with other things. Thats
why I only reported this as I know I won't have time to fix it for some
time.

 Jocke



More information about the Linuxppc-dev mailing list