Re: [PATCH v2 1/8] dt-bindings: dma: marvell,mmp-dma: Add SpacemiT K1 PDMA support

[Krzysztof Kozlowski]

On 01/07/2025 07:36, Guodong Xu wrote:
> Add "spacemit,k1-pdma" compatible string to support SpacemiT K1 PDMA
> controller. This variant requires:

Why is this marvell? This should be explained here, it's really unexpected.

> - clocks: Clock controller for the DMA
> - resets: Reset controller for the DMA
> 
> Also add explicit #dma-cells property definition with proper constraints:
> - 2 cells for marvell,pdma-1.0 and spacemit,k1-pdma
>     - (request number + unused)
> - 1 cell for other variants
>     - (request number only)
> 
> This fixes "make dtbs_check W=3" warnings about unevaluated properties.

How can I reproduce these warnings? Looks like this is not relevant
here. Each patch is applied one after another and commit msg must be
correct at this point.


> 
> Signed-off-by: Guodong Xu <guodong@xxxxxxxxxxxx>
> ---
> v2:
> - Used more specific compatible string "spacemit,k1-pdma"
> - Enhanced DT bindings with conditional constraints:
>   - clocks/resets properties only required for SpacemiT K1
>   - #dma-cells set to 2 for marvell,pdma-1.0 and spacemit,k1-pdma
>   - #dma-cells set to 1 for other variants, ie.
>       marvell,adma-1.0 and  marvell,pxa910-squ
> ---
>  .../devicetree/bindings/dma/marvell,mmp-dma.yaml   | 49 ++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml b/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml
> index d447d5207be0436bc7fb648dffe31f8b780b491d..7b5f7ccfc9dbb69bfef250146cba5434548f3702 100644
> --- a/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml
> +++ b/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml
> @@ -18,6 +18,7 @@ properties:
>        - marvell,pdma-1.0
>        - marvell,adma-1.0
>        - marvell,pxa910-squ
> +      - spacemit,k1-pdma
>  
>    reg:
>      maxItems: 1
> @@ -32,6 +33,19 @@ properties:
>        A phandle to the SRAM pool
>      $ref: /schemas/types.yaml#/definitions/phandle
>  
> +  clocks:
> +    description: Clock for the controller
> +    maxItems: 1
> +
> +  resets:
> +    description: Reset controller for the DMA controller
> +    maxItems: 1
> +
> +  '#dma-cells':
> +    description:
> +      DMA specifier, consisting of a phandle to DMA controller plus the
> +      following integer cells

Why do you need it here? Isn't this coming from dma common schemas?
> +
>    '#dma-channels':
>      deprecated: true
>  
> @@ -52,12 +66,47 @@ allOf:
>            contains:
>              enum:
>                - marvell,pdma-1.0
> +              - spacemit,k1-pdma
>      then:
>        properties:
>          asram: false
>      else:
>        required:
>          - asram
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: spacemit,k1-pdma
> +    then:
> +      required:
> +        - clocks
> +        - resets
> +    else:
> +      properties:
> +        clocks: false
> +        resets: false
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - marvell,pdma-1.0
> +              - spacemit,k1-pdma
> +    then:
> +      properties:
> +        '#dma-cells':
> +          const: 2
> +          description:
> +            The first cell contains the DMA request number for the peripheral
> +            device. The second cell is currently unused but must be present for
> +            backward compatibility.
> +    else:
> +      properties:
> +        '#dma-cells':
> +          const: 1
> +          description:
> +            The cell contains the DMA request number for the peripheral device.


It's getting complicated. I suggest to make your own schema. Then you
would also switch to preferred 'sram' property instead of that legacy
'asram'.

Really, ancient schemas should not be grown for new, completely
different platforms.


Best regards,
Krzysztof