On Tue, Aug 9, 2022 at 9:32 AM Marcus Folkesson <marcus.folkesson@xxxxxxxxx> wrote: Below comments are minor and may be addressed in case you have to issue another version of the series (depending on what Jonathan asks). ... > Add support for buffers to make the driver fit for more usecases. use cases ... > #include <linux/iio/iio.h> > +#include <linux/iio/buffer.h> > +#include <linux/iio/triggered_buffer.h> > +#include <linux/iio/trigger_consumer.h> > +#include <linux/iio/trigger.h> I would split this group... > #include <linux/module.h> > #include <linux/mod_devicetable.h> > #include <linux/property.h> > #include <linux/regulator/consumer.h> > #include <linux/spi/spi.h> > ...and put it here to explicitly show that the driver belongs to the IIO subsystem. > +#include <asm/unaligned.h> ... > + struct spi_transfer xfer[] = { > + { > + .tx_buf = &adc->tx_buf, > + .len = 1, > + }, { > + .rx_buf = adc->rx_buf, > + .len = sizeof(adc->rx_buf), > + }, > + }; > + Redundant blank line. > + int scan_index; ... > + for_each_set_bit(scan_index, indio_dev->active_scan_mask, > + indio_dev->masklength) { One line? > + const struct iio_chan_spec *scan_chan = &indio_dev->channels[scan_index]; > + > + adc->scan.channels[i] = get_unaligned_be24(&adc->rx_buf[scan_chan->channel * 3]); > + i++; > + } -- With Best Regards, Andy Shevchenko