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á
