| Message ID | 20230228005218.1635781-1-jassisinghbrar@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | FWU: Handle meta-data in common code | expand |
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
On Tue, Feb 28, 2023 at 4:50 AM Michal Simek <michal.simek@amd.com> wrote: > 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? > That could be done, but currently the FWU code assumes all the artifacts are in the same storage. Same for GPT based layout, do we go change that as well? So I kept it simple for now. > > + 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? > These are the two required meta-data partitions, primary and secondary (backup of primary). Meta-Data partitions store information about the current setup of banks/images. > > + > > + 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. > Yes, fwu-bank0 and fwu-image0 seems better. > > + 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. > I will CC them. cheers.
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>; + mdata-parts = "MDATA-Pri", "MDATA-Sec"; + + fwu-bank@0 { + id = <0>; + 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"; + }; + }; + }; +...