Hi Sean, > > + int ret; > > + struct ir_spi_data *idata = (struct ir_spi_data *) dev->priv; > > No cast needed. yes, thanks. > > + ret = regulator_enable(idata->regulator); > > + if (ret) > > + return ret; > > + > > + mutex_lock(&idata->mutex); > > + idata->xfer.len = n; > > + idata->xfer.tx_buf = buffer; > > + mutex_unlock(&idata->mutex); > > I'm not convinced the locking works here. You want to guard against > someone modifying xfer while you are sending (so in spi_sync_transfer), > which this locking is not doing. You could declare a > local "struct spi_transfer xfer" and avoid the mutex altogether. I cannot declare xfer locally because the spi framework needs a statically allocated xfer, so that either I dynamically allocate it in the function or I declare it global in idata. With the mutex I would like to prevent different tasks to change the value at the same time, it's an easy case, it shouldn't make much difference. There are checkpatch issues, in the next patchset I will fix them. Thanks a lot for your review, Andi -- 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
