* [PATCH v2 1/2] iio: adc: ad4695: implement triggered buffer
2024-08-13 17:26 [PATCH v2 0/2] iio: adc: ad4695: implement triggered buffer David Lechner
@ 2024-08-13 17:26 ` David Lechner
2024-08-13 17:26 ` [PATCH v2 2/2] doc: iio: ad4695: document buffered read David Lechner
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: David Lechner @ 2024-08-13 17:26 UTC (permalink / raw)
To: Jonathan Cameron
Cc: David Lechner, Michael Hennerich, Nuno Sá, Jonathan Corbet,
linux-iio, linux-kernel, linux-doc
This implements buffered reads for the ad4695 driver using the typical
triggered buffer implementation, including adding a soft timestamp
channel.
The chip has 4 different modes for doing conversions. The driver is
using the advanced sequencer mode since that is the only mode that
allows individual configuration of all aspects each channel (e.g.
bipolar config currently and oversampling to be added in the future).
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
drivers/iio/adc/ad4695.c | 250 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 247 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c
index 007ecb951bc3..b8547914f00f 100644
--- a/drivers/iio/adc/ad4695.c
+++ b/drivers/iio/adc/ad4695.c
@@ -11,6 +11,7 @@
* Copyright 2024 BayLibre, SAS
*/
+#include <linux/align.h>
#include <linux/bitfield.h>
#include <linux/bits.h>
#include <linux/compiler.h>
@@ -18,7 +19,10 @@
#include <linux/device.h>
#include <linux/err.h>
#include <linux/gpio/consumer.h>
+#include <linux/iio/buffer.h>
#include <linux/iio/iio.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
#include <linux/property.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
@@ -61,6 +65,7 @@
#define AD4695_REG_GPIO_CTRL 0x0026
#define AD4695_REG_GP_MODE 0x0027
#define AD4695_REG_TEMP_CTRL 0x0029
+#define AD4695_REG_TEMP_CTRL_TEMP_EN BIT(0)
#define AD4695_REG_CONFIG_IN(n) (0x0030 | (n))
#define AD4695_REG_CONFIG_IN_MODE BIT(6)
#define AD4695_REG_CONFIG_IN_PAIR GENMASK(5, 4)
@@ -84,9 +89,13 @@
#define AD4695_T_WAKEUP_HW_MS 3
#define AD4695_T_WAKEUP_SW_MS 3
#define AD4695_T_REFBUF_MS 100
+#define AD4695_T_REGCONFIG_NS 20
#define AD4695_REG_ACCESS_SCLK_HZ (10 * MEGA)
+/* Max number of voltage input channels. */
#define AD4695_MAX_CHANNELS 16
+/* Max size of 1 raw sample in bytes. */
+#define AD4695_MAX_CHANNEL_SIZE 2
enum ad4695_in_pair {
AD4695_IN_PAIR_REFGND,
@@ -97,6 +106,7 @@ enum ad4695_in_pair {
struct ad4695_chip_info {
const char *name;
int max_sample_rate;
+ u32 t_acq_ns;
u8 num_voltage_inputs;
};
@@ -112,15 +122,21 @@ struct ad4695_state {
struct spi_device *spi;
struct regmap *regmap;
struct gpio_desc *reset_gpio;
- struct iio_chan_spec iio_chan[AD4695_MAX_CHANNELS + 1];
+ /* voltages channels plus temperature and timestamp */
+ struct iio_chan_spec iio_chan[AD4695_MAX_CHANNELS + 2];
struct ad4695_channel_config channels_cfg[AD4695_MAX_CHANNELS];
const struct ad4695_chip_info *chip_info;
/* Reference voltage. */
unsigned int vref_mv;
/* Common mode input pin voltage. */
unsigned int com_mv;
+ /* 1 per voltage and temperature chan plus 1 xfer to trigger 1st CNV */
+ struct spi_transfer buf_read_xfer[AD4695_MAX_CHANNELS + 2];
+ struct spi_message buf_read_msg;
/* Raw conversion data received. */
- u16 raw_data __aligned(IIO_DMA_MINALIGN);
+ u8 buf[ALIGN((AD4695_MAX_CHANNELS + 2) * AD4695_MAX_CHANNEL_SIZE,
+ sizeof(s64)) + sizeof(s64)] __aligned(IIO_DMA_MINALIGN);
+ u16 raw_data;
/* Commands to send for single conversion. */
u16 cnv_cmd;
u8 cnv_cmd2;
@@ -193,6 +209,9 @@ static const struct iio_chan_spec ad4695_temp_channel_template = {
},
};
+static const struct iio_chan_spec ad4695_soft_timestamp_channel_template =
+ IIO_CHAN_SOFT_TIMESTAMP(0);
+
static const char * const ad4695_power_supplies[] = {
"avdd", "vio"
};
@@ -200,24 +219,28 @@ static const char * const ad4695_power_supplies[] = {
static const struct ad4695_chip_info ad4695_chip_info = {
.name = "ad4695",
.max_sample_rate = 500 * KILO,
+ .t_acq_ns = 1715,
.num_voltage_inputs = 16,
};
static const struct ad4695_chip_info ad4696_chip_info = {
.name = "ad4696",
.max_sample_rate = 1 * MEGA,
+ .t_acq_ns = 715,
.num_voltage_inputs = 16,
};
static const struct ad4695_chip_info ad4697_chip_info = {
.name = "ad4697",
.max_sample_rate = 500 * KILO,
+ .t_acq_ns = 1715,
.num_voltage_inputs = 8,
};
static const struct ad4695_chip_info ad4698_chip_info = {
.name = "ad4698",
.max_sample_rate = 1 * MEGA,
+ .t_acq_ns = 715,
.num_voltage_inputs = 8,
};
@@ -254,6 +277,61 @@ static int ad4695_set_single_cycle_mode(struct ad4695_state *st,
AD4695_REG_SETUP_SPI_CYC_CTRL);
}
+/**
+ * ad4695_enter_advanced_sequencer_mode - Put the ADC in advanced sequencer mode
+ * @st: The driver state
+ * @n: The number of slots to use - must be >= 2, <= 128
+ *
+ * As per the datasheet, to enable advanced sequencer, we need to set
+ * STD_SEQ_EN=0, NUM_SLOTS_AS=n-1 and CYC_CTRL=0 (Table 15). Setting SPI_MODE=1
+ * triggers the first conversion using the channel in AS_SLOT0.
+ *
+ * Return: 0 on success, a negative error code on failure
+ */
+static int ad4695_enter_advanced_sequencer_mode(struct ad4695_state *st, u32 n)
+{
+ int ret;
+
+ ret = regmap_update_bits(st->regmap, AD4695_REG_SEQ_CTRL,
+ AD4695_REG_SEQ_CTRL_STD_SEQ_EN |
+ AD4695_REG_SEQ_CTRL_NUM_SLOTS_AS,
+ FIELD_PREP(AD4695_REG_SEQ_CTRL_STD_SEQ_EN, 0) |
+ FIELD_PREP(AD4695_REG_SEQ_CTRL_NUM_SLOTS_AS, n - 1));
+ if (ret)
+ return ret;
+
+ return regmap_update_bits(st->regmap, AD4695_REG_SETUP,
+ AD4695_REG_SETUP_SPI_MODE | AD4695_REG_SETUP_SPI_CYC_CTRL,
+ FIELD_PREP(AD4695_REG_SETUP_SPI_MODE, 1) |
+ FIELD_PREP(AD4695_REG_SETUP_SPI_CYC_CTRL, 0));
+}
+
+/**
+ * ad4695_exit_conversion_mode - Exit conversion mode
+ * @st: The AD4695 state
+ *
+ * Sends SPI command to exit conversion mode.
+ *
+ * Return: 0 on success, a negative error code on failure
+ */
+static int ad4695_exit_conversion_mode(struct ad4695_state *st)
+{
+ struct spi_transfer xfer = {
+ .tx_buf = &st->cnv_cmd2,
+ .len = 1,
+ .delay.value = AD4695_T_REGCONFIG_NS,
+ .delay.unit = SPI_DELAY_UNIT_NSECS,
+ };
+
+ /*
+ * Technically, could do a 5-bit transfer, but shifting to start of
+ * 8 bits instead for better SPI controller support.
+ */
+ st->cnv_cmd2 = AD4695_CMD_EXIT_CNV_MODE << 3;
+
+ return spi_sync_transfer(st->spi, &xfer, 1);
+}
+
static int ad4695_set_ref_voltage(struct ad4695_state *st, int vref_mv)
{
u8 val;
@@ -296,6 +374,161 @@ static int ad4695_write_chn_cfg(struct ad4695_state *st,
mask, val);
}
+static int ad4695_buffer_preenable(struct iio_dev *indio_dev)
+{
+ struct ad4695_state *st = iio_priv(indio_dev);
+ struct spi_transfer *xfer;
+ u8 temp_chan_bit = st->chip_info->num_voltage_inputs;
+ u32 bit, num_xfer, num_slots;
+ u32 temp_en = 0;
+ int ret;
+
+ /*
+ * We are using the advanced sequencer since it is the only way to read
+ * multiple channels that allows individual configuration of each
+ * voltage input channel. Slot 0 in the advanced sequencer is used to
+ * account for the gap between trigger polls - we don't read data from
+ * this slot. Each enabled voltage channel is assigned a slot starting
+ * with slot 1.
+ */
+ num_slots = 1;
+
+ memset(st->buf_read_xfer, 0, sizeof(st->buf_read_xfer));
+
+ /* First xfer is only to trigger conversion of slot 1, so no rx. */
+ xfer = &st->buf_read_xfer[0];
+ xfer->cs_change = 1;
+ xfer->delay.value = st->chip_info->t_acq_ns;
+ xfer->delay.unit = SPI_DELAY_UNIT_NSECS;
+ xfer->cs_change_delay.value = AD4695_T_CONVERT_NS;
+ xfer->cs_change_delay.unit = SPI_DELAY_UNIT_NSECS;
+ num_xfer = 1;
+
+ iio_for_each_active_channel(indio_dev, bit) {
+ xfer = &st->buf_read_xfer[num_xfer];
+ xfer->bits_per_word = 16;
+ xfer->rx_buf = &st->buf[(num_xfer - 1) * 2];
+ xfer->len = 2;
+ xfer->cs_change = 1;
+ xfer->cs_change_delay.value = AD4695_T_CONVERT_NS;
+ xfer->cs_change_delay.unit = SPI_DELAY_UNIT_NSECS;
+
+ if (bit == temp_chan_bit) {
+ temp_en = 1;
+ } else {
+ ret = regmap_write(st->regmap,
+ AD4695_REG_AS_SLOT(num_slots),
+ FIELD_PREP(AD4695_REG_AS_SLOT_INX, bit));
+ if (ret)
+ return ret;
+
+ num_slots++;
+ }
+
+ num_xfer++;
+ }
+
+ /*
+ * The advanced sequencer requires that at least 2 slots are enabled.
+ * Since slot 0 is always used for other purposes, we need only 1
+ * enabled voltage channel to meet this requirement. If the temperature
+ * channel is the only enabled channel, we need to add one more slot
+ * in the sequence but not read from it.
+ */
+ if (num_slots < 2) {
+ /* move last xfer so we can insert one more xfer before it */
+ st->buf_read_xfer[num_xfer] = *xfer;
+ num_xfer++;
+
+ /* modify 2nd to last xfer for extra slot */
+ memset(xfer, 0, sizeof(*xfer));
+ xfer->cs_change = 1;
+ xfer->delay.value = st->chip_info->t_acq_ns;
+ xfer->delay.unit = SPI_DELAY_UNIT_NSECS;
+ xfer->cs_change_delay.value = AD4695_T_CONVERT_NS;
+ xfer->cs_change_delay.unit = SPI_DELAY_UNIT_NSECS;
+ xfer++;
+
+ /* and add the extra slot in the sequencer */
+ ret = regmap_write(st->regmap,
+ AD4695_REG_AS_SLOT(num_slots),
+ FIELD_PREP(AD4695_REG_AS_SLOT_INX, 0));
+ if (ret)
+ return ret;
+
+ num_slots++;
+ }
+
+ /*
+ * Don't keep CS asserted after last xfer. Also triggers conversion of
+ * slot 0.
+ */
+ xfer->cs_change = 0;
+
+ /*
+ * Temperature channel isn't included in the sequence, but rather
+ * controlled by setting a bit in the TEMP_CTRL register.
+ */
+
+ ret = regmap_update_bits(st->regmap, AD4695_REG_TEMP_CTRL,
+ AD4695_REG_TEMP_CTRL_TEMP_EN,
+ FIELD_PREP(AD4695_REG_TEMP_CTRL_TEMP_EN, temp_en));
+ if (ret)
+ return ret;
+
+ spi_message_init_with_transfers(&st->buf_read_msg, st->buf_read_xfer,
+ num_xfer);
+
+ ret = spi_optimize_message(st->spi, &st->buf_read_msg);
+ if (ret)
+ return ret;
+
+ /* This triggers conversion of slot 0. */
+ ret = ad4695_enter_advanced_sequencer_mode(st, num_slots);
+ if (ret)
+ spi_unoptimize_message(&st->buf_read_msg);
+
+ return ret;
+}
+
+static int ad4695_buffer_postdisable(struct iio_dev *indio_dev)
+{
+ struct ad4695_state *st = iio_priv(indio_dev);
+ int ret;
+
+ ret = ad4695_exit_conversion_mode(st);
+ if (ret)
+ return ret;
+
+ spi_unoptimize_message(&st->buf_read_msg);
+
+ return 0;
+}
+
+static const struct iio_buffer_setup_ops ad4695_buffer_setup_ops = {
+ .preenable = ad4695_buffer_preenable,
+ .postdisable = ad4695_buffer_postdisable,
+};
+
+static irqreturn_t ad4695_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct ad4695_state *st = iio_priv(indio_dev);
+ int ret;
+
+ ret = spi_sync(st->spi, &st->buf_read_msg);
+ if (ret)
+ goto out;
+
+ iio_push_to_buffers_with_timestamp(indio_dev, st->buf, pf->timestamp);
+
+out:
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
/**
* ad4695_read_one_sample - Read a single sample using single-cycle mode
* @st: The AD4695 state
@@ -527,6 +760,10 @@ static int ad4695_parse_channel_cfg(struct ad4695_state *st)
/* Temperature channel must be next scan index after voltage channels. */
st->iio_chan[i] = ad4695_temp_channel_template;
st->iio_chan[i].scan_index = i;
+ i++;
+
+ st->iio_chan[i] = ad4695_soft_timestamp_channel_template;
+ st->iio_chan[i].scan_index = i;
return 0;
}
@@ -695,7 +932,14 @@ static int ad4695_probe(struct spi_device *spi)
indio_dev->info = &ad4695_info;
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->channels = st->iio_chan;
- indio_dev->num_channels = st->chip_info->num_voltage_inputs + 1;
+ indio_dev->num_channels = st->chip_info->num_voltage_inputs + 2;
+
+ ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
+ iio_pollfunc_store_time,
+ ad4695_trigger_handler,
+ &ad4695_buffer_setup_ops);
+ if (ret)
+ return ret;
return devm_iio_device_register(dev, indio_dev);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2 0/2] iio: adc: ad4695: implement triggered buffer
2024-08-13 17:26 [PATCH v2 0/2] iio: adc: ad4695: implement triggered buffer David Lechner
2024-08-13 17:26 ` [PATCH v2 1/2] " David Lechner
2024-08-13 17:26 ` [PATCH v2 2/2] doc: iio: ad4695: document buffered read David Lechner
@ 2024-08-17 14:17 ` Jonathan Cameron
2024-08-17 14:28 ` Jonathan Cameron
3 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2024-08-17 14:17 UTC (permalink / raw)
To: David Lechner
Cc: Michael Hennerich, Nuno Sá, Jonathan Corbet, linux-iio,
linux-kernel, linux-doc
On Tue, 13 Aug 2024 12:26:39 -0500
David Lechner <dlechner@baylibre.com> wrote:
> This is a fairly simple series that adds support for triggered buffers
> to the ad4695 driver.
>
> Not directly related to this patch, but as a side discussion about
> future possibilities with this chip while we are here...
>
> The advanced sequencer on this chip can repeat the same channel multiple
> times which, when combined with the autocycle feature, can be used to
> create different effective sampling rates for individual channels.
>
> For example if we set up the sequence [IN1, IN2, IN1, IN3] and the time
> between each individual sample in the sequence is the same, then IN1 has
> an effective sampling rate of 2x the other channels.
>
> Have there ever been discussions before about implementing something
> like this in the IIO subsystem? I didn't see anything that looked like
> this already implemented in the kernel.
Yes. There has been discussion in the past, but it's hard to define
an ABI around so we have either given up, or had the driver unwind the
data flow into multiple buffers. There are some IMUs that do tagged
data in their hardware fifos and you can configure the sampling frequencies
for sets of channels independently.
So you get
1 tag_accel : accel_x accel_y accel_z
2 tag_accel : accel_x accel_y accel_z
3 tag_magn : magn_x, magn_y magn_z
4 tag_accel : accel_x accel_y accel_z
5 tag_accel : accel_x accel_y accel_z
6 tag_magn : magn_x, magn_y magn_z
7 tag_accel : accel_x accel_y accel_z
The driver then pushes to multiple buffers.
accel_buffer:
1 tag_accel : accel_x accel_y accel_z
2 tag_accel : accel_x accel_y accel_z
4 tag_accel : accel_x accel_y accel_z
5 tag_accel : accel_x accel_y accel_z
7 tag_accel : accel_x accel_y accel_z
magn_buffer:
3 tag_magn : magn_x, magn_y magn_z
6 tag_magn : magn_x, magn_y magn_z
etc.
See imu/st_lsm6dsx/st_lsm6dsx_buffer.c
and st_lsm6dsx_push_tagged_data()
I think we discussed more complex setups where channels could be offered
for different buffers, each with own sampling rate and the driver (note not
the IIO core) would try to unwind what you'd asked for into each buffer +
figure out how to configure the device. Don't think that got implemented
though as the simple multibuffer solution was enough.
Note this predated per iio_dev multiple buffer support so it was done with additional
IIO devices.
Otherwise we do have option to do repeat scan types. That works for
IN0 IN0 IN0 IN0 IN1 IN0 IN0 IN0 IN0 IN1 IN0 etc
That is really meant for multi value channels - e.g. quarternions where
a single value on it's own is meaningless (you need the scaling across
all 4 elements).
Jonathan
>
> ---
> Changes in v2:
> v2 changes:
> * Don't error when only temperature channel is enabled, just add extra
> xfer instead.
> * Drop use of reg/mask/val local variables.
> * Use struct initializer for xfer in ad4695_exit_conversion_mode().
> * Simplify return at end ad4695_buffer_preenable().
> * Use t_ACQ instead of t_CNVL to ensure proper acquisition time.
> * The temperature channel changes weren't quite trivial, so I didn't
> pick up Nuno's Reviewed-by.
> * Link to v1: https://lore.kernel.org/r/20240807-iio-adc-ad4695-buffered-read-v1-0-bdafc39b2283@baylibre.com
>
> ---
> David Lechner (2):
> iio: adc: ad4695: implement triggered buffer
> doc: iio: ad4695: document buffered read
>
> Documentation/iio/ad4695.rst | 9 +-
> drivers/iio/adc/ad4695.c | 250 ++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 255 insertions(+), 4 deletions(-)
> ---
> base-commit: d505a9dc8de83df2a8ce8c9e780f7b29887f34af
> change-id: 20240807-iio-adc-ad4695-buffered-read-f49eb511e300
>
> Best regards,
^ permalink raw reply [flat|nested] 5+ messages in thread