On 12-02-20 10:29 AM, Jean Delvare wrote:
Hi Doug,
Sorry for the late reply, I wanted to make sure I remembered all the
I2C_M_RECV_LEN logic before replying.
On Sat, 04 Feb 2012 16:42:29 -0500, Douglas Gilbert wrote:
The I2C_M_RECV_LEN flag indicates that the length of
an I2C response is in the first byte read. So the maximum
size of the read buffer using this option is 256 bytes.
Currently the i2c-dev driver returns EINVAL when an
attempt is made to use ioctl(I2C_RDWR) with the
I2C_M_RECV_LEN flag set. That is overly paranoid:
No, this is playing it safe, in the absence of use case and complete
review of all involved code paths.
ChangeLog:
- allow I2C_M_RECV_LEN flag in the i2c-dev driver as
long as the associated buffer length can cope with
the worst case size (which is 256 bytes).
This means that you expect user-space to provide a 256 byte message
buffer when passing the I2C_M_RECV_LEN flag. Underlying bus drivers
OTOH expect len to be set to 1 when I2C_M_RECV_LEN is used (and they
add the received block length to that to come up with the actual used
length.) I don't think this is documented formally anywhere, but reading
the code of function i2c_smbus_xfer_emulated() in i2c-core will show you
the calling conventions and expectations. Function readbytes() in
i2c-algo-bit is also worth reading. So your patch is not correct.
To be honest, I think I recall being the one designing things that way
but I can't remember why I did so. Some git and mailing list digging
might be needed. Might be related to the support of SMBus PEC but I'm
not sure.
Unfortunately there is only i2c_msg.len available to pass length
information, so it isn't possible to dissociate the buffer size from
the used size. It happens to be the same in most cases so it has never
been a problem in practice. Only for the I2C_M_RECV_LEN case it would
be useful to distinguish between both, and presumably SMBus PEC too.
It has never been a problem so far because only the SMBus layer is
using I2C_M_RECV_LEN and PEC, and there we know that the block size
cannot exceed I2C_SMBUS_BLOCK_MAX == 32. Every bus driver can (and
should) enforce that, and buffers are always large enough to contain 32
bytes, by design.
Passing I2C_M_RECV_LEN at the I2C (not SMBus) level wouldn't work
safely, not even in the kernel. The only non-SMBus implementation is in
i2c-algo-bit as far as I know, and it enforces the I2C_SMBUS_BLOCK_MAX
limit. So it should be pretty clear that flag I2C_M_RECV_LEN was
introduced for and designed with SMBus in mind. Using it for I2C
messaging just doesn't work, thus the ban in i2c-dev.
This makes me wonder how you did test your patch, as I can't see how it
would work with the upstream driver code. But more importantly, can you
please explain what you are trying to achieve in the first place?
Receiving block length as the first data byte is an SMBus thing,
traditionally non-SMBus devices don't do that. And for SMBus devices
through i2c-dev, you'd be using ioctl I2C_SMBUS not I2C_RDWR.
If you have a legitimate use case for I2C_M_RECV_LEN, then we can
discuss it, but it will take a lot more than your 2-line patch to get
it right.
In the embedded space I only see I2C (TWI) so I don't understand
the fixation with SMBus (some subset I believe).
My illegitimate use case is:
Sonmicro 13.56 MHz RFID Mifare Module:
http://www.sonmicro.com/en/downloads/Mifare/ds_SM130.pdf
http://www.sonmicro.com/en/downloads/Mifare/AN601.pdf
I can make it work by requesting the maximum number of bytes it
will ever respond with on all reads.
Some suggestions for when and if the I2C pass-through is rewritten:
- make it clean to the user space, don't use it for internal
plumbing within the kernel (to avoid horrors like those you
allude to above)
- put some version number in it so when you want to put some
extra fields through it (e.g. extra i2c_msg.len field) you
can bump version number ***
- the multiple I2C transfers in one structure is great, but
would be more useful if a delay could be placed between
each one.
*** Getting new ioctls into the kernel is really difficult since
the management seems to think pass-throughs subvert the OS
(they do indeed) and where absolutely necessary sysfs can be
used for the purpose.
Doug Gilbert
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
