Hi Ben,
> > It just happened to me that this test is bogus. len == 0 && rw ==
> > I2C_SMBUS_READ is perfectly valid according to the SMBus specification.
> > It's a "quick command" with data (1 bit) == 1. I never saw it used in
> > practice, and everyone seems to agree that this is a weirdness in SMBus'
> > specification (using the read/write bit as data in this specific case)
> > but it's still valid.
>
> It looks like the case for state_quick won't work for 0-len reads.
> It should unconditionally jump to state_write for a 0-len state_quick operation.
> I'll write up a separate patch to fix that and remove the check tomorrow.
Oops, you may be right. My comment was solely based on my knowledge of
the SMBus specification. I did not actually check if the code would
handle it properly.
This state machine looks rather strange to me, I don't think I ever saw
an i2c bus driver implemented that way.
> > > if (len && !buffer) {
> > > - dev_warn(&adapter->dev, "nonzero length but no buffer\n");
> > > + dev_dbg(&adapter->dev, "nonzero length but no buffer\n");
> > > return -EFAULT;
> > > }
> >
> > I think that the whole block should be enclosed by #ifdef DEBUG/#endif,
> > as this error condition is simply not supposed to happen.
>
> I assume other drivers don't check for this... why not just delete it?
Sure, that's fine with me too.
--
Jean Delvare
