[PATCH v5 1/6] dt/bindings: fwu-mdata-mtd: drop changes outside FWU

[Michal Simek]

On 2/28/23 01:52, jassisinghbrar at gmail.com wrote:
> From: Jassi Brar <jaswinder.singh at linaro.org>
> 
> Any requirement of FWU should not require changes to bindings
> of other subsystems. For example, for mtd-backed storage we
> can do without requiring 'fixed-partitions' children to also
> carry 'uuid', a property which is non-standard and not in the
> bindings.
> 
>   There exists no code yet, so we can change the fwu-mtd bindings
> to contain all properties within the fwu-mdata node.
> 
> Signed-off-by: Jassi Brar <jaswinder.singh at linaro.org>
> ---
>   .../firmware/fwu-mdata-mtd.yaml               | 105 +++++++++++++++---
>   1 file changed, 91 insertions(+), 14 deletions(-)
> 
> diff --git a/doc/device-tree-bindings/firmware/fwu-mdata-mtd.yaml b/doc/device-tree-bindings/firmware/fwu-mdata-mtd.yaml
> index 4f5404f999..4b87fb8624 100644
> --- a/doc/device-tree-bindings/firmware/fwu-mdata-mtd.yaml
> +++ b/doc/device-tree-bindings/firmware/fwu-mdata-mtd.yaml
> @@ -1,13 +1,13 @@
>   # SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>   %YAML 1.2
>   ---
> -$id: http://devicetree.org/schemas/firmware/u-boot,fwu-mdata-sf.yaml#
> -$schema: http://devicetree.org/meta-schemas/core.yaml#
> +$id: http://devicetree.org/schemas/firmware/u-boot,fwu-mdata-mtd.yaml#
> +$schema: http://devicetree.org/meta-schemas/base.yaml#
>   
>   title: FWU metadata on MTD device without GPT
>   
>   maintainers:
> - - Masami Hiramatsu <masami.hiramatsu at linaro.org>
> + - Jassi Brar <jaswinder.singh at linaro.org>
>   
>   properties:
>     compatible:
> @@ -15,24 +15,101 @@ properties:
>         - const: u-boot,fwu-mdata-mtd
>   
>     fwu-mdata-store:
> -    maxItems: 1
> -    description: Phandle of the MTD device which contains the FWU medatata.
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Phandle of the MTD device which contains the FWU MetaData and Banks.
>   
> -  mdata-offsets:
> +  mdata-parts:
> +    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
>       minItems: 2
> -    description: Offsets of the primary and secondary FWU metadata in the NOR flash.
> +    maxItems: 2
> +    description: labels of the primary and secondary FWU metadata partitions in the 'fixed-partitions' subnode of the 'jedec,spi-nor' flash device node.
> +
> +  patternProperties:
> +    "fwu-bank@[0-9]":
> +    type: object
> +    description: List of FWU mtd-backed banks. Typically two banks.
> +
> +    properties:
> +      id:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: Index of the bank.
> +
> +      label:
> +        $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> +        minItems: 1
> +        maxItems: 1
> +        description: label of the partition, in the 'fixed-partitions' subnode of the 'jedec,spi-nor' flash device node, that holds this bank.
> +
> +      patternProperties:
> +        "fwu-image@[0-9]":
> +        type: object
> +        description: List of images in the FWU mtd-backed bank.
> +
> +        properties:
> +          id:
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            description: Index of the bank.
> +
> +          offset:
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            description: Offset, from start of the bank, where the image is located.
> +
> +          size:
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            description: Size reserved for the image.
> +
> +          uuid:
> +            $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> +            minItems: 1
> +            maxItems: 1
> +            description: UUID of the image.
> +
> +        required:
> +          - id
> +          - offset
> +          - size
> +          - uuid
> +        additionalProperties: false
> +
> +    required:
> +      - id
> +      - label
> +      - fwu-images
> +    additionalProperties: false
>   
>   required:
>     - compatible
>     - fwu-mdata-store
> -  - mdata-offsets
> -
> +  - mdata-parts
> +  - fwu-banks
>   additionalProperties: false
>   
>   examples:
>     - |
> -    fwu-mdata {
> -        compatible = "u-boot,fwu-mdata-mtd";
> -        fwu-mdata-store = <&spi-flash>;
> -        mdata-offsets = <0x500000 0x530000>;
> -    };
> +	fwu-mdata {
> +		compatible = "u-boot,fwu-mdata-mtd";
> +		fwu-mdata-store = <&flash0>;

This is primary saying that both copies are in flash0.
Isn't it better to also via DT binding to support that it can be different 
physical devices?

> +		mdata-parts = "MDATA-Pri", "MDATA-Sec";

This is not clear to me what this is used for. Can you please explain what you 
want to use this for?


> +
> +		fwu-bank at 0 {
> +			id = <0>;

I though that @X is all the time associated with reg property.
It means maybe you wanted to use fwu-bank0 instead or switch id to reg.

The same applies below to fwuimage.

> +			label = "FIP-Bank0";
> +			fwu-image at 0 {
> +				id = <0>;
> +				offset = <0x0>;
> +				size = <0x400000>;
> +				uuid = "5a66a702-99fd-4fef-a392-c26e261a2828";
> +			};
> +		};
> +		fwu-bank at 1 {
> +			id = <1>;
> +			label = "FIP-Bank1";
> +			fwu-image at 0 {
> +				id = <0>;
> +				offset = <0x0>;
> +				size = <0x400000>;
> +				uuid = "a8f868a1-6e5c-4757-878d-ce63375ef2c0";
> +			};
> +		};
> +	};
> +...

I think it will be good to get review from Krzysztof Kozlowski or Rob to make 
sure that it will never change.

Thanks,
Michal