Hi Krzysztof,
miquel.raynal@xxxxxxxxxxx wrote on Tue, 6 Jun 2023 12:57:24 +0200:
> Hi Krzysztof,
>
> krzysztof.kozlowski@xxxxxxxxxx wrote on Tue, 6 Jun 2023 12:40:45 +0200:
>
> > On 06/06/2023 12:37, Krzysztof Kozlowski wrote:
> > > On 06/06/2023 12:28, Miquel Raynal wrote:
> > >> Hi Krzysztof,
> > >>
> > >> krzysztof.kozlowski@xxxxxxxxxx wrote on Tue, 6 Jun 2023 10:44:34 +0200:
> > >>
> > >>> On 06/06/2023 09:48, Miquel Raynal wrote:
> > >>>>>>>>>> + it (otherwise it is harmless).
> > >>>>>>>>>> + $ref: /schemas/types.yaml#/definitions/flag
> > >>>>>>>>>> + deprecated: true
> > >>>>>>>>>> +
> > >>>>>>>>>> + additionalProperties: false
> > >>>>>>>>> unevaluatedProperties: false
> > >>>>>>>> It was hiding by '"^nand@[0-3]$":'. Should I move it here?
> > >>>>>>> You cannot have both additionalProps and unevaluatedProps at the same
> > >>>>>>> time, so we do not talk about same thing or this was never working?
> > >>>>>>
> > >>>>>> Hmm, I'm a little confused then. At various times I've been told to
> > >>>>>> put 'additionalProperties: false' or 'unevaluatedProperties: false'
> > >>>>>> (although never at the same time). I'm not sure when to use one or the
> > >>>>>> other.
> > >>>>>>
> > >>>>>> From what I've been able to glean 'additionalProperties: true'
> > >>>>>> indicates that the node is expected to have child nodes defined in a
> > >>>>>> different schema so I would have thought 'additionalProperties: false'
> > >>>>>> would be appropriate for a schema covering a leaf node.
> > >>>>>> 'unevaluatedProperties: false' seems to enable stricter checking which
> > >>>>>> makes sense when all the properties are described in the schema.
> > >>>>>
> > >>>>> So I think this might be the problem. If I look at qcom,nandc.yaml or
> > >>>>> ingenic,nand.yaml which both have a partitions property in their
> > >>>>> example. Neither have 'unevaluatedProperties: false' on the nand@...
> > >>>>> subnode. If I add it sure enough I start getting complaints about the
> > >>>>> 'partitions' node being unexpected.
> > >>>>
> > >>>> Sorry if that was unclear, I think the whole logic around the yaml
> > >>>> files is to progressively constrain the descriptions, schema after
> > >>>> schema. IOW, in the marvell binding you should set
> > >>>> unevaluatedProperties: false for the NAND controller. What is inside
> > >>>> (NAND chips, partition container, partition parsers, "mtd" properties,
> > >>>> etc) will be handled by other files. Of course you can constrain a bit
> > >>>> what can/cannot be used inside these subnodes, but I think you don't
> > >>>> need to set unevaluatedProperties in these subnodes (the NAND chip in
> > >>>> this case, or even the partitions) because you already reference
> > >>>> nand-controller.yaml which references nand-chip.yaml, mtd.yaml,
> > >>>> partitions.yaml, etc. *they* will make the generic checks and hopefully
> > >>>> apply stricter checks, when deemed relevant.
> > >>>
> > >>> No, neither nand-controller.yaml nor nand-chip.yaml limit the properties
> > >>> in this context, so each device schema must have unevaluatedProperties:
> > >>> false, for which I asked few emails ago.
> > >>
> > >> The controller description shall be guarded by unevaluatedProperties:
> > >> false, we agree. Do you mean the nand chip description in each nand
> > >> controller binding should also include it at its own level? Because
> > >> that is not what we enforced so far IIRC. I am totally fine doing so
> > >> starting from now on if this is a new requirement (which makes sense).
> > >>
> > >> If yes, then it means we would need to list *all* the nand
> > >> chip properties in each schema, which clearly involves a lot of
> > >> duplication as you would need to define all types of partitions,
> > >> partition parsers, generic properties, etc in order for the examples to
> > >> pass all the checks. Only the properties like pinctrl-* would not need
> > >> to be listed I guess.
> > >
> > > Yes, this is what should be done. Each node should have either
> >
> > Eh, no, I responded in wrong part of message. My yes was for:
> >
> > " Do you mean the nand chip description in each nand
> > controller binding should also include it at its own level?"
>
> Clear.
>
> >
> > Now for actual paragraph:
> >
> > "If yes, then it means we would need to list *all* the nand chip
> > properties in each schema,"
> >
> > No, why? I don't understand. Use the same pattern as all other bindings,
> > this is not special. Absolutely all have the same behavior, e.g.
> > mentioned leds. You finish with unevaluatedProps and you're done, which
> > is what I wrote here long, long time ago.
>
> Maybe because so far we did not bother referencing another schema in
> the NAND chip nodes? For your hint to work I guess we should have, in
> each controller binding, something along:
>
> patternProperties:
> "^nand@[a-f0-9]$":
> type: object
> + $ref: nand-chip.yaml#
> properties:
>
> If yes, please ignore the series sent aside, I will work on it again
> and send a v2.
Actually I already see a problem, let's the ingenic,nand.yaml example.
The goal, IIUC, is to do:
patternProperties:
"^nand@[a-f0-9]$":
type: object
+ $ref: nand-chip.yaml
properties:
...
+ unevaluatedProperties: false
The example in this file uses a property, nand-on-flash-bbt, which is
described inside nand-controller.yaml instead of nand-chip.yaml.
Indeed, the former actually describes many properties which are a bit
more controller related than chip related. With the above description,
the example fails because nand-on-flash-bbt is not allowed (it is not
listed in nand-chip.yaml).
How would you proceed in this case?
Maybe I could move all the NAND chip properties which are somehow
related to NAND controllers (and defined in nand-controller.yaml) in a
dedicated file and reference it from nand-chip.yaml? Any other idea is
welcome.
Thanks, Miquèl
