Manu Abraham wrote:
> Mauro,
> 
> Please pull http://jusst.de/hg/stv090x changesets: from 13880 - 13891
> inclusive of both.

The third changeset has some locking troubles:

# Node ID 4dd17a6a5ecc320eab58590a8567e5ba03b9d53a
[STV090x] Added mutex protection around tuner I2C access.

After the patch, the code will look like:

static int stv090x_i2c_gate_ctrl(struct dvb_frontend *fe, int enable)
{
        struct stv090x_state *state = fe->demodulator_priv;
        u32 reg;

        if (enable)
                mutex_lock(&state->internal->tuner_lock);

        reg = STV090x_READ_DEMOD(state, I2CRPT);
        if (enable) {
                dprintk(FE_DEBUG, 1, "Enable Gate");
                STV090x_SETFIELD_Px(reg, I2CT_ON_FIELD, 1);
                if (STV090x_WRITE_DEMOD(state, I2CRPT, reg) < 0)
                        goto err;

        } else {

                dprintk(FE_DEBUG, 1, "Disable Gate");
                STV090x_SETFIELD_Px(reg, I2CT_ON_FIELD, 0);
                if ((STV090x_WRITE_DEMOD(state, I2CRPT, reg)) < 0)
                        goto err;
        }

        if (!enable)
                mutex_unlock(&state->internal->tuner_lock);

        return 0;
err:
        dprintk(FE_ERROR, 1, "I/O error");
        mutex_unlock(&state->internal->tuner_lock);
        return -1;
}

As the err: is called on both enable and disable conditions, the error
code will try to unlock an already unlocked mutex, if !enable.

Also, it doesn't make sense to lock only if the gate is enabled,
since it seems that the lock is meant to protect the i2c gate bus and
both conditions will call STV090x_SETFIELD_Px(reg, I2CT_ON_FIELD, enable ? 1 : 
0);

The better would be simply to remove the if (enable)/if (!enable) tests
on the above code.


> 
> Regards,
> Manu
> --
> 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

--
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

Reply via email to