On Fri, 2009-10-09 at 14:55 +0200, Ujfalusi Peter (Nokia-D/Tampere)
wrote:
> +static int tpa6130a2_i2c_write(int reg, u8 value)
> +{
> + struct tpa6130a2_data *data;
> + int val = 0;
> +
> + BUG_ON(tpa6130a2_client == NULL);
> + data = i2c_get_clientdata(tpa6130a2_client);
> +
> + if (data->power_state) {
> + val = i2c_smbus_write_byte_data(tpa6130a2_client, reg, value);
> + if (val < 0)
> + dev_err(&tpa6130a2_client->dev, "Write failed\n");
> + }
> +
> + /* Either powered on or off, we save the context */
> + data->regs[reg] = value;
> +
> + return val;
> +}
> +
<snip>
> +static void tpa6130a2_channel_enable(u8 channel, int enable)
> +{
> + struct tpa6130a2_data *data;
> + u8 val;
> +
> + BUG_ON(tpa6130a2_client == NULL);
> + data = i2c_get_clientdata(tpa6130a2_client);
> +
mutex_lock()
> + if (enable) {
> + /* Enable channel */
> + /* Enable amplifier */
> + val = tpa6130a2_read(TPA6130A2_REG_CONTROL);
> + val |= channel;
> + tpa6130a2_i2c_write(TPA6130A2_REG_CONTROL, val);
During the tpa6130a2_i2c_write, you read the data->power_state, that
may change it's state meanwhile (preempted). Thus, I suggest you using
the mutex to cover all i2c writes. (and all tpa read <-> write cycles,
so that things are consistent?)
A call to tpa6130a2_power() -> preemted over somewhere here, you may
have i2c accesses to a chip that's not powered up?
> +
> + /* Unmute channel */
> + val = tpa6130a2_read(TPA6130A2_REG_VOL_MUTE);
> + val &= ~channel;
> + tpa6130a2_i2c_write(TPA6130A2_REG_VOL_MUTE, val);
> + } else {
> + /* Disable channel */
> + /* Mute channel */
> + val = tpa6130a2_read(TPA6130A2_REG_VOL_MUTE);
> + val |= channel;
> + tpa6130a2_i2c_write(TPA6130A2_REG_VOL_MUTE, val);
> +
> + /* Disable amplifier */
> + val = tpa6130a2_read(TPA6130A2_REG_CONTROL);
> + val &= ~channel;
> + tpa6130a2_i2c_write(TPA6130A2_REG_CONTROL, val);
> + }
mutex_unlock() ?
> +}
> +
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
