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 _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev