| Message ID | 20240109-axi-spi-engine-series-3-v1-12-e42c6a986580@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spi: axi-spi-engine: add offload support | expand |
On Wed, 2024-01-10 at 13:49 -0600, David Lechner wrote: > This adds a new driver for handling SPI offloading using a PWM as the > trigger and DMA for the received data. This will be used by ADCs in > conjunction with SPI controllers with offloading support to be able > to sample at high rates without CPU intervention. > > Signed-off-by: David Lechner <dlechner@baylibre.com> > --- > drivers/iio/Kconfig | 1 + > drivers/iio/Makefile | 1 + > .../iio/buffer/industrialio-hw-triggered-buffer.c | 1 + > drivers/iio/offload/Kconfig | 21 ++ > drivers/iio/offload/Makefile | 2 + > drivers/iio/offload/iio-pwm-triggered-dma-buffer.c | 212 > +++++++++++++++++++++ > 6 files changed, 238 insertions(+) > > diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig > index 52eb46ef84c1..56738282d82f 100644 > --- a/drivers/iio/Kconfig > +++ b/drivers/iio/Kconfig > @@ -90,6 +90,7 @@ source "drivers/iio/imu/Kconfig" > source "drivers/iio/light/Kconfig" > source "drivers/iio/magnetometer/Kconfig" > source "drivers/iio/multiplexer/Kconfig" > +source "drivers/iio/offload/Kconfig" The offload stuff is something very particular to a spi controller feature. Not sure if having this as a generic thing makes sense at this point. IMO, the IIO way of looking at the offload engine is as an HW triggered core for capturing data. Hence, I would support the whole thing as an HW triggered buffer. And, if we are really going the path of having the offload core as a platform device, we could have different compatibles for each pair of trigger + data_capture (or explicit dt properties). Just my 2 cents... - Nuno Sá
On Wed, 2024-01-10 at 13:49 -0600, David Lechner wrote: > This adds a new driver for handling SPI offloading using a PWM as the > trigger and DMA for the received data. This will be used by ADCs in > conjunction with SPI controllers with offloading support to be able > to sample at high rates without CPU intervention. > > Signed-off-by: David Lechner <dlechner@baylibre.com> > --- > drivers/iio/Kconfig | 1 + > drivers/iio/Makefile | 1 + > .../iio/buffer/industrialio-hw-triggered-buffer.c | 1 + > drivers/iio/offload/Kconfig | 21 ++ > drivers/iio/offload/Makefile | 2 + > drivers/iio/offload/iio-pwm-triggered-dma-buffer.c | 212 > +++++++++++++++++++++ > 6 files changed, 238 insertions(+) > > diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig > index 52eb46ef84c1..56738282d82f 100644 > --- a/drivers/iio/Kconfig > +++ b/drivers/iio/Kconfig > @@ -90,6 +90,7 @@ source "drivers/iio/imu/Kconfig" > source "drivers/iio/light/Kconfig" > source "drivers/iio/magnetometer/Kconfig" > source "drivers/iio/multiplexer/Kconfig" > +source "drivers/iio/offload/Kconfig" > source "drivers/iio/orientation/Kconfig" > source "drivers/iio/test/Kconfig" > if IIO_TRIGGER > diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile > index 9622347a1c1b..20acf5e1a4a7 100644 > --- a/drivers/iio/Makefile > +++ b/drivers/iio/Makefile > @@ -34,6 +34,7 @@ obj-y += imu/ > obj-y += light/ > obj-y += magnetometer/ > obj-y += multiplexer/ > +obj-y += offload/ > obj-y += orientation/ > obj-y += position/ > obj-y += potentiometer/ > diff --git a/drivers/iio/buffer/industrialio-hw-triggered-buffer.c > b/drivers/iio/buffer/industrialio-hw-triggered-buffer.c > index 7a8a71066b0e..a2fae6059616 100644 > --- a/drivers/iio/buffer/industrialio-hw-triggered-buffer.c > +++ b/drivers/iio/buffer/industrialio-hw-triggered-buffer.c > @@ -86,6 +86,7 @@ static int iio_hw_trigger_buffer_probe(struct > auxiliary_device *adev, > } > > static const struct auxiliary_device_id iio_hw_trigger_buffer_id_table[] = { > + { .name = "pwm-triggered-dma-buffer.triggered-buffer" }, > { } > }; > MODULE_DEVICE_TABLE(auxiliary, iio_hw_trigger_buffer_id_table); > diff --git a/drivers/iio/offload/Kconfig b/drivers/iio/offload/Kconfig > new file mode 100644 > index 000000000000..760c0cfe0e9c > --- /dev/null > +++ b/drivers/iio/offload/Kconfig > @@ -0,0 +1,21 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +# > +# SPI offload handlers for Industrial I/O > +# > +# When adding new entries keep the list in alphabetical order > + > +menu "SPI offload handlers" > + > +config IIO_PWM_TRIGGERED_DMA_BUFFER > + tristate "PWM trigger and DMA buffer connected to SPI offload" > + select AUXILIARY_BUS > + select IIO_BUFFER_DMAENGINE > + help > + Provides a periodic hardware trigger via a PWM connected to the > + trigger input of a SPI offload and a hardware buffer implemented > + via DMA connected to the data output stream the a SPI offload. > + > + To compile this driver as a module, choose M here: the > + module will be called "iio-pwm-triggered-dma-buffer". > + > +endmenu > diff --git a/drivers/iio/offload/Makefile b/drivers/iio/offload/Makefile > new file mode 100644 > index 000000000000..7300ce82f066 > --- /dev/null > +++ b/drivers/iio/offload/Makefile > @@ -0,0 +1,2 @@ > + > +obj-$(CONFIG_IIO_PWM_TRIGGERED_DMA_BUFFER) := iio-pwm-triggered-dma-buffer.o > diff --git a/drivers/iio/offload/iio-pwm-triggered-dma-buffer.c > b/drivers/iio/offload/iio-pwm-triggered-dma-buffer.c > new file mode 100644 > index 000000000000..970ea82316f6 > --- /dev/null > +++ b/drivers/iio/offload/iio-pwm-triggered-dma-buffer.c > @@ -0,0 +1,212 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Platform driver for a PWM trigger and DMA buffer connected to a SPI > + * controller offload instance implementing the iio-hw-triggered-buffer > + * interface. > + * > + * Copyright (C) 2023 Analog Devices, Inc. > + * Copyright (C) 2023 BayLibre, SAS > + */ > + > +#include <linux/auxiliary_bus.h> > +#include <linux/pwm.h> > +#include <linux/module.h> > +#include <linux/mod_devicetable.h> > +#include <linux/platform_device.h> > +#include <linux/iio/buffer.h> > +#include <linux/iio/hw_triggered_buffer_impl.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/trigger.h> > +#include <linux/iio/sysfs.h> > +#include <linux/iio/buffer-dmaengine.h> > +#include <linux/sysfs.h> > + > +struct iio_pwm_triggered_dma_buffer { > + struct iio_hw_triggered_buffer_device hw; > + struct pwm_device *pwm; > +}; > + > +static const struct iio_trigger_ops iio_pwm_triggered_dma_buffer_ops; > + > +static int iio_pwm_triggered_dma_buffer_set_state(struct iio_trigger *trig, > bool state) > +{ > + struct iio_pwm_triggered_dma_buffer *st = > iio_trigger_get_drvdata(trig); > + > + if (state) > + return pwm_enable(st->pwm); > + > + pwm_disable(st->pwm); > + > + return 0; > +} > + > +static int iio_pwm_triggered_dma_buffer_validate_device(struct iio_trigger > *trig, > + struct iio_dev > *indio_dev) > +{ > + /* Don't allow assigning trigger via sysfs. */ > + return -EINVAL; > +} > + > +static const struct iio_trigger_ops iio_pwm_triggered_dma_buffer_ops = { > + .set_trigger_state = iio_pwm_triggered_dma_buffer_set_state, > + .validate_device = iio_pwm_triggered_dma_buffer_validate_device, > +}; > + > +static u32 axi_spi_engine_offload_pwm_trigger_get_rate(struct iio_trigger > *trig) > +{ > + struct iio_pwm_triggered_dma_buffer *st = > iio_trigger_get_drvdata(trig); > + u64 period_ns = pwm_get_period(st->pwm); > + > + if (period_ns) > + return DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC, period_ns); > + > + return 0; > +} > + > +static int > +axi_spi_engine_offload_set_samp_freq(struct iio_pwm_triggered_dma_buffer *st, > + u32 requested_hz) > +{ > + int period_ns; > + > + if (requested_hz == 0) > + return -EINVAL; > + > + period_ns = DIV_ROUND_UP(NSEC_PER_SEC, requested_hz); > + > + /* > + * FIXME: We really just need a clock, not a PWM. The current duty > cycle > + * value is a hack to work around the edge vs. level offload trigger > + * issue in the ADI AXI SPI Engine firmware. > + */ > + return pwm_config(st->pwm, 10, period_ns); > +} > + > +static ssize_t sampling_frequency_show(struct device *dev, > + struct device_attribute *attr, char > *buf) > +{ > + struct iio_trigger *trig = to_iio_trigger(dev); > + > + return sysfs_emit(buf, "%u\n", > + axi_spi_engine_offload_pwm_trigger_get_rate(trig)); > +} > + > +static ssize_t sampling_frequency_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t len) > +{ > + struct iio_trigger *trig = to_iio_trigger(dev); > + struct iio_pwm_triggered_dma_buffer *st = > iio_trigger_get_drvdata(trig); > + int ret; > + u32 val; > + > + ret = kstrtou32(buf, 10, &val); > + if (ret) > + return ret; > + > + ret = axi_spi_engine_offload_set_samp_freq(st, val); > + if (ret) > + return ret; > + > + return len; > +} > + This is also something that made me wonder... In the end of the day, the frequency at which we want to sample the data depends on the converter device. This completely detached interface makes it more easy for the user to screw up things, right? Things might even become more complicated in usecases where we have an additional pwm (on top of the data fetch trigger one) for triggering conversions in the converter and we might need to properly control the phase between those two signals. So, how would we handle it in the current form? We have one pwm belonging to the offload engine and one belonging to the converter but they need to cope together... I'm aware you have some alternative ideas for not using pwms but the series is using pwms... And the above usecase is real and it's sitting out of tree waiting for the offload stuff to get merged so I can upstream that driver :) - Nuno Sá
On Wed, 10 Jan 2024 13:49:53 -0600 David Lechner <dlechner@baylibre.com> wrote: > This adds a new driver for handling SPI offloading using a PWM as the > trigger and DMA for the received data. This will be used by ADCs in > conjunction with SPI controllers with offloading support to be able > to sample at high rates without CPU intervention. > > Signed-off-by: David Lechner <dlechner@baylibre.com> Ah. So if I read this right, the trigger only exists to provide somewhere to hang the frequency control? Not sure it's worth the complexity for that. Do we expect a given offload engine to allow a bunch of 'standard' triggers? If that's the case then I don't mind them being exposed as triggers. We do that for a few other devices where we need to pick between a bunch of different internal signals and it works fine. > + > +static int iio_pwm_triggered_dma_buffer_probe(struct platform_device *pdev) > +{ > + struct iio_pwm_triggered_dma_buffer *st; > + struct auxiliary_device *adev; > + int ret; > + > + st = devm_kzalloc(&pdev->dev, sizeof(*st), GFP_KERNEL); > + if (!st) > + return -ENOMEM; > + > + st->pwm = devm_pwm_get(&pdev->dev, NULL); > + if (IS_ERR(st->pwm)) > + return dev_err_probe(&pdev->dev, PTR_ERR(st->pwm), > + "failed to get PWM\n"); > + > + st->hw.buffer = devm_iio_dmaengine_buffer_alloc(&pdev->dev, "rx"); > + if (IS_ERR(st->hw.buffer)) > + return dev_err_probe(&pdev->dev, PTR_ERR(st->hw.buffer), > + "failed to allocate buffer\n"); > + > + st->hw.trig = devm_iio_trigger_alloc(&pdev->dev, "%s-%s-pwm-trigger", > + dev_name(pdev->dev.parent), > + dev_name(&pdev->dev)); > + if (!st->hw.trig) > + return -ENOMEM; > + > + st->hw.trig->ops = &iio_pwm_triggered_dma_buffer_ops; > + st->hw.trig->dev.parent = &pdev->dev; > + st->hw.trig->dev.groups = iio_pwm_triggered_dma_buffer_groups; > + iio_trigger_set_drvdata(st->hw.trig, st); > + > + /* start with a reasonable default value */ > + ret = axi_spi_engine_offload_set_samp_freq(st, 1000); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, > + "failed to set sampling frequency\n"); > + > + ret = devm_iio_trigger_register(&pdev->dev, st->hw.trig); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, > + "failed to register trigger\n"); > + > + adev = &st->hw.adev; > + adev->name = "triggered-buffer"; > + adev->dev.parent = &pdev->dev; > + adev->dev.release = iio_pwm_triggered_dma_buffer_adev_release; > + adev->id = 0; > + > + ret = auxiliary_device_init(adev); > + if (ret) > + return ret; > + > + ret = auxiliary_device_add(adev); > + if (ret) { > + auxiliary_device_uninit(adev); > + return ret; > + } > + > + return devm_add_action_or_reset(&pdev->dev, > + iio_pwm_triggered_dma_buffer_unregister_adev, adev); Split this an register the uninit and delete as separate callbacks to so we can clearly see what each is doing. > +}
On Thu, 11 Jan 2024 15:59:02 +0100 Nuno Sá <noname.nuno@gmail.com> wrote: > On Wed, 2024-01-10 at 13:49 -0600, David Lechner wrote: > > This adds a new driver for handling SPI offloading using a PWM as the > > trigger and DMA for the received data. This will be used by ADCs in > > conjunction with SPI controllers with offloading support to be able > > to sample at high rates without CPU intervention. > > > > Signed-off-by: David Lechner <dlechner@baylibre.com> > > --- > > drivers/iio/Kconfig | 1 + > > drivers/iio/Makefile | 1 + > > .../iio/buffer/industrialio-hw-triggered-buffer.c | 1 + > > drivers/iio/offload/Kconfig | 21 ++ > > drivers/iio/offload/Makefile | 2 + > > drivers/iio/offload/iio-pwm-triggered-dma-buffer.c | 212 > > +++++++++++++++++++++ > > 6 files changed, 238 insertions(+) > > > > diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig > > index 52eb46ef84c1..56738282d82f 100644 > > --- a/drivers/iio/Kconfig > > +++ b/drivers/iio/Kconfig > > @@ -90,6 +90,7 @@ source "drivers/iio/imu/Kconfig" > > source "drivers/iio/light/Kconfig" > > source "drivers/iio/magnetometer/Kconfig" > > source "drivers/iio/multiplexer/Kconfig" > > +source "drivers/iio/offload/Kconfig" > > source "drivers/iio/orientation/Kconfig" > > source "drivers/iio/test/Kconfig" > > if IIO_TRIGGER > > diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile > > index 9622347a1c1b..20acf5e1a4a7 100644 > > --- a/drivers/iio/Makefile > > +++ b/drivers/iio/Makefile > > @@ -34,6 +34,7 @@ obj-y += imu/ > > obj-y += light/ > > obj-y += magnetometer/ > > obj-y += multiplexer/ > > +obj-y += offload/ > > obj-y += orientation/ > > obj-y += position/ > > obj-y += potentiometer/ > > diff --git a/drivers/iio/buffer/industrialio-hw-triggered-buffer.c > > b/drivers/iio/buffer/industrialio-hw-triggered-buffer.c > > index 7a8a71066b0e..a2fae6059616 100644 > > --- a/drivers/iio/buffer/industrialio-hw-triggered-buffer.c > > +++ b/drivers/iio/buffer/industrialio-hw-triggered-buffer.c > > @@ -86,6 +86,7 @@ static int iio_hw_trigger_buffer_probe(struct > > auxiliary_device *adev, > > } > > > > static const struct auxiliary_device_id iio_hw_trigger_buffer_id_table[] = { > > + { .name = "pwm-triggered-dma-buffer.triggered-buffer" }, > > { } > > }; > > MODULE_DEVICE_TABLE(auxiliary, iio_hw_trigger_buffer_id_table); > > diff --git a/drivers/iio/offload/Kconfig b/drivers/iio/offload/Kconfig > > new file mode 100644 > > index 000000000000..760c0cfe0e9c > > --- /dev/null > > +++ b/drivers/iio/offload/Kconfig > > @@ -0,0 +1,21 @@ > > +# SPDX-License-Identifier: GPL-2.0-only > > +# > > +# SPI offload handlers for Industrial I/O > > +# > > +# When adding new entries keep the list in alphabetical order > > + > > +menu "SPI offload handlers" > > + > > +config IIO_PWM_TRIGGERED_DMA_BUFFER > > + tristate "PWM trigger and DMA buffer connected to SPI offload" > > + select AUXILIARY_BUS > > + select IIO_BUFFER_DMAENGINE > > + help > > + Provides a periodic hardware trigger via a PWM connected to the > > + trigger input of a SPI offload and a hardware buffer implemented > > + via DMA connected to the data output stream the a SPI offload. > > + > > + To compile this driver as a module, choose M here: the > > + module will be called "iio-pwm-triggered-dma-buffer". > > + > > +endmenu > > diff --git a/drivers/iio/offload/Makefile b/drivers/iio/offload/Makefile > > new file mode 100644 > > index 000000000000..7300ce82f066 > > --- /dev/null > > +++ b/drivers/iio/offload/Makefile > > @@ -0,0 +1,2 @@ > > + > > +obj-$(CONFIG_IIO_PWM_TRIGGERED_DMA_BUFFER) := iio-pwm-triggered-dma-buffer.o > > diff --git a/drivers/iio/offload/iio-pwm-triggered-dma-buffer.c > > b/drivers/iio/offload/iio-pwm-triggered-dma-buffer.c > > new file mode 100644 > > index 000000000000..970ea82316f6 > > --- /dev/null > > +++ b/drivers/iio/offload/iio-pwm-triggered-dma-buffer.c > > @@ -0,0 +1,212 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Platform driver for a PWM trigger and DMA buffer connected to a SPI > > + * controller offload instance implementing the iio-hw-triggered-buffer > > + * interface. > > + * > > + * Copyright (C) 2023 Analog Devices, Inc. > > + * Copyright (C) 2023 BayLibre, SAS > > + */ > > + > > +#include <linux/auxiliary_bus.h> > > +#include <linux/pwm.h> > > +#include <linux/module.h> > > +#include <linux/mod_devicetable.h> > > +#include <linux/platform_device.h> > > +#include <linux/iio/buffer.h> > > +#include <linux/iio/hw_triggered_buffer_impl.h> > > +#include <linux/iio/iio.h> > > +#include <linux/iio/trigger.h> > > +#include <linux/iio/sysfs.h> > > +#include <linux/iio/buffer-dmaengine.h> > > +#include <linux/sysfs.h> > > + > > +struct iio_pwm_triggered_dma_buffer { > > + struct iio_hw_triggered_buffer_device hw; > > + struct pwm_device *pwm; > > +}; > > + > > +static const struct iio_trigger_ops iio_pwm_triggered_dma_buffer_ops; > > + > > +static int iio_pwm_triggered_dma_buffer_set_state(struct iio_trigger *trig, > > bool state) > > +{ > > + struct iio_pwm_triggered_dma_buffer *st = > > iio_trigger_get_drvdata(trig); > > + > > + if (state) > > + return pwm_enable(st->pwm); > > + > > + pwm_disable(st->pwm); > > + > > + return 0; > > +} > > + > > +static int iio_pwm_triggered_dma_buffer_validate_device(struct iio_trigger > > *trig, > > + struct iio_dev > > *indio_dev) > > +{ > > + /* Don't allow assigning trigger via sysfs. */ > > + return -EINVAL; > > +} > > + > > +static const struct iio_trigger_ops iio_pwm_triggered_dma_buffer_ops = { > > + .set_trigger_state = iio_pwm_triggered_dma_buffer_set_state, > > + .validate_device = iio_pwm_triggered_dma_buffer_validate_device, > > +}; > > + > > +static u32 axi_spi_engine_offload_pwm_trigger_get_rate(struct iio_trigger > > *trig) > > +{ > > + struct iio_pwm_triggered_dma_buffer *st = > > iio_trigger_get_drvdata(trig); > > + u64 period_ns = pwm_get_period(st->pwm); > > + > > + if (period_ns) > > + return DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC, period_ns); > > + > > + return 0; > > +} > > + > > +static int > > +axi_spi_engine_offload_set_samp_freq(struct iio_pwm_triggered_dma_buffer *st, > > + u32 requested_hz) > > +{ > > + int period_ns; > > + > > + if (requested_hz == 0) > > + return -EINVAL; > > + > > + period_ns = DIV_ROUND_UP(NSEC_PER_SEC, requested_hz); > > + > > + /* > > + * FIXME: We really just need a clock, not a PWM. The current duty > > cycle > > + * value is a hack to work around the edge vs. level offload trigger > > + * issue in the ADI AXI SPI Engine firmware. > > + */ > > + return pwm_config(st->pwm, 10, period_ns); > > +} > > + > > +static ssize_t sampling_frequency_show(struct device *dev, > > + struct device_attribute *attr, char > > *buf) > > +{ > > + struct iio_trigger *trig = to_iio_trigger(dev); > > + > > + return sysfs_emit(buf, "%u\n", > > + axi_spi_engine_offload_pwm_trigger_get_rate(trig)); > > +} > > + > > +static ssize_t sampling_frequency_store(struct device *dev, > > + struct device_attribute *attr, > > + const char *buf, size_t len) > > +{ > > + struct iio_trigger *trig = to_iio_trigger(dev); > > + struct iio_pwm_triggered_dma_buffer *st = > > iio_trigger_get_drvdata(trig); > > + int ret; > > + u32 val; > > + > > + ret = kstrtou32(buf, 10, &val); > > + if (ret) > > + return ret; > > + > > + ret = axi_spi_engine_offload_set_samp_freq(st, val); > > + if (ret) > > + return ret; > > + > > + return len; > > +} > > + > > This is also something that made me wonder... In the end of the day, the > frequency at which we want to sample the data depends on the converter device. > This completely detached interface makes it more easy for the user to screw up > things, right? It's easy to do that anyway :) If you think of a normal SPI attached device that has it's own internal clocking then it's usually easy to set the device to grab data faster than than the spi bus can drain it. We drop samples. Here it might make sense to add some bounds I guess as it's all in hardware - either have the ADC provide them or push it to DT, > > Things might even become more complicated in usecases where we have an > additional pwm (on top of the data fetch trigger one) for triggering conversions > in the converter and we might need to properly control the phase between those > two signals. So, how would we handle it in the current form? We have one pwm > belonging to the offload engine and one belonging to the converter but they need > to cope together... Groan loudly? If someone wants careful sync between a trigger for the ADC and the read back triggering they should do it in hardware. Sure we can do it if the pwm is sophisticated enough but the mess of layering etc needed to make it work is just nasty. I guess you make a custom trigger that bakes in your requirements. > > I'm aware you have some alternative ideas for not using pwms but the series is > using pwms... And the above usecase is real and it's sitting out of tree waiting > for the offload stuff to get merged so I can upstream that driver :) Gah. So much for hoping you were conjecturing something unusual. Jonathan > > - Nuno Sá > > >
On Fri, 2024-01-12 at 12:51 +0000, Jonathan Cameron wrote: > On Thu, 11 Jan 2024 15:59:02 +0100 > Nuno Sá <noname.nuno@gmail.com> wrote: > > > On Wed, 2024-01-10 at 13:49 -0600, David Lechner wrote: > > > This adds a new driver for handling SPI offloading using a PWM as the > > > trigger and DMA for the received data. This will be used by ADCs in > > > conjunction with SPI controllers with offloading support to be able > > > to sample at high rates without CPU intervention. > > > > > > Signed-off-by: David Lechner <dlechner@baylibre.com> > > > --- > > > drivers/iio/Kconfig | 1 + > > > drivers/iio/Makefile | 1 + > > > .../iio/buffer/industrialio-hw-triggered-buffer.c | 1 + > > > drivers/iio/offload/Kconfig | 21 ++ > > > drivers/iio/offload/Makefile | 2 + > > > drivers/iio/offload/iio-pwm-triggered-dma-buffer.c | 212 > > > +++++++++++++++++++++ > > > 6 files changed, 238 insertions(+) > > > > > > diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig > > > index 52eb46ef84c1..56738282d82f 100644 > > > --- a/drivers/iio/Kconfig > > > +++ b/drivers/iio/Kconfig > > > @@ -90,6 +90,7 @@ source "drivers/iio/imu/Kconfig" > > > source "drivers/iio/light/Kconfig" > > > source "drivers/iio/magnetometer/Kconfig" > > > source "drivers/iio/multiplexer/Kconfig" > > > +source "drivers/iio/offload/Kconfig" > > > source "drivers/iio/orientation/Kconfig" > > > source "drivers/iio/test/Kconfig" > > > if IIO_TRIGGER > > > diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile > > > index 9622347a1c1b..20acf5e1a4a7 100644 > > > --- a/drivers/iio/Makefile > > > +++ b/drivers/iio/Makefile > > > @@ -34,6 +34,7 @@ obj-y += imu/ > > > obj-y += light/ > > > obj-y += magnetometer/ > > > obj-y += multiplexer/ > > > +obj-y += offload/ > > > obj-y += orientation/ > > > obj-y += position/ > > > obj-y += potentiometer/ > > > diff --git a/drivers/iio/buffer/industrialio-hw-triggered-buffer.c > > > b/drivers/iio/buffer/industrialio-hw-triggered-buffer.c > > > index 7a8a71066b0e..a2fae6059616 100644 > > > --- a/drivers/iio/buffer/industrialio-hw-triggered-buffer.c > > > +++ b/drivers/iio/buffer/industrialio-hw-triggered-buffer.c > > > @@ -86,6 +86,7 @@ static int iio_hw_trigger_buffer_probe(struct > > > auxiliary_device *adev, > > > } > > > > > > static const struct auxiliary_device_id iio_hw_trigger_buffer_id_table[] = { > > > + { .name = "pwm-triggered-dma-buffer.triggered-buffer" }, > > > { } > > > }; > > > MODULE_DEVICE_TABLE(auxiliary, iio_hw_trigger_buffer_id_table); > > > diff --git a/drivers/iio/offload/Kconfig b/drivers/iio/offload/Kconfig > > > new file mode 100644 > > > index 000000000000..760c0cfe0e9c > > > --- /dev/null > > > +++ b/drivers/iio/offload/Kconfig > > > @@ -0,0 +1,21 @@ > > > +# SPDX-License-Identifier: GPL-2.0-only > > > +# > > > +# SPI offload handlers for Industrial I/O > > > +# > > > +# When adding new entries keep the list in alphabetical order > > > + > > > +menu "SPI offload handlers" > > > + > > > +config IIO_PWM_TRIGGERED_DMA_BUFFER > > > + tristate "PWM trigger and DMA buffer connected to SPI offload" > > > + select AUXILIARY_BUS > > > + select IIO_BUFFER_DMAENGINE > > > + help > > > + Provides a periodic hardware trigger via a PWM connected to the > > > + trigger input of a SPI offload and a hardware buffer implemented > > > + via DMA connected to the data output stream the a SPI offload. > > > + > > > + To compile this driver as a module, choose M here: the > > > + module will be called "iio-pwm-triggered-dma-buffer". > > > + > > > +endmenu > > > diff --git a/drivers/iio/offload/Makefile b/drivers/iio/offload/Makefile > > > new file mode 100644 > > > index 000000000000..7300ce82f066 > > > --- /dev/null > > > +++ b/drivers/iio/offload/Makefile > > > @@ -0,0 +1,2 @@ > > > + > > > +obj-$(CONFIG_IIO_PWM_TRIGGERED_DMA_BUFFER) := iio-pwm-triggered-dma-buffer.o > > > diff --git a/drivers/iio/offload/iio-pwm-triggered-dma-buffer.c > > > b/drivers/iio/offload/iio-pwm-triggered-dma-buffer.c > > > new file mode 100644 > > > index 000000000000..970ea82316f6 > > > --- /dev/null > > > +++ b/drivers/iio/offload/iio-pwm-triggered-dma-buffer.c > > > @@ -0,0 +1,212 @@ > > > +// SPDX-License-Identifier: GPL-2.0-only > > > +/* > > > + * Platform driver for a PWM trigger and DMA buffer connected to a SPI > > > + * controller offload instance implementing the iio-hw-triggered-buffer > > > + * interface. > > > + * > > > + * Copyright (C) 2023 Analog Devices, Inc. > > > + * Copyright (C) 2023 BayLibre, SAS > > > + */ > > > + > > > +#include <linux/auxiliary_bus.h> > > > +#include <linux/pwm.h> > > > +#include <linux/module.h> > > > +#include <linux/mod_devicetable.h> > > > +#include <linux/platform_device.h> > > > +#include <linux/iio/buffer.h> > > > +#include <linux/iio/hw_triggered_buffer_impl.h> > > > +#include <linux/iio/iio.h> > > > +#include <linux/iio/trigger.h> > > > +#include <linux/iio/sysfs.h> > > > +#include <linux/iio/buffer-dmaengine.h> > > > +#include <linux/sysfs.h> > > > + > > > +struct iio_pwm_triggered_dma_buffer { > > > + struct iio_hw_triggered_buffer_device hw; > > > + struct pwm_device *pwm; > > > +}; > > > + > > > +static const struct iio_trigger_ops iio_pwm_triggered_dma_buffer_ops; > > > + > > > +static int iio_pwm_triggered_dma_buffer_set_state(struct iio_trigger *trig, > > > bool state) > > > +{ > > > + struct iio_pwm_triggered_dma_buffer *st = > > > iio_trigger_get_drvdata(trig); > > > + > > > + if (state) > > > + return pwm_enable(st->pwm); > > > + > > > + pwm_disable(st->pwm); > > > + > > > + return 0; > > > +} > > > + > > > +static int iio_pwm_triggered_dma_buffer_validate_device(struct iio_trigger > > > *trig, > > > + struct iio_dev > > > *indio_dev) > > > +{ > > > + /* Don't allow assigning trigger via sysfs. */ > > > + return -EINVAL; > > > +} > > > + > > > +static const struct iio_trigger_ops iio_pwm_triggered_dma_buffer_ops = { > > > + .set_trigger_state = iio_pwm_triggered_dma_buffer_set_state, > > > + .validate_device = iio_pwm_triggered_dma_buffer_validate_device, > > > +}; > > > + > > > +static u32 axi_spi_engine_offload_pwm_trigger_get_rate(struct iio_trigger > > > *trig) > > > +{ > > > + struct iio_pwm_triggered_dma_buffer *st = > > > iio_trigger_get_drvdata(trig); > > > + u64 period_ns = pwm_get_period(st->pwm); > > > + > > > + if (period_ns) > > > + return DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC, period_ns); > > > + > > > + return 0; > > > +} > > > + > > > +static int > > > +axi_spi_engine_offload_set_samp_freq(struct iio_pwm_triggered_dma_buffer *st, > > > + u32 requested_hz) > > > +{ > > > + int period_ns; > > > + > > > + if (requested_hz == 0) > > > + return -EINVAL; > > > + > > > + period_ns = DIV_ROUND_UP(NSEC_PER_SEC, requested_hz); > > > + > > > + /* > > > + * FIXME: We really just need a clock, not a PWM. The current duty > > > cycle > > > + * value is a hack to work around the edge vs. level offload trigger > > > + * issue in the ADI AXI SPI Engine firmware. > > > + */ > > > + return pwm_config(st->pwm, 10, period_ns); > > > +} > > > + > > > +static ssize_t sampling_frequency_show(struct device *dev, > > > + struct device_attribute *attr, char > > > *buf) > > > +{ > > > + struct iio_trigger *trig = to_iio_trigger(dev); > > > + > > > + return sysfs_emit(buf, "%u\n", > > > + axi_spi_engine_offload_pwm_trigger_get_rate(trig)); > > > +} > > > + > > > +static ssize_t sampling_frequency_store(struct device *dev, > > > + struct device_attribute *attr, > > > + const char *buf, size_t len) > > > +{ > > > + struct iio_trigger *trig = to_iio_trigger(dev); > > > + struct iio_pwm_triggered_dma_buffer *st = > > > iio_trigger_get_drvdata(trig); > > > + int ret; > > > + u32 val; > > > + > > > + ret = kstrtou32(buf, 10, &val); > > > + if (ret) > > > + return ret; > > > + > > > + ret = axi_spi_engine_offload_set_samp_freq(st, val); > > > + if (ret) > > > + return ret; > > > + > > > + return len; > > > +} > > > + > > > > This is also something that made me wonder... In the end of the day, the > > frequency at which we want to sample the data depends on the converter device. > > This completely detached interface makes it more easy for the user to screw up > > things, right? > It's easy to do that anyway :) If you think of a normal SPI attached device that > has it's own internal clocking then it's usually easy to set the device to grab > data faster than than the spi bus can drain it. We drop samples. > Here it might make sense to add some bounds I guess as it's all in hardware > - either have the ADC provide them or push it to DT, Indeed... DT would likely be simpler > > > > > Things might even become more complicated in usecases where we have an > > additional pwm (on top of the data fetch trigger one) for triggering conversions > > in the converter and we might need to properly control the phase between those > > two signals. So, how would we handle it in the current form? We have one pwm > > belonging to the offload engine and one belonging to the converter but they need > > to cope together... > > Groan loudly? > If someone wants careful sync between a trigger for the ADC and the read back > triggering > they should do it in hardware. Sure we can do it if the pwm is sophisticated enough > but the mess of layering etc needed to make it work is just nasty. > > I guess you make a custom trigger that bakes in your requirements. > Yeah, we have yet another FPGA soft core implementing a pwm controller (also to be upstreamed soon) which can output multiple signals at different phases, frequency, etc... - Nuno Sá
diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig index 52eb46ef84c1..56738282d82f 100644 --- a/drivers/iio/Kconfig +++ b/drivers/iio/Kconfig @@ -90,6 +90,7 @@ source "drivers/iio/imu/Kconfig" source "drivers/iio/light/Kconfig" source "drivers/iio/magnetometer/Kconfig" source "drivers/iio/multiplexer/Kconfig" +source "drivers/iio/offload/Kconfig" source "drivers/iio/orientation/Kconfig" source "drivers/iio/test/Kconfig" if IIO_TRIGGER diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile index 9622347a1c1b..20acf5e1a4a7 100644 --- a/drivers/iio/Makefile +++ b/drivers/iio/Makefile @@ -34,6 +34,7 @@ obj-y += imu/ obj-y += light/ obj-y += magnetometer/ obj-y += multiplexer/ +obj-y += offload/ obj-y += orientation/ obj-y += position/ obj-y += potentiometer/ diff --git a/drivers/iio/buffer/industrialio-hw-triggered-buffer.c b/drivers/iio/buffer/industrialio-hw-triggered-buffer.c index 7a8a71066b0e..a2fae6059616 100644 --- a/drivers/iio/buffer/industrialio-hw-triggered-buffer.c +++ b/drivers/iio/buffer/industrialio-hw-triggered-buffer.c @@ -86,6 +86,7 @@ static int iio_hw_trigger_buffer_probe(struct auxiliary_device *adev, } static const struct auxiliary_device_id iio_hw_trigger_buffer_id_table[] = { + { .name = "pwm-triggered-dma-buffer.triggered-buffer" }, { } }; MODULE_DEVICE_TABLE(auxiliary, iio_hw_trigger_buffer_id_table); diff --git a/drivers/iio/offload/Kconfig b/drivers/iio/offload/Kconfig new file mode 100644 index 000000000000..760c0cfe0e9c --- /dev/null +++ b/drivers/iio/offload/Kconfig @@ -0,0 +1,21 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +# SPI offload handlers for Industrial I/O +# +# When adding new entries keep the list in alphabetical order + +menu "SPI offload handlers" + +config IIO_PWM_TRIGGERED_DMA_BUFFER + tristate "PWM trigger and DMA buffer connected to SPI offload" + select AUXILIARY_BUS + select IIO_BUFFER_DMAENGINE + help + Provides a periodic hardware trigger via a PWM connected to the + trigger input of a SPI offload and a hardware buffer implemented + via DMA connected to the data output stream the a SPI offload. + + To compile this driver as a module, choose M here: the + module will be called "iio-pwm-triggered-dma-buffer". + +endmenu diff --git a/drivers/iio/offload/Makefile b/drivers/iio/offload/Makefile new file mode 100644 index 000000000000..7300ce82f066 --- /dev/null +++ b/drivers/iio/offload/Makefile @@ -0,0 +1,2 @@ + +obj-$(CONFIG_IIO_PWM_TRIGGERED_DMA_BUFFER) := iio-pwm-triggered-dma-buffer.o diff --git a/drivers/iio/offload/iio-pwm-triggered-dma-buffer.c b/drivers/iio/offload/iio-pwm-triggered-dma-buffer.c new file mode 100644 index 000000000000..970ea82316f6 --- /dev/null +++ b/drivers/iio/offload/iio-pwm-triggered-dma-buffer.c @@ -0,0 +1,212 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Platform driver for a PWM trigger and DMA buffer connected to a SPI + * controller offload instance implementing the iio-hw-triggered-buffer + * interface. + * + * Copyright (C) 2023 Analog Devices, Inc. + * Copyright (C) 2023 BayLibre, SAS + */ + +#include <linux/auxiliary_bus.h> +#include <linux/pwm.h> +#include <linux/module.h> +#include <linux/mod_devicetable.h> +#include <linux/platform_device.h> +#include <linux/iio/buffer.h> +#include <linux/iio/hw_triggered_buffer_impl.h> +#include <linux/iio/iio.h> +#include <linux/iio/trigger.h> +#include <linux/iio/sysfs.h> +#include <linux/iio/buffer-dmaengine.h> +#include <linux/sysfs.h> + +struct iio_pwm_triggered_dma_buffer { + struct iio_hw_triggered_buffer_device hw; + struct pwm_device *pwm; +}; + +static const struct iio_trigger_ops iio_pwm_triggered_dma_buffer_ops; + +static int iio_pwm_triggered_dma_buffer_set_state(struct iio_trigger *trig, bool state) +{ + struct iio_pwm_triggered_dma_buffer *st = iio_trigger_get_drvdata(trig); + + if (state) + return pwm_enable(st->pwm); + + pwm_disable(st->pwm); + + return 0; +} + +static int iio_pwm_triggered_dma_buffer_validate_device(struct iio_trigger *trig, + struct iio_dev *indio_dev) +{ + /* Don't allow assigning trigger via sysfs. */ + return -EINVAL; +} + +static const struct iio_trigger_ops iio_pwm_triggered_dma_buffer_ops = { + .set_trigger_state = iio_pwm_triggered_dma_buffer_set_state, + .validate_device = iio_pwm_triggered_dma_buffer_validate_device, +}; + +static u32 axi_spi_engine_offload_pwm_trigger_get_rate(struct iio_trigger *trig) +{ + struct iio_pwm_triggered_dma_buffer *st = iio_trigger_get_drvdata(trig); + u64 period_ns = pwm_get_period(st->pwm); + + if (period_ns) + return DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC, period_ns); + + return 0; +} + +static int +axi_spi_engine_offload_set_samp_freq(struct iio_pwm_triggered_dma_buffer *st, + u32 requested_hz) +{ + int period_ns; + + if (requested_hz == 0) + return -EINVAL; + + period_ns = DIV_ROUND_UP(NSEC_PER_SEC, requested_hz); + + /* + * FIXME: We really just need a clock, not a PWM. The current duty cycle + * value is a hack to work around the edge vs. level offload trigger + * issue in the ADI AXI SPI Engine firmware. + */ + return pwm_config(st->pwm, 10, period_ns); +} + +static ssize_t sampling_frequency_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct iio_trigger *trig = to_iio_trigger(dev); + + return sysfs_emit(buf, "%u\n", + axi_spi_engine_offload_pwm_trigger_get_rate(trig)); +} + +static ssize_t sampling_frequency_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t len) +{ + struct iio_trigger *trig = to_iio_trigger(dev); + struct iio_pwm_triggered_dma_buffer *st = iio_trigger_get_drvdata(trig); + int ret; + u32 val; + + ret = kstrtou32(buf, 10, &val); + if (ret) + return ret; + + ret = axi_spi_engine_offload_set_samp_freq(st, val); + if (ret) + return ret; + + return len; +} + +static DEVICE_ATTR_RW(sampling_frequency); + +static struct attribute *iio_pwm_triggered_dma_buffer_attrs[] = { + &dev_attr_sampling_frequency.attr, + NULL +}; + +ATTRIBUTE_GROUPS(iio_pwm_triggered_dma_buffer); + +static void iio_pwm_triggered_dma_buffer_adev_release(struct device *dev) +{ +} + +static void iio_pwm_triggered_dma_buffer_unregister_adev(void *adev) +{ + auxiliary_device_delete(adev); + auxiliary_device_uninit(adev); +} + +static int iio_pwm_triggered_dma_buffer_probe(struct platform_device *pdev) +{ + struct iio_pwm_triggered_dma_buffer *st; + struct auxiliary_device *adev; + int ret; + + st = devm_kzalloc(&pdev->dev, sizeof(*st), GFP_KERNEL); + if (!st) + return -ENOMEM; + + st->pwm = devm_pwm_get(&pdev->dev, NULL); + if (IS_ERR(st->pwm)) + return dev_err_probe(&pdev->dev, PTR_ERR(st->pwm), + "failed to get PWM\n"); + + st->hw.buffer = devm_iio_dmaengine_buffer_alloc(&pdev->dev, "rx"); + if (IS_ERR(st->hw.buffer)) + return dev_err_probe(&pdev->dev, PTR_ERR(st->hw.buffer), + "failed to allocate buffer\n"); + + st->hw.trig = devm_iio_trigger_alloc(&pdev->dev, "%s-%s-pwm-trigger", + dev_name(pdev->dev.parent), + dev_name(&pdev->dev)); + if (!st->hw.trig) + return -ENOMEM; + + st->hw.trig->ops = &iio_pwm_triggered_dma_buffer_ops; + st->hw.trig->dev.parent = &pdev->dev; + st->hw.trig->dev.groups = iio_pwm_triggered_dma_buffer_groups; + iio_trigger_set_drvdata(st->hw.trig, st); + + /* start with a reasonable default value */ + ret = axi_spi_engine_offload_set_samp_freq(st, 1000); + if (ret) + return dev_err_probe(&pdev->dev, ret, + "failed to set sampling frequency\n"); + + ret = devm_iio_trigger_register(&pdev->dev, st->hw.trig); + if (ret) + return dev_err_probe(&pdev->dev, ret, + "failed to register trigger\n"); + + adev = &st->hw.adev; + adev->name = "triggered-buffer"; + adev->dev.parent = &pdev->dev; + adev->dev.release = iio_pwm_triggered_dma_buffer_adev_release; + adev->id = 0; + + ret = auxiliary_device_init(adev); + if (ret) + return ret; + + ret = auxiliary_device_add(adev); + if (ret) { + auxiliary_device_uninit(adev); + return ret; + } + + return devm_add_action_or_reset(&pdev->dev, + iio_pwm_triggered_dma_buffer_unregister_adev, adev); +} + +static const struct of_device_id iio_pwm_triggered_dma_buffer_match_table[] = { + { .compatible = "adi,spi-offload-pwm-trigger-dma-buffer" }, + { } +}; +MODULE_DEVICE_TABLE(of, iio_pwm_triggered_dma_buffer_match_table); + +static struct platform_driver iio_pwm_triggered_dma_buffer_driver = { + .probe = iio_pwm_triggered_dma_buffer_probe, + .driver = { + .name = "iio-pwm-triggered-dma-buffer", + .of_match_table = iio_pwm_triggered_dma_buffer_match_table, + }, +}; +module_platform_driver(iio_pwm_triggered_dma_buffer_driver); + +MODULE_AUTHOR("David Lechner <dlechner@baylibre.com>"); +MODULE_DESCRIPTION("AXI SPI Engine Offload PWM Trigger"); +MODULE_LICENSE("GPL");
This adds a new driver for handling SPI offloading using a PWM as the trigger and DMA for the received data. This will be used by ADCs in conjunction with SPI controllers with offloading support to be able to sample at high rates without CPU intervention. Signed-off-by: David Lechner <dlechner@baylibre.com> --- drivers/iio/Kconfig | 1 + drivers/iio/Makefile | 1 + .../iio/buffer/industrialio-hw-triggered-buffer.c | 1 + drivers/iio/offload/Kconfig | 21 ++ drivers/iio/offload/Makefile | 2 + drivers/iio/offload/iio-pwm-triggered-dma-buffer.c | 212 +++++++++++++++++++++ 6 files changed, 238 insertions(+)