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

Jassi Brar jassisinghbrar at gmail.com
Tue Feb 28 16:27:13 CET 2023


On Tue, Feb 28, 2023 at 4:50 AM Michal Simek <michal.simek at amd.com> wrote:
> 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?
>
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 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.
>
Yes, fwu-bank0 and fwu-image0 seems better.

> > +                     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.
>
I will CC them.

cheers.


More information about the U-Boot mailing list