On 9/9/24 12:19 PM, David Lechner wrote: > On 9/9/24 9:03 AM, Nuno Sá wrote: >> On Mon, 2024-09-09 at 13:46 +0100, Conor Dooley wrote: >>> On Sun, Sep 08, 2024 at 01:29:25PM +0100, Jonathan Cameron wrote: >>>> On Thu, 05 Sep 2024 17:17:31 +0200 >>>> Angelo Dureghello <adureghello@xxxxxxxxxxxx> wrote: >>>> >>>>> From: Angelo Dureghello <adureghello@xxxxxxxxxxxx> >>>>> >>>>> There is a version AXI DAC IP block (for FPGAs) that provides >>>>> a physical bus for AD3552R and similar chips. This can be used >>>>> instead of a typical SPI controller to be able to use the chip >>>>> in ways that typical SPI controllers are not capable of. >>>>> >>>>> The binding is modified so that either the device is a SPI >>>>> peripheral or it uses an io-backend. >>>>> >>>>> Signed-off-by: Angelo Dureghello <adureghello@xxxxxxxxxxxx> >>>> >>>>> >>>>> required: >>>>> - compatible >>>>> - - reg >>>>> - - spi-max-frequency >>>> Sort of feels like both reg and spi-max-frequency >>>> are valid things to specify. >>>> >>>> Maybe we have an excellent IP and dodgy wiring so want >>>> to clamp the frequency (long term - don't need to support >>>> in the driver today). >>>> >>>> Maybe we have an axi_dac IP that supports multiple >>>> front end devices? So maybe just keep reg? >>> >>> I'd like to be convinced that this incarnation of the AXI DAC IP is not >>> a spi controller and that a ref to spi-controller.yaml is not out of >>> place here. It may not be something that you'd ever use generally, given >>> the "weird" interface to it, but it does seem to be one regardless. >>> >> >> Agreed.. As weird as it get's, it's acting as a spi controller. >> >>> I'd also really like to know how this fits in with spi-offloads. It >>> /feels/, and I'd like to reiterate the word feels, like a rather similar >>> idea just applied to a DAC instead of an ADC. >> >> The offload main principle is to replay a spi transfer periodically given an >> input trigger. I'm not so sure we have that same principle in here. In here I >> guess we stream data over the qspi interface based on SCLK which can look >> similar. The difference is that this IP does not need any trigger for any spi >> transfer replay (I think). >> > > Looking at the AD3552R from a SPI offload perspective of triggered SPI > messages, I think it still works. > > The trigger doesn't have to be a clock/PWM. In this case, the trigger would > be whenever the IIO buffer is full and ready to send a burst of data (not > sure if this would be a hardware or software trigger - but it works either > way). > > Also, the DAC_CUSTOM_CTRL::ADDRESS register field in the AXI DAC IP core > acts as an offload to record and play back a SPI write transfer. > > If we were using the AXI SPI Engine, this would be one SPI message with > two xfers, one for the address write followed by one for the data write. > The size of the data write would be the size of the IIO buffer - or in > the case of a cyclic DMA, the size of the write data would be channel > data size * num channels and the xfer would have a special cyclic offload > flag set. > > So I think we could make a single binding that works for the the AXI DAC > backend/offload and the AXI SPI Engine offload. (I don't think it would > be so easy to integrate the AXI DAC into the SPI framework on the driver > side - and hopefully we won't have to, but the DT still could use the > proposed SPI offload bindings.) > > axi_dac: spi@44a70000 { > compatible = "adi,axi-ad3225r"; > reg = <0x44a70000 0x1000>; > dmas = <&dac_tx_dma_1 0>; > dma-names = "tx"; > clocks = <&ref_clk>; > #spi-offload-cells = <0>; One thing I forgot... The AXI AD3552R IP core can be wired up as a loopback to pipe data directly from some ADC instead of using DMA. In the case of the ADC loopback, we would also have io-channels = <&adc1>, <&adc2>; here in the controller. And we would need #spi-offload-cell = <1>; to have a cell to specify the data source. > > #address-cells = <1>; > #size-cells = <0>; > > dac@0 { > compatible = "adi,ad3552r"; > reg = <0>; > > spi-max-frequency = <30000000>; > spi-3-wire; > spi-tx-bus-width = <4>; > spi-rx-bus-width = <4>; > > reset-gpios = <&gpio0 92 GPIO_ACTIVE_LOW>; > spi-offloads = <&axi_dac>; And then here I guess it would be: spi-offloads = <&axi_dac 0>, <&axi_dac 1>; spi-offload-names = "dma", "adc"; where 0 would select the DMA stream and 1 would select the ADC stream. Or of the ADC part wasn't wired up, just: spi-offloads = <&axi_dac 0>; spi-offload-names = "dma"; > > #address-cells = <1>; > #size-cells = <0>; > > channel@0 { > reg = <0>; > adi,output-range-microvolt = <(-10000000) (10000000)>; > }; > }; > }; > > axi_spi_engine: spi@44a80000 { > compatible = "adi,axi-spi-engine-1.00.a"; > reg = <0x44a80000 0x1000>; > dmas = <&dac_tx_dma_2 0>; > dma-names = "offload0-tx"; > clocks = <&ref_clk>; > #spi-offload-cells = <1>; > > #address-cells = <1>; > #size-cells = <0>; > > dac@0 { > compatible = "adi,ad3552r"; > reg = <0>; > > spi-max-frequency = <30000000>; > spi-3-wire; > spi-tx-bus-width = <4>; > spi-rx-bus-width = <4>; > > reset-gpios = <&gpio0 92 GPIO_ACTIVE_LOW>; > spi-offloads = <&axi_spi_engine 0>; > > #address-cells = <1>; > #size-cells = <0>; > > channel@0 { > reg = <0>; > adi,output-range-microvolt = <(-10000000) (10000000)>; > }; > }; > }; >