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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9fd9ed71-4b2d-49a7-9432-1747ae2e9aef@baylibre.com>
Date: Tue, 23 Jul 2024 08:48:02 -0500
From: David Lechner <dlechner@...libre.com>
To: Nuno Sá <noname.nuno@...il.com>,
 Mark Brown <broonie@...nel.org>, Jonathan Cameron <jic23@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Nuno Sá
 <nuno.sa@...log.com>
Cc: Michael Hennerich <Michael.Hennerich@...log.com>,
 Lars-Peter Clausen <lars@...afoo.de>, David Jander <david@...tonic.nl>,
 Martin Sperl <kernel@...tin.sperl.org>, linux-spi@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-iio@...r.kernel.org
Subject: Re: [PATCH RFC v3 0/9] spi: axi-spi-engine: add offload support

On 7/23/24 2:35 AM, Nuno Sá wrote:
> Hi David,
> 
> 
> 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 |  |      |
> |  |  +---------------+  |      |
> |                               |
> +-------------------------------+
> 
> From the above, the spi controller for typical register access/configuration is
> not the spi_enigine and the offload core is pretty much only used for streaming
> data. So I think your current approach would not work with this usecase. In your
> first RFC you had something overly complicated (IMHO) but you already had a
> concept that maybe it's worth looking at again. I mean having a spi_offload
> object that could describe it and more importantly have a provider/consumer
> logic where a spi consumer (or maybe even something else?) can get()/put() an
> offload object to stream data.

Although it isn't currently implemented this way in the core SPI code, I think
the DT bindings proposed in this version of the series would allow for this.
The offload provider is just the one with the #spi-offload-cells and doesn't
necessarily have to be the parent of the SPI peripheral.

> 
> I know, I did said that I did not liked for spi consumers to have to explicitly
> call something like spi_offload_get() but I guess I have been proved wrong :).
> We can also try to be smart about it as an explicit get is only needed (likely)
> in the above scenario (or maybe we can even do it directly in the spi core
> during spi_probe()). Or maybe it's not worth it to play smart and just let
> consumers do it (that's the typical pattern anyways).
> 
> Having said the above, I still think your current proposal for triggers and
> getting DMA streams is valid for the above usecase.
> 
> FWIW, I'm also trying to understand with the HW guys why the hell can't we just
> use the spi_engine controller for everything. But the whole discussion is
> already showing us that we may need more flexibility.
> 
> Thanks!
> - Nuno Sá