[1/5,media] dvb-frontends/stv0910: release lock on gate_ctrl() failure

Commit Message

Daniel Scheller Aug. 23, 2017, 4:09 p.m. UTC

From: Daniel Scheller <[e-mail]>

Whenever write_reg() fails to open/close the demod's I2C gate, release the
lock to avoid deadlocking situations. If I2c gate open failed, there's no
need to hold a lock, and if close fails, the mutex_unlock() at the end of
the function is never reached, leaving the mutex_lock in locked state,
which in turn will cause potential for deadlocks. Thus, release the lock
on failure.

While we're touching gate_ctrl(), add some explanation about the need for
locking and the shared I2C bus/gate.

Cc: Julia Lawall <[e-mail]>
Signed-off-by: Daniel Scheller <[e-mail]>
---
 drivers/media/dvb-frontends/stv0910.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

Patch

diff --git a/drivers/media/dvb-frontends/stv0910.c b/drivers/media/dvb-frontends/stv0910.c
index d1ae9553f74c..0d4a6a115159 100644
--- a/drivers/media/dvb-frontends/stv0910.c
+++ b/drivers/media/dvb-frontends/stv0910.c
@@ -1221,17 +1221,32 @@  static int gate_ctrl(struct dvb_frontend *fe, int enable)
 	struct stv *state = fe->demodulator_priv;
 	u8 i2crpt = state->i2crpt & ~0x86;
 
-	if (enable)
-		mutex_lock(&state->base->i2c_lock);
+	/*
+	 * mutex_lock note: Concurrent I2C gate bus accesses must be
+	 * prevented (STV0910 = dual demod on a single IC with a single I2C
+	 * gate/bus, and two tuners attached), similar to most (if not all)
+	 * other I2C host interfaces/busses.
+	 *
+	 * enable=1 (open I2C gate) will grab the lock
+	 * enable=0 (close I2C gate) releases the lock
+	 */
 
-	if (enable)
+	if (enable) {
+		mutex_lock(&state->base->i2c_lock);
 		i2crpt |= 0x80;
-	else
+	} else {
 		i2crpt |= 0x02;
+	}
 
 	if (write_reg(state, state->nr ? RSTV0910_P2_I2CRPT :
-		      RSTV0910_P1_I2CRPT, i2crpt) < 0)
+		      RSTV0910_P1_I2CRPT, i2crpt) < 0) {
+		/* don't hold the I2C bus lock on failure */
+		mutex_unlock(&state->base->i2c_lock);
+		dev_err(&state->base->i2c->dev,
+			"%s() write_reg failure (enable=%d)\n",
+			__func__, enable);
 		return -EIO;
+	}
 
 	state->i2crpt = i2crpt;