Am Sat, 21 Oct 2017 11:28:10 +0200 schrieb Ralph Metzler <rjkm@xxxxxxxxxxxxxx>: > Daniel Scheller writes: > > From: Daniel Scheller <d.scheller@xxxxxxx> > > > > When calling gate_ctrl() with enable=0 if previously the mutex > > wasn't locked (ie. on enable=1 failure and subdrivers not handling > > this properly, or by otherwise badly behaving drivers), the > > i2c_lock could be unlocked > > I think drivers and subdrivers should rather be fixed so that this > cannot happen. As long as stv6111 remains the only chip/driver interfacing with the stv0910, that's an easy task. However, if other hardware has some other stv0910+tunerchip combination, things get interesting. In a perfect world with unicorns and such, every component interacts as intended, but that's not the case, so I believe this should be handled at the root. > But to do this we will first need to define exactly how a failure in > gate_ctrl() is supposed to be handled, both inside gate_ctrl() and > by calling drivers. Well, IMHO (and thats the intention) if gate_ctrl fails due to a hardware/I2C problem, it isn't opened so there's no need to hold the lock (since the gate isn't - exclusively - opened). For reasons stated above this keeps things safe from deadlocking (and we want to avoid that, even more than double unlocking). > > consecutively which isn't allowed. Prevent this by keeping track > > of the lock state, and actually call mutex_unlock() only when > > certain the lock is held. > > Why not use mutex_is_locked()? Good catch (I should try harder finding out what the kernel API has to offer...). If you prefer that, I'll respin with this and without the var as v2. > And there should be a debug message if it (tried double unlocking) > happens. Ok. Should IMHO go to dev_dbg then - if drivers don't catch that situation, this may else lead do kernel log spam. Best regards, Daniel Scheller -- https://github.com/herrnst
