On Sat, Aug 12, 2017 at 05:24:03PM +0530, Vikram N wrote: > else > - status = spi_sync(spi, message); > + status = spidev->bus_locked ? spi_sync_locked(spi, message) : > + spi_sync(spi, message);
Please don't abuse the ternery operator, people need to be able to read the code. > + case SPI_IOC_BUS_LOCK: > + spi_bus_lock(spi->master); > + spidev->bus_locked = true; > + break; > + > + case SPI_IOC_BUS_UNLOCK: > + spi_bus_unlock(spi->master); > + spidev->bus_locked = false; > + break; I'm not super convinced that this API is a good idea in general - it seems extremely niche to be using multiple userspace programs that don't need to coordinate at all except for a single lock (which they will all need to use to avoid just bouncing off with errors). That all seems very narrow. I'm also worried that even if there is such a use case this code is very fragile as it stands. If an application crashes then nothing will free a lock it holds and any application can through simple error drop locks that are supposed to be held by other applications. This isn't going to be terribly robust.
signature.asc
Description: PGP signature
