| Message ID | 20240109-axi-spi-engine-series-3-v1-4-e42c6a986580@baylibre.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | spi: axi-spi-engine: add offload support | expand |
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 <dlechner@baylibre.com> > --- > .../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 <Michael.Hennerich@analog.com> > + - Nuno Sá <nuno.sa@analog.com> > + > +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 <robh@kernel.org> 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 <dlechner@baylibre.com> > > --- > > .../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 <Michael.Hennerich@analog.com> > > + - Nuno Sá <nuno.sa@analog.com> > > + > > +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-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, 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 <robh@kernel.org> 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á >
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 <Michael.Hennerich@analog.com> + - Nuno Sá <nuno.sa@analog.com> + +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#
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 <dlechner@baylibre.com> --- .../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(-)