On Mon, 24 Jun 2024 17:01:54 -0500 David Lechner <dlechner@xxxxxxxxxxxx> wrote: > This is a new driver for Analog Devices Inc. AD4695 and similar ADCs. > The initial driver supports initializing the chip including configuring > all possible LDO and reference voltage sources as well as any possible > voltage input channel wiring configuration. > > Only the 4-wire SPI wiring mode where the CNV pin is tied to the CS pin > is supported at this time. And reading sample data from the ADC can only > be done in direct mode for now. > > Co-developed-by: Ramona Gradinariu <ramona.gradinariu@xxxxxxxxxx> > Signed-off-by: Ramona Gradinariu <ramona.gradinariu@xxxxxxxxxx> > Signed-off-by: David Lechner <dlechner@xxxxxxxxxxxx> Some minor bits an pieces from me. > diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c > new file mode 100644 > index 000000000000..72ab5b997f6e > --- /dev/null > +++ b/drivers/iio/adc/ad4695.c > @@ -0,0 +1,730 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * SPI ADC driver for Analog Devices Inc. AD4695 and similar chips > + * > + * https://www.analog.com/en/products/ad4695.html > + * https://www.analog.com/en/products/ad4696.html > + * https://www.analog.com/en/products/ad4697.html > + * https://www.analog.com/en/products/ad4698.html > + * > + * Copyright 2024 Analog Devices Inc. > + * Copyright 2024 BayLibre, SAS > + */ > + > +#include <dt-bindings/iio/adi,ad4695.h> Stick driver specific includes after linux ones. Probably a blank line in between as well. > +#include <linux/bitfield.h> > +#include <linux/bits.h> > +#include <linux/compiler.h> > +#include <linux/delay.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/gpio/consumer.h> > +#include <linux/iio/iio.h> > +#include <linux/property.h> > +#include <linux/regmap.h> > +#include <linux/regulator/consumer.h> > +#include <linux/spi/spi.h> > +#include <linux/units.h> > + > + > +/** > + * ad4695_read_one_sample - Read a single sample using single-cycle mode > + * @st: The AD4695 state > + * @address: The address of the channel to read > + * > + * Upon return, the sample will be stored in the raw_data field of @st. > + * > + * Context: can sleep, must be called with iio_device_claim_direct held > + * Return: 0 on success, a negative error code on failure > + */ > +static int ad4695_read_one_sample(struct ad4695_state *st, unsigned int address) > +{ > + struct spi_transfer xfer[2] = { }; > + int ret; > + > + ret = ad4695_set_single_cycle_mode(st, address); > + if (ret) > + return ret; > + > + /* > + * Setting the first channel to the temperature channel isn't supported > + * in single-cycle mode, so we have to do an extra xfer to read the > + * temperature. > + */ > + if (address == AD4695_CMD_TEMP_CHAN) { > + /* We aren't reading, so we can make this a short xfer. */ I'd be tempted to let the compiler figure out it can combine storage for xfer and do something like struct spi_transfer xfer[2] = { { .bits_per_word = 8, .tx_buf = ... }, { }, }; st->cnv_cmd2 = ... etc Advantage is that it is clearly structured data. Up to you though to decide if this is worth doing. I don't care that much! > + st->cnv_cmd2 = AD4695_CMD_TEMP_CHAN << 3; > + xfer[0].bits_per_word = 8; > + xfer[0].tx_buf = &st->cnv_cmd2; > + xfer[0].len = 1; > + xfer[0].cs_change = 1; > + xfer[0].cs_change_delay.value = AD4695_T_CONVERT_NS; > + xfer[0].cs_change_delay.unit = SPI_DELAY_UNIT_NSECS; > + > + /* Then read the result and exit conversion mode. */ > + st->cnv_cmd = AD4695_CMD_EXIT_CNV_MODE << 11; > + xfer[1].bits_per_word = 16; > + xfer[1].tx_buf = &st->cnv_cmd; > + xfer[1].rx_buf = &st->raw_data; > + xfer[1].len = 2; > + > + return spi_sync_transfer(st->spi, xfer, 2); > + } then an else here to reduce the scope of another xfer structure. > + > + /* > + * The conversion has already been done and we just have to read the > + * result and exit conversion mode. > + */ > + st->cnv_cmd = AD4695_CMD_EXIT_CNV_MODE << 11; > + xfer[0].bits_per_word = 16; > + xfer[0].tx_buf = &st->cnv_cmd; > + xfer[0].rx_buf = &st->raw_data; > + xfer[0].len = 2; > + > + return spi_sync_transfer(st->spi, xfer, 1); > +}
