* Re: [PATCH] si2168: use i2c controlled mux interface [not found] <1452058920-9797-1-git-send-email-crope@iki.fi> @ 2016-03-23 16:58 ` Peter Rosin 2016-04-05 14:50 ` Antti Palosaari 0 siblings, 1 reply; 3+ messages in thread From: Peter Rosin @ 2016-03-23 16:58 UTC (permalink / raw) To: Antti Palosaari, linux-media; +Cc: Peter Rosin, linux-i2c On 2016-01-06 06:42, Antti Palosaari wrote: > Recent i2c mux locking update offers support for i2c controlled i2c > muxes. Use it and get the rid of homemade hackish i2c adapter > locking code. [actual patch elided] I had a 2nd look and it seems that the saa7164 driver has support for a HVR2055 card with dual si2168 chips. These two chips appear to sit on the same i2c-bus with different i2c-addresses (0x64 and 0x66) and with gates (implemented as muxes) to two identical tuners with the same i2c-address (0x60). Do I read it right? With the current i2c-mux-locking (parent-locked muxes), this works fine as an access to one of the tuners locks the root i2c adapter and thus the other tuner is also locked out. But with the upcoming i2c-mux-locking for i2c-controlled muxes (self-locked muxes), the root i2c adapter would no longer be locked for the full transaction when one of the tuners is accessed. This means that accesses to the two tuners may interleave and cause all kinds of trouble, should both gates be open at the same time. So, is it really correct and safe to change the si2168 driver to use a self-locked mux? Unless there is some other mechanism that prevents the two tuners from being accessed in parallel, I think not. But maybe there is such a mechanism? Cheers, Peter ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] si2168: use i2c controlled mux interface 2016-03-23 16:58 ` [PATCH] si2168: use i2c controlled mux interface Peter Rosin @ 2016-04-05 14:50 ` Antti Palosaari 2016-04-05 15:38 ` Peter Rosin 0 siblings, 1 reply; 3+ messages in thread From: Antti Palosaari @ 2016-04-05 14:50 UTC (permalink / raw) To: Peter Rosin, linux-media; +Cc: Peter Rosin, linux-i2c On 03/23/2016 06:58 PM, Peter Rosin wrote: > On 2016-01-06 06:42, Antti Palosaari wrote: >> Recent i2c mux locking update offers support for i2c controlled i2c >> muxes. Use it and get the rid of homemade hackish i2c adapter >> locking code. > > [actual patch elided] > > I had a 2nd look and it seems that the saa7164 driver has support for > a HVR2055 card with dual si2168 chips. These two chips appear to sit > on the same i2c-bus with different i2c-addresses (0x64 and 0x66) and > with gates (implemented as muxes) to two identical tuners with the > same i2c-address (0x60). Do I read it right? saa7164 has 3 different I2C adapters. saa7164 I2C bus #0: * eeprom * Si2157 #1 saa7164 I2C bus #1: * Si2157 #2 saa7164 I2C bus #2: * Si2168 #1 * Si2168 #2 So both of the Si2157 tuners could have same addresses. (It is Hauppauge WinTV-HVR2205, not HVR-2055). > With the current i2c-mux-locking (parent-locked muxes), this works > fine as an access to one of the tuners locks the root i2c adapter > and thus the other tuner is also locked out. But with the upcoming > i2c-mux-locking for i2c-controlled muxes (self-locked muxes), the > root i2c adapter would no longer be locked for the full transaction > when one of the tuners is accessed. This means that accesses to the > two tuners may interleave and cause all kinds of trouble, should > both gates be open at the same time. So, is it really correct and > safe to change the si2168 driver to use a self-locked mux? > > Unless there is some other mechanism that prevents the two tuners > from being accessed in parallel, I think not. But maybe there is such > a mechanism? Good point. Actually there is pretty often this kind of configuration used for those dual tuner devices and it will cause problems... Currently all of those implements hackish i2c_gate_ctrl() callback to switch mux. ____________ ____________ ____________ |I2C-adapter | | I2C-mux | | I2C-client | |------------| |------------| |------------| | | | addr 0x1c | | addr 0x60 | | | | | | | | |-+-I2C---|-----/ -----|---I2C---| | |____________| | |____________| |____________| | ____________ ____________ | | I2C-mux | | I2C-client | | |------------| |------------| | | addr 0x1d | | addr 0x60 | | | | | | +-I2C---|-----/ -----|---I2C---| | |____________| |____________| regards Antti -- http://palosaari.fi/ ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] si2168: use i2c controlled mux interface 2016-04-05 14:50 ` Antti Palosaari @ 2016-04-05 15:38 ` Peter Rosin 0 siblings, 0 replies; 3+ messages in thread From: Peter Rosin @ 2016-04-05 15:38 UTC (permalink / raw) To: Antti Palosaari, linux-media; +Cc: Peter Rosin, linux-i2c On 2016-04-05 16:50, Antti Palosaari wrote: > On 03/23/2016 06:58 PM, Peter Rosin wrote: >> On 2016-01-06 06:42, Antti Palosaari wrote: >>> Recent i2c mux locking update offers support for i2c controlled i2c >>> muxes. Use it and get the rid of homemade hackish i2c adapter >>> locking code. >> >> [actual patch elided] >> >> I had a 2nd look and it seems that the saa7164 driver has support for >> a HVR2055 card with dual si2168 chips. These two chips appear to sit >> on the same i2c-bus with different i2c-addresses (0x64 and 0x66) and >> with gates (implemented as muxes) to two identical tuners with the >> same i2c-address (0x60). Do I read it right? > > saa7164 has 3 different I2C adapters. > > saa7164 I2C bus #0: > * eeprom > * Si2157 #1 > > saa7164 I2C bus #1: > * Si2157 #2 > > saa7164 I2C bus #2: > * Si2168 #1 > * Si2168 #2 > > So both of the Si2157 tuners could have same addresses. > > (It is Hauppauge WinTV-HVR2205, not HVR-2055). Ok, so I did read it right. >> With the current i2c-mux-locking (parent-locked muxes), this works >> fine as an access to one of the tuners locks the root i2c adapter >> and thus the other tuner is also locked out. But with the upcoming >> i2c-mux-locking for i2c-controlled muxes (self-locked muxes), the >> root i2c adapter would no longer be locked for the full transaction >> when one of the tuners is accessed. This means that accesses to the >> two tuners may interleave and cause all kinds of trouble, should >> both gates be open at the same time. So, is it really correct and >> safe to change the si2168 driver to use a self-locked mux? >> >> Unless there is some other mechanism that prevents the two tuners >> from being accessed in parallel, I think not. But maybe there is such >> a mechanism? > > Good point. Actually there is pretty often this kind of configuration used for those dual tuner devices and it will cause problems... Currently all of those implements hackish i2c_gate_ctrl() callback to switch mux. > > ____________ ____________ ____________ > |I2C-adapter | | I2C-mux | | I2C-client | > |------------| |------------| |------------| > | | | addr 0x1c | | addr 0x60 | > | | | | | | > | |-+-I2C---|-----/ -----|---I2C---| | > |____________| | |____________| |____________| > | ____________ ____________ > | | I2C-mux | | I2C-client | > | |------------| |------------| > | | addr 0x1d | | addr 0x60 | > | | | | | > +-I2C---|-----/ -----|---I2C---| | > |____________| |____________| Right, so self-locked muxes (as in v5 [1]) can't do the job, but mux-locked muxes (as in v6 [2]) should be fine. The v6 mux- locked muxes grab a new mux_lock in the parent adapter where the v5 self-locked muxes grabbed a lock in the mux itself. Which means that sibling muxes will lock each other out with v6 when they go for the same lock, which is what we want. Cheers, Peter (v5 and v6 refer to the mux locking update patch series) [1] http://marc.info/?l=linux-i2c&m=145805103325611&w=2 [2] http://marc.info/?l=linux-i2c&m=145967417924732&w=2 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-04-05 15:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1452058920-9797-1-git-send-email-crope@iki.fi>
2016-03-23 16:58 ` [PATCH] si2168: use i2c controlled mux interface Peter Rosin
2016-04-05 14:50 ` Antti Palosaari
2016-04-05 15:38 ` Peter Rosin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).