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

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

 



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 majordomo@xxxxxxxxxxxxxxx
> 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 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