Re: [PATCH RFC v2 1/8] spi: dt-bindings: spi-peripheral-props: add spi-offloads property

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3319 bytes --]

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@baylibre.com>
> ---
> 
> 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?

> 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.

> 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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]