On Mon, May 13, 2024 at 11:46 AM Conor Dooley <conor@xxxxxxxxxx> wrote: > > On Fri, May 10, 2024 at 07:44:24PM -0500, David Lechner wrote: > > This adds a new property to the spi-peripheral-props binding for use > > with peripherals connected to controllers that support offloading. > > > > Here, offloading means that the controller has the ability to perform > > complex SPI transactions without CPU intervention in some shape or form. > > > > This property will be used to assign controller offload resources to > > each peripheral that needs them. What these resources are will be > > defined by each specific controller binding. > > > > Signed-off-by: David Lechner <dlechner@xxxxxxxxxxxx> > > --- > > > > v2 changes: > > > > In v1, instead of generic SPI bindings, there were only controller- > > specific bindings, so this is a new patch. > > > > In the previous version I also had an offloads object node that described > > what the offload capabilities were but it was suggested that this was > > not necessary/overcomplicated. So I've gone to the other extreme and > > made it perhaps over-simplified now by requiring all information about > > how each offload is used to be encoded in a single u32. > > The property is a u32-array, so I guess, not a single u32? It is an array to handle cases where a peripheral might need more than one offload. But the idea was it put everything about each individual offload in a single u32. e.g. 0x0101 could be offload 1 with hardware trigger 1 and 0x0201 could be offload 1 with hardware trigger 2. Then a peripheral could have spi-offloads = <0x0101>, <0x0201>; if it needed to select between both triggers at runtime. > > > We could of course consider using #spi-offload-cells instead for > > allowing encoding multiple parameters for each offload instance if that > > would be preferable. > > A -cells property was my gut reaction to what you'd written here and > seems especially appropriate if there's any likelihood of some future > device using some external resources for spi-offloading. > However, -cells properties go in providers, not consumers, so it wouldn't > end up in spi-periph-props.yaml, but rather in the controller binding, > and instead there'd be a cell array type property in here. I think you > know that though and I'm interpreting what's been written rather than > what you meant. Indeed you guess correctly. So the next question is if it should be the kind of #-cells that implies a phandle like most providers or without phandles like #address-cells? Asking because I got pushback on v1 for using a phandle with offloads (although in that case, the phandle was for the offload instance itself instead for the SPI controller, so maybe this is different in this case?). > > > I also considered adding spi-offload-names that could be used as sort > > of a compatible string (more of an interface name really) in case some > > peripherals may want to support more than 1 specialized type of offload. > > --- > > .../devicetree/bindings/spi/spi-peripheral-props.yaml | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml > > index 15938f81fdce..32991a2d2264 100644 > > --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml > > +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml > > @@ -113,6 +113,16 @@ properties: > > minItems: 2 > > maxItems: 4 > > > > + spi-offloads: > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > + description: > > + Array of controller offload instances that are reserved for use by the > > + peripheral device. The semantic meaning of the values of the array > > + elements is defined by the controller. For example, it could be a simple > > + 0-based index of the offload instance, or it could be a bitfield where > > + a few bits represent the assigned hardware trigger, a few bits represent > > + the assigned RX stream, etc. > > + > > st,spi-midi-ns: > > description: | > > Only for STM32H7, (Master Inter-Data Idleness) minimum time > > > > -- > > 2.43.2 > >
