Re: [PATCH] dt-bindings: mtd: convert "fixed-partitions" to the json-schema

[Rob Herring <robh@kernel.org>]

On Wed, Dec 09, 2020 at 02:02:35PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> This standardizes its documentation, allows validating with Makefile
> checks and helps writing DTS files.
> 
> Noticeable changes:
> 1. Dropped "Partitions can be represented by sub-nodes of a flash
>    device." as we also support subpartitions (don't have to be part of
>    flash device node)
> 2. Dropped "to Linux" as bindings are meant to be os agnostic.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  .../devicetree/bindings/mtd/partition.txt     | 131 +---------------
>  .../mtd/partitions/fixed-partitions.yaml      | 146 ++++++++++++++++++
>  2 files changed, 148 insertions(+), 129 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
> index 4a39698221a2..ead90e8274d6 100644
> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> @@ -24,137 +24,10 @@ another partitioning method.
>  Available bindings are listed in the "partitions" subdirectory.
>  
>  
> -Fixed Partitions
> -================
> -
> -Partitions can be represented by sub-nodes of a flash device. This can be used
> -on platforms which have strong conventions about which portions of a flash are
> -used for what purposes, but which don't use an on-flash partition table such
> -as RedBoot.
> -
> -The partition table should be a subnode of the flash node and should be named
> -'partitions'. This node should have the following property:
> -- compatible : (required) must be "fixed-partitions"
> -Partitions are then defined in subnodes of the partitions node.
> +Deprecated: partitions defined in flash node
> +============================================
>  
>  For backwards compatibility partitions as direct subnodes of the flash device are
>  supported. This use is discouraged.
>  NOTE: also for backwards compatibility, direct subnodes that have a compatible
>  string are not considered partitions, as they may be used for other bindings.
> -
> -#address-cells & #size-cells must both be present in the partitions subnode of the
> -flash device. There are two valid values for both:
> -<1>: for partitions that require a single 32-bit cell to represent their
> -     size/address (aka the value is below 4 GiB)
> -<2>: for partitions that require two 32-bit cells to represent their
> -     size/address (aka the value is 4 GiB or greater).
> -
> -Required properties:
> -- reg : The partition's offset and size within the flash
> -
> -Optional properties:
> -- label : The label / name for this partition.  If omitted, the label is taken
> -  from the node name (excluding the unit address).
> -- read-only : This parameter, if present, is a hint to Linux that this
> -  partition should only be mounted read-only. This is usually used for flash
> -  partitions containing early-boot firmware images or data which should not be
> -  clobbered.
> -- lock : Do not unlock the partition at initialization time (not supported on
> -  all devices)
> -- slc-mode: This parameter, if present, allows one to emulate SLC mode on a
> -  partition attached to an MLC NAND thus making this partition immune to
> -  paired-pages corruptions
> -
> -Examples:
> -
> -
> -flash@0 {
> -	partitions {
> -		compatible = "fixed-partitions";
> -		#address-cells = <1>;
> -		#size-cells = <1>;
> -
> -		partition@0 {
> -			label = "u-boot";
> -			reg = <0x0000000 0x100000>;
> -			read-only;
> -		};
> -
> -		uimage@100000 {
> -			reg = <0x0100000 0x200000>;
> -		};
> -	};
> -};
> -
> -flash@1 {
> -	partitions {
> -		compatible = "fixed-partitions";
> -		#address-cells = <1>;
> -		#size-cells = <2>;
> -
> -		/* a 4 GiB partition */
> -		partition@0 {
> -			label = "filesystem";
> -			reg = <0x00000000 0x1 0x00000000>;
> -		};
> -	};
> -};
> -
> -flash@2 {
> -	partitions {
> -		compatible = "fixed-partitions";
> -		#address-cells = <2>;
> -		#size-cells = <2>;
> -
> -		/* an 8 GiB partition */
> -		partition@0 {
> -			label = "filesystem #1";
> -			reg = <0x0 0x00000000 0x2 0x00000000>;
> -		};
> -
> -		/* a 4 GiB partition */
> -		partition@200000000 {
> -			label = "filesystem #2";
> -			reg = <0x2 0x00000000 0x1 0x00000000>;
> -		};
> -	};
> -};
> -
> -flash@3 {
> -	partitions {
> -		compatible = "fixed-partitions";
> -		#address-cells = <1>;
> -		#size-cells = <1>;
> -
> -		partition@0 {
> -			label = "bootloader";
> -			reg = <0x000000 0x100000>;
> -			read-only;
> -		};
> -
> -		firmware@100000 {
> -			label = "firmware";
> -			reg = <0x100000 0xe00000>;
> -			compatible = "brcm,trx";
> -		};
> -
> -		calibration@f00000 {
> -			label = "calibration";
> -			reg = <0xf00000 0x100000>;
> -			compatible = "fixed-partitions";
> -			ranges = <0 0xf00000 0x100000>;
> -			#address-cells = <1>;
> -			#size-cells = <1>;
> -
> -			partition@0 {
> -				label = "wifi0";
> -				reg = <0x000000 0x080000>;
> -			};
> -
> -			partition@80000 {
> -				label = "wifi1";
> -				reg = <0x080000 0x080000>;
> -			};
> -		};
> -	};
> -};
> diff --git a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> new file mode 100644
> index 000000000000..c5e509e08f31
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> @@ -0,0 +1,146 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/partitions/fixed-partitions.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Fixed partitions
> +
> +description: |
> +  This binding can be used on platforms which have strong conventions about
> +  which portions of a flash are used for what purposes, but which don't use an
> +  on-flash partition table such as RedBoot.
> +
> +  The partition table should be a node named "partitions". Partitions are then
> +  defined as subnodes.
> +
> +maintainers:
> +  - Rafał Miłecki <rafal@milecki.pl>
> +
> +properties:
> +  compatible:
> +    const: fixed-partitions
> +
> +patternProperties:
> +  "^.*@[0-9a-f]+$":

You can drop '^.*'.

This needs to recurse to nested nodes. 

I think here you can do just:
  
$ref: #/

And drop 'compatible' as required. It's redundant anyways because the 
schema will only be applied if compatible matches.

This creates a circular schema which I'm not sure what will happen. If 
that doesn't work, then we'll have to define how many levels we support 
and then do something like this:

$defs:
  partition:
    # everything you have here...

And then here, just:

       $ref: #/$defs/partition
       unevaluatedProperties: false
       patternProperties:
         "@[0-9a-f]+$":
           $ref: #/$defs/partition

And repeat for however many levels we want to nest.

Also, I just added '$def' support to dt-schema last week.

> +    description: node describing a single flash partition
> +    type: object
> +
> +    properties:
> +      reg:
> +        description: partition's offset and size within the flash

I don't think we support more than 1 entry here, so add 'maxItems: 1'.

> +
> +      label:
> +        description: The label / name for this partition. If omitted, the label
> +          is taken from the node name (excluding the unit address).
> +
> +      read-only:
> +        description: This parameter, if present, is a hint that this partition
> +          should only be mounted read-only. This is usually used for flash
> +          partitions containing early-boot firmware images or data which should
> +          not be clobbered.
> +        type: boolean
> +
> +      lock:
> +        description: Do not unlock the partition at initialization time (not
> +          supported on all devices)
> +        type: boolean
> +
> +      slc-mode:
> +        description: This parameter, if present, allows one to emulate SLC mode
> +          on a partition attached to an MLC NAND thus making this partition
> +          immune to paired-pages corruptions
> +        type: boolean
> +
> +    required:
> +      - reg
> +
> +required:
> +  - compatible
> +
> +additionalProperties: true

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/