Re: [PATCH RFC v3 0/9] spi: axi-spi-engine: add offload support

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

Hi David,

On Mon, 2024-07-22 at 16:57 -0500, David Lechner wrote:
> There is a recap at the end of this cover letter for those not familiar
> with the previous discussions. For those that are, we'll get right to
> the changes since the last version.
> 
> In RFC v2, most of the discussion was around the DT bindings, so that
> is what has mostly changed since then. I think we mostly settled on
> what properties are needed and where they should go. There are probably
> still some details to work out (see PATCH 5/9 for more discussion) but
> I think we have the big-picture stuff figured out.
> 
> Here is the actual devicetree used for testing to show how it all
> comes together:
> 
> 	trigger_clk: adc-trigger-clock {
> 		compatible = "pwm-clock";
> 		#clock-cells = <0>;
> 		#trigger-source-cells = <0>;
> 		pwms = <&adc_trigger 0 10000>;
> 	};
> 
> 	...
> 
> 	axi_spi_engine_0: spi@44a00000 {
> 		compatible = "adi,axi-spi-engine-1.00.a";
> 		reg = <0x44a00000 0x1000>;
> 		interrupt-parent = <&intc>;
> 		interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
> 		clocks = <&clkc 15>, <&spi_clk>;
> 		clock-names = "s_axi_aclk", "spi_clk";
> 
> 		/* offload-specific properties */
> 		#spi-offload-cells = <1>;
> 		dmas = <&rx_dma 0>;
> 		dma-names = "offload0-rx";
> 		trigger-sources = <&trigger_clk>;
> 
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		ad7986: adc@0 {
> 			compatible = "adi,ad7986";
> 			reg = <0>;
> 			spi-max-frequency = <111111111>; /* 9 ns period */
> 			adi,spi-mode = "single";
> 			avdd-supply = <&eval_u12>;
> 			dvdd-supply = <&eval_u12>;
> 			vio-supply = <&eval_u3>;
> 			bvdd-supply = <&eval_u10>;
> 			ref-supply = <&eval_u5>;
> 			turbo-gpios = <&gpio0 87 GPIO_ACTIVE_HIGH>;
> 
> 			spi-offloads = <&axi_spi_engine_0 0>;
> 		};
> 	};
> 
> A working branch complete with extra hacks can be found at [1].
> 
> Also, I took a detour looking into what it would take to get Martin
> Sperl's Raspberry Pi DMA offload proof-of-concept [2] updated to work
> with this. This way we could have a second user to help guide the
> design process. Given all of the SPI hardware quirks on that platform
> and the unsolved technical issues, like how to get accurate time delays
> and how to work around the 32-bit DMA word limitation, it would be more
> work than I have time for (at least without someone sponsoring the work).
> 
> [1]: https://github.com/dlech/linux/tree/axi-spi-engine-offload-v3
> [2]:
> https://github.com/msperl/spi-bcm2835/blob/refactor_dmachain_for_prepared_messages/spi-bcm2835dma.c
> 
> ---
> Changes in v3:
> - See individual patches for more detailed changes.
> - Reworked DT bindings to have things physically connected to the SPI
>   controller be properties of the SPI controller and use more
>   conventional provider/consumer properties.
> - Added more SPI APIs for peripheral drivers to use to get auxillary
>   offload resources, like triggers.
> - Link to v2:
> https://lore.kernel.org/r/20240510-dlech-mainline-spi-engine-offload-2-v2-0-8707a870c435@xxxxxxxxxxxx
> 
> ---
> 
> As a recap, here is the background and end goal of this series:
> 
> The AXI SPI Engine is a SPI controller that has the ability to record a
> series of SPI transactions and then play them back using a hardware
> trigger. This allows operations to be performed, repeating many times,
> without any CPU intervention. This is needed for achieving high data
> rates (millions of samples per second) from ADCs and DACs that are
> connected via a SPI bus.
> 
> 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 memories in the controller (RX buffer is not used since RX data gets
> streamed to an external sink). 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 (AD7944 ADC) that use
> them. This enables capturing analog data at 2 million samples per
> second.
> 
> The hardware setup looks like this:
> 
> +-------------------------------+   +------------------+
> >                               |   |                  |
> >  SOC/FPGA                     |   |  AD7944 ADC      |
> >  +---------------------+      |   |                  |
> >  | AXI SPI Engine      |      |   |                  |
> >  |             SPI Bus ============ SPI Bus          |
> >  |                     |      |   |                  |
> >  |  +---------------+  |      |   |                  |
> >  |  | Offload 0     |  |      |   +------------------+
> >  |  |   RX DATA OUT > > > >   |
> >  |  |    TRIGGER IN < < <  v  |
> >  |  +---------------+  | ^ v  |
> >  +---------------------+ ^ v  |
> >  | AXI PWM             | ^ v  |
> >  |                 CH0 > ^ v  |
> >  +---------------------+   v  |
> >  | AXI DMA             |   v  |
> >  |                 CH0 < < <  |
> >  +---------------------+      |
> >                               |
> +-------------------------------+
> 
> To: Mark Brown <broonie@xxxxxxxxxx>
> To: Jonathan Cameron <jic23@xxxxxxxxxx>
> To: Rob Herring <robh@xxxxxxxxxx>
> To: Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>
> To: Conor Dooley <conor+dt@xxxxxxxxxx>
> To: Nuno Sá <nuno.sa@xxxxxxxxxx>
> Cc: Michael Hennerich <Michael.Hennerich@xxxxxxxxxx>
> Cc: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Cc: David Jander <david@xxxxxxxxxxx>
> Cc: Martin Sperl <kernel@xxxxxxxxxxxxxxxx>
> Cc:  <linux-spi@xxxxxxxxxxxxxxx>
> Cc:  <devicetree@xxxxxxxxxxxxxxx>
> Cc:  <linux-kernel@xxxxxxxxxxxxxxx>
> Cc:  <linux-iio@xxxxxxxxxxxxxxx>
> 
> ---
> 

I think there are things that we need to better figure but things are improving
IMO :)

I'm only doing a very superficial review since I need to better look at the
patches...

But one thing that I do want to mention is a scenario (another funny one...)
that I've discussing and that might be a reality. Something like:

+-------------------------------+    +------------------+
|                               |    |                  |
|  SOC/FPGA                     |    |   ADC            |
|                               |    |                  |
|	+---------------+       |    |                  |
|       |  SPI PS Zynq  |============== SPI Bus         |
|	+---------------+	|    |	                |
|                               |    |                  |
|  +---------------------+      |    |                  |
|  | AXI SPI Engine      |      |    |                  |
|  |                 v================ DATA Bus         |
|  |                 v   |      |    |                  |
|  |   +---------------+ |      |    |                  |
|  |  | Offload 0     |  |      |    +------------------+
|  |  |   RX DATA OUT |  |      |
|  |  |    TRIGGER IN |  |      |
|  |  +---------------+  |      |
|                               |
+-------------------------------+