This is the culmination of the previous AXI SPI Engine improvement
series [1] and [2] where we made fixes and improvements to prepare
for adding offload support.
Simply put, "offload" support is defined as a capability of advanced
SPI controllers to record a series of SPI transactions and then play
them back using a hardware trigger. This allows operations to be
performed, possibly repeating many times, without any CPU intervention.
The offload hardware interface consists of a trigger input and a data
output for the RX data. These are connected to other hardware external
to the SPI controller.
To record one or more transactions, commands and TX data are written
to FIFOs on the controller (RX buffer is not used since RX data gets
piped to external hardware). This sequence of transactions can then be
played back when the trigger input is asserted.
This series includes core SPI support along with the first SPI
controller (AXI SPI Engine) and SPI peripheral (AD7380 ADC) that use
them. This enables capturing analog data at 2 million samples per second
with virtually no jitter.
The hardware setup looks like this:
+-------------------------------+ +------------------+
| | | |
| SOC/FPGA | | AD7380 ADC |
| +---------------------+ | | |
| | AXI SPI Engine | | | |
| | SDO/SDI/SCK/CS ============ SDI/SDO/SCK/CS |
| | | | | |
| | +---------------+ | | | |
| | | Offload 0 | | | +------------------+
| | | RX DATA OUT > > > > |
| | | TRIGGER IN < < < v |
| | +---------------+ | ^ v |
| +---------------------+ ^ v |
| | AXI PWM | ^ v |
| | CH0 > ^ v |
| +---------------------+ v |
| | AXI DMA | v |
| | CH0 < < < |
| +---------------------+ |
| |
+-------------------------------+
This series adds support in three phases.
1. Adding support in the SPI subsystem.
This is broken down into two parts.
1. Adding offload support to the SPI core.
* "spi: add core support for controllers with offload capabilities"
2. Implementing the new offload interface in the AXI SPI Engine
controller driver.
Prerequisites to avoid errors with new DT bindings:
* "scripts: dtc: checks: don't warn on SPI non-peripheral child
nodes"
* "spi: do not attempt to register DT nodes without @ in name"
DT bindings and corresponding driver changes:
* "spi: dt-bindings: adi,axi-spi-engine: add offload bindings"
* "spi: axi-spi-engine: add SPI offload support"
RFC question for this part: I have made the device tree bindings
specific to the controller. Would it be better to make them generic
SPI bindings since offload is intended to be a generic SPI feature?
Or should we require each controller to have its own bindings?
2. Adding a new offload hardware trigger buffer driver in the IIO
subsystem.
Since offloads are generic, we need to specify what is attached to
the trigger input and the data output. This is modeled as a platform
device that uses a compatible string to specify what is connected.
In this case, we have a PWM that is used to periodically trigger
the offload to read a sample from the ADC. The received data is then
piped to a DMA channel that transfers it to an IIO buffer.
This is broken down into two parts.
1. Adding a generic interface/helper functions for hardware
triggered hardware buffers.
* "iio: buffer: add hardware triggered buffer support"
* "iio: buffer: dmaengine: add INDIO_HW_BUFFER_TRIGGERED flag"
* "iio: buffer: add new hardware triggered buffer driver"
2. Adding a specific implementation of this interface for this
particular hardware configuration.
Prerequisites for new driver:
* "bus: auxiliary: increase AUXILIARY_NAME_SIZE"
* "iio: buffer: dmaengine: export devm_iio_dmaengine_buffer_alloc()"
DT bindings and corresponding new driver:
* "dt-bindings: iio: offload: add binding for PWM/DMA triggered
buffer"
* "iio: offload: add new PWM triggered DMA buffer driver"
3. Adding offload support to the AD7380 ADC driver.
Once the two components above are in place, we can add offload
support to the AD7380 ADC driver.
* "iio: adc: ad7380: add SPI offload support"
Build and runtime dependencies:
* The "spi: axi-spi-engine:" patch depends on the previous to series
[1] and [2] to apply cleanly (these have been applied to spi/for-6.8).
* The "iio: adc: ad7380:" patch depends on the driver introduced in [3]
(accepted but not applied yet).
* The "iio: buffer:" patches have a runtime dependency on [4] to
function properly.
* A branch with all dependencies and additional patches for a full
working system can be found at [5].
[1]: https://lore.kernel.org/linux-spi/[email protected]
[2]: https://lore.kernel.org/linux-spi/[email protected]
[3]: https://lore.kernel.org/linux-iio/[email protected]
[4]: https://lore.kernel.org/linux-iio/[email protected]
[5]: https://github.com/analogdevicesinc/linux/tree/dlech/spi-engine-offload-ad7980
---
David Lechner (13):
spi: add core support for controllers with offload capabilities
scripts: dtc: checks: don't warn on SPI non-peripheral child nodes
spi: do not attempt to register DT nodes without @ in name
spi: dt-bindings: adi,axi-spi-engine: add offload bindings
spi: axi-spi-engine: add SPI offload support
iio: buffer: add hardware triggered buffer support
iio: buffer: dmaengine: add INDIO_HW_BUFFER_TRIGGERED flag
iio: buffer: add new hardware triggered buffer driver
bus: auxiliary: increase AUXILIARY_NAME_SIZE
iio: buffer: dmaengine: export devm_iio_dmaengine_buffer_alloc()
dt-bindings: iio: offload: add binding for PWM/DMA triggered buffer
iio: offload: add new PWM triggered DMA buffer driver
iio: adc: ad7380: add SPI offload support
.../adi,spi-offload-pwm-trigger-dma-buffer.yaml | 59 +++++
.../spi/adi,axi-spi-engine-peripheral-props.yaml | 24 ++
.../bindings/spi/adi,axi-spi-engine.yaml | 49 +++-
.../bindings/spi/spi-peripheral-props.yaml | 1 +
Documentation/driver-api/driver-model/devres.rst | 2 +
drivers/iio/Kconfig | 1 +
drivers/iio/Makefile | 1 +
drivers/iio/adc/Kconfig | 1 +
drivers/iio/adc/ad7380.c | 84 ++++++-
drivers/iio/buffer/Kconfig | 7 +
drivers/iio/buffer/Makefile | 1 +
drivers/iio/buffer/industrialio-buffer-dmaengine.c | 5 +-
.../iio/buffer/industrialio-hw-triggered-buffer.c | 105 ++++++++
drivers/iio/industrialio-buffer.c | 43 +++-
drivers/iio/offload/Kconfig | 21 ++
drivers/iio/offload/Makefile | 2 +
drivers/iio/offload/iio-pwm-triggered-dma-buffer.c | 212 ++++++++++++++++
drivers/spi/spi-axi-spi-engine.c | 270 +++++++++++++++++++++
drivers/spi/spi.c | 43 +++-
include/linux/iio/buffer-dmaengine.h | 2 +
include/linux/iio/hw_triggered_buffer.h | 14 ++
include/linux/iio/hw_triggered_buffer_impl.h | 16 ++
include/linux/iio/iio.h | 16 +-
include/linux/mod_devicetable.h | 2 +-
include/linux/spi/spi.h | 123 ++++++++++
scripts/dtc/checks.c | 4 +
26 files changed, 1092 insertions(+), 16 deletions(-)
---
base-commit: 036589475658287626a9bb78bcb8538a33d3ed34
prerequisite-patch-id: a57defd70c3f6e806bdff12d940a1c1d3f1ec78f
change-id: 20231215-axi-spi-engine-series-3-1c6a584d408d
According to the spi-controller.yaml bindings, SPI peripheral child
nodes match the pattern "^.*@[0-9a-f]+$".
A SPI controller binding may require a child object node that is not a
peripheral. For example, the adi,axi-spi-engine binding requires an
"offloads" child node that is not a peripheral but rather a part of the
controller itself.
By checking for '@' in the node name, we can avoids a warnings like:
Warning (spi_bus_reg): /example-0/spi@44a00000/offloads: missing or empty reg property
for a binding like:
spi {
...
offloads {
offload@0 {
...
};
...
};
peripheral@0 {
...
};
};
Signed-off-by: David Lechner <[email protected]>
---
scripts/dtc/checks.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/scripts/dtc/checks.c b/scripts/dtc/checks.c
index 9f31d2607182..5af68642f231 100644
--- a/scripts/dtc/checks.c
+++ b/scripts/dtc/checks.c
@@ -1144,6 +1144,10 @@ static void check_spi_bus_reg(struct check *c, struct dt_info *dti, struct node
if (!node->parent || (node->parent->bus != &spi_bus))
return;
+ /* only nodes with '@' in name are SPI devices */
+ if (!strchr(unitname, '@'))
+ return;
+
if (get_property(node->parent, "spi-slave"))
return;
--
2.43.0
This adds a feature for specialized SPI controllers that can record
a series of SPI transfers, including tx data, cs assertions, delays,
etc. and then play them back using a hardware trigger without CPU
intervention.
The intended use case for this is with the AXI SPI Engine to capture
data from ADCs at high rates (MSPS) with a stable sample period.
Most of the implementation is controller-specific and will be handled by
drivers that implement the offload_ops callbacks. The API follows a
prepare/enable pattern that should be familiar to users of the clk
subsystem.
Consumers of this API will make calls similar to this:
/* in probe() */
offload = spi_offload_get(spi, 0);
...
/* in some setup function */
ret = spi_offload_prepare(offload, xfers, ARRAY_SIZE(xfers));
...
/* in some enable function */
ret = spi_offload_enable(offload);
...
/* in corresponding disable function */
spi_offload_disable(offload);
...
/* in corresponding teardown function */
spi_offload_unprepare(offload);
...
Signed-off-by: David Lechner <[email protected]>
---
drivers/spi/spi.c | 39 +++++++++++++++
include/linux/spi/spi.h | 123 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 162 insertions(+)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index a4b8c07c5951..f1d66b5d5491 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -3057,6 +3057,13 @@ static int spi_controller_check_ops(struct spi_controller *ctlr)
}
}
+ if (ctlr->offload_ops && !(ctlr->offload_ops->get &&
+ ctlr->offload_ops->prepare &&
+ ctlr->offload_ops->unprepare &&
+ ctlr->offload_ops->enable &&
+ ctlr->offload_ops->disable))
+ return -EINVAL;
+
return 0;
}
@@ -4448,6 +4455,38 @@ int spi_write_then_read(struct spi_device *spi,
}
EXPORT_SYMBOL_GPL(spi_write_then_read);
+/**
+ * spi_offload_prepare - prepare offload hardware for a transfer
+ * @offload: The offload instance.
+ * @spi: The spi device to use for the transfers.
+ * @xfers: The transfers to be executed.
+ * @num_xfers: The number of transfers.
+ *
+ * Records a series of transfers to be executed later by the offload hardware
+ * trigger.
+ *
+ * Return: 0 on success, else a negative error code.
+ */
+int spi_offload_prepare(struct spi_offload *offload, struct spi_device *spi,
+ struct spi_transfer *xfers, unsigned int num_xfers)
+{
+ struct spi_controller *ctlr = offload->controller;
+ struct spi_message msg;
+ int ret;
+
+ spi_message_init_with_transfers(&msg, xfers, num_xfers);
+
+ ret = __spi_validate(spi, &msg);
+ if (ret)
+ return ret;
+
+ msg.spi = spi;
+ ret = ctlr->offload_ops->prepare(offload, &msg);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(spi_offload_prepare);
+
/*-------------------------------------------------------------------------*/
#if IS_ENABLED(CONFIG_OF_DYNAMIC)
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 5d65a6273dcf..f116dfc1d52c 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -28,6 +28,8 @@ struct spi_transfer;
struct spi_controller_mem_ops;
struct spi_controller_mem_caps;
struct spi_message;
+struct spi_controller_offload_ops;
+struct spi_offload;
/*
* INTERFACES between SPI master-side drivers and SPI slave protocol handlers,
@@ -713,6 +715,9 @@ struct spi_controller {
const struct spi_controller_mem_ops *mem_ops;
const struct spi_controller_mem_caps *mem_caps;
+ /* Operations for controllers with offload support. */
+ const struct spi_controller_offload_ops *offload_ops;
+
/* GPIO chip select */
struct gpio_desc **cs_gpiods;
bool use_gpio_descriptors;
@@ -1505,6 +1510,124 @@ static inline ssize_t spi_w8r16be(struct spi_device *spi, u8 cmd)
/*---------------------------------------------------------------------------*/
+/*
+ * Offloading support.
+ *
+ * Some SPI controllers support offloading of SPI transfers. Essentially,
+ * this allows the SPI controller to record SPI transfers and then play them
+ * back later via a hardware trigger.
+ */
+
+/**
+ * SPI_OFFLOAD_RX - placeholder for indicating read transfers for offloads
+ *
+ * Assign xfer->rx_buf to this value for any read transfer passed to
+ * spi_offload_prepare(). This will act as a flag to indicate to the offload
+ * that it should do something with the data read during this transfer. What
+ * that something can be is determined by the specific hardware, e.g. it could
+ * be piped to DMA or a DSP, etc.
+ */
+#define SPI_OFFLOAD_RX_SENTINEL ((void *)1)
+
+/**
+ * struct spi_controller_offload_ops - callbacks for offload support
+ *
+ * Drivers for hardware with offload support need to implement all of these
+ * callbacks.
+ */
+struct spi_controller_offload_ops {
+ /**
+ * @get: Callback to get the offload assigned to the given SPI device.
+ * Index is an index in the offloads array fwnode property of the device.
+ * Implementations must return the pointer to the device or a negative
+ * error code (return -ENODEV rather than NULL if no matching device).
+ */
+ struct spi_offload *(*get)(struct spi_device *spi, unsigned int index);
+ /**
+ * @prepare: Callback to prepare the offload for the given SPI message.
+ * @msg and any of its members (including any xfer->tx_buf) is not
+ * guaranteed to be valid beyond the lifetime of this call.
+ */
+ int (*prepare)(struct spi_offload *offload, struct spi_message *msg);
+ /**
+ * @unprepare: Callback to release any resources used by prepare().
+ */
+ void (*unprepare)(struct spi_offload *offload);
+ /**
+ * @enable: Callback to enable the offload.
+ */
+ int (*enable)(struct spi_offload *offload);
+ /**
+ * @disable: Callback to disable the offload.
+ */
+ void (*disable)(struct spi_offload *offload);
+};
+
+/** struct spi_offload - offload handle */
+struct spi_offload {
+ /** @controller: The associated SPI controller. */
+ struct spi_controller *controller;
+ /** @dev: The device associated with the offload instance. */
+ struct device *dev;
+ /** @priv: Private instance data used by the SPI controller. */
+ void *priv;
+};
+
+/**
+ * spi_offload_get - gets an offload assigned to the given SPI device
+ * @spi: SPI device.
+ * @index: Index of the offload in the SPI device's fwnode int array.
+ *
+ * The lifetime of the returned offload is tied to the struct spi_controller
+ * instance. Since @spi owns a reference to the controller, most consumers
+ * should not have to do anything extra. But if the offload is passed somewhere
+ * outside of the control of the SPI device driver, then an additional reference
+ * to the controller must be made.
+ *
+ * Return: Pointer to the offload handle or negative error code.
+ */
+static inline struct spi_offload *spi_offload_get(struct spi_device *spi,
+ unsigned int index)
+{
+ if (!spi->controller->offload_ops)
+ return ERR_PTR(-EOPNOTSUPP);
+
+ return spi->controller->offload_ops->get(spi, index);
+}
+
+int spi_offload_prepare(struct spi_offload *offload, struct spi_device *spi,
+ struct spi_transfer *xfers, unsigned int num_xfers);
+
+/**
+ * spi_offload_unprepare - releases any resources used by spi_offload_prepare()
+ * @offload: The offload instance.
+ */
+static inline void spi_offload_unprepare(struct spi_offload *offload)
+{
+ offload->controller->offload_ops->unprepare(offload);
+}
+
+/**
+ * spi_offload_enable - enables the offload
+ * @offload: The offload instance.
+ * Return: 0 on success or negative error code.
+ */
+static inline int spi_offload_enable(struct spi_offload *offload)
+{
+ return offload->controller->offload_ops->enable(offload);
+}
+
+/**
+ * spi_offload_disable - disables the offload
+ * @offload: The offload instance.
+ */
+static inline void spi_offload_disable(struct spi_offload *offload)
+{
+ offload->controller->offload_ops->disable(offload);
+}
+
+/*---------------------------------------------------------------------------*/
+
/*
* INTERFACE between board init code and SPI infrastructure.
*
--
2.43.0
In the DT bindings for SPI devices, it is specified that peripheral
nodes have the @ character in the node name. A SPI controller may need
to create bindings with child nodes that are not peripherals. For
example, the AXI SPI Engine bindings will use an "offloads" child node
to describe what is connected to the offload interfaces of the SPI
controller.
Without this change, the SPI controller would attempt to register all
child nodes as SPI devices. After this change, only nodes with '@' in
the name will be registered as SPI devices.
Signed-off-by: David Lechner <[email protected]>
---
drivers/spi/spi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index f1d66b5d5491..5be5e654284c 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2379,7 +2379,9 @@ static void of_register_spi_devices(struct spi_controller *ctlr)
struct device_node *nc;
for_each_available_child_of_node(ctlr->dev.of_node, nc) {
- if (of_node_test_and_set_flag(nc, OF_POPULATED))
+ /* Only nodes with '@' in the name are peripheral nodes. */
+ if (of_node_test_and_set_flag(nc, OF_POPULATED) ||
+ !strchr(kbasename(nc->full_name), '@'))
continue;
spi = of_register_spi_device(ctlr, nc);
if (IS_ERR(spi)) {
--
2.43.0
The ADI AXI SPI Engine driver supports offloading SPI transfers to
hardware. This is essentially a feature that allows recording an
arbitrary sequence of SPI transfers and then playing them back with
no CPU intervention via a hardware trigger.
This adds the bindings for this feature. Each SPI Engine instance
can have from 0 to 32 offload instances. Each offload instance has a
trigger input and a data stream output. As an example, this could be
used with an ADC SPI peripheral. In this case the trigger is connected
to a PWM/clock to determine the sampling rate for the ADC and the output
stream is connected to a DMA channel to pipe the sample data to memory.
SPI peripherals act as consumers of the offload instances. Typically,
one SPI peripheral will be connected to one offload instance. But to
make the bindings future-proof, the property is an array.
Signed-off-by: David Lechner <[email protected]>
---
.../spi/adi,axi-spi-engine-peripheral-props.yaml | 24 +++++++++++
.../bindings/spi/adi,axi-spi-engine.yaml | 49 +++++++++++++++++++++-
.../bindings/spi/spi-peripheral-props.yaml | 1 +
3 files changed, 73 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/spi/adi,axi-spi-engine-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine-peripheral-props.yaml
new file mode 100644
index 000000000000..19b685fc3b39
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine-peripheral-props.yaml
@@ -0,0 +1,24 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/adi,axi-spi-engine-peripheral-props.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Peripheral properties for Analog Devices AXI SPI Engine Controller
+
+maintainers:
+ - Michael Hennerich <[email protected]>
+ - Nuno Sá <[email protected]>
+
+properties:
+ adi,offloads:
+ description:
+ List of AXI SPI Engine offload instances assigned to this peripheral.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ maxItems: 32
+ items:
+ items:
+ - minimum: 0
+ maximum: 31
+
+additionalProperties: true
diff --git a/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
index d48faa42d025..69f3261bab47 100644
--- a/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
+++ b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
@@ -21,6 +21,23 @@ maintainers:
allOf:
- $ref: /schemas/spi/spi-controller.yaml#
+$defs:
+ offload:
+ description:
+ Describes the connections of the trigger input and the data output stream
+ of one or more offload instances.
+
+ properties:
+ reg:
+ description:
+ Index of the offload instance.
+ items:
+ - minimum: 0
+ maximum: 31
+
+ required:
+ - reg
+
properties:
compatible:
const: adi,axi-spi-engine-1.00.a
@@ -41,6 +58,22 @@ properties:
- const: s_axi_aclk
- const: spi_clk
+ offloads:
+ type: object
+ description: Zero or more offloads supported by the controller.
+
+ properties:
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ patternProperties:
+ "^offload@[0-8a-f]+$":
+ type: object
+ $ref: '#/$defs/offload'
+
required:
- compatible
- reg
@@ -62,5 +95,19 @@ examples:
#address-cells = <1>;
#size-cells = <0>;
- /* SPI devices */
+ offloads {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ offload@0 {
+ compatible = "adi,example-offload";
+ reg = <0>;
+ };
+ };
+
+ adc@0 {
+ compatible = "adi,example-adc";
+ reg = <0>;
+ adi,offloads = <0>;
+ };
};
diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
index 1c8e71c18234..7beb5a3798a5 100644
--- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
@@ -132,6 +132,7 @@ properties:
# The controller specific properties go here.
allOf:
+ - $ref: adi,axi-spi-engine-peripheral-props.yaml#
- $ref: arm,pl022-peripheral-props.yaml#
- $ref: cdns,qspi-nor-peripheral-props.yaml#
- $ref: samsung,spi-peripheral-props.yaml#
--
2.43.0
This adds the new INDIO_HW_BUFFER_TRIGGERED flag to the available modes
of the dmaengine buffer. This allows it to be used as the buffer of
devices that use the INDIO_HW_BUFFER_TRIGGERED flag.
Signed-off-by: David Lechner <[email protected]>
---
drivers/iio/buffer/industrialio-buffer-dmaengine.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
index 5f85ba38e6f6..c67ddf963bfb 100644
--- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
+++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
@@ -120,7 +120,7 @@ static const struct iio_buffer_access_funcs iio_dmaengine_buffer_ops = {
.data_available = iio_dma_buffer_data_available,
.release = iio_dmaengine_buffer_release,
- .modes = INDIO_BUFFER_HARDWARE,
+ .modes = INDIO_BUFFER_HARDWARE | INDIO_HW_BUFFER_TRIGGERED,
.flags = INDIO_BUFFER_FLAG_FIXED_WATERMARK,
};
--
2.43.0
This adds a new mode INDIO_HW_BUFFER_TRIGGERED to the IIO subsystem.
This mode is essentially the hardware version of INDIO_BUFFER_TRIGGERED
where the trigger has the semantics of INDIO_HARDWARE_TRIGGERED and the
buffer has the semantics of INDIO_BUFFER_HARDWARE.
So basically INDIO_HW_BUFFER_TRIGGERED is the same as
INDIO_BUFFER_HARDWARE except that it also enables the trigger when the
buffer is enabled.
Signed-off-by: David Lechner <[email protected]>
---
drivers/iio/industrialio-buffer.c | 43 ++++++++++++++++++++++++++++++++++++---
include/linux/iio/iio.h | 16 ++++++++++++---
2 files changed, 53 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 176d31d9f9d8..ffee3043c65a 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -27,6 +27,7 @@
#include <linux/iio/sysfs.h>
#include <linux/iio/buffer.h>
#include <linux/iio/buffer_impl.h>
+#include <linux/iio/trigger.h>
static const char * const iio_endian_prefix[] = {
[IIO_BE] = "be",
@@ -867,8 +868,17 @@ static int iio_verify_update(struct iio_dev *indio_dev,
insert_buffer->watermark);
}
- /* Definitely possible for devices to support both of these. */
- if ((modes & INDIO_BUFFER_TRIGGERED) && indio_dev->trig) {
+ /* Definitely possible for devices to support all of these. */
+ if (modes & INDIO_HW_BUFFER_TRIGGERED) {
+ /*
+ * Keep things simple for now and only allow a single buffer to
+ * be connected in hardware mode.
+ */
+ if (insert_buffer && !list_empty(&iio_dev_opaque->buffer_list))
+ return -EINVAL;
+ config->mode = INDIO_HW_BUFFER_TRIGGERED;
+ strict_scanmask = true;
+ } else if ((modes & INDIO_BUFFER_TRIGGERED) && indio_dev->trig) {
config->mode = INDIO_BUFFER_TRIGGERED;
} else if (modes & INDIO_BUFFER_HARDWARE) {
/*
@@ -1107,11 +1117,21 @@ static int iio_enable_buffers(struct iio_dev *indio_dev,
}
}
+ if (iio_dev_opaque->currentmode == INDIO_HW_BUFFER_TRIGGERED) {
+ struct iio_trigger *trig = indio_dev->trig;
+
+ if (trig->ops && trig->ops->set_trigger_state) {
+ ret = trig->ops->set_trigger_state(trig, true);
+ if (ret)
+ goto err_disable_buffers;
+ }
+ }
+
if (iio_dev_opaque->currentmode == INDIO_BUFFER_TRIGGERED) {
ret = iio_trigger_attach_poll_func(indio_dev->trig,
indio_dev->pollfunc);
if (ret)
- goto err_disable_buffers;
+ goto err_disable_hw_trigger;
}
if (indio_dev->setup_ops->postenable) {
@@ -1130,6 +1150,16 @@ static int iio_enable_buffers(struct iio_dev *indio_dev,
iio_trigger_detach_poll_func(indio_dev->trig,
indio_dev->pollfunc);
}
+err_disable_hw_trigger:
+ if (iio_dev_opaque->currentmode == INDIO_HW_BUFFER_TRIGGERED) {
+ struct iio_trigger *trig = indio_dev->trig;
+
+ if (trig->ops && trig->ops->set_trigger_state) {
+ ret = trig->ops->set_trigger_state(trig, false);
+ if (ret)
+ return ret;
+ }
+ }
err_disable_buffers:
buffer = list_prepare_entry(tmp, &iio_dev_opaque->buffer_list, buffer_list);
list_for_each_entry_continue_reverse(buffer, &iio_dev_opaque->buffer_list,
@@ -1174,6 +1204,13 @@ static int iio_disable_buffers(struct iio_dev *indio_dev)
indio_dev->pollfunc);
}
+ if (iio_dev_opaque->currentmode == INDIO_HW_BUFFER_TRIGGERED) {
+ struct iio_trigger *trig = indio_dev->trig;
+
+ if (trig->ops && trig->ops->set_trigger_state)
+ trig->ops->set_trigger_state(trig, false);
+ }
+
list_for_each_entry(buffer, &iio_dev_opaque->buffer_list, buffer_list) {
ret2 = iio_buffer_disable(buffer, indio_dev);
if (ret2 && !ret)
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index d0ce3b71106a..16f62bd38041 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -366,6 +366,11 @@ s64 iio_get_time_ns(const struct iio_dev *indio_dev);
* they must be managed by the core, but without the entire interrupts/poll
* functions burden. Interrupts are irrelevant as the data flow is hardware
* mediated and distributed.
+ * @INDIO_HW_BUFFER_TRIGGERED: Very unusual mode.
+ * This is similar to INDIO_BUFFER_TRIGGERED but everything is done in hardware
+ * therefore there are no poll functions attached. It also implies the semantics
+ * of both INDIO_HARDWARE_TRIGGERED for the trigger and INDIO_BUFFER_HARDWARE
+ * for the buffer.
*/
#define INDIO_DIRECT_MODE 0x01
#define INDIO_BUFFER_TRIGGERED 0x02
@@ -373,14 +378,19 @@ s64 iio_get_time_ns(const struct iio_dev *indio_dev);
#define INDIO_BUFFER_HARDWARE 0x08
#define INDIO_EVENT_TRIGGERED 0x10
#define INDIO_HARDWARE_TRIGGERED 0x20
+#define INDIO_HW_BUFFER_TRIGGERED 0x40
-#define INDIO_ALL_BUFFER_MODES \
- (INDIO_BUFFER_TRIGGERED | INDIO_BUFFER_HARDWARE | INDIO_BUFFER_SOFTWARE)
+#define INDIO_ALL_BUFFER_MODES \
+ (INDIO_BUFFER_TRIGGERED \
+ | INDIO_BUFFER_HARDWARE \
+ | INDIO_BUFFER_SOFTWARE \
+ | INDIO_HW_BUFFER_TRIGGERED)
#define INDIO_ALL_TRIGGERED_MODES \
(INDIO_BUFFER_TRIGGERED \
| INDIO_EVENT_TRIGGERED \
- | INDIO_HARDWARE_TRIGGERED)
+ | INDIO_HARDWARE_TRIGGERED \
+ | INDIO_HW_BUFFER_TRIGGERED)
#define INDIO_MAX_RAW_ELEMENTS 4
--
2.43.0
This adds an implementation of the SPI offload_ops to the AXI SPI Engine
driver to provide offload support.
Offload lookup is done by device property lookup. SPI Engine commands
and tx data are recorded by writing to offload-specific FIFOs in the
SPI Engine hardware.
Co-developed-by: Lars-Peter Clausen <[email protected]>
Signed-off-by: Lars-Peter Clausen <[email protected]>
Signed-off-by: David Lechner <[email protected]>
---
drivers/spi/spi-axi-spi-engine.c | 270 +++++++++++++++++++++++++++++++++++++++
1 file changed, 270 insertions(+)
diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index 58280dd1c901..1d7ddc867b50 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -2,9 +2,11 @@
/*
* SPI-Engine SPI controller driver
* Copyright 2015 Analog Devices Inc.
+ * Copyright 2023 BayLibre, SAS
* Author: Lars-Peter Clausen <[email protected]>
*/
+#include <linux/cleanup.h>
#include <linux/clk.h>
#include <linux/idr.h>
#include <linux/interrupt.h>
@@ -38,11 +40,22 @@
#define SPI_ENGINE_REG_SDI_DATA_FIFO 0xe8
#define SPI_ENGINE_REG_SDI_DATA_FIFO_PEEK 0xec
+#define SPI_ENGINE_MAX_NUM_OFFLOADS 32
+
+#define SPI_ENGINE_REG_OFFLOAD_CTRL(x) (0x100 + (SPI_ENGINE_MAX_NUM_OFFLOADS * x))
+#define SPI_ENGINE_REG_OFFLOAD_STATUS(x) (0x104 + (SPI_ENGINE_MAX_NUM_OFFLOADS * x))
+#define SPI_ENGINE_REG_OFFLOAD_RESET(x) (0x108 + (SPI_ENGINE_MAX_NUM_OFFLOADS * x))
+#define SPI_ENGINE_REG_OFFLOAD_CMD_FIFO(x) (0x110 + (SPI_ENGINE_MAX_NUM_OFFLOADS * x))
+#define SPI_ENGINE_REG_OFFLOAD_SDO_FIFO(x) (0x114 + (SPI_ENGINE_MAX_NUM_OFFLOADS * x))
+
#define SPI_ENGINE_INT_CMD_ALMOST_EMPTY BIT(0)
#define SPI_ENGINE_INT_SDO_ALMOST_EMPTY BIT(1)
#define SPI_ENGINE_INT_SDI_ALMOST_FULL BIT(2)
#define SPI_ENGINE_INT_SYNC BIT(3)
+#define SPI_ENGINE_OFFLOAD_CTRL_ENABLE BIT(0)
+#define SPI_ENGINE_OFFLOAD_STATUS_ENABLED BIT(0)
+
#define SPI_ENGINE_CONFIG_CPHA BIT(0)
#define SPI_ENGINE_CONFIG_CPOL BIT(1)
#define SPI_ENGINE_CONFIG_3WIRE BIT(2)
@@ -76,6 +89,10 @@
#define SPI_ENGINE_CMD_SYNC(id) \
SPI_ENGINE_CMD(SPI_ENGINE_INST_MISC, SPI_ENGINE_MISC_SYNC, (id))
+/* default sizes - can be changed when SPI Engine firmware is compiled */
+#define SPI_ENGINE_OFFLOAD_CMD_FIFO_SIZE 16
+#define SPI_ENGINE_OFFLOAD_SDO_FIFO_SIZE 16
+
struct spi_engine_program {
unsigned int length;
uint16_t instructions[];
@@ -107,6 +124,10 @@ struct spi_engine_message_state {
u8 sync_id;
};
+struct spi_engine_offload {
+ unsigned int index;
+};
+
struct spi_engine {
struct clk *clk;
struct clk *ref_clk;
@@ -119,6 +140,9 @@ struct spi_engine {
struct spi_controller *controller;
unsigned int int_enable;
+
+ struct spi_offload offloads[SPI_ENGINE_MAX_NUM_OFFLOADS];
+ struct spi_engine_offload offload_priv[SPI_ENGINE_MAX_NUM_OFFLOADS];
};
static void spi_engine_program_add_cmd(struct spi_engine_program *p,
@@ -603,6 +627,239 @@ static int spi_engine_transfer_one_message(struct spi_controller *host,
return 0;
}
+static struct spi_offload *spi_engine_offload_get(struct spi_device *spi,
+ unsigned int index)
+{
+ struct spi_controller *host = spi->controller;
+ struct spi_engine *spi_engine = spi_controller_get_devdata(host);
+ struct spi_offload *offload;
+ u32 vals[SPI_ENGINE_MAX_NUM_OFFLOADS];
+ int ret;
+
+ /* Use the adi,offloads array to find the offload at index. */
+
+ if (index >= ARRAY_SIZE(vals))
+ return ERR_PTR(-EINVAL);
+
+ ret = device_property_read_u32_array(&spi->dev, "adi,offloads", vals,
+ index + 1);
+ if (ret < 0)
+ return ERR_PTR(ret);
+
+ if (vals[index] >= SPI_ENGINE_MAX_NUM_OFFLOADS)
+ return ERR_PTR(-EINVAL);
+
+ offload = &spi_engine->offloads[vals[index]];
+
+ return offload;
+}
+
+static int spi_engine_offload_prepare(struct spi_offload *offload,
+ struct spi_message *msg)
+{
+ struct spi_controller *host = offload->controller;
+ struct spi_engine_offload *priv = offload->priv;
+ struct spi_engine *spi_engine = spi_controller_get_devdata(host);
+ struct spi_engine_program p_dry, *p __free(kfree) = NULL;
+ struct spi_transfer *xfer;
+ void __iomem *cmd_addr;
+ void __iomem *sdo_addr;
+ size_t tx_word_count = 0;
+ unsigned int i;
+
+ /* count total number of tx words in message */
+ list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+ if (!xfer->tx_buf)
+ continue;
+
+ if (xfer->bits_per_word <= 8)
+ tx_word_count += xfer->len;
+ else if (xfer->bits_per_word <= 16)
+ tx_word_count += xfer->len / 2;
+ else
+ tx_word_count += xfer->len / 4;
+ }
+
+ /* REVISIT: could get actual size from devicetree if needed */
+ if (tx_word_count > SPI_ENGINE_OFFLOAD_SDO_FIFO_SIZE)
+ return -EINVAL;
+
+ spi_engine_precompile_message(msg);
+
+ /* dry run to get length */
+ p_dry.length = 0;
+ spi_engine_compile_message(msg, true, &p_dry);
+
+ /* REVISIT: could get actual size from devicetree if needed */
+ if (p_dry.length > SPI_ENGINE_OFFLOAD_CMD_FIFO_SIZE)
+ return -EINVAL;
+
+ p = kzalloc(sizeof(*p) + sizeof(*p->instructions) * p_dry.length, GFP_KERNEL);
+ if (!p)
+ return -ENOMEM;
+
+ spi_engine_compile_message(msg, false, p);
+
+ cmd_addr = spi_engine->base + SPI_ENGINE_REG_OFFLOAD_CMD_FIFO(priv->index);
+ sdo_addr = spi_engine->base + SPI_ENGINE_REG_OFFLOAD_SDO_FIFO(priv->index);
+
+ list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+ if (!xfer->tx_buf)
+ continue;
+
+ if (xfer->bits_per_word <= 8) {
+ const u8 *buf = xfer->tx_buf;
+
+ for (i = 0; i < xfer->len; i++)
+ writel_relaxed(buf[i], sdo_addr);
+ } else if (xfer->bits_per_word <= 16) {
+ const u16 *buf = xfer->tx_buf;
+
+ for (i = 0; i < xfer->len / 2; i++)
+ writel_relaxed(buf[i], sdo_addr);
+ } else {
+ const u32 *buf = xfer->tx_buf;
+
+ for (i = 0; i < xfer->len / 4; i++)
+ writel_relaxed(buf[i], sdo_addr);
+ }
+ }
+
+ for (i = 0; i < p->length; i++)
+ writel_relaxed(p->instructions[i], cmd_addr);
+
+ return 0;
+}
+
+static void spi_engine_offload_unprepare(struct spi_offload *offload)
+{
+ struct spi_controller *host = offload->controller;
+ struct spi_engine_offload *priv = offload->priv;
+ struct spi_engine *spi_engine = spi_controller_get_devdata(host);
+
+ writel_relaxed(1, spi_engine->base +
+ SPI_ENGINE_REG_OFFLOAD_RESET(priv->index));
+ writel_relaxed(0, spi_engine->base +
+ SPI_ENGINE_REG_OFFLOAD_RESET(priv->index));
+}
+
+static int spi_engine_offload_enable(struct spi_offload *offload)
+{
+ struct spi_controller *host = offload->controller;
+ struct spi_engine_offload *priv = offload->priv;
+ struct spi_engine *spi_engine = spi_controller_get_devdata(host);
+ unsigned int reg;
+
+ reg = readl_relaxed(spi_engine->base +
+ SPI_ENGINE_REG_OFFLOAD_CTRL(priv->index));
+ reg |= SPI_ENGINE_OFFLOAD_CTRL_ENABLE;
+ writel_relaxed(reg, spi_engine->base +
+ SPI_ENGINE_REG_OFFLOAD_CTRL(priv->index));
+
+ return 0;
+}
+
+static void spi_engine_offload_disable(struct spi_offload *offload)
+{
+ struct spi_controller *host = offload->controller;
+ struct spi_engine_offload *priv = offload->priv;
+ struct spi_engine *spi_engine = spi_controller_get_devdata(host);
+ unsigned int reg;
+
+ reg = readl_relaxed(spi_engine->base +
+ SPI_ENGINE_REG_OFFLOAD_CTRL(priv->index));
+ reg &= ~SPI_ENGINE_OFFLOAD_CTRL_ENABLE;
+ writel_relaxed(reg, spi_engine->base +
+ SPI_ENGINE_REG_OFFLOAD_CTRL(priv->index));
+}
+
+static const struct spi_controller_offload_ops spi_engine_offload_ops = {
+ .get = spi_engine_offload_get,
+ .prepare = spi_engine_offload_prepare,
+ .unprepare = spi_engine_offload_unprepare,
+ .enable = spi_engine_offload_enable,
+ .disable = spi_engine_offload_disable,
+};
+
+static void spi_engine_offload_release(void *p)
+{
+ struct spi_offload *offload = p;
+ struct platform_device *pdev = container_of(offload->dev,
+ struct platform_device, dev);
+
+ offload->dev = NULL;
+ platform_device_unregister(pdev);
+}
+
+/**
+ * devm_spi_engine_register_offload() - Registers platform device for offload.
+ *
+ * @dev: The parent platform device node.
+ * @offload: The offload firmware node.
+ *
+ * Return: 0 on success, negative error code otherwise.
+ */
+static int devm_spi_engine_register_offload(struct device *dev,
+ struct spi_engine *spi_engine,
+ struct fwnode_handle *fwnode)
+{
+ struct platform_device_info pdevinfo = {
+ .parent = dev,
+ .name = "offload",
+ .fwnode = fwnode,
+ };
+ struct platform_device *pdev;
+ struct spi_offload *offload;
+ u32 index;
+ int ret;
+
+ ret = fwnode_property_read_u32(fwnode, "reg", &index);
+ if (ret)
+ return ret;
+
+ if (index >= SPI_ENGINE_MAX_NUM_OFFLOADS)
+ return -EINVAL;
+
+ pdevinfo.id = index;
+
+ pdev = platform_device_register_full(&pdevinfo);
+ if (IS_ERR(pdev))
+ return PTR_ERR(pdev);
+
+ offload = &spi_engine->offloads[index];
+ offload->dev = &pdev->dev;
+
+ return devm_add_action_or_reset(dev, spi_engine_offload_release, offload);
+}
+
+/**
+ * spi_engine_offload_populate() - Registers platform device for each offload instance.
+ * @host: The SPI controller.
+ * @spi_engine: The SPI engine.
+ * @dev: The parent platform device.
+ */
+static void spi_engine_offload_populate(struct spi_controller *host,
+ struct spi_engine *spi_engine,
+ struct device *dev)
+{
+ struct fwnode_handle *offloads;
+ struct fwnode_handle *child;
+ int ret;
+
+ /* offloads are optional */
+ offloads = device_get_named_child_node(dev, "offloads");
+ if (!offloads)
+ return;
+
+ fwnode_for_each_available_child_node(offloads, child) {
+ ret = devm_spi_engine_register_offload(dev, spi_engine, child);
+ if (ret)
+ dev_warn(dev, "failed to register offload: %d\n", ret);
+ }
+
+ fwnode_handle_put(offloads);
+}
+
static void spi_engine_timeout(struct timer_list *timer)
{
struct spi_engine *spi_engine = from_timer(spi_engine, timer, watchdog_timer);
@@ -633,6 +890,7 @@ static int spi_engine_probe(struct platform_device *pdev)
unsigned int version;
int irq;
int ret;
+ int i;
irq = platform_get_irq(pdev, 0);
if (irq < 0)
@@ -670,6 +928,15 @@ static int spi_engine_probe(struct platform_device *pdev)
return -ENODEV;
}
+ for (i = 0; i < SPI_ENGINE_MAX_NUM_OFFLOADS; i++) {
+ struct spi_engine_offload *priv = &spi_engine->offload_priv[i];
+ struct spi_offload *offload = &spi_engine->offloads[i];
+
+ priv->index = i;
+ offload->controller = host;
+ offload->priv = priv;
+ }
+
writel_relaxed(0x00, spi_engine->base + SPI_ENGINE_REG_RESET);
writel_relaxed(0xff, spi_engine->base + SPI_ENGINE_REG_INT_PENDING);
writel_relaxed(0x00, spi_engine->base + SPI_ENGINE_REG_INT_ENABLE);
@@ -692,6 +959,7 @@ static int spi_engine_probe(struct platform_device *pdev)
host->prepare_message = spi_engine_prepare_message;
host->unprepare_message = spi_engine_unprepare_message;
host->num_chipselect = 8;
+ host->offload_ops = &spi_engine_offload_ops;
if (host->max_speed_hz == 0)
return dev_err_probe(&pdev->dev, -EINVAL, "spi_clk rate is 0");
@@ -702,6 +970,8 @@ static int spi_engine_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, host);
+ spi_engine_offload_populate(host, spi_engine, &pdev->dev);
+
return 0;
}
--
2.43.0
The auxiliary bus uses names in the form of "module_name.device_name"
for matching devices to drivers. Since the module name is the actual
module name, it can be quite long which doesn't leave enough room for
the device name.
This patch increases the size AUXILIARY_NAME_SIZE to 64 to allow for
both a ~32 character module name and a ~32 character device name.
Signed-off-by: David Lechner <[email protected]>
---
include/linux/mod_devicetable.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index f458469c5ce5..4bd2d20067b6 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -861,7 +861,7 @@ struct mhi_device_id {
kernel_ulong_t driver_data;
};
-#define AUXILIARY_NAME_SIZE 32
+#define AUXILIARY_NAME_SIZE 64
#define AUXILIARY_MODULE_PREFIX "auxiliary:"
struct auxiliary_device_id {
--
2.43.0
This adds a new hardware triggered buffer driver for the IIO subsystem.
This driver is intended to be used by IIO device drivers that have
a hardware buffer that is triggered by a hardware signal.
It is expected that components such as those providing a backend via the
IIO backend framework will provide the actual implementation of this
functionality by registering a matching device on the auxiliary bus.
The auxiliary bus was chosen since it allows us to make use of existing
kernel infrastructure instead of implementing our own registration and
lookup system.
Signed-off-by: David Lechner <[email protected]>
---
Documentation/driver-api/driver-model/devres.rst | 1 +
drivers/iio/buffer/Kconfig | 7 ++
drivers/iio/buffer/Makefile | 1 +
.../iio/buffer/industrialio-hw-triggered-buffer.c | 104 +++++++++++++++++++++
include/linux/iio/hw_triggered_buffer.h | 14 +++
include/linux/iio/hw_triggered_buffer_impl.h | 16 ++++
6 files changed, 143 insertions(+)
diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst
index c5f99d834ec5..b23d4a2b68a6 100644
--- a/Documentation/driver-api/driver-model/devres.rst
+++ b/Documentation/driver-api/driver-model/devres.rst
@@ -296,6 +296,7 @@ IIO
devm_iio_channel_get()
devm_iio_channel_get_all()
devm_iio_hw_consumer_alloc()
+ devm_iio_hw_triggered_buffer_setup()
devm_fwnode_iio_channel_get_by_name()
INPUT
diff --git a/drivers/iio/buffer/Kconfig b/drivers/iio/buffer/Kconfig
index 047b931591a9..925c5bf074bc 100644
--- a/drivers/iio/buffer/Kconfig
+++ b/drivers/iio/buffer/Kconfig
@@ -53,3 +53,10 @@ config IIO_TRIGGERED_BUFFER
select IIO_KFIFO_BUF
help
Provides helper functions for setting up triggered buffers.
+
+config IIO_HW_TRIGGERED_BUFFER
+ tristate "Industrial I/O hardware triggered buffer support"
+ select AUXILIARY_BUS
+ select IIO_TRIGGER
+ help
+ Provides helper functions for setting up hardware triggered buffers.
diff --git a/drivers/iio/buffer/Makefile b/drivers/iio/buffer/Makefile
index 1403eb2f9409..d1142bb20f61 100644
--- a/drivers/iio/buffer/Makefile
+++ b/drivers/iio/buffer/Makefile
@@ -9,4 +9,5 @@ obj-$(CONFIG_IIO_BUFFER_DMA) += industrialio-buffer-dma.o
obj-$(CONFIG_IIO_BUFFER_DMAENGINE) += industrialio-buffer-dmaengine.o
obj-$(CONFIG_IIO_BUFFER_HW_CONSUMER) += industrialio-hw-consumer.o
obj-$(CONFIG_IIO_TRIGGERED_BUFFER) += industrialio-triggered-buffer.o
+obj-$(CONFIG_IIO_HW_TRIGGERED_BUFFER) += industrialio-hw-triggered-buffer.o
obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o
diff --git a/drivers/iio/buffer/industrialio-hw-triggered-buffer.c b/drivers/iio/buffer/industrialio-hw-triggered-buffer.c
new file mode 100644
index 000000000000..7a8a71066b0e
--- /dev/null
+++ b/drivers/iio/buffer/industrialio-hw-triggered-buffer.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024 Analog Devices, Inc.
+ * Copyright (c) 2024 BayLibre, SAS
+ */
+
+#include <linux/auxiliary_bus.h>
+#include <linux/container_of.h>
+#include <linux/export.h>
+#include <linux/module.h>
+#include <linux/iio/hw_triggered_buffer_impl.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger.h>
+
+static int iio_hw_triggered_buffer_match(struct device *dev, const void *match)
+{
+ return dev->parent == match;
+}
+
+static struct iio_hw_triggered_buffer_device
+*iio_hw_trigger_buffer_get(struct device *match)
+{
+ struct auxiliary_device *adev;
+
+ adev = auxiliary_find_device(NULL, match, iio_hw_triggered_buffer_match);
+ if (!adev)
+ return ERR_PTR(-ENOENT);
+
+ return container_of(adev, struct iio_hw_triggered_buffer_device, adev);
+}
+
+static void iio_hw_trigger_buffer_put(void *dev)
+{
+ put_device(dev);
+}
+
+/**
+ * devm_iio_hw_triggered_buffer_setup - Setup a hardware triggered buffer
+ * @dev: Device for devm management
+ * @indio_dev: An unconfigured/partially configured IIO device struct
+ * @match: Device for matching the auxiliary bus device that provides the
+ * interface to the hardware triggered buffer
+ * @ops: Buffer setup functions to use for this IIO device
+ *
+ * Return: 0 on success, negative error code on failure.
+ *
+ * This function will search all registered hardware triggered buffers for one
+ * that matches the given indio_dev. If found, it will be used to setup both
+ * the trigger and the buffer on the indio_dev.
+ */
+int devm_iio_hw_triggered_buffer_setup(struct device *dev,
+ struct iio_dev *indio_dev,
+ struct device *match,
+ const struct iio_buffer_setup_ops *ops)
+{
+ struct iio_hw_triggered_buffer_device *hw;
+ int ret;
+
+ hw = iio_hw_trigger_buffer_get(match);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+
+ ret = devm_add_action_or_reset(dev, iio_hw_trigger_buffer_put, &hw->adev.dev);
+ if (ret)
+ return ret;
+
+ indio_dev->modes |= INDIO_HW_BUFFER_TRIGGERED;
+ indio_dev->trig = iio_trigger_get(hw->trig);
+ indio_dev->setup_ops = ops;
+
+ return iio_device_attach_buffer(indio_dev, hw->buffer);
+}
+EXPORT_SYMBOL_GPL(devm_iio_hw_triggered_buffer_setup);
+
+static int iio_hw_trigger_buffer_probe(struct auxiliary_device *adev,
+ const struct auxiliary_device_id *id)
+{
+ struct iio_hw_triggered_buffer_device *hw =
+ container_of(adev, struct iio_hw_triggered_buffer_device, adev);
+
+ if (!hw->buffer || !hw->trig)
+ return -EINVAL;
+
+ return 0;
+}
+
+static const struct auxiliary_device_id iio_hw_trigger_buffer_id_table[] = {
+ { }
+};
+MODULE_DEVICE_TABLE(auxiliary, iio_hw_trigger_buffer_id_table);
+
+static struct auxiliary_driver iio_hw_trigger_buffer_driver = {
+ .driver = {
+ .name = "iio-hw-triggered-buffer",
+ },
+ .probe = iio_hw_trigger_buffer_probe,
+ .id_table = iio_hw_trigger_buffer_id_table,
+};
+module_auxiliary_driver(iio_hw_trigger_buffer_driver);
+
+MODULE_AUTHOR("David Lechner <[email protected]>");
+MODULE_DESCRIPTION("IIO helper functions for setting up hardware triggered buffers");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/iio/hw_triggered_buffer.h b/include/linux/iio/hw_triggered_buffer.h
new file mode 100644
index 000000000000..6bd8035f1b92
--- /dev/null
+++ b/include/linux/iio/hw_triggered_buffer.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_IIO_HW_TRIGGEREDED_BUFFER_H_
+#define _LINUX_IIO_HW_TRIGGEREDED_BUFFER_H_
+
+struct device;
+struct iio_dev;
+struct iio_buffer_setup_ops;
+
+int devm_iio_hw_triggered_buffer_setup(struct device *dev,
+ struct iio_dev *indio_dev,
+ struct device *match,
+ const struct iio_buffer_setup_ops *ops);
+
+#endif /* _LINUX_IIO_HW_TRIGGEREDED_BUFFER_H_ */
diff --git a/include/linux/iio/hw_triggered_buffer_impl.h b/include/linux/iio/hw_triggered_buffer_impl.h
new file mode 100644
index 000000000000..d9a3ad2c8c24
--- /dev/null
+++ b/include/linux/iio/hw_triggered_buffer_impl.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_IIO_HW_TRIGGEREDED_BUFFER_IMPL_H_
+#define _LINUX_IIO_HW_TRIGGEREDED_BUFFER_IMPL_H_
+
+#include <linux/auxiliary_bus.h>
+
+struct iio_buffer;
+struct iio_trigger;
+
+struct iio_hw_triggered_buffer_device {
+ struct auxiliary_device adev;
+ struct iio_buffer *buffer;
+ struct iio_trigger *trig;
+};
+
+#endif /* _LINUX_IIO_HW_TRIGGEREDED_BUFFER_IMPL_H_ */
--
2.43.0
This changes devm_iio_dmaengine_buffer_alloc() to an exported symbol.
This will be used by drivers that need to allocate a DMA buffer without
attaching it to an IIO device.
Signed-off-by: David Lechner <[email protected]>
---
Documentation/driver-api/driver-model/devres.rst | 1 +
drivers/iio/buffer/Kconfig | 14 +++++++-------
drivers/iio/buffer/industrialio-buffer-dmaengine.c | 3 ++-
include/linux/iio/buffer-dmaengine.h | 2 ++
4 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst
index b23d4a2b68a6..60e4b7ba38e5 100644
--- a/Documentation/driver-api/driver-model/devres.rst
+++ b/Documentation/driver-api/driver-model/devres.rst
@@ -285,6 +285,7 @@ I2C
IIO
devm_iio_device_alloc()
devm_iio_device_register()
+ devm_iio_dmaengine_buffer_alloc()
devm_iio_dmaengine_buffer_setup()
devm_iio_kfifo_buffer_setup()
devm_iio_kfifo_buffer_setup_ext()
diff --git a/drivers/iio/buffer/Kconfig b/drivers/iio/buffer/Kconfig
index 925c5bf074bc..27d82fb4bc4d 100644
--- a/drivers/iio/buffer/Kconfig
+++ b/drivers/iio/buffer/Kconfig
@@ -40,6 +40,13 @@ config IIO_BUFFER_HW_CONSUMER
Should be selected by drivers that want to use the generic Hw consumer
interface.
+config IIO_HW_TRIGGERED_BUFFER
+ tristate "Industrial I/O hardware triggered buffer support"
+ select AUXILIARY_BUS
+ select IIO_TRIGGER
+ help
+ Provides helper functions for setting up hardware triggered buffers.
+
config IIO_KFIFO_BUF
tristate "Industrial I/O buffering based on kfifo"
help
@@ -53,10 +60,3 @@ config IIO_TRIGGERED_BUFFER
select IIO_KFIFO_BUF
help
Provides helper functions for setting up triggered buffers.
-
-config IIO_HW_TRIGGERED_BUFFER
- tristate "Industrial I/O hardware triggered buffer support"
- select AUXILIARY_BUS
- select IIO_TRIGGER
- help
- Provides helper functions for setting up hardware triggered buffers.
diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
index c67ddf963bfb..03225939f223 100644
--- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
+++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
@@ -244,7 +244,7 @@ static void __devm_iio_dmaengine_buffer_free(void *buffer)
*
* The buffer will be automatically de-allocated once the device gets destroyed.
*/
-static struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
+struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
const char *channel)
{
struct iio_buffer *buffer;
@@ -261,6 +261,7 @@ static struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
return buffer;
}
+EXPORT_SYMBOL_GPL(devm_iio_dmaengine_buffer_alloc);
/**
* devm_iio_dmaengine_buffer_setup() - Setup a DMA buffer for an IIO device
diff --git a/include/linux/iio/buffer-dmaengine.h b/include/linux/iio/buffer-dmaengine.h
index 5c355be89814..3ac616ddf5b9 100644
--- a/include/linux/iio/buffer-dmaengine.h
+++ b/include/linux/iio/buffer-dmaengine.h
@@ -10,6 +10,8 @@
struct iio_dev;
struct device;
+struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
+ const char *channel);
int devm_iio_dmaengine_buffer_setup(struct device *dev,
struct iio_dev *indio_dev,
const char *channel);
--
2.43.0
This adds a new binding for a PWM trigger and DMA data output connected
to an SPI controller offload instance.
Signed-off-by: David Lechner <[email protected]>
---
.../adi,spi-offload-pwm-trigger-dma-buffer.yaml | 59 ++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/offload/adi,spi-offload-pwm-trigger-dma-buffer.yaml b/Documentation/devicetree/bindings/iio/offload/adi,spi-offload-pwm-trigger-dma-buffer.yaml
new file mode 100644
index 000000000000..748cfab19eff
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/offload/adi,spi-offload-pwm-trigger-dma-buffer.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/offload/adi,spi-offload-pwm-trigger-dma-buffer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SPI Offload with PWM Trigger and DMA Buffer Data Output
+
+maintainers:
+ - Michael Hennerich <[email protected]>
+ - Nuno Sá <[email protected]>
+
+description: |
+ This binding describes the connection of a PWM device to the trigger input
+ and a DMA channel to the output data stream of a SPI Offload instance.
+
+ https://wiki.analog.com/resources/fpga/peripherals/spi_engine/offload
+ https://wiki.analog.com/resources/fpga/peripherals/spi_engine/tutorial
+
+$ref: /schemas/spi/adi,axi-spi-engine.yaml#/$defs/offload
+
+properties:
+ compatible:
+ const: adi,spi-offload-pwm-trigger-dma-buffer
+
+ reg:
+ maxItems: 1
+
+ pwms:
+ maxItems: 1
+
+ dmas:
+ maxItems: 1
+
+required:
+ - compatible
+ - pwms
+ - dmas
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ offloads {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ offload@0 {
+ compatible = "adi,spi-offload-pwm-trigger-dma-buffer";
+ reg = <0>;
+ pwms = <&pwm 0>;
+ dmas = <&dma 0>;
+ };
+ };
+ };
--
2.43.0
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 <[email protected]>
---
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;
+}
+
+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 <[email protected]>");
+MODULE_DESCRIPTION("AXI SPI Engine Offload PWM Trigger");
+MODULE_LICENSE("GPL");
--
2.43.0
This extends the ad7380 ADC driver to use the offload capabilities of
capable SPI controllers. When offload support is available, a hardware
triggered buffer is used to allow sampling a high rates without CPU
intervention.
To keep things simple, when this feature is present in hardware we
disable the usual IIO triggered buffer and software timestamp rather
than trying to support multiple buffers.
Signed-off-by: David Lechner <[email protected]>
---
drivers/iio/adc/Kconfig | 1 +
drivers/iio/adc/ad7380.c | 84 +++++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 80 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index cbfd626712e3..da44b585ea46 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -128,6 +128,7 @@ config AD7380
select IIO_BUFFER
select IIO_TRIGGER
select IIO_TRIGGERED_BUFFER
+ select IIO_HW_TRIGGERED_BUFFER
help
AD7380 is a family of simultaneous sampling ADCs that share the same
SPI register map and have similar pinouts.
diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c
index 80712aaa9548..a71e8b81950b 100644
--- a/drivers/iio/adc/ad7380.c
+++ b/drivers/iio/adc/ad7380.c
@@ -20,6 +20,7 @@
#include <linux/sysfs.h>
#include <linux/iio/buffer.h>
+#include <linux/iio/hw_triggered_buffer.h>
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
#include <linux/iio/trigger_consumer.h>
@@ -133,6 +134,7 @@ struct ad7380_state {
struct spi_device *spi;
struct regulator *vref;
struct regmap *regmap;
+ struct spi_offload *spi_offload;
/*
* DMA (thus cache coherency maintenance) requires the
* transfer buffers to live in their own cache lines.
@@ -335,6 +337,50 @@ static const struct iio_info ad7380_info = {
.debugfs_reg_access = &ad7380_debugfs_reg_access,
};
+static int ad7380_buffer_preenable(struct iio_dev *indio_dev)
+{
+ struct ad7380_state *st = iio_priv(indio_dev);
+ struct spi_transfer xfer = {
+ .bits_per_word = st->chip_info->channels[0].scan_type.realbits,
+ .len = 4,
+ .rx_buf = SPI_OFFLOAD_RX_SENTINEL,
+ };
+
+ return spi_offload_prepare(st->spi_offload, st->spi, &xfer, 1);
+}
+
+static int ad7380_buffer_postenable(struct iio_dev *indio_dev)
+{
+ struct ad7380_state *st = iio_priv(indio_dev);
+
+ return spi_offload_enable(st->spi_offload);
+}
+
+static int ad7380_buffer_predisable(struct iio_dev *indio_dev)
+{
+ struct ad7380_state *st = iio_priv(indio_dev);
+
+ spi_offload_disable(st->spi_offload);
+
+ return 0;
+}
+
+static int ad7380_buffer_postdisable(struct iio_dev *indio_dev)
+{
+ struct ad7380_state *st = iio_priv(indio_dev);
+
+ spi_offload_unprepare(st->spi_offload);
+
+ return 0;
+}
+
+static const struct iio_buffer_setup_ops ad7380_buffer_ops = {
+ .preenable = &ad7380_buffer_preenable,
+ .postenable = &ad7380_buffer_postenable,
+ .predisable = &ad7380_buffer_predisable,
+ .postdisable = &ad7380_buffer_postdisable,
+};
+
static int ad7380_init(struct ad7380_state *st)
{
int ret;
@@ -417,11 +463,39 @@ static int ad7380_probe(struct spi_device *spi)
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->available_scan_masks = ad7380_2_channel_scan_masks;
- ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
- iio_pollfunc_store_time,
- ad7380_trigger_handler, NULL);
- if (ret)
- return ret;
+ st->spi_offload = spi_offload_get(spi, 0);
+ if (IS_ERR(st->spi_offload)) {
+ ret = PTR_ERR(st->spi_offload);
+
+ if (ret == -EOPNOTSUPP)
+ st->spi_offload = NULL;
+ else
+ return dev_err_probe(&spi->dev, ret,
+ "failed to get SPI offload\n");
+ }
+
+ if (st->spi_offload) {
+ /*
+ * We can't have a soft timestamp (always last channel) when
+ * using a hardware triggered buffer.
+ */
+ indio_dev->num_channels -= 1;
+
+ ret = devm_iio_hw_triggered_buffer_setup(&spi->dev,
+ indio_dev,
+ st->spi_offload->dev,
+ &ad7380_buffer_ops);
+ if (ret)
+ return dev_err_probe(&spi->dev, ret,
+ "failed to setup offload\n");
+ } else {
+ ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
+ iio_pollfunc_store_time,
+ ad7380_trigger_handler,
+ NULL);
+ if (ret)
+ return ret;
+ }
ret = ad7380_init(st);
if (ret)
--
2.43.0
On Wed, 10 Jan 2024 13:49:52 -0600, David Lechner wrote:
> This adds a new binding for a PWM trigger and DMA data output connected
> to an SPI controller offload instance.
>
> Signed-off-by: David Lechner <[email protected]>
> ---
> .../adi,spi-offload-pwm-trigger-dma-buffer.yaml | 59 ++++++++++++++++++++++
> 1 file changed, 59 insertions(+)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/offload/adi,spi-offload-pwm-trigger-dma-buffer.yaml:
Error in referenced schema matching $id: http://devicetree.org/schemas/spi/adi,axi-spi-engine.yaml
Documentation/devicetree/bindings/iio/offload/adi,spi-offload-pwm-trigger-dma-buffer.example.dts:22.22-32.15: Warning (spi_bus_reg): /example-0/spi/offloads: missing or empty reg property
Traceback (most recent call last):
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 966, in resolve_fragment
document = document[part]
~~~~~~~~^^^^^^
TypeError: 'bool' object is not subscriptable
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/usr/local/bin/dt-validate", line 8, in <module>
sys.exit(main())
^^^^^^
File "/usr/local/lib/python3.11/dist-packages/dtschema/dtb_validate.py", line 144, in main
sg.check_dtb(filename)
File "/usr/local/lib/python3.11/dist-packages/dtschema/dtb_validate.py", line 89, in check_dtb
self.check_subtree(dt, subtree, False, "/", "/", filename)
File "/usr/local/lib/python3.11/dist-packages/dtschema/dtb_validate.py", line 82, in check_subtree
self.check_subtree(tree, value, disabled, name, fullname + name, filename)
File "/usr/local/lib/python3.11/dist-packages/dtschema/dtb_validate.py", line 82, in check_subtree
self.check_subtree(tree, value, disabled, name, fullname + name, filename)
File "/usr/local/lib/python3.11/dist-packages/dtschema/dtb_validate.py", line 82, in check_subtree
self.check_subtree(tree, value, disabled, name, fullname + name, filename)
[Previous line repeated 1 more time]
File "/usr/local/lib/python3.11/dist-packages/dtschema/dtb_validate.py", line 77, in check_subtree
self.check_node(tree, subtree, disabled, nodename, fullname, filename)
File "/usr/local/lib/python3.11/dist-packages/dtschema/dtb_validate.py", line 33, in check_node
for error in self.validator.iter_errors(node, filter=match_schema_file):
File "/usr/local/lib/python3.11/dist-packages/dtschema/validator.py", line 405, in iter_errors
for error in self.DtValidator(sch,
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 288, in iter_errors
for error in errors:
File "/usr/local/lib/python3.11/dist-packages/jsonschema/_validators.py", line 414, in if_
yield from validator.descend(instance, then, schema_path="then")
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 305, in descend
for error in self.evolve(schema=schema).iter_errors(instance):
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 288, in iter_errors
for error in errors:
File "/usr/local/lib/python3.11/dist-packages/jsonschema/_validators.py", line 294, in ref
scope, resolved = validator.resolver.resolve(ref)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 898, in resolve
return url, self._remote_cache(url)
^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 916, in resolve_from_url
return self.resolve_fragment(document, fragment)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 968, in resolve_fragment
raise exceptions.RefResolutionError(
jsonschema.exceptions.RefResolutionError: Unresolvable JSON pointer: '$defs/offload'
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
On Wed, Jan 10, 2024 at 01:49:42PM -0600, David Lechner wrote:
> This adds a feature for specialized SPI controllers that can record
> a series of SPI transfers, including tx data, cs assertions, delays,
> etc. and then play them back using a hardware trigger without CPU
> intervention.
> The intended use case for this is with the AXI SPI Engine to capture
> data from ADCs at high rates (MSPS) with a stable sample period.
> Most of the implementation is controller-specific and will be handled by
> drivers that implement the offload_ops callbacks. The API follows a
> prepare/enable pattern that should be familiar to users of the clk
> subsystem.
This is a lot to do in one go, and I think it's a bit too off on the
side and unintegrated with the core. There's two very high level bits
here, there's the pre-cooking a message for offloading to be executed by
a hardware engine and there's the bit where that's triggered by some
hardwar event rather than by software.
There was a bunch of discussion of the former case with David Jander
(CCed) a while back when he was doing all the work he did on optimising
the core for uncontended uses, the thinking there was to have a
spi_prepare_message() (or similar) API that drivers could call and then
reuse the same transfer repeatedly, and even without any interface for
client drivers it's likely that we'd be able to take advantage of it in
the core for multi-transfer messages. I'd be surprised if there weren't
wins when the message goes over the DMA copybreak size. A much wider
range of hardware would be able to do this bit, for example David's case
was a Raspberry Pi using the DMA controller to write into the SPI
controller control registers in order to program it for multiple
transfers, bounce chip select and so on. You could also use the
microcontroller cores that many embedded SoCs have, and even with zero
hardware support for offloading anything there's savings in the message
validation and DMA mapping.
The bit where messages are initiated by hardware is a step beyond that,
I think we need a bit more API for connecting up the triggers and we
also need to have something handling what happens with normal operation
of the device while these triggers are enabled. I think it could be
useful to split this bit out since there's a lot more to work out there
in terms of interfaces.
> +/**
> + * SPI_OFFLOAD_RX - placeholder for indicating read transfers for offloads
> + *
> + * Assign xfer->rx_buf to this value for any read transfer passed to
> + * spi_offload_prepare(). This will act as a flag to indicate to the offload
> + * that it should do something with the data read during this transfer. What
> + * that something can be is determined by the specific hardware, e.g. it could
> + * be piped to DMA or a DSP, etc.
> + */
> +#define SPI_OFFLOAD_RX_SENTINEL ((void *)1)
This feels like something where there are likely to be multiple options
and we need configurability. I'd also expect to see a similar transmit
option.
> +int spi_offload_prepare(struct spi_offload *offload, struct spi_device *spi,
> + struct spi_transfer *xfers, unsigned int num_xfers)
I would expect us to just generically prepare a message, then pass a
prepared message into the API that enables a trigger. We would need
something that handles the difference between potentially offloading for
better performance and having a hardware trigger, I think that might be
a case of just not exposing the engine's prepare to client drivers and
then having the core track if it needs to do that when enabling a
hardware trigger.
> + /**
> + * @enable: Callback to enable the offload.
> + */
> + int (*enable)(struct spi_offload *offload);
> + /**
> + * @disable: Callback to disable the offload.
> + */
> + void (*disable)(struct spi_offload *offload);
I'm not seeing anything in this API that provides a mechanism for
configuring what triggers things to start, even in the case where things
are triggered by hardware rather than initiated by software I'd expect
to see hardware with runtime configurability. The binding is a bit
unclear but it seems to be expecting this to be statically configured in
hardware and that there will be a 1:1 mapping between triggers and
scripts that can be configured, if nothing else I would expect that
there will be hardware with more possible triggers than scripts.
I'd also expect some treatement of what happens with the standard SPI
API while something is enabled.
On Wed, Jan 10, 2024 at 01:49:44PM -0600, David Lechner wrote:
> In the DT bindings for SPI devices, it is specified that peripheral
> nodes have the @ character in the node name. A SPI controller may need
> to create bindings with child nodes that are not peripherals. For
> example, the AXI SPI Engine bindings will use an "offloads" child node
> to describe what is connected to the offload interfaces of the SPI
> controller.
Have you done any auditing to ensure that this rule is followed?
On Wed, Jan 10, 2024 at 01:49:46PM -0600, David Lechner wrote:
> This adds an implementation of the SPI offload_ops to the AXI SPI Engine
> driver to provide offload support.
>
> Offload lookup is done by device property lookup. SPI Engine commands
> and tx data are recorded by writing to offload-specific FIFOs in the
> SPI Engine hardware.
Glancing through here I'm not seeing anything here that handles DMA
mapping, given that the controller will clearly be doing DMA here that
seems surprising.
On Wed, Jan 10, 2024 at 3:39 PM Mark Brown <[email protected]> wrote:
>
> On Wed, Jan 10, 2024 at 01:49:46PM -0600, David Lechner wrote:
> > This adds an implementation of the SPI offload_ops to the AXI SPI Engine
> > driver to provide offload support.
> >
> > Offload lookup is done by device property lookup. SPI Engine commands
> > and tx data are recorded by writing to offload-specific FIFOs in the
> > SPI Engine hardware.
>
> Glancing through here I'm not seeing anything here that handles DMA
> mapping, given that the controller will clearly be doing DMA here that
> seems surprising.
In the use case implemented in this series, the RX data is going to
DMA, but in general, that doesn't have to be the case. In theory, it
could get piped directly to a DSP or something like that. So I left
the RX DMA part out of the SPI controller and implemented as a
separate device in "iio: offload: add new PWM triggered DMA buffer
driver". The SPI controller itself isn't aware that it is connected to
DMA (i.e. there are no registers that have to be poked to enable DMA
or anything like that).
On Wed, Jan 10, 2024 at 01:49:52PM -0600, David Lechner wrote:
> This adds a new binding for a PWM trigger and DMA data output connected
> to an SPI controller offload instance.
>
> Signed-off-by: David Lechner <[email protected]>
> ---
> .../adi,spi-offload-pwm-trigger-dma-buffer.yaml | 59 ++++++++++++++++++++++
> 1 file changed, 59 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/offload/adi,spi-offload-pwm-trigger-dma-buffer.yaml b/Documentation/devicetree/bindings/iio/offload/adi,spi-offload-pwm-trigger-dma-buffer.yaml
> new file mode 100644
> index 000000000000..748cfab19eff
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/offload/adi,spi-offload-pwm-trigger-dma-buffer.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/offload/adi,spi-offload-pwm-trigger-dma-buffer.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SPI Offload with PWM Trigger and DMA Buffer Data Output
> +
> +maintainers:
> + - Michael Hennerich <[email protected]>
> + - Nuno S? <[email protected]>
> +
> +description: |
> + This binding describes the connection of a PWM device to the trigger input
> + and a DMA channel to the output data stream of a SPI Offload instance.
> +
> + https://wiki.analog.com/resources/fpga/peripherals/spi_engine/offload
> + https://wiki.analog.com/resources/fpga/peripherals/spi_engine/tutorial
> +
> +$ref: /schemas/spi/adi,axi-spi-engine.yaml#/$defs/offload
Not really worth the complexity just for 'reg'. Generally, the bus
schema would define general constraints on reg like range of address
values and the device schema (this one) is just how many entries.
> +
> +properties:
> + compatible:
> + const: adi,spi-offload-pwm-trigger-dma-buffer
> +
> + reg:
> + maxItems: 1
> +
> + pwms:
> + maxItems: 1
> +
> + dmas:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - pwms
> + - dmas
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + offloads {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + offload@0 {
> + compatible = "adi,spi-offload-pwm-trigger-dma-buffer";
> + reg = <0>;
> + pwms = <&pwm 0>;
> + dmas = <&dma 0>;
> + };
> + };
Just make one complete example for the device.
> + };
>
> --
> 2.43.0
>
On Wed, Jan 10, 2024 at 01:49:45PM -0600, David Lechner wrote:
> The ADI AXI SPI Engine driver supports offloading SPI transfers to
> hardware. This is essentially a feature that allows recording an
> arbitrary sequence of SPI transfers and then playing them back with
> no CPU intervention via a hardware trigger.
>
> This adds the bindings for this feature. Each SPI Engine instance
> can have from 0 to 32 offload instances. Each offload instance has a
> trigger input and a data stream output. As an example, this could be
> used with an ADC SPI peripheral. In this case the trigger is connected
> to a PWM/clock to determine the sampling rate for the ADC and the output
> stream is connected to a DMA channel to pipe the sample data to memory.
>
> SPI peripherals act as consumers of the offload instances. Typically,
> one SPI peripheral will be connected to one offload instance. But to
> make the bindings future-proof, the property is an array.
Is there some sort of arbitration between multiple offload engines on
the same chip select? If not, I don't see how it would work.
I think this whole thing could be simplified down to just 3
SPI controller properties: pwms, dmas, and adi,offload-cs-map. Each
property is has entries equal the number of offload engines. The last
one maps an offload engine to a SPI chip-select.
>
> Signed-off-by: David Lechner <[email protected]>
> ---
> .../spi/adi,axi-spi-engine-peripheral-props.yaml | 24 +++++++++++
> .../bindings/spi/adi,axi-spi-engine.yaml | 49 +++++++++++++++++++++-
> .../bindings/spi/spi-peripheral-props.yaml | 1 +
> 3 files changed, 73 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/spi/adi,axi-spi-engine-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine-peripheral-props.yaml
> new file mode 100644
> index 000000000000..19b685fc3b39
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine-peripheral-props.yaml
> @@ -0,0 +1,24 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/adi,axi-spi-engine-peripheral-props.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Peripheral properties for Analog Devices AXI SPI Engine Controller
> +
> +maintainers:
> + - Michael Hennerich <[email protected]>
> + - Nuno S? <[email protected]>
> +
> +properties:
> + adi,offloads:
> + description:
> + List of AXI SPI Engine offload instances assigned to this peripheral.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + maxItems: 32
> + items:
> + items:
> + - minimum: 0
> + maximum: 31
This defines a matrix. You want:
minItems: 1
maxItems: 32
items:
maximum: 31
(0 is already the min).
> +
> +additionalProperties: true
> diff --git a/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
> index d48faa42d025..69f3261bab47 100644
> --- a/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
> +++ b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
> @@ -21,6 +21,23 @@ maintainers:
> allOf:
> - $ref: /schemas/spi/spi-controller.yaml#
>
> +$defs:
> + offload:
> + description:
> + Describes the connections of the trigger input and the data output stream
> + of one or more offload instances.
> +
> + properties:
> + reg:
> + description:
> + Index of the offload instance.
> + items:
> + - minimum: 0
> + maximum: 31
> +
> + required:
> + - reg
> +
> properties:
> compatible:
> const: adi,axi-spi-engine-1.00.a
> @@ -41,6 +58,22 @@ properties:
> - const: s_axi_aclk
> - const: spi_clk
>
> + offloads:
> + type: object
> + description: Zero or more offloads supported by the controller.
> +
> + properties:
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + patternProperties:
> + "^offload@[0-8a-f]+$":
> + type: object
> + $ref: '#/$defs/offload'
> +
> required:
> - compatible
> - reg
> @@ -62,5 +95,19 @@ examples:
> #address-cells = <1>;
> #size-cells = <0>;
>
> - /* SPI devices */
> + offloads {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + offload@0 {
> + compatible = "adi,example-offload";
No fake examples please. This should give you a warning.
> + reg = <0>;
> + };
> + };
> +
> + adc@0 {
> + compatible = "adi,example-adc";
> + reg = <0>;
> + adi,offloads = <0>;
> + };
> };
> diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> index 1c8e71c18234..7beb5a3798a5 100644
> --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> @@ -132,6 +132,7 @@ properties:
>
> # The controller specific properties go here.
> allOf:
> + - $ref: adi,axi-spi-engine-peripheral-props.yaml#
> - $ref: arm,pl022-peripheral-props.yaml#
> - $ref: cdns,qspi-nor-peripheral-props.yaml#
> - $ref: samsung,spi-peripheral-props.yaml#
>
> --
> 2.43.0
>
On Wed, Jan 10, 2024 at 5:15 PM Rob Herring <[email protected]> wrote:
>
> On Wed, Jan 10, 2024 at 01:49:45PM -0600, David Lechner wrote:
> > The ADI AXI SPI Engine driver supports offloading SPI transfers to
> > hardware. This is essentially a feature that allows recording an
> > arbitrary sequence of SPI transfers and then playing them back with
> > no CPU intervention via a hardware trigger.
> >
> > This adds the bindings for this feature. Each SPI Engine instance
> > can have from 0 to 32 offload instances. Each offload instance has a
> > trigger input and a data stream output. As an example, this could be
> > used with an ADC SPI peripheral. In this case the trigger is connected
> > to a PWM/clock to determine the sampling rate for the ADC and the output
> > stream is connected to a DMA channel to pipe the sample data to memory.
> >
> > SPI peripherals act as consumers of the offload instances. Typically,
> > one SPI peripheral will be connected to one offload instance. But to
> > make the bindings future-proof, the property is an array.
>
> Is there some sort of arbitration between multiple offload engines on
> the same chip select? If not, I don't see how it would work.
There is only one SPI engine driving the SPI controller, so if two
offloads are triggered at the same time, they will be executed
serially.
>
> I think this whole thing could be simplified down to just 3
> SPI controller properties: pwms, dmas, and adi,offload-cs-map. Each
> property is has entries equal the number of offload engines. The last
> one maps an offload engine to a SPI chip-select.
Offloads could be connected to virtually anything, not just pwms and
dmas, so making pwms and dmas controller properties doesn't seem right
to me. What if we have one that uses a gpio trigger or clock trigger?
Or what if we have one where the output goes to a DSP instead of DMA?
This is why I made offload@ nodes with a compatible property - to
describe what is actually connected to each offload instance since it
could be an unlimited range of different things.
>
> >
> > Signed-off-by: David Lechner <[email protected]>
> > ---
> > .../spi/adi,axi-spi-engine-peripheral-props.yaml | 24 +++++++++++
> > .../bindings/spi/adi,axi-spi-engine.yaml | 49 +++++++++++++++++++++-
> > .../bindings/spi/spi-peripheral-props.yaml | 1 +
> > 3 files changed, 73 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/spi/adi,axi-spi-engine-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine-peripheral-props.yaml
> > new file mode 100644
> > index 000000000000..19b685fc3b39
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine-peripheral-props.yaml
> > @@ -0,0 +1,24 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/spi/adi,axi-spi-engine-peripheral-props.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Peripheral properties for Analog Devices AXI SPI Engine Controller
> > +
> > +maintainers:
> > + - Michael Hennerich <[email protected]>
> > + - Nuno Sá <[email protected]>
> > +
> > +properties:
> > + adi,offloads:
> > + description:
> > + List of AXI SPI Engine offload instances assigned to this peripheral.
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
> > + maxItems: 32
> > + items:
> > + items:
> > + - minimum: 0
> > + maximum: 31
>
> This defines a matrix. You want:
>
> minItems: 1
> maxItems: 32
> items:
> maximum: 31
>
> (0 is already the min).
thanks
>
> > +
> > +additionalProperties: true
> > diff --git a/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
> > index d48faa42d025..69f3261bab47 100644
> > --- a/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
> > +++ b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
> > @@ -21,6 +21,23 @@ maintainers:
> > allOf:
> > - $ref: /schemas/spi/spi-controller.yaml#
> >
> > +$defs:
> > + offload:
> > + description:
> > + Describes the connections of the trigger input and the data output stream
> > + of one or more offload instances.
> > +
> > + properties:
> > + reg:
> > + description:
> > + Index of the offload instance.
> > + items:
> > + - minimum: 0
> > + maximum: 31
> > +
> > + required:
> > + - reg
> > +
> > properties:
> > compatible:
> > const: adi,axi-spi-engine-1.00.a
> > @@ -41,6 +58,22 @@ properties:
> > - const: s_axi_aclk
> > - const: spi_clk
> >
> > + offloads:
> > + type: object
> > + description: Zero or more offloads supported by the controller.
> > +
> > + properties:
> > + "#address-cells":
> > + const: 1
> > +
> > + "#size-cells":
> > + const: 0
> > +
> > + patternProperties:
> > + "^offload@[0-8a-f]+$":
> > + type: object
> > + $ref: '#/$defs/offload'
> > +
> > required:
> > - compatible
> > - reg
> > @@ -62,5 +95,19 @@ examples:
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > - /* SPI devices */
> > + offloads {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + offload@0 {
> > + compatible = "adi,example-offload";
>
> No fake examples please. This should give you a warning.
Ack.
FYI, unknown compatibles don't currently give a warning.
$ dt-validate --version
2023.12.dev6+gfb80ec4
$ make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
ARCH=arm KBUILD_OUTPUT=\$HOME/build-area/ad7944-mainline
make[1]: Entering directory
'/home/david/work/linux/OME/build-area/ad7944-mainline'
DTEX Documentation/devicetree/bindings/spi/adi,axi-spi-engine.example.dts
DTC_CHK Documentation/devicetree/bindings/spi/adi,axi-spi-engine.example.dtb
make[1]: Leaving directory
'/home/david/work/linux/OME/build-area/ad7944-mainline'
>
> > + reg = <0>;
> > + };
> > + };
> > +
> > + adc@0 {
> > + compatible = "adi,example-adc";
> > + reg = <0>;
> > + adi,offloads = <0>;
> > + };
> > };
> > diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-propsyaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > index 1c8e71c18234..7beb5a3798a5 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > @@ -132,6 +132,7 @@ properties:
> >
> > # The controller specific properties go here.
> > allOf:
> > + - $ref: adi,axi-spi-engine-peripheral-props.yaml#
> > - $ref: arm,pl022-peripheral-props.yaml#
> > - $ref: cdns,qspi-nor-peripheral-props.yaml#
> > - $ref: samsung,spi-peripheral-props.yaml#
> >
> > --
> > 2.43.0
> >
Hi David,
On Wed, 2024-01-10 at 13:49 -0600, David Lechner wrote:
> This adds a feature for specialized SPI controllers that can record
> a series of SPI transfers, including tx data, cs assertions, delays,
> etc. and then play them back using a hardware trigger without CPU
> intervention.
>
> The intended use case for this is with the AXI SPI Engine to capture
> data from ADCs at high rates (MSPS) with a stable sample period.
>
> Most of the implementation is controller-specific and will be handled by
> drivers that implement the offload_ops callbacks. The API follows a
> prepare/enable pattern that should be familiar to users of the clk
> subsystem.
>
> Consumers of this API will make calls similar to this:
>
> /* in probe() */
> offload = spi_offload_get(spi, 0);
> ...
>
On top of what Mark already stated, and as we already discussed offline, I
personally don't like this provider - consumer interface for the offload. The
first thing is that this is taking into account the possibility of having
multiple offload cores. While the FGPA core was designed with that in mind, we
don't really have any design using multiple offloads in one spi engine (always
one). Hence this is all pretty much untested.
If we want to already have this support, my feeling is that we should have a
simple integer dt property for the peripheral devices (similar to cs). When a
device is being created/added, the spi core would parse this property and get
it's offload index. The point is that this would all be transparent for spi
devices drivers that would only have to call the SPI API's and the core would
make sure the right index is passed to the controller.
But honestly, IMO, I would just keep things simple for now and assume one core
per engine.
I would probably also prefer to see all the new interfaces part of the
spi_controller struct directly...
- Nuno Sá
On Wed, 2024-01-10 at 17:14 -0600, Rob Herring wrote:
> On Wed, Jan 10, 2024 at 01:49:45PM -0600, David Lechner wrote:
> > The ADI AXI SPI Engine driver supports offloading SPI transfers to
> > hardware. This is essentially a feature that allows recording an
> > arbitrary sequence of SPI transfers and then playing them back with
> > no CPU intervention via a hardware trigger.
> >
> > This adds the bindings for this feature. Each SPI Engine instance
> > can have from 0 to 32 offload instances. Each offload instance has a
> > trigger input and a data stream output. As an example, this could be
> > used with an ADC SPI peripheral. In this case the trigger is connected
> > to a PWM/clock to determine the sampling rate for the ADC and the output
> > stream is connected to a DMA channel to pipe the sample data to memory.
> >
> > SPI peripherals act as consumers of the offload instances. Typically,
> > one SPI peripheral will be connected to one offload instance. But to
> > make the bindings future-proof, the property is an array.
>
> Is there some sort of arbitration between multiple offload engines on
> the same chip select? If not, I don't see how it would work.
>
> I think this whole thing could be simplified down to just 3
> SPI controller properties: pwms, dmas, and adi,offload-cs-map. Each
> property is has entries equal the number of offload engines. The last
> one maps an offload engine to a SPI chip-select.
>
I think the whole reason why the offload is being treated as a node + platform
device is to have these properties (or other possible properties depending on
the trigger and data capture being used) in it and so respect the HW
configuration.
While that is conceptually correct I feel that this is bringing a lot of extra
complexity. The end consumer of the offload core (which is a property/feature of
the spi cotroller) are obviously the peripheral devices that in our usecases are
converters and hence IIO devices. So those are the ones consuming all the data.
I saw Mark already giving some pointers and speaking about having a way to
support the triggers (being it pwm, clock, gpio, etc...) directly in the spi
framework and I think that would be nice. For the dmas, I think it would be more
complicated. While we can setup the dma transfer directly in the spi controller,
we would need a mechanism to transfer each block of data (periodically) as soon
as we have it to the peripheral device. In case of IIO, that would then have to
connect to IIO DMA buffers so the data can be consumed in userspace and that
would be the tricky part I believe.
What we have been doing out of tree is to control the trigger and dmas in the
peripheral device even if that does not really directly respect the HW setup (as
these are properties of the offload core). Hence, we can just allocate an IIO
DMA buffer, enable the offload message with the messages we want and data can be
directly transferred to userspace (without any intervention of the peripheral
driver) using the IIO interfaces for it.
To sum it up, I think having the trigger being handled by the spi framework or
even have it as an IIO generic trigger would be simple. To have the dma
transfers in the spi controller would be more complex but anything is possible,
I guess :).
- Nuno Sá
>
>
On Wed, 2024-01-10 at 18:06 -0600, David Lechner wrote:
> On Wed, Jan 10, 2024 at 5:15 PM Rob Herring <[email protected]> wrote:
> >
> > On Wed, Jan 10, 2024 at 01:49:45PM -0600, David Lechner wrote:
> > > The ADI AXI SPI Engine driver supports offloading SPI transfers to
> > > hardware. This is essentially a feature that allows recording an
> > > arbitrary sequence of SPI transfers and then playing them back with
> > > no CPU intervention via a hardware trigger.
> > >
> > > This adds the bindings for this feature. Each SPI Engine instance
> > > can have from 0 to 32 offload instances. Each offload instance has a
> > > trigger input and a data stream output. As an example, this could be
> > > used with an ADC SPI peripheral. In this case the trigger is connected
> > > to a PWM/clock to determine the sampling rate for the ADC and the output
> > > stream is connected to a DMA channel to pipe the sample data to memory.
> > >
> > > SPI peripherals act as consumers of the offload instances. Typically,
> > > one SPI peripheral will be connected to one offload instance. But to
> > > make the bindings future-proof, the property is an array.
> >
> > Is there some sort of arbitration between multiple offload engines on
> > the same chip select? If not, I don't see how it would work.
>
> There is only one SPI engine driving the SPI controller, so if two
> offloads are triggered at the same time, they will be executed
> serially.
>
> >
> > I think this whole thing could be simplified down to just 3
> > SPI controller properties: pwms, dmas, and adi,offload-cs-map. Each
> > property is has entries equal the number of offload engines. The last
> > one maps an offload engine to a SPI chip-select.
>
> Offloads could be connected to virtually anything, not just pwms and
> dmas, so making pwms and dmas controller properties doesn't seem right
> to me. What if we have one that uses a gpio trigger or clock trigger?
> Or what if we have one where the output goes to a DSP instead of DMA?
> This is why I made offload@ nodes with a compatible property - to
> describe what is actually connected to each offload instance since it
> could be an unlimited range of different things.
>
Yeah, again, that is conceptually correct but I'm just not sure if the extra
complexity pays off. The peripheral device connected to the offload should know
what trigger + data path it has. So I don't really think that horrible to have
the properties in the peripheral device. And there's not much that boilerplate
code anyways (at least in ADI usecases). And, as already said, for the triggers
we might be able to have something generic but for the dmas (or whatever else)
would be more tricky. In IIO case, setting up an IIO DMA buffer, is one API call
- so not much repetition in it...
- Nuno Sá
>
On Wed, 2024-01-10 at 13:49 -0600, David Lechner wrote:
> This adds a new hardware triggered buffer driver for the IIO subsystem.
> This driver is intended to be used by IIO device drivers that have
> a hardware buffer that is triggered by a hardware signal.
>
> It is expected that components such as those providing a backend via the
> IIO backend framework will provide the actual implementation of this
> functionality by registering a matching device on the auxiliary bus.
> The auxiliary bus was chosen since it allows us to make use of existing
> kernel infrastructure instead of implementing our own registration and
> lookup system.
>
> Signed-off-by: David Lechner <[email protected]>
> ---
> Documentation/driver-api/driver-model/devres.rst | 1 +
> drivers/iio/buffer/Kconfig | 7 ++
> drivers/iio/buffer/Makefile | 1 +
> .../iio/buffer/industrialio-hw-triggered-buffer.c | 104
> +++++++++++++++++++++
> include/linux/iio/hw_triggered_buffer.h | 14 +++
> include/linux/iio/hw_triggered_buffer_impl.h | 16 ++++
> 6 files changed, 143 insertions(+)
>
> diff --git a/Documentation/driver-api/driver-model/devres.rst
> b/Documentation/driver-api/driver-model/devres.rst
> index c5f99d834ec5..b23d4a2b68a6 100644
> --- a/Documentation/driver-api/driver-model/devres.rst
> +++ b/Documentation/driver-api/driver-model/devres.rst
> @@ -296,6 +296,7 @@ IIO
> devm_iio_channel_get()
> devm_iio_channel_get_all()
> devm_iio_hw_consumer_alloc()
> + devm_iio_hw_triggered_buffer_setup()
> devm_fwnode_iio_channel_get_by_name()
>
> INPUT
> diff --git a/drivers/iio/buffer/Kconfig b/drivers/iio/buffer/Kconfig
> index 047b931591a9..925c5bf074bc 100644
> --- a/drivers/iio/buffer/Kconfig
> +++ b/drivers/iio/buffer/Kconfig
> @@ -53,3 +53,10 @@ config IIO_TRIGGERED_BUFFER
> select IIO_KFIFO_BUF
> help
> Provides helper functions for setting up triggered buffers.
> +
> +config IIO_HW_TRIGGERED_BUFFER
> + tristate "Industrial I/O hardware triggered buffer support"
> + select AUXILIARY_BUS
> + select IIO_TRIGGER
> + help
> + Provides helper functions for setting up hardware triggered
> buffers.
> diff --git a/drivers/iio/buffer/Makefile b/drivers/iio/buffer/Makefile
> index 1403eb2f9409..d1142bb20f61 100644
> --- a/drivers/iio/buffer/Makefile
> +++ b/drivers/iio/buffer/Makefile
> @@ -9,4 +9,5 @@ obj-$(CONFIG_IIO_BUFFER_DMA) += industrialio-buffer-dmao
> obj-$(CONFIG_IIO_BUFFER_DMAENGINE) += industrialio-buffer-dmaengine.o
> obj-$(CONFIG_IIO_BUFFER_HW_CONSUMER) += industrialio-hw-consumer.o
> obj-$(CONFIG_IIO_TRIGGERED_BUFFER) += industrialio-triggered-buffer.o
> +obj-$(CONFIG_IIO_HW_TRIGGERED_BUFFER) += industrialio-hw-triggered-buffer.o
> obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o
> diff --git a/drivers/iio/buffer/industrialio-hw-triggered-buffer.c
> b/drivers/iio/buffer/industrialio-hw-triggered-buffer.c
> new file mode 100644
> index 000000000000..7a8a71066b0e
> --- /dev/null
> +++ b/drivers/iio/buffer/industrialio-hw-triggered-buffer.c
> @@ -0,0 +1,104 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2024 Analog Devices, Inc.
> + * Copyright (c) 2024 BayLibre, SAS
> + */
> +
> +#include <linux/auxiliary_bus.h>
> +#include <linux/container_of.h>
> +#include <linux/export.h>
> +#include <linux/module.h>
> +#include <linux/iio/hw_triggered_buffer_impl.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger.h>
> +
> +static int iio_hw_triggered_buffer_match(struct device *dev, const void
> *match)
> +{
> + return dev->parent == match;
> +}
> +
> +static struct iio_hw_triggered_buffer_device
> +*iio_hw_trigger_buffer_get(struct device *match)
> +{
> + struct auxiliary_device *adev;
> +
> + adev = auxiliary_find_device(NULL, match,
> iio_hw_triggered_buffer_match);
> + if (!adev)
> + return ERR_PTR(-ENOENT);
> +
> + return container_of(adev, struct iio_hw_triggered_buffer_device,
> adev);
> +}
> +
> +static void iio_hw_trigger_buffer_put(void *dev)
> +{
> + put_device(dev);
> +}
> +
> +/**
> + * devm_iio_hw_triggered_buffer_setup - Setup a hardware triggered buffer
> + * @dev: Device for devm management
> + * @indio_dev: An unconfigured/partially configured IIO device struct
> + * @match: Device for matching the auxiliary bus device that provides
> the
> + * interface to the hardware triggered buffer
> + * @ops: Buffer setup functions to use for this IIO device
> + *
> + * Return: 0 on success, negative error code on failure.
> + *
> + * This function will search all registered hardware triggered buffers for
> one
> + * that matches the given indio_dev. If found, it will be used to setup both
> + * the trigger and the buffer on the indio_dev.
> + */
> +int devm_iio_hw_triggered_buffer_setup(struct device *dev,
> + struct iio_dev *indio_dev,
> + struct device *match,
> + const struct iio_buffer_setup_ops
> *ops)
> +{
> + struct iio_hw_triggered_buffer_device *hw;
> + int ret;
> +
> + hw = iio_hw_trigger_buffer_get(match);
> + if (IS_ERR(hw))
> + return PTR_ERR(hw);
> +
> + ret = devm_add_action_or_reset(dev, iio_hw_trigger_buffer_put, &hw-
> >adev.dev);
> + if (ret)
> + return ret;
> +
> + indio_dev->modes |= INDIO_HW_BUFFER_TRIGGERED;
> + indio_dev->trig = iio_trigger_get(hw->trig);
> + indio_dev->setup_ops = ops;
> +
> + return iio_device_attach_buffer(indio_dev, hw->buffer);
> +}
> +EXPORT_SYMBOL_GPL(devm_iio_hw_triggered_buffer_setup);
> +
> +static int iio_hw_trigger_buffer_probe(struct auxiliary_device *adev,
> + const struct auxiliary_device_id *id)
> +{
> + struct iio_hw_triggered_buffer_device *hw =
> + container_of(adev, struct iio_hw_triggered_buffer_device,
> adev);
> +
> + if (!hw->buffer || !hw->trig)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static const struct auxiliary_device_id iio_hw_trigger_buffer_id_table[] = {
> + { }
> +};
> +MODULE_DEVICE_TABLE(auxiliary, iio_hw_trigger_buffer_id_table);
> +
> +static struct auxiliary_driver iio_hw_trigger_buffer_driver = {
> + .driver = {
> + .name = "iio-hw-triggered-buffer",
> + },
> + .probe = iio_hw_trigger_buffer_probe,
> + .id_table = iio_hw_trigger_buffer_id_table,
> +};
> +module_auxiliary_driver(iio_hw_trigger_buffer_driver);
This is one more example why I think the whole thing is overly complicated. If
I'm understanding things right, we have the spi controller creating platform
devices that will be probed by the new IIO offload driver that then creates an
auxiliary device that is probed by this driver. Even by looking at the probe
function, it already feels to me that something is architecturally wrong (as we
are essentially doing error handling in there).
One idea that just occurred to me now is to perhaps extend the IIO inkernel
interface so that we can split the logic a bit and have IIO devices consuming or
taking ownership of buffers created by other devices. Hmm, maybe not that good
of an idea :(
- 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 <[email protected]>
> ---
> 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, Jan 10, 2024 at 04:31:25PM -0600, David Lechner wrote:
> On Wed, Jan 10, 2024 at 3:39 PM Mark Brown <[email protected]> wrote:
> > Glancing through here I'm not seeing anything here that handles DMA
> > mapping, given that the controller will clearly be doing DMA here that
> > seems surprising.
> In the use case implemented in this series, the RX data is going to
> DMA, but in general, that doesn't have to be the case. In theory, it
> could get piped directly to a DSP or something like that. So I left
> the RX DMA part out of the SPI controller and implemented as a
> separate device in "iio: offload: add new PWM triggered DMA buffer
> driver". The SPI controller itself isn't aware that it is connected to
> DMA (i.e. there are no registers that have to be poked to enable DMA
> or anything like that).
If there's a buffer being assigned to the device (or removed from the
device) it needs mapping, this will ensure the device is allowed to
access it if there's IOMMUs involved, and that there's no pending cache
operations which could corrupt data.
On Thu, Jan 11, 2024 at 09:49:08AM +0100, Nuno S? wrote:
> On Wed, 2024-01-10 at 13:49 -0600, David Lechner wrote:
> > ??? /* in probe() */
> > ??? offload = spi_offload_get(spi, 0);
> On top of what Mark already stated, and as we already discussed offline, I
> personally don't like this provider - consumer interface for the offload. The
> first thing is that this is taking into account the possibility of having
> multiple offload cores. While the FGPA core was designed with that in mind, we
> don't really have any design using multiple offloads in one spi engine (always
> one). Hence this is all pretty much untested.
I tend to agree that we shouldn't be exposing this to SPI device drivers
however we will want to keep track of if the unit is busy, and designing
it to cope with multiple offloads does seem like sensible future
proofing. There's also the possibility that one engine might be able to
cope with multiple scripts being active at once (eg, triggering a
different action depending on the trigger).
On Thu, 2024-01-11 at 13:33 +0000, Mark Brown wrote:
> On Thu, Jan 11, 2024 at 09:49:08AM +0100, Nuno Sá wrote:
> > On Wed, 2024-01-10 at 13:49 -0600, David Lechner wrote:
>
> > > /* in probe() */
> > > offload = spi_offload_get(spi, 0);
>
> > On top of what Mark already stated, and as we already discussed offline, I
> > personally don't like this provider - consumer interface for the offload.
> > The
> > first thing is that this is taking into account the possibility of having
> > multiple offload cores. While the FGPA core was designed with that in mind,
> > we
> > don't really have any design using multiple offloads in one spi engine
> > (always
> > one). Hence this is all pretty much untested.
>
> I tend to agree that we shouldn't be exposing this to SPI device drivers
> however we will want to keep track of if the unit is busy, and designing
> it to cope with multiple offloads does seem like sensible future
> proofing. There's also the possibility that one engine might be able to
Fair enough. But wouldn't a simple DT integer property (handled by the spi core)
to identify the offload index be easier for SPI device drivers? We could still
have dedicated interfaces for checking if the unit is busy or not... The point
is that we would not need an explicit get() from SPI drivers.
I'm of course assuming that one spi device can only be connected to one engine
which seems reasonable to me.
- 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 <[email protected]>
> ---
> 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 Thu, Jan 11, 2024 at 03:11:32PM +0100, Nuno S? wrote:
> On Thu, 2024-01-11 at 13:33 +0000, Mark Brown wrote:
> > I tend to agree that we shouldn't be exposing this to SPI device drivers
> > however we will want to keep track of if the unit is busy, and designing
> > it to cope with multiple offloads does seem like sensible future
> > proofing.? There's also the possibility that one engine might be able to
> Fair enough. But wouldn't a simple DT integer property (handled by the spi core)
> to identify the offload index be easier for SPI device drivers? We could still
> have dedicated interfaces for checking if the unit is busy or not... The point
> is that we would not need an explicit get() from SPI drivers.
It feels like we'd need a get/release operation of some kind for mutual
exclusion, it's not just the discovery it's also figuring out if the
hardware is in use at a given moment.
> I'm of course assuming that one spi device can only be connected to one engine
> which seems reasonable to me.
I can see someone implementing this with for example the microcontroller
cores a lot of SoCs have in which case all bets are off.
On Thu, Jan 11, 2024 at 7:00 AM Mark Brown <[email protected]> wrote:
>
> On Wed, Jan 10, 2024 at 04:31:25PM -0600, David Lechner wrote:
> > On Wed, Jan 10, 2024 at 3:39 PM Mark Brown <[email protected]> wrote:
>
> > > Glancing through here I'm not seeing anything here that handles DMA
> > > mapping, given that the controller will clearly be doing DMA here that
> > > seems surprising.
>
> > In the use case implemented in this series, the RX data is going to
> > DMA, but in general, that doesn't have to be the case. In theory, it
> > could get piped directly to a DSP or something like that. So I left
> > the RX DMA part out of the SPI controller and implemented as a
> > separate device in "iio: offload: add new PWM triggered DMA buffer
> > driver". The SPI controller itself isn't aware that it is connected to
> > DMA (i.e. there are no registers that have to be poked to enable DMA
> > or anything like that).
>
> If there's a buffer being assigned to the device (or removed from the
> device) it needs mapping, this will ensure the device is allowed to
> access it if there's IOMMUs involved, and that there's no pending cache
> operations which could corrupt data.
Currently, in this series, the mapping is being handled by the
existing DMA buffer framework in the IIO subsystem. It is the IIO
device that owns/manages the DMA rather than the SPI controller. Nuno
has also made some relevant comments in some of the other threads
about why it would be preferable to do it that way. But this sounds
like something we should come back to later after we have a look at
breaking down this series into smaller parts.
On Wed, Jan 10, 2024 at 3:36 PM Mark Brown <[email protected]> wrote:
>
> On Wed, Jan 10, 2024 at 01:49:42PM -0600, David Lechner wrote:
> > This adds a feature for specialized SPI controllers that can record
> > a series of SPI transfers, including tx data, cs assertions, delays,
> > etc. and then play them back using a hardware trigger without CPU
> > intervention.
>
> > The intended use case for this is with the AXI SPI Engine to capture
> > data from ADCs at high rates (MSPS) with a stable sample period.
>
> > Most of the implementation is controller-specific and will be handled by
> > drivers that implement the offload_ops callbacks. The API follows a
> > prepare/enable pattern that should be familiar to users of the clk
> > subsystem.
>
> This is a lot to do in one go, and I think it's a bit too off on the
> side and unintegrated with the core. There's two very high level bits
> here, there's the pre-cooking a message for offloading to be executed by
> a hardware engine and there's the bit where that's triggered by some
> hardwar event rather than by software.
>
> There was a bunch of discussion of the former case with David Jander
I found [1] which appears to be the conversation you are referring to.
Is that all or is there more that I missed?
[1]: https://lore.kernel.org/linux-spi/20220512163445.6dcca126@erd992/
> (CCed) a while back when he was doing all the work he did on optimising
> the core for uncontended uses, the thinking there was to have a
> spi_prepare_message() (or similar) API that drivers could call and then
> reuse the same transfer repeatedly, and even without any interface for
> client drivers it's likely that we'd be able to take advantage of it in
> the core for multi-transfer messages. I'd be surprised if there weren't
> wins when the message goes over the DMA copybreak size. A much wider
> range of hardware would be able to do this bit, for example David's case
> was a Raspberry Pi using the DMA controller to write into the SPI
> controller control registers in order to program it for multiple
> transfers, bounce chip select and so on. You could also use the
> microcontroller cores that many embedded SoCs have, and even with zero
> hardware support for offloading anything there's savings in the message
> validation and DMA mapping.
>
I can see how such a spi_prepare_message() API could be useful in
general and would be a good first step towards what we are wanting to
accomplish too.
For example, in the IIO subsystem, it is a common pattern when using a
triggered buffer to prepare some spi xfer structs in the buffer setup
phase that get reused multiple times. So this could, as you said, at
least save the overhead of validating/mapping the same xfers over and
over.
I will look into this first and then we can come back to the second
part about hardware triggers once that is done.
On Thu, Jan 11, 2024 at 2:54 PM David Lechner <[email protected]> wrote:
>
> On Wed, Jan 10, 2024 at 3:36 PM Mark Brown <[email protected]> wrote:
> >
> > On Wed, Jan 10, 2024 at 01:49:42PM -0600, David Lechner wrote:
> > > This adds a feature for specialized SPI controllers that can record
> > > a series of SPI transfers, including tx data, cs assertions, delays,
> > > etc. and then play them back using a hardware trigger without CPU
> > > intervention.
> >
> > > The intended use case for this is with the AXI SPI Engine to capture
> > > data from ADCs at high rates (MSPS) with a stable sample period.
> >
> > > Most of the implementation is controller-specific and will be handled by
> > > drivers that implement the offload_ops callbacks. The API follows a
> > > prepare/enable pattern that should be familiar to users of the clk
> > > subsystem.
> >
> > This is a lot to do in one go, and I think it's a bit too off on the
> > side and unintegrated with the core. There's two very high level bits
> > here, there's the pre-cooking a message for offloading to be executed by
> > a hardware engine and there's the bit where that's triggered by some
> > hardwar event rather than by software.
> >
> > There was a bunch of discussion of the former case with David Jander
>
> I found [1] which appears to be the conversation you are referring to.
> Is that all or is there more that I missed?
>
> [1]: https://lore.kernel.org/linux-spi/20220512163445.6dcca126@erd992/
>
> > (CCed) a while back when he was doing all the work he did on optimising
> > the core for uncontended uses, the thinking there was to have a
> > spi_prepare_message() (or similar) API that drivers could call and then
> > reuse the same transfer repeatedly, and even without any interface for
> > client drivers it's likely that we'd be able to take advantage of it in
> > the core for multi-transfer messages. I'd be surprised if there weren't
> > wins when the message goes over the DMA copybreak size. A much wider
> > range of hardware would be able to do this bit, for example David's case
> > was a Raspberry Pi using the DMA controller to write into the SPI
For those, following along, it looks like the RPi business was
actually a 2013 discussion with Martin Sperl [2]. Both this and [1]
discuss proposed spi_prepare_message() APIs.
[2]: https://lore.kernel.org/linux-spi/CACRpkdb4mn_Hxg=3tuBu89n6eyJ082EETkwtNbzZDFZYTHbVVg@mail.gmail.com/T/#u
On Thu, Jan 11, 2024 at 03:32:54PM -0600, David Lechner wrote:
> On Thu, Jan 11, 2024 at 2:54 PM David Lechner <[email protected]> wrote:
> > > (CCed) a while back when he was doing all the work he did on optimising
> > > the core for uncontended uses, the thinking there was to have a
> > > spi_prepare_message() (or similar) API that drivers could call and then
> > > reuse the same transfer repeatedly, and even without any interface for
> > > client drivers it's likely that we'd be able to take advantage of it in
> > > the core for multi-transfer messages. I'd be surprised if there weren't
> > > wins when the message goes over the DMA copybreak size. A much wider
> > > range of hardware would be able to do this bit, for example David's case
> > > was a Raspberry Pi using the DMA controller to write into the SPI
> For those, following along, it looks like the RPi business was
> actually a 2013 discussion with Martin Sperl [2]. Both this and [1]
> discuss proposed spi_prepare_message() APIs.
> [2]: https://lore.kernel.org/linux-spi/CACRpkdb4mn_Hxg=3tuBu89n6eyJ082EETkwtNbzZDFZYTHbVVg@mail.gmail.com/T/#u
Oh, yes - sorry, I'd misremembered which optimisation effort it was
associated with. Apologies.
On Wed, Jan 10, 2024 at 1:51 PM David Lechner <[email protected]> wrote:
>
> According to the spi-controller.yaml bindings, SPI peripheral child
> nodes match the pattern "^.*@[0-9a-f]+$".
>
> A SPI controller binding may require a child object node that is not a
> peripheral. For example, the adi,axi-spi-engine binding requires an
> "offloads" child node that is not a peripheral but rather a part of the
> controller itself.
>
> By checking for '@' in the node name, we can avoids a warnings like:
>
> Warning (spi_bus_reg): /example-0/spi@44a00000/offloads: missing or empty reg property
>
> for a binding like:
>
> spi {
> ...
>
> offloads {
> offload@0 {
> ...
> };
> ...
> };
>
> peripheral@0 {
> ...
> };
> };
>
> Signed-off-by: David Lechner <[email protected]>
> ---
> scripts/dtc/checks.c | 4 ++++
> 1 file changed, 4 insertions(+)
Check the commit history. We don't take changes to kernel's dtc copy.
They must go upstream first.
Rob
On Thu, 2024-01-11 at 15:41 +0000, Mark Brown wrote:
> On Thu, Jan 11, 2024 at 03:11:32PM +0100, Nuno Sá wrote:
> > On Thu, 2024-01-11 at 13:33 +0000, Mark Brown wrote:
>
> > > I tend to agree that we shouldn't be exposing this to SPI device drivers
> > > however we will want to keep track of if the unit is busy, and designing
> > > it to cope with multiple offloads does seem like sensible future
> > > proofing. There's also the possibility that one engine might be able to
>
> > Fair enough. But wouldn't a simple DT integer property (handled by the spi core)
> > to identify the offload index be easier for SPI device drivers? We could still
> > have dedicated interfaces for checking if the unit is busy or not... The point
> > is that we would not need an explicit get() from SPI drivers.
>
> It feels like we'd need a get/release operation of some kind for mutual
> exclusion, it's not just the discovery it's also figuring out if the
> hardware is in use at a given moment.
>
Hmm did not thought about the busy case. Still, I could see this being easily done on
the controller driver (at least until we have a clear idea if this is useful or if it
will attract more users) or even at the spi core on the prepare + unprepare
interfaces. A flag could be enough to return EBUSY if someone is already in the
process of preparing + enabling the engine.
Bah, anyways, it's just I'm really not thrilled about that kind of interface in here
but yeah, as long as you think it's worth it...
>
- Nuno Sá
Hi Mark, David,
Thanks for CC'ing me. Been reading the discussion so far.
On Thu, 11 Jan 2024 21:49:53 +0000
Mark Brown <[email protected]> wrote:
> On Thu, Jan 11, 2024 at 03:32:54PM -0600, David Lechner wrote:
> > On Thu, Jan 11, 2024 at 2:54 PM David Lechner <[email protected]> wrote:
>
> > > > (CCed) a while back when he was doing all the work he did on optimising
> > > > the core for uncontended uses, the thinking there was to have a
> > > > spi_prepare_message() (or similar) API that drivers could call and then
> > > > reuse the same transfer repeatedly, and even without any interface for
> > > > client drivers it's likely that we'd be able to take advantage of it in
> > > > the core for multi-transfer messages. I'd be surprised if there weren't
> > > > wins when the message goes over the DMA copybreak size. A much wider
> > > > range of hardware would be able to do this bit, for example David's case
> > > > was a Raspberry Pi using the DMA controller to write into the SPI
>
> > For those, following along, it looks like the RPi business was
> > actually a 2013 discussion with Martin Sperl [2]. Both this and [1]
> > discuss proposed spi_prepare_message() APIs.
>
> > [2]: https://lore.kernel.org/linux-spi/CACRpkdb4mn_Hxg=3tuBu89n6eyJ082EETkwtNbzZDFZYTHbVVg@mail.gmail.com/T/#u
>
> Oh, yes - sorry, I'd misremembered which optimisation effort it was
> associated with. Apologies.
Yes. It was Martin Sperl who proposed this on a Rpi. I mentioned something
similar toward the end of my 2nd email reply in that thread [1]. That might
have triggered the confusion.
As for my interests, I am all for devising ways to make the SPI subsystem more
suitable for optimized high-performance use-cases. In that regard, I think
re-usable messages (spi_prepare_message()) can be useful. More capable
hardware can enable very powerful use-cases for SPI, and it would be cool if
the spi subsystem had the needed infrastructure to support those. As for
hardware-triggers, I still need to wrap my head around how to have a
universally usable API that works nice for the first use-case that comes along
and also doesn't screw up the next use-case that might follow. Keep me posted.
[1] https://lore.kernel.org/linux-spi/20220513144645.2d16475c@erd992/
Best regards,
--
David Jander
Protonic Holland.
On Fri, 2024-01-12 at 12:51 +0000, Jonathan Cameron wrote:
> On Thu, 11 Jan 2024 15:59:02 +0100
> Nuno Sá <[email protected]> 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 <[email protected]>
> > > ---
> > > 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á
On Fri, Jan 12, 2024 at 6:38 AM Jonathan Cameron
<[email protected]> wrote:
>
> On Wed, 10 Jan 2024 13:49:51 -0600
> David Lechner <[email protected]> wrote:
>
> > This changes devm_iio_dmaengine_buffer_alloc() to an exported symbol.
> > This will be used by drivers that need to allocate a DMA buffer without
> > attaching it to an IIO device.
> >
> > Signed-off-by: David Lechner <[email protected]>
> > ---
> > Documentation/driver-api/driver-model/devres.rst | 1 +
> > drivers/iio/buffer/Kconfig | 14 +++++++-------
> > drivers/iio/buffer/industrialio-buffer-dmaengine.c | 3 ++-
> > include/linux/iio/buffer-dmaengine.h | 2 ++
> > 4 files changed, 12 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst
> > index b23d4a2b68a6..60e4b7ba38e5 100644
> > --- a/Documentation/driver-api/driver-model/devres.rst
> > +++ b/Documentation/driver-api/driver-model/devres.rst
> > @@ -285,6 +285,7 @@ I2C
> > IIO
> > devm_iio_device_alloc()
> > devm_iio_device_register()
> > + devm_iio_dmaengine_buffer_alloc()
> > devm_iio_dmaengine_buffer_setup()
> > devm_iio_kfifo_buffer_setup()
> > devm_iio_kfifo_buffer_setup_ext()
> > diff --git a/drivers/iio/buffer/Kconfig b/drivers/iio/buffer/Kconfig
> > index 925c5bf074bc..27d82fb4bc4d 100644
> > --- a/drivers/iio/buffer/Kconfig
> > +++ b/drivers/iio/buffer/Kconfig
> > @@ -40,6 +40,13 @@ config IIO_BUFFER_HW_CONSUMER
> > Should be selected by drivers that want to use the generic Hw consumer
> > interface.
> >
> > +config IIO_HW_TRIGGERED_BUFFER
> > + tristate "Industrial I/O hardware triggered buffer support"
> > + select AUXILIARY_BUS
> > + select IIO_TRIGGER
> > + help
> > + Provides helper functions for setting up hardware triggered buffers.
> > +
> > config IIO_KFIFO_BUF
> > tristate "Industrial I/O buffering based on kfifo"
> > help
> > @@ -53,10 +60,3 @@ config IIO_TRIGGERED_BUFFER
> > select IIO_KFIFO_BUF
> > help
> > Provides helper functions for setting up triggered buffers.
> > -
> > -config IIO_HW_TRIGGERED_BUFFER
> > - tristate "Industrial I/O hardware triggered buffer support"
> > - select AUXILIARY_BUS
> > - select IIO_TRIGGER
> > - help
> > - Provides helper functions for setting up hardware triggered buffers.
>
> Why move this?
>
Oops, this shouldn't be here. Looks like an artifact from a botched
rebase. The intention was to put it in alphabetical order.
On Fri, Jan 12, 2024 at 6:37 AM Jonathan Cameron
<[email protected]> wrote:
>
> On Wed, 10 Jan 2024 13:49:47 -0600
> David Lechner <[email protected]> wrote:
>
> > This adds a new mode INDIO_HW_BUFFER_TRIGGERED to the IIO subsystem.
> >
> > This mode is essentially the hardware version of INDIO_BUFFER_TRIGGERED
> > where the trigger has the semantics of INDIO_HARDWARE_TRIGGERED and the
> > buffer has the semantics of INDIO_BUFFER_HARDWARE.
> >
> > So basically INDIO_HW_BUFFER_TRIGGERED is the same as
> > INDIO_BUFFER_HARDWARE except that it also enables the trigger when the
> > buffer is enabled.
>
> If the trigger isn't routeable to multiple devices we normally don't
> make it visible at all.
>
> I'm not yet understanding what a trigger actually means in this case.
> Why do you need it to be userspace configurable?
>
> J
>
It looks like this question was answered in another thread (we need to
configure the sampling frequency from userspace). But there you
mentioned that adding a trigger for that seemed overkill. So you would
you suggest to add the sampling_frequency sysfs attribute to the
iio:deviceX instead and just forget about the trigger part? It seems a
bit odd to me to have an attribute that may or may not be there
depending other hardware external to the ADC chip. But if that is a
normal acceptable thing to do, then it does seem like the simpler
thing to do.
On Fri, 2024-01-12 at 09:42 -0600, David Lechner wrote:
> On Fri, Jan 12, 2024 at 6:37 AM Jonathan Cameron
> <[email protected]> wrote:
> >
> > On Wed, 10 Jan 2024 13:49:47 -0600
> > David Lechner <[email protected]> wrote:
> >
> > > This adds a new mode INDIO_HW_BUFFER_TRIGGERED to the IIO subsystem.
> > >
> > > This mode is essentially the hardware version of INDIO_BUFFER_TRIGGERED
> > > where the trigger has the semantics of INDIO_HARDWARE_TRIGGERED and the
> > > buffer has the semantics of INDIO_BUFFER_HARDWARE.
> > >
> > > So basically INDIO_HW_BUFFER_TRIGGERED is the same as
> > > INDIO_BUFFER_HARDWARE except that it also enables the trigger when the
> > > buffer is enabled.
> >
> > If the trigger isn't routeable to multiple devices we normally don't
> > make it visible at all.
> >
> > I'm not yet understanding what a trigger actually means in this case.
> > Why do you need it to be userspace configurable?
> >
> > J
> >
>
> It looks like this question was answered in another thread (we need to
> configure the sampling frequency from userspace). But there you
> mentioned that adding a trigger for that seemed overkill. So you would
> you suggest to add the sampling_frequency sysfs attribute to the
> iio:deviceX instead and just forget about the trigger part? It seems a
> bit odd to me to have an attribute that may or may not be there
> depending other hardware external to the ADC chip. But if that is a
> normal acceptable thing to do, then it does seem like the simpler
> thing to do.
Well, for these converters you usually always need some sort of trigger to tell the
engine to fetch the data. But if not, you could make it optional in the driver (I
guess a trigger will always be a pwm, gpio, clk, etc...) and only expose the
interface if needed. Yes, if we start having tons of devices with optional triggers
(which is not the case so far) I agree we would be duplicating logic all over the
place.
But let's see where all this discussion leads us. AFAIU, having the triggers somehow
directly in the spi framework is also an option. If we can make that generic enough
and with a nice interface I think that would make the most sense as this trigger
affects the offload engine which is a spi controller thing. Let's see where we end up
:)
- Nuno Sá
On Thu, Jan 11, 2024 at 3:32 PM David Lechner <[email protected]> wrote:
>
> On Thu, Jan 11, 2024 at 2:54 PM David Lechner <[email protected]> wrote:
> >
> > On Wed, Jan 10, 2024 at 3:36 PM Mark Brown <[email protected]> wrote:
> > >
> > > On Wed, Jan 10, 2024 at 01:49:42PM -0600, David Lechner wrote:
> > > > This adds a feature for specialized SPI controllers that can record
> > > > a series of SPI transfers, including tx data, cs assertions, delays,
> > > > etc. and then play them back using a hardware trigger without CPU
> > > > intervention.
> > >
> > > > The intended use case for this is with the AXI SPI Engine to capture
> > > > data from ADCs at high rates (MSPS) with a stable sample period.
> > >
> > > > Most of the implementation is controller-specific and will be handled by
> > > > drivers that implement the offload_ops callbacks. The API follows a
> > > > prepare/enable pattern that should be familiar to users of the clk
> > > > subsystem.
> > >
> > > This is a lot to do in one go, and I think it's a bit too off on the
> > > side and unintegrated with the core. There's two very high level bits
> > > here, there's the pre-cooking a message for offloading to be executed by
> > > a hardware engine and there's the bit where that's triggered by some
> > > hardwar event rather than by software.
> > >
> > > There was a bunch of discussion of the former case with David Jander
> >
> > I found [1] which appears to be the conversation you are referring to.
> > Is that all or is there more that I missed?
> >
> > [1]: https://lore.kernel.org/linux-spi/20220512163445.6dcca126@erd992/
> >
> > > (CCed) a while back when he was doing all the work he did on optimising
> > > the core for uncontended uses, the thinking there was to have a
> > > spi_prepare_message() (or similar) API that drivers could call and then
> > > reuse the same transfer repeatedly, and even without any interface for
> > > client drivers it's likely that we'd be able to take advantage of it in
> > > the core for multi-transfer messages. I'd be surprised if there weren't
> > > wins when the message goes over the DMA copybreak size. A much wider
> > > range of hardware would be able to do this bit, for example David's case
> > > was a Raspberry Pi using the DMA controller to write into the SPI
>
> For those, following along, it looks like the RPi business was
> actually a 2013 discussion with Martin Sperl [2]. Both this and [1]
> discuss proposed spi_prepare_message() APIs.
>
> [2]: https://lore.kernel.org/linux-spi/CACRpkdb4mn_Hxg=3tuBu89n6eyJ082EETkwtNbzZDFZYTHbVVg@mail.gmail.com/T/#u
I found one more. A patch from Martin with the basic proposed API but
not much in the way of implementation. It looks like this is where the
idea fizzled out.
https://lore.kernel.org/linux-spi/[email protected]/
On Wed, Jan 10, 2024 at 3:36 PM Mark Brown <[email protected]> wrote:
>
> On Wed, Jan 10, 2024 at 01:49:42PM -0600, David Lechner wrote:
> > This adds a feature for specialized SPI controllers that can record
> > a series of SPI transfers, including tx data, cs assertions, delays,
> > etc. and then play them back using a hardware trigger without CPU
> > intervention.
>
> > The intended use case for this is with the AXI SPI Engine to capture
> > data from ADCs at high rates (MSPS) with a stable sample period.
>
> > Most of the implementation is controller-specific and will be handled by
> > drivers that implement the offload_ops callbacks. The API follows a
> > prepare/enable pattern that should be familiar to users of the clk
> > subsystem.
>
> This is a lot to do in one go, and I think it's a bit too off on the
> side and unintegrated with the core. There's two very high level bits
> here, there's the pre-cooking a message for offloading to be executed by
> a hardware engine and there's the bit where that's triggered by some
> hardwar event rather than by software.
>
..
>
> The bit where messages are initiated by hardware is a step beyond that,
> I think we need a bit more API for connecting up the triggers and we
> also need to have something handling what happens with normal operation
> of the device while these triggers are enabled. I think it could be
> useful to split this bit out since there's a lot more to work out there
> in terms of interfaces.
Now that we have addressed the pre-cooking messages bit [1] I'm coming
back to the hardware trigger bit. Since the hardware trigger part
hasn't been discussed in the past, it's not so clear to me what is
being requested here (also see specific questions below).
[1]: https://lore.kernel.org/linux-spi/20240219-mainline-spi-precook-message-v2-0-4a762c6701b9@baylibre.com/T/#t
>
> > +/**
> > + * SPI_OFFLOAD_RX - placeholder for indicating read transfers for offloads
> > + *
> > + * Assign xfer->rx_buf to this value for any read transfer passed to
> > + * spi_offload_prepare(). This will act as a flag to indicate to the offload
> > + * that it should do something with the data read during this transfer What
> > + * that something can be is determined by the specific hardware, e.g. it could
> > + * be piped to DMA or a DSP, etc.
> > + */
> > +#define SPI_OFFLOAD_RX_SENTINEL ((void *)1)
>
> This feels like something where there are likely to be multiple options
> and we need configurability. I'd also expect to see a similar transmit
> option.
Having something similar for TX makes sense. What other sorts of
options are you envisioning here?
>
> > +int spi_offload_prepare(struct spi_offload *offload, struct spi_device *spi,
> > + struct spi_transfer *xfers, unsigned int num_xfers)
>
> I would expect us to just generically prepare a message, then pass a
> prepared message into the API that enables a trigger. We would need
> something that handles the difference between potentially offloading for
> better performance and having a hardware trigger, I think that might be
> a case of just not exposing the engine's prepare to client drivers and
> then having the core track if it needs to do that when enabling a
> hardware trigger.
Not exposing the offload prepare to client drivers sounds reasonable.
I'm not sure I understand the potential need for an offload without a
hardware trigger though.
>
> > + /**
> > + * @enable: Callback to enable the offload.
> > + */
> > + int (*enable)(struct spi_offload *offload);
> > + /**
> > + * @disable: Callback to disable the offload.
> > + */
> > + void (*disable)(struct spi_offload *offload);
>
> I'm not seeing anything in this API that provides a mechanism for
> configuring what triggers things to start, even in the case where things
> are triggered by hardware rather than initiated by software I'd expect
> to see hardware with runtime configurability. The binding is a bit
> unclear but it seems to be expecting this to be statically configured in
> hardware and that there will be a 1:1 mapping between triggers and
> scripts that can be configured, if nothing else I would expect that
> there will be hardware with more possible triggers than scripts.
For the use case of ADCs/DACs we would want a periodic trigger where
the period of the trigger is runtime configurable (via sysfs). Is this
the sort of thing you had in mind here? What other sorts of triggers
do you have in mind?
>
> I'd also expect some treatement of what happens with the standard SPI
> API while something is enabled.
I suppose it makes sense to return -EBUSY from
spi_sync()/spi_async()/spi_bus_lock() when a hardware trigger is
enabled.
On Mon, Mar 04, 2024 at 05:21:21PM -0600, David Lechner wrote:
> On Wed, Jan 10, 2024 at 3:36 PM Mark Brown <[email protected]> wrote:
> > The bit where messages are initiated by hardware is a step beyond that,
> > I think we need a bit more API for connecting up the triggers and we
> > also need to have something handling what happens with normal operation
> > of the device while these triggers are enabled. I think it could be
> > useful to split this bit out since there's a lot more to work out there
> > in terms of interfaces.
> > > +/**
> > > + * SPI_OFFLOAD_RX - placeholder for indicating read transfers for offloads
> > > + *
> > > + * Assign xfer->rx_buf to this value for any read transfer passed to
> > > + * spi_offload_prepare(). This will act as a flag to indicate to the offload
> > > + * that it should do something with the data read during this transfer. What
> > > + * that something can be is determined by the specific hardware, e.g. it could
> > > + * be piped to DMA or a DSP, etc.
> > > + */
> > > +#define SPI_OFFLOAD_RX_SENTINEL ((void *)1)
> > This feels like something where there are likely to be multiple options
> > and we need configurability. I'd also expect to see a similar transmit
> > option.
> Having something similar for TX makes sense. What other sorts of
> options are you envisioning here?
You list two options for something that could be done with the data
above - sending it to DMA or a DSP. My concern here is that a given
piece of hardware might support more than one option and need to choose
between them.
> > > +int spi_offload_prepare(struct spi_offload *offload, struct spi_device *spi,
> > > + struct spi_transfer *xfers, unsigned int num_xfers)
> > I would expect us to just generically prepare a message, then pass a
> > prepared message into the API that enables a trigger. We would need
> > something that handles the difference between potentially offloading for
> > better performance and having a hardware trigger, I think that might be
> > a case of just not exposing the engine's prepare to client drivers and
> > then having the core track if it needs to do that when enabling a
> > hardware trigger.
> Not exposing the offload prepare to client drivers sounds reasonable.
> I'm not sure I understand the potential need for an offload without a
> hardware trigger though.
Something like pre-cooking the commands to read the interrupt status
registers from a device, send a network transfer, or to download
firmware and settings if you power the device off frequently. Basically
anything with more than one operation that you might want to run
repeatedly and care about the performance of.
> > I'm not seeing anything in this API that provides a mechanism for
> > configuring what triggers things to start, even in the case where things
> > are triggered by hardware rather than initiated by software I'd expect
> > to see hardware with runtime configurability. The binding is a bit
> > unclear but it seems to be expecting this to be statically configured in
> > hardware and that there will be a 1:1 mapping between triggers and
> > scripts that can be configured, if nothing else I would expect that
> > there will be hardware with more possible triggers than scripts.
> For the use case of ADCs/DACs we would want a periodic trigger where
> the period of the trigger is runtime configurable (via sysfs). Is this
> the sort of thing you had in mind here? What other sorts of triggers
> do you have in mind?
Well, it could be pretty much any signal - I'd imagine there will be
things that can trigger off GPIOs for example. Some sort of timer like
you mention does sound plausible too. I think the API needs to be
general enough to just cope with a very broad range of things in a
possibly system/device specified manner and not have a short,
prescriptive list.
> > I'd also expect some treatement of what happens with the standard SPI
> > API while something is enabled.
> I suppose it makes sense to return -EBUSY from
> spi_sync()/spi_async()/spi_bus_lock() when a hardware trigger is
> enabled.
That sounds reasonable. If something is software triggered then I'd
expect to integrate with the current queuing mechanism.
On Mon, 4 Mar 2024 17:21:21 -0600
David Lechner <[email protected]> wrote:
> On Wed, Jan 10, 2024 at 3:36 PM Mark Brown <[email protected]> wrote:
> >
> > On Wed, Jan 10, 2024 at 01:49:42PM -0600, David Lechner wrote:
> > > This adds a feature for specialized SPI controllers that can record
> > > a series of SPI transfers, including tx data, cs assertions, delays,
> > > etc. and then play them back using a hardware trigger without CPU
> > > intervention.
> >
> > > The intended use case for this is with the AXI SPI Engine to capture
> > > data from ADCs at high rates (MSPS) with a stable sample period.
> >
> > > Most of the implementation is controller-specific and will be handled by
> > > drivers that implement the offload_ops callbacks. The API follows a
> > > prepare/enable pattern that should be familiar to users of the clk
> > > subsystem.
> >
> > This is a lot to do in one go, and I think it's a bit too off on the
> > side and unintegrated with the core. There's two very high level bits
> > here, there's the pre-cooking a message for offloading to be executed by
> > a hardware engine and there's the bit where that's triggered by some
> > hardwar event rather than by software.
> >
>
> ...
>
> >
> > The bit where messages are initiated by hardware is a step beyond that,
> > I think we need a bit more API for connecting up the triggers and we
> > also need to have something handling what happens with normal operation
> > of the device while these triggers are enabled. I think it could be
> > useful to split this bit out since there's a lot more to work out there
> > in terms of interfaces.
>
> Now that we have addressed the pre-cooking messages bit [1] I'm coming
> back to the hardware trigger bit. Since the hardware trigger part
> hasn't been discussed in the past, it's not so clear to me what is
> being requested here (also see specific questions below).
>
> [1]: https://lore.kernel.org/linux-spi/20240219-mainline-spi-precook-message-v2-0-4a762c6701b9@baylibre.com/T/#t
Mark took the spi patches so we don't need to do anything complex next
cycle. Just need the IIO driver with these additions as by the time we hit
rc1 all the dependencies will be available.
Rest were questions for Mark I think.