Re: PULL http://jusst.de/hg/stv090x

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

 



On Fri, Jan 22, 2010 at 1:23 PM, Manu Abraham <abraham.manu@xxxxxxxxx> wrote:
> The point there is that it is a dual demod which implements a FSM for
> it's tuner flow control and hence. The demodulators in the tree are
> generally single/standalone and don't need such a FSM to handle that.
> Even if it's a dual frontend, most of them just do locking for the
> register access, while here it is not register access locking, while
> it is controlling state, similar to Finite State Machine (FSM)

Ok, I see what is going on now.  I didn't notice before that the gate
enable was keeping the "state->internal->tuner_lock" mutex set even
after the call returned.

My only concern here then is that I've seen cases where the
i2c_gate_ctrl() function is called in a non-symmetric fashion (where
the enable/disable calls are not necessarily properly balanced),
depending on the driver.  While I am relatively confident that you
have balanced the instances where it is called within your driver, I
would be concerned about how it could possibly be called from other
places such as from the tuner driver or the dvb frontend core.

Today, there is no real problem if a particular call path attempts to
enable the gate if it is already open (or disable if it is already
closed).  With your proposed change, you will result in a deadlock.

Also, you might want to consider replacing the calls themselves with
"fe->ops.i2c_gate_ctrl()" as opposed to calling the internal function
directly, since that will allow the "frontend ioctl override"
framework to continue to work properly.

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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux