Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme

Hi Krzysztof,

krzysztof.kozlowski@linaro.org wrote on Tue, 6 Jun 2023 12:40:45 +0200:

On 06/06/2023 12:37, Krzysztof Kozlowski wrote:

quoted

On 06/06/2023 12:28, Miquel Raynal wrote:  

quoted

Hi Krzysztof,

krzysztof.kozlowski@linaro.org wrote on Tue, 6 Jun 2023 10:44:34 +0200:
 

quoted

On 06/06/2023 09:48, Miquel Raynal wrote:  

quoted

quoted

quoted

quoted

quoted

quoted

quoted

+          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.

Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel