Manu Abraham wrote:
> Trent Piepho wrote:
>> On Sat, 19 May 2007, e9hack wrote:
>>> Trent Piepho wrote:
>>>> On Fri, 18 May 2007, Johannes Stezenbach wrote:
>>>>> Or you could propose a change to i2c-core to add
>>>>> a I2C_M_STOP flag (analogous to I2C_M_NOSTART), which
>>>>> then would have to be implemented by all i2c bus drivers.
>>>> It seems like this is the only way to send multiple stops in a single
>>>> atomic transaction. All drivers wouldn't have to implement it, just the
>>>> ones where someone actually wants this ability. Few drivers actually
>>>> implement all of the existing i2c features.
>>>>
>>> It is a big difference between the I2C_M_STOP and the I2C_M_NOSTART flag. I2C_M_NOSTART is a nice feature, which a
>>> master may implement. If I look through the different implementations, it isn't defined exactly, what it means.
>>> I2C_M_STOP is a must have for the master, if a slave is connected to the bus like the stv0297.
>> It doesn't seem any different than I2C_M_TEN, I2C_M_NO_RD_ACK, etc. in that a
>> slave that needs one of these features won't work with a master (i2c_adapter
>> in Linux I2C terms) that doesn't implement them. There are plenty of i2c
>> adapters that don't even support messages longer than two bytes or
>> transactions with more than one or two parts.
>>
>>> It isn't necessary, that I2C_M_STOP is implemented by the master itself. It is possible to implement it in i2ctransfer.
>>> If the flag is set, each entry of the message array is send as a single request to the underlaying transfer function of
>> That's a good point, i2c_tranfer() is basically:
>>
>> mutex_lock(i2c_adapter->mutex);
>> i2c_adapter->master_xfer(...);
>> mutex_unlock(i2c_adapter->mutex);
>>
>> The problem with stv0297 is that one wants multiple calls to master_xfer()
>> inside the mutex protected region. It would be easy to have this code break a
>> series of i2c messages into multiple calls to master_xfer() with a I2C_M_STOP
>> flag. Then all i2c_adapters could support I2C_M_STOP without needing to be
>> changed. One could also support the flag inside an i2c_adapter driver, but
>> there doesn't seem to be much point
>
>
> Don't you think that the STV0297 (the obvious case that i looked at was
> b2c2 flexcop) B2C2 cablestar.
> In this case looking at flexcop-i2c.c flexcop_i2c_read4(foobar)
>
> if ((ret = flexcop_i2c_operation(fc, &r100)) != 0) {
> /* The cablestar needs a different kind of i2c_transfer (does not
> * support "Repeat Start");
> * wait for the ACK failure,
> * and do a subsequent read with the Bit 30 enabled
> */
>
> ..
>
> Now, to me it looks a rather bit strange: the reason being the transfer
> here just needs a stop bit in between which exactly is a very normal
> transaction with START/STOP bits, just that the whole transaction is
> looped for 4 bytes.
>
> This mode is exactly a BYTE mode which is a loop for 4 bytes and not a
> PAGE mode transfer, what we normally have under Linux.
>
> So what i think would be better, probably would be like this:
>
> * pass a flag to master_xfer to define the mode of operation say a flag
> "PAGE_MODE" or "BYTE_MODE" [1] Page 9
> now master_xfer looks for the FOO_MODE flag and decides what transfer to
> be done.
> eg:
>
> master_xfer(... FOO_MODE)
> {
> lock()
>
> if (FOO_MODE ) {
> /* The I2C bridge does not issue a STOP bit in between
> * most devices have an internal Address sequencer, which
> * handles the addresses
> */
> if (I2C_M_RD)
> page_mode_read(length)
> else
> page_mode_write(length)
> } else {
> /* In this case the STOP bit needs to transmitted for each byte
> * and the slave device needs the sub address on each transaction
> * These devices are simpler and do not have an internal Address sequencer
> */
> if (I2C_M_RD)
> byte_mode_read(length)
> else
> byte_mode_write(length)
> }
>
> unlock()
> }
>
> Don't you think, that this is a better way of doing it according to the
> specs ? I see this mode of operation on many I2C bridges, (B2C2, Mantis,
> Hopper, 878, .. the ones that i looked at a quick glance. In fact i had
> stumbled over this issue long back on the Mantis, when i pointed out to
> Johannes, but he was of the opinion that we shouldn't mess up with Linux
> I2C. At that point it was perfectly fine since we didn't have any such
> issues.) but unfortunately Linux I2C just does pagemode only, which
> brings us into a pseudo problem of a STOP bit, which exactly isn't an
> issue due to a "crappy" hardware design, but just that Linux i2C chose
> not to implement certain features of I2C bridges, AFAICS.
>
> [1] http://www.tranzistoare.ro/datasheets/150/501070_DS.pdf
>
I think, there are some mistakes in the mode description. What you mean with 'byte mode' is a 'current address read' for
a single byte. 'Byte mode' doesn't exist for write requests, because it isn't possible to have a STOP/START after the
sub address byte. 'Page mode' is a 'write' or a 'random address read' for multiple bytes. The 'random address read'
needs a repeated START condition between the sub address write and the data read, because the r/w mode for the device
must be change. Usually, EEPROM's do not implement a 'multi byte write'. They have a 'page write'. A write request must
not touch a page boundary (2, 4, 8 or 16 bytes). If a page boundary is touch, the bytes are written to the beginning of
the current page. Only the address counter within the current page is incremented. The mode description and the code
sample have nothing to do with the stv0297 problem. The stv0297 doesn't implement the 'random address read'. This is the
reason, why a read request is split in a single write request for the sub address and a single 'current address read'
for some bytes.
- Hartmut
_______________________________________________
linux-dvb mailing list
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
