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

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.

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

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
>

On Wed, 21 Jul 2021 16:03:35 +0200, Maxime Ripard 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.
> 
> 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
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/nvmem/rmem.example.dt.yaml: reserved-memory: nvram@10000000:compatible: 'oneOf' conditional failed, one must be fixed:
	['raspberrypi,bootloader-config', 'nvmem-rmem'] is too long
	Additional items are not allowed ('nvmem-rmem' was unexpected)
	'shared-dma-pool' was expected
	'mediatek,trustzone-bootinfo' was expected
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.example.dt.yaml: reserved-memory: emc-table@83400000:compatible: 'oneOf' conditional failed, one must be fixed:
	'shared-dma-pool' was expected
	'mediatek,trustzone-bootinfo' was expected
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
\ndoc reference errors (make refcheckdocs):
Documentation/devicetree/bindings/display/arm,hdlcd.txt: Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
Documentation/devicetree/bindings/display/arm,komeda.txt: Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
Documentation/devicetree/bindings/display/arm,malidp.txt: Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
Documentation/devicetree/bindings/display/arm,pl11x.txt: Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
Documentation/devicetree/bindings/firmware/intel,stratix10-svc.txt: Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
Documentation/devicetree/bindings/gpu/aspeed-gfx.txt: Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
Documentation/devicetree/bindings/media/aspeed-video.txt: Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
Documentation/devicetree/bindings/media/mediatek-vpu.txt: Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
Documentation/devicetree/bindings/remoteproc/ti,davinci-rproc.txt: Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml: Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml: Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt: Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
Documentation/devicetree/bindings/remoteproc/ti,omap-remoteproc.yaml: Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
arch/arm/mm/dma-mapping-nommu.c: Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

See https://patchwork.ozlabs.org/patch/1508249

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Hi Rob,

On Wed, Jul 21, 2021 at 08:30:43AM -0600, Rob Herring wrote:
> > +%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.

I didn't realise it could be used that way too, I'll change it.

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

I'm not entirely sure how we can just ignore the vendor compatibles
without raising a warning. Do you have any suggestion?

Thanks!
Maxime

On Wed, Aug 18, 2021 at 5:00 AM Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi Rob,
>
> On Wed, Jul 21, 2021 at 08:30:43AM -0600, Rob Herring wrote:
> > > +%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.
>
> I didn't realise it could be used that way too, I'll change it.
>
> > > +    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).
>
> I'm not entirely sure how we can just ignore the vendor compatibles
> without raising a warning. Do you have any suggestion?

You need 1 schema file with all the common (child) properties and then
1 schema file for each compatible (maybe some can be grouped) that
references the common schema.

You'd lose checking that the child nodes are actually children of
/reserved-memory, but I'm not too worried about that.

Rob