On 26/04/23 07:18PM, Jonathan Cameron wrote: > On Wed, 22 Apr 2026 15:45:54 +0100 > Rodrigo Alencar via B4 Relay <[email protected]> > wrote: > > > From: Rodrigo Alencar <[email protected]> > > > > Use of local SPI bus data to manage a collection of SPI transfers and > > flush them to the SPI platform driver with the sync() operation. This > > allows for faster handling of multiple channel DAC writes, avoiding kernel > > overhead per spi_sync() call, which will be helpful when enabling > > triggered buffer support. > > > > Signed-off-by: Rodrigo Alencar <[email protected]> > Interesting approach to potentially optimise for SPI. Do you have > perf numbers or similar to support it being worth the effort?
Short answer is no, I can try to produce some benchmarks, but I am not sure it is needed. The implementation here prepares the ground for triggered buffer support, which adds the capability of implementing a control loop in userspace that updates all the enabled channels at once. That will happen in a trigger handler, then we want that to be as fast as possible, so it can be prepared for the next trigger event. It is not really about calling a function once rather than multiple times, but the problem happens when the device is sharing the spi bus with other devices and how busy the system is in general, so there may be context switches in the spi platform driver with one or two mutex acquire/release operations, which may add bigger delays between transfers, which depends on what the other devices are doing. I suppose that devices with high channel count like 8 or 16 can benefit with this. > Otherwise a few minor comments inline. ... > > @@ -84,8 +107,18 @@ static int ad5686_spi_read(struct ad5686_state *st, u8 > > addr) > > AD5686_ADDR(addr)); > > st->data[1].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP)); > > > > - ret = spi_sync_transfer(spi, t, ARRAY_SIZE(t)); > > - if (ret < 0) > > + xfer[0].tx_buf = &st->data[0].d8[1]; > > + xfer[0].len = 3; > > + xfer[0].cs_change = 1; > > + xfer[1].tx_buf = &st->data[1].d8[1]; > > + xfer[1].rx_buf = &st->data[2].d8[1]; > > + xfer[1].len = 3; > > + xfer[1].cs_change = 0; > > + > > + spi_message_init_with_transfers(&bus_data->msg, xfer, 2); > > Why not carry on using spi_sync_transfer() here? > We'll end up with an spi message the stack but I suspect that's > not a significant performance cost. True, it would be around 200 extra bytes in the stack, and read operations would not happen concurrently with writes, so why not reuse resources (message and transfer structs) that is now available in the bus data? To me, it allows for consistency in this implementation. > > + > > + ret = spi_sync(spi, &bus_data->msg); > > + if (ret) > > return ret; > > > > return be32_to_cpu(st->data[2].d32); -- Kind regards, Rodrigo Alencar
