[i2c] i2c-nforce patch adding a check for read/writes of >=31 bytes

Jean Delvare khali at linux-fr.org
Mon May 14 17:50:05 CEST 2007


Hi Oleg,

On Tue, 08 May 2007 19:13:17 -0700, Oleg Ryjkov wrote:
> Hello Hans, hello list,
> 
> I'm currently working on an issue with the i2c-nforce2 driver, which 
> happens when a block read/write is performed on a non-block register( up 
> until the patch of i2c-nforce.c dated 2006-12-10 it supported the block 
> reads - which is the version I'm using).
> However, if a block read was done on, say a byte register, the 
> controller would halt and the only way I could bring it back up was to 
> reboot the host.

Note that you are simply not supposed to do that in the first place.
SMBus block transactions should only be run on "registers" which expect
them.

> I've came across the following patch:
> 
> http://lists.lm-sensors.org/pipermail/i2c/2006-October/000356.html
> 
> which seems to be working around this issue. However it adds a 
> constraint that the max number of bytes to transfer is 31.
> So my question is, why is it 31? I've tried applying the patch and then 
> modifying it to raise this limit to 32 bytes and it seems to work fine 
> on the MCP51 host controller . (And since we are resending the command 
> following the length check, all of the 32 data registers are available 
> to us).

Hans-Frieder and myself found that SMBus block transactions with length
32 would lock the SMBus master. I seem to remember we observed it on
both nForce2 and nForce4 chips, and that greater values were OK, just
multiples of 32 were not - which suggested that the value was trimmed
to 5 bits somewhere. We were looking for a workaround back then, and
the only thing we could come up with was to check the size and reject
block transactions with length 32.

Now, if you say that the MCP51 (and presumably next chips) doesn't have
the problem, this might mean that nVidia fixed the hardware bug, which
would be good news. Too bad they don't publish datasheets for their
chips, that would obviously help.

If you want to provide a patch implementing SMBus block transactions
for the MCP51 and later chips, this is welcome.

> Additionally, what were the reasons not to include the patch in the kernel?

Just read the following posts in the thread for the reason why it was
rejected:
http://lists.lm-sensors.org/pipermail/i2c/2006-October/000358.html

On top of that, the fact is that there are almost no users of SMBus
block transactions in the kernel anyway, so we really had no reason to
bother. Do you need these yourself?

-- 
Jean Delvare



More information about the i2c mailing list