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

[Nuno Sá <noname.nuno@xxxxxxxxx>]

On Wed, 2024-01-10 at 18:06 -0600, David Lechner wrote:
> On Wed, Jan 10, 2024 at 5:15 PM Rob Herring <robh@xxxxxxxxxx> 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á
>