[04/13] spi: dt-bindings: adi,axi-spi-engine: add offload bindings

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(-)

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á
>