Re: [PATCH 05/54] dt-bindings: Convert Reserved Memory binding to a schema

[Rob Herring <robh+dt@kernel.org>]

On Wed, Jul 21, 2021 at 8:04 AM Maxime Ripard <maxime@cerno.tech> wrote:
>
> The Reserved Memory mechanism is supported by Linux thanks to its device
> tree binding.
>
> Now that we have the DT validation in place, let's convert the device
> tree bindings for that driver over to a YAML schema.

Thanks for this!

>
> Cc: Mailing List <devicetree-spec@vger.kernel.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  .../reserved-memory/reserved-memory.txt       | 141 ---------------
>  .../reserved-memory/reserved-memory.yaml      | 167 ++++++++++++++++++
>  2 files changed, 167 insertions(+), 141 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
>  create mode 100644 Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml

> diff --git a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> new file mode 100644
> index 000000000000..b61527f11953
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> @@ -0,0 +1,167 @@
> +# SPDX-License-Identifier: GPL-2.0

I think this is okay to dual license. Grant (Linaro) is the original
author and there's only a few lines from other authors.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reserved-memory/reserved-memory.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: /reserved-memory Node
> +
> +maintainers:
> +  - Devicetree Specification Mailing List <devicetree-spec@vger.kernel.org>
> +
> +description: >
> +  Reserved memory is specified as a node under the /reserved-memory node. The
> +  operating system shall exclude reserved memory from normal usage one can
> +  create child nodes describing particular reserved (excluded from normal use)
> +  memory regions. Such memory regions are usually designed for the special
> +  usage by various device drivers.
> +
> +properties:
> +  $nodename:
> +    const: reserved-memory
> +
> +  "#address-cells": true
> +  "#size-cells": true
> +  ranges: true
> +
> +patternProperties:
> +  "^(?!(ranges))[a-z,-]*(@[0-9]+)?$":

Note that you could drop this and put under 'additionalProperties'.
You would lose some node name checking, but there's really little
standard on these nodes.

> +    type: object
> +
> +    description: >
> +      Each child of the reserved-memory node specifies one or more regions of
> +      reserved memory. Each child node may either use a 'reg' property to
> +      specify a specific range of reserved memory, or a 'size' property with
> +      optional constraints to request a dynamically allocated block of memory.
> +
> +      Following the generic-names recommended practice, node names should
> +      reflect the purpose of the node (ie. "framebuffer" or "dma-pool"). Unit
> +      address (@<address>) should be appended to the name if the node is a
> +      static allocation.
> +
> +    properties:
> +      reg: true
> +
> +      size:
> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        description: >
> +          Length based on parent's \#size-cells. Size in bytes of memory to
> +          reserve.
> +
> +      alignment:
> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        description: >
> +          Length based on parent's \#size-cells. Address boundary for
> +          alignment of allocation.
> +
> +      alloc-ranges:
> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        description: >
> +          Address and Length pairs. Specifies regions of memory that are
> +          acceptable to allocate from.
> +
> +      compatible:
> +        oneOf:
> +          - const: shared-dma-pool
> +            description: >
> +              This indicates a region of memory meant to be used as a shared
> +              pool of DMA buffers for a set of devices. It can be used by an
> +              operating system to instantiate the necessary pool management
> +              subsystem if necessary.
> +
> +          # Vendor-specific compatibles in the form <vendor>,[<device>-]<usage>
> +          - const: mediatek,trustzone-bootinfo

I think these should be separate schema files. At least, we're going
to need to support separate files because I don't think we want ones
adding custom properties here. This would fail unless we add every
compatible here. We could also be a bit more exact as to which
properties below apply (e.g. linux,.*-default is only valid for
shared-dma-pool).

> +
> +      no-map:
> +        type: boolean
> +        description: >
> +          Indicates the operating system must not create a virtual mapping of
> +          the region as part of its standard mapping of system memory, nor
> +          permit speculative access to it under any circumstances other than
> +          under the control of the device driver using the region.
> +
> +      reusable:
> +        type: boolean
> +        description: >
> +          The operating system can use the memory in this region with the
> +          limitation that the device driver(s) owning the region need to be
> +          able to reclaim it back. Typically that means that the operating
> +          system can use that region to store volatile or cached data that
> +          can be otherwise regenerated or migrated elsewhere.
> +
> +      linux,cma-default:
> +        type: boolean
> +        description: >
> +          If this property is present, then Linux will use the region for the
> +          default pool of the contiguous memory allocator.
> +
> +      linux,dma-default:
> +        type: boolean
> +        description: >
> +          If this property is present, then Linux will use the region for the
> +          default pool of the consistent DMA allocator.
> +
> +    allOf:
> +      - if:
> +          required:
> +            - no-map
> +
> +        then:
> +          not:
> +            required:
> +              - reusable
> +
> +      - if:
> +          required:
> +            - reusable
> +
> +        then:
> +          not:
> +            required:
> +              - no-map
> +
> +    oneOf:
> +      - required:
> +          - reg
> +
> +      - required:
> +          - size
> +
> +    additionalProperties: true
> +
> +additionalProperties: true

This should be false, right?

> +
> +examples:
> +  - |
> +      / {
> +          #address-cells = <1>;
> +          #size-cells = <1>;
> +          model = "MediaTek MT2701 evaluation board";
> +          compatible = "mediatek,mt2701-evb", "mediatek,mt2701";
> +
> +          reserved-memory {
> +              #address-cells = <1>;
> +              #size-cells = <1>;
> +              ranges;
> +
> +              /* global autoconfigured region for contiguous allocations */
> +              linux,cma {
> +                  compatible = "shared-dma-pool";
> +                  reusable;
> +                  size = <0x4000000>;
> +                  alignment = <0x2000>;
> +                  linux,cma-default;
> +              };
> +
> +              display_reserved: framebuffer@78000000 {
> +                  reg = <0x78000000 0x800000>;
> +              };
> +
> +              trustzone-bootinfo@80002000 {
> +                  compatible = "mediatek,trustzone-bootinfo";
> +                  reg = <0x80002000 0x1000>;
> +              };
> +          };
> +      };
> +
> +...
> --
> 2.31.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel