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? Otherwise a few minor comments inline. Thanks, J > --- > drivers/iio/dac/ad5686-spi.c | 110 > +++++++++++++++++++++++++++++++------------ > drivers/iio/dac/ad5686.c | 4 +- > drivers/iio/dac/ad5686.h | 8 +++- > drivers/iio/dac/ad5696-i2c.c | 2 +- > 4 files changed, 89 insertions(+), 35 deletions(-) > > diff --git a/drivers/iio/dac/ad5686-spi.c b/drivers/iio/dac/ad5686-spi.c > index ebfc40efa679..bacfb1deab31 100644 > --- a/drivers/iio/dac/ad5686-spi.c > +++ b/drivers/iio/dac/ad5686-spi.c > @@ -13,57 +13,80 @@ > #include <linux/err.h> > #include <linux/mod_devicetable.h> > #include <linux/module.h> > +#include <linux/overflow.h> > #include <linux/spi/spi.h> > > #include "ad5686.h" > > +struct ad5686_spi_data { > + struct spi_message msg; > + unsigned int size; > + unsigned int capacity; > + struct spi_transfer xfers[] __counted_by(capacity); > +}; > + > static int ad5686_spi_write(struct ad5686_state *st, > u8 cmd, u8 addr, u16 val) > { > - struct spi_device *spi = to_spi_device(st->dev); > - u8 tx_len, *buf; > + struct ad5686_spi_data *bus_data = st->bus_data; > + struct spi_transfer *xfer; > + > + if (bus_data->size >= bus_data->capacity) > + return -E2BIG; > + > + if (bus_data->size) > + bus_data->xfers[bus_data->size - 1].cs_change = 1; > + else > + spi_message_init(&bus_data->msg); > + > + xfer = &bus_data->xfers[bus_data->size]; > + xfer->rx_buf = NULL; > + xfer->cs_change = 0; > > switch (st->chip_info->regmap_type) { > case AD5310_REGMAP: > - st->data[0].d16 = cpu_to_be16(AD5310_CMD(cmd) | > - val); Fits on oneline. > - buf = &st->data[0].d8[0]; > - tx_len = 2; > + st->data[bus_data->size].d16 = cpu_to_be16(AD5310_CMD(cmd) | > + val); Even these I'd put on one line to improve readability, or split as: st->data[bus_data->size].d16 = cpu_to_be16(...) > + xfer->tx_buf = &st->data[bus_data->size].d8[0]; > + xfer->len = 2; > break; > case AD5683_REGMAP: > - st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) | > - AD5683_DATA(val)); > - buf = &st->data[0].d8[1]; > - tx_len = 3; > + st->data[bus_data->size].d32 = cpu_to_be32(AD5686_CMD(cmd) | > + AD5683_DATA(val)); > + xfer->tx_buf = &st->data[bus_data->size].d8[1]; > + xfer->len = 3; > break; > case AD5686_REGMAP: > - st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) | > - AD5686_ADDR(addr) | > - val); > - buf = &st->data[0].d8[1]; > - tx_len = 3; > + st->data[bus_data->size].d32 = cpu_to_be32(AD5686_CMD(cmd) | > + AD5686_ADDR(addr) | > + val); > + xfer->tx_buf = &st->data[bus_data->size].d8[1]; > + xfer->len = 3; > break; > default: > return -EINVAL; > } > > - return spi_write(spi, buf, tx_len); > + spi_message_add_tail(xfer, &bus_data->msg); > + bus_data->size++; > + > + return 0; > +} > > static int ad5686_spi_read(struct ad5686_state *st, u8 addr) > { > - struct spi_transfer t[] = { > - { > - .tx_buf = &st->data[0].d8[1], > - .len = 3, > - .cs_change = 1, > - }, { > - .tx_buf = &st->data[1].d8[1], > - .rx_buf = &st->data[2].d8[1], > - .len = 3, > - }, > - }; > struct spi_device *spi = to_spi_device(st->dev); > + struct ad5686_spi_data *bus_data = st->bus_data; > + struct spi_transfer *xfer = &bus_data->xfers[0]; > u8 cmd = 0; > int ret; > > @@ -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. > + > + ret = spi_sync(spi, &bus_data->msg); > + if (ret) > return ret; > > return be32_to_cpu(st->data[2].d32);
