Hi Sean, > > > > + 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. no, actually it's just dirty data and laziness, a memset to 0 fixes it :) > > 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. yes, as I said, it's not a big thing, I can remove the mutex. Thanks, 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
