Hi Andi, On Thu, Jul 21, 2016 at 10:09:26AM +0900, Andi Shyti wrote: > > > + 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. It can be stack allocated for sync transfers. You might want to lock the spi bus. > 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. That's cargo-cult locking. It does not achieve anything. Sean -- 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
