| Message ID | 20240510-dlech-mainline-spi-engine-offload-2-v2-8-8707a870c435@baylibre.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | spi: axi-spi-engine: add offload support | expand |
On Fri, 10 May 2024 19:44:31 -0500 David Lechner <dlechner@baylibre.com> wrote: > This adds support for SPI offload to the ad7944 driver. This allows > reading data at the max sample rate of 2.5 MSPS. > > Signed-off-by: David Lechner <dlechner@baylibre.com> > --- > > v2 changes: > > In the previous version, there was a new separate driver for the PWM > trigger and DMA hardware buffer. This was deemed too complex so they > are moved into the ad7944 driver. > > It has also been reworked to accommodate for the changes described in > the other patches. > > RFC: This isn't very polished yet, just FYI. A few things to sort out: > > Rather than making the buffer either triggered buffer or hardware buffer, > I'm considering allowing both, e.g. buffer0 will always be the triggered > buffer and buffer1 will will be the hardware buffer if connected to a SPI > controller with offload support, otherwise buffer1 is absent. But since > multiple buffers haven't been used much so far, more investigation is > needed to see how that would work in practice. If we do that though, then > we would always have the sampling_frequency attribute though even though > it only applies to one buffer. Why would someone who has this nice IP in the path want the conventional triggered buffer? I'm not against the two buffer option, but I'd like to know the reasoning not to just provide the hardware buffer if this SPI offload is available. I can conjecture reasons but would like you to write them out for me :) This feels like if someone has paid for the expensive hardware they probably only want the best performance. Jonathan > --- > drivers/iio/adc/ad7944.c | 147 +++++++++++++++++++++++++++++++++++------------ > 1 file changed, 111 insertions(+), 36 deletions(-) > > diff --git a/drivers/iio/adc/ad7944.c b/drivers/iio/adc/ad7944.c > index 4602ab5ed2a6..6724d6c92778 100644 > --- a/drivers/iio/adc/ad7944.c > +++ b/drivers/iio/adc/ad7944.c > @@ -9,6 +9,7 @@ > #include <linux/align.h> > #include <linux/bitfield.h> > #include <linux/bitops.h> > +#include <linux/clk.h> > #include <linux/delay.h> > #include <linux/device.h> > #include <linux/err.h> > @@ -21,6 +22,7 @@ > > #include <linux/iio/iio.h> > #include <linux/iio/sysfs.h> > +#include <linux/iio/buffer-dmaengine.h> > #include <linux/iio/trigger_consumer.h> > #include <linux/iio/triggered_buffer.h> > > @@ -65,6 +67,8 @@ struct ad7944_adc { > bool always_turbo; > /* Reference voltage (millivolts). */ > unsigned int ref_mv; > + /* Clock that triggers SPI offload. */ > + struct clk *trigger_clk; > > /* > * DMA (thus cache coherency maintenance) requires the > @@ -123,6 +127,7 @@ static const struct ad7944_chip_info _name##_chip_info = { \ > .scan_type.endianness = IIO_CPU, \ > .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) \ > | BIT(IIO_CHAN_INFO_SCALE), \ > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ),\ > }, \ > IIO_CHAN_SOFT_TIMESTAMP(1), \ > }, \ > @@ -134,18 +139,12 @@ AD7944_DEFINE_CHIP_INFO(ad7985, ad7944, 16, 0); > /* fully differential */ > AD7944_DEFINE_CHIP_INFO(ad7986, ad7986, 18, 1); > > -static void ad7944_unoptimize_msg(void *msg) > -{ > - spi_unoptimize_message(msg); > -} > - > -static int ad7944_3wire_cs_mode_init_msg(struct device *dev, struct ad7944_adc *adc, > - const struct iio_chan_spec *chan) > +static void ad7944_3wire_cs_mode_init_msg(struct device *dev, struct ad7944_adc *adc, > + const struct iio_chan_spec *chan) > { > unsigned int t_conv_ns = adc->always_turbo ? adc->timing_spec->turbo_conv_ns > : adc->timing_spec->conv_ns; > struct spi_transfer *xfers = adc->xfers; > - int ret; > > /* > * NB: can get better performance from some SPI controllers if we use > @@ -174,21 +173,14 @@ static int ad7944_3wire_cs_mode_init_msg(struct device *dev, struct ad7944_adc * > xfers[2].bits_per_word = chan->scan_type.realbits; > > spi_message_init_with_transfers(&adc->msg, xfers, 3); > - > - ret = spi_optimize_message(adc->spi, &adc->msg); > - if (ret) > - return ret; > - > - return devm_add_action_or_reset(dev, ad7944_unoptimize_msg, &adc->msg); > } > > -static int ad7944_4wire_mode_init_msg(struct device *dev, struct ad7944_adc *adc, > - const struct iio_chan_spec *chan) > +static void ad7944_4wire_mode_init_msg(struct device *dev, struct ad7944_adc *adc, > + const struct iio_chan_spec *chan) > { > unsigned int t_conv_ns = adc->always_turbo ? adc->timing_spec->turbo_conv_ns > : adc->timing_spec->conv_ns; > struct spi_transfer *xfers = adc->xfers; > - int ret; > > /* > * NB: can get better performance from some SPI controllers if we use > @@ -208,12 +200,6 @@ static int ad7944_4wire_mode_init_msg(struct device *dev, struct ad7944_adc *adc > xfers[1].bits_per_word = chan->scan_type.realbits; > > spi_message_init_with_transfers(&adc->msg, xfers, 2); > - > - ret = spi_optimize_message(adc->spi, &adc->msg); > - if (ret) > - return ret; > - > - return devm_add_action_or_reset(dev, ad7944_unoptimize_msg, &adc->msg); > } > > static int ad7944_chain_mode_init_msg(struct device *dev, struct ad7944_adc *adc, > @@ -345,6 +331,30 @@ static int ad7944_read_raw(struct iio_dev *indio_dev, > return -EINVAL; > } > > + case IIO_CHAN_INFO_SAMP_FREQ: > + if (!adc->trigger_clk) > + return -EOPNOTSUPP; > + > + *val = clk_get_rate(adc->trigger_clk); > + return IIO_VAL_INT; > + > + default: > + return -EINVAL; > + } > +} > + > +static int ad7944_write_raw(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > + int val, int val2, long info) > +{ > + struct ad7944_adc *adc = iio_priv(indio_dev); > + > + switch (info) { > + case IIO_CHAN_INFO_SAMP_FREQ: > + if (!adc->trigger_clk) > + return -EOPNOTSUPP; > + > + return clk_set_rate(adc->trigger_clk, val); > default: > return -EINVAL; > } > @@ -352,6 +362,28 @@ static int ad7944_read_raw(struct iio_dev *indio_dev, > > static const struct iio_info ad7944_iio_info = { > .read_raw = &ad7944_read_raw, > + .write_raw = &ad7944_write_raw, > +}; > + > +static int ad7944_offload_ex_buffer_postenable(struct iio_dev *indio_dev) > +{ > + struct ad7944_adc *adc = iio_priv(indio_dev); > + > + return spi_offload_hw_trigger_enable(adc->spi, 0); > +} > + > +static int ad7944_offload_ex_buffer_predisable(struct iio_dev *indio_dev) > +{ > + struct ad7944_adc *adc = iio_priv(indio_dev); > + > + spi_offload_hw_trigger_disable(adc->spi, 0); > + > + return 0; > +} > + > +static const struct iio_buffer_setup_ops ad7944_offload_ex_buffer_setup_ops = { > + .postenable = &ad7944_offload_ex_buffer_postenable, > + .predisable = &ad7944_offload_ex_buffer_predisable, > }; > > static irqreturn_t ad7944_trigger_handler(int irq, void *p) > @@ -471,6 +503,18 @@ static void ad7944_ref_disable(void *ref) > regulator_disable(ref); > } > > +static void ad7944_offload_unprepare(void *p) > +{ > + struct ad7944_adc *adc = p; > + > + spi_offload_unprepare(adc->spi, 0, &adc->msg); > +} > + > +static void ad7944_unoptimize_msg(void *msg) > +{ > + spi_unoptimize_message(msg); > +} > + > static int ad7944_probe(struct spi_device *spi) > { > const struct ad7944_chip_info *chip_info; > @@ -603,16 +647,10 @@ static int ad7944_probe(struct spi_device *spi) > > switch (adc->spi_mode) { > case AD7944_SPI_MODE_DEFAULT: > - ret = ad7944_4wire_mode_init_msg(dev, adc, &chip_info->channels[0]); > - if (ret) > - return ret; > - > + ad7944_4wire_mode_init_msg(dev, adc, &chip_info->channels[0]); > break; > case AD7944_SPI_MODE_SINGLE: > - ret = ad7944_3wire_cs_mode_init_msg(dev, adc, &chip_info->channels[0]); > - if (ret) > - return ret; > - > + ad7944_3wire_cs_mode_init_msg(dev, adc, &chip_info->channels[0]); > break; > case AD7944_SPI_MODE_CHAIN: > ret = device_property_read_u32(dev, "#daisy-chained-devices", > @@ -649,11 +687,48 @@ static int ad7944_probe(struct spi_device *spi) > indio_dev->num_channels = ARRAY_SIZE(chip_info->channels); > } > > - ret = devm_iio_triggered_buffer_setup(dev, indio_dev, > - iio_pollfunc_store_time, > - ad7944_trigger_handler, NULL); > - if (ret) > - return ret; > + if (device_property_present(dev, "spi-offloads")) { > + /* TODO: make this a parameter to ad7944_3wire_cs_mode_init_msg() */ > + /* FIXME: wrong index for 4-wire mode */ > + adc->xfers[2].rx_buf = NULL; > + adc->xfers[2].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM; > + > + ret = spi_offload_prepare(adc->spi, 0, &adc->msg); > + if (ret) > + return dev_err_probe(dev, ret, "failed to prepare offload\n"); > + > + ret = devm_add_action_or_reset(dev, ad7944_offload_unprepare, adc); > + if (ret) > + return ret; > + > + adc->trigger_clk = devm_clk_get_enabled(dev, "trigger"); > + if (IS_ERR(adc->trigger_clk)) > + return dev_err_probe(dev, PTR_ERR(adc->trigger_clk), > + "failed to get trigger clk\n"); > + > + ret = devm_iio_dmaengine_buffer_setup(dev, indio_dev, "rx"); > + if (ret) > + return ret; > + > + indio_dev->setup_ops = &ad7944_offload_ex_buffer_setup_ops; > + /* offload can't have soft timestamp */ > + indio_dev->num_channels--; > + } else { > + ret = spi_optimize_message(adc->spi, &adc->msg); > + if (ret) > + return ret; > + > + ret = devm_add_action_or_reset(dev, ad7944_unoptimize_msg, &adc->msg); > + if (ret) > + return ret; > + > + ret = devm_iio_triggered_buffer_setup(dev, indio_dev, > + iio_pollfunc_store_time, > + ad7944_trigger_handler, > + NULL); > + if (ret) > + return ret; > + } > > return devm_iio_device_register(dev, indio_dev); > } >
On Sat, May 11, 2024 at 11:58 AM Jonathan Cameron <jic23@kernel.org> wrote: > > On Fri, 10 May 2024 19:44:31 -0500 > David Lechner <dlechner@baylibre.com> wrote: > > > This adds support for SPI offload to the ad7944 driver. This allows > > reading data at the max sample rate of 2.5 MSPS. > > > > Signed-off-by: David Lechner <dlechner@baylibre.com> > > --- > > > > v2 changes: > > > > In the previous version, there was a new separate driver for the PWM > > trigger and DMA hardware buffer. This was deemed too complex so they > > are moved into the ad7944 driver. > > > > It has also been reworked to accommodate for the changes described in > > the other patches. > > > > RFC: This isn't very polished yet, just FYI. A few things to sort out: > > > > Rather than making the buffer either triggered buffer or hardware buffer, > > I'm considering allowing both, e.g. buffer0 will always be the triggered > > buffer and buffer1 will will be the hardware buffer if connected to a SPI > > controller with offload support, otherwise buffer1 is absent. But since > > multiple buffers haven't been used much so far, more investigation is > > needed to see how that would work in practice. If we do that though, then > > we would always have the sampling_frequency attribute though even though > > it only applies to one buffer. > > Why would someone who has this nice IP in the path want the conventional > triggered buffer? I'm not against the two buffer option, but I'd like to know > the reasoning not to just provide the hardware buffer if this SPI offload > is available. > > I can conjecture reasons but would like you to write them out for me :) > This feels like if someone has paid for the expensive hardware they probably > only want the best performance. > For me, it was more of a question of if we need to keep the userspace interface consistent between both with or without offload support. But if you are happy with it this way where we have only one or the other, it is less work for me. :-)
On Sat, 11 May 2024 13:41:09 -0500 David Lechner <dlechner@baylibre.com> wrote: > On Sat, May 11, 2024 at 11:58 AM Jonathan Cameron <jic23@kernel.org> wrote: > > > > On Fri, 10 May 2024 19:44:31 -0500 > > David Lechner <dlechner@baylibre.com> wrote: > > > > > This adds support for SPI offload to the ad7944 driver. This allows > > > reading data at the max sample rate of 2.5 MSPS. > > > > > > Signed-off-by: David Lechner <dlechner@baylibre.com> > > > --- > > > > > > v2 changes: > > > > > > In the previous version, there was a new separate driver for the PWM > > > trigger and DMA hardware buffer. This was deemed too complex so they > > > are moved into the ad7944 driver. > > > > > > It has also been reworked to accommodate for the changes described in > > > the other patches. > > > > > > RFC: This isn't very polished yet, just FYI. A few things to sort out: > > > > > > Rather than making the buffer either triggered buffer or hardware buffer, > > > I'm considering allowing both, e.g. buffer0 will always be the triggered > > > buffer and buffer1 will will be the hardware buffer if connected to a SPI > > > controller with offload support, otherwise buffer1 is absent. But since > > > multiple buffers haven't been used much so far, more investigation is > > > needed to see how that would work in practice. If we do that though, then > > > we would always have the sampling_frequency attribute though even though > > > it only applies to one buffer. > > > > Why would someone who has this nice IP in the path want the conventional > > triggered buffer? I'm not against the two buffer option, but I'd like to know > > the reasoning not to just provide the hardware buffer if this SPI offload > > is available. > > > > I can conjecture reasons but would like you to write them out for me :) > > This feels like if someone has paid for the expensive hardware they probably > > only want the best performance. > > > > For me, it was more of a question of if we need to keep the userspace > interface consistent between both with or without offload support. But > if you are happy with it this way where we have only one or the other, > it is less work for me. :-) So inconsistency in userspace interfaces can occur for many reasons like whether the interrupt is wired or not, but in this particularly case I guess we have ABI stability issue because there are boards out there today and people using the driver without this offload functionality. I'd not really thought that bit through, so I think you are correct that we need to maintain the triggered buffer interface and 'add' the new ABI for the offloaded case. The multibuffer approach should work for this. Will be interesting if any problem surface from having two very different types of buffer on the same device. Jonathan
On Sun, May 12, 2024 at 6:52 AM Jonathan Cameron <jic23@kernel.org> wrote: > > On Sat, 11 May 2024 13:41:09 -0500 > David Lechner <dlechner@baylibre.com> wrote: > > > On Sat, May 11, 2024 at 11:58 AM Jonathan Cameron <jic23@kernel.org> wrote: > > > > > > On Fri, 10 May 2024 19:44:31 -0500 > > > David Lechner <dlechner@baylibre.com> wrote: > > > > > > > This adds support for SPI offload to the ad7944 driver. This allows > > > > reading data at the max sample rate of 2.5 MSPS. > > > > > > > > Signed-off-by: David Lechner <dlechner@baylibre.com> > > > > --- > > > > > > > > v2 changes: > > > > > > > > In the previous version, there was a new separate driver for the PWM > > > > trigger and DMA hardware buffer. This was deemed too complex so they > > > > are moved into the ad7944 driver. > > > > > > > > It has also been reworked to accommodate for the changes described in > > > > the other patches. > > > > > > > > RFC: This isn't very polished yet, just FYI. A few things to sort out: > > > > > > > > Rather than making the buffer either triggered buffer or hardware buffer, > > > > I'm considering allowing both, e.g. buffer0 will always be the triggered > > > > buffer and buffer1 will will be the hardware buffer if connected to a SPI > > > > controller with offload support, otherwise buffer1 is absent. But since > > > > multiple buffers haven't been used much so far, more investigation is > > > > needed to see how that would work in practice. If we do that though, then > > > > we would always have the sampling_frequency attribute though even though > > > > it only applies to one buffer. > > > > > > Why would someone who has this nice IP in the path want the conventional > > > triggered buffer? I'm not against the two buffer option, but I'd like to know > > > the reasoning not to just provide the hardware buffer if this SPI offload > > > is available. > > > > > > I can conjecture reasons but would like you to write them out for me :) > > > This feels like if someone has paid for the expensive hardware they probably > > > only want the best performance. > > > > > > > For me, it was more of a question of if we need to keep the userspace > > interface consistent between both with or without offload support. But > > if you are happy with it this way where we have only one or the other, > > it is less work for me. :-) > > So inconsistency in userspace interfaces can occur for many reasons like > whether the interrupt is wired or not, but in this particularly > case I guess we have ABI stability issue because there are boards out there > today and people using the driver without this offload functionality. FWIW, the ad7944 driver will be landing in 6.10, so no users to speak of yet. > I'd not really thought that bit through, so I think you are correct that > we need to maintain the triggered buffer interface and 'add' the new > ABI for the offloaded case. The multibuffer approach should work for this. > Will be interesting if any problem surface from having two very different > types of buffer on the same device. > In this particular case, I think the issues will be: 1. The hardware buffer can't allow the soft timestamp. Otherwise, the buffer layout is the same (on other chips, we won't always be so lucky). 2. The hardware trigger (sampling_frequency attribute) will only apply if there is the SPI offload support.
diff --git a/drivers/iio/adc/ad7944.c b/drivers/iio/adc/ad7944.c index 4602ab5ed2a6..6724d6c92778 100644 --- a/drivers/iio/adc/ad7944.c +++ b/drivers/iio/adc/ad7944.c @@ -9,6 +9,7 @@ #include <linux/align.h> #include <linux/bitfield.h> #include <linux/bitops.h> +#include <linux/clk.h> #include <linux/delay.h> #include <linux/device.h> #include <linux/err.h> @@ -21,6 +22,7 @@ #include <linux/iio/iio.h> #include <linux/iio/sysfs.h> +#include <linux/iio/buffer-dmaengine.h> #include <linux/iio/trigger_consumer.h> #include <linux/iio/triggered_buffer.h> @@ -65,6 +67,8 @@ struct ad7944_adc { bool always_turbo; /* Reference voltage (millivolts). */ unsigned int ref_mv; + /* Clock that triggers SPI offload. */ + struct clk *trigger_clk; /* * DMA (thus cache coherency maintenance) requires the @@ -123,6 +127,7 @@ static const struct ad7944_chip_info _name##_chip_info = { \ .scan_type.endianness = IIO_CPU, \ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) \ | BIT(IIO_CHAN_INFO_SCALE), \ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ),\ }, \ IIO_CHAN_SOFT_TIMESTAMP(1), \ }, \ @@ -134,18 +139,12 @@ AD7944_DEFINE_CHIP_INFO(ad7985, ad7944, 16, 0); /* fully differential */ AD7944_DEFINE_CHIP_INFO(ad7986, ad7986, 18, 1); -static void ad7944_unoptimize_msg(void *msg) -{ - spi_unoptimize_message(msg); -} - -static int ad7944_3wire_cs_mode_init_msg(struct device *dev, struct ad7944_adc *adc, - const struct iio_chan_spec *chan) +static void ad7944_3wire_cs_mode_init_msg(struct device *dev, struct ad7944_adc *adc, + const struct iio_chan_spec *chan) { unsigned int t_conv_ns = adc->always_turbo ? adc->timing_spec->turbo_conv_ns : adc->timing_spec->conv_ns; struct spi_transfer *xfers = adc->xfers; - int ret; /* * NB: can get better performance from some SPI controllers if we use @@ -174,21 +173,14 @@ static int ad7944_3wire_cs_mode_init_msg(struct device *dev, struct ad7944_adc * xfers[2].bits_per_word = chan->scan_type.realbits; spi_message_init_with_transfers(&adc->msg, xfers, 3); - - ret = spi_optimize_message(adc->spi, &adc->msg); - if (ret) - return ret; - - return devm_add_action_or_reset(dev, ad7944_unoptimize_msg, &adc->msg); } -static int ad7944_4wire_mode_init_msg(struct device *dev, struct ad7944_adc *adc, - const struct iio_chan_spec *chan) +static void ad7944_4wire_mode_init_msg(struct device *dev, struct ad7944_adc *adc, + const struct iio_chan_spec *chan) { unsigned int t_conv_ns = adc->always_turbo ? adc->timing_spec->turbo_conv_ns : adc->timing_spec->conv_ns; struct spi_transfer *xfers = adc->xfers; - int ret; /* * NB: can get better performance from some SPI controllers if we use @@ -208,12 +200,6 @@ static int ad7944_4wire_mode_init_msg(struct device *dev, struct ad7944_adc *adc xfers[1].bits_per_word = chan->scan_type.realbits; spi_message_init_with_transfers(&adc->msg, xfers, 2); - - ret = spi_optimize_message(adc->spi, &adc->msg); - if (ret) - return ret; - - return devm_add_action_or_reset(dev, ad7944_unoptimize_msg, &adc->msg); } static int ad7944_chain_mode_init_msg(struct device *dev, struct ad7944_adc *adc, @@ -345,6 +331,30 @@ static int ad7944_read_raw(struct iio_dev *indio_dev, return -EINVAL; } + case IIO_CHAN_INFO_SAMP_FREQ: + if (!adc->trigger_clk) + return -EOPNOTSUPP; + + *val = clk_get_rate(adc->trigger_clk); + return IIO_VAL_INT; + + default: + return -EINVAL; + } +} + +static int ad7944_write_raw(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + int val, int val2, long info) +{ + struct ad7944_adc *adc = iio_priv(indio_dev); + + switch (info) { + case IIO_CHAN_INFO_SAMP_FREQ: + if (!adc->trigger_clk) + return -EOPNOTSUPP; + + return clk_set_rate(adc->trigger_clk, val); default: return -EINVAL; } @@ -352,6 +362,28 @@ static int ad7944_read_raw(struct iio_dev *indio_dev, static const struct iio_info ad7944_iio_info = { .read_raw = &ad7944_read_raw, + .write_raw = &ad7944_write_raw, +}; + +static int ad7944_offload_ex_buffer_postenable(struct iio_dev *indio_dev) +{ + struct ad7944_adc *adc = iio_priv(indio_dev); + + return spi_offload_hw_trigger_enable(adc->spi, 0); +} + +static int ad7944_offload_ex_buffer_predisable(struct iio_dev *indio_dev) +{ + struct ad7944_adc *adc = iio_priv(indio_dev); + + spi_offload_hw_trigger_disable(adc->spi, 0); + + return 0; +} + +static const struct iio_buffer_setup_ops ad7944_offload_ex_buffer_setup_ops = { + .postenable = &ad7944_offload_ex_buffer_postenable, + .predisable = &ad7944_offload_ex_buffer_predisable, }; static irqreturn_t ad7944_trigger_handler(int irq, void *p) @@ -471,6 +503,18 @@ static void ad7944_ref_disable(void *ref) regulator_disable(ref); } +static void ad7944_offload_unprepare(void *p) +{ + struct ad7944_adc *adc = p; + + spi_offload_unprepare(adc->spi, 0, &adc->msg); +} + +static void ad7944_unoptimize_msg(void *msg) +{ + spi_unoptimize_message(msg); +} + static int ad7944_probe(struct spi_device *spi) { const struct ad7944_chip_info *chip_info; @@ -603,16 +647,10 @@ static int ad7944_probe(struct spi_device *spi) switch (adc->spi_mode) { case AD7944_SPI_MODE_DEFAULT: - ret = ad7944_4wire_mode_init_msg(dev, adc, &chip_info->channels[0]); - if (ret) - return ret; - + ad7944_4wire_mode_init_msg(dev, adc, &chip_info->channels[0]); break; case AD7944_SPI_MODE_SINGLE: - ret = ad7944_3wire_cs_mode_init_msg(dev, adc, &chip_info->channels[0]); - if (ret) - return ret; - + ad7944_3wire_cs_mode_init_msg(dev, adc, &chip_info->channels[0]); break; case AD7944_SPI_MODE_CHAIN: ret = device_property_read_u32(dev, "#daisy-chained-devices", @@ -649,11 +687,48 @@ static int ad7944_probe(struct spi_device *spi) indio_dev->num_channels = ARRAY_SIZE(chip_info->channels); } - ret = devm_iio_triggered_buffer_setup(dev, indio_dev, - iio_pollfunc_store_time, - ad7944_trigger_handler, NULL); - if (ret) - return ret; + if (device_property_present(dev, "spi-offloads")) { + /* TODO: make this a parameter to ad7944_3wire_cs_mode_init_msg() */ + /* FIXME: wrong index for 4-wire mode */ + adc->xfers[2].rx_buf = NULL; + adc->xfers[2].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM; + + ret = spi_offload_prepare(adc->spi, 0, &adc->msg); + if (ret) + return dev_err_probe(dev, ret, "failed to prepare offload\n"); + + ret = devm_add_action_or_reset(dev, ad7944_offload_unprepare, adc); + if (ret) + return ret; + + adc->trigger_clk = devm_clk_get_enabled(dev, "trigger"); + if (IS_ERR(adc->trigger_clk)) + return dev_err_probe(dev, PTR_ERR(adc->trigger_clk), + "failed to get trigger clk\n"); + + ret = devm_iio_dmaengine_buffer_setup(dev, indio_dev, "rx"); + if (ret) + return ret; + + indio_dev->setup_ops = &ad7944_offload_ex_buffer_setup_ops; + /* offload can't have soft timestamp */ + indio_dev->num_channels--; + } else { + ret = spi_optimize_message(adc->spi, &adc->msg); + if (ret) + return ret; + + ret = devm_add_action_or_reset(dev, ad7944_unoptimize_msg, &adc->msg); + if (ret) + return ret; + + ret = devm_iio_triggered_buffer_setup(dev, indio_dev, + iio_pollfunc_store_time, + ad7944_trigger_handler, + NULL); + if (ret) + return ret; + } return devm_iio_device_register(dev, indio_dev); }
This adds support for SPI offload to the ad7944 driver. This allows reading data at the max sample rate of 2.5 MSPS. Signed-off-by: David Lechner <dlechner@baylibre.com> --- v2 changes: In the previous version, there was a new separate driver for the PWM trigger and DMA hardware buffer. This was deemed too complex so they are moved into the ad7944 driver. It has also been reworked to accommodate for the changes described in the other patches. RFC: This isn't very polished yet, just FYI. A few things to sort out: Rather than making the buffer either triggered buffer or hardware buffer, I'm considering allowing both, e.g. buffer0 will always be the triggered buffer and buffer1 will will be the hardware buffer if connected to a SPI controller with offload support, otherwise buffer1 is absent. But since multiple buffers haven't been used much so far, more investigation is needed to see how that would work in practice. If we do that though, then we would always have the sampling_frequency attribute though even though it only applies to one buffer. --- drivers/iio/adc/ad7944.c | 147 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 111 insertions(+), 36 deletions(-)