dt-bindings: firmware: arm,scmi: Restrict protocol child node properties

The SCMI protocol child nodes are missing any constraints on unknown properties. Specifically, either 'unevaluatedProperties' or 'additionalProperties' is needed. The current structure with a regex match for all child nodes doesn't work for this purpose, so let's move the common properties '$defs' entry which each specific protocol node can reference and set 'unevaluatedProperties: false'.

The SCMI protocol child nodes are missing any constraints on unknown properties. Specifically, either 'unevaluatedProperties' or 'additionalProperties' is needed. The current structure with a regex match for all child nodes doesn't work for this purpose, so let's move the common properties '$defs' entry which each specific protocol node can reference and set 'unevaluatedProperties: false'.

Signed-off-by Rob Herring <[email protected]>

---
 .../bindings/firmware/arm,scmi.yaml           | 43 ++++++++++++++-----
 1 file changed, 33 insertions(+), 10 deletions(-)

Documentation/devicetree/bindings/firmware/arm,scmi.yaml +31 -8

index 176796931a22..2f7c51c75e85 100644
@@ -100,7 +100,9 @@ properties:
       Channel specifier required when using OP-TEE transport.
 
   protocol@11:
-    type: object
+    $ref: '#/$defs/protocol-node'
+    unevaluatedProperties: false
+
     properties:
       reg:
         const: 0x11

@@ -112,7 +114,9 @@ properties:
       - '#power-domain-cells'
 
   protocol@13:
-    type: object
+    $ref: '#/$defs/protocol-node'
+    unevaluatedProperties: false
+
     properties:
       reg:
         const: 0x13

@@ -124,7 +128,9 @@ properties:
       - '#clock-cells'
 
   protocol@14:
-    type: object
+    $ref: '#/$defs/protocol-node'
+    unevaluatedProperties: false
+
     properties:
       reg:
         const: 0x14

@@ -136,7 +142,9 @@ properties:
       - '#clock-cells'
 
   protocol@15:
-    type: object
+    $ref: '#/$defs/protocol-node'
+    unevaluatedProperties: false
+
     properties:
       reg:
         const: 0x15

@@ -148,7 +156,9 @@ properties:
       - '#thermal-sensor-cells'
 
   protocol@16:
-    type: object
+    $ref: '#/$defs/protocol-node'
+    unevaluatedProperties: false
+
     properties:
       reg:
         const: 0x16

@@ -160,20 +170,31 @@ properties:
       - '#reset-cells'
 
   protocol@17:
-    type: object
+    $ref: '#/$defs/protocol-node'
+    unevaluatedProperties: false
+
     properties:
       reg:
         const: 0x17
 
       regulators:
         type: object
+        additionalProperties: false
         description:
           The list of all regulators provided by this SCMI controller.
 
+        properties:
+          '#address-cells':
+            const: 1
+
+          '#size-cells':
+            const: 0
+
         patternProperties:
-          '^regulators@[0-9a-f]+$':
+          '^regulator@[0-9a-f]+$':
             type: object
             $ref: "../regulator/regulator.yaml#"
+            unevaluatedProperties: false
 
             properties:
               reg:

@@ -184,15 +205,17 @@ properties:
               - reg
 
   protocol@18:
-    type: object
+    $ref: '#/$defs/protocol-node'
+    unevaluatedProperties: false
+
     properties:
       reg:

so now that the catch-all protocol@ patternProperty is gone in favour of the 'protocol-node' definition and $refs, does that mean that any current and future SCMI officially published protocol <N> has to be added to the above explicit protocol list, even though it does not have any special additional required property beside reg ? (like protocol@18 above...)

Quoted 7 lines · Cristian Marussi

so now that the catch-all protocol@ patternProperty is gone in favour of the 'protocol-node' definition and $refs, does that mean that any current and future SCMI officially published protocol <N> has to be added to the above explicit protocol list, even though it does not have any special additional required property beside reg ? (like protocol@18 above...)

If there are no consumers, should we just not add and deal with it entirely within the kernel. I know we rely today on presence of node before we initialise, but hey we have exception for system power protocol for other reasons, why not add this one too.

In short we shouldn't have to add a node if there are no consumers. It was one of the topic of discussion initially when SCMI binding was added and they exist only for the consumers otherwise we don't need it as everything is discoverable from the interface.

Quoted 7 lines · Cristian Marussi

so now that the catch-all protocol@ patternProperty is gone in favour of the 'protocol-node' definition and $refs, does that mean that any current and future SCMI officially published protocol <N> has to be added to the above explicit protocol list, even though it does not have any special additional required property beside reg ? (like protocol@18 above...)

If there are no consumers, should we just not add and deal with it entirely within the kernel. I know we rely today on presence of node before we initialise, but hey we have exception for system power protocol for other reasons, why not add this one too.

In short we shouldn't have to add a node if there are no consumers. It was one of the topic of discussion initially when SCMI binding was added and they exist only for the consumers otherwise we don't need it as everything is discoverable from the interface.

Quoted 7 lines · Cristian Marussi

so now that the catch-all protocol@ patternProperty is gone in favour of the 'protocol-node' definition and $refs, does that mean that any current and future SCMI officially published protocol <N> has to be added to the above explicit protocol list, even though it does not have any special additional required property beside reg ? (like protocol@18 above...)

If there are no consumers, should we just not add and deal with it entirely within the kernel. I know we rely today on presence of node before we initialise, but hey we have exception for system power protocol for other reasons, why not add this one too.

In short we shouldn't have to add a node if there are no consumers. It was one of the topic of discussion initially when SCMI binding was added and they exist only for the consumers otherwise we don't need it as everything is discoverable from the interface.

Quoted 14 lines · Sudeep Holla

Quoted 7 lines · Cristian Marussi

so now that the catch-all protocol@ patternProperty is gone in favour of the 'protocol-node' definition and $refs, does that mean that any current and future SCMI officially published protocol <N> has to be added to the above explicit protocol list, even though it does not have any special additional required property beside reg ? (like protocol@18 above...)

If there are no consumers, should we just not add and deal with it entirely within the kernel. I know we rely today on presence of node before we initialise, but hey we have exception for system power protocol for other reasons, why not add this one too.

Quoted 4 lines

In short we shouldn't have to add a node if there are no consumers. It was one of the topic of discussion initially when SCMI binding was added and they exist only for the consumers otherwise we don't need it as everything is discoverable from the interface.

It is fine for me the no-consumers/no-node argument (which anyway would require a few changes in the core init logic anyway to work this way...), BUT is it not that ANY protocol (even future-ones) does have, potentially, consumers indeed, since each protocol-node can potentially have a dedicated channel and related DT channel-descriptor ? (when multiple channels are allowed by the transport)

I mean, as an example, you dont strictly need protos 0x18/0x12 nodes for anything (if we patch the core init as said) UNLESS you want to dedicate a channel to those protocols; so I'm just checking here if these kind of scenarios will still be allowed with this binding change, or if I am missing something.

Quoted 27 lines · Cristian Marussi

Quoted 14 lines · Sudeep Holla

Quoted 7 lines · Cristian Marussi

so now that the catch-all protocol@ patternProperty is gone in favour of the 'protocol-node' definition and $refs, does that mean that any current and future SCMI officially published protocol <N> has to be added to the above explicit protocol list, even though it does not have any special additional required property beside reg ? (like protocol@18 above...)

If there are no consumers, should we just not add and deal with it entirely within the kernel. I know we rely today on presence of node before we initialise, but hey we have exception for system power protocol for other reasons, why not add this one too.

Quoted 4 lines

In short we shouldn't have to add a node if there are no consumers. It was one of the topic of discussion initially when SCMI binding was added and they exist only for the consumers otherwise we don't need it as everything is discoverable from the interface.

It is fine for me the no-consumers/no-node argument (which anyway would require a few changes in the core init logic anyway to work this way...), BUT is it not that ANY protocol (even future-ones) does have, potentially, consumers indeed, since each protocol-node can potentially have a dedicated channel and related DT channel-descriptor ? (when multiple channels are allowed by the transport)

Quoted 5 lines

I mean, as an example, you dont strictly need protos 0x18/0x12 nodes for anything (if we patch the core init as said) UNLESS you want to dedicate a channel to those protocols; so I'm just checking here if these kind of scenarios will still be allowed with this binding change, or if I am missing something.

Ah, good point on the transport information. Yes we will need a node if a protocol has a dedicated transport. No one has used so far other than Juno perf, but we never know. We can always extended the bindings if needed.

Sorry for missing the dedicated transport part.

Quoted 35 lines · Sudeep Holla

Quoted 32 lines · Cristian Marussi

Quoted 14 lines · Sudeep Holla

Quoted 7 lines · Cristian Marussi

so now that the catch-all protocol@ patternProperty is gone in favour of the 'protocol-node' definition and $refs, does that mean that any current and future SCMI officially published protocol <N> has to be added to the above explicit protocol list, even though it does not have any special additional required property beside reg ? (like protocol@18 above...)

If there are no consumers, should we just not add and deal with it entirely within the kernel. I know we rely today on presence of node before we initialise, but hey we have exception for system power protocol for other reasons, why not add this one too.

Quoted 4 lines

In short we shouldn't have to add a node if there are no consumers. It was one of the topic of discussion initially when SCMI binding was added and they exist only for the consumers otherwise we don't need it as everything is discoverable from the interface.

It is fine for me the no-consumers/no-node argument (which anyway would require a few changes in the core init logic anyway to work this way...), BUT is it not that ANY protocol (even future-ones) does have, potentially, consumers indeed, since each protocol-node can potentially have a dedicated channel and related DT channel-descriptor ? (when multiple channels are allowed by the transport)

I mean, as an example, you dont strictly need protos 0x18/0x12 nodes for anything (if we patch the core init as said) UNLESS you want to dedicate a channel to those protocols; so I'm just checking here if these kind of scenarios will still be allowed with this binding change, or if I am missing something.

Quoted 6 lines

Ah, good point on the transport information. Yes we will need a node if a protocol has a dedicated transport. No one has used so far other than Juno perf, but we never know. We can always extended the bindings if needed.

Sorry for missing the dedicated transport part.

So I need to add back 'protocol@.*' or not?

Quoted 36 lines · Rob Herring

Quoted 35 lines · Sudeep Holla

Quoted 32 lines · Cristian Marussi

Quoted 14 lines · Sudeep Holla

Quoted 7 lines · Cristian Marussi

so now that the catch-all protocol@ patternProperty is gone in favour of the 'protocol-node' definition and $refs, does that mean that any current and future SCMI officially published protocol <N> has to be added to the above explicit protocol list, even though it does not have any special additional required property beside reg ? (like protocol@18 above...)

If there are no consumers, should we just not add and deal with it entirely within the kernel. I know we rely today on presence of node before we initialise, but hey we have exception for system power protocol for other reasons, why not add this one too.

Quoted 4 lines

In short we shouldn't have to add a node if there are no consumers. It was one of the topic of discussion initially when SCMI binding was added and they exist only for the consumers otherwise we don't need it as everything is discoverable from the interface.

It is fine for me the no-consumers/no-node argument (which anyway would require a few changes in the core init logic anyway to work this way...), BUT is it not that ANY protocol (even future-ones) does have, potentially, consumers indeed, since each protocol-node can potentially have a dedicated channel and related DT channel-descriptor ? (when multiple channels are allowed by the transport)

I mean, as an example, you dont strictly need protos 0x18/0x12 nodes for anything (if we patch the core init as said) UNLESS you want to dedicate a channel to those protocols; so I'm just checking here if these kind of scenarios will still be allowed with this binding change, or if I am missing something.

Quoted 8 lines

Quoted 6 lines

Ah, good point on the transport information. Yes we will need a node if a protocol has a dedicated transport. No one has used so far other than Juno perf, but we never know. We can always extended the bindings if needed.

Sorry for missing the dedicated transport part.

So I need to add back 'protocol@.*' or not?

IMO it is better to know what exactly gets added under each of these protocol sub-nodes and so better to have entry specific to each known protocols. I liked that fact with this change as I have seen some crazy vendor extensions adding all sorts of non-sense defining some vendor protocol. For example [1], in which case we can catch those better than existing schema which matches all. So let's not add protocol@.* if possible or until that becomes the only cleaner way to maintain this.

TBC, 'protocol@.*' would not allow anything but the properties defined in the /$defs/protocol-node. So [1] would throw errors without a schema addition.

We should either do that along with dropping 'protocol@18' or we keep protocol 0x18 node and add all other providerless protocols. I don't think we need the latter to just check unit-address vs. reg.

Hi Rob,

Quoted 4 lines · Rob Herring

TBC, 'protocol@.*' would not allow anything but the properties defined in the /$defs/protocol-node. So [1] would throw errors without a schema addition.

Right I clearly missed that, somehow I assumed it would allow.

Quoted 3 lines

We should either do that along with dropping 'protocol@18' or we keep protocol 0x18 node and add all other providerless protocols. I don't think we need the latter to just check unit-address vs. reg.

I only argument today it to allow protocol specific transport. So we could delay addition of it until someone needs that way. So far we haven't seen anyone using it other than performance(even that is not needed with the introduction of fast channels that are auto discoverable in relatively newer versions of the spec).