| Message ID | 20240722-dlech-mainline-spi-engine-offload-2-v3-5-7420e45df69b@baylibre.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | spi: axi-spi-engine: add offload support | expand |
On Mon, Jul 22, 2024 at 04:57:12PM -0500, David Lechner wrote: > The AXI SPI Engine has support for hardware offloading capabilities. > There can be up to 32 offload instances per SPI controller, so the > bindings limit the value accordingly. > > Signed-off-by: David Lechner <dlechner@baylibre.com> > --- > > RFC: I have a few questions about this one... > > 1. The trigger-source properties are borrowed from the leds bindings. > Do we want to promote this to a generic binding that can be used by > any type of device? I would make it specific to spi-offload. > > 2. Some folks are working on adding DMA to TX stream support to the > AXI SPI Engine hardware. I assume that the `dmas` property is like > others where the order/index in the phandle array matters. So this > would mean that for device that only uses 1 out of the 32 offloads > and only uses 1 TX DMA channel, we would have to have 32 <0>s for > each of the possible RX dmas in the array. Any way to do some kind > of mapping to avoid this? That's why -names exists. > > 3. In v2, we discussed about having some sort of data processing unit > between the AXI SPI Engine RX stream interface and the DMA channel > interface on the DMA controller. I haven't included this in the > bindings yet because we don't have a user yet. But it was suggested > that we could use the graph bindings for this. So here is what that > might look like: > > Additional property for the AXI SPI Engine controller bindings: > > out-ports: > $ref: /schemas/graph.yaml#/properties/ports > unevaluatedProperties: false > patternProperties: > "^port@1?[0-9a-f]$": > $ref: /schemas/graph.yaml#/properties/port > unevaluatedProperties: false > > And this would be connected to a device node similar to this: > > ip-block@3000 { > // Something similar to, but not exactly like > // http://analogdevicesinc.github.io/hdl/library/util_extract/index.html > compatible = "adi,crc-check"; > // clock that runs this IP block > clocks = <&sysclk 15>; > // interrupt raised on bad CRC > interrupts = <&intc 99>; > interrupt-names = "crc"; > // output stream with CRC byte removed piped to DMA > dmas = <&adc_dma 0>; > dma-names = "rx"; > > port { > adc_crc_check: endpoint { > remote-endpoint: <&offload0_rx>; > }; > }; > }; > > Does this sound reasonable? Shrug. Unlike the offload which is internal to the controller driver?, isn't this specific to the device because it needs to be aware of any processing done or not done. Or maybe it wants to configure the processing. OTOH, maybe this isn't any different than offload? Rob
On 7/26/24 7:38 AM, Rob Herring wrote: > On Mon, Jul 22, 2024 at 04:57:12PM -0500, David Lechner wrote: >> The AXI SPI Engine has support for hardware offloading capabilities. >> There can be up to 32 offload instances per SPI controller, so the >> bindings limit the value accordingly. >> >> Signed-off-by: David Lechner <dlechner@baylibre.com> >> --- >> >> RFC: I have a few questions about this one... >> >> 1. The trigger-source properties are borrowed from the leds bindings. >> Do we want to promote this to a generic binding that can be used by >> any type of device? > > I would make it specific to spi-offload. OK Meanwhile, we are working on some other ADCs (without SPI offload) and finding that they are using basically the same sorts of triggers. And on the driver side of things in this series, I'm getting feedback that we should have some sort of generic trigger device rather than using, e.g. a clk directly. If we need this same sort of trigger abstraction for both SPI offloads and IIO device, it does seems like we might want to consider something like a new trigger subsystem. > >> >> 2. Some folks are working on adding DMA to TX stream support to the >> AXI SPI Engine hardware. I assume that the `dmas` property is like >> others where the order/index in the phandle array matters. So this >> would mean that for device that only uses 1 out of the 32 offloads >> and only uses 1 TX DMA channel, we would have to have 32 <0>s for >> each of the possible RX dmas in the array. Any way to do some kind >> of mapping to avoid this? > > That's why -names exists. OK > >> >> 3. In v2, we discussed about having some sort of data processing unit >> between the AXI SPI Engine RX stream interface and the DMA channel >> interface on the DMA controller. I haven't included this in the >> bindings yet because we don't have a user yet. But it was suggested >> that we could use the graph bindings for this. So here is what that >> might look like: >> >> Additional property for the AXI SPI Engine controller bindings: >> >> out-ports: >> $ref: /schemas/graph.yaml#/properties/ports >> unevaluatedProperties: false >> patternProperties: >> "^port@1?[0-9a-f]$": >> $ref: /schemas/graph.yaml#/properties/port >> unevaluatedProperties: false >> >> And this would be connected to a device node similar to this: >> >> ip-block@3000 { >> // Something similar to, but not exactly like >> // http://analogdevicesinc.github.io/hdl/library/util_extract/index.html >> compatible = "adi,crc-check"; >> // clock that runs this IP block >> clocks = <&sysclk 15>; >> // interrupt raised on bad CRC >> interrupts = <&intc 99>; >> interrupt-names = "crc"; >> // output stream with CRC byte removed piped to DMA >> dmas = <&adc_dma 0>; >> dma-names = "rx"; >> >> port { >> adc_crc_check: endpoint { >> remote-endpoint: <&offload0_rx>; >> }; >> }; >> }; >> >> Does this sound reasonable? > > Shrug. > > Unlike the offload which is internal to the controller driver? Correct. And in the case of the AXI SPI Engine, the offload is part of the controller IP block in hardware as well. > isn't > this specific to the device because it needs to be aware of any > processing done or not done. Or maybe it wants to configure the > processing. Yes, the SPI peripheral driver would be the one needing to know what sort of data processing unit it is connected to so it knows how to configure the chip and how to interpret the received data or other signals from the data processing unit. > > OTOH, maybe this isn't any different than offload? Also true since the SPI peripheral needs to know what kind of capabilities that the offload itself has. > > Rob
On Fri, Jul 26, 2024 at 02:17:00PM -0500, David Lechner wrote: > On 7/26/24 7:38 AM, Rob Herring wrote: > > On Mon, Jul 22, 2024 at 04:57:12PM -0500, David Lechner wrote: > >> The AXI SPI Engine has support for hardware offloading capabilities. > >> There can be up to 32 offload instances per SPI controller, so the > >> bindings limit the value accordingly. > >> > >> Signed-off-by: David Lechner <dlechner@baylibre.com> > >> --- > >> > >> RFC: I have a few questions about this one... > >> > >> 1. The trigger-source properties are borrowed from the leds bindings. > >> Do we want to promote this to a generic binding that can be used by > >> any type of device? > > > > I would make it specific to spi-offload. > > OK > > Meanwhile, we are working on some other ADCs (without SPI offload) and > finding that they are using basically the same sorts of triggers. And > on the driver side of things in this series, I'm getting feedback that > we should have some sort of generic trigger device rather than using, > e.g. a clk directly. If we need this same sort of trigger abstraction > for both SPI offloads and IIO device, it does seems like we might want > to consider something like a new trigger subsystem. A "device" in the sense that "pwm-clk" is a device I suppose. Are any of these other things WIP on the lists (that I may have missed while I was away) or are they still something you're working on internally.
On Wed, Aug 14, 2024 at 10:58 AM Conor Dooley <conor@kernel.org> wrote: > > On Fri, Jul 26, 2024 at 02:17:00PM -0500, David Lechner wrote: > > On 7/26/24 7:38 AM, Rob Herring wrote: > > > On Mon, Jul 22, 2024 at 04:57:12PM -0500, David Lechner wrote: > > >> The AXI SPI Engine has support for hardware offloading capabilities. > > >> There can be up to 32 offload instances per SPI controller, so the > > >> bindings limit the value accordingly. > > >> > > >> Signed-off-by: David Lechner <dlechner@baylibre.com> > > >> --- > > >> > > >> RFC: I have a few questions about this one... > > >> > > >> 1. The trigger-source properties are borrowed from the leds bindings. > > >> Do we want to promote this to a generic binding that can be used by > > >> any type of device? > > > > > > I would make it specific to spi-offload. > > > > OK > > > > Meanwhile, we are working on some other ADCs (without SPI offload) and > > finding that they are using basically the same sorts of triggers. And > > on the driver side of things in this series, I'm getting feedback that > > we should have some sort of generic trigger device rather than using, > > e.g. a clk directly. If we need this same sort of trigger abstraction > > for both SPI offloads and IIO device, it does seems like we might want > > to consider something like a new trigger subsystem. > > A "device" in the sense that "pwm-clk" is a device I suppose. > In simple cases, yes it could be like "pwm-clk" where a PWM/clock/gpio is used directly as the trigger. We also have a case where there is a PWM output combined with a clock output using an AND gate, so more of a "real" device. And finally, there is actual dedicated hardware, like this [1] time division duplexing (TDD) controller that, in addition to it's primary purpose for RF applications, can be used as a general purpose trigger source - mostly useful for creating burst of a finite number of pulses. [1]: http://analogdevicesinc.github.io/hdl/library/axi_tdd/index.html > Are any of > these other things WIP on the lists (that I may have missed while I was > away) or are they still something you're working on internally. My ideas on actual trigger devices and bindings are still mostly on paper, but we do have a couple of ADCs on the mailing lists right now where I think it would make more sense to have a flexible "trigger" but we have been making due with what is currently available. ad7525 In this case, we need two coordinated triggers for the CNV and CLK signals, one that generates a single pulse and one that generates a burst of 16 or 18 pulses, both repeating periodically. Right now, the proposed DT bindings only allow specifying a PWM to provide the CNV signal and a second PWM combined with a clock and an AND gate (same one mentioned above) to provide the CLK signal because that is the reference hardware design. But technically if one wanted to use, for example, the aforementioned TDD controller to create these signals for CNV and CLK instead, it should work just the same. [ad7525]: https://lore.kernel.org/linux-iio/20240809-ad7625_r1-v2-0-f85e7ac83150@baylibre.com/ ad4030 This also needs a CNV trigger, but it works slightly differently than ad7525. For now, the proposed DT bindings just have a cnv-gpios to describe what is connected to the CNV pin. But for certain applications, a GPIO is not the best choice. For example, to use the oversampling feature of the chip, we have to provide a burst of some power of 2 pulses, up to 16k pulses, with specific timing to trigger 2**N conversions before reading one sample. This can be done by bit-banging the GPIO but could be done much better/faster by something like the TDD controller that is specifically designed to create a burst of a finite number of pulses. [ad4030]: https://lore.kernel.org/linux-iio/20240627-eblanc-ad4630_v1-v1-0-fdc0610c23b0@baylibre.com/ Having a generic DT binding for these ADC input pins that can be connected to a wide variety of outputs seems more future proof than having to modify the bindings each time someone wants to support a new type of output provider.
diff --git a/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml index d48faa42d025..ec18eabb993a 100644 --- a/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml +++ b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml @@ -41,6 +41,42 @@ properties: - const: s_axi_aclk - const: spi_clk + '#spi-offload-cells': + description: The cell value is the offload instance number. + const: 1 + + trigger-sources: + description: + An array of trigger source phandles for offload instances. The index in + the array corresponds to the offload instance number. + $ref: /schemas/types.yaml#/definitions/phandle-array + + dmas: + description: + DMA channels connected to the output stream interface of an offload instance. + minItems: 1 + maxItems: 32 + + dma-names: + minItems: 1 + maxItems: 32 + items: + pattern: "^offload(?:[12]?[0-9]|3[01])-rx$" + +patternProperties: + "^.*@[0-9a-f]+$": + type: object + $ref: spi-peripheral-props.yaml + additionalProperties: true + properties: + spi-offloads: + description: + An array of 1 or more offload instance numbers assigned to this + peripheral. + items: + minimum: 0 + maximum: 31 + required: - compatible - reg @@ -59,6 +95,11 @@ examples: clocks = <&clkc 15>, <&clkc 15>; clock-names = "s_axi_aclk", "spi_clk"; + #spi-offload-cells = <1>; + trigger-sources = <&trigger_clock>; + dmas = <&dma 0>; + dma-names = "offload0-rx"; + #address-cells = <1>; #size-cells = <0>;
The AXI SPI Engine has support for hardware offloading capabilities. There can be up to 32 offload instances per SPI controller, so the bindings limit the value accordingly. Signed-off-by: David Lechner <dlechner@baylibre.com> --- RFC: I have a few questions about this one... 1. The trigger-source properties are borrowed from the leds bindings. Do we want to promote this to a generic binding that can be used by any type of device? 2. Some folks are working on adding DMA to TX stream support to the AXI SPI Engine hardware. I assume that the `dmas` property is like others where the order/index in the phandle array matters. So this would mean that for device that only uses 1 out of the 32 offloads and only uses 1 TX DMA channel, we would have to have 32 <0>s for each of the possible RX dmas in the array. Any way to do some kind of mapping to avoid this? 3. In v2, we discussed about having some sort of data processing unit between the AXI SPI Engine RX stream interface and the DMA channel interface on the DMA controller. I haven't included this in the bindings yet because we don't have a user yet. But it was suggested that we could use the graph bindings for this. So here is what that might look like: Additional property for the AXI SPI Engine controller bindings: out-ports: $ref: /schemas/graph.yaml#/properties/ports unevaluatedProperties: false patternProperties: "^port@1?[0-9a-f]$": $ref: /schemas/graph.yaml#/properties/port unevaluatedProperties: false And this would be connected to a device node similar to this: ip-block@3000 { // Something similar to, but not exactly like // http://analogdevicesinc.github.io/hdl/library/util_extract/index.html compatible = "adi,crc-check"; // clock that runs this IP block clocks = <&sysclk 15>; // interrupt raised on bad CRC interrupts = <&intc 99>; interrupt-names = "crc"; // output stream with CRC byte removed piped to DMA dmas = <&adc_dma 0>; dma-names = "rx"; port { adc_crc_check: endpoint { remote-endpoint: <&offload0_rx>; }; }; }; Does this sound reasonable? v3 changes: * Added #spi-offload-cells property. * Added properties for triggers and RX data stream connected to DMA. v2 changes: This is basically a new patch. It partially replaces "dt-bindings: iio: offload: add binding for PWM/DMA triggered buffer". The controller no longer has an offloads object node and the spi-offloads property is now a standard SPI peripheral property. --- .../bindings/spi/adi,axi-spi-engine.yaml | 41 ++++++++++++++++++++++ 1 file changed, 41 insertions(+)