Hi Rob and Vladimir,

On Thu, Oct 27, 2022 at 04:25:53AM +0300, Vladimir Oltean wrote:

Hi Rob,

On Tue, Oct 25, 2022 at 04:21:14PM -0500, Rob Herring wrote:

quoted

On Mon, Oct 24, 2022 at 10:03:51PM -0700, Colin Foster wrote:

quoted

The dsa.yaml binding contains duplicated bindings for address and size
cells, as well as the reference to dsa-port.yaml. Instead of duplicating
this information, remove the reference to dsa-port.yaml and include the
full reference to dsa.yaml.

I don't think this works without further restructuring. Essentially, 
'unevaluatedProperties' on works on a single level. So every level has 
to define all properties at that level either directly in 
properties/patternProperties or within a $ref.

See how graph.yaml is structured and referenced for an example how this 
has to work.

quoted

@@ -104,8 +98,6 @@ patternProperties:
               SGMII on the QCA8337, it is advised to set this unless a communication
               issue is observed.
 
-        unevaluatedProperties: false
-

Dropping this means any undefined properties in port nodes won't be an 
error. Once I fix all the issues related to these missing, there will be 
a meta-schema checking for this (this could be one I fixed already).

I may be misreading, but here, "unevaluatedProperties: false" from dsa.yaml
(under patternProperties: "^(ethernet-)?port@[0-9]+$":) is on the same
level as the "unevaluatedProperties: false" that Colin is deleting.

In fact, I believe that it is precisely due to the "unevaluatedProperties: false"
from dsa.yaml that this is causing a failure now:

net/dsa/qca8k.example.dtb: switch@10: ports:port@6: Unevaluated properties are not allowed ('qca,sgmii-rxclk-falling-edge' was unexpected)

Could you please explain why is the 'qca,sgmii-rxclk-falling-edge'
property not evaluated from the perspective of dsa.yaml in the example?
It's a head scratcher to me.

May it have something to do with the fact that Colin's addition:

$ref: "dsa.yaml#"

is not expressed as:

allOf:
  - $ref: "dsa.yaml#"

?

Looking into documentation (I promise I did some reading / research to
try to get a stronger understanding of the documentation yaml) I came
across the history of ethernet-controller.yaml which suggests to me that
the pattern:

allOf:
  - $ref: 

is frowned upon
commit 3d21a4609335: ("dt-bindings: Remove cases of 'allOf' containing a
'$ref'")

I do have a knack for misinterpreting data, but I read that as:
allOf:
  - $ref:
shouldn't be used unless there's more than one list entry.


All that aside, I did upgrade from 2022.5 to 2022.9 just now and do see
these dtschema errors now. I'll be sure to use this before resubmitting.

If yes, can you explain exactly what is the difference with respect to
unevaluatedProperties?

quoted

quoted

 oneOf:
   - required:
       - ports

@@ -116,7 +108,7 @@ required:
   - compatible
   - reg
 
-additionalProperties: true

This should certainly be changed though. We should only have 'true' for 
incomplete collections of properties. IOW, for common bindings.

That makes a lot of sense - and helps me understand why I had so much
trouble understanding why it originally was "additionalProperties: true"


I'll obviously take another look at this. The nxp,sja1105.yaml seemed to
be most akin to what the qca8k.yaml needed to be - that is "take
dsa.yaml and add a couple extra properties to the ports nodes". But
there's always subleties.

quoted

quoted

+unevaluatedProperties: false