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: <d65739eededdf2f430f60736518a98aaa3f0b3ca.camel@gmail.com>
Date: Wed, 24 Jul 2024 15:16:42 +0200
From: Nuno Sá <noname.nuno@...il.com>
To: David Lechner <dlechner@...libre.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 Tue, 2024-07-23 at 08:48 -0500, David Lechner wrote:
> 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 get that but the thing is that when the spi device consuming an offload service is
under the same controller providing that service we have guarantees regarding
lifetimes. In case the offload is another controller, those guarantees are not true
anymore and we need to make sure to handle things correctly (like the controller
going away under our feet). That's why a proper provider/consumer interface is needed
and I think that's a significant shift from what we have now?

Out of my head (the minimal interface):

spi_controller_offload_register() - we may need a list of register offloads in the
controller struct.
(devm_)spi_offload_get() - and this one could return a nice struct spi_offload
abstraction to pass to the other APIs
spi_offload_put()

Or for starters we may skip the registering in the core and have it per driver but if
we assume more controlles will have this we might just make it common code already.

Just some ideas...

- Nuno Sá

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ