On Wed, Oct 9, 2024 at 7:38 PM Rob Herring <robh@xxxxxxxxxx> wrote:
>
> On Wed, Oct 09, 2024 at 02:50:44PM -0700, Sam Edwards wrote:
> > The BCM4908 partition "parser" is really just a fixed partitions table,
> > with a special partition compatible (`brcm,bcm4908-firmware`) that
> > automatically labels the partition as "firmware" or "backup" depending
> > on what CFE is communicating as the selected active partition.
> >
> > The bcm4908-partitions schema is currently too restrictive, requiring
> > that all child nodes use this special compatible or none at all. This
> > not only contracits what is allowed by the "parser" but also causes
> > warnings for an existing file ("bcm4908-asus-gt-ac5300.dts").
> >
> > Modify the schema to be strict only for child partitions that use the
> > -firmware compatible. Also update the child name regex to agree with
> > fixed-partitions, so that these differences apply consistently.
> >
> > Signed-off-by: Sam Edwards <CFSworks@xxxxxxxxx>
> > ---
> > .../mtd/partitions/brcm,bcm4908-partitions.yaml | 17 +++++++++++------
> > 1 file changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/partitions/brcm,bcm4908-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/brcm,bcm4908-partitions.yaml
> > index 94f0742b375c..aed37922a5fc 100644
> > --- a/Documentation/devicetree/bindings/mtd/partitions/brcm,bcm4908-partitions.yaml
> > +++ b/Documentation/devicetree/bindings/mtd/partitions/brcm,bcm4908-partitions.yaml
> > @@ -30,12 +30,17 @@ properties:
> > enum: [ 1, 2 ]
> >
> > patternProperties:
> > - "^partition@[0-9a-f]+$":
> > - $ref: partition.yaml#
> > - properties:
> > - compatible:
> > - const: brcm,bcm4908-firmware
> > - unevaluatedProperties: false
> > + "^partition(-.+|@[0-9a-f]+)$":
> > + type: object
> > + if:
> > + properties:
> > + compatible:
> > + const: brcm,bcm4908-firmware
Hi Rob,
Thanks for your attention on this one! This is an odd case where I'm
not sure what else could be done, and I'm a newbie to modifying DT
schemata, so I'm glad to be receiving your feedback. :)
> What schema applies to the node if this is not true? That needs to be
> addressed. You should be able to use oneOf here rather than if/then
> schema.
The schema that should apply "in general" is partition.yaml, though
moving the '$ref:' outside of 'then:' made the 'unevaluatedProperties:
false' disallow everything. The if block here is just trying to
memorialize bcm4908-firmware as a valid compatible at this level, and
(ideally) disallow unevaluated properties if so.
Could that be done with a oneOf? I would think if one of the arms of
the oneOf was "unmodified $partition.yaml, unevaluated allowed" it
would always match, rendering the "unevaluated disallowed when
compatible=bcm4908-firmware" arm unused.
Best,
Sam
>
> > + then:
> > + $ref: partition.yaml#
> > + properties:
> > + compatible: true
> > + unevaluatedProperties: false
> >
> > required:
> > - "#address-cells"
> > --
> > 2.44.2
> >
