Re: recursive locking problem
On Wed, Sep 14, 2011 at 04:03:58AM +0300, Antti Palosaari wrote: On 09/09/2011 02:46 PM, Daniel Glöckner wrote: On Thu, Sep 08, 2011 at 07:34:32PM +0300, Antti Palosaari wrote: I am working with AF9015 I2C-adapter lock. I need lock I2C-bus since there is two tuners having same I2C address on same bus, demod I2C gate is used to select correct tuner. Would it be possible to use the i2c-mux framework to handle this? Each tuner will then have its own i2c bus. Interesting idea, but it didn't worked. It deadlocks. I think it locks since I2C-mux is controlled by I2C switch in same I2C bus, not GPIO or some other HW. Take a look at drivers/i2c/muxes/pca954x.c. You need to use parent-algo-master_xfer/smbus_xfer directly as the lock that protects you from having both gates open is the lock of the root i2c bus. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: recursive locking problem
On 09/14/2011 09:19 AM, Daniel Glöckner wrote: On Wed, Sep 14, 2011 at 04:03:58AM +0300, Antti Palosaari wrote: On 09/09/2011 02:46 PM, Daniel Glöckner wrote: On Thu, Sep 08, 2011 at 07:34:32PM +0300, Antti Palosaari wrote: I am working with AF9015 I2C-adapter lock. I need lock I2C-bus since there is two tuners having same I2C address on same bus, demod I2C gate is used to select correct tuner. Would it be possible to use the i2c-mux framework to handle this? Each tuner will then have its own i2c bus. Interesting idea, but it didn't worked. It deadlocks. I think it locks since I2C-mux is controlled by I2C switch in same I2C bus, not GPIO or some other HW. Take a look at drivers/i2c/muxes/pca954x.c. You need to use parent-algo-master_xfer/smbus_xfer directly as the lock that protects you from having both gates open is the lock of the root i2c bus. Ah yes, rather similar case. I see it as commented in pca954x.c: /* Write to mux register. Don't use i2c_transfer()/i2c_smbus_xfer() for this as they will try to lock adapter a second time */ This looks even more hackish solution than calling existing demod .i2c_gate_ctrl() callback from USB-interface driver. But yes, it must work - not beautiful but workable workaround. regards Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: recursive locking problem
On Wed, Sep 14, 2011 at 01:45:47PM +0300, Antti Palosaari wrote: Interesting idea, but it didn't worked. It deadlocks. I think it locks since I2C-mux is controlled by I2C switch in same I2C bus, not GPIO or some other HW. Take a look at drivers/i2c/muxes/pca954x.c. You need to use parent-algo-master_xfer/smbus_xfer directly as the lock that protects you from having both gates open is the lock of the root i2c bus. Ah yes, rather similar case. I see it as commented in pca954x.c: /* Write to mux register. Don't use i2c_transfer()/i2c_smbus_xfer() for this as they will try to lock adapter a second time */ This looks even more hackish solution than calling existing demod .i2c_gate_ctrl() callback from USB-interface driver. But yes, it must work - not beautiful but workable workaround. This is not a hack. This is the official way to do it. The I2C mux framework was designed to allow multiplexers controlled through the same I2C bus. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: recursive locking problem
On 09/09/2011 01:45 PM, David Waring wrote:
On 08/09/11 17:34, Antti Palosaari wrote:
[snip]
Is there any lock can do recursive locking but unlock frees all locks?
Like that:
gate_open
+gate_open
+gate_close
== lock is free
AFAIK mutex can do only simple lock() + unlock(). Semaphore can do
recursive locking, like lock() + lock() + unlock() + unlock(). But how I
can do lock() + lock() + unlock() == free.
Antti,
It's a very bad idea to try and use a mutex like that. The number of
locks and unlocks must be balanced otherwise you risk accessing
variables without a lock.
Consider:
static struct mutex foo_mutex;
static int foo=3;
void a() {
mutex_lock(foo_mutex);
if (foo5) foo++;
b();
foo--; /* still need lock here */
mutex_unlock(foo_mutex);
}
void b() {
mutex_lock(foo_mutex);
if (foo6) foo=(foo1);
mutex_unlock(foo_mutex);
}
Note: this assumes mutex_lock will allow the same thread get multiple
locks as you would like (which it doesn't).
As pointed out in the code, when a() is called, you still need the lock
for accesses to foo after the call to b() that also requires the lock.
If we used the locks in the way you propose then foo would be accessed
without a lock.
To code properly for cases like these I usually use a wrapper functions
to acquire the lock and call a thread unsafe version (i.e. doesn't use
locks) of the function that only uses other thread unsafe functions. e.g.
void a() {
mutex_lock(foo_mutex);
__a_thr_unsafe();
mutex_unlock(foo_mutex);
}
void b() {
mutex_lock(foo_mutex);
__b_thr_unsafe();
mutex_unlock(foo_mutex);
}
static void __a_thr_unsafe() {
if (foo5) foo++;
__b_thr_unsafe();
foo--;
}
static void __b_thr_unsafe() {
if (foo6) foo=(foo1);
}
This way a call to a() or b() will acquire the lock once for that
thread, perform all actions and then release the lock. The mutex is
handled properly.
Can you restructure the code so that you don't need multiple locks?
Thank you for very long and detailed reply with examples :)
I need lock for hardware access. Single I2C-adapter have two I2C-clients
that have same I2C-address in same bus. There is gate (demod I2C-gate)
logic that is used to select desired tuner. See that:
http://palosaari.fi/linux/v4l-dvb/controlling_tuner_af9015_dual_demod.txt
You can never know surely how tuner drivers calls to open or close gate,
very commonly there is situations where multiple close or open happens.
That's why lock/unlock is problematic.
.i2c_gate_ctrl() is demod driver callback (struct dvb_frontend_ops)
which controls gate that gate. That callback is always called from tuner
driver when gate is needed to open or close.
regards
Antti
--
http://palosaari.fi/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: recursive locking problem
At the risk of sounding silly, why do we rely on i2c gating so much? The
whole point of i2c is that you can sit a bunch of devices on the same
pair of wires and talk to one at a time.
Why not just open up the gates and be done with it, except for
situations where the i2c chain foolishly has two devices that have the
same address because somebody didn't net the address configuration pins
correctly, or it's a really big system with more devices on it that a
particular chip family has sub-addresses?
From a signal-driving point of view, the gate circuitry is surely a
sufficient buffer. I guess the only two things I can think of as real
reasons are to 1) reduce the chance of misbehaving devices from causing
problems and 2) to save power by not sending clocks and data where we
know they aren't needed.
Can any one shed some light on this? I appreciate it's not a linux or
indeed linux-media specific issue as the hardware itself is designed
this way.
Getting back to the subject, Antti's af9015 example demonstrates
precisely when gating is needed. But I have to ask, does the MXL5003
really only support one address; can it not be reconfigured? it's a bit
late to ask that question once the PCB is fabbed, I admit.
Would it make any sense to haul the gate locking (or some wrapper at
least) up into the uC for that particular configuration?
E.g:
Tuner driver wants to i2c write tuner 0
Calls i2c_gate_ctrl(open)
uC checks if a gate is already open, if it is, and is the same gate as
is already open, just carry on. If not, wait on a lock, then set which
gate is open and proceed to the 'real' gate_ctrl op.
Similar path for i2c_gate_ctrl(close), as relaxed as it needs to be to
cope with how different tuner drivers work.
You could wrap the checking of which gate is open in a lock for
atomicity, and only wait on a device lock where necessary. This way you
can also check whether you're trying to change the state of a gate or
not and early-out (unless calling gate_ctrl(open) more than once does
something /different/ to calling it just once, in which case we're
doomed and this was nothing more than naive rambling).
Cheers,
--
Steve Kerrison MEng Hons.
http://www.stevekerrison.com/
On Tue, 2011-09-13 at 23:59 +0300, Antti Palosaari wrote:
On 09/09/2011 01:45 PM, David Waring wrote:
On 08/09/11 17:34, Antti Palosaari wrote:
[snip]
Is there any lock can do recursive locking but unlock frees all locks?
Like that:
gate_open
+gate_open
+gate_close
== lock is free
AFAIK mutex can do only simple lock() + unlock(). Semaphore can do
recursive locking, like lock() + lock() + unlock() + unlock(). But how I
can do lock() + lock() + unlock() == free.
Antti,
It's a very bad idea to try and use a mutex like that. The number of
locks and unlocks must be balanced otherwise you risk accessing
variables without a lock.
Consider:
static struct mutex foo_mutex;
static int foo=3;
void a() {
mutex_lock(foo_mutex);
if (foo5) foo++;
b();
foo--; /* still need lock here */
mutex_unlock(foo_mutex);
}
void b() {
mutex_lock(foo_mutex);
if (foo6) foo=(foo1);
mutex_unlock(foo_mutex);
}
Note: this assumes mutex_lock will allow the same thread get multiple
locks as you would like (which it doesn't).
As pointed out in the code, when a() is called, you still need the lock
for accesses to foo after the call to b() that also requires the lock.
If we used the locks in the way you propose then foo would be accessed
without a lock.
To code properly for cases like these I usually use a wrapper functions
to acquire the lock and call a thread unsafe version (i.e. doesn't use
locks) of the function that only uses other thread unsafe functions. e.g.
void a() {
mutex_lock(foo_mutex);
__a_thr_unsafe();
mutex_unlock(foo_mutex);
}
void b() {
mutex_lock(foo_mutex);
__b_thr_unsafe();
mutex_unlock(foo_mutex);
}
static void __a_thr_unsafe() {
if (foo5) foo++;
__b_thr_unsafe();
foo--;
}
static void __b_thr_unsafe() {
if (foo6) foo=(foo1);
}
This way a call to a() or b() will acquire the lock once for that
thread, perform all actions and then release the lock. The mutex is
handled properly.
Can you restructure the code so that you don't need multiple locks?
Thank you for very long and detailed reply with examples :)
I need lock for hardware access. Single I2C-adapter have two I2C-clients
that have same I2C-address in same bus. There is gate (demod I2C-gate)
logic that is used to select desired tuner. See that:
http://palosaari.fi/linux/v4l-dvb/controlling_tuner_af9015_dual_demod.txt
You can never know surely how tuner drivers calls to open or close gate,
very commonly there is situations where multiple close or open happens.
That's why lock/unlock is problematic.
.i2c_gate_ctrl() is demod driver callback (struct dvb_frontend_ops)
which
Re: recursive locking problem
Can any one shed some light on this? I appreciate it's not a linux or indeed linux-media specific issue as the hardware itself is designed this way. i2c gates exist to isolate the downstream components from any spurious RF noise generated by noisy components on the i2c bus. You don't want to couple demod noise into the tuner by default. -- Steven Toth - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: recursive locking problem
On Tue, Sep 13, 2011 at 5:34 PM, Steve Kerrison st...@stevekerrison.com wrote: At the risk of sounding silly, why do we rely on i2c gating so much? The whole point of i2c is that you can sit a bunch of devices on the same pair of wires and talk to one at a time. Steve, There are essentially two issues here. To address the general question, many tuner chips require an i2c gate because their onboard i2c controller is implemented using interrupts, and servicing the interrupts to even check if the traffic is intended for the tuner can interfere with the core tuning function. In other words, the cost of the chip watching for traffic can adversely effect tuning quality. As a result, most hardware designs are such that the demodulator gates the i2c traffic such that the tuner only *ever* sees traffic intended for it. The second issue is that within the LinuxTV drivers there is inconsistency regarding whether the i2c gate is opened/closed by the tuner driver or whether it's done by the demod. Some drivers have the demod driver open the gate, issue the tuning request, and then close the gate, while in other drivers the tuner driver opens/closes the gate whenever there are register reads/writes to the tuner. It's all about the granularity of implementation (the demod approach only involves one open/close but it's for potentially a longer period of time, versus the tuner approach which opens/closes the gate repeatedly as needed, which means more open/closes but the gate is open for the bare minimum of time required). Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: recursive locking problem
On Tue, Sep 13, 2011 at 5:58 PM, Steven Toth st...@kernellabs.com wrote: Can any one shed some light on this? I appreciate it's not a linux or indeed linux-media specific issue as the hardware itself is designed this way. i2c gates exist to isolate the downstream components from any spurious RF noise generated by noisy components on the i2c bus. You don't want to couple demod noise into the tuner by default. Steve (Kerrison), I would take Steven Toth's explanation as more authoritative than mine any day of the week. It's possible that I may have been misinformed regarding the rationale for why the gate is required. Regards, Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: recursive locking problem
I would take Steven Toth's explanation as more authoritative than mine any day of the week. It's possible that I may have been misinformed regarding the rationale for why the gate is required. In general, complete bus isolation lowers noise levels, whether through RF coupling or needless interrupt servicing related. Either rationale is valid and varies between designs. -- Steven Toth - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: recursive locking problem
On 09/09/2011 02:46 PM, Daniel Glöckner wrote: On Thu, Sep 08, 2011 at 07:34:32PM +0300, Antti Palosaari wrote: I am working with AF9015 I2C-adapter lock. I need lock I2C-bus since there is two tuners having same I2C address on same bus, demod I2C gate is used to select correct tuner. Would it be possible to use the i2c-mux framework to handle this? Each tuner will then have its own i2c bus. Interesting idea, but it didn't worked. It deadlocks. I think it locks since I2C-mux is controlled by I2C switch in same I2C bus, not GPIO or some other HW. * tuner does I2C xfer on I2C-mux adapter * I2C-mux adapter calls demod .i2c_gate_ctrl() * demod does register access using I2C * DEADLOCK Maybe since tuner I2C xfer have already locked I2C-adater. Then demod access same adapter and it is locked. But nice I2C mux anyhow. @David Daney, see that pic in order to get understanding what kind of problem I am working; http://palosaari.fi/linux/v4l-dv/controlling_tuner_af9015_dual_demod.txt regards Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: recursive locking problem
On Thursday 08 September 2011 18:34:32 Antti Palosaari wrote: lock() + lock() + unlock() == free. Hi, As far as I can see the Linux kernel's mutex API doesn't have support for checking if a mutex is owned. I guess you would have to do something like: while (mutex_owned(xxx)) mutex_unlock(xxx); --HPS -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: recursive locking problem
On Thu, Sep 08, 2011 at 07:34:32PM +0300, Antti Palosaari wrote: I am working with AF9015 I2C-adapter lock. I need lock I2C-bus since there is two tuners having same I2C address on same bus, demod I2C gate is used to select correct tuner. Would it be possible to use the i2c-mux framework to handle this? Each tuner will then have its own i2c bus. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
recursive locking problem
I am working with AF9015 I2C-adapter lock. I need lock I2C-bus since there is two tuners having same I2C address on same bus, demod I2C gate is used to select correct tuner. I am trapping demod .i2c_gate_ctrl() calls and locking bus according to that. Is there any lock can do recursive locking but unlock frees all locks? Like that: gate_open +gate_open +gate_close == lock is free AFAIK mutex can do only simple lock() + unlock(). Semaphore can do recursive locking, like lock() + lock() + unlock() + unlock(). But how I can do lock() + lock() + unlock() == free. Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
