On 5/23/24 7:15 AM, Nuno Sá wrote: > On Wed, 2024-05-22 at 19:24 +0100, Conor Dooley wrote: >> On Tue, May 21, 2024 at 09:54:39AM -0500, David Lechner wrote: >>> On Sun, May 19, 2024 at 7:53 AM Conor Dooley <conor@xxxxxxxxxx> wrote: >>>> >>>> On Fri, May 17, 2024 at 11:51:58AM -0500, David Lechner wrote: >>>>> On Thu, May 16, 2024 at 4:32 PM Conor Dooley <conor@xxxxxxxxxx> wrote: >>>>>> On Tue, May 14, 2024 at 05:56:47PM -0500, David Lechner wrote: >>>> ... >> To remind myself, "Application 2" featured an offload engine designed >> specifically to work with a particular data format that would strip a >> CRC byte and check the validity of the data stream. >> > > I think the data manipulation is not really a property of the engine. Typically data > going out of the offload engine goes into another "data reorder" block that is pure > HW. > >> I think you're right something like that is a stretch to say that that >> is a feature of the SPI controller - but I still don't believe that >> modelling it as part of the ADC is correct. I don't fully understand the >> io-backends and how they work yet, but the features you describe there >> seem like something that should/could be modelled as one, with its own >> node and compatible etc. Describing custom RTL stuff ain't always >> strightforward, but the stuff from Analog is versioned and documented >> etc so it shouldn't be quite that hard. >> > > Putting this in io-backends is likely a stretch but one thing to add is that the > peripheral is always (I think) kind of the consumer of the resources. Taking the > trigger (PWM) as an example and even when it is directly connected with the offload > block, the peripheral still needs to know about it. Think of sampling frequency... > The period of the trigger signal is strictly connected with the sampling frequency of > the peripheral for example. So I see 2 things: > > 1) Enabling/Disabling the trigger could be easily done from the peripheral even with > the resource in the spi engine. I think David already has some code in the series > that would make this trivial and so having the property in the spi controller brings > no added complexity. > > 2) Controlling things like the trigger period/sample_rate. This could be harder to do > over SPI (or making it generic enough) so we would still need to have the same > property on the peripheral (even if not directly connected to it). I kind of agree > with David that having the property both in the peripheral and controller is a bit > weird. > > And the DMA block is a complete different story. Sharing that data back with the > peripheral driver (in this case, the IIO subsystem) would be very interesting at the > very least. Note that the DMA block is not really something that is part of the > controller nor the offload block. It is an external block that gets the data coming > out of the offload engine (or the data reorder block). In IIO, we already have a DMA > buffer interface so users of the peripheral can get the data without any intervention > of the driver (on the data). We "just" enable buffering and then everything happens > on HW and userspace can start requesting data. If we are going to attach the DMA in > the controller, I have no idea how we can handle it. Moreover, the offload it's > really just a way of replaying the same spi transfer over and over and that happens > in HW so I'm not sure how we could "integrate" that with dmaengine. > > But maybe I'm overlooking things... And thinking more in how this can be done in SW > rather than what makes sense from an HW perspective. > > >> continuation: >> If offload engines have their own register region in the memory map, > > > Don't think it has it's own register region... David? I think the question here was if the CRC checker IP block (or descrambler shown in the link below, or whatever) had registers in the offload/SPI controller to control that extra part or if they had their own dedicated registers. So far, these have been fixed-purpose, so have no registers at all. But I could see needing a register, e.g. for turning it on or off. In this case, I think it does become something like an io-backend. Or would we add this on/off switch to the AXI SPI Engine registers? Also, as shown in the link below, the extra bits share a clock domain with the AXI SPI Engine. So, yes, technically I suppose they could/should? be independent consumers of the same clock like Conor suggests below. It does seems kind of goofy if we have to write a driver just to turn on a clock that is already guaranteed to be on though. > >> their own resources (the RTL is gonna need at the very least a clock) >> and potentially also provide other services (like acting as an >> io-backend type device that pre-processes data) it doesn't sound like >> either the controller or peripheral nodes are the right place for these >> properties. And uh, spi-offloads gets a phandle once more... >> > > But then it would be valid for both peripheral and controller to reference that > phandle right (and get the resources of interest)? > > FWIW, maybe look at the below usecase. It has some block diagrams that may be helpful > to you: > > https://wiki.analog.com/resources/eval/user-guides/ad463x/hdl > > - Nuno Sá > >
