On 25/08/2022 16:13, Rob Herring wrote: > On Thu, Aug 25, 2022 at 3:23 AM Krzysztof Kozlowski > <krzysztof.kozlowski@xxxxxxxxxx> wrote: >> >> On 23/08/2022 17:56, Rob Herring wrote: >>> In order to ensure only documented properties are present, node schemas >>> must have unevaluatedProperties or additionalProperties set to false >>> (typically). >>> >>> Signed-off-by: Rob Herring <robh@xxxxxxxxxx> >>> --- >>> .../devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml | 1 + >>> .../devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml | 1 + >>> .../devicetree/bindings/remoteproc/qcom,sc7280-wpss-pil.yaml | 1 + >>> 3 files changed, 3 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml >>> index e76c861165dd..e4a7da8020f4 100644 >>> --- a/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml >>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml >>> @@ -140,6 +140,7 @@ properties: >>> >>> glink-edge: >>> $ref: qcom,glink-edge.yaml# >>> + unevaluatedProperties: false >> >> Is it actually needed? The qcom,glink-edge.yaml has >> additionalProperties:false, so I expect it to complain if anything >> appears here. > > Perhaps not, but I'm trying to come up with a meta-schema to check > these though I'm not sure I can get to no warnings which is how I > found all these cases. The main remaining warnings are bus child node > pattern schemas which can perhaps be handled with > 'additionalProperties: true'. The rule I have says if properties or > patternProperties is present then unevaluatedProperties or > additionalProperties must be. To handle this case, I think we'd have > to walk the $ref and check it. > > Anyways, we can hold off on this one until when and if there's a > meta-schema in place. For me adding unevaluatedProp:false everywhere with $ref is okay and it makes the code easier to read - no need to dive into referenced schema to remember if it allows or does not allow additional properties. It is also a safer choice if referenced schema forgot to set additionalProp:false. However if referenced schema has additionalProp:false, then unevaluatedProp:false here is redundant and question is whether the redundancy is worth additional readability/obviousness. To me, unevaluatedProp:false here would during review save time - no need to jump into referenced schema to check what is there. If we make it a rule / coding convention, then I am in. Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> Best regards, Krzysztof
