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

[Jean Delvare <khali@linux-fr.org>]

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.

Now that you've identified these bugs, what about sending patches
to fix them?

-- 
Jean Delvare