Re: [PATCH 3/8] dt-bindings: Add bindings for Tegra234 NVDEC

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 06/09/2022 15:28, Mikko Perttunen wrote:
> From: Mikko Perttunen <mperttunen@nvidia.com>
> 
> Update NVDEC bindings for Tegra234. This new engine version only has
> two memory clients, but now requires three clocks, and as a bigger
> change the engine loads firmware from a secure carveout configured by
> the bootloader.
> 
> For the latter, we need to add a phandle to the memory controller
> to query the location of this carveout, and several other properties
> containing offsets into the firmware inside the carveout. These
> properties are intended to be populated through a device tree overlay
> configured at flashing time, so that the values correspond to the
> flashed NVDEC firmware.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>  .../gpu/host1x/nvidia,tegra210-nvdec.yaml     | 118 +++++++++++++++---
>  1 file changed, 98 insertions(+), 20 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
> index 3cf862976448..27128a195b66 100644
> --- a/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
> +++ b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
> @@ -24,17 +24,11 @@ properties:
>        - nvidia,tegra210-nvdec
>        - nvidia,tegra186-nvdec
>        - nvidia,tegra194-nvdec
> +      - nvidia,tegra234-nvdec
>  
>    reg:
>      maxItems: 1
>  
> -  clocks:
> -    maxItems: 1
> -
> -  clock-names:
> -    items:
> -      - const: nvdec

Please leave them here with wide constraints (min/maxItems).

> -
>    resets:
>      maxItems: 1
>  
> @@ -50,18 +44,6 @@ properties:
>  
>    dma-coherent: true
>  
> -  interconnects:
> -    items:
> -      - description: DMA read memory client
> -      - description: DMA read 2 memory client
> -      - description: DMA write memory client
> -
> -  interconnect-names:
> -    items:
> -      - const: dma-mem
> -      - const: read-1
> -      - const: write

Please leave them here with wide constraints (min/maxItems).

> -
>    nvidia,host1x-class:
>      description: |
>        Host1x class of the engine, used to specify the targeted engine
> @@ -79,7 +61,103 @@ required:
>    - reset-names
>    - power-domains
>  
> -additionalProperties: false
> +unevaluatedProperties: false

This looks not needed/related.

> +
> +allOf:

Put allOf before additionalProperties:false.

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - nvidia,tegra234-nvdec
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: NVDEC clock
> +            - description: FUSE clock
> +            - description: TSEC_PKA clock
> +        clock-names:
> +          items:
> +            - const: nvdec
> +            - const: fuse
> +            - const: tsec_pka
> +        interconnects:
> +          items:
> +            - description: DMA read memory client
> +            - description: DMA write memory client
> +        interconnect-names:
> +          items:
> +            - const: dma-mem
> +            - const: write
> +        nvidia,memory-controller:
> +          $ref: /schemas/types.yaml#/definitions/phandle
> +          description:
> +            phandle to the memory controller for determining carveout information.

All fields should be defined in top-level. You can disallow them for
other variants, but if the allOf:if:then gets too big, it's a sign to
split the binding.

> +        nvidia,bl-manifest-offset:
> +          $ref: /schemas/types.yaml#/definitions/uint32
> +          description:
> +            Offset to bootloader manifest from beginning of firmware. Typically set as
> +            part of a device tree overlay corresponding to flashed firmware.
> +        nvidia,bl-code-offset:
> +          $ref: /schemas/types.yaml#/definitions/uint32
> +          description:
> +            Offset to bootloader code section from beginning of firmware. Typically set as
> +            part of a device tree overlay corresponding to flashed firmware.
> +        nvidia,bl-data-offset:
> +          $ref: /schemas/types.yaml#/definitions/uint32
> +          description:
> +            Offset to bootloader data section from beginning of firmware. Typically set as
> +            part of a device tree overlay corresponding to flashed firmware.
> +        nvidia,os-manifest-offset:
> +          $ref: /schemas/types.yaml#/definitions/uint32
> +          description:
> +            Offset to operating system manifest from beginning of firmware. Typically set as
> +            part of a device tree overlay corresponding to flashed firmware.
> +        nvidia,os-code-offset:
> +          $ref: /schemas/types.yaml#/definitions/uint32
> +          description:
> +            Offset to operating system code section from beginning of firmware. Typically set as
> +            part of a device tree overlay corresponding to flashed firmware.
> +        nvidia,os-data-offset:
> +          $ref: /schemas/types.yaml#/definitions/uint32
> +          description:
> +            Offset to operating system data section from beginning of firmware. Typically set as
> +            part of a device tree overlay corresponding to flashed firmware.
> +      required:
> +        - nvidia,memory-controller
> +        - nvidia,bl-manifest-offset
> +        - nvidia,bl-code-offset
> +        - nvidia,bl-data-offset
> +        - nvidia,os-manifest-offset
> +        - nvidia,os-code-offset
> +        - nvidia,os-data-offset

blank line

> +  - if:
> +      properties:
> +        compatible:
> +          contains:

Best regards,
Krzysztof