diff mbox series

[RFC,v3,5/9] spi: dt-bindings: axi-spi-engine: document spi-offloads

Message ID 20240722-dlech-mainline-spi-engine-offload-2-v3-5-7420e45df69b@baylibre.com
State New
Headers show
Series spi: axi-spi-engine: add offload support | expand

Commit Message

David Lechner July 22, 2024, 9:57 p.m. UTC
The AXI SPI Engine has support for hardware offloading capabilities.
There can be up to 32 offload instances per SPI controller, so the
bindings limit the value accordingly.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---

RFC: I have a few questions about this one...

1.  The trigger-source properties are borrowed from the leds bindings.
    Do we want to promote this to a generic binding that can be used by
    any type of device?

2.  Some folks are working on adding DMA to TX stream support to the
    AXI SPI Engine hardware. I assume that the `dmas` property is like
    others where the order/index in the phandle array matters. So this
    would mean that for device that only uses 1 out of the 32 offloads
    and only uses 1 TX DMA channel, we would have to have 32 <0>s for
    each of the possible RX dmas in the array. Any way to do some kind
    of mapping to avoid this?

3.  In v2, we discussed about having some sort of data processing unit
    between the AXI SPI Engine RX stream interface and the DMA channel
    interface on the DMA controller. I haven't included this in the
    bindings yet because we don't have a user yet. But it was suggested
    that we could use the graph bindings for this. So here is what that
    might look like:

    Additional property for the AXI SPI Engine controller bindings:

        out-ports:
            $ref: /schemas/graph.yaml#/properties/ports
            unevaluatedProperties: false
            patternProperties:
            "^port@1?[0-9a-f]$":
                $ref: /schemas/graph.yaml#/properties/port
                unevaluatedProperties: false

    And this would be connected to a device node similar to this:

        ip-block@3000 {
            // Something similar to, but not exactly like
            // http://analogdevicesinc.github.io/hdl/library/util_extract/index.html
            compatible = "adi,crc-check";
            // clock that runs this IP block
            clocks = <&sysclk 15>;
            // interrupt raised on bad CRC
            interrupts = <&intc 99>;
            interrupt-names = "crc";
            // output stream with CRC byte removed piped to DMA
            dmas = <&adc_dma 0>;
            dma-names = "rx";

            port {
                adc_crc_check: endpoint {
                    remote-endpoint: <&offload0_rx>;
                };
            };
        };

    Does this sound reasonable?

v3 changes:
* Added #spi-offload-cells property.
* Added properties for triggers and RX data stream connected to DMA.

v2 changes:

This is basically a new patch. It partially replaces "dt-bindings: iio:
offload: add binding for PWM/DMA triggered buffer".

The controller no longer has an offloads object node and the
spi-offloads property is now a standard SPI peripheral property.
---
 .../bindings/spi/adi,axi-spi-engine.yaml           | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)

Comments

Conor Dooley Aug. 14, 2024, 3:58 p.m. UTC | #1
On Fri, Jul 26, 2024 at 02:17:00PM -0500, David Lechner wrote:
> On 7/26/24 7:38 AM, Rob Herring wrote:
> > On Mon, Jul 22, 2024 at 04:57:12PM -0500, David Lechner wrote:
> >> The AXI SPI Engine has support for hardware offloading capabilities.
> >> There can be up to 32 offload instances per SPI controller, so the
> >> bindings limit the value accordingly.
> >>
> >> Signed-off-by: David Lechner <dlechner@baylibre.com>
> >> ---
> >>
> >> RFC: I have a few questions about this one...
> >>
> >> 1.  The trigger-source properties are borrowed from the leds bindings.
> >>     Do we want to promote this to a generic binding that can be used by
> >>     any type of device?
> > 
> > I would make it specific to spi-offload.
> 
> OK
> 
> Meanwhile, we are working on some other ADCs (without SPI offload) and
> finding that they are using basically the same sorts of triggers. And
> on the driver side of things in this series, I'm getting feedback that
> we should have some sort of generic trigger device rather than using,
> e.g. a clk directly. If we need this same sort of trigger abstraction
> for both SPI offloads and IIO device, it does seems like we might want
> to consider something like a new trigger subsystem.

A "device" in the sense that "pwm-clk" is a device I suppose. Are any of
these other things WIP on the lists (that I may have missed while I was
away) or are they still something you're working on internally.
David Lechner Aug. 14, 2024, 5:14 p.m. UTC | #2
On Wed, Aug 14, 2024 at 10:58 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Fri, Jul 26, 2024 at 02:17:00PM -0500, David Lechner wrote:
> > On 7/26/24 7:38 AM, Rob Herring wrote:
> > > On Mon, Jul 22, 2024 at 04:57:12PM -0500, David Lechner wrote:
> > >> The AXI SPI Engine has support for hardware offloading capabilities.
> > >> There can be up to 32 offload instances per SPI controller, so the
> > >> bindings limit the value accordingly.
> > >>
> > >> Signed-off-by: David Lechner <dlechner@baylibre.com>
> > >> ---
> > >>
> > >> RFC: I have a few questions about this one...
> > >>
> > >> 1.  The trigger-source properties are borrowed from the leds bindings.
> > >>     Do we want to promote this to a generic binding that can be used by
> > >>     any type of device?
> > >
> > > I would make it specific to spi-offload.
> >
> > OK
> >
> > Meanwhile, we are working on some other ADCs (without SPI offload) and
> > finding that they are using basically the same sorts of triggers. And
> > on the driver side of things in this series, I'm getting feedback that
> > we should have some sort of generic trigger device rather than using,
> > e.g. a clk directly. If we need this same sort of trigger abstraction
> > for both SPI offloads and IIO device, it does seems like we might want
> > to consider something like a new trigger subsystem.
>
> A "device" in the sense that "pwm-clk" is a device I suppose. >

In simple cases, yes it could be like "pwm-clk" where a PWM/clock/gpio
is used directly as the trigger. We also have a case where there is a
PWM output combined with a clock output using an AND gate, so more of
a "real" device. And finally, there is actual dedicated hardware, like
this [1] time division duplexing (TDD) controller that, in addition to
it's primary purpose for RF applications, can be used as a general
purpose trigger source - mostly useful for creating burst of a finite
number of pulses.

[1]: http://analogdevicesinc.github.io/hdl/library/axi_tdd/index.html

> Are any of
> these other things WIP on the lists (that I may have missed while I was
> away) or are they still something you're working on internally.

My ideas on actual trigger devices and bindings are still mostly on
paper, but we do have a couple of ADCs on the mailing lists right now
where I think it would make more sense to have a flexible "trigger"
but we have been making due with what is currently available.

ad7525

In this case, we need two coordinated triggers for the CNV and CLK
signals, one that generates a single pulse and one that generates a
burst of 16 or 18 pulses, both repeating periodically. Right now, the
proposed DT bindings only allow specifying a PWM to provide the CNV
signal and a second PWM combined with a clock and an AND gate (same
one mentioned above) to provide the CLK signal because that is the
reference hardware design. But technically if one wanted to use, for
example, the aforementioned TDD controller to create these signals for
CNV and CLK instead, it should work just the same.

[ad7525]: https://lore.kernel.org/linux-iio/20240809-ad7625_r1-v2-0-f85e7ac83150@baylibre.com/

ad4030

This also needs a CNV trigger, but it works slightly differently than
ad7525. For now, the proposed DT bindings just have a cnv-gpios to
describe what is connected to the CNV pin. But for certain
applications, a GPIO is not the best choice. For example, to use the
oversampling feature of the chip, we have to provide a burst of some
power of 2 pulses, up to 16k pulses, with specific timing to trigger
2**N conversions before reading one sample. This can be done by
bit-banging the GPIO but could be done much better/faster by something
like the TDD controller that is specifically designed to create a
burst of a finite number of pulses.

[ad4030]: https://lore.kernel.org/linux-iio/20240627-eblanc-ad4630_v1-v1-0-fdc0610c23b0@baylibre.com/

Having a generic DT binding for these ADC input pins that can be
connected to a wide variety of outputs seems more future proof than
having to modify the bindings each time someone wants to support a new
type of output provider.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
index d48faa42d025..ec18eabb993a 100644
--- a/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
+++ b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
@@ -41,6 +41,42 @@  properties:
       - const: s_axi_aclk
       - const: spi_clk
 
+  '#spi-offload-cells':
+    description: The cell value is the offload instance number.
+    const: 1
+
+  trigger-sources:
+    description:
+      An array of trigger source phandles for offload instances. The index in
+      the array corresponds to the offload instance number.
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+
+  dmas:
+    description:
+      DMA channels connected to the output stream interface of an offload instance.
+    minItems: 1
+    maxItems: 32
+
+  dma-names:
+    minItems: 1
+    maxItems: 32
+    items:
+      pattern: "^offload(?:[12]?[0-9]|3[01])-rx$"
+
+patternProperties:
+  "^.*@[0-9a-f]+$":
+    type: object
+    $ref: spi-peripheral-props.yaml
+    additionalProperties: true
+    properties:
+      spi-offloads:
+        description:
+          An array of 1 or more offload instance numbers assigned to this
+          peripheral.
+        items:
+          minimum: 0
+          maximum: 31
+
 required:
   - compatible
   - reg
@@ -59,6 +95,11 @@  examples:
         clocks = <&clkc 15>, <&clkc 15>;
         clock-names = "s_axi_aclk", "spi_clk";
 
+        #spi-offload-cells = <1>;
+        trigger-sources = <&trigger_clock>;
+        dmas = <&dma 0>;
+        dma-names = "offload0-rx";
+
         #address-cells = <1>;
         #size-cells = <0>;