Re: [PATCH] make the registers of the stv0297 visible for other applications (e.g. i2cdump)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

_______________________________________________
linux-dvb mailing list
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

[Index of Archives]     [Linux Media]     [Video 4 Linux]     [Asterisk]     [Samba]     [Xorg]     [Xfree86]     [Linux USB]

  Powered by Linux