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

On 2/28/23 01:52, jassisinghbrar@gmail.com wrote:

From: Jassi Brar jaswinder.singh@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@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@linaro.org

• • Jassi Brar jaswinder.singh@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@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@0 {
  

• 		id = <0>;
  

• 		offset = <0x0>;
  

• 		size = <0x400000>;
  

• 		uuid = "5a66a702-99fd-4fef-a392-c26e261a2828";
  

• 	};
  

• };
  

• fwu-bank@1 {
  

• 	id = <1>;
  

• 	label = "FIP-Bank1";
  

• 	fwu-image@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